[clang] -fsanitize=alignment: check memcpy/memmove arguments (PR #67766)

Fangrui Song via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 28 22:55:45 PDT 2023


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 at 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) {



More information about the cfe-commits mailing list