[AArch64][FMV] Enable PAuth and BTI hardening of resolver functions #141573

atrosinenko · 2025-05-27T10:07:06Z

No description provided.

llvmbot · 2025-05-27T10:56:59Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Anatoly Trosinenko (atrosinenko)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/141573.diff

2 Files Affected:

(modified) clang/lib/CodeGen/CodeGenModule.cpp (+9)
(added) clang/test/CodeGen/ptrauth-resolver-attributes.c (+25)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 16e010adbeb5f..af8d4cc60de57 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4638,6 +4638,13 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
   if (FD->isTargetMultiVersion() || FD->isTargetClonesMultiVersion())
     AddDeferredMultiVersionResolverToEmit(GD);
 
+  auto SetResolverAttrs = [&](llvm::Function &Resolver) {
+    TargetInfo::BranchProtectionInfo BPI(getLangOpts());
+    TargetCodeGenInfo::setBranchProtectionFnAttributes(BPI, Resolver);
+    TargetCodeGenInfo::setPointerAuthFnAttributes(CodeGenOpts.PointerAuth,
+                                                  Resolver);
+  };
+
   // For cpu_specific, don't create an ifunc yet because we don't know if the
   // cpu_dispatch will be emitted in this translation unit.
   if (ShouldReturnIFunc) {
@@ -4652,6 +4659,7 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
                                   "", Resolver, &getModule());
     GIF->setName(ResolverName);
     SetCommonAttributes(FD, GIF);
+    SetResolverAttrs(cast<llvm::Function>(*Resolver));
     if (ResolverGV)
       replaceDeclarationWith(ResolverGV, GIF);
     return GIF;
@@ -4662,6 +4670,7 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
   assert(isa<llvm::GlobalValue>(Resolver) && !ResolverGV &&
          "Resolver should be created for the first time");
   SetCommonAttributes(FD, cast<llvm::GlobalValue>(Resolver));
+  SetResolverAttrs(cast<llvm::Function>(*Resolver));
   return Resolver;
 }
 
diff --git a/clang/test/CodeGen/ptrauth-resolver-attributes.c b/clang/test/CodeGen/ptrauth-resolver-attributes.c
new file mode 100644
index 0000000000000..8bdedd2c549be
--- /dev/null
+++ b/clang/test/CodeGen/ptrauth-resolver-attributes.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -mbranch-target-enforce -msign-return-address=all      -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,BTI-SIGNRA %s
+// RUN: %clang_cc1 -triple arm64-apple-ios   -mbranch-target-enforce -msign-return-address=all      -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,BTI-SIGNRA %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,PAUTHTEST %s
+// RUN: %clang_cc1 -triple arm64-apple-ios   -fptrauth-calls -fptrauth-returns -fptrauth-auth-traps -emit-llvm %s -o - | FileCheck --check-prefixes=CHECK,PAUTHTEST %s
+
+// Check that resolver functions generated by clang have the correct attributes.
+
+int __attribute__((target_clones("crc", "default"))) ftc(void) { return 0; }
+
+int __attribute__((target_version("crc")))     fmv(void) { return 0; }
+int __attribute__((target_version("default"))) fmv(void) { return 0; }
+
+// CHECK: define{{.*}} i32 @ftc._Mcrc() #0
+// CHECK: define{{.*}} ptr @ftc.resolver() #1
+// CHECK: define{{.*}} i32 @fmv._Mcrc() #0
+// CHECK: define{{.*}} i32 @fmv.default() #2
+// CHECK: define{{.*}} i32 @ftc.default() #2
+// CHECK: define{{.*}} ptr @fmv.resolver() #1
+
+// BTI-SIGNRA: attributes #0 = { {{.*}}"branch-target-enforcement" {{.*}}"sign-return-address"="all" "sign-return-address-key"="a_key"{{.*}} }
+// BTI-SIGNRA: attributes #1 = { {{.*}}"branch-target-enforcement" {{.*}}"sign-return-address"="all" "sign-return-address-key"="a_key"{{.*}} }
+// BTI-SIGNRA: attributes #2 = { {{.*}}"branch-target-enforcement" {{.*}}"sign-return-address"="all" "sign-return-address-key"="a_key"{{.*}} }
+// PAUTHTEST: attributes #0 = { {{.*}}"ptrauth-auth-traps" "ptrauth-calls" "ptrauth-returns"{{.*}} }
+// PAUTHTEST: attributes #1 = { {{.*}}"ptrauth-auth-traps" "ptrauth-calls" "ptrauth-returns"{{.*}} }
+// PAUTHTEST: attributes #2 = { {{.*}}"ptrauth-auth-traps" "ptrauth-calls" "ptrauth-returns"{{.*}} }

