[lldb][cherry-pick] Various fixes to definition DIE parsing #9788
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvm#120218) With all the recent versions of Clang that I tested, ObjC forward declarations like ``` @Class ForwardObjcClass; ``` don't emit the kind of DWARF that this workaround was put in place for. Also, zero-sized structures are valid in C (and thus Objective-C), so this workaround makes things confusing to reason about when mixing the two languages. This workaround has been in place for at least a decade, and given that recent compilers don't produce this anymore, we think it's a good time to remove it. (cherry picked from commit 794cd81)
The `llvm-gcc` front-end has been EOL'd at least since 2011 (based on some `git` archeology). And Clang/LLVM has been removing references to it ever since. This patch removes the remaining references to it from LLDB. One benefit of this is that it will allow us to remove the code checking for `DW_AT_decl_file_attributes_are_invalid` and `Supports_DW_AT_APPLE_objc_complete_type`. (cherry picked from commit e0a79ee)
…lete_type and DW_AT_decl_file_attributes_are_invalid (llvm#120226) Depends on llvm#120225 With `llvm-gcc` support being removed from LLDB, these APIs are now trivial and can be removed too. (cherry picked from commit f176388)
…ecordType (llvm#120456) Became unused since the recent llvm#110648 (cherry picked from commit 6fd267d)
…ymbolFileDWARFDebugMap (llvm#120569) The problem here manifests as follows: 1. We are stopped in main.o, so the first `ParseTypeFromDWARF` on `FooImpl<char>` gets called on `main.o`'s SymbolFile. This adds a mapping from *declaration die* -> `TypeSP` into `main.o`'s `GetDIEToType` map. 2. We then `CompleteType(FooImpl<char>)`. Depending on the order of entries in the debug-map, this might call `CompleteType` on `lib.o`'s SymbolFile. In which case, `GetDIEToType().lookup(decl_die)` will return a `nullptr`. This is already a bit iffy because some of the surrounding code assumes we don't call `CompleteTypeFromDWARF` with a `nullptr` `Type*`. E.g., `CompleteEnumType` blindly dereferences it (though enums will never encounter this because their definition is fetched in ParseEnum, unlike for structures). 3. While in `CompleteTypeFromDWARF`, we call `ParseTypeFromDWARF` again. This will parse the member function `FooImpl::Create` and its return type which is a typedef to `FooImpl*`. But now we're inside `lib.o`'s SymbolFile, so we call it on the definition DIE. In step (2) we just inserted a `nullptr` into `GetDIEToType` for the definition DIE, so we trivially return a `nullptr` from `ParseTypeFromDWARF`. Instead of reporting back this parse failure to the user LLDB trucks on and marks `FooImpl::Ref` to be `void*`. This test-case will trigger an assert in `TypeSystemClang::VerifyDecl` even if we just `frame var` (but only in debug-builds). In release builds where this function is a no-op, we'll create an incorrect Clang AST node for the `Ref` typedef. The proposed fix here is to share the `GetDIEToType` map between SymbolFiles if a debug-map exists. **Alternatives considered** * Check the `GetDIEToType` map of the `SymbolFile` that the declaration DIE belongs to. The assumption here being that if we called `ParseTypeFromDWARF` on a declaration, the `GetDIEToType` map that the result was inserted into was the one on that DIE's SymbolFile. This was the first version of this patch, but that felt like a weaker version sharing the map. It complicates the code in `CompleteType` and is less consistent with the other bookkeeping structures we already share between SymbolFiles * Return from `SymbolFileDWARF::CompleteType` if there is no type in the current `GetDIEToType`. Then `SymbolFileDWARFDebugMap::CompleteType` could continue to the next `SymbolFile` which does own the type. But that didn't quite work because we remove the `GetForwardCompilerTypeToDie` entry in `SymbolFile::CompleteType`, which `SymbolFileDWARFDebugMap::CompleteType` relies upon for iterating (cherry picked from commit 28d1490)
…queDWARFASTTypeList in C++ (llvm#120809) This addresses an issue encountered when investigating llvm#120569. In `ParseTypeFromDWARF`, we insert the parsed type into `UniqueDWARFASTTypeMap` using the typename and `Declaration` obtained with `GetUniqueTypeNameAndDeclaration`. For C++ `GetUniqueTypeNameAndDeclaration` will zero the `Declaration` info (presumably because forward declaration may not have it, and for C++, ODR means we can rely on fully qualified typenames for uniqueing). When we then called `CompleteType`, we would first `MapDeclDIEToDefDIE`, updating the `UniqueDWARFASTType` we inserted previously with the `Declaration` of the definition DIE (without zeroing it). We would then call into `ParseTypeFromDWARF` for the same type again, and search the `UniqueDWARFASTTypeMap`. But we do the search using a zeroed declaration we get from `GetUniqueTypeNameAndDeclaration`. This particular example was fixed by llvm#120569 but this still feels a little inconsistent. I'm not entirely sure we can get into that situation after that patch anymore. So the only test-case I could come up with was the unit-test which calls `MapDeclDIEToDefDIE` directly. (cherry picked from commit c660b28)
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The last two commits address bugs related to the lazy definition DIE parsing (in the presence of debug-maps). The remaining commits are NFC cleanups around the area.
rdar://141002517