[clang] [llvm] [PAC][Driver] Implement `-mbranch-protection=pauthabi` option (PR #97237)

Daniil Kovalev via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 4 15:22:12 PDT 2024


================
@@ -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


More information about the cfe-commits mailing list