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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis created this revision.
eugenis added reviewers: glider, dvyukov, efriedma.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

Select folding in JumpThreading can create a conditional branch on a
code patch that did not have one in the original program. This is not a
valid transformation in sanitize_memory functions.

Note that JumpThreading does select folding in 3 different places. Two
of them seem safe - they apply to a select instruction in a BB that ends
with an unconditional branch to another BB, which (in turn) ends with a
conditional branch or a switch with the same condition.

Fixes PR45220.


Repository:
  rG LLVM Github Monorepo

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
@@ -2821,6 +2821,13 @@
 /// select is not jump-threaded, it will be folded again in the later
 /// optimizations.
 bool JumpThreadingPass::TryToUnfoldSelectInCurrBB(BasicBlock *BB) {
+  // Under MemorySanitizer, conditional branch with a condition that depends on
+  // an undef value has an externally visible side effect. This transformation
+  // can introduce such branch on a code path that did not have one before. It
+  // is invalid under MSan.
+  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.250929.patch
Type: text/x-patch
Size: 1845 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200317/52fa8b97/attachment.bin>