[llvm] [clang] [LLVM][DWARF] Fix accelerator table switching between CU and TU (PR #77511)

Alexander Yermolovich via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 11 13:48:03 PST 2024


https://github.com/ayermolo updated https://github.com/llvm/llvm-project/pull/77511

>From 5e6ee63fabac0dabc692c00d3c017e2542c98273 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Tue, 9 Jan 2024 10:51:55 -0800
Subject: [PATCH 1/5] [LLVM][DWARF] Fix accelerator swtiching with TU re-use

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().
---
 .../CodeGen/thinlto-debug-names-tu-reuse.ll   | 58 +++++++++++++++++++
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp    | 10 +++-
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h      |  2 +
 3 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll

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 }
+
+ at fGlobal = dso_local global %struct.Foo zeroinitializer, align 1, !dbg !0
+ at 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;

>From 0cfe1ed989f0a92a1fd54d9eb304ac908e40f847 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Tue, 9 Jan 2024 15:29:06 -0800
Subject: [PATCH 2/5] Fix second bug where we reset to cu table, while still
 processing TUs.

---
 .../debug-names-compound-type-units.ll        | 73 +++++++++++++++++++
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp    |  2 +-
 2 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/debug-names-compound-type-units.ll

diff --git a/clang/test/CodeGen/debug-names-compound-type-units.ll b/clang/test/CodeGen/debug-names-compound-type-units.ll
new file mode 100644
index 00000000000000..0a7f5d3d45d1d2
--- /dev/null
+++ b/clang/test/CodeGen/debug-names-compound-type-units.ll
@@ -0,0 +1,73 @@
+; 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 f;
+;; };
+;; struct Foo2 {
+;;   char f;
+;;   Foo f1;
+;; };
+;; void fooFunc() {
+;; Foo2 global2;
+;; }
+;; clang++ <file>.cpp -O0 -g2 -fdebug-types-section -gpubnames -S -emit-llvm -o <file>.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 = 'main.cpp'
+source_filename = "main.cpp"
+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.Foo2 = type { i8, %struct.Foo }
+%struct.Foo = type { i8 }
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define dso_local void @_Z7fooFuncv() #0 !dbg !10 {
+entry:
+  %global2 = alloca %struct.Foo2, align 1
+  call void @llvm.dbg.declare(metadata ptr %global2, metadata !14, metadata !DIExpression()), !dbg !23
+  ret void, !dbg !24
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { mustprogress noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 18.0.0git", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false)
+!1 = !DIFile(filename: "main.cpp", directory: "/home/ayermolo/local/tasks/T173324295/smallMultipleTUs", checksumkind: CSK_MD5, checksum: "70bff4d50322126f3d8ca4178afad376")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 18.0.0git"}
+!10 = distinct !DISubprogram(name: "fooFunc", linkageName: "_Z7fooFuncv", scope: !1, file: !1, line: 9, type: !11, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !13)
+!11 = !DISubroutineType(types: !12)
+!12 = !{null}
+!13 = !{}
+!14 = !DILocalVariable(name: "global2", scope: !10, file: !1, line: 10, type: !15)
+!15 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo2", file: !1, line: 4, size: 16, flags: DIFlagTypePassByValue, elements: !16, identifier: "_ZTS4Foo2")
+!16 = !{!17, !19}
+!17 = !DIDerivedType(tag: DW_TAG_member, name: "f", scope: !15, file: !1, line: 5, baseType: !18, size: 8)
+!18 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!19 = !DIDerivedType(tag: DW_TAG_member, name: "f1", scope: !15, file: !1, line: 6, baseType: !20, size: 8, offset: 8)
+!20 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo", file: !1, line: 1, size: 8, flags: DIFlagTypePassByValue, elements: !21, identifier: "_ZTS3Foo")
+!21 = !{!22}
+!22 = !DIDerivedType(tag: DW_TAG_member, name: "f", scope: !20, file: !1, line: 2, baseType: !18, size: 8)
+!23 = !DILocation(line: 10, column: 6, scope: !10)
+!24 = !DILocation(line: 11, column: 1, scope: !10)
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 0a922fcd54a061..9a1eb688a96f4a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -3549,9 +3549,9 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU,
     AccelTypeUnitsDebugNames.convertDieToOffset();
     AccelDebugNames.addTypeEntries(AccelTypeUnitsDebugNames);
     AccelTypeUnitsDebugNames.clear();
+    setCurrentDWARF5AccelTable(DWARF5AccelTableKind::CU);
   }
   CU.addDIETypeSignature(RefDie, Signature);
