Skip to content

Commit e10adfd

Browse files
committed
Reland "[DebugMetadata][DwarfDebug] Support function-local types in lexical 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.
1 parent bf82cb4 commit e10adfd

File tree

15 files changed

+215
-49
lines changed

15 files changed

+215
-49
lines changed

clang/test/DebugInfo/CXX/lambda-capture-packs.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,22 @@
22
// RUN: -debug-info-kind=standalone -std=c++26 %s -o - | FileCheck %s
33

44

5-
// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
6-
// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
7-
// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
8-
// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
9-
// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
10-
// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
11-
// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
12-
135
template<typename... Args>
146
auto capture_pack(Args... args) {
157
return [args..., ...params = args] {
168
return 0;
179
}();
1810
}
1911

12+
// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
2013
// CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK1]]
2114
// CHECK-SAME: elements: ![[PACK1_ELEMS:[0-9]+]]
2215
// CHECK-DAG: ![[PACK1_ELEMS]] = !{![[PACK1_ARGS:[0-9]+]], ![[PACK1_PARAMS:[0-9]+]]}
2316
// CHECK-DAG: ![[PACK1_ARGS]] = !DIDerivedType(tag: DW_TAG_member, name: "args"
2417
// CHECK-DAG: ![[PACK1_PARAMS]] = !DIDerivedType(tag: DW_TAG_member, name: "params"
2518
// CHECK-NOT: DW_TAG_member
2619

20+
// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
2721
// CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK2]]
2822
// CHECK-SAME: elements: ![[PACK2_ELEMS:[0-9]+]]
2923
// CHECK: ![[PACK2_ELEMS]] = !{![[PACK2_ARGS:[0-9]+]]
@@ -42,6 +36,7 @@ auto capture_pack_and_locals(int x, Args... args) {
4236
}();
4337
}
4438

39+
// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
4540
// CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK3]]
4641
// CHECK-SAME: elements: ![[PACK3_ELEMS:[0-9]+]]
4742
// CHECK: ![[PACK3_ELEMS]] = !{![[PACK3_ARGS:[0-9]+]]
@@ -55,6 +50,7 @@ auto capture_pack_and_locals(int x, Args... args) {
5550
// CHECK-DAG: ![[PACK3_W]] = !DIDerivedType(tag: DW_TAG_member, name: "w"
5651
// CHECK-NOT: DW_TAG_member
5752

53+
// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
5854
// CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK4]]
5955
// CHECK-SAME: elements: ![[PACK4_ELEMS:[0-9]+]]
6056
// CHECK: ![[PACK4_ELEMS]] = !{![[PACK4_ARGS:[0-9]+]]
@@ -90,6 +86,7 @@ struct Foo {
9086
int w = 10;
9187
} f;
9288

89+
// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
9390
// CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK5]]
9491
// CHECK-SAME: elements: ![[PACK5a_ELEMS:[0-9]+]]
9592
// CHECK: ![[PACK5a_ELEMS]] = !{![[PACK5a_THIS:[0-9]+]]
@@ -120,6 +117,7 @@ struct Foo {
120117
// CHECK-DAG: ![[PACK5c_THIS]] = !DIDerivedType(tag: DW_TAG_member, name: "this"
121118
// CHECK-NOT: DW_TAG_member
122119

120+
// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
123121
// CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK6]]
124122
// CHECK-SAME: elements: ![[PACK6a_ELEMS:[0-9]+]]
125123
// CHECK: ![[PACK6a_ELEMS]] = !{![[PACK6a_THIS:[0-9]+]]
@@ -168,6 +166,7 @@ auto capture_binding_and_param_pack(Args... args) {
168166
}();
169167
}
170168

169+
// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
171170
// CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "C"
172171
// CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK7]]
173172
// CHECK-SAME: elements: ![[PACK7_ELEMS:[0-9]+]]

llvm/include/llvm/AsmParser/LLParser.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ namespace llvm {
181181
/// Keeps track of source locations for Values, BasicBlocks, and Functions.
182182
AsmParserContext *ParserContext;
183183

184+
/// retainedNodes of these subprograms should be cleaned up from incorrectly
185+
/// scoped local types.
186+
SmallVector<DISubprogram *> NewDistinctSPs;
187+
184188
/// Only the llvm-as tool may set this to false to bypass
185189
/// UpgradeDebuginfo so it can generate broken bitcode.
186190
bool UpgradeDebugInfo;

llvm/include/llvm/IR/DebugInfoMetadata.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2596,6 +2596,21 @@ class DISubprogram : public DILocalScope {
25962596
[](auto *N) { llvm_unreachable("Unexpected retained node!"); });
25972597
}
25982598

2599+
/// Remove types that do not belong to the subprogram's scope from
2600+
/// retainedNodes list.
2601+
void cleanupRetainedNodes();
2602+
2603+
/// When DebugTypeODRUniquing is enabled, after multiple modules are loaded,
2604+
/// some subprograms (that are from different compilation units, usually)
2605+
/// may have references to the same local type in their retainedNodes lists.
2606+
///
2607+
/// Clean up such references.
2608+
template <typename RangeT>
2609+
static void cleanupRetainedNodes(const RangeT &NewDistinctSPs) {
2610+
for (DISubprogram *SP : NewDistinctSPs)
2611+
SP->cleanupRetainedNodes();
2612+
}
2613+
25992614
/// Check if this subprogram describes the given function.
26002615
///
26012616
/// FIXME: Should this be looking through bitcasts?

llvm/lib/AsmParser/LLParser.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,8 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
428428
N.second->resolveCycles();
429429
}
430430

