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

[Dominik Steenken via llvm-commits]

dominik-steenken wrote:

> The latest commit [931e649](https://github.com/llvm/llvm-project/commit/931e6493d95dc399461e3311334fb928c11b60e2) seem to be broken?

That is quite strange, debugging it now.

> 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? ]

I don't think that uses of `LOAD_STACK_GUARD` would remain. We could assert out in the asm printer if we encounter one instead of trying to properly expand it.

> 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. ]

Could we not use the same approach that the existing `LOAD_STACK_GUARD` uses in order to avoid the address from being spilled? I.e. could we not mark `LOAD_STACK_GUARD_ADDRESS` `isReMaterializable`, and do some further nudging with kill flags to ensure that the value isn't reused? Defining it as `mayLoad` and adding a volatile `MachineMemOperand` might also, help, right? Although it is not technically a load in all cases, and `mayLoad = 1` also adds some code paths that assume that there is an operand attached to the instruction that is loaded "from", so there may be additional work there.

> 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 understand but i am hoping that this can be avoided with the same approach that current code uses to avoid leaking the canary to the stack (see above).
 
> 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!

I did have issues with scratch registers, yes. From what i'm reading, i would have to use the RegisterScavenger to obtain an unused register, and I was having some trouble with that. I guess that could be taken up again.

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