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?
--
H.J.
