From c742e3a68d27d14fa9b8dc96066a166499d63b7b Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Wed, 7 Jan 2015 21:32:27 +0000 Subject: [PATCH] Linker: Don't use MDNode::replaceOperandWith() `MDNode::replaceOperandWith()` changes all instances of metadata. Stop using it when linking module flags, since (due to uniquing) the flag values could be used by other metadata. Instead, use new API `NamedMDNode::setOperand()` to update the reference directly. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@225397 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/Metadata.h | 1 + lib/IR/Metadata.cpp | 5 +++ lib/Linker/LinkModules.cpp | 32 ++++++++++++------- .../Inputs/module-flags-dont-change-others.ll | 8 +++++ .../Linker/module-flags-dont-change-others.ll | 26 +++++++++++++++ 5 files changed, 61 insertions(+), 11 deletions(-) create mode 100644 test/Linker/Inputs/module-flags-dont-change-others.ll create mode 100644 test/Linker/module-flags-dont-change-others.ll diff --git a/include/llvm/IR/Metadata.h b/include/llvm/IR/Metadata.h index 241d5fe2ed2..a010341305c 100644 --- a/include/llvm/IR/Metadata.h +++ b/include/llvm/IR/Metadata.h @@ -847,6 +847,7 @@ public: MDNode *getOperand(unsigned i) const; unsigned getNumOperands() const; void addOperand(MDNode *M); + void setOperand(unsigned I, MDNode *New); StringRef getName() const; void print(raw_ostream &ROS) const; void dump() const; diff --git a/lib/IR/Metadata.cpp b/lib/IR/Metadata.cpp index 1a651573733..cc3d13992b7 100644 --- a/lib/IR/Metadata.cpp +++ b/lib/IR/Metadata.cpp @@ -836,6 +836,11 @@ MDNode *NamedMDNode::getOperand(unsigned i) const { void NamedMDNode::addOperand(MDNode *M) { getNMDOps(Operands).emplace_back(M); } +void NamedMDNode::setOperand(unsigned I, MDNode *New) { + assert(I < getNumOperands() && "Invalid operand number"); + getNMDOps(Operands)[I].reset(New); +} + void NamedMDNode::eraseFromParent() { getParent()->eraseNamedMetadata(this); } diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp index f9fba5e0a80..7a8025f897e 100644 --- a/lib/Linker/LinkModules.cpp +++ b/lib/Linker/LinkModules.cpp @@ -1312,7 +1312,7 @@ bool ModuleLinker::linkModuleFlagsMetadata() { } // First build a map of the existing module flags and requirements. - DenseMap Flags; + DenseMap> Flags; SmallSetVector Requirements; for (unsigned I = 0, E = DstModFlags->getNumOperands(); I != E; ++I) { MDNode *Op = DstModFlags->getOperand(I); @@ -1322,7 +1322,7 @@ bool ModuleLinker::linkModuleFlagsMetadata() { if (Behavior->getZExtValue() == Module::Require) { Requirements.insert(cast(Op->getOperand(2))); } else { - Flags[ID] = Op; + Flags[ID] = std::make_pair(Op, I); } } @@ -1334,7 +1334,9 @@ bool ModuleLinker::linkModuleFlagsMetadata() { ConstantInt *SrcBehavior = mdconst::extract(SrcOp->getOperand(0)); MDString *ID = cast(SrcOp->getOperand(1)); - MDNode *DstOp = Flags.lookup(ID); + MDNode *DstOp; + unsigned DstIndex; + std::tie(DstOp, DstIndex) = Flags.lookup(ID); unsigned SrcBehaviorValue = SrcBehavior->getZExtValue(); // If this is a requirement, add it and continue. @@ -1349,7 +1351,7 @@ bool ModuleLinker::linkModuleFlagsMetadata() { // If there is no existing flag with this ID, just add it. if (!DstOp) { - Flags[ID] = SrcOp; + Flags[ID] = std::make_pair(SrcOp, DstModFlags->getNumOperands()); DstModFlags->addOperand(SrcOp); continue; } @@ -1370,8 +1372,8 @@ bool ModuleLinker::linkModuleFlagsMetadata() { continue; } else if (SrcBehaviorValue == Module::Override) { // Update the destination flag to that of the source. - DstOp->replaceOperandWith(0, ConstantAsMetadata::get(SrcBehavior)); - DstOp->replaceOperandWith(2, SrcOp->getOperand(2)); + DstModFlags->setOperand(DstIndex, SrcOp); + Flags[ID].first = SrcOp; continue; } @@ -1382,6 +1384,13 @@ bool ModuleLinker::linkModuleFlagsMetadata() { continue; } + auto replaceDstValue = [&](MDNode *New) { + Metadata *FlagOps[] = {DstOp->getOperand(0), ID, New}; + MDNode *Flag = MDNode::get(DstM->getContext(), FlagOps); + DstModFlags->setOperand(DstIndex, Flag); + Flags[ID].first = Flag; + }; + // Perform the merge for standard behavior types. switch (SrcBehaviorValue) { case Module::Require: @@ -1411,7 +1420,8 @@ bool ModuleLinker::linkModuleFlagsMetadata() { MDs.push_back(DstValue->getOperand(i)); for (unsigned i = 0, e = SrcValue->getNumOperands(); i != e; ++i) MDs.push_back(SrcValue->getOperand(i)); - DstOp->replaceOperandWith(2, MDNode::get(DstM->getContext(), MDs)); + + replaceDstValue(MDNode::get(DstM->getContext(), MDs)); break; } case Module::AppendUnique: { @@ -1422,9 +1432,9 @@ bool ModuleLinker::linkModuleFlagsMetadata() { Elts.insert(DstValue->getOperand(i)); for (unsigned i = 0, e = SrcValue->getNumOperands(); i != e; ++i) Elts.insert(SrcValue->getOperand(i)); - DstOp->replaceOperandWith( - 2, MDNode::get(DstM->getContext(), - makeArrayRef(Elts.begin(), Elts.end()))); + + replaceDstValue(MDNode::get(DstM->getContext(), + makeArrayRef(Elts.begin(), Elts.end()))); break; } } @@ -1436,7 +1446,7 @@ bool ModuleLinker::linkModuleFlagsMetadata() { MDString *Flag = cast(Requirement->getOperand(0)); Metadata *ReqValue = Requirement->getOperand(1); - MDNode *Op = Flags[Flag]; + MDNode *Op = Flags[Flag].first; if (!Op || Op->getOperand(2) != ReqValue) { HasErr |= emitError("linking module flags '" + Flag->getString() + "': does not have the required value"); diff --git a/test/Linker/Inputs/module-flags-dont-change-others.ll b/test/Linker/Inputs/module-flags-dont-change-others.ll new file mode 100644 index 00000000000..61d57e52556 --- /dev/null +++ b/test/Linker/Inputs/module-flags-dont-change-others.ll @@ -0,0 +1,8 @@ +!llvm.module.flags = !{!3, !4, !5} + +!0 = !{} +!1 = !{!0} +!2 = !{!0, !1} +!3 = !{i32 4, !"foo", i32 37} ; Override the "foo" value. +!4 = !{i32 5, !"bar", !1} +!5 = !{i32 6, !"baz", !2} diff --git a/test/Linker/module-flags-dont-change-others.ll b/test/Linker/module-flags-dont-change-others.ll new file mode 100644 index 00000000000..4cc9f39027a --- /dev/null +++ b/test/Linker/module-flags-dont-change-others.ll @@ -0,0 +1,26 @@ +; RUN: llvm-link %s %S/Inputs/module-flags-dont-change-others.ll -S -o - | FileCheck %s + +; Test that module-flag linking doesn't change other metadata. In particular, +; !named should still point at the unmodified tuples (!3, !4, and !5) that +; happen to also serve as module flags. + +; CHECK: !named = !{!0, !1, !2, !3, !4, !5} +; CHECK: !llvm.module.flags = !{!6, !7, !8} +!named = !{!0, !1, !2, !3, !4, !5} +!llvm.module.flags = !{!3, !4, !5} + +; CHECK: !0 = !{} +; CHECK: !1 = !{!0} +; CHECK: !2 = !{!0, !1} +; CHECK: !3 = !{i32 1, !"foo", i32 927} +; CHECK: !4 = !{i32 5, !"bar", !0} +; CHECK: !5 = !{i32 6, !"baz", !1} +; CHECK: !6 = !{i32 4, !"foo", i32 37} +; CHECK: !7 = !{i32 5, !"bar", !1} +; CHECK: !8 = !{i32 6, !"baz", !2} +!0 = !{} +!1 = !{!0} +!2 = !{!0, !1} +!3 = !{i32 1, !"foo", i32 927} +!4 = !{i32 5, !"bar", !0} +!5 = !{i32 6, !"baz", !1}