[llvm] b81a337 - [LoopUnroll] Avoid UB when converting from WeakVH to `Value *`

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 23 10:36:58 PST 2020


Author: Justin Bogner
Date: 2020-01-23T10:36:39-08:00
New Revision: b81a337be7bcc84b2ab052103ce918c9fbfc839a

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

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

Calling `operator*` on a WeakVH with a null value yields a null
reference, which is UB. Avoid this by implicitly converting the WeakVH
to a `Value *` rather than dereferencing and then taking the address
for the type conversion.

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

Added: 
    llvm/test/Transforms/LoopUnroll/partial-unroll-dead-instructions.ll

Modified: 
    llvm/include/llvm/IR/ValueHandle.h
    llvm/lib/Transforms/Utils/LoopUnroll.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h
index 50b7701f6716..a21d017b193a 100644
--- a/llvm/include/llvm/IR/ValueHandle.h
+++ b/llvm/include/llvm/IR/ValueHandle.h
@@ -89,7 +89,11 @@ class ValueHandleBase {
   }
 
   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; }

diff  --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
index 4b94b371e70a..bb0b6ed848cb 100644
--- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -207,10 +207,11 @@ void llvm::simplifyLoopAfterUnroll(Loop *L, bool SimplifyIVs, LoopInfo *LI,
 
     // 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

diff  --git a/llvm/test/Transforms/LoopUnroll/partial-unroll-dead-instructions.ll b/llvm/test/Transforms/LoopUnroll/partial-unroll-dead-instructions.ll
new file mode 100644
index 000000000000..9cfa8e1e6419
--- /dev/null
+++ b/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
+}


        


More information about the llvm-commits mailing list