MaskRay created this revision.
MaskRay added reviewers: efriedma, rjmccall, simon_tatham, samitolvanen.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fix https://github.com/llvm/llvm-project/issues/63579
% cat a.c
void foo() {}
% clang --target=arm-none-eabi -mthumb -mno-unaligned-access -fsanitize=kcfi
a.c -S -o - | grep p2align
.p2align 1
% clang --target=armv6m-none-eabi -fsanitize=function a.c -S -o - | grep
p2align
.p2align 1
With -mno-unaligned-access (possibly implicit), we should ensure that
-fsanitize={function,kcfi} instrumented functions are aligned by at least 4, so
that loading the type hash before the function label will not cause a misaligned
access, even if the backend doesn't set `setMinFunctionAlignment` to 4 or
greater.
With this patch, the generated assembly for the examples above will contain
`.p2align 2`.
If `-falign-functions=` is specified, take the maxiumum.
If `__attribute__((aligned(2)))` is specified, arbitrarily let the function
attribute win.
Since `SanOpts` is per-function, move the alignment setting code from
CodeGenModule::SetLLVMFunctionAttributesForDefinition to CodeGenFunction.
This move requires some attention.
Note: CodeGenModule::SetLLVMFunctionAttributesForDefinition is called by many
thunk codegen code with a dummy GlobalDecl/FunctionDecl.
However, in one call site, MicrosoftCXXABI::EmitVirtualMemPtrThunk has a
`SetLLVMFunctionAttributesForDefinition` use case that requires the
"Some C++ ABIs require 2-byte alignment for member functions" code. So
keep this part in CodeGenModule.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154043
Files:
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/kcfi.c
clang/test/CodeGen/ubsan-function.cpp
Index: clang/test/CodeGen/ubsan-function.cpp
===================================================================
--- clang/test/CodeGen/ubsan-function.cpp
+++ clang/test/CodeGen/ubsan-function.cpp
@@ -3,9 +3,23 @@
// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,64
// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,ARM,32
-// CHECK: define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
+/// With -munaligned-access, ensure that the alignment is at least 4.
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all -target-feature +strict-align | FileCheck %s --check-prefix=ALIGN4
+/// Smaller -faligned-function= is overridden while larger -faligned-function= wins.
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all -target-feature +strict-align -function-alignment 1 | FileCheck %s --check-prefix=ALIGN4
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all -target-feature +strict-align -function-alignment 5 | FileCheck %s --check-prefix=ALIGN32
+
+// CHECK: define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
+// ALIGN4: define{{.*}} void @_Z3funv() #0 align 4 !func_sanitize ![[FUNCSAN:.*]] {
+// ALIGN32: define{{.*}} void @_Z3funv() #0 align 32 !func_sanitize ![[FUNCSAN:.*]] {
void fun() {}
+// CHECK: define{{.*}} void @_Z8aligned2v() #[[#]] align 2
+// ALIGN4: define{{.*}} void @_Z8aligned2v() #[[#]] align 2
+// ALIGN32: define{{.*}} void @_Z8aligned2v() #[[#]] align 2
+__attribute__((aligned(2)))
+void aligned2() {}
+
// CHECK-LABEL: define{{.*}} void @_Z6callerPFvvE(ptr noundef %f)
// ARM: ptrtoint ptr {{.*}} to i32, !nosanitize !5
// ARM: and i32 {{.*}}, -2, !nosanitize !5
Index: clang/test/CodeGen/kcfi.c
===================================================================
--- clang/test/CodeGen/kcfi.c
+++ clang/test/CodeGen/kcfi.c
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -x c++ -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fpatchable-function-entry-offset=3 -o - %s | FileCheck %s --check-prefixes=CHECK,OFFSET
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s --check-prefixes=CHECK,BUNDLE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -x c++ -o - %s | FileCheck %s --check-prefixes=CHECK,BUNDLE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fpatchable-function-entry-offset=3 -o - %s | FileCheck %s --check-prefixes=CHECK,BUNDLE,OFFSET
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -fsanitize=kcfi -target-feature -strict-align -o - %s | FileCheck %s --check-prefix=CHECK
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -fsanitize=kcfi -target-feature +strict-align -o - %s | FileCheck %s --check-prefix=STRICTALIGN
#if !__has_feature(kcfi)
#error Missing kcfi?
#endif
@@ -13,6 +15,7 @@
typedef int (*fn_t)(void);
// CHECK: define dso_local{{.*}} i32 @{{f1|_Z2f1v}}(){{.*}} !kcfi_type ![[#TYPE:]]
+// STRICTALIGN: define{{.*}} i32 @f1() #[[#]] align 4 !kcfi_type ![[#TYPE:]]
int f1(void) { return 0; }
// CHECK: define dso_local{{.*}} i32 @{{f2|_Z2f2v}}(){{.*}} !kcfi_type ![[#TYPE2:]]
@@ -26,7 +29,7 @@
// CHECK: define dso_local{{.*}} i32 @{{call|_Z4callPFivE}}(ptr{{.*}} %f){{.*}}
int call(fn_t f) {
- // CHECK: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#HASH]]) ]
+ // BUNDLE: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#HASH]]) ]
return f();
}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2373,14 +2373,6 @@
F->addFnAttrs(B);
- unsigned alignment = D->getMaxAlignment() / Context.getCharWidth();
- if (alignment)
- F->setAlignment(llvm::Align(alignment));
-
- if (!D->hasAttr<AlignedAttr>())
- if (LangOpts.FunctionAlignment)
- F->setAlignment(llvm::Align(1ull << LangOpts.FunctionAlignment));
-
// Some C++ ABIs require 2-byte alignment for member functions, in order to
// reserve a bit for differentiating between virtual and non-virtual member
// functions. If the current target's C++ ABI requires this and this is a
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -818,6 +818,27 @@
FD->getBody()->getStmtClass() == Stmt::CoroutineBodyStmtClass)
SanOpts.Mask &= ~SanitizerKind::Null;
+ if (FD && FD->hasAttr<AlignedAttr>()) {
+ if (unsigned alignment =
+ FD->getMaxAlignment() / getContext().getCharWidth())
+ Fn->setAlignment(llvm::Align(alignment));
+ } else if (FD) {
+ if (getLangOpts().FunctionAlignment)
+ Fn->setAlignment(llvm::Align(1ull << getLangOpts().FunctionAlignment));
+ // -fsanitize=function and -fsanitize=kcfi instrument indirect function
+ // calls to load a type hash before the function label. Ensure the function
+ // is aligned by a least 4 to avoid unaligned access for
+ // -mno-unaligned-access, even if the backend does not increase the
+ // alignment.
+ if (Fn->getAlignment() < 4 && (SanOpts.has(SanitizerKind::Function) ||
+ SanOpts.has(SanitizerKind::KCFI))) {
+ llvm::StringMap<bool> FeatureMap;
+ getContext().getFunctionFeatureMap(FeatureMap, GD);
+ if (FeatureMap.lookup("strict-align"))
+ Fn->setAlignment(llvm::Align(4));
+ }
+ }
+
// Apply xray attributes to the function (as a string, for now)
bool AlwaysXRayAttr = false;
if (const auto *XRayAttr = D ? D->getAttr<XRayInstrumentAttr>() : nullptr) {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
