Skip to content

[CodeGen][AArch64][FMV] PAC the stub_helper's frame on arm64e #84704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[CodeGen][AArch64][FMV] PAC the stub_helper's frame on arm64e #84704

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b352a41
[CodeGen][AArch64][FMV] PAC the stub_helper's frame on arm64e
jroelofs Mar 10, 2024
73aa834
fixup tests
jroelofs Mar 10, 2024
499382f
[CodeGen][AArch64][FMV] PAC the stub_helper's frame on arm64e
jroelofs Mar 10, 2024
492d65f
Merge commit '499382fcb792' into jroelofs/fmv-arm64e
jroelofs Jul 22, 2024
ec43ccb
drop support for ifunc ptrauth tail. we'll stick the revert of this o…
jroelofs Jul 22, 2024
bd8b5e1
test ptrauth-{calls,returns} attrs on the resolver instead of isArm64e()
jroelofs Jul 22, 2024
96dbf9f
clang-format
jroelofs Jul 22, 2024
94ce822
tighten the test a little
jroelofs Jul 22, 2024
4d5b066
drop ARM64 check prefix
jroelofs Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions clang/lib/CodeGen/CodeGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4365,13 +4365,16 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
// 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 (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
llvm::Type *ResolverType = llvm::FunctionType::get(
llvm::FunctionType *ResolverType = llvm::FunctionType::get(
llvm::PointerType::get(DeclTy,
getTypes().getTargetAddressSpace(FD->getType())),
false);
llvm::Constant *Resolver = GetOrCreateLLVMFunction(
MangledName + ".resolver", ResolverType, GlobalDecl{},
/*ForVTable=*/false);
llvm::Function *Resolver = cast<llvm::Function>(
CreateRuntimeFunction(ResolverType, MangledName + ".resolver")
.getCallee());
llvm::AttrBuilder Attrs(getLLVMContext());
addDefaultFunctionDefinitionAttributes(Attrs);
Resolver->addFnAttrs(Attrs);
llvm::GlobalIFunc *GIF =
llvm::GlobalIFunc::create(DeclTy, 0, getMultiversionLinkage(*this, GD),
"", Resolver, &getModule());
Expand All @@ -4381,8 +4384,11 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
return GIF;
}

llvm::Constant *Resolver = GetOrCreateLLVMFunction(
ResolverName, DeclTy, GlobalDecl{}, /*ForVTable=*/false);
llvm::Function *Resolver = cast<llvm::Function>(
CreateRuntimeFunction(DeclTy, ResolverName).getCallee());
llvm::AttrBuilder Attrs(getLLVMContext());
addDefaultFunctionDefinitionAttributes(Attrs);
Resolver->addFnAttrs(Attrs);
assert(isa<llvm::GlobalValue>(Resolver) &&
"Resolver should be created for the first time");
SetCommonAttributes(FD, cast<llvm::GlobalValue>(Resolver));
Expand Down
58 changes: 58 additions & 0 deletions clang/test/CodeGen/attr-target-version-arm64e.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals --include-generated-funcs
// RUN: %clang_cc1 -triple arm64e-apple-ios -target-feature +ls64 -target-feature +fullfp16 -S -emit-llvm -o - %s | FileCheck %s

int __attribute__((target_version("sha1"))) fmv(void) { return 1; }
int __attribute__((target_version("default"))) fmv(void) { return 0; }
int foo() {
return fmv();
}

