[PATCH] D78717: [SystemZ] Implement -fstack-clash-protection

[serge via Phabricator via llvm-commits]

serge-sans-paille added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:708
+  if (Residual)
+    allocateAndProbe(*MBB, MBBI, Residual, true/*EmitCFI*/);
+
----------------
jonpa wrote:
> serge-sans-paille wrote:
> > On X86, we don't need to probe that final allocation because the two ways the stack could grow after the final alloca are
> > 
> > 1. A function call, and in that case we get a free probe when we make a function call
> > 2. A PROBED_ALLOCA, and in that case we get a probe at Residual + PAGE_SIZE, which is right into the Page Guard.
> > 
> > I assume that's different for SystemZ? 
> There's not always a free probe with a function call on SystemZ (only if stack space is allocated by the called function), but regardless of that I think the residual is needed. Since the probe is done on the topmost byte of each allocated block on SystemZ, the guard page could fit within the last allocated full block and a residual of just 8 or more bytes (probing the high byte is what GCC is doing on SystemZ).
> 
> I still wonder if that would not be necessary also on X86 in case "2": Given that you on X86 probe the lower byte of each full block, you could get a residual into the guard page, and then if the dynamic alloca started with a full block, the next probe would not be into the guard page, but past it:
> 
> ```
> |     |     |GGGGG|     |
>      |    P|   R|    P|
> 
> ```
> 
> Maybe I am missing something?
> 
I took my pen and paper, and you're definitively right. Fortunately, there's the possibility of always probing the upper byte when doing a dynamic alloca, so that we always avoid this extra probe. That way the common case (alloc_size < PAGE_SIZE) remains costless. Correct?


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

https://reviews.llvm.org/D78717