431+
DISubprogram::cleanupRetainedNodes(std::exchange(NewDistinctSPs, {}));
432+
431433
for (auto *Inst : InstsWithTBAATag) {
432434
MDNode *MD = Inst->getMetadata(LLVMContext::MD_tbaa);
433435
// With incomplete IR, the tbaa metadata may have been dropped.
@@ -5991,6 +5993,10 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) {
59915993
thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val,
59925994
declaration.Val, retainedNodes.Val, thrownTypes.Val, annotations.Val,
59935995
targetFuncName.Val, keyInstructions.Val));
5996+
5997+
if (IsDistinct)
5998+
NewDistinctSPs.push_back(cast<DISubprogram>(Result));
5999+
59946000
return false;
59956001
}
59966002

llvm/lib/Bitcode/Reader/MetadataLoader.cpp

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,10 @@ class MetadataLoader::MetadataLoaderImpl {
452452
/// metadata.
453453
SmallDenseMap<Function *, DISubprogram *, 16> FunctionsWithSPs;
454454

455+
/// retainedNodes of these subprograms should be cleaned up from incorrectly
456+
/// scoped local types.
457+
SmallVector<DISubprogram *> NewDistinctSPs;
458+
455459
// Map the bitcode's custom MDKind ID to the Module's MDKind ID.
456460
DenseMap<unsigned, unsigned> MDKindMap;
457461

@@ -728,29 +732,7 @@ class MetadataLoader::MetadataLoaderImpl {
728732
upgradeCUVariables();
729733
if (ModuleLevel)
730734
upgradeCULocals();
731-
}
732-
733-
void cloneLocalTypes() {
734-
for (Metadata *M : MetadataList) {
735-
if (auto *SP = dyn_cast_or_null<DISubprogram>(M)) {
736-
auto RetainedNodes = SP->getRetainedNodes();
737-
SmallVector<Metadata *> MDs(RetainedNodes.begin(), RetainedNodes.end());
738-
bool HasChanged = false;
739-
for (auto &N : MDs)
740-
if (auto *T = dyn_cast<DIType>(N))
741-
if (auto *LS = dyn_cast_or_null<DILocalScope>(T->getScope()))
742-
if (auto *Parent = findEnclosingSubprogram(LS))
743-
if (Parent != SP) {
744-
HasChanged = true;
745-
auto NewT = T->clone();
746-
NewT->replaceOperandWith(1, SP);
747-
N = MDNode::replaceWithUniqued(std::move(NewT));
748-
}
749-
750-
if (HasChanged)
751-
SP->replaceRetainedNodes(MDNode::get(Context, MDs));
752-
}
753-
}
735+
DISubprogram::cleanupRetainedNodes(std::exchange(NewDistinctSPs, {}));
754736
}
755737

756738
void callMDTypeCallback(Metadata **Val, unsigned TypeID);
@@ -1129,7 +1111,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
11291111
// placeholders, that we flush here.
11301112
resolveForwardRefsAndPlaceholders(Placeholders);
11311113
upgradeDebugInfo(ModuleLevel);
1132-
cloneLocalTypes();
11331114
// Return at the beginning of the block, since it is easy to skip it
11341115
// entirely from there.
11351116
Stream.ReadBlockEnd(); // Pop the abbrev block context.
@@ -1161,7 +1142,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
11611142
case BitstreamEntry::EndBlock:
11621143
resolveForwardRefsAndPlaceholders(Placeholders);
11631144
upgradeDebugInfo(ModuleLevel);
1164-
cloneLocalTypes();
11651145
return Error::success();
11661146
case BitstreamEntry::Record:
11671147
// The interesting case.
@@ -2044,6 +2024,9 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
20442024
MetadataList.assignValue(SP, NextMetadataNo);
20452025
NextMetadataNo++;
20462026

2027+
if (IsDistinct)
2028+
NewDistinctSPs.push_back(SP);
2029+
20472030
// Upgrade sp->function mapping to function->sp mapping.
20482031
if (HasFn) {
20492032
if (auto *CMD = dyn_cast_or_null<ConstantAsMetadata>(CUorFn))

llvm/lib/IR/DIBuilder.cpp

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,13 @@ DIDerivedType *DIBuilder::createTypedef(DIType *Ty, StringRef Name,
379379
DIScope *Context, uint32_t AlignInBits,
380380
DINode::DIFlags Flags,
381381
DINodeArray Annotations) {
382-
return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
383-
LineNo, getNonCompileUnitScope(Context), Ty,
384-
(uint64_t)0, AlignInBits, (uint64_t)0, std::nullopt,
385-
std::nullopt, Flags, nullptr, Annotations);
382+
auto *T = DIDerivedType::get(
383+
VMContext, dwarf::DW_TAG_typedef, Name, File, LineNo,
384+
getNonCompileUnitScope(Context), Ty, (uint64_t)0, AlignInBits,
385+
(uint64_t)0, std::nullopt, std::nullopt, Flags, nullptr, Annotations);
386+
if (isa_and_nonnull<DILocalScope>(Context))
387+
getSubprogramNodesTrackingVector(Context).emplace_back(T);
388+
return T;
386389
}
387390

388391
DIDerivedType *
@@ -606,6 +609,8 @@ DICompositeType *DIBuilder::createStructType(
606609
nullptr, UniqueIdentifier, nullptr, nullptr, nullptr, nullptr, nullptr,
607610
nullptr, Specification, NumExtraInhabitants);
608611
trackIfUnresolved(R);
612+
if (isa_and_nonnull<DILocalScope>(Context))
613+
getSubprogramNodesTrackingVector(Context).emplace_back(R);
609614
return R;
610615
}
611616

