lebedev.ri updated this revision to Diff 335893.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
Unguard the UBSan check under `-fstrict*`.
In D100037#2674610 <https://reviews.llvm.org/D100037#2674610>, @rsmith wrote:
> I assume your intent here is to eventually add LLVM parameter attributes
> indicating the function argument is correctly-aligned, which is why you're
> checking on call boundaries?
Yep, D99791 <https://reviews.llvm.org/D99791>.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100037/new/
https://reviews.llvm.org/D100037
Files:
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGen/catch-function-pointer-argument-alignment.cpp
compiler-rt/lib/ubsan/ubsan_handlers.cpp
compiler-rt/test/ubsan/TestCases/Pointer/function-pointer-argument.cpp
Index: compiler-rt/test/ubsan/TestCases/Pointer/function-pointer-argument.cpp
===================================================================
--- /dev/null
+++ 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 <stdlib.h>
+
+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;
+}
Index: compiler-rt/lib/ubsan/ubsan_handlers.cpp
===================================================================
--- compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ 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) {
Index: clang/test/CodeGen/catch-function-pointer-argument-alignment.cpp
===================================================================
--- /dev/null
+++ 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
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ 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.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ 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<PointerType>();
+ 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();
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ 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) {
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ 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
==========================
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
