[PAC][CodeGen][ELF][AArch64] Support signed GOT #105798
Conversation
kovdan01
force-pushed
the
dkovalev/pauth-signed-got-codegen-new
branch
from
49d812a to
805ab05
Compare
|
I was unable to reproduce the original buildbot failure https://lab.llvm.org/buildbot/#/builders/153/builds/5329. In that build,
Maybe the failure was somehow related to #100854 (which touched the failing test) being merged several commits earlier than #96164. The build, corresponding to that PR, also passed - see https://lab.llvm.org/buildbot/#/builders/153/builds/5325. Unfortunately, I can't now say what was the reason of the original failure. Even though the src dir and the build dir were not cleaned before build, that should not make any difference in this case. Would be glad to see everyone's thoughts on probable reasons of original build failure if you have some. As for now - I just re-submit the patch (currently treating the failure as a strange instability which was not reproduced neither on buildbot nor locally) with a couple of changes described below. |
|
Comparing to original PR #96164, this one has a couple of differences:
|
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-aarch64 Author: Daniil Kovalev (kovdan01) ChangesThis re-applies #96164 after revert in #102434. Support the following relocations and assembly operators:
If a resign is requested, like below, If FPAC bit is not set and resign is requested, a check+trap sequence similar to one used for Both SelectionDAG and GlobalISel are suppported. Tests starting with 'ptrauth-' have corresponding variants w/o this prefix. See also specification Patch is 42.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105798.diff 20 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index b8f9b58a216446..e80d094a14b7e2 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -160,6 +160,9 @@ class AArch64AsmPrinter : public AsmPrinter {
// adrp-add followed by PAC sign)
void LowerMOVaddrPAC(const MachineInstr &MI);
+ // Emit the sequence for LOADgotAUTH (load+auth pointer from signed ELF GOT)
+ void LowerLOADgotAUTH(const MachineInstr &MI);
+
/// tblgen'erated driver function for lowering simple MI->MC
/// pseudo instructions.
bool lowerPseudoInstExpansion(const MachineInstr *MI, MCInst &Inst);
@@ -2168,6 +2171,10 @@ void AArch64AsmPrinter::LowerMOVaddrPAC(const MachineInstr &MI) {
};
const bool IsGOTLoad = MI.getOpcode() == AArch64::LOADgotPAC;
+ const bool IsELFSignedGOT = MI.getParent()
+ ->getParent()
+ ->getInfo<AArch64FunctionInfo>()
+ ->hasELFSignedGOT();
MachineOperand GAOp = MI.getOperand(0);
const uint64_t KeyC = MI.getOperand(1).getImm();
assert(KeyC <= AArch64PACKey::LAST &&
@@ -2184,9 +2191,17 @@ void AArch64AsmPrinter::LowerMOVaddrPAC(const MachineInstr &MI) {
// Emit:
// target materialization:
// - via GOT:
- // adrp x16, :got:target
- // ldr x16, [x16, :got_lo12:target]
- // add offset to x16 if offset != 0
+ // - unsigned GOT:
+ // adrp x16, :got:target
+ // ldr x16, [x16, :got_lo12:target]
+ // add offset to x16 if offset != 0
+ // - ELF signed GOT:
+ // adrp x17, :got:target
+ // add x17, x17, :got_auth_lo12:target
+ // ldr x16, [x17]
+ // aut{i|d}a x16, x17
+ // check+trap sequence (if no FPAC)
+ // add offset to x16 if offset != 0
//
// - direct:
// adrp x16, target
@@ -2229,13 +2244,81 @@ void AArch64AsmPrinter::LowerMOVaddrPAC(const MachineInstr &MI) {
MCInstLowering.lowerOperand(GAMOLo, GAMCLo);
EmitAndIncrement(
- MCInstBuilder(AArch64::ADRP).addReg(AArch64::X16).addOperand(GAMCHi));
+ MCInstBuilder(AArch64::ADRP)
+ .addReg(IsGOTLoad && IsELFSignedGOT ? AArch64::X17 : AArch64::X16)
+ .addOperand(GAMCHi));
if (IsGOTLoad) {
- EmitAndIncrement(MCInstBuilder(AArch64::LDRXui)
- .addReg(AArch64::X16)
- .addReg(AArch64::X16)
- .addOperand(GAMCLo));
+ if (IsELFSignedGOT) {
+ EmitAndIncrement(MCInstBuilder(AArch64::ADDXri)
+ .addReg(AArch64::X17)
+ .addReg(AArch64::X17)
+ .addOperand(GAMCLo)
+ .addImm(0));
+
+ EmitAndIncrement(MCInstBuilder(AArch64::LDRXui)
+ .addReg(AArch64::X16)
+ .addReg(AArch64::X17)
+ .addImm(0));
+
+ assert(GAOp.isGlobal());
+ assert(GAOp.getGlobal()->getValueType() != nullptr);
+ unsigned AuthOpcode = GAOp.getGlobal()->getValueType()->isFunctionTy()
+ ? AArch64::AUTIA
+ : AArch64::AUTDA;
+
+ EmitAndIncrement(MCInstBuilder(AuthOpcode)
+ .addReg(AArch64::X16)
+ .addReg(AArch64::X16)
+ .addReg(AArch64::X17));
+
+ // The logic in the following if statement is partially taken from
+ // emitPtrauthAuthResign.
+ if (!STI->hasFPAC()) {
+ auto AuthKey = (AuthOpcode == AArch64::AUTIA ? AArch64PACKey::IA
+ : AArch64PACKey::DA);
+ unsigned XPACOpc = getXPACOpcodeForKey(AuthKey);
+ MCSymbol *SuccessSym = createTempSymbol("auth_success_");
+
+ // XPAC has tied src/dst: use x17 as a temporary copy.
+ // mov x17, x16
+ EmitAndIncrement(MCInstBuilder(AArch64::ORRXrs)
+ .addReg(AArch64::X17)
+ .addReg(AArch64::XZR)
+ .addReg(AArch64::X16)
+ .addImm(0));
+
+ // xpaci x17
+ EmitAndIncrement(
+ MCInstBuilder(XPACOpc).addReg(AArch64::X17).addReg(AArch64::X17));
+
+ // cmp x16, x17
+ EmitAndIncrement(MCInstBuilder(AArch64::SUBSXrs)
+ .addReg(AArch64::XZR)
+ .addReg(AArch64::X16)
+ .addReg(AArch64::X17)
+ .addImm(0));
+
+ // b.eq Lsuccess
+ EmitAndIncrement(
+ MCInstBuilder(AArch64::Bcc)
+ .addImm(AArch64CC::EQ)
+ .addExpr(MCSymbolRefExpr::create(SuccessSym, OutContext)));
+
+ // Trapping sequences do a 'brk'.
+ // brk #<0xc470 + aut key>
+ EmitAndIncrement(MCInstBuilder(AArch64::BRK).addImm(0xc470 | AuthKey));
+
+ // If the auth check succeeds, we can continue.
+ // Lsuccess:
+ OutStreamer->emitLabel(SuccessSym);
+ }
+ } else {
+ EmitAndIncrement(MCInstBuilder(AArch64::LDRXui)
+ .addReg(AArch64::X16)
+ .addReg(AArch64::X16)
+ .addOperand(GAMCLo));
+ }
} else {
EmitAndIncrement(MCInstBuilder(AArch64::ADDXri)
.addReg(AArch64::X16)
@@ -2320,6 +2403,53 @@ void AArch64AsmPrinter::LowerMOVaddrPAC(const MachineInstr &MI) {
assert(STI->getInstrInfo()->getInstSizeInBytes(MI) >= InstsEmitted * 4);
}
+void AArch64AsmPrinter::LowerLOADgotAUTH(const MachineInstr &MI) {
+ unsigned InstsEmitted = 0;
+ auto EmitAndIncrement = [this, &InstsEmitted](const MCInst &Inst) {
+ EmitToStreamer(*OutStreamer, Inst);
+ ++InstsEmitted;
+ };
+
+ Register DstReg = MI.getOperand(0).getReg();
+ const MachineOperand &GAMO = MI.getOperand(1);
+ assert(GAMO.getOffset() == 0);
+
+ MachineOperand GAHiOp(GAMO);
+ MachineOperand GALoOp(GAMO);
+ GAHiOp.addTargetFlag(AArch64II::MO_PAGE);
+ GALoOp.addTargetFlag(AArch64II::MO_PAGEOFF | AArch64II::MO_NC);
+
+ MCOperand GAMCHi, GAMCLo;
+ MCInstLowering.lowerOperand(GAHiOp, GAMCHi);
+ MCInstLowering.lowerOperand(GALoOp, GAMCLo);
+
+ EmitAndIncrement(
+ MCInstBuilder(AArch64::ADRP).addReg(AArch64::X16).addOperand(GAMCHi));
+
+ EmitAndIncrement(MCInstBuilder(AArch64::ADDXri)
+ .addReg(AArch64::X16)
+ .addReg(AArch64::X16)
+ .addOperand(GAMCLo)
+ .addImm(0));
+
+ EmitAndIncrement(MCInstBuilder(AArch64::LDRXui)
+ .addReg(DstReg)
+ .addReg(AArch64::X16)
+ .addImm(0));
+
+ assert(GAMO.isGlobal());
+ assert(GAMO.getGlobal()->getValueType() != nullptr);
+ unsigned AuthOpcode = GAMO.getGlobal()->getValueType()->isFunctionTy()
+ ? AArch64::AUTIA
+ : AArch64::AUTDA;
+ EmitAndIncrement(MCInstBuilder(AuthOpcode)
+ .addReg(DstReg)
+ .addReg(DstReg)
+ .addReg(AArch64::X16));
+
+ assert(STI->getInstrInfo()->getInstSizeInBytes(MI) >= InstsEmitted * 4);
+}
+
const MCExpr *
AArch64AsmPrinter::lowerBlockAddressConstant(const BlockAddress &BA) {
const MCExpr *BAE = AsmPrinter::lowerBlockAddressConstant(BA);
@@ -2484,6 +2614,10 @@ void AArch64AsmPrinter::emitInstruction(const MachineInstr *MI) {
LowerMOVaddrPAC(*MI);
return;
+ case AArch64::LOADgotAUTH:
+ LowerLOADgotAUTH(*MI);
+ return;
+
case AArch64::BRA:
case AArch64::BLRA:
emitPtrauthBranch(MI);
diff --git a/llvm/lib/Target/AArch64/AArch64FastISel.cpp b/llvm/lib/Target/AArch64/AArch64FastISel.cpp
index cbf38f2c57a35e..4487d34a936c4d 100644
--- a/llvm/lib/Target/AArch64/AArch64FastISel.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FastISel.cpp
@@ -453,6 +453,9 @@ unsigned AArch64FastISel::materializeGV(const GlobalValue *GV) {
if (!Subtarget->useSmallAddressing() && !Subtarget->isTargetMachO())
return 0;
+ if (FuncInfo.MF->getInfo<AArch64FunctionInfo>()->hasELFSignedGOT())
+ return 0;
+
unsigned OpFlags = Subtarget->ClassifyGlobalReference(GV, TM);
EVT DestEVT = TLI.getValueType(DL, GV->getType(), true);
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 8c2f85657ff87e..67cdb1a26680e0 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -9357,6 +9357,11 @@ SDValue AArch64TargetLowering::getGOT(NodeTy *N, SelectionDAG &DAG,
SDValue GotAddr = getTargetNode(N, Ty, DAG, AArch64II::MO_GOT | Flags);
// FIXME: Once remat is capable of dealing with instructions with register
// operands, expand this into two nodes instead of using a wrapper node.
+ if (DAG.getMachineFunction()
+ .getInfo<AArch64FunctionInfo>()
+ ->hasELFSignedGOT())
+ return SDValue(DAG.getMachineNode(AArch64::LOADgotAUTH, DL, Ty, GotAddr),
+ 0);
return DAG.getNode(AArch64ISD::LOADgot, DL, Ty, GotAddr);
}
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 2fff6fffcd7c6d..131785ec921aa1 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -1885,8 +1885,9 @@ let Predicates = [HasPAuth] in {
Sched<[WriteI, ReadI]> {
let isReMaterializable = 1;
let isCodeGenOnly = 1;
- let Size = 40; // 12 fixed + 28 variable, for pointer offset, and discriminator
- let Defs = [X16,X17];
+ let Size = 68; // 12 fixed + 56 variable, for pointer offset, discriminator and
+ // ELF signed GOT signed pointer authentication (if no FPAC)
+ let Defs = [X16,X17,NZCV];
}
// Load a signed global address from a special $auth_ptr$ stub slot.
@@ -1924,6 +1925,12 @@ let Predicates = [HasPAuth] in {
tcGPR64:$AddrDisc),
(AUTH_TCRETURN_BTI tcGPRx16x17:$dst, imm:$FPDiff, imm:$Key,
imm:$Disc, tcGPR64:$AddrDisc)>;
+
+ def LOADgotAUTH : Pseudo<(outs GPR64common:$dst), (ins i64imm:$addr), []>,
+ Sched<[WriteI, ReadI]> {
+ let Defs = [X16];
+ let Size = 16;
+ }
}
// v9.5-A pointer authentication extensions
diff --git a/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp b/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp
index 48672241f905d5..9f234b0f917058 100644
--- a/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp
+++ b/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "AArch64MCInstLower.h"
+#include "AArch64MachineFunctionInfo.h"
#include "MCTargetDesc/AArch64MCExpr.h"
#include "Utils/AArch64BaseInfo.h"
#include "llvm/CodeGen/AsmPrinter.h"
@@ -185,9 +186,12 @@ MCOperand AArch64MCInstLower::lowerSymbolOperandELF(const MachineOperand &MO,
MCSymbol *Sym) const {
uint32_t RefFlags = 0;
- if (MO.getTargetFlags() & AArch64II::MO_GOT)
- RefFlags |= AArch64MCExpr::VK_GOT;
- else if (MO.getTargetFlags() & AArch64II::MO_TLS) {
+ if (MO.getTargetFlags() & AArch64II::MO_GOT) {
+ const MachineFunction *MF = MO.getParent()->getParent()->getParent();
+ RefFlags |= (MF->getInfo<AArch64FunctionInfo>()->hasELFSignedGOT()
+ ? AArch64MCExpr::VK_GOT_AUTH
+ : AArch64MCExpr::VK_GOT);
+ } else if (MO.getTargetFlags() & AArch64II::MO_TLS) {
TLSModel::Model Model;
if (MO.isGlobal()) {
const GlobalValue *GV = MO.getGlobal();
diff --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp
index e96c5a953ff2bb..a0f0a489816c41 100644
--- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp
@@ -16,6 +16,7 @@
#include "AArch64MachineFunctionInfo.h"
#include "AArch64InstrInfo.h"
#include "AArch64Subtarget.h"
+#include "llvm/BinaryFormat/ELF.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
@@ -72,6 +73,29 @@ static bool ShouldSignWithBKey(const Function &F, const AArch64Subtarget &STI) {
return Key == "b_key";
}
+// Determine if we need to treat pointers in GOT as signed (as described in
+// https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#appendix-signed-got)
+// based on PAuth core info encoded as "aarch64-elf-pauthabi-platform" and
+// "aarch64-elf-pauthabi-version" module flags. Currently, only
+// AARCH64_PAUTH_PLATFORM_LLVM_LINUX platform supports signed GOT with
+// AARCH64_PAUTH_PLATFORM_LLVM_LINUX_VERSION_GOT bit in version value set.
+static bool hasELFSignedGOTHelper(const Function &F,
+ const AArch64Subtarget *STI) {
+ if (!Triple(STI->getTargetTriple()).isOSBinFormatELF())
+ return false;
+ const Module *M = F.getParent();
+ const auto *PAP = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag("aarch64-elf-pauthabi-platform"));
+ if (!PAP || PAP->getZExtValue() != ELF::AARCH64_PAUTH_PLATFORM_LLVM_LINUX)
+ return false;
+ const auto *PAV = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag("aarch64-elf-pauthabi-version"));
+ if (!PAV)
+ return false;
+ return PAV->getZExtValue() &
+ (1 << ELF::AARCH64_PAUTH_PLATFORM_LLVM_LINUX_VERSION_GOT);
+}
+
AArch64FunctionInfo::AArch64FunctionInfo(const Function &F,
const AArch64Subtarget *STI) {
// If we already know that the function doesn't have a redzone, set
@@ -80,6 +104,7 @@ AArch64FunctionInfo::AArch64FunctionInfo(const Function &F,
HasRedZone = false;
std::tie(SignReturnAddress, SignReturnAddressAll) = GetSignReturnAddress(F);
SignWithBKey = ShouldSignWithBKey(F, *STI);
+ HasELFSignedGOT = hasELFSignedGOTHelper(F, STI);
// TODO: skip functions that have no instrumented allocas for optimization
IsMTETagged = F.hasFnAttribute(Attribute::SanitizeMemTag);
diff --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
index 72f110cebbdc8f..9ae45848834377 100644
--- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
@@ -177,6 +177,14 @@ class AArch64FunctionInfo final : public MachineFunctionInfo {
/// SignWithBKey modifies the default PAC-RET mode to signing with the B key.
bool SignWithBKey = false;
+ /// HasELFSignedGOT is true if the target binary format is ELF and the IR
+ /// module containing the corresponding function has the following flags:
+ /// - aarch64-elf-pauthabi-platform flag equal to
+ /// AARCH64_PAUTH_PLATFORM_LLVM_LINUX;
+ /// - aarch64-elf-pauthabi-version flag with
+ /// AARCH64_PAUTH_PLATFORM_LLVM_LINUX_VERSION_GOT bit set.
+ bool HasELFSignedGOT = false;
+
/// SigningInstrOffset captures the offset of the PAC-RET signing instruction
/// within the prologue, so it can be re-used for authentication in the
/// epilogue when using PC as a second salt (FEAT_PAuth_LR)
@@ -509,6 +517,8 @@ class AArch64FunctionInfo final : public MachineFunctionInfo {
bool shouldSignWithBKey() const { return SignWithBKey; }
+ bool hasELFSignedGOT() const { return HasELFSignedGOT; }
+
MCSymbol *getSigningInstrLabel() const { return SignInstrLabel; }
void setSigningInstrLabel(MCSymbol *Label) { SignInstrLabel = Label; }
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 1c5909c64bccd3..6aa510f4fe6aa2 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -875,6 +875,7 @@ class AArch64Operand : public MCParsedAsmOperand {
if (DarwinRefKind == MCSymbolRefExpr::VK_PAGEOFF ||
ELFRefKind == AArch64MCExpr::VK_LO12 ||
ELFRefKind == AArch64MCExpr::VK_GOT_LO12 ||
+ ELFRefKind == AArch64MCExpr::VK_GOT_AUTH_LO12 ||
ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12 ||
ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12_NC ||
ELFRefKind == AArch64MCExpr::VK_TPREL_LO12 ||
@@ -986,19 +987,20 @@ class AArch64Operand : public MCParsedAsmOperand {
int64_t Addend;
if (AArch64AsmParser::classifySymbolRef(Expr, ELFRefKind,
DarwinRefKind, Addend)) {
- return DarwinRefKind == MCSymbolRefExpr::VK_PAGEOFF
- || DarwinRefKind == MCSymbolRefExpr::VK_TLVPPAGEOFF
- || (DarwinRefKind == MCSymbolRefExpr::VK_GOTPAGEOFF && Addend == 0)
- || ELFRefKind == AArch64MCExpr::VK_LO12
- || ELFRefKind == AArch64MCExpr::VK_DTPREL_HI12
- || ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12
- || ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12_NC
- || ELFRefKind == AArch64MCExpr::VK_TPREL_HI12
- || ELFRefKind == AArch64MCExpr::VK_TPREL_LO12
- || ELFRefKind == AArch64MCExpr::VK_TPREL_LO12_NC
- || ELFRefKind == AArch64MCExpr::VK_TLSDESC_LO12
- || ELFRefKind == AArch64MCExpr::VK_SECREL_HI12
- || ELFRefKind == AArch64MCExpr::VK_SECREL_LO12;
+ return DarwinRefKind == MCSymbolRefExpr::VK_PAGEOFF ||
+ DarwinRefKind == MCSymbolRefExpr::VK_TLVPPAGEOFF ||
+ (DarwinRefKind == MCSymbolRefExpr::VK_GOTPAGEOFF && Addend == 0) ||
+ ELFRefKind == AArch64MCExpr::VK_LO12 ||
+ ELFRefKind == AArch64MCExpr::VK_GOT_AUTH_LO12 ||
+ ELFRefKind == AArch64MCExpr::VK_DTPREL_HI12 ||
+ ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12 ||
+ ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12_NC ||
+ ELFRefKind == AArch64MCExpr::VK_TPREL_HI12 ||
+ ELFRefKind == AArch64MCExpr::VK_TPREL_LO12 ||
+ ELFRefKind == AArch64MCExpr::VK_TPREL_LO12_NC ||
+ ELFRefKind == AArch64MCExpr::VK_TLSDESC_LO12 ||
+ ELFRefKind == AArch64MCExpr::VK_SECREL_HI12 ||
+ ELFRefKind == AArch64MCExpr::VK_SECREL_LO12;
}
// If it's a constant, it should be a real immediate in range.
@@ -3250,6 +3252,7 @@ ParseStatus AArch64AsmParser::tryParseAdrpLabel(OperandVector &Operands) {
DarwinRefKind != MCSymbolRefExpr::VK_TLVPPAGE &&
ELFRefKind != AArch64MCExpr::VK_ABS_PAGE_NC &&
ELFRefKind != AArch64MCExpr::VK_GOT_PAGE &&
+ ELFRefKind != AArch64MCExpr::VK_GOT_AUTH_PAGE &&
ELFRefKind != AArch64MCExpr::VK_GOT_PAGE_LO15 &&
ELFRefKind != AArch64MCExpr::VK_GOTTPREL_PAGE &&
ELFRefKind != AArch64MCExpr::VK_TLSDESC_PAGE) {
@@ -4335,6 +4338,8 @@ bool AArch64AsmParser::parseSymbolicImmVal(const MCExpr *&ImmVal) {
.Case("got", AArch64MCExpr::VK_GOT_PAGE)
.Case("gotpage_lo15", AArch64MCExpr::VK_GOT_PAGE_LO15)
.Case("got_lo12", AArch64MCExpr::VK_GOT_LO12)
+ .Case("got_auth", AArch64MCExpr::VK_GOT_AUTH_PAGE)
+ .Case("got_auth_lo12", AArch64MCExpr::VK_GOT_AUTH_LO12)
.Case("gottprel", AArch64MCExpr::VK_GOTTPREL_PAGE)
.Case("gottprel_lo12", AArch64MCExpr::VK_GOTTPREL_LO12_NC)
.Case("gottprel_g1", AArch64MCExpr::VK_GOTTPREL_G1)
@@ -5709,6 +5714,7 @@ bool AArch64AsmParser::validateInstruction(MCInst &Inst, SMLoc &IDLoc,
// Only allow these with ADDXri/ADDWri
if ((ELFRefKind == AArch64MCExpr::VK_LO12 ||
+ ELFRefKind == AArch64MCExpr::VK_GOT_AUTH_LO12 ||
ELFRefKind == AArch64MCExpr::VK_DTPREL_HI12 ||
ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12 ||
ELFRefKind == AArch64MCExpr::VK_DTPREL_LO12_NC ||
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index e9e6b6cb68d0d1..ef8fcc07417013 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/...
[truncated]
|
kovdan01
force-pushed
the
dkovalev/pauth-signed-got-codegen-new
branch
from
805ab05 to
d2b7ddc
Compare
One new difference proposed with latest PR update - do not rely on bitmask pauthabi version value to determine if codegen needs to use signed GOT. It's specific to llvm_linux platform. Instead, check if module flag "ptrauth-elf-got" is present and equal to 1. |
|
@MaskRay @tmatheson-arm @smithp35 Would be glad to see your feedback on this |
kovdan01
force-pushed
the
dkovalev/pauth-signed-got-codegen-new
branch
from
d2b7ddc to
1fcc7ff
Compare
|
The latest PR update fixes the following issue. With signed ELF GOT enabled, the linker looks at the symbol type to choose between keys IA (for STT_FUNC) and DA (for other types). Symbols for functions not defined in the module have STT_NOTYPE type by default. This makes linker to emit signing schema with DA key (instead of IA) for corresponding R_AARCH64_AUTH_GLOB_DAT dynamic reloc. To avoid that, force all function symbols used in the module to have STT_FUNC type. See https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#default-signing-schema @MaskRay could you please clarify if this is OK from linker's perspective? Does lld rely on the fact that undefined functions are STT_NOTYPE or is this OK to have them STT_FUNC? I've ran test-suite built with pauth enabled (including signed GOT), and nothing seems to break, but maybe there are some non-obvious issues which might appear under some conditions. Would be glad to see everyone's feedback on this @tmatheson-arm @smithp35 |
| M.getModuleFlag("ptrauth-elf-got")); | ||
| if (PtrAuthELFGOTFlag && PtrAuthELFGOTFlag->getZExtValue() == 1) | ||
| for (const GlobalValue &GV : M.global_values()) | ||
| if (!GV.use_empty() && GV.getValueType()->isFunctionTy() && |
There was a problem hiding this comment.
Can it be simplified down to isa<Function>(GV)? Also, what happens with aliases?
There was a problem hiding this comment.
Can it be simplified down to
isa<Function>(GV)?
Yes, it can, thanks for suggestion! Fixed in cc9681063f645a808fec804253c7f0e598377b98
Also, what happens with aliases?
Aliases must point to a definition (if I don't miss smth). Symbols for functions with definitions will anyway be marked as STT_FUNC. The point of the loop is to mark all function symbols which only have declarations as STT_FUNC. Yes, the loop also covers defined functions (not function aliases), but there is no issue in "double-marking" them - it'll not result in duplicate annotation in assembly. A test for alias to a defined function is present in llvm/test/CodeGen/AArch64/ptrauth-got-abuse.ll.
Closest I can find in terms of use of STT_NOTYPE is this thread in the GABI https://groups.google.com/g/generic-abi/c/_ZPPq_c8FSQ which to my reading suggests that emitting the type on the reference could be a good thing as it would permit static linker's to diagnose mismatches in symbol types such as STT_FUNC and STT_OBJECT. However I think the discussion stopped short of requiring it. It doesn't however forbid it. There's a bug report in GCC advocating it, with a patch that doesn't seem to have been applied https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35514. From what I can see, it looks like the main advantage and disadvantage of setting the type on the symbol reference is that the static linker has to do some extra work to make sure that a combination of two references to sym one with STT_NOTYPE and one with any other type preserves the type. It also has to decide what to do if there is a mismatch of types on the reference and definition. It could ignore the reference, or could warn. The most likely alternative is to provide alternative relocation types for code and non-code references. This is essentially moving the symbol type into the relocation code though. |
@smithp35 Thanks for your feedback! Yes, providing alternative relocation types is a viable solution for this. But, as far as I see, the current way it's resolved now (by unconditionally setting @MaskRay Could you please give your feedback on the problem? Would be glad to hear which solution looks better from linker's perspective. |
This re-applies llvm#96164 after revert in llvm#102434. Support the following relocations and assembly operators: - `R_AARCH64_AUTH_ADR_GOT_PAGE` (`:got_auth:` for `adrp`) - `R_AARCH64_AUTH_LD64_GOT_LO12_NC` (`:got_auth_lo12:` for `ldr`) - `R_AARCH64_AUTH_GOT_ADD_LO12_NC` (`:got_auth_lo12:` for `add`) `LOADgotAUTH` pseudo-instruction is introduced which is later expanded to actual instruction sequence like the following. ``` adrp x16, :got_auth:sym add x16, x16, :got_auth_lo12:sym ldr x0, [x16] autia x0, x16 ``` If a resign is requested, like below, `LOADgotPAC` pseudo is used, and GOT load is lowered similarly to `LOADgotAUTH`. ``` @var = global i32 0 define ptr @resign_globalvar() { ret ptr ptrauth (ptr @var, i32 3, i64 43) } ``` If FPAC bit is not set and resign is requested, a check+trap sequence similar to one used for `AUT` pseudo is emitted. Both SelectionDAG and GlobalISel are suppported. For FastISel, we fall back to SelectionDAG. Tests starting with 'ptrauth-' have corresponding variants w/o this prefix. See also specification https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#appendix-signed-got
Before executing auth instruction on a signed address from a GOT slot, we must ensure that the address is not zero since zero addresses are not signed to preserve zero-checks working as expected. A GOT slot might be filled with zero when we have an undefined weak symbol.
kovdan01
force-pushed
the
dkovalev/pauth-signed-got-codegen-new
branch
from
cc96810 to
61bae48
Compare
|
Comments regarding latest update:
|
|
@MaskRay @tmatheson-arm @smithp35 Would be glad to see your feedback on this. @MaskRay Earlier, there was a question regarding setting |
|
I've re-opened this as #113811 since I had to change the source branch to allow stacked PR (codegen support for signed GOT with tiny code model #113812 and for signed TLSDESC #113813 depend on this). The new PR is identical to this one, and the commit history is preserved. Would be glad to continue discussion in #113811 - closing this one in favor of that. |
This re-applies #96164 after revert in #102434.
Support the following relocations and assembly operators:
R_AARCH64_AUTH_ADR_GOT_PAGE(:got_auth:foradrp)R_AARCH64_AUTH_LD64_GOT_LO12_NC(:got_auth_lo12:forldr)R_AARCH64_AUTH_GOT_ADD_LO12_NC(:got_auth_lo12:foradd)LOADgotAUTHpseudo-instruction is introduced which is later expanded to actual instruction sequence like the following.If a resign is requested, like below,
LOADgotPACpseudo is used, and GOT load is lowered similarly toLOADgotAUTH.If FPAC bit is not set and resign is requested, a check+trap sequence similar to one used for
AUTpseudo is emitted.Both SelectionDAG and GlobalISel are suppported.
For FastISel, we fall back to SelectionDAG.
Tests starting with 'ptrauth-' have corresponding variants w/o this prefix.
See also specification
https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#appendix-signed-got