[PATCH] D68720: Support -fstack-clash-protection for x86

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> (b) is an issue, as pointed out in https://lwn.net/Articles/726587/ (grep for valgrind) : from valgrind point of view, accessing un-allocated stack memory triggers error, and we probably want to please valgrind
> 
> Doing the call *after* the stack allocation is also not an option, as a signal could be raised between the stack allocation and the stack probing, escaping the stack probe if a custom signal handler is executed.

I'm not sure I follow.  How are you solving this problem in your patch?  By limiting the amount you adjust the stack at a time?  What limit is sufficient to avoid this issue?

-----

Can you give a complete assembly listing for small examples of static and dynamic stack probing?



================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:400
+        !(STI.isOSWindows() && !STI.isTargetMachO());
+    if (InlineStackClashProtector && !InEpilogue) {
+      const uint64_t PageSize = TLI.getStackProbeSize(MF);
----------------
Why is this code in a different location from the stack probing code that generates a call?


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:408
+        CurrentAbsOffset += ChunkSize;
+        MI->getOperand(3).setIsDead(); // The EFLAGS implicit def is dead.
+
----------------
This algorithm needs some documentation; it isn't at all obvious what it's doing.  Particularly the interaction with "free" stack probes.

Should we generate a loop if the stack frame is large?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720