RE: [PATCH, i686] Fix for asan test failures with -m32 happened after EBX enabling in PIC mode

["Zamyatin, Igor" <igor dot zamyatin at intel dot com>]

> On Fri, Nov 14, 2014 at 8:59 AM, Zamyatin, Igor <igor.zamyatin@intel.com>
> wrote:
> >> >> >
> >> >> > ChangeLog:
> >> >> >
> >> >> > 2014-10-30  Igor Zamyatin  <igor.zamyatin@intel.com>
> >> >> >
> >> >> >     * function.c (assign_parms): Move init of pic_offset_table_rtx
> >> >> >     from here to...
> >> >> >     * cfgexpand.c (expand_used_vars): ...here.
> >> >> The patch is probably fine.  However, it would be good to have the
> >> >> analysis why you want to move initialization of the PIC register earlier.
> >> >
> >> > Asan (and anybody else can) emits global variable(s) in
> >> > expand_used_vars
> >> during function expanding while pic reg is currently initialized
> >> later, during expand_function_start in assign_parms thus to be late
> >> in asan case in PIC mode.
> >> >
> >> > So to avoid such cases we put pic reg initialization in the
> >> > beginning of
> >> expand_used_vars. This seems to be early enough.
> >> >
> >>
> >> Please mention PR in ChangeLog and add a few testcases so that the
> >> fix will be tested on Linux.
> >>
> >
> > Bootstrapped and regtested on x86_64 and i686 incl pic mode.
> > Is it ok?
> >
> > Thanks,
> > Igor
> >
> > gcc/Changelog:
> >
> > 2014-11-14  Igor Zamyatin  <igor.zamyatin@intel.com>
> >
> >         PR sanitizer/63845
> >         * function.c (assign_parms): Move init of pic_offset_table_rtx
> >         from here to...
> >         * cfgexpand.c (expand_used_vars): ...here.
> >
> > gcc/testsuite/Changelog:
> >
> > 2014-11-14  Igor Zamyatin  <igor.zamyatin@intel.com>
> >
> >         PR sanitizer/63845
> >         * gcc.target/i386/pr63845.c: New test.
> >
> >
> >
> > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 15d7638..bcd3b35
> > 100644
> > --- a/gcc/cfgexpand.c
> > +++ b/gcc/cfgexpand.c
> > @@ -1722,6 +1722,9 @@ expand_used_vars (void)
> >
> >    init_vars_expansion ();
> >
> > +  if (targetm.use_pseudo_pic_reg ())
> > +    pic_offset_table_rtx = gen_reg_rtx (Pmode);
> > +
> >    hash_map<tree, tree> ssa_name_decls;
> >    for (i = 0; i < SA.map->num_partitions; i++)
> >      {
> > diff --git a/gcc/function.c b/gcc/function.c index ef98091..97e0b79
> > 100644
> > --- a/gcc/function.c
> > +++ b/gcc/function.c
> > @@ -3679,11 +3679,6 @@ assign_parms (tree fndecl)
> >
> >    fnargs.release ();
> >
> > -  /* Initialize pic_offset_table_rtx with a pseudo register
> > -     if required.  */
> > -  if (targetm.use_pseudo_pic_reg ())
> > -    pic_offset_table_rtx = gen_reg_rtx (Pmode);
> > -
> >    /* Output all parameter conversion instructions (possibly including calls)
> >       now that all parameters have been copied out of hard registers.  */
> >    emit_insn (all.first_conversion_insn); diff --git
> > a/gcc/testsuite/gcc.target/i386/pr63845.c
> > b/gcc/testsuite/gcc.target/i386/pr63845.c
> > new file mode 100644
> > index 0000000..4b675e0
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr63845.c
> > @@ -0,0 +1,20 @@
> > +/* PR sanitizer/63845 */
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target ia32 } */
> > +/* { dg-require-effective-target fpic } */
> > +/* { dg-skip-if "No Windows PIC" { *-*-mingw* *-*-cygwin } { "*" } {
> > +"" } } */
> > +/* { dg-options "-fPIC" } */
> > +
> > +int __attribute__ ((noinline, noclone)) foo (void *p) {
> > +  return *(int*)p;
> > +}
> > +
> > +int main ()
> > +{
> > +  char a = 0;
> > +  foo (&a);
> > +  return 0;
> > +}
> > +
> >
> 
> Will this test fail on Linux without your fix? Doesn't testcase need -
> fsanitize=address to fail?

Sure, you're right, will add it

Thanks,
Igor

> 
> 
> --
> H.J.