[compiler-rt] [llvm] [msan][aarch64] Fix cleanup of unused part of overflow area (PR #72722)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 17 16:21:51 PST 2023
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/72722
>From 6d1d61d01f1ed01bc0e6ac2ad59e7c1c92c48e2e Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 17 Nov 2023 16:02:33 -0800
Subject: [PATCH 1/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
=?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
---
compiler-rt/test/msan/vararg_shadow.cpp | 4 +-
.../Instrumentation/MemorySanitizer.cpp | 54 ++++++++++---------
.../MemorySanitizer/AArch64/vararg_shadow.ll | 1 +
3 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/compiler-rt/test/msan/vararg_shadow.cpp b/compiler-rt/test/msan/vararg_shadow.cpp
index e491a4a53871de0..0fa000ea0bfd528 100644
--- a/compiler-rt/test/msan/vararg_shadow.cpp
+++ b/compiler-rt/test/msan/vararg_shadow.cpp
@@ -3,8 +3,8 @@
// Without -fno-sanitize-memory-param-retval we can't even pass poisoned values.
// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -fsanitize-memory-track-origins=0 -O3 %s -o %t
-// Nothing works yet.
-// XFAIL: target={{(aarch64|loongarch64|mips|powerpc64).*}}
+// FIXME: The rest is likely still broken.
+// XFAIL: target={{(loongarch64|mips|powerpc64).*}}
#include <sanitizer/msan_interface.h>
#include <stdarg.h>
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 06cccab322100f6..bcafbf3085a5e24 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -4696,6 +4696,18 @@ struct VarArgHelperBase : public VarArgHelper {
"_msarg_va_o");
}
+ void CleanUnusedTLS(IRBuilder<> &IRB, Value *ShadowBase, unsigned BaseOffset) {
+ // The tails of __msan_va_arg_tls is not large enough to fit full
+ // value shadow, but it will be copied to backup anyway. Make it
+ // clean.
+ if (BaseOffset >= kParamTLSSize)
+ return;
+ Value *TailSize =
+ ConstantInt::getSigned(IRB.getInt32Ty(), kParamTLSSize - BaseOffset);
+ IRB.CreateMemSet(ShadowBase, ConstantInt::getNullValue(IRB.getInt8Ty()),
+ TailSize, Align(8));
+ }
+
void unpoisonVAListTagForInst(IntrinsicInst &I) {
IRBuilder<> IRB(&I);
Value *VAListTag = I.getArgOperand(0);
@@ -4779,23 +4791,6 @@ struct VarArgAMD64Helper : public VarArgHelperBase {
unsigned OverflowOffset = AMD64FpEndOffset;
const DataLayout &DL = F.getParent()->getDataLayout();
- auto CleanUnusedTLS = [&](Value *ShadowBase, unsigned BaseOffset) {
- // Make sure we don't overflow __msan_va_arg_tls.
- if (OverflowOffset <= kParamTLSSize)
- return false; // Not needed, end is not reacheed.
-
- // The tails of __msan_va_arg_tls is not large enough to fit full
- // value shadow, but it will be copied to backup anyway. Make it
- // clean.
- if (BaseOffset < kParamTLSSize) {
- Value *TailSize = ConstantInt::getSigned(IRB.getInt32Ty(),
- kParamTLSSize - BaseOffset);
- IRB.CreateMemSet(ShadowBase, ConstantInt::getNullValue(IRB.getInt8Ty()),
- TailSize, Align(8));
- }
- return true; // Incomplete
- };
-
for (const auto &[ArgNo, A] : llvm::enumerate(CB.args())) {
bool IsFixed = ArgNo < CB.getFunctionType()->getNumParams();
bool IsByVal = CB.paramHasAttr(ArgNo, Attribute::ByVal);
@@ -4817,8 +4812,10 @@ struct VarArgAMD64Helper : public VarArgHelperBase {
OriginBase = getOriginPtrForVAArgument(IRB, OverflowOffset);
OverflowOffset += AlignedSize;
- if (CleanUnusedTLS(ShadowBase, BaseOffset))
+ if (OverflowOffset > kParamTLSSize) {
+ CleanUnusedTLS(IRB, ShadowBase, BaseOffset);
continue; // We have no space to copy shadow there.
+ }
Value *ShadowPtr, *OriginPtr;
std::tie(ShadowPtr, OriginPtr) =
@@ -4863,8 +4860,11 @@ struct VarArgAMD64Helper : public VarArgHelperBase {
OriginBase = getOriginPtrForVAArgument(IRB, OverflowOffset);
}
OverflowOffset += AlignedSize;
- if (CleanUnusedTLS(ShadowBase, BaseOffset))
- continue; // We have no space to copy shadow there.
+ if (OverflowOffset > kParamTLSSize) {
+ // We have no space to copy shadow there.
+ CleanUnusedTLS(IRB, ShadowBase, BaseOffset);
+ continue;
+ }
}
// Take fixed arguments into account for GpOffset and FpOffset,
// but don't actually store shadows for them.
@@ -5118,17 +5118,21 @@ struct VarArgAArch64Helper : public VarArgHelperBase {
if (IsFixed)
continue;
uint64_t ArgSize = DL.getTypeAllocSize(A->getType());
- Base = getShadowPtrForVAArgument(A->getType(), IRB, OverflowOffset,
- alignTo(ArgSize, 8));
- OverflowOffset += alignTo(ArgSize, 8);
+ uint64_t AlignedSize = alignTo(ArgSize, 8);
+ unsigned BaseOffset = OverflowOffset;
+ Base = getShadowPtrForVAArgument(A->getType(), IRB, BaseOffset);
+ OverflowOffset += AlignedSize;
+ if (OverflowOffset > kParamTLSSize) {
+ // We have no space to copy shadow there.
+ CleanUnusedTLS(IRB, Base, BaseOffset);
+ continue;
+ }
break;
}
// Count Gp/Vr fixed arguments to their respective offsets, but don't
// bother to actually store a shadow.
if (IsFixed)
continue;
- if (!Base)
- continue;
IRB.CreateAlignedStore(MSV.getShadow(A), Base, kShadowTLSAlignment);
}
Constant *OverflowSize =
diff --git a/llvm/test/Instrumentation/MemorySanitizer/AArch64/vararg_shadow.ll b/llvm/test/Instrumentation/MemorySanitizer/AArch64/vararg_shadow.ll
index 66f4d61f444f34e..ff9d4eea1596bc4 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/AArch64/vararg_shadow.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/AArch64/vararg_shadow.ll
@@ -1859,6 +1859,7 @@ define linkonce_odr dso_local void @_Z4test2I11LongDouble4EvT_([4 x fp128] align
; CHECK-NEXT: store [4 x i128] [[TMP35]], ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 576) to ptr), align 8
; CHECK-NEXT: store [4 x i128] [[TMP35]], ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 640) to ptr), align 8
; CHECK-NEXT: store [4 x i128] [[TMP35]], ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 704) to ptr), align 8
+; CHECK-NEXT: call void @llvm.memset.p0.i32(ptr align 8 inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 768) to ptr), i8 0, i32 32, i1 false)
; CHECK-NEXT: store i64 1280, ptr @__msan_va_arg_overflow_size_tls, align 8
; CHECK-NEXT: call void ([4 x fp128], i32, ...) @_Z5test2I11LongDouble4EvT_iz([4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], i32 noundef 20, [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]], [4 x fp128] alignstack(16) [[DOTFCA_3_INSERT121]])
; CHECK-NEXT: ret void
>From e585e2f06030d2db36880dcf970ce456854bc74b Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 17 Nov 2023 16:21:34 -0800
Subject: [PATCH 2/2] runtime test is not ready yet
Created using spr 1.3.4
---
compiler-rt/test/msan/vararg_shadow.cpp | 4 ++--
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 7 ++++---
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/compiler-rt/test/msan/vararg_shadow.cpp b/compiler-rt/test/msan/vararg_shadow.cpp
index 0fa000ea0bfd528..e491a4a53871de0 100644
--- a/compiler-rt/test/msan/vararg_shadow.cpp
+++ b/compiler-rt/test/msan/vararg_shadow.cpp
@@ -3,8 +3,8 @@
// Without -fno-sanitize-memory-param-retval we can't even pass poisoned values.
// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -fsanitize-memory-track-origins=0 -O3 %s -o %t
-// FIXME: The rest is likely still broken.
-// XFAIL: target={{(loongarch64|mips|powerpc64).*}}
+// Nothing works yet.
+// XFAIL: target={{(aarch64|loongarch64|mips|powerpc64).*}}
#include <sanitizer/msan_interface.h>
#include <stdarg.h>
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index bcafbf3085a5e24..170e267356d58ec 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -4696,7 +4696,8 @@ struct VarArgHelperBase : public VarArgHelper {
"_msarg_va_o");
}
- void CleanUnusedTLS(IRBuilder<> &IRB, Value *ShadowBase, unsigned BaseOffset) {
+ void CleanUnusedTLS(IRBuilder<> &IRB, Value *ShadowBase,
+ unsigned BaseOffset) {
// The tails of __msan_va_arg_tls is not large enough to fit full
// value shadow, but it will be copied to backup anyway. Make it
// clean.
@@ -4705,7 +4706,7 @@ struct VarArgHelperBase : public VarArgHelper {
Value *TailSize =
ConstantInt::getSigned(IRB.getInt32Ty(), kParamTLSSize - BaseOffset);
IRB.CreateMemSet(ShadowBase, ConstantInt::getNullValue(IRB.getInt8Ty()),
- TailSize, Align(8));
+ TailSize, Align(8));
}
void unpoisonVAListTagForInst(IntrinsicInst &I) {
@@ -4863,7 +4864,7 @@ struct VarArgAMD64Helper : public VarArgHelperBase {
if (OverflowOffset > kParamTLSSize) {
// We have no space to copy shadow there.
CleanUnusedTLS(IRB, ShadowBase, BaseOffset);
- continue;
+ continue;
}
}
// Take fixed arguments into account for GpOffset and FpOffset,
More information about the llvm-commits
mailing list