[PATCH] D73280: [LoopUnroll] Avoid UB when converting from WeakVH to `Value *`

[Justin Bogner via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb81a337be7bc: [LoopUnroll] Avoid UB when converting from WeakVH to `Value *` (authored by bogner).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73280

Files:
  llvm/include/llvm/IR/ValueHandle.h
  llvm/lib/Transforms/Utils/LoopUnroll.cpp
  llvm/test/Transforms/LoopUnroll/partial-unroll-dead-instructions.ll


Index: llvm/test/Transforms/LoopUnroll/partial-unroll-dead-instructions.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/LoopUnroll/partial-unroll-dead-instructions.ll
@@ -0,0 +1,42 @@
+; RUN: opt -S < %s -loop-unroll -unroll-allow-partial=1 | FileCheck %s
+;
+; Bugpointed test that triggered UB while cleaning up dead
+; instructions after simplifying indvars
+
+; We just check that some unrolling happened here - the assert we've
+; added to ValueHandleBase::operator* would fire if the bug was still
+; present.
+; CHECK: atomicrmw volatile add i32*
+; CHECK: atomicrmw volatile add i32*
+; CHECK: atomicrmw volatile add i32*
+
+ at global = external global i32, align 4
+
+define void @widget() {
+bb:
+  br label %bb1
+
+bb1:
+  br label %bb2
+
+bb2:
+  %tmp = phi i32 [ 0, %bb1 ], [ %tmp34, %bb33 ]
+  %tmp3 = phi i32 [ 0, %bb1 ], [ %tmp34, %bb33 ]
+  %tmp26 = and i32 %tmp, 1073741823
+  %tmp27 = getelementptr inbounds i32, i32* @global, i32 %tmp26
+  %tmp28 = atomicrmw volatile add i32* %tmp27, i32 1 monotonic
+  %tmp29 = icmp ugt i32 %tmp28, 23
+  %tmp30 = shl i32 %tmp, 6
+  %tmp31 = add i32 %tmp30, undef
+  %tmp32 = add i32 %tmp31, %tmp28
+  store i32 undef, i32* undef, align 4
+  br label %bb33
+
+bb33:
+  %tmp34 = add nuw nsw i32 %tmp3, 1
+  %tmp35 = icmp ult i32 %tmp3, 15
+  br i1 %tmp35, label %bb2, label %bb36
+
+bb36:
+  br label %bb1
+}
Index: llvm/lib/Transforms/Utils/LoopUnroll.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -207,10 +207,11 @@
 
     // Aggressively clean up dead instructions that simplifyLoopIVs already
     // identified. Any remaining should be cleaned up below.
-    while (!DeadInsts.empty())
-      if (Instruction *Inst =
-              dyn_cast_or_null<Instruction>(&*DeadInsts.pop_back_val()))
+    while (!DeadInsts.empty()) {
+      Value *V = DeadInsts.pop_back_val();
+      if (Instruction *Inst = dyn_cast_or_null<Instruction>(V))
         RecursivelyDeleteTriviallyDeadInstructions(Inst);
+    }
   }
 
   // At this point, the code is well formed.  We now do a quick sweep over the
Index: llvm/include/llvm/IR/ValueHandle.h
===================================================================
--- llvm/include/llvm/IR/ValueHandle.h
+++ llvm/include/llvm/IR/ValueHandle.h
@@ -89,7 +89,11 @@
   }
 
   Value *operator->() const { return getValPtr(); }
-  Value &operator*() const { return *getValPtr(); }
+  Value &operator*() const {
+    Value *V = getValPtr();
+    assert(V && "Dereferencing deleted ValueHandle");
+    return *V;
+  }
 
 protected:
   Value *getValPtr() const { return Val; }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73280.239941.patch
Type: text/x-patch
Size: 2738 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200123/67385d9a/attachment.bin>