From 20d6dee40d507d467d3312d5e7dfdf088f106d31 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 24 May 2023 09:28:33 +0100 Subject: [PATCH] -fsanitize=function: fix alignment fault on Arm targets. Function pointers are checked by loading a prefix structure from just before the function's entry point. However, on Arm, the function pointer is not always exactly equal to the address of the entry point, because Thumb function pointers have the low bit set to tell the BX instruction to enter them in Thumb state. So the generated code loads from an odd address and suffers an alignment fault. Fixed by clearing the low bit of the function pointer before subtracting 8. Differential Revision: https://reviews.llvm.org/D151308 --- clang/lib/CodeGen/CGExpr.cpp | 24 +++++++++++++++++++++++- clang/test/CodeGen/ubsan-function.cpp | 13 +++++++++---- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 2c219d6..c074732 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5364,8 +5364,30 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee llvm::Value *CalleePtr = Callee.getFunctionPointer(); + // On 32-bit Arm, the low bit of a function pointer indicates whether + // it's using the Arm or Thumb instruction set. The actual first + // instruction lives at the same address either way, so we must clear + // that low bit before using the function address to find the prefix + // structure. + // + // This applies to both Arm and Thumb target triples, because + // either one could be used in an interworking context where it + // might be passed function pointers of both types. + llvm::Value *AlignedCalleePtr; + if (CGM.getTriple().isARM() || CGM.getTriple().isThumb()) { + llvm::Value *CalleeAddress = + Builder.CreatePtrToInt(CalleePtr, IntPtrTy); + llvm::Value *Mask = llvm::ConstantInt::get(IntPtrTy, ~1); + llvm::Value *AlignedCalleeAddress = + Builder.CreateAnd(CalleeAddress, Mask); + AlignedCalleePtr = + Builder.CreateIntToPtr(AlignedCalleeAddress, CalleePtr->getType()); + } else { + AlignedCalleePtr = CalleePtr; + } + llvm::Value *CalleePrefixStruct = Builder.CreateBitCast( - CalleePtr, llvm::PointerType::getUnqual(PrefixStructTy)); + AlignedCalleePtr, llvm::PointerType::getUnqual(PrefixStructTy)); llvm::Value *CalleeSigPtr = Builder.CreateConstGEP2_32(PrefixStructTy, CalleePrefixStruct, -1, 0); llvm::Value *CalleeSig = diff --git a/clang/test/CodeGen/ubsan-function.cpp b/clang/test/CodeGen/ubsan-function.cpp index fc9f60f..ba55ee0 100644 --- a/clang/test/CodeGen/ubsan-function.cpp +++ b/clang/test/CodeGen/ubsan-function.cpp @@ -1,11 +1,15 @@ -// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s -// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s -// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,64 +// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,64 +// 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:.*]] { void fun() {} // CHECK-LABEL: define{{.*}} void @_Z6callerPFvvE(ptr noundef %f) +// ARM: ptrtoint ptr {{.*}} to i32, !nosanitize !5 +// ARM: and i32 {{.*}}, -2, !nosanitize !5 +// ARM: inttoptr i32 {{.*}} to ptr, !nosanitize !5 // CHECK: getelementptr <{ i32, i32 }>, ptr {{.*}}, i32 -1, i32 0, !nosanitize // CHECK: load i32, ptr {{.*}}, align {{.*}}, !nosanitize // CHECK: icmp eq i32 {{.*}}, -1056584962, !nosanitize @@ -16,7 +20,8 @@ void fun() {} // CHECK: icmp eq i32 {{.*}}, -1522505972, !nosanitize // CHECK: br i1 {{.*}}, label %[[LABEL3:.*]], label %[[LABEL2:[^,]*]], {{.*}}!nosanitize // CHECK: [[LABEL2]]: -// CHECK: call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], i64 %[[#]]) #[[#]], !nosanitize +// 64: call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], i64 %[[#]]) #[[#]], !nosanitize +// 32: call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], i32 %[[#]]) #[[#]], !nosanitize // CHECK-NEXT: unreachable, !nosanitize // CHECK-EMPTY: // CHECK-NEXT: [[LABEL3]]: -- 2.7.4