@@ -690,6 +695,8 @@ DIDerivedType *DIBuilder::createSetType(DIScope *Scope, StringRef Name,
690695
SizeInBits, AlignInBits, 0, std::nullopt,
691696
std::nullopt, DINode::FlagZero);
692697
trackIfUnresolved(R);
698+
if (isa_and_nonnull<DILocalScope>(Scope))
699+
getSubprogramNodesTrackingVector(Scope).emplace_back(R);
693700
return R;
694701
}
695702

@@ -725,6 +732,8 @@ DICompositeType *DIBuilder::createArrayType(
725732
: (Metadata *)cast<DIVariable *>(RK),
726733
nullptr, nullptr, 0, BitStride);
727734
trackIfUnresolved(R);
735+
if (isa_and_nonnull<DILocalScope>(Scope))
736+
getSubprogramNodesTrackingVector(Scope).emplace_back(R);
728737
return R;
729738
}
730739

@@ -879,9 +888,12 @@ DISubrangeType *DIBuilder::createSubrangeType(
879888
uint64_t SizeInBits, uint32_t AlignInBits, DINode::DIFlags Flags,
880889
DIType *Ty, Metadata *LowerBound, Metadata *UpperBound, Metadata *Stride,
881890
Metadata *Bias) {
882-
return DISubrangeType::get(VMContext, Name, File, LineNo, Scope, SizeInBits,
883-
AlignInBits, Flags, Ty, LowerBound, UpperBound,
884-
Stride, Bias);
891+
auto *T = DISubrangeType::get(VMContext, Name, File, LineNo, Scope,
892+
SizeInBits, AlignInBits, Flags, Ty, LowerBound,
893+
UpperBound, Stride, Bias);
894+
if (isa_and_nonnull<DILocalScope>(Scope))
895+
getSubprogramNodesTrackingVector(Scope).emplace_back(T);
896+
return T;
885897
}
886898

