diff options
| author | Ricky Zhou <ricky@rzhou.org> | 2024-01-18 10:55:30 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-01-18 10:55:30 -0800 |
| commit | 1522333c3c7e458b2e3cb6d175a56741e23f4007 (patch) | |
| tree | 7fb4a102889f41a8010618cb3e15d3cf860ddabc /llvm/lib/Bitcode/Reader/MetadataLoader.cpp | |
| parent | 0ac992e0ada60c670498ac3276150e1632ab0039 (diff) | |
| download | llvm-1522333c3c7e458b2e3cb6d175a56741e23f4007.zip llvm-1522333c3c7e458b2e3cb6d175a56741e23f4007.tar.gz llvm-1522333c3c7e458b2e3cb6d175a56741e23f4007.tar.bz2 | |
[ThinLTO][DebugInfo] Emit full type definitions when importing anonymous types. (#78461)
This fixes some cases of missing debuginfo caused by an interaction
between:
https://github.com/llvm/llvm-project/commit/f0d66559ea345db4b2116cae044aaf3399d7e829,
which drops the identifier from a DICompositeType in the module
containing its
vtable.
and
https://github.com/llvm/llvm-project/commit/a61f5e379675732666744bcba25efbc9922e016a,
which causes ThinLTO to import composite types as declarations when they
have
an identifier.
If a virtual class's DICompositeType has no identifier due to the first
change,
and contains a nested anonymous type which does have an identifier, then
the
second change can cause ThinLTO to output the classes's DICompositeType
as a
type definition that links to a non-defining declaration for the nested
type.
Since the nested anonyous type does not have a name, debuggers are
unable to
find the definition for the declaration.
Repro case:
```
cat > a.h <<EOF
class A {
public:
A();
virtual ~A();
private:
union {
int val;
};
};
EOF
cat > a.cc <<EOF
#include "a.h"
A::A() { asm(""); }
A::~A() {}
EOF
cat > main.cc <<EOF
#include "a.h"
int main(int argc, char **argv) {
A a;
return 0;
}
EOF
clang++ -O2 -g -flto=thin -mllvm -force-import-all main.cc a.cc
gdb ./a.out -batch -ex 'pt /rmt A'
```
The gdb command outputs:
```
type = class A {
private:
union {
<incomplete type>
};
}
```
and dwarfdump -i a.out shows a DW_TAG_class_type for A with an
incomplete union
type (note that there is also a duplicate entry with the full union type
that
comes after).
```
< 1><0x0000001e> DW_TAG_class_type
DW_AT_containing_type <0x0000001e>
DW_AT_calling_convention DW_CC_pass_by_reference
DW_AT_name (indexed string: 0x00000007)A
DW_AT_byte_size 0x00000010
DW_AT_decl_file 0x00000001 /path/to/./a.h
DW_AT_decl_line 0x00000001
...
< 2><0x0000002f> DW_TAG_member
DW_AT_type <0x00000037>
DW_AT_decl_file 0x00000001 /path/to/./a.h
DW_AT_decl_line 0x00000007
DW_AT_data_member_location 8
< 2><0x00000037> DW_TAG_union_type
DW_AT_export_symbols yes(1)
DW_AT_calling_convention DW_CC_pass_by_value
DW_AT_declaration yes(1)
```
This change works around this by making ThinLTO always import full
definitions
for anonymous types.
Diffstat (limited to 'llvm/lib/Bitcode/Reader/MetadataLoader.cpp')
| -rw-r--r-- | llvm/lib/Bitcode/Reader/MetadataLoader.cpp | 37 |
1 files changed, 21 insertions, 16 deletions
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp index 910e9748..770eb83 100644 --- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp +++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp @@ -1615,27 +1615,32 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata( Metadata *Annotations = nullptr; auto *Identifier = getMDString(Record[15]); // If this module is being parsed so that it can be ThinLTO imported - // into another module, composite types only need to be imported - // as type declarations (unless full type definitions requested). - // Create type declarations up front to save memory. Also, buildODRType - // handles the case where this is type ODRed with a definition needed - // by the importing module, in which case the existing definition is - // used. - if (IsImporting && !ImportFullTypeDefinitions && Identifier && + // into another module, composite types only need to be imported as + // type declarations (unless full type definitions are requested). + // Create type declarations up front to save memory. This is only + // done for types which have an Identifier, and are therefore + // subject to the ODR. + // + // buildODRType handles the case where this is type ODRed with a + // definition needed by the importing module, in which case the + // existing definition is used. + // + // We always import full definitions for anonymous composite types, + // as without a name, debuggers cannot easily resolve a declaration + // to its definition. + if (IsImporting && !ImportFullTypeDefinitions && Identifier && Name && (Tag == dwarf::DW_TAG_enumeration_type || Tag == dwarf::DW_TAG_class_type || Tag == dwarf::DW_TAG_structure_type || Tag == dwarf::DW_TAG_union_type)) { Flags = Flags | DINode::FlagFwdDecl; - if (Name) { - // This is a hack around preserving template parameters for simplified - // template names - it should probably be replaced with a - // DICompositeType flag specifying whether template parameters are - // required on declarations of this type. - StringRef NameStr = Name->getString(); - if (!NameStr.contains('<') || NameStr.starts_with("_STN|")) - TemplateParams = getMDOrNull(Record[14]); - } + // This is a hack around preserving template parameters for simplified + // template names - it should probably be replaced with a + // DICompositeType flag specifying whether template parameters are + // required on declarations of this type. + StringRef NameStr = Name->getString(); + if (!NameStr.contains('<') || NameStr.starts_with("_STN|")) + TemplateParams = getMDOrNull(Record[14]); } else { BaseType = getDITypeRefOrNull(Record[6]); OffsetInBits = Record[9]; |
