From 7245c87ca801003839db877f4e7cec67ea42097f Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 12 Jun 2025 02:12:29 +0000 Subject: [PATCH 1/3] [NFCI][msan] Show shadow incorrectly computed for partially undefined vectors This happens because `getShadow(Value *V)` has a special case for fully undefined/poisoned values, but partially undefined values fall-through and are given a clean shadow. Showing that the same flaw happens for other composite types is left as an exercise for the reader. --- .../MemorySanitizer/partial-poison.ll | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll diff --git a/llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll b/llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll new file mode 100644 index 0000000000000..89ceda67a04d0 --- /dev/null +++ b/llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll @@ -0,0 +1,77 @@ +; 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 case to show that MSan incorrectly computes shadows for partially poisoned vectors + +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" + +define <2 x i64> @left_poison(ptr %add.ptr) sanitize_memory { +; CHECK-LABEL: define <2 x i64> @left_poison( +; CHECK-SAME: ptr [[ADD_PTR:%.*]]) #[[ATTR0:[0-9]+]] { +; CHECK-NEXT: call void @llvm.donothing() +; CHECK-NEXT: store <2 x i64> zeroinitializer, ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: ret <2 x i64> +; + ret <2 x i64> +} + +define <2 x i64> @right_poison(ptr %add.ptr) sanitize_memory { +; CHECK-LABEL: define <2 x i64> @right_poison( +; CHECK-SAME: ptr [[ADD_PTR:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: call void @llvm.donothing() +; CHECK-NEXT: store <2 x i64> zeroinitializer, ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: ret <2 x i64> +; + ret <2 x i64> +} + +define <2 x i64> @full_poison(ptr %add.ptr) sanitize_memory { +; CHECK-LABEL: define <2 x i64> @full_poison( +; CHECK-SAME: ptr [[ADD_PTR:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: call void @llvm.donothing() +; CHECK-NEXT: store <2 x i64> splat (i64 -1), ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: ret <2 x i64> poison +; + ret <2 x i64> +} + +define <2 x i64> @no_poison_or_undef(ptr %add.ptr) sanitize_memory { +; CHECK-LABEL: define <2 x i64> @no_poison_or_undef( +; CHECK-SAME: ptr [[ADD_PTR:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: call void @llvm.donothing() +; CHECK-NEXT: store <2 x i64> zeroinitializer, ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: ret <2 x i64> splat (i64 42) +; + ret <2 x i64> +} + +define <2 x i64> @left_undef(ptr %add.ptr) sanitize_memory { +; CHECK-LABEL: define <2 x i64> @left_undef( +; CHECK-SAME: ptr [[ADD_PTR:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: call void @llvm.donothing() +; CHECK-NEXT: store <2 x i64> zeroinitializer, ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: ret <2 x i64> +; + ret <2 x i64> +} + +define <2 x i64> @right_undef(ptr %add.ptr) sanitize_memory { +; CHECK-LABEL: define <2 x i64> @right_undef( +; CHECK-SAME: ptr [[ADD_PTR:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: call void @llvm.donothing() +; CHECK-NEXT: store <2 x i64> zeroinitializer, ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: ret <2 x i64> +; + ret <2 x i64> +} + +define <2 x i64> @full_undef(ptr %add.ptr) sanitize_memory { +; CHECK-LABEL: define <2 x i64> @full_undef( +; CHECK-SAME: ptr [[ADD_PTR:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: call void @llvm.donothing() +; CHECK-NEXT: store <2 x i64> splat (i64 -1), ptr @__msan_retval_tls, align 8 +; CHECK-NEXT: ret <2 x i64> undef +; + ret <2 x i64> +} From c9115c6240e8aaf79b145051717f2c5fe92c4dc8 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 12 Jun 2025 02:52:37 +0000 Subject: [PATCH 2/3] Add TODO in getShadow --- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index c2315d5de7041..4a618cf42c559 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -2085,6 +2085,10 @@ struct MemorySanitizerVisitor : public InstVisitor { assert(ShadowPtr && "Could not find shadow for an argument"); return ShadowPtr; } + + // TODO: partially undefined vectors are not correctly handled + // (see partial-poison.ll) + // For everything else the shadow is zero. return getCleanShadow(V); } From dae762cefea21b84bf5e1483df3d399bcbde4438 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 12 Jun 2025 02:54:56 +0000 Subject: [PATCH 3/3] Clarify that this leads to false negatives --- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 4 ++-- llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 4a618cf42c559..d3c6a7151ec37 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -2086,8 +2086,8 @@ struct MemorySanitizerVisitor : public InstVisitor { return ShadowPtr; } - // TODO: partially undefined vectors are not correctly handled - // (see partial-poison.ll) + // TODO: Partially undefined vectors are handled by the fall-through case + // below (see partial-poison.ll); this causes false negatives. // For everything else the shadow is zero. return getCleanShadow(V); diff --git a/llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll b/llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll index 89ceda67a04d0..5164441c17e10 100644 --- a/llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll +++ b/llvm/test/Instrumentation/MemorySanitizer/partial-poison.ll @@ -1,7 +1,8 @@ ; 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 case to show that MSan incorrectly computes shadows for partially poisoned vectors +; Test case to show that MSan computes shadows for partially poisoned vectors +; as fully initialized, resulting in false negatives. 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"