[llvm] f9471b0 - Fix MSan false positive due to select folding.

[Evgenii Stepanov via llvm-commits]

Author: Evgenii Stepanov
Date: 2020-03-31T15:25:42-07:00
New Revision: f9471b001089c744050c7a9cff39ebda2ff69011

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

LOG: Fix MSan false positive due to select folding.

Summary:
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.

Reviewers: glider, dvyukov, efriedma

Subscribers: hiraditya, llvm-commits

Tags: #llvm

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

Added: 
    llvm/test/Transforms/JumpThreading/select-unfold-msan.ll

Modified: 
    llvm/lib/Transforms/Scalar/JumpThreading.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 327a1a6f2e7b..a9a0070c1d57 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2833,6 +2833,16 @@ bool JumpThreadingPass::TryToUnfoldSelect(CmpInst *CondCmp, BasicBlock *BB) {
 /// 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))

diff  --git a/llvm/test/Transforms/JumpThreading/select-unfold-msan.ll b/llvm/test/Transforms/JumpThreading/select-unfold-msan.ll
new file mode 100644
index 000000000000..ea336e0f0f7e
--- /dev/null
+++ b/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
+}
+