//.
// CHECK: @__aarch64_cpu_features = external dso_local global { i64 }
// CHECK: @fmv.ifunc = weak_odr alias i32 (), ptr @fmv
// CHECK: @fmv = weak_odr ifunc i32 (), ptr @fmv.resolver
//.
// CHECK: Function Attrs: noinline nounwind optnone
// CHECK-LABEL: define {{[^@]+}}@fmv._Msha1
// CHECK-SAME: () #[[ATTR0:[0-9]+]] {
// CHECK-NEXT: entry:
// CHECK-NEXT: ret i32 1
//
//
// CHECK: Function Attrs: noinline nounwind optnone
// CHECK-LABEL: define {{[^@]+}}@foo
// CHECK-SAME: () #[[ATTR1:[0-9]+]] {
// CHECK-NEXT: entry:
// CHECK-NEXT: [[CALL:%.*]] = call i32 @fmv()
// CHECK-NEXT: ret i32 [[CALL]]
//
//
// CHECK-LABEL: define {{[^@]+}}@fmv.resolver
// CHECK-SAME: () #[[ATTR2:[0-9]+]] {
// CHECK-NEXT: resolver_entry:
// CHECK-NEXT: call void @__init_cpu_features_resolver()
// CHECK-NEXT: [[TMP0:%.*]] = load i64, ptr @__aarch64_cpu_features, align 8
// CHECK-NEXT: [[TMP1:%.*]] = and i64 [[TMP0]], 2048
// CHECK-NEXT: [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 2048
// CHECK-NEXT: [[TMP3:%.*]] = and i1 true, [[TMP2]]
// CHECK-NEXT: br i1 [[TMP3]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
// CHECK: resolver_return:
// CHECK-NEXT: ret ptr @fmv._Msha1
// CHECK: resolver_else:
// CHECK-NEXT: ret ptr @fmv.default
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The machinery to sign these hasn't been upstreamed yet, but they'll be e.g. @fmv.default.pauth when that happens.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks useful for arm64 as well, could you reveal more details on the machinery and approach to PAC resolver?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing that's missing is this:

https://github.com/apple/llvm-project/blob/970b6231f607e211c23e15a582572e6f18f4e6b2/clang/include/clang/CodeGen/CodeGenABITypes.h#L122-L126

along with this hunk:

