[PATCH] D56717: [SLH] AArch64: correctly pick temporary register to mask SP

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls marked 5 inline comments as done.
kristof.beyls added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp:252
+  }
+  llvm_unreachable("The nature of call instructions and the ABI must result in "
+                   "finding at least one register above.");
----------------
kristof.beyls wrote:
> olista01 wrote:
> > This can be hit by this code:
> > 
> >   typedef int (*fptr_t)(void);
> >   int foo(fptr_t f) {
> >     register fptr_t f2 asm("x30") = f;
> >     asm("" : "+r"(f2));
> >     return f2() + 1;
> >   }
> > 
> > This uses inline assembly to force the function pointer into x30, but it is an allocatable register so I think it is possible for this to happen "naturally".
> Thanks for that example. I didn't manage to come up with an example myself.
> My understanding is that the llvm_unreachable is reached because the ABI rules on the indirect call are not modelled very accurately.
> Some of the registers will be clobbered by the call, and could be used as temporary register, but it seems this isn't modelled on the BLR mir instruction.
> Anyway, as I said above - probably I'll need to implement generation of the ISB/DSB full speculation barrier in such a rare case.
The example you provided is now (further simplified and as MIR) in one of the regression tests.


================
Comment at: llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp:253
+  llvm_unreachable("The nature of call instructions and the ABI must result in "
+                   "finding at least one register above.");
+}
----------------
kristof.beyls wrote:
> zbrid wrote:
> > Since I don't know much about the ABI or the assumptions around calls, I had some quick questions about this for clarification. Can you be sure that this is unreachable because the way the calling convention is specified means there will always be at least one dead register before a call? For example, since there are several call clobbered registers specified in the calling convention even if x17 is used as an address for an indirect branch instruction (and is therefore live across the call), there's no way for all of the call clobbered registers to be live since there are no call instructions that could force all of them to be live? 
> Yes, indeed, the assumption is that the calling convention will result in at least one register being "killed" during the call. Or at least not being guaranteed to retain the same value.
> The ABI actually specifies that x17 can never be live across a call, which is why this was picked initially. However, as can be seen in the new test cases in this patch, x17 could still be legally be used as containing the address an indirect call has to branch to - making the register still live just before the call.
The new version of the patch handles no temporary register being available by inserting a full speculation barrier.
Across all of the test suite, this case was encountered just twice.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56717/new/

https://reviews.llvm.org/D56717