[PATCH] D84419: Fix interaction between stack alignment and inline-asm stack clash protection

[Kai Luo via Phabricator via llvm-commits]

lkail added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1102
+
+  if (Reg == StackPtr && EmitInlineStackProbe && MaxAlign > StackProbeSize) {
+    {
----------------
efriedma wrote:
> I don't think this condition is right.
> 
> Say MaxAlign == StackProbeSize.  Then an "and" can allocate up to StackProbeSize-4 bytes.  So any subsequent stack allocation can jump over a guard page.  (This is an extreme example.  Really, it doesn't matter what the alignment is; it's just harder to cause a practical issue if the alignment is small.)
> 
> In general, we can't skip a probe for a stack allocation.  We can only merge the probes for adjacent stack allocations.  Say, for example, we realign the stack then allocate "Offset" bytes of aligned memory.  We can get away with considering both allocations as a single "allocation" if `MaxAlign+Offset <= StackProbeSize`.  But that method of proof works if you analyze them together. If you analyze each allocation independently, you can't prove the safety, so the realignment needs its own probe.
Good catch. Looks PPC64's implementation also has the same issue. I'll post a patch to fix this issue for PPC64.


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

https://reviews.llvm.org/D84419