[PATCH] D40863: [AArch64][Darwin] Implement stack probing for static and dynamic stack objects

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D40863#957575, @aemerson wrote:

> The motivation for stack probing here is different to the Windows case, although the mechanism is the same. For Darwin this is a security feature to protect against stack clash based attacks.


Ah, thanks for explaining!



================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:478
+  if (!LRIsSaved) {
+    // LR wasn't saved as a CSR, so we need to save it ourselves. We don't
+    // bother updating SP, as we know the probe function won't modify any
----------------
aemerson wrote:
> mstorsjo wrote:
> > Instead of manually saving LR here if it wasn't already saved, in D41131 I instead tried to estimate whether the stack probe would end up being necessary in determineCalleeSaves, and made sure LR is saved already there if a stack probe will be necessary.
> Is that estimate always conservative? I.e. does it always at least over-estimate the stack size?
I think it should be possible to make it conservative. I found an issue with the estimate in my patch, but I'll update it.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:502
+
+  BuildMI(MBB, MBBI, DL, TII->get(AArch64::BL))
+      .addExternalSymbol(MF.createExternalSymbolName(Symbol))
----------------
aemerson wrote:
> mstorsjo wrote:
> > In the implementation in D41131 (based on the one for Windows on ARM by @compnerd in rL207615), I check for CodeModel::Large as well, and do the function call via MOVaddrEXT and BLR for that case.
> This function is specific to the Darwin stack probing ABI under consideration, so I don't think we'll be able to share this code. No objections to your patch though.
Yes, the ABI for the function call themselves is different so this particular piece of code can't be shared, but ideally all the logic for when to emit it could be shared though.


Repository:
  rL LLVM

https://reviews.llvm.org/D40863