[llvm] Reapply "[msan] Switch switch() from strict handling to (icmp eq)-style handling" (#180636) (PR #181112)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 12 02:04:44 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Thurston Dang (thurstond)
<details>
<summary>Changes</summary>
This reverts https://github.com/llvm/llvm-project/pull/180636 i.e., relands https://github.com/llvm/llvm-project/pull/179851.
It was reverted because when compiling large switch statements (e.g., AMDGPUGenMCCodeEmitter.inc has >30,000 cases), MSan's instrumentation created an extremely long chained expression for the shadow computation, which caused the JumpThreadingPass to have a stack overflow. This reland avoids the issue by limiting the number of cases considered, with the tradeoff of niche false negatives (only in the case where the condition is partly uninitialized and the first 99 cases still have a defined comparison, but a case > 100 does not have a fully-defined comparison).
This reland also adds some TODOs for possible improvements.
----
Original commit message:
Currently, the SwitchInst:
```
switch i32 %Val, label %else [ i32 0, label %A
i32 1, label %B
i32 2, label %C ]
```
is strictly handled i.e., MSan will check that %Val is fully initialized. This is appropriate nearly all the time.
However, sometimes the compiler may convert (icmp + br) into a switch statement. (icmp + br) has different semantics: 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 e.g., suppose:
```
%A = 00000000 00001010
%B = 00000000 00000110
%C = 00000000 00000011
%Val = 00000001 ???????? (where ? denotes an uninitialized bit)
```
Even though %Val has uninitialized bits, the initialized '1' bit immediately to the left, compared to the corresponding initialized '0' bit in %A/%B/%C suffices to prove that %Val does not match any of those cases. This is similar to a real-world case with std::optional (where the has_value bit may be initialized but the value is not).
This patch adds this relaxed icmp logic to the switch instrumentation as well, to make MSan's behavior equivalent under optimization.
Note that this edge case only applies if the switch input value definitively does not match *any* of the cases (matching any of the cases requires an exact, fully initialized match). If it is uncertain whether the switch input value could, depending on the uninitialized bits, match one of the cases or not, MSan will report use-of-uninitialized memory.
---
Full diff: https://github.com/llvm/llvm-project/pull/181112.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp (+57)
- (modified) llvm/test/Instrumentation/MemorySanitizer/switch-icmp.ll (+20-5)
``````````diff
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 5e766d8e0ff8c..1f94bcab1b075 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2460,6 +2460,63 @@ 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);
+ // 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));
+
+ if (ShadowCases)
+ ShadowCases = IRB.CreateOr(ShadowCases, ComparisonShadow);
+ else
+ ShadowCases = ComparisonShadow;
+
+ casesToConsider--;
+ if (casesToConsider <= 0)
+ break;
+ }
+
+ 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
``````````
</details>
https://github.com/llvm/llvm-project/pull/181112
More information about the llvm-commits
mailing list