[llvm] Reapply "[msan] Switch switch() from strict handling to (icmp eq)-style handling" (#180636) (PR #181112)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 13 00:48:24 PST 2026
https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/181112
>From 15f46ed9153b8ef88fb4db63248c34873669ec52 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 11 Feb 2026 10:45:26 +0000
Subject: [PATCH 1/4] Reapply "[msan] Switch switch() from strict handling to
(icmp eq)-style handling" (#180636)
This reverts commit cfbb9a66ae92aa477c6ea38b006d64ec4b92170d.
---
.../Instrumentation/MemorySanitizer.cpp | 41 +++++++++++++++++++
.../MemorySanitizer/switch-icmp.ll | 25 ++++++++---
2 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 5e766d8e0ff8c..3e7eafd756e89 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2460,6 +2460,47 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
return Si;
}
+ // Instrument:
+ // switch i32 %Val, label %else [ i32 0, label %A
+ // i32 1, label %B
+ // i32 2, label %C ]
+ //
+ // Typically, the switch input value (%Val) is fully initialized.
+ //
+ // Sometimes the compiler may convert (icmp + br) into a switch statement.
+ // MSan allows icmp eq/ne with partly initialized inputs to still result in a
+ // fully initialized output, if there exists a bit that is initialized in
+ // both inputs with a differing value. For compatibility, we support this in
+ // the switch instrumentation as well. Note that this edge case only applies
+ // if the switch input value does not match *any* of the cases (matching any
+ // of the cases requires an exact, fully initialized match).
+ //
+ // ShadowCases = 0
+ // | propagateEqualityComparison(Val, 0)
+ // | propagateEqualityComparison(Val, 1)
+ // | propagateEqualityComparison(Val, 2))
+ void visitSwitchInst(SwitchInst &SI) {
+ IRBuilder<> IRB(&SI);
+
+ Value *Val = SI.getCondition();
+ Value *ShadowVal = getShadow(Val);
+
+ Value *ShadowCases = nullptr;
+ for (auto Case : SI.cases()) {
+ Value *Comparator = Case.getCaseValue();
+ Value *ComparisonShadow = propagateEqualityComparison(
+ IRB, Val, Comparator, ShadowVal, getShadow(Comparator));
+
+ if (ShadowCases)
+ ShadowCases = IRB.CreateOr(ShadowCases, ComparisonShadow);
+ else
+ ShadowCases = ComparisonShadow;
+ }
+
+ if (ShadowCases)
+ insertCheckShadow(ShadowCases, getOrigin(Val), &SI);
+ }
+
// Vector manipulation.
void visitExtractElementInst(ExtractElementInst &I) {
insertCheckShadowOf(I.getOperand(1), &I);
diff --git a/llvm/test/Instrumentation/MemorySanitizer/switch-icmp.ll b/llvm/test/Instrumentation/MemorySanitizer/switch-icmp.ll
index 08c9a5a62febf..08e56516ff5b5 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/switch-icmp.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/switch-icmp.ll
@@ -8,9 +8,10 @@
; uninitialized, if a bit is initialized in both inputs but has a different
; value.
;
-; TODO: since the compiler/optimizer may freely choose between (icmp eq + br)
-; vs. switch, MSan's switch instrumentation also needs to be able to
-; handle partly-uninitialized inputs.
+; If switch has a partly uninitialized input, but it is possible to rule out
+; matching any of the cases, it will use the default case instead of reporting
+; use-of-uninitialized memory. This is equivalent to if the switch was replaced
+; by a series of (icmp eq + br).
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
@@ -20,8 +21,22 @@ define i64 @switch_test(i32 %wii) sanitize_memory {
; CHECK-SAME: i32 [[WII:%.*]]) #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr @__msan_param_tls, align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[_MSCMP:%.*]] = icmp ne i32 [[TMP1]], 0
-; CHECK-NEXT: br i1 [[_MSCMP]], label %[[BB2:.*]], label %[[BB3:.*]], !prof [[PROF1:![0-9]+]]
+; CHECK-NEXT: [[TMP3:%.*]] = xor i32 [[WII]], 42
+; CHECK-NEXT: [[TMP4:%.*]] = or i32 [[TMP1]], 0
+; CHECK-NEXT: [[TMP5:%.*]] = icmp ne i32 [[TMP4]], 0
+; CHECK-NEXT: [[TMP6:%.*]] = xor i32 [[TMP4]], -1
+; CHECK-NEXT: [[TMP7:%.*]] = and i32 [[TMP6]], [[TMP3]]
+; CHECK-NEXT: [[TMP8:%.*]] = icmp eq i32 [[TMP7]], 0
+; CHECK-NEXT: [[_MSPROP_ICMP:%.*]] = and i1 [[TMP5]], [[TMP8]]
+; CHECK-NEXT: [[TMP9:%.*]] = xor i32 [[WII]], 43
+; CHECK-NEXT: [[TMP10:%.*]] = or i32 [[TMP1]], 0
+; CHECK-NEXT: [[TMP11:%.*]] = icmp ne i32 [[TMP10]], 0
+; CHECK-NEXT: [[TMP12:%.*]] = xor i32 [[TMP10]], -1
+; CHECK-NEXT: [[TMP13:%.*]] = and i32 [[TMP12]], [[TMP9]]
+; CHECK-NEXT: [[TMP14:%.*]] = icmp eq i32 [[TMP13]], 0
+; CHECK-NEXT: [[_MSPROP_ICMP1:%.*]] = and i1 [[TMP11]], [[TMP14]]
+; CHECK-NEXT: [[TMP15:%.*]] = or i1 [[_MSPROP_ICMP]], [[_MSPROP_ICMP1]]
+; CHECK-NEXT: br i1 [[TMP15]], label %[[BB2:.*]], label %[[BB3:.*]], !prof [[PROF1:![0-9]+]]
; CHECK: [[BB2]]:
; CHECK-NEXT: call void @__msan_warning_noreturn() #[[ATTR3:[0-9]+]]
; CHECK-NEXT: unreachable
>From d0eb8877108abda7855494f13cff0275eb6b1a84 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 12 Feb 2026 09:54:23 +0000
Subject: [PATCH 2/4] Only consider the first 99 cases per switch, to avoid
pathological behavior in jump-threading.
Add TODOs on possible improvements.
---
.../Instrumentation/MemorySanitizer.cpp | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 3e7eafd756e89..1f94bcab1b075 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2484,10 +2484,22 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *Val = SI.getCondition();
Value *ShadowVal = getShadow(Val);
+ // TODO: add fast path - if the condition is fully initialized, we know
+ // there is no UUM, without needing to consider the case values below.
+
+ // Some code (e.g., AMDGPUGenMCCodeEmitter.inc) has tens of thousands of
+ // cases. This results in an extremely long chained expression for MSan's
+ // switch instrumentation, which can cause the JumpThreadingPass to have a
+ // stack overflow or excessive runtime. We limit the number of cases
+ // considered, with the tradeoff of niche false negatives.
+ // TODO: figure out a better solution.
+ int casesToConsider = 99;
Value *ShadowCases = nullptr;
for (auto Case : SI.cases()) {
Value *Comparator = Case.getCaseValue();
+ // TODO: some simplification is possible when comparing multiple cases
+ // simultaneously.
Value *ComparisonShadow = propagateEqualityComparison(
IRB, Val, Comparator, ShadowVal, getShadow(Comparator));
@@ -2495,6 +2507,10 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
ShadowCases = IRB.CreateOr(ShadowCases, ComparisonShadow);
else
ShadowCases = ComparisonShadow;
+
+ casesToConsider--;
+ if (casesToConsider <= 0)
+ break;
}
if (ShadowCases)
>From c75a16ce1e496833352666eb40eabf59dfd7b1f6 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 12 Feb 2026 11:09:32 +0000
Subject: [PATCH 3/4] Reorder check
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 1f94bcab1b075..d699c2e4048d2 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2497,6 +2497,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *ShadowCases = nullptr;
for (auto Case : SI.cases()) {
+ if (casesToConsider <= 0)
+ break;
+
Value *Comparator = Case.getCaseValue();
// TODO: some simplification is possible when comparing multiple cases
// simultaneously.
@@ -2509,8 +2512,6 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
ShadowCases = ComparisonShadow;
casesToConsider--;
- if (casesToConsider <= 0)
- break;
}
if (ShadowCases)
>From 215cf263ac1ab9b060cbad91b5011d66baf8bde3 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Fri, 13 Feb 2026 08:47:37 +0000
Subject: [PATCH 4/4] Add -msan-switch-precision flag
---
.../Transforms/Instrumentation/MemorySanitizer.cpp | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index d699c2e4048d2..2c0232a266d04 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -299,6 +299,15 @@ static cl::opt<bool>
cl::desc("exact handling of relational integer ICmp"),
cl::Hidden, cl::init(true));
+static cl::opt<int> ClSwitchPrecision(
+ "msan-switch-precision",
+ cl::desc("Controls the number of cases considered by MSan for LLVM switch "
+ "instructions. 0 means no UUMs detected. Higher values lead to "
+ "fewer false negatives but may impact compiler and/or "
+ "application performance. N.B. LLVM switch instructions do not "
+ "correspond exactly to C++ switch statements."),
+ cl::Hidden, cl::init(99));
+
static cl::opt<bool> ClHandleLifetimeIntrinsics(
"msan-handle-lifetime-intrinsics",
cl::desc(
@@ -2493,7 +2502,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// stack overflow or excessive runtime. We limit the number of cases
// considered, with the tradeoff of niche false negatives.
// TODO: figure out a better solution.
- int casesToConsider = 99;
+ int casesToConsider = ClSwitchPrecision;
Value *ShadowCases = nullptr;
for (auto Case : SI.cases()) {
More information about the llvm-commits
mailing list