[PATCH] D76332: Fix MSan false positive due to select folding.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis updated this revision to Diff 253719.
eugenis added a comment.

updated the comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76332

Files:
  llvm/lib/Transforms/Scalar/JumpThreading.cpp
  llvm/test/Transforms/JumpThreading/select-unfold-msan.ll


Index: llvm/test/Transforms/JumpThreading/select-unfold-msan.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/JumpThreading/select-unfold-msan.ll
@@ -0,0 +1,28 @@
+; PR45220
+; RUN: opt -S -jump-threading < %s | FileCheck %s
+
+declare i1 @NOP()
+
+define dso_local i32 @f(i1 %b, i1 %u) sanitize_memory {
+entry:
+  br i1 %b, label %if.end, label %if.else
+
+if.else:
+  %call = call i1 @NOP()
+  br label %if.end
+
+if.end:
+; Check that both selects in this BB are still in place,
+; and were not replaced with a conditional branch.
+; CHECK:      phi
+; CHECK-NEXT: phi
+; CHECK-NEXT: select
+; CHECK-NEXT: select
+; CHECK-NEXT: ret
+  %u1 = phi i1 [ true, %if.else ], [ %u, %entry ]
+  %v = phi i1 [ %call, %if.else ], [ false, %entry ]
+  %s = select i1 %u1, i32 22, i32 0
+  %v1 = select i1 %v, i32 %s, i32 42
+  ret i32 %v1
+}
+
Index: llvm/lib/Transforms/Scalar/JumpThreading.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2833,6 +2833,16 @@
 /// select is not jump-threaded, it will be folded again in the later
 /// optimizations.
 bool JumpThreadingPass::TryToUnfoldSelectInCurrBB(BasicBlock *BB) {
+  // This transform can introduce a UB (a conditional branch that depends on a
+  // poison value) that was not present in the original program. See
+  // @TryToUnfoldSelectInCurrBB test in test/Transforms/JumpThreading/select.ll.
+  // Disable this transform under MemorySanitizer.
+  // FIXME: either delete it or replace with a valid transform. This issue is
+  // not limited to MemorySanitizer (but has only been observed as an MSan false
+  // positive in practice so far).
+  if (BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory))
+    return false;
+
   // If threading this would thread across a loop header, don't thread the edge.
   // See the comments above FindLoopHeaders for justifications and caveats.
   if (LoopHeaders.count(BB))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76332.253719.patch
Type: text/x-patch
Size: 2056 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200330/b0ee7d7b/attachment-0001.bin>