Skip to content

Commit e97af0a

Browse files
dzhidzhoevdzhidzhoev
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 #75385. The issue with it was investigated in #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 #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 #75385 and the new changes are placed in separate commits to simplify review process.
1 parent b113d5c commit e97af0a

File tree

15 files changed

+220
-47
lines changed

15 files changed

+220
-47
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
@@ -2536,6 +2536,21 @@ class DISubprogram : public DILocalScope {
25362536
replaceOperandWith(7, N.get());
25372537
}
25382538

2539+
/// Remove types that do not belong to the subprogram's scope from
2540+
/// retainedNodes list.
2541+
void cleanupRetainedNodes();
2542+
2543+
/// When DebugTypeODRUniquing is enabled, after multiple modules are loaded,
2544+
/// some subprograms (that are from different compilation units, usually)
2545+
/// may have references to the same local type in their retainedNodes lists.
2546+
///
2547+
/// Clean up such references.
2548+
template <typename RangeT>
2549+
static void cleanupRetainedNodes(const RangeT &NewDistinctSPs) {
2550+
for (DISubprogram *SP : NewDistinctSPs)
2551+
SP->cleanupRetainedNodes();
2552+
}
2553+
25392554
/// Check if this subprogram describes the given function.
25402555
///
25412556
/// FIXME: Should this be looking through bitcasts?

‎llvm/lib/AsmParser/LLParser.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
#include <cassert>
5555
#include <cstring>
5656
#include <optional>
57+
#include <utility>
5758
#include <vector>
5859

5960
using namespace llvm;
@@ -428,6 +429,8 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
428429
N.second->resolveCycles();
429430
}
430431

432+
DISubprogram::cleanupRetainedNodes(std::exchange(NewDistinctSPs, {}));
433+
431434
for (auto *Inst : InstsWithTBAATag) {
432435
MDNode *MD = Inst->getMetadata(LLVMContext::MD_tbaa);
433436
// With incomplete IR, the tbaa metadata may have been dropped.
@@ -5987,6 +5990,10 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) {
59875990
thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val,
59885991
declaration.Val, retainedNodes.Val, thrownTypes.Val, annotations.Val,
59895992
targetFuncName.Val, keyInstructions.Val));
5993+
5994+
if (IsDistinct)
5995+
NewDistinctSPs.push_back(cast<DISubprogram>(Result));
5996+
59905997
return false;
59915998
}
59925999

‎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

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

760742
void callMDTypeCallback(Metadata **Val, unsigned TypeID);
@@ -1133,7 +1115,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
11331115
// placeholders, that we flush here.
11341116
resolveForwardRefsAndPlaceholders(Placeholders);
11351117
upgradeDebugInfo(ModuleLevel);
1136-
cloneLocalTypes();
11371118
// Return at the beginning of the block, since it is easy to skip it
11381119
// entirely from there.
11391120
Stream.ReadBlockEnd(); // Pop the abbrev block context.
@@ -1165,7 +1146,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
11651146
case BitstreamEntry::EndBlock:
11661147
resolveForwardRefsAndPlaceholders(Placeholders);
11671148
upgradeDebugInfo(ModuleLevel);
1168-
cloneLocalTypes();
11691149
return Error::success();
11701150
case BitstreamEntry::Record:
11711151
// The interesting case.
@@ -2048,6 +2028,9 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
20482028
MetadataList.assignValue(SP, NextMetadataNo);
20492029
NextMetadataNo++;
20502030

2031+
if (IsDistinct)
2032+
NewDistinctSPs.push_back(SP);
2033+
20512034
// Upgrade sp->function mapping to function->sp mapping.
20522035
if (HasFn) {
20532036
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
@@ -377,10 +377,13 @@ DIDerivedType *DIBuilder::createTypedef(DIType *Ty, StringRef Name,
377377
DIScope *Context, uint32_t AlignInBits,
378378
DINode::DIFlags Flags,
379379
DINodeArray Annotations) {
380-
return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
381-
LineNo, getNonCompileUnitScope(Context), Ty,
382-
(uint64_t)0, AlignInBits, (uint64_t)0, std::nullopt,
383-
std::nullopt, Flags, nullptr, Annotations);
380+
auto *T = DIDerivedType::get(
381+
VMContext, dwarf::DW_TAG_typedef, Name, File, LineNo,
382+
getNonCompileUnitScope(Context), Ty, (uint64_t)0, AlignInBits,
383+
(uint64_t)0, std::nullopt, std::nullopt, Flags, nullptr, Annotations);
384+
if (isa_and_nonnull<DILocalScope>(Context))
385+
getSubprogramNodesTrackingVector(Context).emplace_back(T);
386+
return T;
384387
}
385388

386389
DIDerivedType *
@@ -604,6 +607,8 @@ DICompositeType *DIBuilder::createStructType(
604607
nullptr, UniqueIdentifier, nullptr, nullptr, nullptr, nullptr, nullptr,
605608
nullptr, Specification, NumExtraInhabitants);
606609
trackIfUnresolved(R);
610+
if (isa_and_nonnull<DILocalScope>(Context))
611+
getSubprogramNodesTrackingVector(Context).emplace_back(R);
607612
return R;
608613
}
609614

@@ -688,6 +693,8 @@ DIDerivedType *DIBuilder::createSetType(DIScope *Scope, StringRef Name,
688693
SizeInBits, AlignInBits, 0, std::nullopt,
689694
std::nullopt, DINode::FlagZero);
690695
trackIfUnresolved(R);
696+
if (isa_and_nonnull<DILocalScope>(Scope))
697+
getSubprogramNodesTrackingVector(Scope).emplace_back(R);
691698
return R;
692699
}
693700

@@ -723,6 +730,8 @@ DICompositeType *DIBuilder::createArrayType(
723730
: (Metadata *)cast<DIVariable *>(RK),
724731
nullptr, nullptr, 0, BitStride);
725732
trackIfUnresolved(R);
733+
if (isa_and_nonnull<DILocalScope>(Scope))
734+
getSubprogramNodesTrackingVector(Scope).emplace_back(R);
726735
return R;
727736
}
728737

@@ -877,9 +886,12 @@ DISubrangeType *DIBuilder::createSubrangeType(
877886
uint64_t SizeInBits, uint32_t AlignInBits, DINode::DIFlags Flags,
878887
DIType *Ty, Metadata *LowerBound, Metadata *UpperBound, Metadata *Stride,
879888
Metadata *Bias) {
880-
return DISubrangeType::get(VMContext, Name, File, LineNo, Scope, SizeInBits,
881-
AlignInBits, Flags, Ty, LowerBound, UpperBound,
882-
Stride, Bias);
889+
auto *T = DISubrangeType::get(VMContext, Name, File, LineNo, Scope,
890+
SizeInBits, AlignInBits, Flags, Ty, LowerBound,
891+
UpperBound, Stride, Bias);
892+
if (isa_and_nonnull<DILocalScope>(Scope))
893+
getSubprogramNodesTrackingVector(Scope).emplace_back(T);
894+
return T;
883895
}
884896

885897
static void checkGlobalVariableScope(DIScope *Context) {

‎llvm/lib/IR/DebugInfoMetadata.cpp‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,6 +1425,45 @@ bool DISubprogram::describes(const Function *F) const {
14251425
assert(F && "Invalid function");
14261426
return F->getSubprogram() == this;
14271427
}
1428+
1429+
void DISubprogram::cleanupRetainedNodes() {
1430+
// Checks if a metadata node from retainedTypes is a type not belonging to
1431+
// this subprogram.
1432+
auto IsAlienType = [this](DINode *N) {
1433+
auto *T = dyn_cast_or_null<DIType>(N);
1434+
if (!T)
1435+
return false;
1436+
1437+
DISubprogram *TypeSP = nullptr;
1438+
// The type might have been global in the previously loaded IR modules.
1439+
if (auto *LS = dyn_cast_or_null<DILocalScope>(T->getScope()))
1440+
TypeSP = LS->getSubprogram();
1441+
1442+
return this != TypeSP;
1443+
};
1444+
1445+
// As this is expected to be called during module loading, before
1446+
// stripping old or incorrect debug info, perform minimal sanity check.
1447+
if (!isa_and_present<MDTuple>(getRawRetainedNodes()))
1448+
return;
1449+
1450+
MDTuple *RetainedNodes = cast<MDTuple>(getRawRetainedNodes());
1451+
SmallVector<Metadata *> MDs;
1452+
MDs.reserve(RetainedNodes->getNumOperands());
1453+
for (const MDOperand &Node : RetainedNodes->operands()) {
1454+
// Ignore malformed retainedNodes.
1455+
if (Node && !isa<DINode>(Node))
1456+
return;
1457+
1458+
auto *N = cast_or_null<DINode>(Node);
1459+
if (!IsAlienType(N))
1460+
MDs.push_back(N);
1461+
}
1462+
1463+
if (MDs.size() != RetainedNodes->getNumOperands())
1464+
replaceRetainedNodes(MDNode::get(getContext(), MDs));
1465+
}
1466+
14281467
DILexicalBlockBase::DILexicalBlockBase(LLVMContext &C, unsigned ID,
14291468
StorageType Storage,
14301469
ArrayRef<Metadata *> Ops)

‎llvm/lib/IR/Verifier.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1568,6 +1568,16 @@ void Verifier::visitDISubprogram(const DISubprogram &N) {
15681568
"invalid retained nodes, expected DILocalVariable, DILabel, "
15691569
"DIImportedEntity or DIType",
15701570
&N, Node, Op);
1571+
if (auto *T = dyn_cast<DIType>(Op)) {
1572+
auto *TypeScope = dyn_cast_or_null<DILocalScope>(T->getScope());
1573+
DISubprogram *TypeSP = TypeScope ? TypeScope->getSubprogram() : nullptr;
1574+
DICompileUnit *TypeSPUnit = TypeSP ? TypeSP->getUnit() : nullptr;
1575+
if (isa_and_nonnull<DILocalScope>(T->getScope()))
1576+
CheckDI(!TypeScope || TypeSP == &N,
1577+
"invalid retained node, DIType should have the scope of "
1578+
"DISubprogram",
1579+
&N, N.getUnit(), Node, Op, TypeScope, TypeSP, TypeSPUnit);
1580+
}
15711581
}
15721582
}
15731583
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)