From 003346177cff3ac5c6f533cbd41a186980116571 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Thu, 22 Jan 2015 21:36:45 +0000 Subject: [PATCH] IR: Update references to temporaries before deleting During `MDNode::deleteTemporary()`, call `replaceAllUsesWith(nullptr)` to update all tracking references to `nullptr`. This fixes PR22280, where inverted destruction order between tracking references and the temporaries themselves caused a use-after-free in `LLParser`. An alternative fix would be to add an assertion that there are no users, and continue to fix inverted destruction order in clients (like `LLParser`), but instead I decided to make getting-teardown-right easy. (If someone disagrees let me know.) git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@226866 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/Metadata.h | 3 ++- lib/IR/Metadata.cpp | 1 + test/Assembler/invalid-mdnode-badref.ll | 5 +++++ unittests/IR/MetadataTest.cpp | 11 +++++++++++ 4 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 test/Assembler/invalid-mdnode-badref.ll diff --git a/include/llvm/IR/Metadata.h b/include/llvm/IR/Metadata.h index 5590bc54881..5a2630b64e8 100644 --- a/include/llvm/IR/Metadata.h +++ b/include/llvm/IR/Metadata.h @@ -727,7 +727,8 @@ public: /// \brief Deallocate a node created by getTemporary. /// - /// The node must not have any users. + /// Calls \c replaceAllUsesWith(nullptr) before deleting, so any remaining + /// references will be reset. static void deleteTemporary(MDNode *N); LLVMContext &getContext() const { return Context.getContext(); } diff --git a/lib/IR/Metadata.cpp b/lib/IR/Metadata.cpp index 6c8f71ffa99..fac9268f786 100644 --- a/lib/IR/Metadata.cpp +++ b/lib/IR/Metadata.cpp @@ -788,6 +788,7 @@ GenericDwarfNode *GenericDwarfNode::getImpl(LLVMContext &Context, unsigned Tag, void MDNode::deleteTemporary(MDNode *N) { assert(N->isTemporary() && "Expected temporary node"); + N->replaceAllUsesWith(nullptr); N->deleteAsSubclass(); } diff --git a/test/Assembler/invalid-mdnode-badref.ll b/test/Assembler/invalid-mdnode-badref.ll new file mode 100644 index 00000000000..cfa03e0b3c6 --- /dev/null +++ b/test/Assembler/invalid-mdnode-badref.ll @@ -0,0 +1,5 @@ +; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s +!named = !{!0} + +; CHECK: [[@LINE+1]]:14: error: use of undefined metadata '!1' +!0 = !{!0, !1} diff --git a/unittests/IR/MetadataTest.cpp b/unittests/IR/MetadataTest.cpp index e2ed3d7946b..b2205776f64 100644 --- a/unittests/IR/MetadataTest.cpp +++ b/unittests/IR/MetadataTest.cpp @@ -517,6 +517,17 @@ TEST_F(MDNodeTest, replaceWithDistinct) { } } +TEST_F(MDNodeTest, deleteTemporaryWithTrackingRef) { + TrackingMDRef Ref; + EXPECT_EQ(nullptr, Ref.get()); + { + auto Temp = MDTuple::getTemporary(Context, None); + Ref.reset(Temp.get()); + EXPECT_EQ(Temp.get(), Ref.get()); + } + EXPECT_EQ(nullptr, Ref.get()); +} + typedef MetadataTest MDLocationTest; TEST_F(MDLocationTest, Overflow) {