Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DebugMetadata][DwarfDebug] Clone uniqued function-local types after metadata loading #68986

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

dzhidzhoev
Copy link
Member

@dzhidzhoev dzhidzhoev commented Oct 13, 2023

  • [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)
  • Clone function-local types after metadata loading.

This is a follow-up for https://reviews.llvm.org/D144006, fixing a crash reported in Chromium (https://reviews.llvm.org/D144006#4651955).

The first commit is added for convenience, as it has already been accepted.
The issue is that when DIType nodes are loaded, they get deduplicated as ODR-uniqued. When two modules get merged by LTO, DISubprogram nodes for the same function from different modules are not uniqued, since they are marked as distinct, whereas their retained types are, which can violate the ownership relationship between DISubprogram and its function-local retained type.
It is fixed by copying a function-local type referenced by one DISubprogram through retainedNodes and referencing another in its scope field. The scope of the copy gets changed correspondingly.

This is meant to be committed along with https://reviews.llvm.org/D144006.

@llvmbot llvmbot added clang Clang issues not falling into any other category debuginfo llvm:ir llvm:transforms labels Oct 13, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 13, 2023

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-clang

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes
  • [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)
  • Clone function-local types after metadata loading.

This is a follow-up for https://reviews.llvm.org/D144006, fixing a crash reported in Chromium (https://reviews.llvm.org/D144006#4651955).

The very first commit is added for convenience, as it has already been accepted.
The issue with it is that when DIType nodes are loaded, they get deduplicated as ODR-uniqued. When two modules get merged by LTO, DISubprogram nodes for the same function from different modules are not uniqued, since they are marked as distinct, whereas their retained type are, which can violate the ownership relationship betweeen DISubprogram and its function-local retained type.
It is fixed by copying a function-local type referenced by one DISubprogram through retainedNodes and referencing another in its scope field. The scope of copy gets changed correspondingly.

This is meant to be committed along with https://reviews.llvm.org/D144006.


Patch is 92.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/68986.diff

31 Files Affected:

  • (modified) clang/test/CodeGen/debug-info-codeview-unnamed.c (+8-8)
  • (modified) clang/test/CodeGen/debug-info-unused-types.c (+9-7)
  • (modified) clang/test/CodeGen/debug-info-unused-types.cpp (+8-6)
  • (modified) clang/test/CodeGenCXX/debug-info-access.cpp (+1-1)
  • (modified) clang/test/CodeGenCXX/debug-info-anon-union-vars.cpp (+6-6)
  • (modified) clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp (+62-48)
  • (modified) clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp (+2-2)
  • (modified) clang/test/CodeGenCXX/debug-lambda-this.cpp (+2-2)
  • (modified) llvm/include/llvm/IR/DIBuilder.h (+3-3)
  • (modified) llvm/include/llvm/IR/DebugInfo.h (+1)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+77-33)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (+40-20)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (+8-8)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (+10-3)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+27-10)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+10-1)
  • (modified) llvm/lib/IR/Verifier.cpp (+3-3)
  • (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+6-1)
  • (added) llvm/test/Bitcode/clone-local-types.ll (+43)
  • (modified) llvm/test/Bitcode/upgrade-cu-locals.ll (+41-27)
  • (modified) llvm/test/Bitcode/upgrade-cu-locals.ll.bc ()
  • (added) llvm/test/DebugInfo/Generic/inlined-local-type.ll (+128)
  • (added) llvm/test/DebugInfo/Generic/lexical-block-retained-types.ll (+55)
  • (added) llvm/test/DebugInfo/Generic/lexical-block-types.ll (+425)
  • (modified) llvm/test/DebugInfo/Generic/verifier-invalid-disubprogram.ll (+1-1)
  • (added) llvm/test/DebugInfo/X86/local-type-as-template-parameter.ll (+161)
  • (modified) llvm/test/DebugInfo/X86/set.ll (+2-2)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import.ll (-1)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import2.ll (-1)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import3.ll ()
  • (modified) llvm/unittests/Transforms/Utils/CloningTest.cpp (+93)
