[clang] [llvm] [SystemZ] Global Stackprotector and associated location section (PR #169317)

[Ulrich Weigand via llvm-commits]

uweigand wrote:

The latest commit https://github.com/llvm/llvm-project/pull/169317/commits/931e6493d95dc399461e3311334fb928c11b60e2 seem to be broken?

Before reviewing the details, I still have more general questions on the approach.   Combining the `LOAD_STACK_GUARD` into `MOVE_STACK_GUARD` and `COMPARE_STACK_GUARD` does make sense to me to ensure the stack canary never ends up in any register.   [ As an aside, are there any uses of `LOAD_STACK_GUARD` remaining after that optimization?  That would unfortunate - maybe we should rather error out if that happens?  ]

However, with your current approach, the *address* of the stack canary not only end up in a register (this seems to be unavoidable), but it might actually end up spilled to a *stack slot* - the `LOAD_STACK_GUARD_ADDRESS` is created before register allocation, so it loads the address into a virtual register that might get spilled later.  [ Note that given that fact, it seems pointless to move the implementation of loading the address all the way to the AsmPrinter.  ]

Having checked some of the history of the stack protector implementation in GCC, it does appear that having the address of the stack canary spilled to the stack is a potential weakness that should be avoided.  Given that, this new implementation is even *worse* in this respect then the current implementation - the current implementation does leak the canary into a register, but at least neither the canary nor its address can end up spilled to the stack.

I seem to recall that in an earlier attempt, you delayed the split between loading and using the address to post-RA, but that ran into register allocation problems with the scratch register?  I think we might have to go back to that approach and fix those register allocation problems - sorry!


https://github.com/llvm/llvm-project/pull/169317