Re: [lto, testsuite] Don't use visibility on targets that don't support it (PR lto/47334)

[Richard Guenther <rguenther at suse dot de>]

On Tue, 5 Apr 2011, Rainer Orth wrote:

> As described in the PR, it seems to make more sense to avoid to use the
> visibility attribute on targets that don't support it rather than using
> it unconditionally and later (and incompletely) prune the resulting
> warning.
> 
> The following patch does exactly that.  It now needs to document the
> explicit visibility requirement in gcc.dg/lto/20081222_0.c (the main
> file of the testcase, rather than in 20081222_1.c that uses it) so the
> whole testcase is skipped.
> 
> Bootstrapped on mainline without regressions on i386-pc-solaris2.8 with
> Sun as which doesn't support the visibility attribute.
> 
> Ok for mainline and the 4.6 branch after testing?

The testsuite change is definitely ok.  I'm not sure about the
lto.c changes - as we make TU statics global but with hidden
visibility those symbols may clash with other global syms at
link time (a pre-existing problem it seems), now with your
change it also may clash with symbols from shared libs.

Honza, why is this promotion to global ok at all?  I can deliberately
break it by linking in a non-LTO object which clashes with the
mangled symbol.

Thanks,
Richard.

> Thanks.
> 	Rainer
> 
> 
> 2011-03-27  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	gcc/lto:
> 	PR lto/47334)
> 	* lto.c (promote_var): Only set VISIBILITY_HIDDEN if
> 	HAVE_GAS_HIDDEN.
> 	(promote_fn): Likewise.
> 
> 	gcc/testsuite:
> 	PR lto/47334)
> 	* lib/lto.exp (lto_prune_warns): Don't prune visibility warning.
> 	* gcc.dg/lto/20081222_0.c: Add dg-require-visibility.
> 
> diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
> --- a/gcc/lto/lto.c
> +++ b/gcc/lto/lto.c
> @@ -1,5 +1,5 @@
>  /* Top-level LTO routines.
> -   Copyright 2009, 2010 Free Software Foundation, Inc.
> +   Copyright 2009, 2010, 2011 Free Software Foundation, Inc.
>     Contributed by CodeSourcery, Inc.
>  
>  This file is part of GCC.
> @@ -1271,11 +1271,13 @@ promote_var (struct varpool_node *vnode)
>      return false;
>    gcc_assert (flag_wpa);
>    TREE_PUBLIC (vnode->decl) = 1;
> +#ifdef HAVE_GAS_HIDDEN
>    DECL_VISIBILITY (vnode->decl) = VISIBILITY_HIDDEN;
>    DECL_VISIBILITY_SPECIFIED (vnode->decl) = true;
>    if (cgraph_dump_file)
>      fprintf (cgraph_dump_file,
>  	    "Promoting var as hidden: %s\n", varpool_node_name (vnode));
> +#endif
>    return true;
>  }
>  
> @@ -1288,8 +1290,10 @@ promote_fn (struct cgraph_node *node)
>    if (TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl))
>      return false;
>    TREE_PUBLIC (node->decl) = 1;
> +#ifdef HAVE_GAS_HIDDEN
>    DECL_VISIBILITY (node->decl) = VISIBILITY_HIDDEN;
>    DECL_VISIBILITY_SPECIFIED (node->decl) = true;
> +#endif
>    if (node->same_body)
>      {
>        struct cgraph_node *alias;
> @@ -1297,14 +1301,18 @@ promote_fn (struct cgraph_node *node)
>  	   alias; alias = alias->next)
>  	{
>  	  TREE_PUBLIC (alias->decl) = 1;
> +#ifdef HAVE_GAS_HIDDEN
>  	  DECL_VISIBILITY (alias->decl) = VISIBILITY_HIDDEN;
>  	  DECL_VISIBILITY_SPECIFIED (alias->decl) = true;
> +#endif
>  	}
>      }
> +#ifdef HAVE_GAS_HIDDEN
>    if (cgraph_dump_file)
>      fprintf (cgraph_dump_file,
>  	     "Promoting function as hidden: %s/%i\n",
>  	     cgraph_node_name (node), node->uid);
> +#endif
>    return true;
>  }
>  
> diff --git a/gcc/testsuite/gcc.dg/lto/20081222_0.c b/gcc/testsuite/gcc.dg/lto/20081222_0.c
> --- a/gcc/testsuite/gcc.dg/lto/20081222_0.c
> +++ b/gcc/testsuite/gcc.dg/lto/20081222_0.c
> @@ -1,4 +1,6 @@
>  /* { dg-require-alias "" } */
> +/* { dg-require-visibility "" } */
> +
>  #include "20081222_0.h"
>  
>  extern void abort (void);
> diff --git a/gcc/testsuite/lib/lto.exp b/gcc/testsuite/lib/lto.exp
> --- a/gcc/testsuite/lib/lto.exp
> +++ b/gcc/testsuite/lib/lto.exp
> @@ -1,4 +1,4 @@
> -# Copyright (C) 2009, 2010 Free Software Foundation, Inc.
> +# Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
>  
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -22,14 +22,6 @@ proc lto_prune_warns { text } {
>  
>      verbose "lto_prune_warns: entry: $text" 2
>  
> -    # Many tests that use visibility will still pass on platforms that don't support it.
> -    regsub -all "(^|\n)\[^\n\]*: warning: visibility attribute not supported in this configuration; ignored\[^\n\]*" $text "" text
> -
> -    # And any stray location lines.
> -    regsub -all "(^|\n)\[^\n\]*: In function \[^\n\]*" $text "" text
> -    regsub -all "(^|\n)In file included from \[^\n\]*" $text "" text
> -    regsub -all "(^|\n)\[ \t\]*from \[^\n\]*" $text "" text
> -
>      # Sun ld warns about common symbols with differing sizes.  Unlike GNU ld
>      # --warn-common (off by default), they cannot be disabled.
>      regsub -all "(^|\n)ld: warning: symbol \[`'\]\[^\n\]*' has differing sizes:" $text "" text
> 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex