[Lldb-commits] [lldb] 2d89a3b - [lldb] Forcefully complete a type when adding nested classes
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 17 02:09:22 PDT 2020
Author: Pavel Labath
Date: 2020-08-17T11:09:13+02:00
New Revision: 2d89a3ba121b96a4af9aecaf52205eab200394c3
URL: https://github.com/llvm/llvm-project/commit/2d89a3ba121b96a4af9aecaf52205eab200394c3
DIFF: https://github.com/llvm/llvm-project/commit/2d89a3ba121b96a4af9aecaf52205eab200394c3.diff
LOG: [lldb] Forcefully complete a type when adding nested classes
With -flimit-debug-info, we can run into cases when we only have a class
as a declaration, but we do have a definition of a nested class. In this
case, clang will hit an assertion when adding a member to an incomplete
type (but only if it's adding a c++ class, and not C struct).
It turns out we already had code to handle a similar situation arising
in the -gmodules scenario. This extends the code to handle
-flimit-debug-info as well, and reorganizes bits of other code handling
completion of types to move functions doing similar things closer
together.
Differential Revision: https://reviews.llvm.org/D85968
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 7e3628504727..1344523522d3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -230,31 +230,73 @@ TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
return type_sp;
}
-static void CompleteExternalTagDeclType(TypeSystemClang &ast,
- ClangASTImporter &ast_importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+static void ForcefullyCompleteType(CompilerType type) {
+ bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
+ lldbassert(started && "Unable to start a class type definition.");
+ TypeSystemClang::CompleteTagDeclarationDefinition(type);
+ const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
+ auto &ts = llvm::cast<TypeSystemClang>(*type.GetTypeSystem());
+ ts.GetMetadata(td)->SetIsForcefullyCompleted();
+}
+
+/// Complete a type from debug info, or mark it as forcefully completed if
+/// there is no definition of the type in the current Module. Call this function
+/// in contexts where the usual C++ rules require a type to be complete (base
+/// class, member, etc.).
+static void RequireCompleteType(CompilerType type) {
+ // Technically, enums can be incomplete too, but we don't handle those as they
+ // are emitted even under -flimit-debug-info.
+ if (!TypeSystemClang::IsCXXClassType(type))
+ return;
+
+ if (type.GetCompleteType())
+ return;
+
+ // No complete definition in this module. Mark the class as complete to
+ // satisfy local ast invariants, but make a note of the fact that
+ // it is not _really_ complete so we can later search for a definition in a
+ //
diff erent module.
+ // Since we provide layout assistance, layouts of types containing this class
+ // will be correct even if we are not able to find the definition elsewhere.
+ ForcefullyCompleteType(type);
+}
+
+/// This function serves a similar purpose as RequireCompleteType above, but it
+/// avoids completing the type if it is not immediately necessary. It only
+/// ensures we _can_ complete the type later.
+static void PrepareContextToReceiveMembers(TypeSystemClang &ast,
+ ClangASTImporter &ast_importer,
+ clang::DeclContext *decl_ctx,
+ DWARFDIE die,
+ const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast<clang::TagDecl>(decl_ctx);
if (!tag_decl_ctx)
+ return; // Non-tag context are always ready.
+
+ // We have already completed the type, or we have found its definition and are
+ // ready to complete it later (cf. ParseStructureLikeDIE).
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
return;
+ // We reach this point of the tag was present in the debug info as a
+ // declaration only. If it was imported from another AST context (in the
+ // gmodules case), we can complete the type by doing a full import.
+
// If this type was not imported from an external AST, there's nothing to do.
CompilerType type = ast.GetTypeForDecl(tag_decl_ctx);
- if (!type || !ast_importer.CanImport(type))
- return;
-
- auto qual_type = ClangUtil::GetQualType(type);
- if (!ast_importer.RequireCompleteType(qual_type)) {
+ if (type && ast_importer.CanImport(type)) {
+ auto qual_type = ClangUtil::GetQualType(type);
+ if (ast_importer.RequireCompleteType(qual_type))
+ return;
die.GetDWARF()->GetObjectFile()->GetModule()->ReportError(
"Unable to complete the Decl context for DIE '%s' at offset "
"0x%8.8x.\nPlease file a bug report.",
type_name_cstr ? type_name_cstr : "", die.GetOffset());
- // We need to make the type look complete otherwise, we might crash in
- // Clang when adding children.
- if (TypeSystemClang::StartTagDeclarationDefinition(type))
- TypeSystemClang::CompleteTagDeclarationDefinition(type);
}
+
+ // We don't have a type definition and/or the import failed. We must
+ // forcefully complete the type to avoid crashes.
+ ForcefullyCompleteType(type);
}
ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) {
@@ -1264,7 +1306,7 @@ TypeSP DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
if (attrs.byte_stride == 0 && attrs.bit_stride == 0)
attrs.byte_stride = element_type->GetByteSize(nullptr).getValueOr(0);
CompilerType array_element_type = element_type->GetForwardCompilerType();
- CompleteType(array_element_type);
+ RequireCompleteType(array_element_type);
uint64_t array_element_bit_stride =
attrs.byte_stride * 8 + attrs.bit_stride;
@@ -1319,28 +1361,6 @@ TypeSP DWARFASTParserClang::ParsePointerToMemberType(
return nullptr;
}
-void DWARFASTParserClang::CompleteType(CompilerType type) {
- // Technically, enums can be incomplete too, but we don't handle those as they
- // are emitted even under -flimit-debug-info.
- if (!TypeSystemClang::IsCXXClassType(type))
- return;
-
- if (type.GetCompleteType())
- return;
-
- // No complete definition in this module. Mark the class as complete to
- // satisfy local ast invariants, but make a note of the fact that
- // it is not _really_ complete so we can later search for a definition in a
- //
diff erent module.
- // Since we provide layout assistance, layouts of types containing this class
- // will be correct even if we are not able to find the definition elsewhere.
- bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
- lldbassert(started && "Unable to start a class type definition.");
- TypeSystemClang::CompleteTagDeclarationDefinition(type);
- const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
- m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
-}
-
TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
const SymbolContext &sc, const DWARFDIE &die, TypeSP type_sp) {
if (!type_sp)
@@ -1560,13 +1580,8 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
clang::DeclContext *decl_ctx =
GetClangDeclContextContainingDIE(die, nullptr);
- // If your decl context is a record that was imported from another
- // AST context (in the gmodules case), we need to make sure the type
- // backing the Decl is complete before adding children to it. This is
- // not an issue in the non-gmodules case because the debug info will
- // always contain a full definition of parent types in that case.
- CompleteExternalTagDeclType(m_ast, GetClangASTImporter(), decl_ctx, die,
- attrs.name.GetCString());
+ PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx, die,
+ attrs.name.GetCString());
if (attrs.accessibility == eAccessNone && decl_ctx) {
// Check the decl context that contains this class/struct/union. If
@@ -2016,7 +2031,7 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
clang::TypeSourceInfo *type_source_info =
base_class->getTypeSourceInfo();
if (type_source_info)
- CompleteType(m_ast.GetType(type_source_info->getType()));
+ RequireCompleteType(m_ast.GetType(type_source_info->getType()));
}
m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
@@ -2685,7 +2700,7 @@ void DWARFASTParserClang::ParseSingleMember(
}
}
- CompleteType(member_clang_type);
+ RequireCompleteType(member_clang_type);
field_decl = TypeSystemClang::AddFieldToRecordType(
class_clang_type, name, member_clang_type, accessibility,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 2ef49abc1da1..e13716b95c1c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -217,12 +217,6 @@ class DWARFASTParserClang : public DWARFASTParser {
ParsedDWARFTypeAttributes &attrs);
lldb::TypeSP ParsePointerToMemberType(const DWARFDIE &die,
const ParsedDWARFTypeAttributes &attrs);
-
- /// Complete a type from debug info, or mark it as forcefully completed if
- /// there is no of the type in the current Module. Call this function in
- /// contexts where the usual C++ rules require a type to be complete (base
- /// class, member, etc.).
- void CompleteType(lldb_private::CompilerType type);
};
/// Parsed form of all attributes that are relevant for type reconstruction.
diff --git a/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
index 7ed33ce50297..e00dc1961749 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
@@ -1,27 +1,48 @@
-# Test that a forward-declared (DW_AT_declaration) structure is treated as a
-# forward-declaration even if it has children. These types can be produced due
-# to vtable-based type homing, or other -flimit-debug-info optimizations.
+# Test handling of forward-declared (DW_AT_declaration) structures. These types
+# can be produced due to vtable-based type homing, or other -flimit-debug-info
+# optimizations.
# REQUIRES: x86
-# RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj > %t
-# RUN: %lldb %t -o "expr a" -o exit 2>&1 | FileCheck %s --check-prefix=EXPR
-# RUN: %lldb %t -o "target var a" -o exit 2>&1 | FileCheck %s --check-prefix=VAR
-
-# EXPR: incomplete type 'A' where a complete type is required
+# RUN: rm -rf %t
+# RUN: split-file %s %t
+# RUN: llvm-mc --triple x86_64-pc-linux %t/asm --filetype=obj > %t.o
+# RUN: %lldb -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN: -s %t/commands -o exit %t.o 2>&1 | FileCheck %s
+#--- commands
+# Type A should be treated as a forward-declaration even though it has a child.
+target var a
+# CHECK-LABEL: target var a
# FIXME: This should also produce some kind of an error.
-# VAR: (A) a = {}
+# CHECK: (A) a = {}
+expr a
+# CHECK-LABEL: expr a
+# CHECK: incomplete type 'A' where a complete type is required
+
+# Parsing B::B1 should not crash even though B is incomplete. Note that in this
+# case B must be forcefully completed.
+target var b1
+# CHECK-LABEL: target var b1
+# CHECK: (B::B1) b1 = (ptr = 0x00000000baadf00d)
+expr b1
+# CHECK-LABEL: expr b1
+# CHECK: (B::B1) $0 = (ptr = 0x00000000baadf00d)
+#--- asm
.text
_ZN1AC2Ev:
retq
.LZN1AC2Ev_end:
.data
+ .p2align 4
a:
- .quad $_ZTV1A+16
- .quad $0xdeadbeef
+ .quad _ZTV1A+16
+ .quad 0xdeadbeef
+
+b1:
+ .quad 0xbaadf00d
.section .debug_abbrev,"", at progbits
.byte 1 # Abbreviation Code
@@ -73,6 +94,24 @@ a:
.byte 25 # DW_FORM_flag_present
.byte 0 # EOM(1)
.byte 0 # EOM(2)
+ .byte 6 # Abbreviation Code
+ .byte 2 # DW_TAG_class_type
+ .byte 1 # DW_CHILDREN_yes
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 7 # Abbreviation Code
+ .byte 13 # DW_TAG_member
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 56 # DW_AT_data_member_location
+ .byte 11 # DW_FORM_data1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
.byte 8 # Abbreviation Code
.byte 15 # DW_TAG_pointer_type
.byte 0 # DW_CHILDREN_no
@@ -120,6 +159,15 @@ a:
.asciz "Hand-written DWARF" # DW_AT_producer
.quad _ZN1AC2Ev # DW_AT_low_pc
.long .LZN1AC2Ev_end-_ZN1AC2Ev # DW_AT_high_pc
+
+# Case 1: The compiler has omitted the declaration of the type, but it still
+# produces an entry for its implicit constructor instantiated in this compile
+# unit.
+# Roughly corresponds to this:
+# struct A {
+# virtual ~A(); // not defined here
+# // implicit A() {}
+# } a;
.byte 2 # Abbrev [2] DW_TAG_variable
.asciz "a" # DW_AT_name
.long .LA-.Lcu_begin0 # DW_AT_type
@@ -156,5 +204,39 @@ a:
.long .LAptr-.Lcu_begin0 # DW_AT_type
# DW_AT_artificial
.byte 0 # End Of Children Mark
+
+# Case 2: A structure has been emitted as a declaration only, but it contains a
+# nested class, which has a full definition present.
+# Rougly corresponds to this:
+# struct B {
+# virtual ~B(); // not defined here
+# class B1 {
+# A* ptr;
+# };
+# };
+# B::B1 b1;
+# Note that it is important that the inner type is a class (not struct) as that
+# triggers a clang assertion.
+ .byte 3 # Abbrev [3] DW_TAG_structure_type
+ .asciz "B" # DW_AT_name
+ # DW_AT_declaration
+.LB1:
+ .byte 6 # Abbrev [6] DW_TAG_class_type
+ .asciz "B1" # DW_AT_name
+ # DW_AT_declaration
+ .byte 7 # Abbrev [5] 0x58:0xc DW_TAG_member
+ .asciz "ptr" # DW_AT_name
+ .long .LAptr # DW_AT_type
+ .byte 0 # DW_AT_data_member_location
+ .byte 0 # End Of Children Mark
+ .byte 0 # End Of Children Mark
+
+ .byte 2 # Abbrev [2] DW_TAG_variable
+ .asciz "b1" # DW_AT_name
+ .long .LB1-.Lcu_begin0 # DW_AT_type
+ .byte 9 # DW_AT_location
+ .byte 3
+ .quad b1
+
.byte 0 # End Of Children Mark
.Ldebug_info_end0:
More information about the lldb-commits
mailing list