[llvm] b608053 - [IR] Fix replaceUsesWithIf ponetial issue with constants

[Stanislav Mekhanoshin via llvm-commits]

Author: Stanislav Mekhanoshin
Date: 2021-06-28T15:55:55-07:00
New Revision: b608053efb88378900d0f08148662e433aa609db

URL: https://github.com/llvm/llvm-project/commit/b608053efb88378900d0f08148662e433aa609db
DIFF: https://github.com/llvm/llvm-project/commit/b608053efb88378900d0f08148662e433aa609db.diff

LOG: [IR] Fix replaceUsesWithIf ponetial issue with constants

There can be a use after free in the Value::replaceUsesWithIf()
if two uses point to the same constant. Patch defers handling
of the constants past the iterator scan.

Another potential issue is that handleOperandChange updates all
the uses in a given Constant, not just the one passed to
ShouldReplace. Added a FIXME comment.

Both issues are not currently exploitable as the only use of
this call with constants avoids it.

Differential Revision: https://reviews.llvm.org/D105061

Added: 
    

Modified: 
    llvm/lib/IR/Value.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index c3796f134922..9cb00569598a 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -531,6 +531,9 @@ void Value::replaceUsesWithIf(Value *New,
   assert(New->getType() == getType() &&
          "replaceUses of value with new value of 
diff erent type!");
 
+  SmallVector<TrackingVH<Constant>, 8> Consts;
+  SmallPtrSet<Constant *, 8> Visited;
+
   for (use_iterator UI = use_begin(), E = use_end(); UI != E;) {
     Use &U = *UI;
     ++UI;
@@ -540,12 +543,19 @@ void Value::replaceUsesWithIf(Value *New,
     // constant because they are uniqued.
     if (auto *C = dyn_cast<Constant>(U.getUser())) {
       if (!isa<GlobalValue>(C)) {
-        C->handleOperandChange(this, New);
+        if (Visited.insert(C).second)
+          Consts.push_back(TrackingVH<Constant>(C));
         continue;
       }
     }
     U.set(New);
   }
+
+  while (!Consts.empty()) {
+    // FIXME: handleOperandChange() updates all the uses in a given Constant,
+    //        not just the one passed to ShouldReplace
+    Consts.pop_back_val()->handleOperandChange(this, New);
+  }
 }
 
 /// Replace llvm.dbg.* uses of MetadataAsValue(ValueAsMetadata(V)) outside BB