clang/lib/CodeGen/CodeGenModule.cpp

labrinea · 2025-06-03T09:21:18Z

Seems somewhat related to #84704. @jroelofs mind taking a look?

efriedma-quic · 2025-06-04T21:20:50Z

clang/lib/CodeGen/CodeGenModule.cpp

@@ -4652,6 +4659,7 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
                                  "", Resolver, &getModule());
    GIF->setName(ResolverName);
    SetCommonAttributes(FD, GIF);
+    SetResolverAttrs(cast<llvm::Function>(Resolver));


Is Resolver actually guaranteed to be a function? GetOrCreateLLVMFunction can return other kinds of GlobalValues in some cases.

Should we be calling SetCommonAttributes/setGVProperties to set the linkage/visibility of the resolver?

GlobalIFunc instructions in the IR currently require that the resolver be a Function with a definition.

I think the question "Should we be calling SetCommonAttributes/setGVProperties to set the linkage/visibility of the resolver?" was never answered.

@efriedma-quic, are you suggesting to remove the call to SetCommonAttributes just above? That would make the resolver non dso_local, which isn't desirable I think. There are tests in place that will start failing.

The part I'm concerned about is that ftc.resolver in the testcase is missing attributes: it's not dso_local, it doesn't get marked "hidden" if you pass -fvisibility=hidden, etc.

However at this point, the IFUNC resolver is just a declaration. We should perhaps call setCommonAttributes and getTargetCodeGenInfo().setTargetAttributes right after the invocation of EmitMultiVersionResolverwhere the actual definition of the resolver function is emitted. I've just tried this and the resolver is indeed marked "hidden" with -fvisibility=hidden.

We usually call setCommonAttributes while we're in the process of emitting the definition, anyway; we don't need the function body for anything. So it's appropriate to call it in the same places we'd call setTargetAttributes().

If you think it would be more clear to move the necessary code somewhere else, that's fine.

When we create the resolver with GetOrCreateLLVMFunction, we construct a GlobalDecl on the spot, and as a result it won't call SetFunctionAttributes because the underlying Decl is nullptr. SetFunctionAttributes is responsible for setting the symbol's visibility and target attributes, therefore it's what we are missing.

Considering calling either SetCommonAttributes or setGVProperties, here are the attributes which could be set with some reasonable value of GD (as I'm not sure if it would be correct to pass through the GD object corresponding to the function defined in the source code when setting attributes of a synthetic resolver function).

SetCommonAttributes: Checking for UsedAttr doesn't make much sense in our case, neither we deal with a variable declaration, thus the only relevant fragment is

const Decl *D = GD.getDecl(); if (isa_and_nonnull<NamedDecl>(D)) setGVProperties(GV, GD); else GV->setVisibility(llvm::GlobalValue::DefaultVisibility);

setGVProperties(llvm::GlobalValue *, GlobalDecl) first calls setDLLImportDLLExport (irrelevant, as it handles externally-visible declaration) and then setGVPropertiesAux:

void CodeGenModule::setGVPropertiesAux(llvm::GlobalValue *GV, const NamedDecl *D) const { setGlobalVisibility(GV, D); setDSOLocal(GV); GV->setPartition(CodeGenOpts.SymbolPartition); }

The only its callee which uses the NamedDecl *D argument is setGlobalVisibility(llvm::GlobalValue *, const NamedDecl *) function that calls GV->setVisibility(...) with the correct visibility type. Notably, if GV has local linkage, it is always DefaultVisibility and D is not even checked.

Considering the EmitMultiVersionResolver, it can be called from two places: either emitCPUDispatchDefinition or emitMultiVersionFunctions, and in both cases the resolver's linkage is explicitly computed to be either WeakODRLinkage or InternalLinkage (see e3b1052).

A proof of concept pushed in 33a92e0

I'd really prefer to share codepaths a bit more... even if it doesn't save any code, having common paths for setting up globals makes future changes to IR a lot easier. If we really can't, though, I guess it's okay.

Visibility doesn't matter for internal functions, but it matters a lot for WeakODRLinkage. I assume we want the resolver to inherit the visibility of the function.

clang/test/CodeGen/ptrauth-resolver-attributes.c

atrosinenko · 2025-07-14T18:48:27Z

I have rebased this PR onto current main branch and moved the new test to AArch64/ subdirectory. There are no unresolved comments anymore, as far as I can see.

labrinea

We need this instead as I explained on the other thread.

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 2d37e0f13199..e26a540a8cb4 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4586,6 +4586,8 @@ void CodeGenModule::emitMultiVersionFunctions() {
         });
     CodeGenFunction CGF(*this);
     CGF.EmitMultiVersionResolver(ResolverFunc, Options);
