diff --git a/lib/Transforms/Utils/ValueMapper.cpp b/lib/Transforms/Utils/ValueMapper.cpp index db8728e3fa9..49c0902addb 100644 --- a/lib/Transforms/Utils/ValueMapper.cpp +++ b/lib/Transforms/Utils/ValueMapper.cpp @@ -154,19 +154,20 @@ static Metadata *mapToSelf(ValueToValueMapTy &VM, const Metadata *MD) { return mapToMetadata(VM, MD, const_cast(MD)); } -static Metadata *MapMetadataImpl(const Metadata *MD, ValueToValueMapTy &VM, - RemapFlags Flags, +static Metadata *MapMetadataImpl(const Metadata *MD, + SmallVectorImpl &Cycles, + ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer); -static Metadata *mapMetadataOp(Metadata *Op, ValueToValueMapTy &VM, - RemapFlags Flags, +static Metadata *mapMetadataOp(Metadata *Op, SmallVectorImpl &Cycles, + ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) { if (!Op) return nullptr; if (Metadata *MappedOp = - MapMetadataImpl(Op, VM, Flags, TypeMapper, Materializer)) + MapMetadataImpl(Op, Cycles, VM, Flags, TypeMapper, Materializer)) return MappedOp; // Use identity map if MappedOp is null and we can ignore missing entries. if (Flags & RF_IgnoreMissingEntries) @@ -186,7 +187,8 @@ static Metadata *mapMetadataOp(Metadata *Op, ValueToValueMapTy &VM, /// Assumes that \c NewNode is already a clone of \c OldNode. /// /// \pre \c NewNode is a clone of \c OldNode. -static bool remap(const MDNode *OldNode, MDNode *NewNode, ValueToValueMapTy &VM, +static bool remap(const MDNode *OldNode, MDNode *NewNode, + SmallVectorImpl &Cycles, ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) { assert(OldNode->getNumOperands() == NewNode->getNumOperands() && @@ -202,8 +204,8 @@ static bool remap(const MDNode *OldNode, MDNode *NewNode, ValueToValueMapTy &VM, assert(NewNode->getOperand(I) == Old && "Expected old operands to already be in place"); - Metadata *New = mapMetadataOp(OldNode->getOperand(I), VM, Flags, TypeMapper, - Materializer); + Metadata *New = mapMetadataOp(OldNode->getOperand(I), Cycles, VM, Flags, + TypeMapper, Materializer); if (Old != New) { AnyChanged = true; NewNode->replaceOperandWith(I, New); @@ -216,29 +218,38 @@ static bool remap(const MDNode *OldNode, MDNode *NewNode, ValueToValueMapTy &VM, /// \brief Map a distinct MDNode. /// /// Distinct nodes are not uniqued, so they must always recreated. -static Metadata *mapDistinctNode(const MDNode *Node, ValueToValueMapTy &VM, - RemapFlags Flags, +static Metadata *mapDistinctNode(const MDNode *Node, + SmallVectorImpl &Cycles, + ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) { assert(Node->isDistinct() && "Expected distinct node"); MDNode *NewMD = MDNode::replaceWithDistinct(Node->clone()); - remap(Node, NewMD, VM, Flags, TypeMapper, Materializer); + remap(Node, NewMD, Cycles, VM, Flags, TypeMapper, Materializer); + + // Track any cycles beneath this node. + for (Metadata *Op : NewMD->operands()) + if (auto *Node = dyn_cast_or_null(Op)) + if (!Node->isResolved()) + Cycles.push_back(Node); + return NewMD; } /// \brief Map a uniqued MDNode. /// /// Uniqued nodes may not need to be recreated (they may map to themselves). -static Metadata *mapUniquedNode(const MDNode *Node, ValueToValueMapTy &VM, - RemapFlags Flags, +static Metadata *mapUniquedNode(const MDNode *Node, + SmallVectorImpl &Cycles, + ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) { assert(Node->isUniqued() && "Expected uniqued node"); // Create a temporary node upfront in case we have a metadata cycle. auto ClonedMD = Node->clone(); - if (!remap(Node, ClonedMD.get(), VM, Flags, TypeMapper, Materializer)) + if (!remap(Node, ClonedMD.get(), Cycles, VM, Flags, TypeMapper, Materializer)) // No operands changed, so use the identity mapping. return mapToSelf(VM, Node); @@ -247,8 +258,9 @@ static Metadata *mapUniquedNode(const MDNode *Node, ValueToValueMapTy &VM, MDNode::replaceWithUniqued(std::move(ClonedMD))); } -static Metadata *MapMetadataImpl(const Metadata *MD, ValueToValueMapTy &VM, - RemapFlags Flags, +static Metadata *MapMetadataImpl(const Metadata *MD, + SmallVectorImpl &Cycles, + ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) { // If the value already exists in the map, use it. @@ -288,19 +300,32 @@ static Metadata *MapMetadataImpl(const Metadata *MD, ValueToValueMapTy &VM, return mapToSelf(VM, MD); if (Node->isDistinct()) - return mapDistinctNode(Node, VM, Flags, TypeMapper, Materializer); + return mapDistinctNode(Node, Cycles, VM, Flags, TypeMapper, Materializer); - return mapUniquedNode(Node, VM, Flags, TypeMapper, Materializer); + return mapUniquedNode(Node, Cycles, VM, Flags, TypeMapper, Materializer); } Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM, RemapFlags Flags, ValueMapTypeRemapper *TypeMapper, ValueMaterializer *Materializer) { - Metadata *NewMD = MapMetadataImpl(MD, VM, Flags, TypeMapper, Materializer); - if (NewMD && NewMD != MD) + SmallVector Cycles; + Metadata *NewMD = + MapMetadataImpl(MD, Cycles, VM, Flags, TypeMapper, Materializer); + + // Resolve cycles underneath MD. + if (NewMD && NewMD != MD) { if (auto *N = dyn_cast(NewMD)) if (!N->isResolved()) N->resolveCycles(); + + for (MDNode *N : Cycles) + if (!N->isResolved()) + N->resolveCycles(); + } else { + // Shouldn't get unresolved cycles if nothing was remapped. + assert(Cycles.empty() && "Expected no unresolved cycles"); + } + return NewMD; } diff --git a/test/Linker/distinct-cycles.ll b/test/Linker/distinct-cycles.ll new file mode 100644 index 00000000000..b9b496c50c1 --- /dev/null +++ b/test/Linker/distinct-cycles.ll @@ -0,0 +1,13 @@ +; RUN: llvm-link -o - -S %s | FileCheck %s +; Crasher for PR22456: MapMetadata() should resolve all cycles. + +; CHECK: !named = !{!0} +!named = !{!0} + +; CHECK: !0 = distinct !{!1} +!0 = distinct !{!1} + +; CHECK-NEXT: !1 = !{!2} +; CHECK-NEXT: !2 = !{!1} +!1 = !{!2} +!2 = !{!1}