Patchwork [PR,libmudflap/53359] don't register symbols not emitted

login
register
mail settings
Submitter Alexandre Oliva
Date Dec. 21, 2012, 5:33 a.m.
Message ID <ory5gsufym.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/207761/
State New
Headers show

Comments

Alexandre Oliva - Dec. 21, 2012, 5:33 a.m.
libmudflap emits a global initializer that registers memory ranges for
global data symbols.  However, even if IPA decides not to emit a symbol
because it's unused, we'd still emit registration sequences for them in
some cases, which, in the PR testcase, would result in TOC references to
the undefined symbols.

This patch fixes the problem, avoiding registration for symbols that are
not present in the varpool.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu; I've also verified
that it removes the TOC references on a ppc64-linux-gnu cross.

Ok to install?
Richard Guenther - Dec. 21, 2012, 9:50 a.m.
On Fri, Dec 21, 2012 at 6:33 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> libmudflap emits a global initializer that registers memory ranges for
> global data symbols.  However, even if IPA decides not to emit a symbol
> because it's unused, we'd still emit registration sequences for them in
> some cases, which, in the PR testcase, would result in TOC references to
> the undefined symbols.
>
> This patch fixes the problem, avoiding registration for symbols that are
> not present in the varpool.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu; I've also verified
> that it removes the TOC references on a ppc64-linux-gnu cross.
>
> Ok to install?

Hmm, I think that at this point of the compilation you are looking for
TREE_ASM_WRITTEN instead.  I'm not sure we will never end up
having a symtab node that not ends up being emitted.

Honza?

Thanks,
Richard.

>
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
>
Jakub Jelinek - Dec. 21, 2012, 9:55 a.m.
On Fri, Dec 21, 2012 at 10:50:58AM +0100, Richard Biener wrote:
> On Fri, Dec 21, 2012 at 6:33 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> > libmudflap emits a global initializer that registers memory ranges for
> > global data symbols.  However, even if IPA decides not to emit a symbol
> > because it's unused, we'd still emit registration sequences for them in
> > some cases, which, in the PR testcase, would result in TOC references to
> > the undefined symbols.
> >
> > This patch fixes the problem, avoiding registration for symbols that are
> > not present in the varpool.
> >
> > Regstrapped on x86_64-linux-gnu and i686-linux-gnu; I've also verified
> > that it removes the TOC references on a ppc64-linux-gnu cross.
> >
> > Ok to install?
> 
> Hmm, I think that at this point of the compilation you are looking for
> TREE_ASM_WRITTEN instead.  I'm not sure we will never end up
> having a symtab node that not ends up being emitted.

Yeah, asan.c also tests TREE_ASM_WRITTEN and doesn't register
!TREE_ASM_WRITTEN decls for instrumentation.

	Jakub
Jan Hubicka - Dec. 21, 2012, 10:42 a.m.
> On Fri, Dec 21, 2012 at 10:50:58AM +0100, Richard Biener wrote:
> > On Fri, Dec 21, 2012 at 6:33 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> > > libmudflap emits a global initializer that registers memory ranges for
> > > global data symbols.  However, even if IPA decides not to emit a symbol
> > > because it's unused, we'd still emit registration sequences for them in
> > > some cases, which, in the PR testcase, would result in TOC references to
> > > the undefined symbols.
> > >
> > > This patch fixes the problem, avoiding registration for symbols that are
> > > not present in the varpool.
> > >
> > > Regstrapped on x86_64-linux-gnu and i686-linux-gnu; I've also verified
> > > that it removes the TOC references on a ppc64-linux-gnu cross.
> > >
> > > Ok to install?
> > 
> > Hmm, I think that at this point of the compilation you are looking for
> > TREE_ASM_WRITTEN instead.  I'm not sure we will never end up
> > having a symtab node that not ends up being emitted.

Ones representing external vars, but that probably doesn't count. 
Also for constant pool the decision whether to emit or not is still
done behind symtab's back.   All other nodes should be removed.
> 
> Yeah, asan.c also tests TREE_ASM_WRITTEN and doesn't register
> !TREE_ASM_WRITTEN decls for instrumentation.

Note that FEs sometimes play dirty games setting TREE_ASM_WRITTEN
to true to avoid fake symbols being output.

Probably none of these matters at this point.  Just to point it out.

Honza
> 
> 	Jakub

Patch

don't let mudflap register global symbols that won't be emitted

From: Alexandre Oliva <aoliva@redhat.com>

for  gcc/ChangeLog

	PR libmudflap/53359
	* tree-mudflap.c (mudflap_finish_file): Skip deferred decls
	not found in the symtab.
---

 gcc/tree-mudflap.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)


diff --git a/gcc/tree-mudflap.c b/gcc/tree-mudflap.c
index 90d0448..a9caaf2 100644
--- a/gcc/tree-mudflap.c
+++ b/gcc/tree-mudflap.c
@@ -1335,6 +1335,10 @@  mudflap_finish_file (void)
           if (! TREE_PUBLIC (obj) && ! TREE_ADDRESSABLE (obj))
             continue;
 
+	  /* If we're not emitting the symbol, don't register it.  */
+	  if (!symtab_get_node (obj))
+	    continue;
+
           if (! COMPLETE_TYPE_P (TREE_TYPE (obj)))
             {
               warning (OPT_Wmudflap,