[llvm] f9471b0 - Fix MSan false positive due to select folding.
Evgenii Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 31 15:25:55 PDT 2020
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
+}
+
More information about the llvm-commits
mailing list