> On Fri, Nov 14, 2014 at 8:59 AM, Zamyatin, Igor <igor.zamya...@intel.com>
> wrote:
> >> >> >
> >> >> > ChangeLog:
> >> >> >
> >> >> > 2014-10-30 Igor Zamyatin <igor.zamya...@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.zamya...@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.zamya...@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.
