| Message ID | CAO2gOZUpcpdSSaaXU4zUAV2WOzE3A18zzQe858MnQeBWEXagjA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Guard also with L_IPO_COMP_MODE as this is lipo specific. David On Sat, Jun 8, 2013 at 8:29 PM, Dehao Chen <dehao@google.com> wrote: > This patch fixes a bug when two weakref symbols are mapped to a same > assembler name. > > Testing on going. > > OK for google branches if test is fine? > > Thanks, > Dehao > > Index: gcc/varasm.c > =================================================================== > --- gcc/varasm.c (revision 199844) > +++ gcc/varasm.c (working copy) > @@ -5502,6 +5502,10 @@ do_assemble_alias (tree decl, tree target) > if (TREE_ASM_WRITTEN (decl)) > return; > > + if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)) > + && TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl))) > + return; > + > /* We must force creation of DECL_RTL for debug info generation, even though > we don't use it here. */ > make_decl_rtl (decl);
On 9 June 2013 05:31, Xinliang David Li <davidxl@google.com> wrote: > Guard also with L_IPO_COMP_MODE as this is lipo specific. Sounds like this is the LIPO incarnation of http://gcc.gnu.org/PR31537 See the patch at http://gcc.gnu.org/PR31537#c9 for not adding the alias in the first place (back then, honza may have changed that recently on trunk). Honza, Joseph, Are there rules for handling weakrefs in some standard? If so, which ones? Just curious.. Thanks, > > David > > On Sat, Jun 8, 2013 at 8:29 PM, Dehao Chen <dehao@google.com> wrote: >> This patch fixes a bug when two weakref symbols are mapped to a same >> assembler name. >> >> Testing on going. >> >> OK for google branches if test is fine? >> >> Thanks, >> Dehao >> >> Index: gcc/varasm.c >> =================================================================== >> --- gcc/varasm.c (revision 199844) >> +++ gcc/varasm.c (working copy) >> @@ -5502,6 +5502,10 @@ do_assemble_alias (tree decl, tree target) >> if (TREE_ASM_WRITTEN (decl)) >> return; >> >> + if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)) >> + && TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl))) >> + return; >> + >> /* We must force creation of DECL_RTL for debug info generation, even though >> we don't use it here. */ >> make_decl_rtl (decl);
> On 9 June 2013 05:31, Xinliang David Li <davidxl@google.com> wrote: > > Guard also with L_IPO_COMP_MODE as this is lipo specific. > > Sounds like this is the LIPO incarnation of http://gcc.gnu.org/PR31537 > > See the patch at http://gcc.gnu.org/PR31537#c9 for not adding the > alias in the first place (back then, honza may have changed that > recently on trunk). > > Honza, Joseph, > Are there rules for handling weakrefs in some standard? If so, which ones? Not that I would know of. It is an extension engineered for glibc pthreads. in mainline I quite changed way weakrefs are handled and I still plan to do more. What LTO does now is to handle weakref symbol as static symbol despite it has DECL_EXTERNAL on it. (I have patch in queue to really make it static). This is because the symbol never hits the exported symbol table anyhow. I.e. you can have: cat f1.i static __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once"))); $ cat f2.i static __gthrw_pthread_once __attribute__ ((__weakref__ ("comething_completely_else"))); that will break with your do_assemble_alias change. with LTO you will now get __gthrw_pthread_once.1 and __gthrw_pthread_once.2 with proper weakref targets. Honza
Index: gcc/varasm.c =================================================================== --- gcc/varasm.c (revision 199844) +++ gcc/varasm.c (working copy) @@ -5502,6 +5502,10 @@ do_assemble_alias (tree decl, tree target) if (TREE_ASM_WRITTEN (decl)) return; + if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)) + && TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl))) + return; + /* We must force creation of DECL_RTL for debug info generation, even though we don't use it here. */ make_decl_rtl (decl);