Skip to content

Commit 06529d3

Browse files
ahatanakahatanak
committed
Address review comments
- Remove unused diagnostic. - Change the size of Key fields to 2 bits. - Stop passing the default argument to CGCallee's constructor. - Remove unused functions and variables. - Add a test for indirect calls. - Run tests on linux too.
1 parent f0a065d commit 06529d3

17 files changed

+63
-77
lines changed

‎clang/include/clang/Basic/DiagnosticDriverKinds.td

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,6 @@ def err_drv_omp_host_ir_file_not_found : Error<
351351
"target regions but cannot be found">;
352352
def err_drv_omp_host_target_not_supported : Error<
353353
"target '%0' is not a supported OpenMP host target">;
354-
def err_drv_ptrauth_not_supported : Error<
355-
"target '%0' does not support native pointer authentication">;
356-
357354
def err_drv_expecting_fopenmp_with_fopenmp_targets : Error<
358355
"'-fopenmp-targets' must be used in conjunction with a '-fopenmp' option "
359356
"compatible with offloading; e.g., '-fopenmp=libomp' or '-fopenmp=libiomp5'">;

‎clang/include/clang/Basic/PointerAuthOptions.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class PointerAuthSchema {
5858
unsigned AuthenticatesNullValues : 1;
5959
PointerAuthenticationMode SelectedAuthenticationMode : 2;
6060
Discrimination DiscriminationKind : 2;
61-
unsigned Key : 4;
61+
unsigned Key : 2;
6262
unsigned ConstantDiscriminator : 16;
6363

6464
public:

‎clang/include/clang/Frontend/CompilerInvocation.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,12 +306,11 @@ class CompilerInvocation : public CompilerInvocationBase {
306306
static std::string GetResourcesPath(const char *Argv0, void *MainAddr);
307307

308308
/// Populate \p Opts with the default set of pointer authentication-related
309-
/// options given \p LangOpts and \p Triple. Return true if defaults are
310-
/// available.
309+
/// options given \p LangOpts and \p Triple.
311310
///
312311
/// Note: This is intended to be used by tools which must be aware of
313312
/// pointer authentication-related code generation, e.g. lldb.
314-
static bool setDefaultPointerAuthOptions(PointerAuthOptions &Opts,
313+
static void setDefaultPointerAuthOptions(PointerAuthOptions &Opts,
315314
const LangOptions &LangOpts,
316315
const llvm::Triple &Triple);
317316

‎clang/lib/CodeGen/CGCall.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ class CGCallee {
109109

110110
/// Construct a callee. Call this constructor directly when this
111111
/// isn't a direct call.
112-
CGCallee(
113-
const CGCalleeInfo &abstractInfo, llvm::Value *functionPtr,
114-
const CGPointerAuthInfo &pointerAuthInfo = /*FIXME*/ CGPointerAuthInfo())
112+
CGCallee(const CGCalleeInfo &abstractInfo, llvm::Value *functionPtr,
113+
/* FIXME: make parameter pointerAuthInfo mandatory */
114+
const CGPointerAuthInfo &pointerAuthInfo = CGPointerAuthInfo())
115115
: KindOrFunctionPointer(
116116
SpecialKind(reinterpret_cast<uintptr_t>(functionPtr))) {
117117
OrdinaryInfo.AbstractInfo = abstractInfo;
@@ -136,12 +136,12 @@ class CGCallee {
136136

137137
static CGCallee forDirect(llvm::Constant *functionPtr,
138138
const CGCalleeInfo &abstractInfo = CGCalleeInfo()) {
139-
return CGCallee(abstractInfo, functionPtr, CGPointerAuthInfo());
139+
return CGCallee(abstractInfo, functionPtr);
140140
}
141141

142142
static CGCallee forDirect(llvm::FunctionCallee functionPtr,
143143
const CGCalleeInfo &abstractInfo = CGCalleeInfo()) {
144-
return CGCallee(abstractInfo, functionPtr.getCallee(), CGPointerAuthInfo());
144+
return CGCallee(abstractInfo, functionPtr.getCallee());
145145
}
146146

147147
static CGCallee forVirtual(const CallExpr *CE, GlobalDecl MD, Address Addr,

‎clang/lib/CodeGen/CGPointerAuth.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,20 +80,16 @@ CodeGen::getConstantSignedPointer(CodeGenModule &CGM, llvm::Constant *Pointer,
8080
OtherDiscriminator);
8181
}
8282

83-
/// If applicable, sign a given constant function pointer with the ABI rules for
84-
/// functionType.
8583
llvm::Constant *CodeGenModule::getFunctionPointer(llvm::Constant *Pointer,
86-
QualType FunctionType,
87-
GlobalDecl GD) {
84+
QualType FunctionType) {
8885
assert(FunctionType->isFunctionType() ||
8986
FunctionType->isFunctionReferenceType() ||
9087
FunctionType->isFunctionPointerType());
9188

92-
if (auto PointerAuth = getFunctionPointerAuthInfo(FunctionType)) {
89+
if (auto PointerAuth = getFunctionPointerAuthInfo(FunctionType))
9390
return getConstantSignedPointer(
94-
Pointer, PointerAuth.getKey(), nullptr,
95-
cast_or_null<llvm::Constant>(PointerAuth.getDiscriminator()));
96-
}
91+
Pointer, PointerAuth.getKey(), /*StorageAddress=*/nullptr,
92+
cast_or_null<llvm::Constant>(PointerAuth.getDiscriminator()));
9793

9894
return Pointer;
9995
}
@@ -102,5 +98,5 @@ llvm::Constant *CodeGenModule::getFunctionPointer(GlobalDecl GD,
10298
llvm::Type *Ty) {
10399
const auto *FD = cast<FunctionDecl>(GD.getDecl());
104100
QualType FuncType = FD->getType();
105-
return getFunctionPointer(getRawFunctionPointer(GD, Ty), FuncType, GD);
101+
return getFunctionPointer(getRawFunctionPointer(GD, Ty), FuncType);
106102
}

‎clang/lib/CodeGen/CGPointerAuthInfo.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class CGPointerAuthInfo {
2626
PointerAuthenticationMode AuthenticationMode : 2;
2727
unsigned IsIsaPointer : 1;
2828
unsigned AuthenticatesNullValues : 1;
29-
unsigned Key : 4;
29+
unsigned Key : 2;
3030
llvm::Value *Discriminator;
3131

3232
public:

‎clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3053,12 +3053,11 @@ void CodeGenFunction::EmitPointerAuthOperandBundle(
30533053
if (!PointerAuth.isSigned())
30543054
return;
30553055

3056-
auto Key = Builder.getInt32(PointerAuth.getKey());
3056+
auto *Key = Builder.getInt32(PointerAuth.getKey());
30573057

30583058
llvm::Value *Discriminator = PointerAuth.getDiscriminator();
3059-
if (!Discriminator) {
3059+
if (!Discriminator)
30603060
Discriminator = Builder.getSize(0);
3061-
}
30623061

30633062
llvm::Value *Args[] = {Key, Discriminator};
30643063
Bundles.emplace_back("ptrauth", Args);

‎clang/lib/CodeGen/CodeGenModule.h

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -952,19 +952,12 @@ class CodeGenModule : public CodeGenTypeCache {
952952

953953
/// Return the ABI-correct function pointer value for a reference
954954
/// to the given function. This will apply a pointer signature if
955-
/// necessary, but will only cache the result if \p FD is passed.
955+
/// necessary.
956956
llvm::Constant *getFunctionPointer(llvm::Constant *Pointer,
957-
QualType FunctionType,
958-
GlobalDecl GD = GlobalDecl());
957+
QualType FunctionType);
959958

960959
CGPointerAuthInfo getFunctionPointerAuthInfo(QualType T);
961960

962-
llvm::Constant *getConstantSignedPointer(llvm::Constant *Pointer,
963-
const PointerAuthSchema &Schema,
964-
llvm::Constant *StorageAddress,
965-
GlobalDecl SchemaDecl,
966-
QualType SchemaType);
967-
968961
llvm::Constant *getConstantSignedPointer(llvm::Constant *Pointer,
969962
unsigned Key,
970963
llvm::Constant *StorageAddress,

‎clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,36 +1458,27 @@ static void setPGOUseInstrumentor(CodeGenOptions &Opts,
14581458
Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
14591459
}
14601460

1461-
bool CompilerInvocation::setDefaultPointerAuthOptions(
1461+
void CompilerInvocation::setDefaultPointerAuthOptions(
14621462
PointerAuthOptions &Opts, const LangOptions &LangOpts,
14631463
const llvm::Triple &Triple) {
1464-
if (Triple.getArch() == llvm::Triple::aarch64) {
1465-
if (LangOpts.PointerAuthCalls) {
1466-
using Key = PointerAuthSchema::ARM8_3Key;
1467-
using Discrimination = PointerAuthSchema::Discrimination;
1468-
// If you change anything here, be sure to update <ptrauth.h>.
1469-
Opts.FunctionPointers =
1470-
PointerAuthSchema(Key::ASIA, false, Discrimination::None);
1471-
}
1472-
return true;
1464+
assert(Triple.getArch() == llvm::Triple::aarch64);
1465+
if (LangOpts.PointerAuthCalls) {
1466+
using Key = PointerAuthSchema::ARM8_3Key;
1467+
using Discrimination = PointerAuthSchema::Discrimination;
1468+
// If you change anything here, be sure to update <ptrauth.h>.
1469+
Opts.FunctionPointers =
1470+
PointerAuthSchema(Key::ASIA, false, Discrimination::None);
14731471
}
1474-
1475-
return false;
14761472
}
14771473

1478-
static bool parsePointerAuthOptions(PointerAuthOptions &Opts,
1474+
static void parsePointerAuthOptions(PointerAuthOptions &Opts,
14791475
const LangOptions &LangOpts,
14801476
const llvm::Triple &Triple,
14811477
DiagnosticsEngine &Diags) {
14821478
if (!LangOpts.PointerAuthCalls)
1483-
return true;
1484-
1485-
if (CompilerInvocation::setDefaultPointerAuthOptions(Opts, LangOpts, Triple))
1486-
return true;
1479+
return;
14871480

1488-
Diags.Report(diag::err_drv_ptrauth_not_supported)
1489-
<< Triple.str();
1490-
return false;
1481+
CompilerInvocation::setDefaultPointerAuthOptions(Opts, LangOpts, Triple);
14911482
}
14921483

14931484
void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,

‎clang/test/CodeGen/ptrauth-function-attributes.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
// RUN: %clang_cc1 -triple arm64-apple-ios -emit-llvm %s -o - | FileCheck %s --check-prefixes=ALL,OFF
21
// RUN: %clang_cc1 -triple arm64e-apple-ios -emit-llvm %s -o - | FileCheck %s --check-prefixes=ALL,OFF
2+
// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefixes=ALL,OFF
33

44
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm %s -o - | FileCheck %s --check-prefixes=ALL,CALLS
5-
// RUN: %clang_cc1 -triple arm64e-apple-ios -fptrauth-calls -emit-llvm %s -o - | FileCheck %s --check-prefixes=ALL,CALLS
5+
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -emit-llvm %s -o - | FileCheck %s --check-prefixes=ALL,CALLS
66

7-
// ALL-LABEL: define void @test() #0
7+
// ALL: define {{(dso_local )?}}void @test() #0
88
void test() {
99
}
1010

‎clang/test/CodeGen/ptrauth-function-init-fail.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// RUN: %clang_cc1 -triple arm64e-apple-ios -fptrauth-calls %s -verify -emit-llvm -o -
2+
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls %s -verify -emit-llvm -o -
23

34
void f(void);
45

6+
// FIXME: We need a better diagnostic here.
57
int *pf = (int *)&f + 1; // expected-error{{cannot compile this static initializer yet}}

‎clang/test/CodeGen/ptrauth-function-init.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// RUN: %clang_cc1 %s -triple arm64e-apple-ios13 -fptrauth-calls -fptrauth-intrinsics -disable-llvm-passes -emit-llvm -o- | FileCheck %s
2+
// RUN: %clang_cc1 %s -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-intrinsics -disable-llvm-passes -emit-llvm -o- | FileCheck %s
23
// RUN: %clang_cc1 -xc++ %s -triple arm64e-apple-ios13 -fptrauth-calls -fptrauth-intrinsics -disable-llvm-passes -emit-llvm -o- | FileCheck %s --check-prefixes=CHECK,CXX
4+
// RUN: %clang_cc1 -xc++ %s -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-intrinsics -disable-llvm-passes -emit-llvm -o- | FileCheck %s --check-prefixes=CHECK,CXX
35

46
#ifdef __cplusplus
57
extern "C" {
@@ -9,15 +11,18 @@ void f(void);
911

1012
#ifdef __cplusplus
1113

12-
// CXX-LABEL: define internal void @__cxx_global_var_init()
14+
// CXX: define {{(dso_local )?}}internal void @__cxx_global_var_init()
1315
// CXX: store ptr getelementptr inbounds (i32, ptr ptrauth (ptr @f, i32 0), i64 2), ptr @_ZL2fp, align 8
1416

17+
// This is rejected in C mode as adding a non-zero constant to a signed pointer
18+
// is unrepresentable in relocations. In C++ mode, this can be done dynamically
19+
// by the global constructor.
1520
__attribute__((used))
16-
void (*const fp)(void) = (void (*)(void))((int *)&f + 2); // Error in C mode.
21+
void (*const fp)(void) = (void (*)(void))((int *)&f + 2);
1722

1823
#endif
1924

20-
// CHECK-LABEL: define void @t1()
25+
// CHECK: define {{(dso_local )?}}void @t1()
2126
void t1() {
2227
// CHECK: [[PF:%.*]] = alloca ptr
2328
// CHECK: store ptr getelementptr inbounds (i32, ptr ptrauth (ptr @f, i32 0), i64 2), ptr [[PF]]

‎clang/test/CodeGen/ptrauth-function-lvalue-cast.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
// RUN: %clang_cc1 %s -triple arm64e-apple-ios13 -fptrauth-calls -fptrauth-intrinsics -emit-llvm -o- | FileCheck %s
2+
// RUN: %clang_cc1 %s -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-intrinsics -emit-llvm -o- | FileCheck %s
23

34
typedef void (*fptr_t)(void);
45

56
char *cptr;
67
void (*fptr)(void);
78

8-
// CHECK-LABEL: define void @test1
9+
// CHECK: define {{(dso_local )?}}void @test1
910
void test1() {
1011
// CHECK: [[LOAD:%.*]] = load ptr, ptr @cptr
1112
// CHECK: call void [[LOAD]]() [ "ptrauth"(i32 0, i64 0) ]
@@ -14,7 +15,7 @@ void test1() {
1415
(*(fptr_t)cptr)();
1516
}
1617

17-
// CHECK-LABEL: define i8 @test2
18+
// CHECK: define {{(dso_local )?}}i8 @test2
1819
char test2() {
1920
return *(char *)fptr;
2021
// CHECK: [[LOAD:%.*]] = load ptr, ptr @fptr

‎clang/test/CodeGen/ptrauth-function.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,31 @@
11
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fptrauth-intrinsics -emit-llvm %s -o - | FileCheck -check-prefix=CHECK %s
2+
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-intrinsics -emit-llvm %s -o - | FileCheck -check-prefix=CHECK %s
23

34
void test_call();
45

5-
// CHECK-LABEL: define void @test_direct_call()
6+
// CHECK: define {{(dso_local )?}}void @test_direct_call()
67
void test_direct_call() {
78
// CHECK: call void @test_call(){{$}}
89
test_call();
910
}
1011

12+
// CHECK: define {{(dso_local )?}}void @test_indirect_call(ptr noundef %[[FP:.*]])
13+
void test_indirect_call(void (*fp(void))) {
14+
// CHECK: %[[FP_ADDR:.*]] = alloca ptr, align 8
15+
// CHECK: store ptr %[[FP]], ptr %[[FP_ADDR]], align 8
16+
// CHECK: %[[V0:.*]] = load ptr, ptr %[[FP_ADDR]], align 8
17+
// CHECK: %[[CALL:.*]] = call ptr %[[V0]]() [ "ptrauth"(i32 0, i64 0) ]
18+
fp();
19+
}
20+
1121
void abort();
12-
// CHECK-LABEL: define void @test_direct_builtin_call()
22+
// CHECK: define {{(dso_local )?}}void @test_direct_builtin_call()
1323
void test_direct_builtin_call() {
1424
// CHECK: call void @abort() {{#[0-9]+$}}
1525
abort();
1626
}
1727

18-
// CHECK-LABEL: define void @test_memcpy_inline(
28+
// CHECK-LABEL: define {{(dso_local )?}}void @test_memcpy_inline(
1929
// CHECK-NOT: call{{.*}}memcpy
2030

2131
extern inline __attribute__((__always_inline__))

‎clang/test/CodeGen/ptrauth-weak_import.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fptrauth-intrinsics -emit-llvm %s -o - | FileCheck %s
2+
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-intrinsics -emit-llvm %s -o - | FileCheck %s
23

34
extern void foo() __attribute__((weak_import));
45

5-
// CHECK-LABEL: define void @bar()
6+
// CHECK: define {{(dso_local )?}}void @bar()
67
// CHECK: br i1 icmp ne (ptr ptrauth (ptr @foo, i32 0), ptr null), label
78
void bar() {
89
if (foo)

‎clang/test/CodeGenCXX/ptrauth.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm -std=c++11 -fexceptions -fcxx-exceptions -o - %s | FileCheck %s
2+
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -emit-llvm -std=c++11 -fexceptions -fcxx-exceptions -o - %s | FileCheck %s
23

34
void f(void);
45
auto &f_ref = f;
56

6-
// CHECK-LABEL: define void @_Z1gv(
7+
// CHECK: define {{(dso_local )?}}void @_Z1gv(
78
// CHECK: call void ptrauth (ptr @_Z1fv, i32 0)() [ "ptrauth"(i32 0, i64 0) ]
89

910
void g() { f_ref(); }
@@ -14,7 +15,7 @@ void test_terminate() noexcept {
1415
foo1();
1516
}
1617

17-
// CHECK: define void @_ZSt9terminatev() #[[ATTR4:.*]] {
18+
// CHECK: define {{(dso_local )?}}void @_ZSt9terminatev() #[[ATTR4:.*]] {
1819

1920
namespace std {
2021
void terminate() noexcept {

‎clang/test/Driver/ptrauth.c

Lines changed: 0 additions & 9 deletions
This file was deleted.

0 commit comments

Comments
 (0)