blob: 6e5e09b8f1027c24035a87101d0b82edbc356878 [file] [log] [blame]
commit 01701646d5555409a6fad920f0e4801d79c154ea
Author: Duncan P. N. Exon Smith <dexonsmith@apple.com>
Date: Mon Feb 15 12:08:06 2021 -0800
Transforms: Clone distinct nodes in metadata mapper unless RF_ReuseAndMutateDistinctMDs
This is a follow up to 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a and a
revert of df763188c9a1ecb1e7e5c4d4ea53a99fbb755903.
With this change, we only skip cloning distinct nodes in
MDNodeMapper::mapDistinct if RF_ReuseAndMutateDistinctMDs, dropping the
no-longer-needed local helper `cloneOrBuildODR()`. Skipping cloning in
other cases is unsound and breaks CloneModule, which is why the textual
IR for PR48841 didn't pass previously. This commit adds the test as:
Transforms/ThinLTOBitcodeWriter/cfi-debug-info-cloned-type-references-global-value.ll
Cloning less often exposed a hole in subprogram cloning in
CloneFunctionInto thanks to df763188c9a1ecb1e7e5c4d4ea53a99fbb755903's
test ThinLTO/X86/Inputs/dicompositetype-unique-alias.ll. If a function
has a subprogram attachment whose scope is a DICompositeType that
shouldn't be cloned, but it has no internal debug info pointing at that
type, that composite type was being cloned. This commit plugs that hole,
calling DebugInfoFinder::processSubprogram from CloneFunctionInto.
As hinted at in 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a's commit
message, I think we need to formalize ownership of metadata a bit more
so that ValueMapper/CloneFunctionInto (and similar functions) can deal
with cloning (or not) metadata in a more generic, less fragile way.
This fixes PR48841.
Differential Revision: https://reviews.llvm.org/D96734
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index e7c1d9a90677..b33e0a90ea41 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -81,6 +81,9 @@ public:
/// Process debug info location.
void processLocation(const Module &M, const DILocation *Loc);
+ /// Process subprogram.
+ void processSubprogram(DISubprogram *SP);
+
/// Clear all lists.
void reset();
@@ -89,7 +92,6 @@ private:
void processCompileUnit(DICompileUnit *CU);
void processScope(DIScope *Scope);
- void processSubprogram(DISubprogram *SP);
void processType(DIType *DT);
bool addCompileUnit(DICompileUnit *CU);
bool addGlobalVariable(DIGlobalVariableExpression *DIG);
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 021c0bb9a1df..0ff28e9b3c29 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -144,6 +144,8 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
DIFinder.emplace();
SPClonedWithinModule = OldFunc->getSubprogram();
+ if (SPClonedWithinModule)
+ DIFinder->processSubprogram(SPClonedWithinModule);
} else {
assert((NewFunc->getParent() == nullptr ||
NewFunc->getParent() != OldFunc->getParent()) &&
@@ -195,7 +197,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
}
if (Changes < CloneFunctionChangeType::DifferentModule &&
- (SPClonedWithinModule || DIFinder->subprogram_count() > 0)) {
+ DIFinder->subprogram_count() > 0) {
// Turn on module-level changes, since we need to clone (some of) the
// debug info metadata.
//
@@ -208,14 +210,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
(void)VMap.MD().try_emplace(N, N);
};
- // Avoid cloning what the subprogram references.
- if (SPClonedWithinModule) {
- mapToSelfIfNew(SPClonedWithinModule->getUnit());
- mapToSelfIfNew(SPClonedWithinModule->getType());
- mapToSelfIfNew(SPClonedWithinModule->getFile());
- }
-
- // Avoid cloning other subprograms, compile units, and types.
+ // Avoid cloning types, compile units, and (other) subprograms.
for (DISubprogram *ISP : DIFinder->subprograms())
if (ISP != SPClonedWithinModule)
mapToSelfIfNew(ISP);
@@ -225,6 +220,9 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
for (DIType *Type : DIFinder->types())
mapToSelfIfNew(Type);
+ } else {
+ assert(!SPClonedWithinModule &&
+ "Subprogram should be in DIFinder->subprogram_count()...");
}
// Duplicate the metadata that is attached to the cloned function.
diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp
index 9557fa9becf0..0b1d8c024d84 100644
--- a/llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -533,23 +533,13 @@ Optional<Metadata *> MDNodeMapper::tryToMapOperand(const Metadata *Op) {
return None;
}
-static Metadata *cloneOrBuildODR(const MDNode &N) {
- auto *CT = dyn_cast<DICompositeType>(&N);
- // If ODR type uniquing is enabled, we would have uniqued composite types
- // with identifiers during bitcode reading, so we can just use CT.
- if (CT && CT->getContext().isODRUniquingDebugTypes() &&
- CT->getIdentifier() != "")
- return const_cast<DICompositeType *>(CT);
- return MDNode::replaceWithDistinct(N.clone());
-}
-
MDNode *MDNodeMapper::mapDistinctNode(const MDNode &N) {
assert(N.isDistinct() && "Expected a distinct node");
assert(!M.getVM().getMappedMD(&N) && "Expected an unmapped node");
- DistinctWorklist.push_back(
- cast<MDNode>((M.Flags & RF_ReuseAndMutateDistinctMDs)
- ? M.mapToSelf(&N)
- : M.mapToMetadata(&N, cloneOrBuildODR(N))));
+ DistinctWorklist.push_back(cast<MDNode>(
+ (M.Flags & RF_ReuseAndMutateDistinctMDs)
+ ? M.mapToSelf(&N)
+ : M.mapToMetadata(&N, MDNode::replaceWithDistinct(N.clone()))));
return DistinctWorklist.back();
}
diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-debug-info-cloned-type-references-global-value.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-debug-info-cloned-type-references-global-value.ll
new file mode 100644
index 000000000000..6d77b19e13c8
--- /dev/null
+++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-debug-info-cloned-type-references-global-value.ll
@@ -0,0 +1,42 @@
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t %s
+; RUN: llvm-modextract -b -n 0 -o - %t | llvm-dis | FileCheck %s
+
+; Crash test for CloneModule when there's a retained DICompositeType that
+; transitively references a global value.
+
+; CHECK: declare !type !{{[0-9]+}} !type !{{[0-9]+}} void @_Z1gIM1iKFivEEvT_(i64, i64)
+; CHECK: !llvm.dbg.cu
+; CHECK-DAG: distinct !DICompositeType({{.*}}, identifier: "_ZTS1oI1iiXadL_ZNKS0_5m_fn1EvEEE"
+; CHECK-DAG: distinct !DICompositeType({{.*}}, identifier: "_ZTS1i"
+; CHECK-DAG: !{i32 4, !"CFI Canonical Jump Tables", i32 0}
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@_ZN1i1pE = dso_local constant [1 x i8] zeroinitializer, align 1
+@_ZNK1i5m_fn1Ev = external global i32
+
+declare !type !17 !type !18 void @_Z1gIM1iKFivEEvT_(i64, i64)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!14, !15}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 12.0.0 (git@github.com:llvm/llvm-project.git 51bf4c0e6d4cbc6dfa57857fc78003413cbeb17f)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !2, nameTableKind: None)
+!1 = !DIFile(filename: "<stdin>", directory: "/tmp")
+!2 = !{}
+!3 = !{!4}
+!4 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "o<i, int, &i::m_fn1>", file: !5, line: 22, size: 8, flags: DIFlagTypePassByValue | DIFlagNonTrivial, elements: !2, templateParams: !6, identifier: "_ZTS1oI1iiXadL_ZNKS0_5m_fn1EvEEE")
+!5 = !DIFile(filename: "t.ii", directory: "/tmp")
+!6 = !{!7}
+!7 = !DITemplateValueParameter(type: !8, value: i64 ptrtoint (i32* @_ZNK1i5m_fn1Ev to i64))
+!8 = !DIDerivedType(tag: DW_TAG_ptr_to_member_type, baseType: !9, size: 128, extraData: !13)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !12}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size: 64, flags: DIFlagArtificial)
+!13 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "i", file: !5, line: 13, size: 8, flags: DIFlagTypePassByValue | DIFlagNonTrivial, elements: !2, identifier: "_ZTS1i")
+!14 = !{i32 2, !"Debug Info Version", i32 3}
+!15 = !{i32 4, !"CFI Canonical Jump Tables", i32 0}
+!16 = !{i64 ptrtoint (i32* @_ZNK1i5m_fn1Ev to i64)}
+!17 = !{i64 0, !"_ZTSFvM1iKFivEE"}
+!18 = !{i64 0, !"_ZTSFvM1iKFivEE.generalized"}