[llvm-branch-commits] [llvm] [compiler-rt] [msan][x86] Fix shadow if vararg overflow beyond kParamTLSSize (PR #72707)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Nov 17 13:58:38 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-compiler-rt-sanitizer
@llvm/pr-subscribers-llvm-transforms
Author: Vitaly Buka (vitalybuka)
<details>
<summary>Changes</summary>
Caller puts argument shadow one by one into __msan_va_arg_tls, until it
reaches kParamTLSSize. After that it still increment OverflowOffset but
does not store the shadow.
Callee needs OverflowOffset to prepare a shadow for the entire overflow
area. It's done by creating "varargs shadow copy" for complete list of
args, copying available shadow from __msan_va_arg_tls, and clearing the
rest.
However callee does not know if the tail of __msan_va_arg_tls was not
able to fit an argument, and callee will copy tail shadow into "varargs
shadow copy", and later used as a shadow for an omitted argument.
So that unused tail of the __msan_va_arg_tls must be cleared if left
unused.
This allows us to enable compiler-rt/test/msan/vararg_shadow.cpp for
x86.
---
Full diff: https://github.com/llvm/llvm-project/pull/72707.diff
3 Files Affected:
- (modified) compiler-rt/test/msan/vararg_shadow.cpp (+1-1)
- (modified) llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp (+49-19)
- (modified) llvm/test/Instrumentation/MemorySanitizer/X86/vararg_shadow.ll (+1)
``````````diff
diff --git a/compiler-rt/test/msan/vararg_shadow.cpp b/compiler-rt/test/msan/vararg_shadow.cpp
index 0c1e5e8d6369c3a..e491a4a53871de0 100644
--- a/compiler-rt/test/msan/vararg_shadow.cpp
+++ b/compiler-rt/test/msan/vararg_shadow.cpp
@@ -4,7 +4,7 @@
// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -fsanitize-memory-track-origins=0 -O3 %s -o %t
// Nothing works yet.
-// XFAIL: *
+// 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 766b6caffd18cea..fcc33832dc6d650 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -4669,16 +4669,22 @@ struct VarArgHelperBase : public VarArgHelper {
/// Compute the shadow address for a given va_arg.
Value *getShadowPtrForVAArgument(Type *Ty, IRBuilder<> &IRB,
- unsigned ArgOffset, unsigned ArgSize) {
- // Make sure we don't overflow __msan_va_arg_tls.
- if (ArgOffset + ArgSize > kParamTLSSize)
- return nullptr;
+ unsigned ArgOffset) {
Value *Base = IRB.CreatePointerCast(MS.VAArgTLS, MS.IntptrTy);
Base = IRB.CreateAdd(Base, ConstantInt::get(MS.IntptrTy, ArgOffset));
return IRB.CreateIntToPtr(Base, PointerType::get(MSV.getShadowTy(Ty), 0),
"_msarg_va_s");
}
+ /// Compute the shadow address for a given va_arg.
+ Value *getShadowPtrForVAArgument(Type *Ty, IRBuilder<> &IRB,
+ unsigned ArgOffset, unsigned ArgSize) {
+ // Make sure we don't overflow __msan_va_arg_tls.
+ if (ArgOffset + ArgSize > kParamTLSSize)
+ return nullptr;
+ return getShadowPtrForVAArgument(Ty, IRB, ArgOffset);
+ }
+
/// Compute the origin address for a given va_arg.
Value *getOriginPtrForVAArgument(IRBuilder<> &IRB, int ArgOffset) {
Value *Base = IRB.CreatePointerCast(MS.VAArgOriginTLS, MS.IntptrTy);
@@ -4772,6 +4778,24 @@ struct VarArgAMD64Helper : public VarArgHelperBase {
unsigned FpOffset = AMD64GpEndOffset;
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);
@@ -4784,19 +4808,22 @@ struct VarArgAMD64Helper : public VarArgHelperBase {
assert(A->getType()->isPointerTy());
Type *RealTy = CB.getParamByValType(ArgNo);
uint64_t ArgSize = DL.getTypeAllocSize(RealTy);
- Value *ShadowBase = getShadowPtrForVAArgument(
- RealTy, IRB, OverflowOffset, alignTo(ArgSize, 8));
+ uint64_t AlignedSize = alignTo(ArgSize, 8);
+ unsigned BaseOffset = OverflowOffset;
+ Value *ShadowBase =
+ getShadowPtrForVAArgument(RealTy, IRB, OverflowOffset);
Value *OriginBase = nullptr;
if (MS.TrackOrigins)
OriginBase = getOriginPtrForVAArgument(IRB, OverflowOffset);
- OverflowOffset += alignTo(ArgSize, 8);
- if (!ShadowBase)
- continue;
+ OverflowOffset += AlignedSize;
+
+ if (CleanUnusedTLS(ShadowBase, BaseOffset))
+ continue; // We have no space to copy shadow there.
+
Value *ShadowPtr, *OriginPtr;
std::tie(ShadowPtr, OriginPtr) =
MSV.getShadowOriginPtr(A, IRB, IRB.getInt8Ty(), kShadowTLSAlignment,
/*isStore*/ false);
-
IRB.CreateMemCpy(ShadowBase, kShadowTLSAlignment, ShadowPtr,
kShadowTLSAlignment, ArgSize);
if (MS.TrackOrigins)
@@ -4811,36 +4838,39 @@ struct VarArgAMD64Helper : public VarArgHelperBase {
Value *ShadowBase, *OriginBase = nullptr;
switch (AK) {
case AK_GeneralPurpose:
- ShadowBase =
- getShadowPtrForVAArgument(A->getType(), IRB, GpOffset, 8);
+ ShadowBase = getShadowPtrForVAArgument(A->getType(), IRB, GpOffset);
if (MS.TrackOrigins)
OriginBase = getOriginPtrForVAArgument(IRB, GpOffset);
GpOffset += 8;
+ assert(GpOffset <= kParamTLSSize);
break;
case AK_FloatingPoint:
- ShadowBase =
- getShadowPtrForVAArgument(A->getType(), IRB, FpOffset, 16);
+ ShadowBase = getShadowPtrForVAArgument(A->getType(), IRB, FpOffset);
if (MS.TrackOrigins)
OriginBase = getOriginPtrForVAArgument(IRB, FpOffset);
FpOffset += 16;
+ assert(FpOffset <= kParamTLSSize);
break;
case AK_Memory:
if (IsFixed)
continue;
uint64_t ArgSize = DL.getTypeAllocSize(A->getType());
+ uint64_t AlignedSize = alignTo(ArgSize, 8);
+ unsigned BaseOffset = OverflowOffset;
ShadowBase =
- getShadowPtrForVAArgument(A->getType(), IRB, OverflowOffset, 8);
- if (MS.TrackOrigins)
+ getShadowPtrForVAArgument(A->getType(), IRB, OverflowOffset);
+ if (MS.TrackOrigins) {
OriginBase = getOriginPtrForVAArgument(IRB, OverflowOffset);
- OverflowOffset += alignTo(ArgSize, 8);
+ }
+ OverflowOffset += AlignedSize;
+ if (CleanUnusedTLS(ShadowBase, BaseOffset))
+ continue; // We have no space to copy shadow there.
}
// Take fixed arguments into account for GpOffset and FpOffset,
// but don't actually store shadows for them.
// TODO(glider): don't call get*PtrForVAArgument() for them.
if (IsFixed)
continue;
- if (!ShadowBase)
- continue;
Value *Shadow = MSV.getShadow(A);
IRB.CreateAlignedStore(Shadow, ShadowBase, kShadowTLSAlignment);
if (MS.TrackOrigins) {
diff --git a/llvm/test/Instrumentation/MemorySanitizer/X86/vararg_shadow.ll b/llvm/test/Instrumentation/MemorySanitizer/X86/vararg_shadow.ll
index 6ac48c517fa6525..aff4d2c55ad6fcc 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/X86/vararg_shadow.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/X86/vararg_shadow.ll
@@ -1303,6 +1303,7 @@ define linkonce_odr dso_local void @_Z4test3I11LongDouble4EvT_(ptr noundef byval
; CHECK-NEXT: [[TMP64:%.*]] = xor i64 [[TMP63]], 87960930222080
; CHECK-NEXT: [[TMP65:%.*]] = inttoptr i64 [[TMP64]] to ptr
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 688) to ptr), ptr align 8 [[TMP65]], i64 64, i1 false)
+; CHECK-NEXT: call void @llvm.memset.p0.i32(ptr align 8 inttoptr (i64 add (i64 ptrtoint (ptr @__msan_va_arg_tls to i64), i64 752) to ptr), i8 0, i32 48, i1 false)
; CHECK-NEXT: store i64 1280, ptr @__msan_va_arg_overflow_size_tls, align 8
; CHECK-NEXT: call void (ptr, i32, ...) @_Z5test2I11LongDouble4EvT_iz(ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], i32 noundef 20, ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]], ptr noundef nonnull byval([[STRUCT_LONGDOUBLE4]]) align 16 [[ARG]])
; CHECK-NEXT: ret void
``````````
</details>
https://github.com/llvm/llvm-project/pull/72707
More information about the llvm-branch-commits
mailing list