[PATCH] D103051: [IR] Allow Value::replaceUsesWithIf() to process constants

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/IR/Value.cpp:543
+    Use &U = *UI;
+    ++UI;
+    if (!ShouldReplace(U))
----------------
efriedma wrote:
> I'm suspicious of the way you're iterating over the use list here; incrementing early isn't enough to avoid a use-after-free, I think.  The incremented `UI` might refer to the same constant.
> 
> `doRAUW` repeatedly accesses `*use_begin()` to avoid issues.
I do not think this is currently exploitable as the only caller with constants `convertConstantExprsToInstructions()` guards it, but this may be true in general. Since not all uses are going to be replaced I cannot do the same thing as `doRAUW`. Do you think this will resolve the potential issue?

```
@@ -527,22 +527,26 @@ void Value::replaceNonMetadataUsesWith(Value *New) {

 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!");

+  SmallPtrSet<Constant *, 8> Visited;
   for (use_iterator UI = use_begin(), E = use_end(); UI != E;) {
     Use &U = *UI;
     ++UI;
+    auto *C = dyn_cast<Constant>(U.getUser());
+    if (C && !Visited.insert(C).second)
+      continue;
     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 (C) {
       if (!isa<GlobalValue>(C)) {
         C->handleOperandChange(this, New);
         continue;
       }
     }
     U.set(New);
   }
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103051/new/

https://reviews.llvm.org/D103051