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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa marked an inline comment as done.
jonpa added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:708
+  if (Residual)
+    allocateAndProbe(*MBB, MBBI, Residual, true/*EmitCFI*/);
+
----------------
serge-sans-paille wrote:
> 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?
IIUC, the topmost byte of each allocated stack range is always naturally probed before the entry of the prologue by the push of the return address by the call. The lowermost byte is then probed in each full block, but the residual needs no probing.

In order get a matching behavior with dynamic allocas, the topmost byte should always be probed, as well as the lowermost byte of each full block.

So yes, I think that would be correct as long as there are no other allocations of stack space anywhere...



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

https://reviews.llvm.org/D78717