[IR] Fix replaceUsesWithIf ponetial issue with constants

[rampitec]

Summary

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.

[efriedma]

llvm/lib/IR/Value.cpp

534

I think you need a TrackingVH here, instead of a raw pointer. Suppose you have something like the following:

ConstantStruct(ConstantStruct(C, C), C)

Say we replaceUsesWithIf on C. Calling handleOperandChange on the inner ConstantStruct will invalidate pointers to the outer CostantStruct. So depending on the use-list order, you might have a dangling pointer.

[rampitec]

Use TrackingVH.

llvm/lib/IR/Value.cpp

534

Like in the updated patch?

[efriedma]

LGTM

llvm/lib/IR/Value.cpp

 }
 
 void Value::replaceUsesWithIf(Value *New,
                               llvm::function_ref<bool(Use &U)> ShouldReplace) {
   assert(New && "Value::replaceUsesWithIf(<null>) is invalid!");
   assert(New->getType() == getType() &&
          "replaceUses of value with new value of different 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;
     if (!ShouldReplace(U))
       continue;
     // Must handle Constants specially, we cannot call replaceUsesOfWith on a
     // 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
 /// with New.
 static void replaceDbgUsesOutsideBlock(Value *V, Value *New, BasicBlock *BB) {
   SmallVector<DbgVariableIntrinsic *> DbgUsers;
   findDbgUsers(DbgUsers, V);
   for (auto *DVI : DbgUsers) {