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

[Magee, Josh]

Hi Bill,

I had wondered if adding a field to AllocaInst might be a problem (especially since the field is for a fairly specialized purpose that is not relevant to the majority of users of the class.)  I'll re-work the patch to avoid modifying AllocaInst.  (My current idea is to maintain a ValueMap of <AllocaInst*, SSPLayoutKinds> in the StackProtector pass itself and then use getAnalysis to access this from the PEI and  LocalStackAllocation passes.  It seems like I might need to split the StackProtector pass into separate analysis and transform passes, but I'm not certain yet.)

Hopefully I can have an update ready within the next couple of days.
Thanks!
Josh


> -----Original Message-----
> From: Bill Wendling [mailto:wendling at apple.com]
> Sent: Tuesday, February 12, 2013 12:31 AM
> To: Magee, Josh
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] -fstack-protector-strong part 3: data layout
> 
> 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>
>