Re: [PATCH] Fix PR59626, _FORTIFY_SOURCE wrappers and LTO

[Richard Biener <rguenther at suse dot de>]

On Fri, 4 Apr 2014, Jan Hubicka wrote:

> Hi,
> here is an updated version of my earlier ipa.c change.  It turns out that the
> problem was that I did not drop always_inline.
> In this version I just drop always_inline attribute on all functions whose body
> is removed.  The patch will affect non-LTO compilation, too, but IMO only by
> making us to not inline&diagnose the cases where indirect call to always inline
> is turned direct in between early opts and inline. Does that seem acceptable?
> (I personally would preffer this over inventing yet another way to special case
> always_inline for LTO only; we never made any strong promises on always_inline
> and indirect calls)

I think it's fine if indirect calls to always-inline fns go to an
offline copy (or cause a link-time error if there is no offline copy).
I've always thought that taking the address of an always_inline fn
should get you the address of an "external" symbol (an inline "clone"
isn't addressable).

So the patch is fine with me.

Thanks,
Richard.

> Honza
> 
> 	* lto-cgraph.c (input_overwrite_node): Check that partitioning
> 	flags are set only during streaming.
> 	* ipa.c (process_references, walk_polymorphic_call_targets,
> 	symtab_remove_unreachable_nodes): Drop bodies of always inline
> 	after early inlining.
> 	(symtab_remove_unreachable_nodes): Remove always_inline attribute.
> 
> 	* gcc.dg/lto/pr59626_0.c: New testcase.
> 	* gcc.dg/lto/pr59626_1.c: New testcase.
> Index: lto-cgraph.c
> ===================================================================
> --- lto-cgraph.c	(revision 209062)
> +++ lto-cgraph.c	(working copy)
> @@ -1001,6 +1001,9 @@ input_overwrite_node (struct lto_file_de
>    node->thunk.thunk_p = bp_unpack_value (bp, 1);
>    node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
>  				     LDPR_NUM_KNOWN);
> +  gcc_assert (flag_ltrans
> +	      || (!node->in_other_partition
> +		  && !node->used_from_other_partition));
>  }
>  
>  /* Return string alias is alias of.  */
> @@ -1169,6 +1172,9 @@ input_varpool_node (struct lto_file_decl
>    node->same_comdat_group = (symtab_node *) (intptr_t) ref;
>    node->resolution = streamer_read_enum (ib, ld_plugin_symbol_resolution,
>  					        LDPR_NUM_KNOWN);
> +  gcc_assert (flag_ltrans
> +	      || (!node->in_other_partition
> +		  && !node->used_from_other_partition));
>  
>    return node;
>  }
> Index: ipa.c
> ===================================================================
> --- ipa.c	(revision 209062)
> +++ ipa.c	(working copy)
> @@ -139,7 +139,10 @@ process_references (struct ipa_ref_list
>  
>        if (node->definition && !node->in_other_partition
>  	  && ((!DECL_EXTERNAL (node->decl) || node->alias)
> -	      || (before_inlining_p
> +	      || (((before_inlining_p
> +		    && (cgraph_state < CGRAPH_STATE_IPA_SSA
> +		        || !lookup_attribute ("always_inline",
> +					      DECL_ATTRIBUTES (node->decl)))))
>  		  /* We use variable constructors during late complation for
>  		     constant folding.  Keep references alive so partitioning
>  		     knows about potential references.  */
> @@ -191,7 +194,10 @@ walk_polymorphic_call_targets (pointer_s
>  	  /* Prior inlining, keep alive bodies of possible targets for
>  	     devirtualization.  */
>  	   if (n->definition
> -	       && before_inlining_p)
> +	       && (before_inlining_p
> +		   && (cgraph_state < CGRAPH_STATE_IPA_SSA
> +		       || !lookup_attribute ("always_inline",
> +					     DECL_ATTRIBUTES (n->decl)))))
>  	     pointer_set_insert (reachable, n);
>  
>  	  /* Even after inlining we want to keep the possible targets in the
> @@ -491,6 +497,12 @@ symtab_remove_unreachable_nodes (bool be
>  	      node->alias = false;
>  	      node->thunk.thunk_p = false;
>  	      node->weakref = false;
> +	      /* After early inlining we drop always_inline attributes on
> +		 bodies of functions that are still referenced (have their
> +		 address taken).  */
> +	      DECL_ATTRIBUTES (node->decl)
> +		= remove_attribute ("always_inline",
> +				    DECL_ATTRIBUTES (node->decl));
>  	      if (!node->in_other_partition)
>  		node->local.local = false;
>  	      cgraph_node_remove_callees (node);
> Index: testsuite/gcc.dg/lto/pr59626_1.c
> ===================================================================
> --- testsuite/gcc.dg/lto/pr59626_1.c	(revision 0)
> +++ testsuite/gcc.dg/lto/pr59626_1.c	(revision 0)
> @@ -0,0 +1,4 @@
> +int bar (int (*fn)(const char *))
> +{
> +  return fn ("0");
> +}
> Index: testsuite/gcc.dg/lto/pr59626_0.c
> ===================================================================
> --- testsuite/gcc.dg/lto/pr59626_0.c	(revision 0)
> +++ testsuite/gcc.dg/lto/pr59626_0.c	(revision 0)
> @@ -0,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);
> +}
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer