[stackprotector] Add the llvm.stackprotectorcheck intrinsic

[Michael Gottesman]

Hey Eli! Sorry for the delay.

On Jul 23, 2013, at 4:14 PM, Eli Friedman <eli.friedman at gmail.com> wrote:

> What happens if the call satisfies all the platform-independent
> requirements, but fails some platform-specific requirement?  Does it
> just not matter because the buffers the stack protector is protecting
> can't be referenced?

The key thing about the patch is that something magical is happening. No matter where the stack protector intrinsic is in the actual basic block, the stack protector code will be inserted right before the terminator whether or not it is a return or a tail return. If the call fails the platform specific requirements, then there will be a call and then the terminator. The stack protector code will be inserted right before the terminator and thus right before the call.

> 
> Also, I don't understand why we need to introduce an intrinsic: if
> steps 1 and 2 are both in the same pass, can't you just insert the
> compare+branch before the tail call?

I can’t actually insert the compare + branch before the tail call at the IR level since the branch will be a terminator implying that I would need to split the basic block at the IR level. Splitting the basic block at the IR level is exactly what we do not want to do since then we would be required to move a failed tail call at the MI level from the success basic block to the parent basic block before the actual stack protector compare happens. This would require doing platform dependent pattern matching, something that I am trying to avoid.

What the intrinsic is really being used for is delaying the code-gen of the stack protector check until we know the exact position where we wish to insert it (which due to the way we represent tail calls at the MI level is always right before the terminator of the basic block no matter whether we have a tail call, failed tail call, or normal call). Thus we can stay platform independent and clean.

Michael

P.S. I found a mistake in my original proposal. In order to be 100% safe, we need to include the loads in the stack protector check as well that way in the case where there is a failed tail call, we avoid the situation where we load from the stack protector variables, then have a stack smash and then perform the check afterwards causing us to miss the stack smash potentially. I additionally found an improvement where I realized that I only needed to have the intrinsic pass in the guard variable and not the stack frame variable since I can look up the stack frame location of the stack protector from MachineFrameInfo::getStackProtectorIndex.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130809/19b4e794/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-stackprotector-Added-intrinsic-llvm.stackprotectorch.patch
Type: application/octet-stream
Size: 3615 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130809/19b4e794/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130809/19b4e794/attachment-0001.html>