[asan] Protection of stack vars

[Jakub Jelinek]

On Fri, Oct 12, 2012 at 10:52:04AM -0700, Xinliang David Li wrote:
> This is related to the way how you implement it. Emitting the stack
> shadow initialization code in GIMPLE would solve the problem. I think
> that would be cleaner.

You'd need to duplicate all the stack slot sharing code, or adjust it so
that it would be usable for both RTL and GIMPLE.  IMHO much uglier.

I don't see how it would solve the problem where to put the cleanup
sequence.  Either you'd put it before tail calls and allow them to be
tail call optimized (but you don't know at the GIMPLE level whether
they will be or won't be tail call optimized), or you'd just put them
after the calls, but then they wouldn't be tail call optimized ever.
It is really not a big deal to put the cleanup sequence somewhere,
it is easy to disable tail call optimizations if var_end_seq is non-NULL,
if we decide that is the way to go, or it is easy to put it before the
calls.
The reason I haven't implemented it is because I run out of hacking time for
today.

> > +static rtx
> > +asan_shadow_cst (unsigned char shadow_bytes[4])
> 
> Missing comments.

Will add.
> 
> > +{
> > +  int i;
> > +  unsigned HOST_WIDE_INT val = 0;
> > +  gcc_assert (WORDS_BIG_ENDIAN == BYTES_BIG_ENDIAN);
> > +  for (i = 0; i < 4; i++)
> > +    val |= (unsigned HOST_WIDE_INT) shadow_bytes[BYTES_BIG_ENDIAN ? 3 - i : i]
> > +          << (BITS_PER_UNIT * i);
> > +  return GEN_INT (trunc_int_for_mode (val, SImode));
> > +}
> > +
> > +rtx
> > +asan_emit_stack_protection (rtx base, HOST_WIDE_INT *offsets, tree *decls,
> > +                           int length)
> 
> Missing comments.

Likewise.

> See my previous comment -- having this in rtl form makes the code less
> readable and less maintainable.

I disagree about that.  I know in LLVM asan is implemented as a single pass
transforming everything at once, but I don't think that is the best thing to
do in GCC, actually I think it is better to let the instrumentation phase
just emit an artificial builtin call instead of the shadow mem address
computation, read, test, and conditional call (e.g. because then it won't
prevent vectorization), and either lower it at some later point during
GIMPLE, or just expand it to RTL directly.

> >  /* A subroutine of expand_used_vars.  Give each partition representative
> >     a unique location within the stack frame.  Update each partition member
> >     with that location.  */
> >
> >  static void
> > -expand_stack_vars (bool (*pred) (tree))
> > +expand_stack_vars (bool (*pred) (tree), VEC(HOST_WIDE_INT, heap) **asan_vec,
> > +                  VEC(tree, heap) **asan_decl_vec)
> 
> 
> With the current implementation, the interface change can also be
> avoided --- can the asan shadow var info be stashed into 'struct
> stack_var' ?

Yeah, sure, it can be groupped into a single struct.  But it is just
an interface between a helper function and the caller anyway, the reason
for the separate expand_stack_vars function is that it needs to be called
up to 3 times for -fstack-protector support.

	Jakub