[llvm] [msan] Fix "Add optional flag to improve instrumentation of disjoint OR (#145990)" (PR #146799)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 2 17:00:30 PDT 2025
https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/146799
>From 7a7aeb0567e5a49fd536dc2317bc4e7ecb4afd66 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 2 Jul 2025 22:37:51 +0000
Subject: [PATCH 1/3] [msan] Fix "Add optional flag to improve instrumentation
of disjoint OR (#145990)"
The "V1" and "V2" values were already NOT'ed, hence the calculation of
disjoint OR was incorrect. This patch fixes the issue by using the
instruction operands directly.
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 7 ++++++-
llvm/test/Instrumentation/MemorySanitizer/or.ll | 5 +++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 6511c197fcb5a..4a0eee177667e 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2512,6 +2512,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// S = S | (V1 & V2)
Value *S1 = getShadow(&I, 0);
Value *S2 = getShadow(&I, 1);
+ // Gotcha: V1 and V2 are NOT'ed here
Value *V1 = IRB.CreateNot(I.getOperand(0));
Value *V2 = IRB.CreateNot(I.getOperand(1));
if (V1->getType() != S1->getType()) {
@@ -2524,7 +2525,11 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *S = IRB.CreateOr({S1S2, V1S2, S1V2});
if (ClPreciseDisjointOr && cast<PossiblyDisjointInst>(&I)->isDisjoint()) {
- Value *V1V2 = IRB.CreateAnd(V1, V2);
+ // "V1" and "V2" were NOT'ed above, but we still want to reuse them
+ // because they were IntCast'ed to the same type as the shadows.
+ //
+ // (V1 & V2) == ~(~V1 | ~V2) (de Morgan)
+ Value *V1V2 = IRB.CreateNot(IRB.CreateOr(V1, V2));
S = IRB.CreateOr({S, V1V2});
}
diff --git a/llvm/test/Instrumentation/MemorySanitizer/or.ll b/llvm/test/Instrumentation/MemorySanitizer/or.ll
index 2d51de13a8ebb..71beb38b466e4 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/or.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/or.ll
@@ -45,8 +45,9 @@ define i8 @test_disjoint_or(i8 %a, i8 %b) sanitize_memory {
; 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-NEXT: [[TMP12:%.*]] = or i8 [[TMP11]], [[TMP10]]
+; CHECK-PRECISE: [[TMP10:%.*]] = or i8 [[TMP3]], [[TMP4]]
+; CHECK-PRECISE-NEXT: [[TMP11:%.*]] = xor i8 [[TMP10]], -1
+; CHECK-PRECISE-NEXT: [[TMP12:%.*]] = or i8 [[TMP9]], [[TMP11]]
; CHECK-PRECISE-NEXT: [[C:%.*]] = or disjoint i8 [[A]], [[B]]
; CHECK-PRECISE-NEXT: store i8 [[TMP12]], ptr @__msan_retval_tls, align 8
;
>From 86a1df3af098588dc450878280b4049d4254323d Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 2 Jul 2025 23:41:19 +0000
Subject: [PATCH 2/3] Recast operands as ints instead, per Florian2
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 9 ++++-----
llvm/test/Instrumentation/MemorySanitizer/or.ll | 7 +++----
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 4a0eee177667e..1783d45ac1d63 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2525,11 +2525,10 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *S = IRB.CreateOr({S1S2, V1S2, S1V2});
if (ClPreciseDisjointOr && cast<PossiblyDisjointInst>(&I)->isDisjoint()) {
- // "V1" and "V2" were NOT'ed above, but we still want to reuse them
- // because they were IntCast'ed to the same type as the shadows.
- //
- // (V1 & V2) == ~(~V1 | ~V2) (de Morgan)
- Value *V1V2 = IRB.CreateNot(IRB.CreateOr(V1, V2));
+ // "V1" and "V2" were NOT'ed above
+ V1 = IRB.CreateIntCast(I.getOperand(0), S1->getType(), false);
+ V2 = IRB.CreateIntCast(I.getOperand(1), S2->getType(), false);
+ Value *V1V2 = IRB.CreateAnd(V1, V2);
S = IRB.CreateOr({S, V1V2});
}
diff --git a/llvm/test/Instrumentation/MemorySanitizer/or.ll b/llvm/test/Instrumentation/MemorySanitizer/or.ll
index 71beb38b466e4..e3ca38d26b69f 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/or.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/or.ll
@@ -45,11 +45,10 @@ define i8 @test_disjoint_or(i8 %a, i8 %b) sanitize_memory {
; CHECK-IMPRECISE: [[C:%.*]] = or disjoint i8 [[A]], [[B]]
; CHECK-IMPRECISE-NEXT: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8
;
-; CHECK-PRECISE: [[TMP10:%.*]] = or i8 [[TMP3]], [[TMP4]]
-; CHECK-PRECISE-NEXT: [[TMP11:%.*]] = xor i8 [[TMP10]], -1
-; CHECK-PRECISE-NEXT: [[TMP12:%.*]] = or i8 [[TMP9]], [[TMP11]]
+; CHECK-PRECISE: [[TMP10:%.*]] = and i8 [[A]], [[B]]
+; CHECK-PRECISE-NEXT: [[TMP11:%.*]] = or i8 [[TMP9]], [[TMP10]]
; CHECK-PRECISE-NEXT: [[C:%.*]] = or disjoint i8 [[A]], [[B]]
-; CHECK-PRECISE-NEXT: store i8 [[TMP12]], ptr @__msan_retval_tls, align 8
+; CHECK-PRECISE-NEXT: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8
;
; CHECK-NEXT: ret i8 [[C]]
;
>From 7eb398ef914b2715a2f323cf7eb246539473dff2 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 2 Jul 2025 23:58:52 +0000
Subject: [PATCH 3/3] Refactor to NotV1 / NotV2
---
.../Instrumentation/MemorySanitizer.cpp | 21 ++++++++++---------
.../Instrumentation/MemorySanitizer/or.ll | 4 ++--
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 1783d45ac1d63..82bafa39a805d 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2512,24 +2512,25 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// S = S | (V1 & V2)
Value *S1 = getShadow(&I, 0);
Value *S2 = getShadow(&I, 1);
- // Gotcha: V1 and V2 are NOT'ed here
- Value *V1 = IRB.CreateNot(I.getOperand(0));
- Value *V2 = IRB.CreateNot(I.getOperand(1));
+ Value *V1 = I.getOperand(0);
+ Value *V2 = I.getOperand(1);
if (V1->getType() != S1->getType()) {
V1 = IRB.CreateIntCast(V1, S1->getType(), false);
V2 = IRB.CreateIntCast(V2, S2->getType(), false);
}
+
+ Value *NotV1 = IRB.CreateNot(V1);
+ Value *NotV2 = IRB.CreateNot(V2);
+
Value *S1S2 = IRB.CreateAnd(S1, S2);
- Value *V1S2 = IRB.CreateAnd(V1, S2);
- Value *S1V2 = IRB.CreateAnd(S1, V2);
+ Value *S2NotV1 = IRB.CreateAnd(NotV1, S2);
+ Value *S1NotV2 = IRB.CreateAnd(S1, NotV2);
+
+ Value *S = IRB.CreateOr({S1S2, S2NotV1, S1NotV2});
- Value *S = IRB.CreateOr({S1S2, V1S2, S1V2});
if (ClPreciseDisjointOr && cast<PossiblyDisjointInst>(&I)->isDisjoint()) {
- // "V1" and "V2" were NOT'ed above
- V1 = IRB.CreateIntCast(I.getOperand(0), S1->getType(), false);
- V2 = IRB.CreateIntCast(I.getOperand(1), S2->getType(), false);
Value *V1V2 = IRB.CreateAnd(V1, V2);
- S = IRB.CreateOr({S, V1V2});
+ S = IRB.CreateOr(S, V1V2, "_ms_disjoint");
}
setShadow(&I, S);
diff --git a/llvm/test/Instrumentation/MemorySanitizer/or.ll b/llvm/test/Instrumentation/MemorySanitizer/or.ll
index e3ca38d26b69f..27a1800aa495b 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/or.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/or.ll
@@ -46,9 +46,9 @@ define i8 @test_disjoint_or(i8 %a, i8 %b) sanitize_memory {
; CHECK-IMPRECISE-NEXT: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8
;
; CHECK-PRECISE: [[TMP10:%.*]] = and i8 [[A]], [[B]]
-; CHECK-PRECISE-NEXT: [[TMP11:%.*]] = or i8 [[TMP9]], [[TMP10]]
+; CHECK-PRECISE-NEXT: [[TMP12:%.*]] = or i8 [[TMP11]], [[TMP10]]
; CHECK-PRECISE-NEXT: [[C:%.*]] = or disjoint i8 [[A]], [[B]]
-; CHECK-PRECISE-NEXT: store i8 [[TMP11]], ptr @__msan_retval_tls, align 8
+; CHECK-PRECISE-NEXT: store i8 [[TMP12]], ptr @__msan_retval_tls, align 8
;
; CHECK-NEXT: ret i8 [[C]]
;
More information about the llvm-commits
mailing list