887899
static void checkGlobalVariableScope(DIScope *Context) {

llvm/lib/IR/DebugInfoMetadata.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,6 +1467,44 @@ DILocalScope *DISubprogram::getRetainedNodeScope(MDNode *N) {
14671467
return cast<DILocalScope>(getRawRetainedNodeScope(N));
14681468
}
14691469

1470+
void DISubprogram::cleanupRetainedNodes() {
1471+
// Checks if a metadata node from retainedTypes is a type not belonging to
1472+
// this subprogram.
1473+
auto IsAlienType = [this](DINode *N) {
1474+
auto *T = dyn_cast_or_null<DIType>(N);
1475+
if (!T)
1476+
return false;
1477+
1478+
DISubprogram *TypeSP = nullptr;
1479+
// The type might have been global in the previously loaded IR modules.
1480+
if (auto *LS = dyn_cast_or_null<DILocalScope>(T->getScope()))
1481+
TypeSP = LS->getSubprogram();
1482+
1483+
return this != TypeSP;
1484+
};
1485+
1486+
// As this is expected to be called during module loading, before
1487+
// stripping old or incorrect debug info, perform minimal sanity check.
1488+
if (!isa_and_present<MDTuple>(getRawRetainedNodes()))
1489+
return;
1490+
1491+
MDTuple *RetainedNodes = cast<MDTuple>(getRawRetainedNodes());
1492+
SmallVector<Metadata *> MDs;
1493+
MDs.reserve(RetainedNodes->getNumOperands());
1494+
for (const MDOperand &Node : RetainedNodes->operands()) {
1495+
// Ignore malformed retainedNodes.
1496+
if (Node && !isa<DINode>(Node))
1497+
return;
1498+
1499+
auto *N = cast_or_null<DINode>(Node);
1500+
if (!IsAlienType(N))
1501+
MDs.push_back(N);
1502+
}
1503+
1504+
if (MDs.size() != RetainedNodes->getNumOperands())
1505+
replaceRetainedNodes(MDNode::get(getContext(), MDs));
1506+
}
1507+
14701508
DILexicalBlockBase::DILexicalBlockBase(LLVMContext &C, unsigned ID,
14711509
StorageType Storage,
14721510
ArrayRef<Metadata *> Ops)

llvm/lib/IR/Verifier.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,10 +1576,15 @@ void Verifier::visitDISubprogram(const DISubprogram &N) {
15761576
CheckDI(RetainedNodeScope,
15771577
"invalid retained nodes, retained node is not local", &N, Node,
15781578
RetainedNode);
1579+
1580+
DISubprogram *RetainedNodeSP = RetainedNodeScope->getSubprogram();
1581+
DICompileUnit *RetainedNodeUnit =
1582+
RetainedNodeSP ? RetainedNodeSP->getUnit() : nullptr;
15791583
CheckDI(
1580-
RetainedNodeScope->getSubprogram() == &N,
1584+
RetainedNodeSP == &N,
15811585
"invalid retained nodes, retained node does not belong to subprogram",
1582-
&N, Node, RetainedNode, RetainedNodeScope);
1586+
&N, Node, RetainedNode, RetainedNodeScope, RetainedNodeSP,
1587+
RetainedNodeUnit);
15831588
}
15841589
}
15851590
CheckDI(!hasConflictingReferenceFlags(N.getFlags()),

llvm/test/Bitcode/clone-local-types.ll renamed to llvm/test/Bitcode/local-type-scope.ll

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
; RUN: llvm-as < %s -o %t0
2-
; RUN: llvm-dis %t0 -o - | FileCheck %s
1+
; RUN: llvm-as --disable-verify < %s -o %t0
2+
; RUN: opt --passes=verify %t0 -o /dev/null
3+
; RUN: llvm-dis %t0 -o - | FileCheck %s --implicit-check-not=DICompositeType
34

4-
; Ensure that function-local types with the same ODR identifier belonging
5-
; to different subprograms are not deduplicated when a module is being loaded.
5+
; During module loading, if a local type appears in retainedNodes
6+
; field of multiple DISubprograms due to ODR-uniquing,
7+
; retainedNodes should be cleaned up, so that only one DISubprogram
8+
; will have this type in its retainedNodes.
69

710
; CHECK: [[CU:![0-9]+]] = distinct !DICompileUnit
811
; CHECK: [[BAR:![0-9]+]] = distinct !DISubprogram(name: "bar", {{.*}}, retainedNodes: [[RN_BAR:![0-9]+]])
9-
; CHECK: [[RN_BAR]] = !{[[T2:![0-9]+]]}
10-
; CHECK: [[T2]] = !DICompositeType(tag: DW_TAG_class_type, {{.*}}, identifier: "local_type")
12+
; CHECK: [[RN_BAR]] = !{}
1113
; CHECK: [[FOO:![0-9]+]] = distinct !DISubprogram(name: "foo", {{.*}}, retainedNodes: [[RN_FOO:![0-9]+]])
1214
; CHECK: [[RN_FOO]] = !{[[T1:![0-9]+]]}
13-
; CHECK: [[T1]] = !DICompositeType(tag: DW_TAG_class_type, {{.*}}, identifier: "local_type")
15+
; CHECK: [[T1]] = !DICompositeType(tag: DW_TAG_class_type, scope: [[FOO]], {{.*}}, identifier: "local_type")
1416

1517
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
1618
target triple = "x86_64-unknown-linux-gnu"

llvm/test/DebugInfo/X86/split-dwarf-local-import.ll renamed to llvm/test/DebugInfo/Generic/split-dwarf-local-import.ll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
; REQUIRES: x86-registered-target
12
; RUN: %llc_dwarf -O1 -filetype=obj -split-dwarf-file=%t.dwo < %s | llvm-dwarfdump -debug-info - | FileCheck %s --implicit-check-not "{{DW_TAG|NULL}}"
23

34
; CHECK-LABEL: debug_info contents

0 commit comments

Comments
 (0)