| Message ID | 0EFAB2BDD0F67E4FB6CCC8B9F87D756969C18BCF@IRSMSX101.ger.corp.intel.com |
|---|---|
| State | New |
| Headers | show |
On 10/31/14 09:34, Zamyatin, Igor wrote: > Hi! > > Following patch (moving initialization of pic_offset_table_rtx earlier) fixes failures for asan tests on 32 bits in PIC mode mentioned here - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63534#c48 > > > Bootstrapped/regtested on x86_64, i686 > > Is it ok for trunk? > > 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. You should also reference the PR in the ChangeLog like this: PR target/63634 So get the analysis posted, and I'll likely approve after reviewing the background info. Thanks, Jeff
> > Hi! > > > > Following patch (moving initialization of pic_offset_table_rtx > earlier) fixes failures for asan tests on 32 bits in PIC mode mentioned here - > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63534#c48 > > > > > > Bootstrapped/regtested on x86_64, i686 > > > > Is it ok for trunk? > > > > 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. Thanks, Igor
On Wed, Nov 5, 2014 at 7:17 AM, Zamyatin, Igor <igor.zamyatin@intel.com> wrote: >> > Hi! >> > >> > Following patch (moving initialization of pic_offset_table_rtx >> earlier) fixes failures for asan tests on 32 bits in PIC mode mentioned here - >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63534#c48 >> > >> > >> > Bootstrapped/regtested on x86_64, i686 >> > >> > Is it ok for trunk? >> > >> > 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.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 9bd6135..cf7d94a 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -1718,6 +1718,11 @@ expand_used_vars (void) init_vars_expansion (); + /* 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); + 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 1ef43c4..0bbcbbb 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -3464,11 +3464,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);