[LLVM][DWARF] Fix accelerator table switching between CU and TU #77511
Conversation
This file contains hidden or 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
This bug is triggered when a TU is already created, and we process the same DICompositeType at a top level. We would switch to TU accelerator table, but would not switch back on early exit. As the result we would add CU entries to the TU accelerator table. When we try to write out TUs and normalize entries, the offsets for DIEs that are part of a CU would not have been computed, and it would assert on getOffset().
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-debuginfo Author: Alexander Yermolovich (ayermolo) ChangesThis bug is triggered when a TU is already created, and we process the same Full diff: https://github.com/llvm/llvm-project/pull/77511.diff 3 Files Affected:
diff --git a/clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll b/clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll
new file mode 100644
index 00000000000000..53aec43a050f8b
--- /dev/null
+++ b/clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll
@@ -0,0 +1,58 @@
+; REQUIRES: asserts
+
+;; Tests that accelerator table switches correctly from TU to CU when a top level TU is re-used.
+;; Assert is not triggered.
+;; File1
+;; struct Foo {
+;; char fChar;
+;; };
+;; Foo fGlobal;
+;; FIle2
+;; struct Foo {
+;; char fChar;
+;; };
+;; Foo fGlobal2;
+;; clang++ <file>.cpp -O0 -g2 -fdebug-types-section -gpubnames -S -emit-llvm -o <file>.ll
+;; llvm-link file1.ll file2.ll -S -o thinlto-debug-names-tu-reuse.ll
+
+; RUN: llc -O0 -dwarf-version=5 -generate-type-units -filetype=obj < %s -o %t.o
+; RUN: llvm-readelf --sections %t.o | FileCheck --check-prefix=OBJ %s
+
+; OBJ: debug_names
+
+; ModuleID = 'llvm-link'
+source_filename = "llvm-link"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.Foo = type { i8 }
+
+@fGlobal = dso_local global %struct.Foo zeroinitializer, align 1, !dbg !0
+@fGlobal2 = dso_local global %struct.Foo zeroinitializer, align 1, !dbg !9
+
+!llvm.dbg.cu = !{!2, !11}
+!llvm.ident = !{!14, !14}
+!llvm.module.flags = !{!15, !16, !17, !18, !19, !20, !21}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "fGlobal", scope: !2, file: !3, line: 5, type: !5, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang version 18.0.0git", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false)
+!3 = !DIFile(filename: "main.cpp", directory: "/smallTUReuse", checksumkind: CSK_MD5, checksum: "4f1831504f0948b03880356fae49cb58")
+!4 = !{!0}
+!5 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo", file: !3, line: 2, size: 8, flags: DIFlagTypePassByValue, elements: !6, identifier: "_ZTS3Foo")
+!6 = !{!7}
+!7 = !DIDerivedType(tag: DW_TAG_member, name: "fChar", scope: !5, file: !3, line: 3, baseType: !8, size: 8)
+!8 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!9 = !DIGlobalVariableExpression(var: !10, expr: !DIExpression())
+!10 = distinct !DIGlobalVariable(name: "fGlobal2", scope: !11, file: !12, line: 5, type: !5, isLocal: false, isDefinition: true)
+!11 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !12, producer: "clang version 18.0.0git", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !13, splitDebugInlining: false)
+!12 = !DIFile(filename: "helper.cpp", directory: "/smallTUReuse", checksumkind: CSK_MD5, checksum: "014145d46991fd1eb6a2192d382feb75")
+!13 = !{!9}
+!14 = !{!"clang version 18.0.0git"}
+!15 = !{i32 7, !"Dwarf Version", i32 5}
+!16 = !{i32 2, !"Debug Info Version", i32 3}
+!17 = !{i32 1, !"wchar_size", i32 4}
+!18 = !{i32 8, !"PIC Level", i32 2}
+!19 = !{i32 7, !"PIE Level", i32 2}
+!20 = !{i32 7, !"uwtable", i32 2}
+!21 = !{i32 7, !"frame-pointer", i32 2}
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 41afbea4561433..0a922fcd54a061 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -3448,7 +3448,6 @@ uint64_t DwarfDebug::makeTypeSignature(StringRef Identifier) {
void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU,
StringRef Identifier, DIE &RefDie,
const DICompositeType *CTy) {
- setCurrentDWARF5AccelTable(DWARF5AccelTableKind::TU);
// Fast path if we're building some type units and one has already used the
// address pool we know we're going to throw away all this work anyway, so
// don't bother building dependent types.
@@ -3461,6 +3460,7 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU,
return;
}
+ setCurrentDWARF5AccelTable(DWARF5AccelTableKind::TU);
bool TopLevelType = TypeUnitsUnderConstruction.empty();
AddrPool.resetUsedFlag();
@@ -3580,6 +3580,14 @@ void DwarfDebug::addAccelNameImpl(
break;
case AccelTableKind::Dwarf: {
DWARF5AccelTable &Current = getCurrentDWARF5AccelTable();
+ assert((CurrentKind == DWARF5AccelTableKind::TU) ||
+ ((CurrentKind == DWARF5AccelTableKind::CU) &&
+ (Unit.getUnitDie().getTag() != dwarf::DW_TAG_type_unit)) &&
+ "Kind is CU but TU is being processed.");
+ assert((CurrentKind == DWARF5AccelTableKind::CU) ||
+ ((CurrentKind == DWARF5AccelTableKind::TU) &&
+ (Unit.getUnitDie().getTag() == dwarf::DW_TAG_type_unit)) &&
+ "Kind is TU but CU is being processed.");
// The type unit can be discarded, so need to add references to final
// acceleration table once we know it's complete and we emit it.
Current.addName(Ref, Die, Unit.getUniqueID());
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
index 452485b632c45f..09e9ca07624f83 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
@@ -509,6 +509,7 @@ class DwarfDebug : public DebugHandlerBase {
DWARF5AccelTable AccelTypeUnitsDebugNames;
/// Used to hide which DWARF5AccelTable we are using now.
DWARF5AccelTable *CurrentDebugNames = &AccelDebugNames;
+ DWARF5AccelTableKind CurrentKind = DWARF5AccelTableKind::CU;
AccelTable<AppleAccelTableOffsetData> AccelNames;
AccelTable<AppleAccelTableOffsetData> AccelObjC;
AccelTable<AppleAccelTableOffsetData> AccelNamespace;
@@ -925,6 +926,7 @@ class DwarfDebug : public DebugHandlerBase {
/// Sets the current DWARF5AccelTable to use.
void setCurrentDWARF5AccelTable(const DWARF5AccelTableKind Kind) {
+ CurrentKind = Kind;
switch (Kind) {
case DWARF5AccelTableKind::CU:
CurrentDebugNames = &AccelDebugNames;
|
|
There is another bug on when we exit from addDwarfTypeUnitType. I need to construct a small repro for it. |
ayermolo
changed the title
dwblaikie
approved these changes
Jan 10, 2024
|
Could this be caused by the patch https://lab.llvm.org/buildbot/#/builders/236/builds/8673 ? |
Passes for me locally. Also patch impacts .debug_names for DWARF. So highly unlikely it would cause this test to fail. |
|
However, the patch broke the Solaris/sparcv9 buildbot: The bot is configured to do a Sparc-only build. |
oh. I think EDIT: Looks like it's already added 6f55c13 |
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
…#77511) Bug 1 is triggered when a TU is already created, and we process the same DICompositeType at a top level. We would switch to TU accelerator table, but would not switch back on early exit. As the result we would add CU entries to the TU accelerator table. When we try to write out TUs and normalize entries, the offsets for DIEs that are part of a CU would not have been computed, and it would assert on getOffset(). Bug 2 is triggered when processing nested TUs. When we exit from addDwarfTypeUnitType we switched back to CU accelerator table. If we were processing nested TUs, the rest of the entries from TUs would be added to CU accelerator table. When we write out TUs, all the DIE pointers will become invalid. Eventually it will assert during normalization step after CU is processed.
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.
Bug 1 is triggered when a TU is already created, and we process the same
DICompositeType at a top level. We would switch to TU accelerator table, but
would not switch back on early exit. As the result we would add CU entries to the TU
accelerator table. When we try to write out TUs and normalize entries, the
offsets for DIEs that are part of a CU would not have been computed, and it
would assert on getOffset().
Bug 2 is triggered when processing nested TUs. When we exit from addDwarfTypeUnitType we switched back to CU accelerator table. If we were processing nested TUs, the rest of the entries from TUs would be added to CU accelerator table. When we write out TUs, all the DIE pointers will become invalid. Eventually it will assert during normalization step after CU is processed.