[CloneFunction][DebugInfo] Avoid cloning DILocalVariables of inlined functions #75385
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
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-clang Author: Vladislav Dzhidzhoev (dzhidzhoev) Changes
This is a follow-up for https://reviews.llvm.org/D144006, fixing a crash reported The first commit is added for convenience, as it has already been accepted. If DISubpogram was not cloned (e.g. we are cloning a function that has other This is meant to be committed along with https://reviews.llvm.org/D144006. Patch is 97.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/75385.diff 31 Files Affected:
diff --git a/clang/test/CodeGen/debug-info-codeview-unnamed.c b/clang/test/CodeGen/debug-info-codeview-unnamed.c
index bd2a7543e56b2b..16ffb3682236f1 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 3e9f7b07658e36..31d608d92a06b4 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 023cac159faa4b..5b01c6dbb39414 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 9f2c044843d0f0..7c0bf8ccb03842 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 61b3c7c0526c8e..c91cf83c0405fe 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 b4c79936ab33e6..9602ac1b024973 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 6b9c9a243decd1..122e4aa62ea7df 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 eecbac6520ac97..3d659e7bfd004a 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 edec161b397155..166cd664aec1de 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 36ef77f9505bc1..d60011e14ed172 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -104,7 +104,7 @@ class DebugInfoFinder {
void processInstruction(const Module &M, const Instruction &I);
/// Process a DILocalVariable.
- void processVariable(const Module &M, const DILocalVariable *DVI);
+ void processVariable(DILocalVariable *DVI);
/// Process debug info location.
void processLocation(const Module &M, const DILocation *Loc);
// Process a DPValue, much like a DbgVariableIntrinsic.
@@ -120,6 +120,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);
@@ -134,6 +135,7 @@ class DebugInfoFinder {
SmallVectorImpl<DIGlobalVariableExpression *>::const_iterator;
using type_iterator = SmallVectorImpl<DIType *>::const_iterator;
using scope_iterator = SmallVectorImpl<DIScope *>::const_iterator;
+ using local_variable_iterator = SmallVectorImpl<DILocalVariable *>::const_iterator;
iterator_range<compile_unit_iterator> compile_units() const {
return make_range(CUs.begin(), CUs.end());
@@ -147,6 +149,10 @@ class DebugInfoFinder {
return make_range(GVs.begin(), GVs.end());
}
+ iterator_range<local_variable_iterator> local_variables() const {
+ return make_range(LVs.begin(), LVs.end());
+ }
+
iterator_range<type_iterator> types() const {
return make_range(TYs.begin(), TYs.end());
}
@@ -157,6 +163,7 @@ class DebugInfoFinder {
unsigned compile_unit_count() const { return CUs.size(); }
unsigned global_variable_count() const { return GVs.size(); }
+ unsigned local_variable_count() const { return LVs.size(); }
unsigned subprogram_count() const { return SPs.size(); }
unsigned type_count() const { return TYs.size(); }
unsigned scope_count() const { return Scopes.size(); }
@@ -165,6 +172,7 @@ class DebugInfoFinder {
SmallVector<DICompileUnit *, 8> CUs;
SmallVector<DISubprogram *, 8> SPs;
SmallVector<DIGlobalVariableExpression *, 8> GVs;
+ SmallVector<DILocalVariable *, 8> LVs;
SmallVector<DIType *, 8> TYs;
SmallVector<DIScope *, 8> Scopes;
SmallPtrSet<const MDNode *, 32> NodesSeen;
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 910e97489dbbe0..d118695b431c5a 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -547,6 +547,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) {
@@ -554,48 +556,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->g...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
dzhidzhoev
requested review from
DavidSpickett,
adrian-prantl,
dexonsmith,
dwblaikie,
jdoerfert,
jmorse,
jryans,
kazutakahirata and
rnk
Collaborator
There was a problem hiding this comment.
Can you add a sentence explaining why some subprograms won't be cloned? Is it because they are inlined and a copy already exists?
Collaborator
adrian-prantl
left a comment
adrian-prantl
left a comment
There was a problem hiding this comment.
I think this LGTM, but it would be good if someone else also took a look.
Member
Author
|
@dwblaikie |
dwblaikie
approved these changes
Jan 9, 2024
Collaborator
dwblaikie
left a comment
dwblaikie
left a comment
There was a problem hiding this comment.
Yeah, looks OK to me - sorry for the delay.
…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. Additionally, if DISubprogram is not cloned in CloneFunctionInto(), cloning of its DILocalVariables is avoided. Otherwise we get duplicated DILocalVariables not tracked in their subprogram's retainedNodes, that crash LTO with Chromium. Reviewed By: jmmartinez Authored-by: Kristina Bessonova <[email protected]> Differential Revision: https://reviews.llvm.org/D144006
dzhidzhoev
force-pushed
the
new-4-squashed
branch
from
56427e2 to
2953d4a
Compare
Member
|
Hi, We're seeing a crash with this commit and reproducer https://gist.github.com/jmorse/b0248c3c9f9195487ffd7c7431a8d15e llc: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2338: virtual void llvm::DwarfDebug::endFunctionImpl(const llvm::MachineFunction *): Assertion `LScopes.getAbstractScopesList().size() == NumAbstractSubprograms && "getOrCreateAbstractScope() inserted an abstract subprogram scope"' failed. I'd previously posted the reproducer on https://reviews.llvm.org/D144006#4656728 , however I'd anonymised the IR too much to the point where it was broken in unrelated ways. Revision 2 of the gist, as linked, should produce the crash. I suspect the extra lexical scopes reachable through the retained-nodes list also need to be explored when the LexicalScopes object gets constructed, to avoid scopes being added late and causing containers to invalidate iterators. (Which is what that assertion is there to detect). |
Collaborator
|
We're hitting the same assert (tracking in https://crbug.com/1518841) but don't have a good reproducer to share (it's during lto of a large binary). Jeremy's reproducer seems good though. Can we revert this while it's being investigated, please? |
Member
|
Hitting the same problem on a bunch of services. I am going to revert this to unblock people, our repro is similar enough to the one Jeremy shared. |
Member
|
Reverted in b6f922f |
Member
|
I think I've nailed down the source of this problem, and it's the matter to do with To reproduce, take the rebased patch from the tip of https://github.com/jmorse/llvm-project/tree/rebased-composite-type-fix-4 , and this code from llvm/test/Linker/odr-lambda-1.ll: Compile thusly: The link should succeed because there are no symbol conflicts. In the output IR in out3.ll.txt, observe that there are:
It's this mismatch where the scope of an element in the retained nodes points somewhere unexpected, which causes the assertion failure reported. It's all because of the ODR-uniquing by type name that LLVM can do, a rationale is here: llvm-project/clang/lib/CodeGen/CGDebugInfo.cpp Line 1086 in 2604830 When DICompositeType::buildODRType de-duplicates on the basis of the type name, if it's a type inside a function then the scope pointer will point at the DISubprogram where the type was first seen. If there's an ODR violation, this might not be the same as the DISubprogram that is eventually selected for that function. IIRC there's precedent in LLVM for throwing away lots of debug-info in the presence of ODR violations, but we should avoid hard errors. |
Collaborator
|
I'm not seeing how your example constitutes an ODR violation, or how merging these lambda types by mangled name is incorrect. They are equivalent in your example. It seems like the issue has more to do with the details of how exactly we do the merge, and where the metadata references from DICompositeType to DISubprogram point. |
Member
|
[This keeps on slipping to the back of my TODO list,] I've been enlightened by the comments on #68929 about ODR rules, and that there isn't a violation in the example; it does at least exercise the code path of interest, my summary of which is that the ODR-uniquing of types is happening at such a low of a level that it causes surprises and can't be easily fixed. Here's a more well designed reproducer: Compile and link this similar to above: Where b.cpp is a copy of the file above with the function renamed from 'a' to 'b' to ensure there aren't multiple conflicting definitions. In this code, we inline the body of "foo" into the 'a' and 'b' functions, and furthermore we inline the get_a method of foo::bar too. In each of the compilation units, this leads to a chain of lexical scopes for the most deeply inlined instruction of:
The trouble comes when the two modules are linked together: the two collections of DILocations / DILexicalScopes / DISubprograms describing source-locations in each module are distinct and kept separate through linking. However the DICompositeType for foo::bar is unique'd based on its name, and its "scope" field will point into one of the metadata collections. Thus, where we used to have two distinct chains of lexical scopes we've now got a tree of them, joining at the unique'd DICompositeType, and llc is not prepared for this. I don't know that this is a bug, more of a design mismatch: most of the rest of LLVM is probably OK with having the lexical-scope chain actually being a tree, given that it only ever looks up it. However LexicalScopes does a top down exploration of a function looking for lexical scopes, and is then surprised when it finds different scopes looking from the bottom up. We could adjust it to search upwards for more lexical scopes (it already does that for block-scopes), but then I imagine we would produce very confusing DWARF that contained two Subprogram scopes for the same function. There's also no easy way of working around this in metadata: we can't describe any other metadata relationship because it's done at such a low level, and we can't selectively not-ODR-unique DICompositeTypes that are inside functions because the lexical scope metadata might not have been loaded yet, so can't be examined. An immediate fix would be to not set the "identifier" field for the DICompositeType when it's created if it's inside a function scope to avoid ODRUniqing. I've only got a light understanding of what the identifier field is for, so there might be unexpected consequences, plus there'll be a metadata/DWARF size cost to that. |
Collaborator
Actually, that seems like probably the correct fix. The identifier field is precisely for "ODR-unique-based-on-this" and only for that, IIRC (@aprantl, can you confirm? these are old memories for me at this point...). If the function-local types should not be ODR-uniqued, then dropping the identifier field sounds correct. |
Collaborator
I can't speak to the complexities of the alternative, but I'm immediately suspicious of this direction. We have stable manglings for static locals in inline functions and static data members of local classes and static locals in methods in local classes in inline functions... Surely if we can compute a unique mangled name for this local class, we should use it as the identifier and merge it. It seems important that, for size reasons, we figure out a way to do this for lambdas, which are common in inline functions in headers. |
Member
|
Hadn't realised lambda types will pop up everywhere, the cost might be too high. I'll test with building clang and our internal codebases to see if the fix works and what the cost is.
Indeed, it's not the ODR-uniquing per-se that's causing the trouble here, it's the fact that we do not merge together the DISubprograms that are the containing scope for that type upon module linking. I'm not familiar with the design decisions behind that, but a courtesy glance suggests it's important to identify the compilation unit that a DISubprogram instance came from. We're then stuck with multiple DISubprograms and a single DIComposite type referring to one of them as being its scope, which presumably breaks a lot of assumptions. Perhaps a worthwhile direction is separating DISubprograms into abstract and definition portions, much like DWARF does, which would allow us to put these types into the declaration and have a well defined scope hierarchy: types that get uniqued refer to the abstract-DISubprogram for their lexical scope. Perhaps that's can be achieved with the existing distinction between definition DISubprograms and "declaration" DISubprograms? A quick survey of callers to If we took that route, the scope-chain would still become a tree of scopes instead of distinct chains, and we would still need to fix the problem of CodeGen LexicalScopes encountering the abstract/declaration DISubprogram. However, I think we'd have the reasonable solution of mapping declaration-DISubprograms in a function to their definition-DISubprogram (assuming the definition had been found already), which is much more reliable than seeking a DISubprogram by name or something. |
Collaborator
This sounds like a promising direction to me, but I have no idea what the relative costs are. |
Collaborator
|
I remember discovering that there are effectively two kinds of DISubprogram already... one for declarations (which usually get uniqued/deduped) and another for definitions (which I believe never do... IIRC they are always "distinct"). I imagine it would be possible/good to separate into two classes. If I'd understood there were two uses I would have done so originally. (The main effort is updating testcases.) Even without two classes, you might be able to clean up the type graph, knowing that there are in fact two ways DISubprogram is used. |
Member
|
I've placed a re-application of this code in #119001, along with a commit that attempts to fix the matter discovered here. To avoid multiple DICompositeTypes in multiple DISubprograms being ODRUnique'd, we instead put them in the declaration DISubprogram which should be unique'd anyway. |
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Sep 17, 2025
…subprogram DIEs With this change, construction of abstract subprogram DIEs is split in two stages/functions: creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE). With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes llvm#29985. LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now it has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for llvm#29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). However, I'm not sure if these patches can be merged before llvm#75385. The last attempt to make it mergeable was llvm#142166. @dwblaikie came up with an idea of making DISubprograms unique in LLVM IR. To implement that, I've tried making a patch that allows DwarfDebug to handle multiple llvm::Functions referring to the same DISubprogram, and it also needs parts of the functionality of this patch. Some changes made by @ellishg in llvm#90523 were taken for this commit.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 10, 2025
…o multiple Functions Depends on: * TODO In llvm#75385 (and the following tries), an attempt was made, to support attaching local types to DILocalScopes, and to store function local types in DISubprogram's `retainedNodes:` field. That patch failed to land due to issues arising during LTO process. If two definition DISubprograms from different compile units represent, essentially, the same source code function, and have common local DICompositeType, and if this DICompositeType is uniqued (due to ODRUniquingDebugTypes feature), the subprograms end up having wrong retainedNodes list/scoping relationship. To tackle this issue, in llvm#142166, it was proposed to force-unique all DISubporgrams even if they don't contain odr-uniqued types (llvm#142166 (comment)). It should establish one-to-one-to-many relationship between DISubprograms, abstract DIEs and function clones (from different CUs, in case of LTO). To implement that, AsmPrinter should support correct emission of debug info for DISubprograms attached to multiple functions. This is the goal of this commit. Here, LexicalScope's function map is changed to multimap between DISubprogram and (possible multiple) functions attached to it. LexicalScope is modified to create an abstract scope for a DISubprogram having multiple lllvm::Function attachments. `DwarfCompileUnit::getOrCreateSubprogramDIE` can recognize the case of DISubprogram attached to multiple Functions, and return abstract DIE when needed. CodeViewDebug is adopted as well. UDTs are ensured to be emmited properly in the cases that are addressed here. Please let me know if more changes to CodeView needed, as I'm not very familiar with the format.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 10, 2025
…o multiple Functions Depends on: * llvm#152680 * llvm#162852 In llvm#75385 (and the following tries), an attempt was made, to support attaching local types to DILocalScopes, and to store function local types in DISubprogram's `retainedNodes:` field. That patch failed to land due to issues arising during LTO process. If two definition DISubprograms from different compile units represent, essentially, the same source code function, and have common local DICompositeType, and if this DICompositeType is uniqued (due to ODRUniquingDebugTypes feature), the subprograms end up having wrong retainedNodes list/scoping relationship. To tackle this issue, in llvm#142166, it was proposed to force-unique all DISubporgrams even if they don't contain odr-uniqued types (llvm#142166 (comment)). It should establish one-to-one-to-many relationship between DISubprograms, abstract DIEs and function clones (from different CUs, in case of LTO). To implement that, AsmPrinter should support correct emission of debug info for DISubprograms attached to multiple functions. This is the goal of this commit. Here, LexicalScope's function map is changed to multimap between DISubprogram and (possible multiple) functions attached to it. LexicalScope is modified to create an abstract scope for a DISubprogram having multiple lllvm::Function attachments. `DwarfCompileUnit::getOrCreateSubprogramDIE` can recognize the case of DISubprogram attached to multiple Functions, and return abstract DIE when needed. CodeViewDebug is adopted as well. UDTs are ensured to be emmited properly in the cases that are addressed here. Please let me know if more changes to CodeView needed, as I'm not very familiar with the format.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 10, 2025
…o multiple Functions Depends on: * llvm#152680 * llvm#162852 In llvm#75385 (and the following tries), an attempt was made, to support attaching local types to DILocalScopes, and to store function local types in DISubprogram's `retainedNodes:` field. That patch failed to land due to issues arising during LTO process. If two definition DISubprograms from different compile units represent, essentially, the same source code function, and have common local DICompositeType, and if this DICompositeType is uniqued (due to ODRUniquingDebugTypes feature), the subprograms end up having wrong retainedNodes list/scoping relationship. To tackle this issue, in llvm#142166, it was proposed to force-unique all DISubporgrams even if they don't contain odr-uniqued types (llvm#142166 (comment)). It should establish one-to-one-to-many relationship between DISubprograms, abstract DIEs and function clones (from different CUs, in case of LTO). To implement that, AsmPrinter should support correct emission of debug info for DISubprograms attached to multiple functions. This is the goal of this commit. Here, LexicalScope's function map is changed to multimap between DISubprogram and (possible multiple) functions attached to it. LexicalScope is modified to create an abstract scope for a DISubprogram having multiple lllvm::Function attachments. `DwarfCompileUnit::getOrCreateSubprogramDIE` can recognize the case of DISubprogram attached to multiple Functions, and return abstract DIE when needed. CodeViewDebug is adopted as well. UDTs are ensured to be emmited properly in the cases that are addressed here. Please let me know if more changes to CodeView needed, as I'm not very familiar with the format.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 15, 2025
…o multiple Functions Depends on: * llvm#152680 * llvm#162852 In llvm#75385 (and the following tries), an attempt was made, to support attaching local types to DILocalScopes, and to store function local types in DISubprogram's `retainedNodes:` field. That patch failed to land due to issues arising during LTO process. If two definition DISubprograms from different compile units represent, essentially, the same source code function, and have common local DICompositeType, and if this DICompositeType is uniqued (due to ODRUniquingDebugTypes feature), the subprograms end up having wrong retainedNodes list/scoping relationship. To tackle this issue, in llvm#142166, it was proposed to force-unique all DISubporgrams even if they don't contain odr-uniqued types (llvm#142166 (comment)). It should establish one-to-one-to-many relationship between DISubprograms, abstract DIEs and function clones (from different CUs, in case of LTO). To implement that, AsmPrinter should support correct emission of debug info for DISubprograms attached to multiple functions. This is the goal of this commit. Here, LexicalScope's function map is changed to multimap between DISubprogram and (possible multiple) functions attached to it. LexicalScope is modified to create an abstract scope for a DISubprogram having multiple lllvm::Function attachments. `DwarfCompileUnit::getOrCreateSubprogramDIE` can recognize the case of DISubprogram attached to multiple Functions, and return abstract DIE when needed. CodeViewDebug is adopted as well. UDTs are ensured to be emmited properly in the cases that are addressed here. Please let me know if more changes to CodeView needed, as I'm not very familiar with the format.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 23, 2025
…exical block scopes (4/7)" This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was llvm#75385. The issue with it was investigated in llvm#75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, (1) previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), (2) for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. With this approach, clang builds without crashes in FullLTO (which is not the case for llvm#75385). Additionally: - a check is added to Verifier to report about local types located in a wrong retainedNodes list, - DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit llvm#75385 is separated from the new changes to simplify review process.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 23, 2025
…exical block scopes (4/7)" This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was llvm#75385. The issue with it was investigated in llvm#75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, (1) previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), (2) for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links. With this approach, clang builds without crashes in FullLTO (which is not the case for llvm#75385). Additionally: - a check is added to Verifier to report about local types located in a wrong retainedNodes list, - DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit llvm#75385 and the new changes are placed in separate commits to simplify review process.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 23, 2025
…exical block scopes (4/7)" This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was llvm#75385. The issue with it was investigated in llvm#75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, * previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), * for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links. With this approach, clang builds without crashes in FullLTO (which is not the case for llvm#75385). Additionally: * a check is added to Verifier to report about local types located in a wrong retainedNodes list, * DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit llvm#75385 and the new changes are placed in separate commits to simplify review process.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 27, 2025
…exical block scopes (4/7)" This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was llvm#75385. The issue with it was investigated in llvm#75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, * previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), * for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links. With this approach, clang builds without crashes in FullLTO (which is not the case for llvm#75385). Additionally: * a check is added to Verifier to report about local types located in a wrong retainedNodes list, * DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit llvm#75385 and the new changes are placed in separate commits to simplify review process.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Oct 27, 2025
…exical block scopes (4/7)" This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was llvm#75385. The issue with it was investigated in llvm#75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, * previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), * for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links. With this approach, clang builds without crashes in FullLTO (which is not the case for llvm#75385). Additionally: * a check is added to Verifier to report about local types located in a wrong retainedNodes list, * DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit llvm#75385 and the new changes are placed in separate commits to simplify review process.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Nov 6, 2025
These checks ensure that retained nodes of a DISubprogram belong to the subprogram. This was helpful to find issues with https://reviews.llvm.org/D144004, llvm#75385, llvm#165032. Also, interface for accessing DISubprogram's retained nodes is slightly refactored. `DISubprogram::visitRetainedNodes` and `DISubprogram::forEachRetainedNode` are added to avoid repeating checks like ``` if (const auto *LV = dyn_cast<DILocalVariable>(N)) ... else if (const auto *L = dyn_cast<DILabel>(N)) ... else if (const auto *IE = dyn_cast<DIImportedEntity>(N)) ... ``` Tests with wrongly scoped retained nodes are fixed.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Nov 10, 2025
…exical block scopes (4/7)" This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was llvm#75385. The issue with it was investigated in llvm#75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, * previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), * for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links. With this approach, clang builds without crashes in FullLTO (which is not the case for llvm#75385). Additionally: * a check is added to Verifier to report about local types located in a wrong retainedNodes list, * DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit llvm#75385 and the new changes are placed in separate commits to simplify review process.
dzhidzhoev
added a commit
to dzhidzhoev/llvm-project
that referenced
this pull request
Nov 11, 2025
…exical block scopes (4/7)" This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was llvm#75385. The issue with it was investigated in llvm#75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, * previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), * for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links. With this approach, clang builds without crashes in FullLTO (which is not the case for llvm#75385). Additionally: * a check is added to Verifier to report about local types located in a wrong retainedNodes list, * DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit llvm#75385 and the new changes are placed in separate commits to simplify review process.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
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.
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.
If DISubpogram was not cloned (e.g. we are cloning a function that has other
functions inlined into it, and subprograms of the inlined functions are
not supposed to be cloned), it doesn't make sense to clone its
DILocalVariables as well.
Otherwise get duplicated DILocalVariables not tracked in their
subprogram's retainedNodes, that crash LTO with Chromium.
This is meant to be committed along with https://reviews.llvm.org/D144006.