-  setCurrentDWARF5AccelTable(DWARF5AccelTableKind::CU);
 }
 
 // Add the Name along with its companion DIE to the appropriate accelerator

>From dcd338bf7f17494420ab44d56c05cb63cb0f9c17 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Tue, 9 Jan 2024 15:32:07 -0800
Subject: [PATCH 3/5] updated test description

---
 clang/test/CodeGen/debug-names-compound-type-units.ll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/CodeGen/debug-names-compound-type-units.ll b/clang/test/CodeGen/debug-names-compound-type-units.ll
index 0a7f5d3d45d1d2..17b4430eceb356 100644
--- a/clang/test/CodeGen/debug-names-compound-type-units.ll
+++ b/clang/test/CodeGen/debug-names-compound-type-units.ll
@@ -1,6 +1,6 @@
 ; REQUIRES: asserts
 
-;; Tests that accelerator table switches correctly from TU to CU when a top level TU is re-used.
+;; Tests that we use correct accelerator table when processing nested TUs.
 ;; Assert is not triggered.
 ;; File1
 ;; struct Foo {

>From c3fbd562e8051ad758337182a4b5ce55c7cc353c Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Tue, 9 Jan 2024 15:37:39 -0800
Subject: [PATCH 4/5] updated directory inside test

---
 clang/test/CodeGen/debug-names-compound-type-units.ll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/CodeGen/debug-names-compound-type-units.ll b/clang/test/CodeGen/debug-names-compound-type-units.ll
index 17b4430eceb356..0d85de286772a3 100644
--- a/clang/test/CodeGen/debug-names-compound-type-units.ll
+++ b/clang/test/CodeGen/debug-names-compound-type-units.ll
@@ -47,7 +47,7 @@ attributes #1 = { nocallback nofree nosync nounwind speculatable willreturn memo
 !llvm.ident = !{!9}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 18.0.0git", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false)
-!1 = !DIFile(filename: "main.cpp", directory: "/home/ayermolo/local/tasks/T173324295/smallMultipleTUs", checksumkind: CSK_MD5, checksum: "70bff4d50322126f3d8ca4178afad376")
+!1 = !DIFile(filename: "main.cpp", directory: "/smallMultipleTUs", checksumkind: CSK_MD5, checksum: "70bff4d50322126f3d8ca4178afad376")
 !2 = !{i32 7, !"Dwarf Version", i32 5}
 !3 = !{i32 2, !"Debug Info Version", i32 3}
 !4 = !{i32 1, !"wchar_size", i32 4}

>From c09e5b0e11726638f9400d97e8bcb4fada3ff594 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Thu, 11 Jan 2024 13:47:44 -0800
Subject: [PATCH 5/5] removed kind variable

---
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 8 ++++----
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h   | 2 --
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 9a1eb688a96f4a..1d9b3d5874756c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -3580,12 +3580,12 @@ void DwarfDebug::addAccelNameImpl(
     break;
   case AccelTableKind::Dwarf: {
     DWARF5AccelTable &Current = getCurrentDWARF5AccelTable();
-    assert((CurrentKind == DWARF5AccelTableKind::TU) ||
-           ((CurrentKind == DWARF5AccelTableKind::CU) &&
+    assert((&Current == &AccelTypeUnitsDebugNames) ||
+           ((&Current == &AccelDebugNames) &&
             (Unit.getUnitDie().getTag() != dwarf::DW_TAG_type_unit)) &&
                "Kind is CU but TU is being processed.");
-    assert((CurrentKind == DWARF5AccelTableKind::CU) ||
-           ((CurrentKind == DWARF5AccelTableKind::TU) &&
+    assert((&Current == &AccelDebugNames) ||
+           ((&Current == &AccelTypeUnitsDebugNames) &&
             (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
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
index 09e9ca07624f83..452485b632c45f 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
@@ -509,7 +509,6 @@ 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;
@@ -926,7 +925,6 @@ class DwarfDebug : public DebugHandlerBase {
 
   /// Sets the current DWARF5AccelTable to use.
   void setCurrentDWARF5AccelTable(const DWARF5AccelTableKind Kind) {
-    CurrentKind = Kind;
     switch (Kind) {
     case DWARF5AccelTableKind::CU:
       CurrentDebugNames = &AccelDebugNames;



More information about the cfe-commits mailing list