diff --git a/clang/test/CodeGen/debug-info-codeview-unnamed.c b/clang/test/CodeGen/debug-info-codeview-unnamed.c
index bd2a7543e56b2ba..16ffb3682236f18 100644
--- a/clang/test/CodeGen/debug-info-codeview-unnamed.c
+++ b/clang/test/CodeGen/debug-info-codeview-unnamed.c
@@ -8,23 +8,23 @@ int main(int argc, char* argv[], char* arge[]) {
   //
   struct { int bar; } one = {42};
   //
-  // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // LINUX-SAME:     type: [[TYPE_OF_ONE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_ONE]] = distinct !DICompositeType(
+  // LINUX:      [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType(
   // LINUX-SAME:     tag: DW_TAG_structure_type
   // LINUX-NOT:      name:
   // LINUX-NOT:      identifier:
   // LINUX-SAME:     )
+  // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
+  // LINUX-SAME:     type: [[TYPE_OF_ONE]]
+  // LINUX-SAME:     )
   //
-  // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // MSVC-SAME:      type: [[TYPE_OF_ONE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_ONE]] = distinct !DICompositeType
+  // MSVC:       [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType
   // MSVC-SAME:      tag: DW_TAG_structure_type
   // MSVC-NOT:       name:
   // MSVC-NOT:       identifier:
   // MSVC-SAME:      )
+  // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
+  // MSVC-SAME:      type: [[TYPE_OF_ONE]]
+  // MSVC-SAME:      )
 
   return 0;
 }
diff --git a/clang/test/CodeGen/debug-info-unused-types.c b/clang/test/CodeGen/debug-info-unused-types.c
index 3e9f7b07658e36e..31d608d92a06b41 100644
--- a/clang/test/CodeGen/debug-info-unused-types.c
+++ b/clang/test/CodeGen/debug-info-unused-types.c
@@ -18,13 +18,15 @@ void quux(void) {
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "bar"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAR"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], [[TYPE6:![0-9]+]], {{![0-9]+}}, [[TYPE7:![0-9]+]], [[TYPE2]], [[TYPE8:![0-9]+]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz"
-// CHECK: [[TYPE7]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], [[TYPE4:![0-9]+]], {{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE5:![0-9]+]], [[TYPE6:![0-9]+]], [[TYPE8:![0-9]+]]}
+// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "y"
+// CHECK: [[TYPE6]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: [[TYPE7:![0-9]+]] = !DIEnumerator(name: "Z"
 // CHECK: [[TYPE8]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "w"
 
 // Check that debug info is not emitted for the typedef, struct, enum, and
diff --git a/clang/test/CodeGen/debug-info-unused-types.cpp b/clang/test/CodeGen/debug-info-unused-types.cpp
index 023cac159faa4b4..5b01c6dbb394149 100644
--- a/clang/test/CodeGen/debug-info-unused-types.cpp
+++ b/clang/test/CodeGen/debug-info-unused-types.cpp
@@ -13,12 +13,14 @@ void quux() {
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAZ"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], {{![0-9]+}}, [[TYPE6:![0-9]+]], [[TYPE2]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], {{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]]}
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y", scope: [[SP]]
+// CHECK: [[TYPE5]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z", scope: [[SP]]
+// CHECK: [[TYPE6:![0-9]+]] = !DIEnumerator(name: "Z"
 
 // NODBG-NOT: !DI{{CompositeType|Enumerator|DerivedType}}
 
diff --git a/clang/test/CodeGenCXX/debug-info-access.cpp b/clang/test/CodeGenCXX/debug-info-access.cpp
index 9f2c044843d0f0f..7c0bf8ccb038428 100644
--- a/clang/test/CodeGenCXX/debug-info-access.cpp
+++ b/clang/test/CodeGenCXX/debug-info-access.cpp
@@ -18,9 +18,9 @@ class B : public A {
   static int public_static;
 
 protected:
+  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+3]],{{.*}} flags: DIFlagProtected)
   // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_typedef",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected)
   typedef int prot_typedef;
-  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected)
   using prot_using = prot_typedef;
   prot_using prot_member;
 
