[PATCH] -fstack-protector-strong part 3: data layout

[Bill Wendling]

Hi Josh,

I'll review this in a few days (sorry, I'm swamped). One comment though, is there *any* way for you to not add a field to AllocaInst? I'm going to push back hard on that because it's a basic class and shouldn't be modified unless absolutely necessary, especially if you're adding fields to it. :)

-bw

On Feb 11, 2013, at 7:01 PM, "Magee, Josh" <Joshua.Magee at am.sony.com> wrote:

> Hi,
> 
> This is part 3 in the stack-protector-strong series.
> This patch adds support for data layout rules.
> 
> The rules for ssp are:
> 1) Large arrays and structures containing large arrays should be closest to
>    the stack protector.  "Large" is defined as >= ssp-buffer-size.
> 2) The remaining stack objects are arranged normally.
> 
> These rules for sspstrong and sspreq are the same:
> 1) Large arrays and structures containing large arrays should be closest to
>    the stack protector.  "Large" is defined as >= ssp-buffer-size.
> 2) Small arrays and structures containing small arrays should be 2nd closet to
>    the protector.  "Small" is defined as < ssp-buffer-size.
> 3) Variables that have had their address taken should be 3rd closest to the
>    protector.
> 4) The remaining stack objects are arranged normally.
> 
> LLVM already allocates SSP-triggering objects near the stack protector, but:
> a) The heuristic used is very basic and doesn't necessarily match what the
>    StackProtector pass does.
> b) It doesn't handle the finer grained rules for strong/req listed above.
> 
> Rather than determining if a stack object may need to be placed near the
> protector in FunctionLoweringInfo.cpp, I thought it is better to make this
> decision in the StackProtector pass (where we already do the needed analysis
> anyway.)  
> 
> The changes I made are:
> 1) Add an enum type to represent the SSP Layout.
>   * SSPLayoutKind
> 2) Add a field to AllocaInst to keep track of the SSPLayoutKind.
> 3) Update the StackProtector pass analysis to set the SSPLayoutKind for an
>    AllocaInst.  This requires two unfortunate changes:
>   * Since all variables vulnerable to stack smashing need to be re-arranged
>     (and not just the first one encountered), the analyze can't exit early
>     after finding the first one.
>   * The analysis needs to be run for SSPReq.  Before it wasn't necessary for
>     SSPReq because all SSPReq functions get a protector regardless of their
>     variables. Now the analysis is needed to mark the vulnerable variables.
> 4) Teach the PrologEpilogInserter and LocalStackAllocation passes the new SSP
>    stack re-arrangement rules.
> 
> Thanks!
> -Josh
> <part3-sspstrong-datalayout.diff>