+    SetFunctionAttributes(GD, ResolverFunc, /*IsIncompleteFunction=*/false,
+                          /*IsThunk=*/false);
   }

   // Ensure that any additions to the deferred decls list caused by emitting a

Also a test to check that hidden is added to the resolver if specified on the commnad line

labrinea · 2025-09-01T12:34:01Z

ping @atrosinenko

atrosinenko · 2025-09-01T12:38:18Z

@labrinea I'm sorry for disappearing, I will update this PR this week.

efriedma-quic

LGTM

atrosinenko requested review from asl, ahmedbougacha, DanielKristofKiss, ojhunt, kovdan01 and ahatanak May 27, 2025 10:08

atrosinenko force-pushed the aarch64-pauth-fmv-resolver branch from 93eca6d to c5a7fea Compare May 27, 2025 10:44

atrosinenko marked this pull request as ready for review May 27, 2025 10:56

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels May 27, 2025

efriedma-quic reviewed May 28, 2025

View reviewed changes

clang/lib/CodeGen/CodeGenModule.cpp Outdated Show resolved Hide resolved

asl added this to Pointer Authentication Tasks Jun 2, 2025

asl moved this to In Progress in Pointer Authentication Tasks Jun 2, 2025

asl assigned atrosinenko Jun 2, 2025

efriedma-quic reviewed Jun 4, 2025

View reviewed changes

labrinea reviewed Jun 5, 2025

View reviewed changes

clang/test/CodeGen/ptrauth-resolver-attributes.c Outdated Show resolved Hide resolved

asl added this to the LLVM 21.x Release milestone Jul 14, 2025

atrosinenko added 4 commits July 14, 2025 21:43

[AArch64][FMV] Enable PAuth and BTI hardening of resolver functions

3e62515

Use setTargetAttributes function

6d63e7e

test: accept functions and attribute groups in any order

2d74565

Move new test file to AArch64/ subdirectory

23a5407

atrosinenko force-pushed the aarch64-pauth-fmv-resolver branch from ec1fe0c to 23a5407 Compare July 14, 2025 18:44

asl removed this from the LLVM 21.x Release milestone Jul 16, 2025

labrinea requested changes Aug 18, 2025

View reviewed changes

[WIP] Set common attributes on the resolver functions

33a92e0

Introduce setMultiVersionResolverAttributes function

01b5690

efriedma-quic approved these changes Sep 5, 2025

View reviewed changes

[AArch64][FMV] Enable PAuth and BTI hardening of resolver functions #141573

Are you sure you want to change the base?

[AArch64][FMV] Enable PAuth and BTI hardening of resolver functions #141573

Conversation

atrosinenko commented May 27, 2025

Uh oh!

llvmbot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

labrinea commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efriedma-quic Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labrinea Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

atrosinenko commented Jul 14, 2025

Uh oh!

labrinea left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labrinea commented Sep 1, 2025

Uh oh!

atrosinenko commented Sep 1, 2025

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented May 27, 2025 •

edited

Loading

efriedma-quic Aug 14, 2025 •

edited

Loading

labrinea Aug 15, 2025 •

edited

Loading

labrinea left a comment •

edited

Loading