From bb34b1de491de2ba2699391d386d65e96a2e2a78 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 26 Jun 2025 23:12:42 +0000 Subject: [PATCH 1/6] [msan] Improve instrumentation of disjoint OR The disjoint OR (https://github.com/llvm/llvm-project/pull/72583) of two '1's is poison, hence the corresponding shadow memory needs to be uninitialized (rather than initialized, as per the existing instrumentation which ignores disjointedness). Updates the test from https://github.com/llvm/llvm-project/pull/145982 --- .../Instrumentation/MemorySanitizer.cpp | 25 ++++++++++++++----- .../Instrumentation/MemorySanitizer/or.ll | 4 ++- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 1a76898bd61c6..f06600a55ef27 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -2497,11 +2497,16 @@ struct MemorySanitizerVisitor : public InstVisitor { void visitOr(BinaryOperator &I) { IRBuilder<> IRB(&I); - // "Or" of 1 and a poisoned value results in unpoisoned value. - // 1|1 => 1; 0|1 => 1; p|1 => 1; - // 1|0 => 1; 0|0 => 0; p|0 => p; - // 1|p => 1; 0|p => p; p|p => p; - // S = (S1 & S2) | (~V1 & S2) | (S1 & ~V2) + // "Or" of 1 and a poisoned value results in unpoisoned value: + // 1|1 => 1; 0|1 => 1; p|1 => 1; + // 1|0 => 1; 0|0 => 0; p|0 => p; + // 1|p => 1; 0|p => p; p|p => p; + // + // S = (S1 & S2) | (~V1 & S2) | (S1 & ~V2) + // + // Addendum if the "Or" is "disjoint": + // 1|1 => p; + // S = S | (V1 & V2) Value *S1 = getShadow(&I, 0); Value *S2 = getShadow(&I, 1); Value *V1 = IRB.CreateNot(I.getOperand(0)); @@ -2513,7 +2518,15 @@ struct MemorySanitizerVisitor : public InstVisitor { Value *S1S2 = IRB.CreateAnd(S1, S2); Value *V1S2 = IRB.CreateAnd(V1, S2); Value *S1V2 = IRB.CreateAnd(S1, V2); - setShadow(&I, IRB.CreateOr({S1S2, V1S2, S1V2})); + + Value *S = IRB.CreateOr({S1S2, V1S2, S1V2}); + auto* MaybeDisjoint = cast(&I); + if (MaybeDisjoint->isDisjoint()) { + Value *V1V2 = IRB.CreateAnd(V1, V2); + S = IRB.CreateOr({S, V1V2}); + } + + setShadow(&I, S); setOriginForNaryOp(I); } diff --git a/llvm/test/Instrumentation/MemorySanitizer/or.ll b/llvm/test/Instrumentation/MemorySanitizer/or.ll index 6570b6d9d91ca..598de90135320 100644 --- a/llvm/test/Instrumentation/MemorySanitizer/or.ll +++ b/llvm/test/Instrumentation/MemorySanitizer/or.ll @@ -41,8 +41,10 @@ define i8 @test_disjoint_or(i8 %a, i8 %b) sanitize_memory { ; CHECK-NEXT: [[TMP7:%.*]] = and i8 [[TMP1]], [[TMP4]] ; CHECK-NEXT: [[TMP8:%.*]] = or i8 [[TMP5]], [[TMP6]] ; CHECK-NEXT: [[TMP11:%.*]] = or i8 [[TMP8]], [[TMP7]] +; CHECK-NEXT: [[TMP10:%.*]] = and i8 [[TMP3]], [[TMP4]] +; CHECK-NEXT: [[TMP12:%.*]] = or i8 [[TMP11]], [[TMP10]] ; CHECK-NEXT: [[C:%.*]] = or disjoint i8 [[A]], [[B]] -; CHECK-NEXT: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: store i8 [[TMP12]], ptr @__msan_retval_tls, align 8 ; CHECK-NEXT: ret i8 [[C]] ; %c = or disjoint i8 %a, %b From 6585af41f85a1beafceb6fe1f3978134fe2627ae Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 26 Jun 2025 23:22:24 +0000 Subject: [PATCH 2/6] clang-format --- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index f06600a55ef27..63b2cd95fe368 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -2520,7 +2520,7 @@ struct MemorySanitizerVisitor : public InstVisitor { Value *S1V2 = IRB.CreateAnd(S1, V2); Value *S = IRB.CreateOr({S1S2, V1S2, S1V2}); - auto* MaybeDisjoint = cast(&I); + auto *MaybeDisjoint = cast(&I); if (MaybeDisjoint->isDisjoint()) { Value *V1V2 = IRB.CreateAnd(V1, V2); S = IRB.CreateOr({S, V1V2}); From 3f8fc04f47b9b9af380fe993f218b06afb288606 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 26 Jun 2025 23:23:39 +0000 Subject: [PATCH 3/6] Update test comment --- llvm/test/Instrumentation/MemorySanitizer/or.ll | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/test/Instrumentation/MemorySanitizer/or.ll b/llvm/test/Instrumentation/MemorySanitizer/or.ll index 598de90135320..0457723f42469 100644 --- a/llvm/test/Instrumentation/MemorySanitizer/or.ll +++ b/llvm/test/Instrumentation/MemorySanitizer/or.ll @@ -1,8 +1,7 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 ; RUN: opt < %s -S -passes=msan 2>&1 | FileCheck %s ; -; Test bitwise OR instructions, especially the "disjoint OR", which is -; currently handled incorrectly by MSan (as if it was a regular OR). +; Test bitwise OR instructions, including "disjoint OR". 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" From d5094b42edfd4bea4842fb935b28e1d47cce817a Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 26 Jun 2025 23:55:51 +0000 Subject: [PATCH 4/6] Control behavior using -msan-precise-disjoint-or and default to legacy behavior --- .../Instrumentation/MemorySanitizer.cpp | 16 ++++++++++++---- llvm/test/Instrumentation/MemorySanitizer/or.ll | 15 ++++++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 63b2cd95fe368..600cb572dca69 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -282,6 +282,12 @@ static cl::opt ClPoisonUndefVectors( "unaffected by this flag (see -msan-poison-undef)."), cl::Hidden, cl::init(false)); +static cl::opt ClPreciseDisjointOr( + "msan-precise-disjoint-or", + cl::desc("Precisely poison disjoint OR. If false (legacy behavior), " + "disjointedness is ignored (i.e., 1|1 is initialized)."), + cl::Hidden, cl::init(false)); + static cl::opt ClHandleICmp("msan-handle-icmp", cl::desc("propagate shadow through ICmpEQ and ICmpNE"), @@ -2520,10 +2526,12 @@ struct MemorySanitizerVisitor : public InstVisitor { Value *S1V2 = IRB.CreateAnd(S1, V2); Value *S = IRB.CreateOr({S1S2, V1S2, S1V2}); - auto *MaybeDisjoint = cast(&I); - if (MaybeDisjoint->isDisjoint()) { - Value *V1V2 = IRB.CreateAnd(V1, V2); - S = IRB.CreateOr({S, V1V2}); + if (ClPreciseDisjointOr) { + auto *MaybeDisjoint = cast(&I); + if (MaybeDisjoint->isDisjoint()) { + Value *V1V2 = IRB.CreateAnd(V1, V2); + S = IRB.CreateOr({S, V1V2}); + } } setShadow(&I, S); diff --git a/llvm/test/Instrumentation/MemorySanitizer/or.ll b/llvm/test/Instrumentation/MemorySanitizer/or.ll index 0457723f42469..1e3481d54b01e 100644 --- a/llvm/test/Instrumentation/MemorySanitizer/or.ll +++ b/llvm/test/Instrumentation/MemorySanitizer/or.ll @@ -1,5 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 -; RUN: opt < %s -S -passes=msan 2>&1 | FileCheck %s +; RUN: opt < %s -S -passes=msan -msan-precise-disjoint-or=false 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-IMPRECISE +; RUN: opt < %s -S -passes=msan -msan-precise-disjoint-or=true 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PRECISE ; ; Test bitwise OR instructions, including "disjoint OR". @@ -40,10 +41,14 @@ define i8 @test_disjoint_or(i8 %a, i8 %b) sanitize_memory { ; CHECK-NEXT: [[TMP7:%.*]] = and i8 [[TMP1]], [[TMP4]] ; CHECK-NEXT: [[TMP8:%.*]] = or i8 [[TMP5]], [[TMP6]] ; CHECK-NEXT: [[TMP11:%.*]] = or i8 [[TMP8]], [[TMP7]] -; CHECK-NEXT: [[TMP10:%.*]] = and i8 [[TMP3]], [[TMP4]] -; CHECK-NEXT: [[TMP12:%.*]] = or i8 [[TMP11]], [[TMP10]] -; CHECK-NEXT: [[C:%.*]] = or disjoint i8 [[A]], [[B]] -; CHECK-NEXT: store i8 [[TMP12]], ptr @__msan_retval_tls, align 8 +; +; CHECK-IMPRECISE: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8 +; +; CHECK-PRECISE: [[TMP10:%.*]] = and i8 [[TMP3]], [[TMP4]] +; CHECK-PRECISE-NEXT: [[TMP12:%.*]] = or i8 [[TMP11]], [[TMP10]] +; CHECK-PRECISE-NEXT: [[C:%.*]] = or disjoint i8 [[A]], [[B]] +; CHECK-PRECISE-NEXT: store i8 [[TMP12]], ptr @__msan_retval_tls, align 8 +; ; CHECK-NEXT: ret i8 [[C]] ; %c = or disjoint i8 %a, %b From cadc15caddaf26f5dd117526e3c37a2ac4096e30 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 26 Jun 2025 23:59:48 +0000 Subject: [PATCH 5/6] Undelete test line --- llvm/test/Instrumentation/MemorySanitizer/or.ll | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/test/Instrumentation/MemorySanitizer/or.ll b/llvm/test/Instrumentation/MemorySanitizer/or.ll index 1e3481d54b01e..2d51de13a8ebb 100644 --- a/llvm/test/Instrumentation/MemorySanitizer/or.ll +++ b/llvm/test/Instrumentation/MemorySanitizer/or.ll @@ -42,9 +42,10 @@ define i8 @test_disjoint_or(i8 %a, i8 %b) sanitize_memory { ; CHECK-NEXT: [[TMP8:%.*]] = or i8 [[TMP5]], [[TMP6]] ; CHECK-NEXT: [[TMP11:%.*]] = or i8 [[TMP8]], [[TMP7]] ; -; CHECK-IMPRECISE: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8 +; CHECK-IMPRECISE: [[C:%.*]] = or disjoint i8 [[A]], [[B]] +; CHECK-IMPRECISE-NEXT: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8 ; -; CHECK-PRECISE: [[TMP10:%.*]] = and i8 [[TMP3]], [[TMP4]] +; CHECK-PRECISE: [[TMP10:%.*]] = and i8 [[TMP3]], [[TMP4]] ; CHECK-PRECISE-NEXT: [[TMP12:%.*]] = or i8 [[TMP11]], [[TMP10]] ; CHECK-PRECISE-NEXT: [[C:%.*]] = or disjoint i8 [[A]], [[B]] ; CHECK-PRECISE-NEXT: store i8 [[TMP12]], ptr @__msan_retval_tls, align 8 From 7b9eff65f51ee305b1d4bf2e6ed6fd99aece2b2f Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Fri, 27 Jun 2025 04:20:53 +0000 Subject: [PATCH 6/6] Refactor per Fldisjointorian --- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 600cb572dca69..ca655b3597671 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -2526,12 +2526,9 @@ struct MemorySanitizerVisitor : public InstVisitor { Value *S1V2 = IRB.CreateAnd(S1, V2); Value *S = IRB.CreateOr({S1S2, V1S2, S1V2}); - if (ClPreciseDisjointOr) { - auto *MaybeDisjoint = cast(&I); - if (MaybeDisjoint->isDisjoint()) { - Value *V1V2 = IRB.CreateAnd(V1, V2); - S = IRB.CreateOr({S, V1V2}); - } + if (ClPreciseDisjointOr && cast(&I)->isDisjoint()) { + Value *V1V2 = IRB.CreateAnd(V1, V2); + S = IRB.CreateOr({S, V1V2}); } setShadow(&I, S);