[llvm] r226866 - IR: Update references to temporaries before deleting

[Duncan P. N. Exon Smith]

Author: dexonsmith
Date: Thu Jan 22 15:36:45 2015
New Revision: 226866

URL: http://llvm.org/viewvc/llvm-project?rev=226866&view=rev
Log:
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.)

Added:
    llvm/trunk/test/Assembler/invalid-mdnode-badref.ll
Modified:
    llvm/trunk/include/llvm/IR/Metadata.h
    llvm/trunk/lib/IR/Metadata.cpp
    llvm/trunk/unittests/IR/MetadataTest.cpp

Modified: llvm/trunk/include/llvm/IR/Metadata.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=226866&r1=226865&r2=226866&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Metadata.h (original)
+++ llvm/trunk/include/llvm/IR/Metadata.h Thu Jan 22 15:36:45 2015
@@ -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(); }

Modified: llvm/trunk/lib/IR/Metadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=226866&r1=226865&r2=226866&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Metadata.cpp (original)
+++ llvm/trunk/lib/IR/Metadata.cpp Thu Jan 22 15:36:45 2015
@@ -788,6 +788,7 @@ GenericDwarfNode *GenericDwarfNode::getI
 
 void MDNode::deleteTemporary(MDNode *N) {
   assert(N->isTemporary() && "Expected temporary node");
+  N->replaceAllUsesWith(nullptr);
   N->deleteAsSubclass();
 }
 

Added: llvm/trunk/test/Assembler/invalid-mdnode-badref.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/invalid-mdnode-badref.ll?rev=226866&view=auto
==============================================================================
--- llvm/trunk/test/Assembler/invalid-mdnode-badref.ll (added)
+++ llvm/trunk/test/Assembler/invalid-mdnode-badref.ll Thu Jan 22 15:36:45 2015
@@ -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}

Modified: llvm/trunk/unittests/IR/MetadataTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=226866&r1=226865&r2=226866&view=diff
==============================================================================
--- llvm/trunk/unittests/IR/MetadataTest.cpp (original)
+++ llvm/trunk/unittests/IR/MetadataTest.cpp Thu Jan 22 15:36:45 2015
@@ -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) {