--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2893,7 +2893,11 @@ static void CreateMultiVersionResolverReturn(CodeGenModule &CGM,
                                              llvm::Function *FuncToReturn,
                                              bool SupportsIFunc) {
   if (SupportsIFunc) {
-    Builder.CreateRet(FuncToReturn);
+    llvm::Constant *Fn = FuncToReturn;
+    if (CGM.getContext().getTargetInfo().getTriple().isArm64e())
+      Fn = CGM.getConstantSignedPointer(
+          Fn, 0, nullptr, llvm::Constant::getNullValue(CGM.SizeTy));
+    Builder.CreateRet(Fn);
     return;
   }

This arranges for the resolver to return signed function pointers. There is no difference for arm64(non-e).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asl the check in the hunk above should be generalized for ELF, and only turned on with -fptrauth-calls. I wasn't sure how to get a QualType here though, which would be needed to get the CGPointerAuthInfo that this would make the check on. Not sure if it's reasonable to "invent" one that's close enough, e.g. for void (*fn_ptr)(void); or something.

//
//
// CHECK: Function Attrs: noinline nounwind optnone
// CHECK-LABEL: define {{[^@]+}}@fmv.default
// CHECK-SAME: () #[[ATTR1]] {
// CHECK-NEXT: entry:
// CHECK-NEXT: ret i32 0
//
//.
// CHECK: attributes #[[ATTR0]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+ls64,+neon" }
// CHECK: attributes #[[ATTR1]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fullfp16,+ls64" }
// CHECK: attributes #[[ATTR2]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fullfp16,+ls64" }
//.
// CHECK: [[META0:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}
// CHECK: [[META1:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
//.
13 changes: 9 additions & 4 deletions llvm/include/llvm/CodeGen/AsmPrinter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,16 +603,21 @@ class AsmPrinter : public MachineFunctionPass {
return nullptr;
}

virtual const MCExpr *
emitMachOIfuncLazyPointerInit(const MCSymbolRefExpr *Init) {
return Init;
}

virtual void emitMachOIFuncStubBody(Module &M, const GlobalIFunc &GI,
MCSymbol *LazyPointer) {
llvm_unreachable(
"Mach-O IFunc lowering is not yet supported on this target");
assert(false &&
"Mach-O IFunc lowering is not yet supported on this target");
}

virtual void emitMachOIFuncStubHelperBody(Module &M, const GlobalIFunc &GI,
MCSymbol *LazyPointer) {
llvm_unreachable(
"Mach-O IFunc lowering is not yet supported on this target");
assert(false &&
"Mach-O IFunc lowering is not yet supported on this target");
}

/// Emit N NOP instructions.
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2259,7 +2259,8 @@ void AsmPrinter::emitGlobalIFunc(Module &M, const GlobalIFunc &GI) {
emitAlignment(Align(DL.getPointerSize()));
OutStreamer->emitLabel(LazyPointer);
emitVisibility(LazyPointer, GI.getVisibility());
OutStreamer->emitValue(MCSymbolRefExpr::create(StubHelper, OutContext), 8);
const MCSymbolRefExpr *Init = MCSymbolRefExpr::create(StubHelper, OutContext);
OutStreamer->emitValue(emitMachOIfuncLazyPointerInit(Init), 8);

OutStreamer->switchSection(OutContext.getObjectFileInfo()->getTextSection());

Expand Down
45 changes: 45 additions & 0 deletions llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ class AArch64AsmPrinter : public AsmPrinter {
assert(STI);
return STI;
}
const MCExpr *
emitMachOIfuncLazyPointerInit(const MCSymbolRefExpr *Init) override;
void emitMachOIFuncStubBody(Module &M, const GlobalIFunc &GI,
MCSymbol *LazyPointer) override;
void emitMachOIFuncStubHelperBody(Module &M, const GlobalIFunc &GI,
Expand Down Expand Up @@ -1897,6 +1899,15 @@ void AArch64AsmPrinter::emitInstruction(const MachineInstr *MI) {
EmitToStreamer(*OutStreamer, TmpInst);
}

const MCExpr *
AArch64AsmPrinter::emitMachOIfuncLazyPointerInit(const MCSymbolRefExpr *Init) {
if (TM.getTargetTriple().isArm64e())
return AArch64AuthMCExpr::create(Init, /*Disc=*/0, AArch64PACKey::IA,
/*HasAddressDiversity=*/false, OutContext);

return Init;
}

void AArch64AsmPrinter::emitMachOIFuncStubBody(Module &M, const GlobalIFunc &GI,
MCSymbol *LazyPointer) {
// _ifunc:
Expand Down Expand Up @@ -1980,6 +1991,9 @@ void AArch64AsmPrinter::emitMachOIFuncStubHelperBody(Module &M,
// ldp fp, lr, [sp], #16
// br x16

if (TM.getTargetTriple().isArm64e())
OutStreamer->emitInstruction(MCInstBuilder(AArch64::PACIBSP), *STI);

OutStreamer->emitInstruction(MCInstBuilder(AArch64::STPXpre)
.addReg(AArch64::SP)
.addReg(AArch64::FP)
Expand Down Expand Up @@ -2085,6 +2099,37 @@ void AArch64AsmPrinter::emitMachOIFuncStubHelperBody(Module &M,
.addImm(2),
*STI);

if (TM.getTargetTriple().isArm64e()) {
// autibsp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave the tail-call checking for the general tail-call checking patches, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a PR up for that that I should tack this bit onto the end of?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not, but send me your patch and I'll tack it onto the relevant commit (and split the tail-call check emission into a helper)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ec43ccb

// eor x17, lr, lr, lsl #1
// tbz x17, #62, Lgoodsig
// brk #0xc741
// Lgoodsig:

OutStreamer->emitInstruction(MCInstBuilder(AArch64::AUTIBSP), *STI);

OutStreamer->emitInstruction(MCInstBuilder(AArch64::EORXrs)
.addReg(AArch64::X17)
.addReg(AArch64::LR)
.addReg(AArch64::LR)
.addImm(1),
*STI);

MCContext &Ctx = OutStreamer->getContext();
MCSymbol *GoodSigSym = Ctx.createTempSymbol();
const MCExpr *GoodSig = MCSymbolRefExpr::create(GoodSigSym, Ctx);
OutStreamer->emitInstruction(MCInstBuilder(AArch64::TBZX)
.addReg(AArch64::X17)
.addImm(62)
.addExpr(GoodSig),
*STI);

OutStreamer->emitInstruction(MCInstBuilder(AArch64::BRK).addImm(0xc471),
*STI);

OutStreamer->emitLabel(GoodSigSym);
}

OutStreamer->emitInstruction(MCInstBuilder(TM.getTargetTriple().isArm64e()
? AArch64::BRAAZ
: AArch64::BR)
Expand Down
21 changes: 16 additions & 5 deletions llvm/test/CodeGen/AArch64/ifunc-asm.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
; RUN: llc -mtriple=arm64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefixes=ELF
; RUN: llc -mtriple=arm64-apple-darwin %s -o - | FileCheck %s --check-prefix=MACHO
; RUN: llc -mtriple=arm64-apple-darwin %s -global-isel -o - | FileCheck %s --check-prefix=MACHO
; RUN: llc -mtriple=arm64-apple-darwin %s -o - | FileCheck %s --check-prefixes=MACHO,ARM64
; RUN: llc -mtriple=arm64-apple-darwin %s -global-isel -o - | FileCheck %s --check-prefixes=MACHO,ARM64
; RUN: llc -mtriple=arm64e-apple-darwin %s -o - | FileCheck %s --check-prefixes=MACHO,PAUTH
; RUN: llc -mtriple=arm64e-apple-darwin %s -global-isel -o - | FileCheck %s --check-prefixes=MACHO,PAUTH

define internal ptr @the_resolver() {
entry:
Expand All @@ -21,7 +23,8 @@ entry:
; MACHO: .section __DATA,__data
; MACHO-NEXT: .p2align 3, 0x0
; MACHO-NEXT: _global_ifunc.lazy_pointer:
; MACHO-NEXT: .quad _global_ifunc.stub_helper
; ARM64-NEXT: .quad _global_ifunc.stub_helper{{$}}
; PAUTH-NEXT: .quad _global_ifunc.stub_helper@AUTH(ia,0)

; MACHO: .section __TEXT,__text,regular,pure_instructions
; MACHO-NEXT: .globl _global_ifunc
Expand All @@ -30,9 +33,11 @@ entry:
; MACHO-NEXT: adrp x16, _global_ifunc.lazy_pointer@GOTPAGE
; MACHO-NEXT: ldr x16, [x16, _global_ifunc.lazy_pointer@GOTPAGEOFF]
; MACHO-NEXT: ldr x16, [x16]
; MACHO-NEXT: br x16
; ARM64-NEXT: br x16
; PAUTH-NEXT: braaz x16
; MACHO-NEXT: .p2align 2
; MACHO-NEXT: _global_ifunc.stub_helper:
; PAUTH-NEXT: pacibsp
; MACHO-NEXT: stp x29, x30, [sp, #-16]!
; MACHO-NEXT: mov x29, sp
; MACHO-NEXT: stp x1, x0, [sp, #-16]!
Expand All @@ -57,7 +62,13 @@ entry:
; MACHO-NEXT: ldp x3, x2, [sp], #16
; MACHO-NEXT: ldp x1, x0, [sp], #16
; MACHO-NEXT: ldp x29, x30, [sp], #16
; MACHO-NEXT: br x16
; PAUTH-NEXT: autibsp
; PAUTH-NEXT: eor x17, x30, x30, lsl #1
; PAUTH-NEXT: tbz x17, #62, [[GOOD_SIG:Ltmp[0-9]+]]
; PAUTH-NEXT: brk #0xc471
; PAUTH-NEXT: [[GOOD_SIG]]:
; ARM64-NEXT: br x16
; PAUTH-NEXT: braaz x16


@weak_ifunc = weak ifunc i32 (i32), ptr @the_resolver
Expand Down