[AArch64][PAC] Simplify emission of authenticated pointer check (NFC) #160899
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue method accepts
two arguments, `bool ShouldTrap` and `const MCSymbol *OnFailure`, that
control the behavior of the emitted instruction sequence when the check
fails:
* `ShouldTrap` requests an error to be generated
* `OnFailure` requests branching to the given label after clearing the
PAC field
An assertion in `emitPtrauthCheckAuthenticatedValue` ensures that
`OnFailure` must be null when `ShouldTrap` is set. But the opposite is
also true: when `ShouldTrap` is false, `OnFailure` is always non-null,
as otherwise the entire sequence following `AUT[ID][AB]` instruction
would turn into a very expensive equivalent of XPAC (unless the CPU
implements FEAT_FPAC):
authenticate Xn
inspect PAC field of Xn
if PAC field was not cleared:
clear PAC field
The only caller that makes use of checking-but-not-trapping mode of
emitPtrauthCheckAuthenticatedValue is emitPtrauthAuthResign, and it
passes a non-null pointer as OnFailure when ShouldTrap is false.
This commit makes the invariant explicit by omitting the ShouldTrap
argument and inferring its value from the OnFailure argument instead.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Sep 26, 2025
atrosinenko
requested review from
ahmedbougacha,
asl,
kbeyls,
kovdan01 and
pcc
|
@llvm/pr-subscribers-backend-aarch64 Author: Anatoly Trosinenko (atrosinenko) ChangesThe AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue method accepts
An assertion in The only caller that makes use of checking-but-not-trapping mode of This commit makes the invariant explicit by omitting the ShouldTrap Full diff: https://github.com/llvm/llvm-project/pull/160899.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index c31a090bba77f..1a2808f4d56d8 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -162,8 +162,7 @@ class AArch64AsmPrinter : public AsmPrinter {
Register ScratchReg,
AArch64PACKey::ID Key,
AArch64PAuth::AuthCheckMethod Method,
- bool ShouldTrap,
- const MCSymbol *OnFailure);
+ const MCSymbol *OnFailure = nullptr);
// Check authenticated LR before tail calling.
void emitPtrauthTailCallHardening(const MachineInstr *TC);
@@ -1939,14 +1938,19 @@ Register AArch64AsmPrinter::emitPtrauthDiscriminator(uint16_t Disc,
return ScratchReg;
}
-/// Emits a code sequence to check an authenticated pointer value.
+/// Emit a code sequence to check an authenticated pointer value.
///
-/// If OnFailure argument is passed, jump there on check failure instead
-/// of proceeding to the next instruction (only if ShouldTrap is false).
+/// This function emits a sequence of instructions that checks if TestedReg was
+/// authenticated successfully. On success, execution continues at the next
+/// instruction after the sequence.
+///
+/// The action performed on failure depends on the OnFailure argument:
+/// * if OnFailure is not nullptr, control is transferred to that label after
+/// clearing the PAC field
+/// * otherwise, BRK instruction is emitted to generate an error
void AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue(
Register TestedReg, Register ScratchReg, AArch64PACKey::ID Key,
- AArch64PAuth::AuthCheckMethod Method, bool ShouldTrap,
- const MCSymbol *OnFailure) {
+ AArch64PAuth::AuthCheckMethod Method, const MCSymbol *OnFailure) {
// Insert a sequence to check if authentication of TestedReg succeeded,
// such as:
//
@@ -1983,7 +1987,7 @@ void AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue(
.addReg(getWRegFromXReg(ScratchReg))
.addReg(TestedReg)
.addImm(0));
- assert(ShouldTrap && !OnFailure && "DummyLoad always traps on error");
+ assert(!OnFailure && "DummyLoad always traps on error");
return;
}
@@ -2037,15 +2041,14 @@ void AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue(
llvm_unreachable("Unsupported check method");
}
- if (ShouldTrap) {
- assert(!OnFailure && "Cannot specify OnFailure with ShouldTrap");
+ if (!OnFailure) {
// Trapping sequences do a 'brk'.
// brk #<0xc470 + aut key>
EmitToStreamer(MCInstBuilder(AArch64::BRK).addImm(0xc470 | Key));
} else {
// Non-trapping checked sequences return the stripped result in TestedReg,
- // skipping over success-only code (such as re-signing the pointer) if
- // there is one.
+ // skipping over success-only code (such as re-signing the pointer) by
+ // jumping to OnFailure label.
// Note that this can introduce an authentication oracle (such as based on
// the high bits of the re-signed value).
@@ -2070,12 +2073,9 @@ void AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue(
MCInstBuilder(XPACOpc).addReg(TestedReg).addReg(TestedReg));
}
- if (OnFailure) {
- // b Lend
- EmitToStreamer(
- MCInstBuilder(AArch64::B)
- .addExpr(MCSymbolRefExpr::create(OnFailure, OutContext)));
- }
+ // b Lend
+ const auto *OnFailureExpr = MCSymbolRefExpr::create(OnFailure, OutContext);
+ EmitToStreamer(MCInstBuilder(AArch64::B).addExpr(OnFailureExpr));
}
// If the auth check succeeds, we can continue.
@@ -2102,9 +2102,8 @@ void AArch64AsmPrinter::emitPtrauthTailCallHardening(const MachineInstr *TC) {
"Neither x16 nor x17 is available as a scratch register");
AArch64PACKey::ID Key =
AArch64FI->shouldSignWithBKey() ? AArch64PACKey::IB : AArch64PACKey::IA;
- emitPtrauthCheckAuthenticatedValue(
- AArch64::LR, ScratchReg, Key, LRCheckMethod,
- /*ShouldTrap=*/true, /*OnFailure=*/nullptr);
+ emitPtrauthCheckAuthenticatedValue(AArch64::LR, ScratchReg, Key,
+ LRCheckMethod);
}
void AArch64AsmPrinter::emitPtrauthAuthResign(
@@ -2178,9 +2177,8 @@ void AArch64AsmPrinter::emitPtrauthAuthResign(
if (IsAUTPAC && !ShouldTrap)
EndSym = createTempSymbol("resign_end_");
- emitPtrauthCheckAuthenticatedValue(AUTVal, Scratch, AUTKey,
- AArch64PAuth::AuthCheckMethod::XPAC,
- ShouldTrap, EndSym);
+ emitPtrauthCheckAuthenticatedValue(
+ AUTVal, Scratch, AUTKey, AArch64PAuth::AuthCheckMethod::XPAC, EndSym);
}
// We already emitted unchecked and checked-but-non-trapping AUTs.
@@ -2519,9 +2517,7 @@ void AArch64AsmPrinter::LowerMOVaddrPAC(const MachineInstr &MI) {
: AArch64PACKey::DA);
emitPtrauthCheckAuthenticatedValue(AArch64::X16, AArch64::X17, AuthKey,
- AArch64PAuth::AuthCheckMethod::XPAC,
- /*ShouldTrap=*/true,
- /*OnFailure=*/nullptr);
+ AArch64PAuth::AuthCheckMethod::XPAC);
}
} else {
EmitToStreamer(MCInstBuilder(AArch64::LDRXui)
@@ -2654,9 +2650,7 @@ void AArch64AsmPrinter::LowerLOADgotAUTH(const MachineInstr &MI) {
(AuthOpcode == AArch64::AUTIA ? AArch64PACKey::IA : AArch64PACKey::DA);
emitPtrauthCheckAuthenticatedValue(AuthResultReg, AArch64::X17, AuthKey,
- AArch64PAuth::AuthCheckMethod::XPAC,
- /*ShouldTrap=*/true,
- /*OnFailure=*/nullptr);
+ AArch64PAuth::AuthCheckMethod::XPAC);
emitMovXReg(DstReg, AuthResultReg);
}
|
|
Ping. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue method accepts
two arguments,
bool ShouldTrapandconst MCSymbol *OnFailure, thatcontrol the behavior of the emitted instruction sequence when the check
fails:
ShouldTraprequests an error to be generatedOnFailurerequests branching to the given label after clearing thePAC field
An assertion in
emitPtrauthCheckAuthenticatedValueensures thatOnFailuremust be null whenShouldTrapis set. But the opposite isalso true: when
ShouldTrapis false,OnFailureis always non-null,as otherwise the entire sequence following
AUT[ID][AB]instructionwould turn into a very expensive equivalent of XPAC (unless the CPU
implements FEAT_FPAC):
The only caller that makes use of checking-but-not-trapping mode of
emitPtrauthCheckAuthenticatedValue is emitPtrauthAuthResign, and it
passes a non-null pointer as OnFailure when ShouldTrap is false.
This commit makes the invariant explicit by omitting the ShouldTrap
argument and inferring its value from the OnFailure argument instead.