https://github.com/MaskRay created https://github.com/llvm/llvm-project/pull/67766
Similar to https://reviews.llvm.org/D9673, emit -fsanitize=alignment check for arguments of builtin memcpy and memmove functions to catch misaligned load like: ``` // Check a void unaligned_load(int *a, void *b) { memcpy(a, b, sizeof(*a)); } ``` For a reference parameter, we emit a -fsanitize=alignment check as well, which can be optimized out by InstCombinePass. We rely on the call site `TCK_ReferenceBinding` check instead. ``` // The check of a will be optimized out. void unaligned_load(int &a, void *b) { memcpy(&a, b, sizeof(a)); } ``` Technically builtin memset functions can be checked for -fsanitize=alignment as well, but it does not seem too useful. >From 17d767e4fb56ed02c7031e21ee9a04790c7bc801 Mon Sep 17 00:00:00 2001 From: Fangrui Song <i...@maskray.me> Date: Thu, 28 Sep 2023 15:22:38 -0700 Subject: [PATCH] -fsanitize=alignment: check memcpy/memmove arguments Similar to https://reviews.llvm.org/D9673, emit -fsanitize=alignment check for arguments of builtin memcpy and memmove functions to catch misaligned load like: ``` // Check a void unaligned_load(int *a, void *b) { memcpy(a, b, sizeof(*a)); } ``` For a reference parameter, we emit a -fsanitize=alignment check as well, which can be optimized out by InstCombinePass. We rely on the call site `TCK_ReferenceBinding` check instead. ``` // The check of a will be optimized out. void unaligned_load(int &a, void *b) { memcpy(&a, b, sizeof(a)); } ``` Technically builtin memset functions can be checked for -fsanitize=alignment as well, but it does not seem too useful. --- clang/include/clang/Basic/Sanitizers.h | 2 + clang/lib/CodeGen/CGBuiltin.cpp | 31 +++++++---- clang/test/CodeGen/catch-undef-behavior.c | 68 ++++++++++++++++++++++- 3 files changed, 87 insertions(+), 14 deletions(-) diff --git a/clang/include/clang/Basic/Sanitizers.h b/clang/include/clang/Basic/Sanitizers.h index 4659e45c7883419..16d241cbf809bbe 100644 --- a/clang/include/clang/Basic/Sanitizers.h +++ b/clang/include/clang/Basic/Sanitizers.h @@ -170,6 +170,8 @@ struct SanitizerSet { Mask = Value ? (Mask | K) : (Mask & ~K); } + void set(SanitizerMask K) { Mask = K; } + /// Disable the sanitizers specified in \p K. void clear(SanitizerMask K = SanitizerKind::All) { Mask &= ~K; } diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index b0fd38408806566..2102d0aaff9d620 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -2730,6 +2730,19 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, } } + auto EmitArgCheck = [&](TypeCheckKind Kind, Address A, const Expr *Arg, + unsigned ParmNum) { + Value *Val = A.getPointer(); + EmitNonNullArgCheck(RValue::get(Val), Arg->getType(), Arg->getExprLoc(), FD, + ParmNum); + + SanitizerSet SkippedChecks; + SkippedChecks.set(SanitizerKind::All); + SkippedChecks.clear(SanitizerKind::Alignment); + EmitTypeCheck(Kind, Arg->getExprLoc(), Val, Arg->getType(), + A.getAlignment(), SkippedChecks); + }; + switch (BuiltinIDIfNoAsmLabel) { default: break; case Builtin::BI__builtin___CFStringMakeConstantString: @@ -3720,10 +3733,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, Address Dest = EmitPointerWithAlignment(E->getArg(0)); Address Src = EmitPointerWithAlignment(E->getArg(1)); Value *SizeVal = EmitScalarExpr(E->getArg(2)); - EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(), - E->getArg(0)->getExprLoc(), FD, 0); - EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(), - E->getArg(1)->getExprLoc(), FD, 1); + EmitArgCheck(TCK_Store, Dest, E->getArg(0), 0); + EmitArgCheck(TCK_Load, Src, E->getArg(1), 1); Builder.CreateMemCpy(Dest, Src, SizeVal, false); if (BuiltinID == Builtin::BImempcpy || BuiltinID == Builtin::BI__builtin_mempcpy) @@ -3738,10 +3749,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, Address Src = EmitPointerWithAlignment(E->getArg(1)); uint64_t Size = E->getArg(2)->EvaluateKnownConstInt(getContext()).getZExtValue(); - EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(), - E->getArg(0)->getExprLoc(), FD, 0); - EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(), - E->getArg(1)->getExprLoc(), FD, 1); + EmitArgCheck(TCK_Store, Dest, E->getArg(0), 0); + EmitArgCheck(TCK_Load, Src, E->getArg(1), 1); Builder.CreateMemCpyInline(Dest, Src, Size); return RValue::get(nullptr); } @@ -3798,10 +3807,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, Address Dest = EmitPointerWithAlignment(E->getArg(0)); Address Src = EmitPointerWithAlignment(E->getArg(1)); Value *SizeVal = EmitScalarExpr(E->getArg(2)); - EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(), - E->getArg(0)->getExprLoc(), FD, 0); - EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(), - E->getArg(1)->getExprLoc(), FD, 1); + EmitArgCheck(TCK_Store, Dest, E->getArg(0), 0); + EmitArgCheck(TCK_Load, Src, E->getArg(1), 1); Builder.CreateMemMove(Dest, Src, SizeVal, false); return RValue::get(Dest.getPointer()); } diff --git a/clang/test/CodeGen/catch-undef-behavior.c b/clang/test/CodeGen/catch-undef-behavior.c index ca0df0f002f8912..3956a475f319ea9 100644 --- a/clang/test/CodeGen/catch-undef-behavior.c +++ b/clang/test/CodeGen/catch-undef-behavior.c @@ -354,21 +354,63 @@ void call_decl_nonnull(int *a) { decl_nonnull(a); } -extern void *memcpy (void *, const void *, unsigned) __attribute__((nonnull(1, 2))); +extern void *memcpy(void *, const void *, unsigned long) __attribute__((nonnull(1, 2))); // CHECK-COMMON-LABEL: @call_memcpy_nonnull void call_memcpy_nonnull(void *p, void *q, int sz) { // CHECK-COMMON: icmp ne ptr {{.*}}, null // CHECK-UBSAN: call void @__ubsan_handle_nonnull_arg // CHECK-TRAP: call void @llvm.ubsantrap(i8 16) + // CHECK-COMMON-NOT: call // CHECK-COMMON: icmp ne ptr {{.*}}, null // CHECK-UBSAN: call void @__ubsan_handle_nonnull_arg // CHECK-TRAP: call void @llvm.ubsantrap(i8 16) + // CHECK-COMMON-NOT: call + + // CHECK-COMMON: call void @llvm.memcpy.p0.p0.i64(ptr align 1 %0, ptr align 1 %1, i64 %conv, i1 false) memcpy(p, q, sz); } -extern void *memmove (void *, const void *, unsigned) __attribute__((nonnull(1, 2))); +// CHECK-COMMON-LABEL: define{{.*}} void @call_memcpy( +void call_memcpy(long *p, short *q, int sz) { + // CHECK-COMMON: icmp ne ptr {{.*}}, null + // CHECK-UBSAN: call void @__ubsan_handle_nonnull_arg( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 16) + // CHECK-COMMON: and i64 %[[#]], 7, !nosanitize + // CHECK-COMMON: icmp eq i64 %[[#]], 0, !nosanitize + // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 22) + + // CHECK-COMMON: icmp ne ptr {{.*}}, null + // CHECK-UBSAN: call void @__ubsan_handle_nonnull_arg( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 16) + // CHECK-COMMON: and i64 %[[#]], 1, !nosanitize + // CHECK-COMMON: icmp eq i64 %[[#]], 0, !nosanitize + // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 22) + + // CHECK-COMMON: call void @llvm.memcpy.p0.p0.i64(ptr align 8 %0, ptr align 2 %1, i64 %conv, i1 false) + memcpy(p, q, sz); +} + +// CHECK-COMMON-LABEL: define{{.*}} void @call_memcpy_inline( +void call_memcpy_inline(long *p, short *q) { + // CHECK-COMMON: and i64 %[[#]], 7, !nosanitize + // CHECK-COMMON: icmp eq i64 %[[#]], 0, !nosanitize + // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 22) + + // CHECK-COMMON: and i64 %[[#]], 1, !nosanitize + // CHECK-COMMON: icmp eq i64 %[[#]], 0, !nosanitize + // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 22) + + // CHECK-COMMON: call void @llvm.memcpy.inline.p0.p0.i64(ptr align 8 %0, ptr align 2 %1, i64 2, i1 false) + __builtin_memcpy_inline(p, q, 2); +} + +extern void *memmove(void *, const void *, unsigned long) __attribute__((nonnull(1, 2))); // CHECK-COMMON-LABEL: @call_memmove_nonnull void call_memmove_nonnull(void *p, void *q, int sz) { @@ -382,6 +424,28 @@ void call_memmove_nonnull(void *p, void *q, int sz) { memmove(p, q, sz); } +// CHECK-COMMON-LABEL: define{{.*}} void @call_memmove( +void call_memmove(long *p, short *q, int sz) { + // CHECK-COMMON: icmp ne ptr {{.*}}, null + // CHECK-UBSAN: call void @__ubsan_handle_nonnull_arg( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 16) + // CHECK-COMMON: and i64 %[[#]], 7, !nosanitize + // CHECK-COMMON: icmp eq i64 %[[#]], 0, !nosanitize + // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 22) + + // CHECK-COMMON: icmp ne ptr {{.*}}, null + // CHECK-UBSAN: call void @__ubsan_handle_nonnull_arg( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 16) + // CHECK-COMMON: and i64 %[[#]], 1, !nosanitize + // CHECK-COMMON: icmp eq i64 %[[#]], 0, !nosanitize + // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1( + // CHECK-TRAP: call void @llvm.ubsantrap(i8 22) + + // CHECK-COMMON: call void @llvm.memmove.p0.p0.i64(ptr align 8 %0, ptr align 2 %1, i64 %conv, i1 false) + memmove(p, q, sz); +} + // CHECK-COMMON-LABEL: @call_nonnull_variadic __attribute__((nonnull)) void nonnull_variadic(int a, ...); void call_nonnull_variadic(int a, int *b) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits