* [PR libmudflap/53359] don't register symbols not emitted @ 2012-12-21 5:34 Alexandre Oliva 2012-12-21 9:51 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2012-12-21 5:34 UTC (permalink / raw) To: gcc-patches; +Cc: Jan Hubicka [-- Attachment #1: Type: text/plain, Size: 565 bytes --] 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? [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: mudflap-global-init-check-symtab-pr53359.patch --] [-- Type: text/x-diff, Size: 816 bytes --] 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, [-- Attachment #3: Type: text/plain, Size: 258 bytes --] -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2012-12-21 5:34 [PR libmudflap/53359] don't register symbols not emitted Alexandre Oliva @ 2012-12-21 9:51 ` Richard Biener 2012-12-21 9:55 ` Jakub Jelinek 2012-12-30 0:23 ` Alexandre Oliva 0 siblings, 2 replies; 13+ messages in thread From: Richard Biener @ 2012-12-21 9:51 UTC (permalink / raw) To: Alexandre Oliva; +Cc: gcc-patches, Jan Hubicka 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2012-12-21 9:51 ` Richard Biener @ 2012-12-21 9:55 ` Jakub Jelinek 2012-12-21 10:42 ` Jan Hubicka 2012-12-30 0:23 ` Alexandre Oliva 1 sibling, 1 reply; 13+ messages in thread From: Jakub Jelinek @ 2012-12-21 9:55 UTC (permalink / raw) To: Richard Biener; +Cc: Alexandre Oliva, gcc-patches, Jan Hubicka 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2012-12-21 9:55 ` Jakub Jelinek @ 2012-12-21 10:42 ` Jan Hubicka 0 siblings, 0 replies; 13+ messages in thread From: Jan Hubicka @ 2012-12-21 10:42 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, Alexandre Oliva, gcc-patches, Jan Hubicka > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2012-12-21 9:51 ` Richard Biener 2012-12-21 9:55 ` Jakub Jelinek @ 2012-12-30 0:23 ` Alexandre Oliva 2013-01-02 14:53 ` Richard Biener 1 sibling, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2012-12-30 0:23 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Jan Hubicka [-- Attachment #1: Type: text/plain, Size: 1078 bytes --] On Dec 21, 2012, Richard Biener <richard.guenther@gmail.com> 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. > Hmm, I think that at this point of the compilation you are looking for > TREE_ASM_WRITTEN instead. That doesn't work, several mudflap regressions show up because accesses to global library symbols that are accessed by template methods compiled with mudflap (say cout) are then verified but not registered. We have to register symbols that are not emitted but that referenced. I've now updated the comment to reflect this. Is this ok to install? Regstrapped again (along with the patches for feraiseexcept, since there weren't any non-comment changes here) on x86_64-linux-gnu and i686-linux-gnu. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: mudflap-global-init-check-symtab-pr53359.patch --] [-- Type: text/x-diff, Size: 1160 bytes --] 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 | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/gcc/tree-mudflap.c b/gcc/tree-mudflap.c index 90d0448..e20f06e 100644 --- a/gcc/tree-mudflap.c +++ b/gcc/tree-mudflap.c @@ -1335,6 +1335,16 @@ mudflap_finish_file (void) if (! TREE_PUBLIC (obj) && ! TREE_ADDRESSABLE (obj)) continue; + /* If we're neither emitting nor referencing the symbol, + don't register it. We have to register external symbols + if they happen to be in other files not compiled with + mudflap (say system libraries), and we must not register + internal symbols that we don't emit or they'll become + dangling references or force symbols to be emitted that + didn't have to. */ + if (!symtab_get_node (obj)) + continue; + if (! COMPLETE_TYPE_P (TREE_TYPE (obj))) { warning (OPT_Wmudflap, [-- Attachment #3: Type: text/plain, Size: 258 bytes --] -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2012-12-30 0:23 ` Alexandre Oliva @ 2013-01-02 14:53 ` Richard Biener 2013-01-06 19:48 ` Alexandre Oliva 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2013-01-02 14:53 UTC (permalink / raw) To: Alexandre Oliva; +Cc: gcc-patches, Jan Hubicka On Sun, Dec 30, 2012 at 1:22 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Dec 21, 2012, Richard Biener <richard.guenther@gmail.com> 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. > >> Hmm, I think that at this point of the compilation you are looking for >> TREE_ASM_WRITTEN instead. > > That doesn't work, several mudflap regressions show up because accesses > to global library symbols that are accessed by template methods compiled > with mudflap (say cout) are then verified but not registered. We have > to register symbols that are not emitted but that referenced. Ehm, how can something be not emitted but still referenced? You mean if it's external? So maybe if (!TREE_ASM_WRITTEN (obj) || DECL_EXTERNAL (obj)) instead? Thanks, Richard. > I've now updated the comment to reflect this. > > Is this ok to install? Regstrapped again (along with the patches for > feraiseexcept, since there weren't any non-comment changes here) on > x86_64-linux-gnu and i686-linux-gnu. > > > > > -- > 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2013-01-02 14:53 ` Richard Biener @ 2013-01-06 19:48 ` Alexandre Oliva 2013-01-07 9:28 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2013-01-06 19:48 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Jan Hubicka On Jan 2, 2013, Richard Biener <richard.guenther@gmail.com> wrote: > On Sun, Dec 30, 2012 at 1:22 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >> On Dec 21, 2012, Richard Biener <richard.guenther@gmail.com> 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. >> >>> Hmm, I think that at this point of the compilation you are looking for >>> TREE_ASM_WRITTEN instead. >> >> That doesn't work, several mudflap regressions show up because accesses >> to global library symbols that are accessed by template methods compiled >> with mudflap (say cout) are then verified but not registered. We have >> to register symbols that are not emitted but that referenced. > Ehm, how can something be not emitted but still referenced? You mean if > it's external? Yeah. > if (!TREE_ASM_WRITTEN (obj) || DECL_EXTERNAL (obj)) > instead? Then we're back to the original bug. How does the test above tell whether we're actually referencing the object? Only when we are do we want to register it with mudflap. If it's referenced and we don't register it, we get mudflap runtime errors. If it's not referenced but we register it, we get link-time errors if the symbol is one we'd have emitted if it was referenced. -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2013-01-06 19:48 ` Alexandre Oliva @ 2013-01-07 9:28 ` Richard Biener 2013-01-16 9:29 ` Alexandre Oliva 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2013-01-07 9:28 UTC (permalink / raw) To: Alexandre Oliva; +Cc: gcc-patches, Jan Hubicka On Sun, Jan 6, 2013 at 8:47 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Jan 2, 2013, Richard Biener <richard.guenther@gmail.com> wrote: > >> On Sun, Dec 30, 2012 at 1:22 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> On Dec 21, 2012, Richard Biener <richard.guenther@gmail.com> 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. >>> >>>> Hmm, I think that at this point of the compilation you are looking for >>>> TREE_ASM_WRITTEN instead. >>> >>> That doesn't work, several mudflap regressions show up because accesses >>> to global library symbols that are accessed by template methods compiled >>> with mudflap (say cout) are then verified but not registered. We have >>> to register symbols that are not emitted but that referenced. > >> Ehm, how can something be not emitted but still referenced? You mean if >> it's external? > > Yeah. > >> if (!TREE_ASM_WRITTEN (obj) || DECL_EXTERNAL (obj)) > >> instead? > > Then we're back to the original bug. > > How does the test above tell whether we're actually referencing the > object? Only when we are do we want to register it with mudflap. If > it's referenced and we don't register it, we get mudflap runtime errors. > If it's not referenced but we register it, we get link-time errors if > the symbol is one we'd have emitted if it was referenced. Then the bug is that we register something but do not actually tell the middle-end that this is a reference. Hmm. I suppose instead of asking for TREE_ASM_WRITTEN you may want to look at DECL_RTL (which should be set for all referenced decls, whether external or not). 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2013-01-07 9:28 ` Richard Biener @ 2013-01-16 9:29 ` Alexandre Oliva 2013-01-16 10:24 ` Jan Hubicka 0 siblings, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2013-01-16 9:29 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Jan Hubicka On Jan 7, 2013, Richard Biener <richard.guenther@gmail.com> wrote: > On Sun, Jan 6, 2013 at 8:47 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> On Jan 2, 2013, Richard Biener <richard.guenther@gmail.com> wrote: >> >>> On Sun, Dec 30, 2012 at 1:22 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >>>> On Dec 21, 2012, Richard Biener <richard.guenther@gmail.com> 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. >>>> >>>>> Hmm, I think that at this point of the compilation you are looking for >>>>> TREE_ASM_WRITTEN instead. >>>> >>>> That doesn't work, several mudflap regressions show up because accesses >>>> to global library symbols that are accessed by template methods compiled >>>> with mudflap (say cout) are then verified but not registered. We have >>>> to register symbols that are not emitted but that referenced. >> >>> Ehm, how can something be not emitted but still referenced? You mean if >>> it's external? >> >> Yeah. >> >>> if (!TREE_ASM_WRITTEN (obj) || DECL_EXTERNAL (obj)) >> >>> instead? >> >> Then we're back to the original bug. >> >> How does the test above tell whether we're actually referencing the >> object? Only when we are do we want to register it with mudflap. If >> it's referenced and we don't register it, we get mudflap runtime errors. >> If it's not referenced but we register it, we get link-time errors if >> the symbol is one we'd have emitted if it was referenced. > Then the bug is that we register something but do not actually tell the > middle-end that this is a reference. No, the bug is that we're registering something because at an earlier point (when we emitted checks) there were references to it. The references were all optimized away, we correctly decided (elsewhere) the object was not referenced, and we removed it from the symbol table, but mudflap still wants to register it because it pays no attention to that decision. This is the reason why I believe the patch I proposed initially is the correct fix. Now, can you please tell me why you believe this diagnosis is incorrect, or why the fix for the diagnosed problem is incorrect, to the point of proposing alternate (faulty) approaches? > I suppose instead of asking for TREE_ASM_WRITTEN you may want to look > at DECL_RTL (which should be set for all referenced decls, whether > external or not). I'm pretty sure the optimized-away objects that we do NOT want to register had DECL_RTL set, but I'm not exactly excited about double checking it without at least a hint of an explanation on what seems to be wrong with the proposed patch. -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2013-01-16 9:29 ` Alexandre Oliva @ 2013-01-16 10:24 ` Jan Hubicka 2013-01-16 13:17 ` Alexandre Oliva 2013-01-18 11:44 ` Alexandre Oliva 0 siblings, 2 replies; 13+ messages in thread From: Jan Hubicka @ 2013-01-16 10:24 UTC (permalink / raw) To: Alexandre Oliva; +Cc: Richard Biener, gcc-patches, Jan Hubicka > No, the bug is that we're registering something because at an earlier > point (when we emitted checks) there were references to it. The > references were all optimized away, we correctly decided (elsewhere) the > object was not referenced, and we removed it from the symbol table, but > mudflap still wants to register it because it pays no attention to that > decision. > > This is the reason why I believe the patch I proposed initially is the > correct fix. Now, can you please tell me why you believe this diagnosis > is incorrect, or why the fix for the diagnosed problem is incorrect, to > the point of proposing alternate (faulty) approaches? > > > I suppose instead of asking for TREE_ASM_WRITTEN you may want to look > > at DECL_RTL (which should be set for all referenced decls, whether > > external or not). > > I'm pretty sure the optimized-away objects that we do NOT want to > register had DECL_RTL set, but I'm not exactly excited about double > checking it without at least a hint of an explanation on what seems to > be wrong with the proposed patch. Well, from symtab point of view, the early mudflap pass (that is collecting vars it wants to later reffer to) should - either build the references to them early and produce the constructor referencing them early. Then symtab has full information about what is going to be output and everything works well. This is what I slowly did to many parts of compiler, like C++, EH or OBJC interfaces that used to be output late. - of if there is good reason to delay this till end of the compilation it should 1) force them to be output when seeing early so they are not optimized away 2) check if they are optimized away or not. 2) has the obvious advantage that unused vars are not going to be output just for sake of checking code. For 2) the symtab_get_node test seems resonable to me. It is what dwarf2out does, too. We keep nodes for external vars that are refereed but we remove all optimized out nodes. At this point TREE_ASM_WRITTEN should have pretty much same info with two differences 1) for const_decls in constant pool varpool is still not representing things accurately 2) I would like to eventually get rid of TREE_ASM_WRITTEN in favor of test in symtab (in 4.9 I will unify the var/function flags in symtab to make this easier and I will add noes for labels since these are also needed for acurate partitioning. I would like to also eventually get rid of on-the-side constant pool but as explained in the HP PR it is not trivial, given how constant pool is tied to rtl codegen and machine reorg for some targets). So I am not really keen for new uses of this flag appearing. I believe CONST_DECLs are not a concern here. Otherwise I guess TREE_ASM_WRITTEN is good hack for 4.8 modulo the fact htat some FEs still trick with it. Honza ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2013-01-16 10:24 ` Jan Hubicka @ 2013-01-16 13:17 ` Alexandre Oliva 2013-01-16 13:48 ` Richard Biener 2013-01-18 11:44 ` Alexandre Oliva 1 sibling, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2013-01-16 13:17 UTC (permalink / raw) To: Jan Hubicka; +Cc: Richard Biener, gcc-patches On Jan 16, 2013, Jan Hubicka <hubicka@ucw.cz> wrote: > 2) has the obvious advantage that unused vars are not going to be output just > for sake of checking code. For 2) the symtab_get_node test seems resonable to > me. That's what I first implemented, and I still firmly believe symtab_get_node is the correct test. TREE_ASM_WRITTEN doesn't carry the same information as far as external objects are concerned, so we ended up failing to register them when I tried it, and that caused regressions in the testsuite, with tests that were designed to catch precisely this sort of error. -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2013-01-16 13:17 ` Alexandre Oliva @ 2013-01-16 13:48 ` Richard Biener 0 siblings, 0 replies; 13+ messages in thread From: Richard Biener @ 2013-01-16 13:48 UTC (permalink / raw) To: Alexandre Oliva; +Cc: Jan Hubicka, gcc-patches On Wed, Jan 16, 2013 at 2:17 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Jan 16, 2013, Jan Hubicka <hubicka@ucw.cz> wrote: > >> 2) has the obvious advantage that unused vars are not going to be output just >> for sake of checking code. For 2) the symtab_get_node test seems resonable to >> me. > > That's what I first implemented, and I still firmly believe > symtab_get_node is the correct test. TREE_ASM_WRITTEN doesn't carry the > same information as far as external objects are concerned, so we ended > up failing to register them when I tried it, and that caused regressions > in the testsuite, with tests that were designed to catch precisely this > sort of error. As it is mudflap we are talking about I don't care very much ... I'm only not sure that using symtab_get won't regress in any way. 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PR libmudflap/53359] don't register symbols not emitted 2013-01-16 10:24 ` Jan Hubicka 2013-01-16 13:17 ` Alexandre Oliva @ 2013-01-18 11:44 ` Alexandre Oliva 1 sibling, 0 replies; 13+ messages in thread From: Alexandre Oliva @ 2013-01-18 11:44 UTC (permalink / raw) To: Jan Hubicka; +Cc: Richard Biener, gcc-patches [-- Attachment #1: Type: text/plain, Size: 381 bytes --] On Jan 16, 2013, Jan Hubicka <hubicka@ucw.cz> wrote: > For 2) the symtab_get_node test seems resonable to me. It is what > dwarf2out does, too. We keep nodes for external vars that are > refereed but we remove all optimized out nodes. Since you agreed this was the right approach and richi said he didn't care much, I went ahead and installed the patch I'd proposed at first. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: mudflap-global-init-check-symtab-pr53359.patch --] [-- Type: text/x-diff, Size: 1160 bytes --] 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 | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/gcc/tree-mudflap.c b/gcc/tree-mudflap.c index 3c866bc..b250bfe 100644 --- a/gcc/tree-mudflap.c +++ b/gcc/tree-mudflap.c @@ -1334,6 +1334,16 @@ mudflap_finish_file (void) if (! TREE_PUBLIC (obj) && ! TREE_ADDRESSABLE (obj)) continue; + /* If we're neither emitting nor referencing the symbol, + don't register it. We have to register external symbols + if they happen to be in other files not compiled with + mudflap (say system libraries), and we must not register + internal symbols that we don't emit or they'll become + dangling references or force symbols to be emitted that + didn't have to. */ + if (!symtab_get_node (obj)) + continue; + if (! COMPLETE_TYPE_P (TREE_TYPE (obj))) { warning (OPT_Wmudflap, [-- Attachment #3: Type: text/plain, Size: 257 bytes --] -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-01-18 11:44 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-12-21 5:34 [PR libmudflap/53359] don't register symbols not emitted Alexandre Oliva 2012-12-21 9:51 ` Richard Biener 2012-12-21 9:55 ` Jakub Jelinek 2012-12-21 10:42 ` Jan Hubicka 2012-12-30 0:23 ` Alexandre Oliva 2013-01-02 14:53 ` Richard Biener 2013-01-06 19:48 ` Alexandre Oliva 2013-01-07 9:28 ` Richard Biener 2013-01-16 9:29 ` Alexandre Oliva 2013-01-16 10:24 ` Jan Hubicka 2013-01-16 13:17 ` Alexandre Oliva 2013-01-16 13:48 ` Richard Biener 2013-01-18 11:44 ` Alexandre Oliva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).