diff --git a/clang/test/CodeGenCXX/debug-info-anon-union-vars.cpp b/clang/test/CodeGenCXX/debug-info-anon-union-vars.cpp
index 61b3c7c0526c8ef..c91cf83c0405fec 100644
--- a/clang/test/CodeGenCXX/debug-info-anon-union-vars.cpp
+++ b/clang/test/CodeGenCXX/debug-info-anon-union-vars.cpp
@@ -51,13 +51,13 @@ void instantiate(int x) {
 // CHECK: !DIGlobalVariable(name: "b",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true
 // CHECK: !DIGlobalVariable(name: "result", {{.*}} isLocal: false, isDefinition: true
 // CHECK: !DIGlobalVariable(name: "value", {{.*}} isLocal: false, isDefinition: true
-// CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial
-// CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial
-// CHECK: !DILocalVariable(
-// CHECK-NOT: name:
-// CHECK: type: ![[UNION:[0-9]+]]
-// CHECK: ![[UNION]] = distinct !DICompositeType(tag: DW_TAG_union_type,
+// CHECK: ![[UNION:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_union_type,
 // CHECK-NOT: name:
 // CHECK: elements
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "i", scope: ![[UNION]],
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "c", scope: ![[UNION]],
+// CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial
+// CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial
+// CHECK: !DILocalVariable(
+// CHECK-NOT: name:
+// CHECK: type: ![[UNION]]
diff --git a/clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp b/clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp
index b4c79936ab33e64..9602ac1b0249730 100644
--- a/clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp
+++ b/clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp
@@ -3,6 +3,60 @@
 
 int main(int argc, char* argv[], char* arge[]) {
   //
+  // LINUX:      [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "<unnamed-type-one>"
+  // MSVC-SAME:      identifier: ".?AU<unnamed-type-one>@?1??main@@9@"
+  // MSVC-SAME:      )
+
+
+  //
+  // LINUX:      [[TYPE_OF_TWO:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_TWO:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "<unnamed-type-two>"
+  // MSVC-SAME:      identifier: ".?AU<unnamed-type-two>@?2??main@@9@"
+  // MSVC-SAME:      )
+
+
+  //
+  // LINUX:      [[TYPE_OF_THREE:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-SAME:     name: "named"
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_THREE:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "named"
+  // MSVC-SAME:      identifier: ".?AUnamed@?1??main@@9@"
+  // MSVC-SAME:      )
+
+  //
+  // LINUX:      [[TYPE_OF_FOUR:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_class_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_FOUR:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_class_type
+  // MSVC-SAME:      name: "<lambda_0>"
+  // MSVC-SAME:      identifier: ".?AV<lambda_0>@?0??main@@9@"
+  // MSVC-SAME:      )
+
+
   // In CodeView, the LF_MFUNCTION entry for "bar()" refers to the forward
   // reference of the unnamed struct. Visual Studio requires a unique
   // identifier to match the LF_STRUCTURE forward reference to the definition.
@@ -10,21 +64,11 @@ int main(int argc, char* argv[], char* arge[]) {
   struct { void bar() {} } one;
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // LINUX-SAME:     type: [[TYPE_OF_ONE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_ONE]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_ONE]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // MSVC-SAME:      type: [[TYPE_OF_ONE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_ONE]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "<unnamed-type-one>"
-  // MSVC-SAME:      identifier: ".?AU<unnamed-type-one>@?1??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_ONE]]
   // MSVC-SAME:      )
 
 
@@ -36,21 +80,11 @@ int main(int argc, char* argv[], char* arge[]) {
   int decltype(two)::*ptr2unnamed = &decltype(two)::bar;
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "two"
-  // LINUX-SAME:     type: [[TYPE_OF_TWO:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_TWO]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_TWO]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "two"
-  // MSVC-SAME:      type: [[TYPE_OF_TWO:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_TWO]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "<unnamed-type-two>"
-  // MSVC-SAME:      identifier: ".?AU<unnamed-type-two>@?2??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_TWO]]
   // MSVC-SAME:      )
 
 
@@ -61,21 +95,11 @@ int main(int argc, char* argv[], char* arge[]) {
   struct named { int bar; int named::* p2mem; } three = { 42, &named::bar };
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "three"
-  // LINUX-SAME:     type: [[TYPE_OF_THREE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_THREE]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-SAME:     name: "named"
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_THREE]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "three"
-  // MSVC-SAME:      type: [[TYPE_OF_THREE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_THREE]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "named"
-  // MSVC-SAME:      identifier: ".?AUnamed@?1??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_THREE]]
   // MSVC-SAME:      )
 
 
@@ -87,21 +111,11 @@ int main(int argc, char* argv[], char* arge[]) {
   auto four = [argc](int i) -> int { return argc == i ? 1 : 0; };
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "four"
-  // LINUX-SAME:     type: [[TYPE_OF_FOUR:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_FOUR]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_class_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_FOUR]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "four"
-  // MSVC-SAME:      type: [[TYPE_OF_FOUR:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_FOUR]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_class_type
-  // MSVC-SAME:      name: "<lambda_0>"
-  // MSVC-SAME:      identifier: ".?AV<lambda_0>@?0??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_FOUR]]
   // MSVC-SAME:      )
 
   return 0;
diff --git a/clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp b/clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp
index 6b9c9a243decd1e..122e4aa62ea7dfc 100644
--- a/clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp
+++ b/clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp
@@ -51,9 +51,9 @@ void test() {
   // CHECK-SAME:                              name: "<lambda_2_1>",
   c.lambda_params();
 
-  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1:[0-9]+]],
-  // CHECK: ![[LAMBDA1]] = !DICompositeType(tag: DW_TAG_class_type,
+  // CHECK: ![[LAMBDA1:[0-9]+]] = !DICompositeType(tag: DW_TAG_class_type,
   // CHECK-SAME:                            name: "<lambda_1>",
   // CHECK-SAME:                            flags: DIFlagFwdDecl
+  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1]],
   c.lambda2();
 }
diff --git a/clang/test/CodeGenCXX/debug-lambda-this.cpp b/clang/test/CodeGenCXX/debug-lambda-this.cpp
index eecbac6520ac97d..3d659e7bfd004a1 100644
--- a/clang/test/CodeGenCXX/debug-lambda-this.cpp
+++ b/clang/test/CodeGenCXX/debug-lambda-this.cpp
@@ -13,10 +13,10 @@ int D::d(int x) {
 }
 
 // CHECK: ![[D:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "D",
-// CHECK: ![[POINTER:.*]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[D]], size: 64)
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "this",
 // CHECK-SAME:           line: 11
-// CHECK-SAME:           baseType: ![[POINTER]]
+// CHECK-SAME:           baseType: ![[POINTER:[0-9]+]]
 // CHECK-SAME:           size: 64
 // CHECK-NOT:            offset: 0
 // CHECK-SAME:           ){{$}}
+// CHECK: ![[POINTER]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[D]], size: 64)
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index ecd6dd7b0a4f822..0aa180aec6d8a0f 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -49,7 +49,7 @@ namespace llvm {
     Function *LabelFn;       ///< llvm.dbg.label
     Function *AssignFn;      ///< llvm.dbg.assign
 
-    SmallVector<TrackingMDNodeRef, 4> AllEnumTypes;
+    SmallVector<TrackingMDNodeRef, 4> EnumTypes;
     /// Track the RetainTypes, since they can be updated later on.
     SmallVector<TrackingMDNodeRef, 4> AllRetainTypes;
     SmallVector<DISubprogram *, 4> AllSubprograms;
@@ -64,8 +64,8 @@ namespace llvm {
     SmallVector<TrackingMDNodeRef, 4> UnresolvedNodes;
     bool AllowUnresolvedNodes;
 
-    /// Each subprogram's preserved local variables, labels and imported
-    /// entities.
+    /// Each subprogram's preserved local variables, labels, imported entities,
+    /// and types.
     ///
     /// Do not use a std::vector.  Some versions of libc++ apparently copy
     /// instead of move on grow operations, and TrackingMDRef is expensive to
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 92beebed8ad51df..2c1fc6fb965fffa 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -114,6 +114,7 @@ class DebugInfoFinder {
   void processCompileUnit(DICompileUnit *CU);
   void processScope(DIScope *Scope);
   void processType(DIType *DT);
+  void processLocalVariable(DILocalVariable *DV);
   bool addCompileUnit(DICompileUnit *CU);
   bool addGlobalVariable(DIGlobalVariableExpression *DIG);
   bool addScope(DIScope *Scope);
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 1e9ed5fcaa581b3..e4fcc20242e186d 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -548,6 +548,8 @@ class MetadataLoader::MetadataLoaderImpl {
 
   /// Move local imports from DICompileUnit's 'imports' field to
   /// DISubprogram's retainedNodes.
+  /// Move fucntion-local enums from DICompileUnit's enums
+  /// to DISubprogram's retainedNodes.
   void upgradeCULocals() {
     if (NamedMDNode *CUNodes = TheModule.getNamedMetadata("llvm.dbg.cu")) {
       for (unsigned I = 0, E = CUNodes->getNumOperands(); I != E; ++I) {
@@ -555,48 +557,66 @@ class MetadataLoader::MetadataLoaderImpl {
         if (!CU)
           continue;
 
-        if (CU->getRawImportedEntities()) {
-          // Collect a set of imported entities to be moved.
-          SetVector<Metadata *> EntitiesToRemove;
+        SetVector<Metadata *> MetadataToRemove;
+        // Collect imported entities to be moved.
+        if (CU->getRawImportedEntities())
           for (Metadata *Op : CU->getImportedEntities()->operands()) {
             auto *IE = cast<DIImportedEntity>(Op);
-            if (dyn_cast_or_null<DILocalScope>(IE->getScope())) {
-              EntitiesToRemove.insert(IE);
-            }
+            if (dyn_cast_or_null<DILocalScope>(IE->getScope()))
+              MetadataToRemove.insert(IE);
+          }
+        // Collect enums to be moved.
+        if (CU->getRawEnumTypes())
+          for (Metadata *Op : CU->getEnumTypes()->operands()) {
+            auto *Enum = cast<DICompositeType>(Op);
+            if (dyn_cast_or_null<DILocalScope>(Enum->getScope()))
+              MetadataToRemove.insert(Enum);
           }
 
-          if (!EntitiesToRemove.empty()) {
-            // Make a new list of CU's 'imports'.
-            SmallVector<Metadata *> NewImports;
-            for (Metadata *Op : CU->getImportedEntities()->operands()) {
-              if (!EntitiesToRemove.contains(cast<DIImportedEntity>(Op))) {
+        if (!MetadataToRemove.empty()) {
+          // Make a new list of CU's 'imports'.
+          SmallVector<Metadata *> NewImports;
+          if (CU->getRawImportedEntities())
+            for (Metadata *Op : CU->getImportedEntities()->operands())
+              if (!MetadataToRemove.contains(Op))
                 NewImports.push_back(Op);
-              }
-            }
 
-            // Find DISubprogram corresponding to each entity.
-            std::map<DISubprogram *, SmallVector<Metadata *>> SPToEntities;
-            for (auto *I : EntitiesToRemove) {
-              auto *Entity = cast<DIImportedEntity>(I);
-              if (auto *SP = findEnclosingSubprogram(
-                      cast<DILocalScope>(Entity->getScope()))) {
-                SPToEntities[SP].push_back(Entity);
-              }
-            }
+          // Make a new list of CU's 'enums'.
+          SmallVector<Metadata *> NewEnums;
+          if (CU->getRawEnumTypes())
+            for (Metadata *Op : CU->getEnumTypes()->operands())
+              if (!MetadataToRemove.contains(Op))
+                NewEnums.push_ba...
[truncated]

@github-actions
Copy link

github-actions bot commented Oct 13, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 98bd0d9dd2da0edec15b5eb7acb480c47c4594cc 3b449bd46a11a55a40cbc0016a99b202fa05248e -- clang/test/CodeGen/debug-info-codeview-unnamed.c clang/test/CodeGen/debug-info-unused-types.c clang/test/CodeGen/debug-info-unused-types.cpp clang/test/CodeGenCXX/debug-info-access.cpp clang/test/CodeGenCXX/debug-info-anon-union-vars.cpp clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp clang/test/CodeGenCXX/debug-lambda-this.cpp llvm/include/llvm/IR/DIBuilder.h llvm/include/llvm/IR/DebugInfo.h llvm/lib/Bitcode/Reader/MetadataLoader.cpp llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp llvm/lib/IR/DIBuilder.cpp llvm/lib/IR/DebugInfo.cpp llvm/lib/IR/Verifier.cpp llvm/lib/Transforms/Utils/CloneFunction.cpp llvm/unittests/Transforms/Utils/CloningTest.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index d3e618b63..f9e3c3db2 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -69,9 +69,9 @@ void DIBuilder::finalize() {
   }
 
   if (!EnumTypes.empty())
-    CUNode->replaceEnumTypes(MDTuple::get(
-        VMContext, SmallVector<Metadata *, 16>(EnumTypes.begin(),
-                                               EnumTypes.end())));
+    CUNode->replaceEnumTypes(
+        MDTuple::get(VMContext, SmallVector<Metadata *, 16>(EnumTypes.begin(),
+                                                            EnumTypes.end())));
 
   SmallVector<Metadata *, 16> RetainValues;
   // Declarations and definitions of the same type may be retained. Some

@dzhidzhoev dzhidzhoev changed the title [DebugMetadata][DwarfDebug] Clone uniqued function-local types after metadata loading. [DebugMetadata][DwarfDebug] Clone uniqued function-local types after metadata loading Oct 13, 2023
@dwblaikie
Copy link
Collaborator

FWIW, I think we also saw, internally at Google, some significant and surprising (growth in sections, like .debug_loclists and .debug_gnu_pubnames/types) that were a bit surprising/not what I'd have expected of the original committed/reverted patch.

Could you run a clang build with/without this patch, and compare the two files using bloaty ( https://github.com/google/bloaty ) or similar - some comparison of the % growth of each section to see how these patches are affecting debug info size?

(patch itself sounds OK/looks good)

@dzhidzhoev
Copy link
Member Author

FWIW, I think we also saw, internally at Google, some significant and surprising (growth in sections, like .debug_loclists and .debug_gnu_pubnames/types) that were a bit surprising/not what I'd have expected of the original committed/reverted patch.

Could you run a clang build with/without this patch, and compare the two files using bloaty ( https://github.com/google/bloaty ) or similar - some comparison of the % growth of each section to see how these patches are affecting debug info size?

(patch itself sounds OK/looks good)

I haven't mentioned any significant change in the total debug info size. The full size of the debug info sections of clang executable file has increased by less than 1%. The size of the debug info sections of test-suite object files has also increased by 0,02% in total.

@dwblaikie
Copy link
Collaborator

FWIW, I think we also saw, internally at Google, some significant and surprising (growth in sections, like .debug_loclists and .debug_gnu_pubnames/types) that were a bit surprising/not what I'd have expected of the original committed/reverted patch.
Could you run a clang build with/without this patch, and compare the two files using bloaty ( https://github.com/google/bloaty ) or similar - some comparison of the % growth of each section to see how these patches are affecting debug info size?
(patch itself sounds OK/looks good)

I haven't mentioned any significant change in the total debug info size. The full size of the debug info sections of clang executable file has increased by less than 1%. The size of the debug info sections of test-suite object files has also increased by 0,02% in total.

Hmm - we saw some much larger swings internally, but hopefully something just got confused... I may need to test/look into it in more detail.

@dzhidzhoev
Copy link
Member Author

Ping

@@ -731,6 +731,29 @@ class MetadataLoader::MetadataLoaderImpl {
upgradeCULocals();
}

void cloneLocalTypes() {
for (int I = MetadataList.size() - 1; I >= 0; --I) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any particular reason this iterates in reverse? Probably worth a comment explaining it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it's just a residue from the function draft. Fixed that.

@@ -0,0 +1,43 @@
; RUN: llvm-as < %s -o %t0
; RUN: llvm-dis %t0 -o - | FileCheck %s

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be worth a comment describing the purpose of this test (some info from the commit message, probably)

…lock scopes (4/7)

RFC https://discourse.llvm.org/t/rfc-dwarfdebug-fix-and-improve-handling-imported-entities-types-and-static-local-in-subprogram-and-lexical-block-scopes/68544

Similar to imported declarations, the patch tracks function-local types in
DISubprogram's 'retainedNodes' field. DwarfDebug is adjusted in accordance with
the aforementioned metadata change and provided a support of function-local
types scoped within a lexical block.

The patch assumes that DICompileUnit's 'enums field' no longer tracks local
types and DwarfDebug would assert if any locally-scoped types get placed there.

Reviewed By: jmmartinez
Authored-by: Kristina Bessonova <kbessonova@accesssoftek.com>
Differential Revision: https://reviews.llvm.org/D144006
@dzhidzhoev dzhidzhoev merged commit 3b449bd into llvm:main Nov 2, 2023
2 of 3 checks passed
@dzhidzhoev
Copy link
Member Author

Reverted due to Chromium build crash https://bugs.chromium.org/p/chromium/issues/detail?id=1500022.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category debuginfo llvm:ir llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants