Skip to content

Commit 99884a4

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 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 d31efd5 commit 99884a4

File tree

15 files changed

+214
-49
lines changed

15 files changed

+214
-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
@@ -2595,6 +2595,21 @@ class DISubprogram : public DILocalScope {
25952595
});
25962596
}
25972597

2598+
/// Remove types that do not belong to the subprogram's scope from
2599+
/// retainedNodes list.
2600+
void cleanupRetainedNodes();
2601+
2602+
/// When DebugTypeODRUniquing is enabled, after multiple modules are loaded,
2603+
/// some subprograms (that are from different compilation units, usually)
2604+
/// may have references to the same local type in their retainedNodes lists.
2605+
///
2606+
/// Clean up such references.
2607+
template <typename RangeT>
2608+
static void cleanupRetainedNodes(const RangeT &NewDistinctSPs) {
2609+
for (DISubprogram *SP : NewDistinctSPs)
2610+
SP->cleanupRetainedNodes();
2611+
}
2612+
25982613
/// Check if this subprogram describes the given function.
25992614
///
26002615
/// 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.
@@ -5991,6 +5994,10 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) {
59915994
thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val,
59925995
declaration.Val, retainedNodes.Val, thrownTypes.Val, annotations.Val,
59935996
targetFuncName.Val, keyInstructions.Val));
5997+
5998+
if (IsDistinct)
5999+
NewDistinctSPs.push_back(cast<DISubprogram>(Result));
6000+
59946001
return false;
59956002
}
59966003

‎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

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

755737
void callMDTypeCallback(Metadata **Val, unsigned TypeID);
@@ -1128,7 +1110,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
11281110
// placeholders, that we flush here.
11291111
resolveForwardRefsAndPlaceholders(Placeholders);
11301112
upgradeDebugInfo(ModuleLevel);
1131-
cloneLocalTypes();
11321113
// Return at the beginning of the block, since it is easy to skip it
11331114
// entirely from there.
11341115
Stream.ReadBlockEnd(); // Pop the abbrev block context.
@@ -1160,7 +1141,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
11601141
case BitstreamEntry::EndBlock:
11611142
resolveForwardRefsAndPlaceholders(Placeholders);
11621143
upgradeDebugInfo(ModuleLevel);
1163-
cloneLocalTypes();
11641144
return Error::success();
11651145
case BitstreamEntry::Record:
11661146
// The interesting case.
@@ -2043,6 +2023,9 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
20432023
MetadataList.assignValue(SP, NextMetadataNo);
20442024
NextMetadataNo++;
20452025

2026+
if (IsDistinct)
2027+
NewDistinctSPs.push_back(SP);
2028+
20462029
// Upgrade sp->function mapping to function->sp mapping.
20472030
if (HasFn) {
20482031
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
@@ -1469,6 +1469,44 @@ DILocalScope *DISubprogram::getRetainedNodeScope(MDNode *N) {
14691469
return cast<DILocalScope>(getRawRetainedNodeScope(N));
14701470
}
14711471

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

‎llvm/lib/IR/Verifier.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,10 +1576,13 @@ 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 = RetainedNodeSP ? RetainedNodeSP->getUnit() : nullptr;
15791582
CheckDI(
1580-
RetainedNodeScope->getSubprogram() == &N,
1583+
RetainedNodeSP == &N,
15811584
"invalid retained nodes, retained node does not belong to subprogram",
1582-
&N, Node, RetainedNode, RetainedNodeScope);
1585+
&N, Node, RetainedNode, RetainedNodeScope, RetainedNodeSP, RetainedNodeUnit);
15831586
}
15841587
}
15851588
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)