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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1102
+
+  if (Reg == StackPtr && EmitInlineStackProbe && MaxAlign > StackProbeSize) {
+    {
----------------
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.


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

https://reviews.llvm.org/D84419