================
@@ -1537,11 +1570,16 @@ static void CollectARMPACBTIOptions(const ToolChain
&TC, const ArgList &Args,
if (!isAArch64 && PBP.Key == "b_key")
D.Diag(diag::warn_unsupported_branch_protection)
<< "b-key" << A->getAsString(Args);
+ if (!isAArch64 && PBP.HasPauthABI)
+ D.Diag(diag::warn_unsupported_branch_protection)
+ << "pauthabi" << A->getAsString(Args);
Scope = PBP.Scope;
Key = PBP.Key;
BranchProtectionPAuthLR = PBP.BranchProtectionPAuthLR;
IndirectBranches = PBP.BranchTargetEnforcement;
GuardedControlStack = PBP.GuardedControlStack;
+ if (isAArch64 && PBP.HasPauthABI)
----------------
kovdan01 wrote:
I suggest not to support `pauthabi` in combination with other branch protection
options as for now. Here are the reasons why.
1. `pac-ret`: this and `-fptrauth-returns` (which is enabled by
`-mbranch-protection=pauthabi`) are intended to do similar stuff, but the
implementation differs.
For `-mbranch-protection=pac-ret`:
- `sign-return-address` llvm module flag is set (and optionally
`sign-return-address-all` with `+leaf`);
- `PAUTH_PROLOGUE` and `PAUTH_EPILOGUE` pseudo-instructions are emitted in
`AArch64FrameLowering::emitPrologue` and `AArch64FrameLowering::emitEpilogue`;
these pseudos are later expanded by `AArch64PointerAuth` pass;
- both A and B keys can be used (depending on `+b-key` and corresponding
function attribute `sign-return-address-key` or module flag
`sign-return-address-with-bkey`);
- using `pc` register as additional modifier is supported with `+pc`
(corresponding module flag is `branch-protection-pauth-lr`).
For `-fptrauth-returns` (I'll talk about downstream Apple implementation
since many parts are not upstreamed yet, see, for example,
https://github.com/apple/llvm-project/commit/13f9944a4c8993f9af32dc634e5d7a08cf0394e7):
- `ptrauth-returns` attribute is set on functions we want this to be enabled;
- actual codegen logic is implemented in
`AArch64FrameLowering::emitPrologue` and `AArch64FrameLowering::emitEpilogue` -
we emit actual instructions like `pacibsp` directly there;
- B key is always used;
- using `pc` register as additional modifier is **not** supported.
If we try to enable both by `-mbranch-protection=pauthabi+pac-ret`, it'll
result in incorrect code with duplicating sign/auth instructions. For example,
for this:
```
int a() {
return b() + c();
}
```
We get this:
```
a:
paciasp
pacibsp
stp x29, x30, [sp, #-32]! // 16-byte Folded Spill
str x19, [sp, #16] // 8-byte Folded Spill
mov x29, sp
bl b
mov w19, w0
bl c
add w0, w0, w19
ldr x19, [sp, #16] // 8-byte Folded Reload
ldp x29, x30, [sp], #32 // 16-byte Folded Reload
autiasp
retab
```
A corresponding issue was already previously opened (mistakenly in mainline
llvm repo while it was and actually still is an issue specific for Apple
downstream). Links:
- the issue in mainline repo
https://github.com/llvm/llvm-project/issues/60239;
- thread on Apple forum regarding the issue
https://forums.developer.apple.com/forums/thread/724568;
- the issue on Apple feedback portal (I was unable to open that actually but
the link should be correct)
https://feedbackassistant.apple.com/feedback/1196543.
I'll probably re-open the issue in mainline repo when codegen support for
`ptrauth-returns` is upstreamed. Alternatively, the Apple's downstream
implementation for return address signing might be dropped since `pac-ret`
seems to be more complete, and we can use `-fptrauth-returns` for setting the
same return address signing options as `pac-ret+b-key`. Tagging @ahmedbougacha.
2. `+leaf` and `+pc`: these are only allowed with `pac-ret`, and while it's not
clear how we'll resolve collisions between `pac-ret` and `ptrauth-returns`
(which is part of `pauthabi`), it's probably better to just disallow
`pauthabi+leaf` and `pauthabi+pc`.
3. `+b-key`: `ptrauth-returns` (which is part of `pauthabi`) already uses B key
by default (but the codegen support is still present only in Apple downstream,
see
https://github.com/apple/llvm-project/commit/13f9944a4c8993f9af32dc634e5d7a08cf0394e7)
4. `gcs`: as far as I understand, guarded control stack is smth like what is
usually called shadow stack. I'm not sure how it's supposed to work with return
address signing - probably, these shouldn't be used together, so, since
`pauthabi` implies return address signing, disallow `pauthabi+gcs`.
5. `bti`: depending on operand value (`c`, `j` or `jc`), the `bti` instruction
inserted at beginning of valid call/jump targets checks that `PSTATE.BTYPE`
matches the value set by `blr` and/or `br` instructions (see
https://developer.arm.com/documentation/100076/0100/A64-Instruction-Set-Reference/A64-General-Instructions/BTI).
As far as I understand, instructions like `blraa` do not set this state (at
least, I was unable to find info regarding this), so further `bti` instruction
will fail. @psmith35 Could you please clarify if `blraa` and other
authenticating branch instruction set `PSTATE.BTYPE` which `bti` instructions
treat as correct?
6. `standard` - a combination of `bti`, `gcs`, `pac-ret` w/o `+leaf` and,
optionally, `+pc` - so, shouldn't be supported with `pauthabi` since its parts
are not supported.
Regarding disallowing separate flags like `-fptrauth-returns` with other branch
protection schemes - I don't think we need to do that since the `-fptrauth-*`
flags are not intended to be used directly - instead, we propose this
"umbrella" option `-mbranch-protection=pauthabi`, which enabled a "good enough"
set of flags at once.
Please let me know your thought about that. I'll update the PR with
corresponding checks as soon as we come to some consensus on the question.
https://github.com/llvm/llvm-project/pull/97237
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
