On Mon, 24 Mar 2014, Jan Hubicka wrote:
> >
> > Currently a _lot_ of packages fail to build with LTO because LTO
> > messes up fortify wrappers by replacing the call to the alias with
> > the symbol itself, making the wrapper look like infinitely
> > recursing.
> >
> > The following patch fixes this by dropping the bodies of
> > DECL_EXTERN always-inline functions on the floor before IPA
> > (and when doing LTO). IMHO we should at some point do this
> > unconditional on LTO as the early inliner is supposed to
> > remove all references to always-inline functions.
> >
> > I'm not 100% sure about the cgraph API use to drop the function
> > body, but at least it seems to work ;) I'm not sure if we want
> > to restrict the set of functions to apply this even more than
> > just those with always-inline and DECL_EXTERNAL set?
> >
> > Now double-checking with a fortified LTO bootstrap.
> >
> > Ok for trunk?
> >
> > Thanks,
> > Richard.
> >
> > 2014-03-24 Richard Biener <rguent...@suse.de>
> >
> > PR lto/59626
> > * passes.c (ipa_write_summaries): Drop function bodies of
> > extern always-inline functions.
>
> I think it is probably better to drop these during unreachable function
> removal same way as we drop extern inlines after inlining (just use
> cgraph_state check to see if we are past early inlining). I will make
> patch this afternoon.
Ah, I didn't know we do that already. But note that these inlines
are extern inlines as well, so it doesn't seem to work ... though
I can't identify the code doing that in symtab_remove_unreachable_nodes.
> Why do you need to remove the always_inline attribute? And how do I do
> fortified LTO bootstrap? :)
If I don't remove the always_inline attribute I still get errors from
tree-inline.c:4154 (there _is_ a caller to the function, it's just
an indirect one discvered during LTO only).
Ideally we'd have the address of an always-inline function resolve
to an offline body _not_ marked always-inline. The same way as we'd
ideally for
int __atoi (const char *) __asm__("atoi");
extern inline __attribute__((always_inline,gnu_inline))
int atoi (const char *x)
{
return __atoi (x);
}
have the cgraph edge(!) from atoi to __atoi _not_ marked as
to always inline. The alias decl after all does _not_ have
the always-inline attribute.
Thus the problems that arise with LTO are solely due to us
smashing all aliases to a single prevailing node.
Getting rid of the inline body before LTO out is the easiest way
out for this common use (and we should be able to throw away all
always-inline bodies after early inlining anyway).
To do a fortified LTO bootstrap you set BOOT_CFLAGS="-O2 -g
-D_FORTIFY_SOURCE=2".
Richard.
> Honza
> >
> > * gcc.dg/lto/pr59626_0.c: New testcase.
> > * gcc.dg/lto/pr59626_1.c: Likewise.
> >
> > Index: gcc/passes.c
> > ===================================================================
> > *** gcc/passes.c (revision 208745)
> > --- gcc/passes.c (working copy)
> > *************** ipa_write_summaries (void)
> > *** 2390,2404 ****
> >
> > if (cgraph_function_with_gimple_body_p (node))
> > {
> > ! /* When streaming out references to statements as part of some IPA
> > ! pass summary, the statements need to have uids assigned and the
> > ! following does that for all the IPA passes here. Naturally, this
> > ! ordering then matches the one IPA-passes get in their stmt_fixup
> > ! hooks. */
> > !
> > ! push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> > ! renumber_gimple_stmt_uids ();
> > ! pop_cfun ();
> > }
> > if (node->definition)
> > lto_set_symtab_encoder_in_partition (encoder, node);
> > --- 2396,2427 ----
> >
> > if (cgraph_function_with_gimple_body_p (node))
> > {
> > ! if (DECL_EXTERNAL (node->decl)
> > ! && lookup_attribute ("always_inline",
> > ! DECL_ATTRIBUTES (node->decl)) != NULL)
> > ! {
> > ! /* We mess up uses of extern always-inline wrappers that
> > ! end up calling an alias to itself like glibc _FORTIFY_SOURCE
> > ! wrappers. Simply drop the bodies of the extern inlines
> > ! here to avoid that. */
> > ! cgraph_release_function_body (node);
> > ! node->analyzed = false;
> > ! node->definition = false;
> > ! DECL_ATTRIBUTES (node->decl)
> > ! = remove_attribute ("always_inline",
> > ! DECL_ATTRIBUTES (node->decl));
> > ! }
> > ! else
> > ! {
> > ! /* When streaming out references to statements as part of some
> > ! IPA pass summary, the statements need to have uids assigned
> > ! and the following does that for all the IPA passes here.
> > ! Naturally, this ordering then matches the one IPA-passes get
> > ! in their stmt_fixup hooks. */
> > ! push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> > ! renumber_gimple_stmt_uids ();
> > ! pop_cfun ();
> > ! }
> > }
> > if (node->definition)
> > lto_set_symtab_encoder_in_partition (encoder, node);
> > Index: gcc/testsuite/gcc.dg/lto/pr59626_0.c
> > ===================================================================
> > *** gcc/testsuite/gcc.dg/lto/pr59626_0.c (revision 0)
> > --- gcc/testsuite/gcc.dg/lto/pr59626_0.c (working copy)
> > ***************
> > *** 0 ****
> > --- 1,15 ----
> > + /* { dg-lto-do run } */
> > +
> > + int __atoi (const char *) __asm__("atoi");
> > + extern inline __attribute__((always_inline,gnu_inline))
> > + int atoi (const char *x)
> > + {
> > + return __atoi (x);
> > + }
> > +
> > + int bar (int (*)(const char *));
> > +
> > + int main()
> > + {
> > + return bar (atoi);
> > + }
> > Index: gcc/testsuite/gcc.dg/lto/pr59626_1.c
> > ===================================================================
> > *** gcc/testsuite/gcc.dg/lto/pr59626_1.c (revision 0)
> > --- gcc/testsuite/gcc.dg/lto/pr59626_1.c (working copy)
> > ***************
> > *** 0 ****
> > --- 1,4 ----
> > + int bar (int (*fn)(const char *))
> > + {
> > + return fn ("0");
> > + }
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
