Skip to content

Commit 5a194c1

Browse files
thurstondthurstond
authored
[msan] Sharpen instrumentation of Intrinsic::{ctlz,cttz} (#145609)
The current instrumentation of Intrinsic::{ctlz,cttz} has false positives. For example, consider `ctlz(0001 11??)` whereby `0` and `1` denotes initialized bits (with concrete values of 0 and 1 respectively) and `?` denotes an uninitialized bit. The result (of 3) is well-defined and the shadow ought to be fully initialized, but the current instrumentation marks it as fully uninitialized. This patch improves the fidelity of the instrumentation by comparing the number of leading (for ctlz; trailing for cttz) zeros in the concrete value and the shadow. This patch also renames the function from 'handleCountZeroes' to 'handleLeadingTrailingCountZeros', to clarify that the intrinsics handled do not count all the zeros (unlike `llvm.ctpop`, which counts all the 1s).
1 parent 9b307eb commit 5a194c1

File tree

2 files changed

+96
-35
lines changed

2 files changed

+96
-35
lines changed

‎llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3294,22 +3294,51 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
32943294
setOrigin(&I, getOrigin(Op));
32953295
}
32963296

3297-
void handleCountZeroes(IntrinsicInst &I) {
3297+
// Uninitialized bits are ok if they appear after the leading/trailing 0's
3298+
// and a 1. If the input is all zero, it is fully initialized iff
3299+
// !is_zero_poison.
3300+
//
3301+
// e.g., for ctlz, with little-endian, if 0/1 are initialized bits with
3302+
// concrete value 0/1, and ? is an uninitialized bit:
3303+
// - 0001 0??? is fully initialized
3304+
// - 000? ???? is fully uninitialized (*)
3305+
// - ???? ???? is fully uninitialized
3306+
// - 0000 0000 is fully uninitialized if is_zero_poison,
3307+
// fully initialized otherwise
3308+
//
3309+
// (*) TODO: arguably, since the number of zeros is in the range [3, 8], we
3310+
// only need to poison 4 bits.
3311+
//
3312+
// OutputShadow =
3313+
// ((ConcreteZerosCount >= ShadowZerosCount) && !AllZeroShadow)
3314+
// || (is_zero_poison && AllZeroSrc)
3315+
void handleCountLeadingTrailingZeros(IntrinsicInst &I) {
32983316
IRBuilder<> IRB(&I);
32993317
Value *Src = I.getArgOperand(0);
3318+
Value *SrcShadow = getShadow(Src);
33003319

3301-
// Set the Output shadow based on input Shadow
3302-
Value *BoolShadow = IRB.CreateIsNotNull(getShadow(Src), "_mscz_bs");
3320+
Value *False = IRB.getInt1(false);
3321+
Value *ConcreteZerosCount = IRB.CreateIntrinsic(
3322+
I.getType(), I.getIntrinsicID(), {Src, /*is_zero_poison=*/False});
3323+
Value *ShadowZerosCount = IRB.CreateIntrinsic(
3324+
I.getType(), I.getIntrinsicID(), {SrcShadow, /*is_zero_poison=*/False});
3325+
3326+
Value *CompareConcreteZeros = IRB.CreateICmpUGE(
3327+
ConcreteZerosCount, ShadowZerosCount, "_mscz_cmp_zeros");
3328+
3329+
Value *NotAllZeroShadow =
3330+
IRB.CreateIsNotNull(SrcShadow, "_mscz_shadow_not_null");
3331+
Value *OutputShadow =
3332+
IRB.CreateAnd(CompareConcreteZeros, NotAllZeroShadow, "_mscz_main");
33033333

33043334
// If zero poison is requested, mix in with the shadow
33053335
Constant *IsZeroPoison = cast<Constant>(I.getOperand(1));
33063336
if (!IsZeroPoison->isZeroValue()) {
33073337
Value *BoolZeroPoison = IRB.CreateIsNull(Src, "_mscz_bzp");
3308-
BoolShadow = IRB.CreateOr(BoolShadow, BoolZeroPoison, "_mscz_bs");
3338+
OutputShadow = IRB.CreateOr(OutputShadow, BoolZeroPoison, "_mscz_bs");
33093339
}
33103340

3311-
Value *OutputShadow =
3312-
IRB.CreateSExt(BoolShadow, getShadowTy(Src), "_mscz_os");
3341+
OutputShadow = IRB.CreateSExt(OutputShadow, getShadowTy(Src), "_mscz_os");
33133342

33143343
setShadow(&I, OutputShadow);
33153344
setOriginForNaryOp(I);
@@ -4726,7 +4755,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
47264755
break;
47274756
case Intrinsic::ctlz:
47284757
case Intrinsic::cttz:
4729-
handleCountZeroes(I);
4758+
handleCountLeadingTrailingZeros(I);
47304759
break;
47314760
case Intrinsic::masked_compressstore:
47324761
handleMaskedCompressStore(I);

‎llvm/test/Instrumentation/MemorySanitizer/count-zeroes.ll

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@ define i64 @test_ctlz_i64_zeropoison(i64 %v) #0 {
99
; CHECK-LABEL: @test_ctlz_i64_zeropoison(
1010
; CHECK-NEXT: [[TMP1:%.*]] = load i64, ptr @__msan_param_tls, align 8
1111
; CHECK-NEXT: call void @llvm.donothing()
12-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne i64 [[TMP1]], 0
13-
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq i64 [[V:%.*]], 0
14-
; CHECK-NEXT: [[_MSCZ_BS1:%.*]] = or i1 [[_MSCZ_BS]], [[_MSCZ_BZP]]
15-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_BS1]] to i64
12+
; CHECK-NEXT: [[TMP2:%.*]] = call i64 @llvm.ctlz.i64(i64 [[V:%.*]], i1 false)
13+
; CHECK-NEXT: [[TMP3:%.*]] = call i64 @llvm.ctlz.i64(i64 [[TMP1]], i1 false)
14+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge i64 [[TMP2]], [[TMP3]]
15+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne i64 [[TMP1]], 0
16+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and i1 [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
17+
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq i64 [[V]], 0
18+
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = or i1 [[_MSCZ_MAIN]], [[_MSCZ_BZP]]
19+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_BS]] to i64
1620
; CHECK-NEXT: [[RES:%.*]] = call i64 @llvm.ctlz.i64(i64 [[V]], i1 true)
1721
; CHECK-NEXT: store i64 [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
1822
; CHECK-NEXT: ret i64 [[RES]]
@@ -24,9 +28,13 @@ define i64 @test_ctlz_i64_nozeropoison(i64 %v) #0 {
2428
; CHECK-LABEL: @test_ctlz_i64_nozeropoison(
2529
; CHECK-NEXT: [[TMP1:%.*]] = load i64, ptr @__msan_param_tls, align 8
2630
; CHECK-NEXT: call void @llvm.donothing()
27-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne i64 [[TMP1]], 0
28-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_BS]] to i64
29-
; CHECK-NEXT: [[RES:%.*]] = call i64 @llvm.ctlz.i64(i64 [[V:%.*]], i1 false)
31+
; CHECK-NEXT: [[TMP2:%.*]] = call i64 @llvm.ctlz.i64(i64 [[V:%.*]], i1 false)
32+
; CHECK-NEXT: [[TMP3:%.*]] = call i64 @llvm.ctlz.i64(i64 [[TMP1]], i1 false)
33+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge i64 [[TMP2]], [[TMP3]]
34+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne i64 [[TMP1]], 0
35+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and i1 [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
36+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_MAIN]] to i64
37+
; CHECK-NEXT: [[RES:%.*]] = call i64 @llvm.ctlz.i64(i64 [[V]], i1 false)
3038
; CHECK-NEXT: store i64 [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
3139
; CHECK-NEXT: ret i64 [[RES]]
3240
;
@@ -39,10 +47,14 @@ define <2 x i64> @test_ctlz_v2i64_zeropoison(<2 x i64> %v) #0 {
3947
; CHECK-LABEL: @test_ctlz_v2i64_zeropoison(
4048
; CHECK-NEXT: [[TMP1:%.*]] = load <2 x i64>, ptr @__msan_param_tls, align 8
4149
; CHECK-NEXT: call void @llvm.donothing()
42-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
43-
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq <2 x i64> [[V:%.*]], zeroinitializer
44-
; CHECK-NEXT: [[_MSCZ_BS1:%.*]] = or <2 x i1> [[_MSCZ_BS]], [[_MSCZ_BZP]]
45-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_BS1]] to <2 x i64>
50+
; CHECK-NEXT: [[TMP2:%.*]] = call <2 x i64> @llvm.ctlz.v2i64(<2 x i64> [[V:%.*]], i1 false)
51+
; CHECK-NEXT: [[TMP3:%.*]] = call <2 x i64> @llvm.ctlz.v2i64(<2 x i64> [[TMP1]], i1 false)
52+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge <2 x i64> [[TMP2]], [[TMP3]]
53+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
54+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and <2 x i1> [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
55+
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq <2 x i64> [[V]], zeroinitializer
56+
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = or <2 x i1> [[_MSCZ_MAIN]], [[_MSCZ_BZP]]
57+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_BS]] to <2 x i64>
4658
; CHECK-NEXT: [[RES:%.*]] = call <2 x i64> @llvm.ctlz.v2i64(<2 x i64> [[V]], i1 true)
4759
; CHECK-NEXT: store <2 x i64> [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
4860
; CHECK-NEXT: ret <2 x i64> [[RES]]
@@ -54,9 +66,13 @@ define <2 x i64> @test_ctlz_v2i64_nozeropoison(<2 x i64> %v) #0 {
5466
; CHECK-LABEL: @test_ctlz_v2i64_nozeropoison(
5567
; CHECK-NEXT: [[TMP1:%.*]] = load <2 x i64>, ptr @__msan_param_tls, align 8
5668
; CHECK-NEXT: call void @llvm.donothing()
57-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
58-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_BS]] to <2 x i64>
59-
; CHECK-NEXT: [[RES:%.*]] = call <2 x i64> @llvm.ctlz.v2i64(<2 x i64> [[V:%.*]], i1 false)
69+
; CHECK-NEXT: [[TMP2:%.*]] = call <2 x i64> @llvm.ctlz.v2i64(<2 x i64> [[V:%.*]], i1 false)
70+
; CHECK-NEXT: [[TMP3:%.*]] = call <2 x i64> @llvm.ctlz.v2i64(<2 x i64> [[TMP1]], i1 false)
71+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge <2 x i64> [[TMP2]], [[TMP3]]
72+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
73+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and <2 x i1> [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
74+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_MAIN]] to <2 x i64>
75+
; CHECK-NEXT: [[RES:%.*]] = call <2 x i64> @llvm.ctlz.v2i64(<2 x i64> [[V]], i1 false)
6076
; CHECK-NEXT: store <2 x i64> [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
6177
; CHECK-NEXT: ret <2 x i64> [[RES]]
6278
;
@@ -69,10 +85,14 @@ define i64 @test_cttz_i64_zeropoison(i64 %v) #0 {
6985
; CHECK-LABEL: @test_cttz_i64_zeropoison(
7086
; CHECK-NEXT: [[TMP1:%.*]] = load i64, ptr @__msan_param_tls, align 8
7187
; CHECK-NEXT: call void @llvm.donothing()
72-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne i64 [[TMP1]], 0
73-
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq i64 [[V:%.*]], 0
74-
; CHECK-NEXT: [[_MSCZ_BS1:%.*]] = or i1 [[_MSCZ_BS]], [[_MSCZ_BZP]]
75-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_BS1]] to i64
88+
; CHECK-NEXT: [[TMP2:%.*]] = call i64 @llvm.cttz.i64(i64 [[V:%.*]], i1 false)
89+
; CHECK-NEXT: [[TMP3:%.*]] = call i64 @llvm.cttz.i64(i64 [[TMP1]], i1 false)
90+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge i64 [[TMP2]], [[TMP3]]
91+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne i64 [[TMP1]], 0
92+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and i1 [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
93+
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq i64 [[V]], 0
94+
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = or i1 [[_MSCZ_MAIN]], [[_MSCZ_BZP]]
95+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_BS]] to i64
7696
; CHECK-NEXT: [[RES:%.*]] = call i64 @llvm.cttz.i64(i64 [[V]], i1 true)
7797
; CHECK-NEXT: store i64 [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
7898
; CHECK-NEXT: ret i64 [[RES]]
@@ -84,9 +104,13 @@ define i64 @test_cttz_i64_nozeropoison(i64 %v) #0 {
84104
; CHECK-LABEL: @test_cttz_i64_nozeropoison(
85105
; CHECK-NEXT: [[TMP1:%.*]] = load i64, ptr @__msan_param_tls, align 8
86106
; CHECK-NEXT: call void @llvm.donothing()
87-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne i64 [[TMP1]], 0
88-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_BS]] to i64
89-
; CHECK-NEXT: [[RES:%.*]] = call i64 @llvm.cttz.i64(i64 [[V:%.*]], i1 false)
107+
; CHECK-NEXT: [[TMP2:%.*]] = call i64 @llvm.cttz.i64(i64 [[V:%.*]], i1 false)
108+
; CHECK-NEXT: [[TMP3:%.*]] = call i64 @llvm.cttz.i64(i64 [[TMP1]], i1 false)
109+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge i64 [[TMP2]], [[TMP3]]
110+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne i64 [[TMP1]], 0
111+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and i1 [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
112+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext i1 [[_MSCZ_MAIN]] to i64
113+
; CHECK-NEXT: [[RES:%.*]] = call i64 @llvm.cttz.i64(i64 [[V]], i1 false)
90114
; CHECK-NEXT: store i64 [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
91115
; CHECK-NEXT: ret i64 [[RES]]
92116
;
@@ -99,10 +123,14 @@ define <2 x i64> @test_cttz_v2i64_zeropoison(<2 x i64> %v) #0 {
99123
; CHECK-LABEL: @test_cttz_v2i64_zeropoison(
100124
; CHECK-NEXT: [[TMP1:%.*]] = load <2 x i64>, ptr @__msan_param_tls, align 8
101125
; CHECK-NEXT: call void @llvm.donothing()
102-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
103-
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq <2 x i64> [[V:%.*]], zeroinitializer
104-
; CHECK-NEXT: [[_MSCZ_BS1:%.*]] = or <2 x i1> [[_MSCZ_BS]], [[_MSCZ_BZP]]
105-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_BS1]] to <2 x i64>
126+
; CHECK-NEXT: [[TMP2:%.*]] = call <2 x i64> @llvm.cttz.v2i64(<2 x i64> [[V:%.*]], i1 false)
127+
; CHECK-NEXT: [[TMP3:%.*]] = call <2 x i64> @llvm.cttz.v2i64(<2 x i64> [[TMP1]], i1 false)
128+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge <2 x i64> [[TMP2]], [[TMP3]]
129+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
130+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and <2 x i1> [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
131+
; CHECK-NEXT: [[_MSCZ_BZP:%.*]] = icmp eq <2 x i64> [[V]], zeroinitializer
132+
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = or <2 x i1> [[_MSCZ_MAIN]], [[_MSCZ_BZP]]
133+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_BS]] to <2 x i64>
106134
; CHECK-NEXT: [[RES:%.*]] = call <2 x i64> @llvm.cttz.v2i64(<2 x i64> [[V]], i1 true)
107135
; CHECK-NEXT: store <2 x i64> [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
108136
; CHECK-NEXT: ret <2 x i64> [[RES]]
@@ -114,9 +142,13 @@ define <2 x i64> @test_cttz_v2i64_nozeropoison(<2 x i64> %v) #0 {
114142
; CHECK-LABEL: @test_cttz_v2i64_nozeropoison(
115143
; CHECK-NEXT: [[TMP1:%.*]] = load <2 x i64>, ptr @__msan_param_tls, align 8
116144
; CHECK-NEXT: call void @llvm.donothing()
117-
; CHECK-NEXT: [[_MSCZ_BS:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
118-
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_BS]] to <2 x i64>
119-
; CHECK-NEXT: [[RES:%.*]] = call <2 x i64> @llvm.cttz.v2i64(<2 x i64> [[V:%.*]], i1 false)
145+
; CHECK-NEXT: [[TMP2:%.*]] = call <2 x i64> @llvm.cttz.v2i64(<2 x i64> [[V:%.*]], i1 false)
146+
; CHECK-NEXT: [[TMP3:%.*]] = call <2 x i64> @llvm.cttz.v2i64(<2 x i64> [[TMP1]], i1 false)
147+
; CHECK-NEXT: [[_MSCZ_CMP_ZEROS:%.*]] = icmp uge <2 x i64> [[TMP2]], [[TMP3]]
148+
; CHECK-NEXT: [[_MSCZ_SHADOW_NOT_NULL:%.*]] = icmp ne <2 x i64> [[TMP1]], zeroinitializer
149+
; CHECK-NEXT: [[_MSCZ_MAIN:%.*]] = and <2 x i1> [[_MSCZ_CMP_ZEROS]], [[_MSCZ_SHADOW_NOT_NULL]]
150+
; CHECK-NEXT: [[_MSCZ_OS:%.*]] = sext <2 x i1> [[_MSCZ_MAIN]] to <2 x i64>
151+
; CHECK-NEXT: [[RES:%.*]] = call <2 x i64> @llvm.cttz.v2i64(<2 x i64> [[V]], i1 false)
120152
; CHECK-NEXT: store <2 x i64> [[_MSCZ_OS]], ptr @__msan_retval_tls, align 8
121153
; CHECK-NEXT: ret <2 x i64> [[RES]]
122154
;

0 commit comments

Comments
 (0)