diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -46,7 +46,14 @@ Major New Features ------------------ -- ... +- Passing underaligned pointers as function arguments is undefined behaviour. + Previously, clang was not making any optimizations based on that, + however, there's interest in doing so now. + UBSan's ``-fsanitize=alignment`` was enhanced to catch the cases where + an underaligned pointer was passed as a function argument, + to allow codebases to be cleaned up in preparation for this optimization, + to avoid miscompiles. + Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -231,6 +238,11 @@ Undefined Behavior Sanitizer (UBSan) ------------------------------------ +- ``-fsanitize=alignment`` was enhanced to catch the cases where + an underaligned pointer was passed as a function argument. + In the future clang will optimize based on that UB. + + Core Analysis Improvements ========================== diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -656,7 +656,8 @@ bool CodeGenFunction::isNullPointerAllowed(TypeCheckKind TCK) { return TCK == TCK_DowncastPointer || TCK == TCK_Upcast || - TCK == TCK_UpcastToVirtualBase || TCK == TCK_DynamicOperation; + TCK == TCK_UpcastToVirtualBase || TCK == TCK_DynamicOperation || + TCK == TCK_FunctionPointerArgument; } bool CodeGenFunction::isVptrCheckRequired(TypeCheckKind TCK, QualType Ty) { diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -2898,7 +2898,9 @@ TCK_NonnullAssign, /// Checking the operand of a dynamic_cast or a typeid expression. Must be /// null or an object within its lifetime. - TCK_DynamicOperation + TCK_DynamicOperation, + /// Checking the alignment of the function's pointer argument. + TCK_FunctionPointerArgument }; /// Determine whether the pointer type check \p TCK permits null pointers. diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1156,6 +1156,32 @@ } } + if (SanOpts.has(SanitizerKind::Alignment)) { + // If we are optimizing based on the strict pointer alignment rules, + // check that each pointer argument is aligned appropriately. + for (const VarDecl *VD : Args) { + QualType Ty = VD->getType(); + const auto *PtrTy = Ty->getAs(); + if (!PtrTy) + continue; + + QualType PTy = PtrTy->getPointeeType(); + if (!PTy->isObjectType()) + continue; + + SanitizerSet SkippedChecks; + SkippedChecks.set(SanitizerKind::Null, true); + SkippedChecks.set(SanitizerKind::ObjectSize, true); + SkippedChecks.set(SanitizerKind::Vptr, true); + + Address AddrOfArg = LocalDeclMap.find(VD)->second; + llvm::Value *Arg = EmitLoadOfScalar(AddrOfArg, /*Volatile=*/false, Ty, + VD->getLocation()); + EmitTypeCheck(TCK_FunctionPointerArgument, VD->getLocation(), Arg, PTy, + /*Alignment=*/CharUnits(), SkippedChecks); + } + } + // If any of the arguments have a variably modified type, make sure to // emit the type size. for (FunctionArgList::const_iterator i = Args.begin(), e = Args.end(); diff --git a/clang/test/CodeGen/catch-function-pointer-argument-alignment.cpp b/clang/test/CodeGen/catch-function-pointer-argument-alignment.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeGen/catch-function-pointer-argument-alignment.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call" +// RUN: %clang_cc1 -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE +// RUN: %clang_cc1 -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER +// RUN: %clang_cc1 -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE + +// CHECK-SANITIZE-ANYRECOVER: @[[INT:.*]] = {{.*}} c"'int'\00" } +// CHECK-SANITIZE-ANYRECOVER: @[[LINE_100_POINTER_ARGUMENT:.*]] = {{.*}}, i32 100, i32 16 }, {{.*}}* @[[INT]], i8 2, i8 12 } + +#line 100 +void func(int *data) { + // CHECK: define{{.*}} void @{{.*}}(i32* %[[DATA:.*]]) + // CHECK-NEXT: [[ENTRY:.*]]: + // CHECK-NEXT: %[[DATA_ADDR:.*]] = alloca i32*, align 8 + // CHECK-NEXT: store i32* %[[DATA]], i32** %[[DATA_ADDR]], align 8 + // CHECK-SANITIZE-NEXT: %[[DATA_ADDR_RELOADED:.*]] = load i32*, i32** %[[DATA_ADDR]], align 8 + // CHECK-SANITIZE-NEXT: %[[PTRINT:.*]] = ptrtoint i32* %[[DATA_ADDR_RELOADED]] to i64 + // CHECK-SANITIZE-NEXT: %[[MASKEDPTR:.*]] = and i64 %[[PTRINT]], 3 + // CHECK-SANITIZE-NEXT: %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0 + // CHECK-SANITIZE-NEXT: br i1 %[[MASKCOND]], label %[[CONT:.*]], label %[[HANDLER_TYPE_MISMATCH:[^,]+]],{{.*}} !nosanitize + + // CHECK-SANITIZE: [[HANDLER_TYPE_MISMATCH]]: + // CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_type_mismatch_v1_abort(i8* bitcast ({ {{{.*}}}, {{{.*}}}*, {{.*}}, {{.*}} }* @[[LINE_100_POINTER_ARGUMENT]] to i8*), i64 %[[PTRINT]]){{.*}}, !nosanitize + // CHECK-SANITIZE-RECOVER-NEXT: call void @__ubsan_handle_type_mismatch_v1(i8* bitcast ({ {{{.*}}}, {{{.*}}}*, {{.*}}, {{.*}} }* @[[LINE_100_POINTER_ARGUMENT]] to i8*), i64 %[[PTRINT]]){{.*}}, !nosanitize + // CHECK-SANITIZE-TRAP-NEXT: call void @llvm.ubsantrap(i8 22){{.*}}, !nosanitize + // CHECK-SANITIZE-UNREACHABLE-NEXT: unreachable, !nosanitize + + // CHECK-SANITIZE: [[CONT]]: + // CHECK: ret void +} diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp --- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp @@ -73,15 +73,25 @@ TCK_NonnullAssign, /// Checking the operand of a dynamic_cast or a typeid expression. Must be /// null or an object within its lifetime. - TCK_DynamicOperation + TCK_DynamicOperation, + /// Checking the alignment of the function's pointer argument. + TCK_FunctionPointerArgument }; -const char *TypeCheckKinds[] = { - "load of", "store to", "reference binding to", "member access within", - "member call on", "constructor call on", "downcast of", "downcast of", - "upcast of", "cast to virtual base of", "_Nonnull binding to", - "dynamic operation on"}; -} +const char *TypeCheckKinds[] = {"load of", + "store to", + "reference binding to", + "member access within", + "member call on", + "constructor call on", + "downcast of", + "downcast of", + "upcast of", + "cast to virtual base of", + "_Nonnull binding to", + "dynamic operation on", + "function pointer argument has"}; +} // namespace __ubsan static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer, ReportOptions Opts) { diff --git a/compiler-rt/test/ubsan/TestCases/Pointer/function-pointer-argument.cpp b/compiler-rt/test/ubsan/TestCases/Pointer/function-pointer-argument.cpp new file mode 100644 --- /dev/null +++ b/compiler-rt/test/ubsan/TestCases/Pointer/function-pointer-argument.cpp @@ -0,0 +1,30 @@ +// RUN: %clang -x c -fsanitize=alignment -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" +// RUN: %clang -x c -fsanitize=alignment -O1 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" +// RUN: %clang -x c -fsanitize=alignment -O2 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" +// RUN: %clang -x c -fsanitize=alignment -O3 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" + +// RUN: %clang -x c++ -fsanitize=alignment -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" +// RUN: %clang -x c++ -fsanitize=alignment -O1 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" +// RUN: %clang -x c++ -fsanitize=alignment -O2 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" +// RUN: %clang -x c++ -fsanitize=alignment -O3 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:" + +#include + +struct S { + int k; +} __attribute__((aligned(8192))); + +void take_pointer_to_overaligned_struct(struct S *x) { +} + +int main(int argc, char *argv[]) { + char *ptr = (char *)malloc(2 * sizeof(struct S)); + + take_pointer_to_overaligned_struct((struct S *)(ptr + 1)); + // CHECK: {{.*}}function-pointer-argument.cpp:[[@LINE-7]]:51: runtime error: function pointer argument has misaligned address {{.*}} for type 'struct S', which requires 8192 byte alignment + // CHECK: 0x{{.*}}: note: pointer points here + + free(ptr); + + return 0; +}