diff mbox

PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking

Message ID 20150207155606.GA14159@gmail.com
State New
Headers show

Commit Message

H.J. Lu Feb. 7, 2015, 3:56 p.m. UTC
On Sat, Feb 07, 2015 at 10:11:01AM -0500, Jack Howarth wrote:
> H.J,,
>     Unfortunately, the answer is yes. This patch still introduces
> regressions in the g++ test suite.l These are all some form of...
> 

It looks like Darwin depends on the old behavior of
default_binds_local_p_1.  Please try this patch.


H.J.

From 2df2188c97760ed10a85bff3dfef8198e41862f2 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 5 Feb 2015 14:28:58 -0800
Subject: [PATCH] Handle symbol visibility/locality for PIE/PIC

If a hidden weak symbol isn't defined in the TU, we can't assume it will
be defined in another TU at link time.  It makes a difference in code
generation when compiling for PIC. If we assume that a hidden weak
undefined symbol is local, the address checking may be optimized out and
leads to the wrong code.  This means that a symbol with user specified
visibility is local only if it is locally resolved or defined, not weak
or not compiling for PIC.  When symbol visibility is specified in the
source, we should always output symbol visibility even if symbol isn't
local to the TU.

If a global data symbol is defined in the TU, it is always local to the
executable, regardless if it is a common symbol or not.  If we aren't
compiling for shared library, locally defined global data symbol binds
locally.

Since some targets call default_binds_local_p_1 directly and depend on
the old behavior of default_binds_local_p_1.  This patch copies
default_binds_local_p_1 to default_binds_local_p_2 and changes the
behavior of default_binds_local_p by calling default_binds_local_p_2
instead of default_binds_local_p_1.

gcc/

	PR rtl-optimization/32219
	* cgraphunit.c (varpool_node::finalize_decl): Set definition
	first before calling notice_global_symbol so that it is
	available to notice_global_symbol.
	* varasm.c (default_binds_local_p_2): Copied from
	default_binds_local_p_1.  Resolve defined data symbol locally if
	not building shared library.  Resolve symbol with user specified
	visibility locally only if it is locally resolved or defined, not
	weak or not compiling for PIC.
	(default_binds_local_p): Replace default_binds_local_p_1 with
	default_binds_local_p_2.
	(default_elf_asm_output_external): Always output visibility
	specified in the source.

gcc/testsuite/

	PR rtl-optimization/32219
	* gcc.dg/visibility-22.c: New test.
	* gcc.dg/visibility-23.c: Likewise.
	* gcc.target/i386/pr32219-1.c: Likewise.
	* gcc.target/i386/pr32219-2.c: Likewise.
	* gcc.target/i386/pr32219-3.c: Likewise.
	* gcc.target/i386/pr32219-4.c: Likewise.
	* gcc.target/i386/pr32219-5.c: Likewise.
	* gcc.target/i386/pr32219-6.c: Likewise.
	* gcc.target/i386/pr32219-7.c: Likewise.
	* gcc.target/i386/pr32219-8.c: Likewise.
	* gcc.target/i386/pr64317.c: Expect GOTOFF relocation instead
	of GOT relocation.
---
 gcc/cgraphunit.c                          |  4 +-
 gcc/testsuite/gcc.dg/visibility-22.c      | 17 ++++++
 gcc/testsuite/gcc.dg/visibility-23.c      | 15 +++++
 gcc/testsuite/gcc.target/i386/pr32219-1.c | 16 ++++++
 gcc/testsuite/gcc.target/i386/pr32219-2.c | 16 ++++++
 gcc/testsuite/gcc.target/i386/pr32219-3.c | 17 ++++++
 gcc/testsuite/gcc.target/i386/pr32219-4.c | 17 ++++++
 gcc/testsuite/gcc.target/i386/pr32219-5.c | 16 ++++++
 gcc/testsuite/gcc.target/i386/pr32219-6.c | 16 ++++++
 gcc/testsuite/gcc.target/i386/pr32219-7.c | 17 ++++++
 gcc/testsuite/gcc.target/i386/pr32219-8.c | 17 ++++++
 gcc/testsuite/gcc.target/i386/pr64317.c   |  2 +-
 gcc/varasm.c                              | 95 ++++++++++++++++++++++++++++++-
 13 files changed, 260 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/visibility-22.c
 create mode 100644 gcc/testsuite/gcc.dg/visibility-23.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-6.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-7.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-8.c

Comments

Jack Howarth Feb. 7, 2015, 7:37 p.m. UTC | #1
H.J.,
      This one bootstrap and appears to give clean c++ test suite
results so far on x86_64-apple-darwin14. I am stopping the regression
testing to try the updated patch you sent later.
                       Jack

On Sat, Feb 7, 2015 at 10:56 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Feb 07, 2015 at 10:11:01AM -0500, Jack Howarth wrote:
>> H.J,,
>>     Unfortunately, the answer is yes. This patch still introduces
>> regressions in the g++ test suite.l These are all some form of...
>>
>
> It looks like Darwin depends on the old behavior of
> default_binds_local_p_1.  Please try this patch.
>
>
> H.J.
>
> From 2df2188c97760ed10a85bff3dfef8198e41862f2 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 5 Feb 2015 14:28:58 -0800
> Subject: [PATCH] Handle symbol visibility/locality for PIE/PIC
>
> If a hidden weak symbol isn't defined in the TU, we can't assume it will
> be defined in another TU at link time.  It makes a difference in code
> generation when compiling for PIC. If we assume that a hidden weak
> undefined symbol is local, the address checking may be optimized out and
> leads to the wrong code.  This means that a symbol with user specified
> visibility is local only if it is locally resolved or defined, not weak
> or not compiling for PIC.  When symbol visibility is specified in the
> source, we should always output symbol visibility even if symbol isn't
> local to the TU.
>
> If a global data symbol is defined in the TU, it is always local to the
> executable, regardless if it is a common symbol or not.  If we aren't
> compiling for shared library, locally defined global data symbol binds
> locally.
>
> Since some targets call default_binds_local_p_1 directly and depend on
> the old behavior of default_binds_local_p_1.  This patch copies
> default_binds_local_p_1 to default_binds_local_p_2 and changes the
> behavior of default_binds_local_p by calling default_binds_local_p_2
> instead of default_binds_local_p_1.
>
> gcc/
>
>         PR rtl-optimization/32219
>         * cgraphunit.c (varpool_node::finalize_decl): Set definition
>         first before calling notice_global_symbol so that it is
>         available to notice_global_symbol.
>         * varasm.c (default_binds_local_p_2): Copied from
>         default_binds_local_p_1.  Resolve defined data symbol locally if
>         not building shared library.  Resolve symbol with user specified
>         visibility locally only if it is locally resolved or defined, not
>         weak or not compiling for PIC.
>         (default_binds_local_p): Replace default_binds_local_p_1 with
>         default_binds_local_p_2.
>         (default_elf_asm_output_external): Always output visibility
>         specified in the source.
>
> gcc/testsuite/
>
>         PR rtl-optimization/32219
>         * gcc.dg/visibility-22.c: New test.
>         * gcc.dg/visibility-23.c: Likewise.
>         * gcc.target/i386/pr32219-1.c: Likewise.
>         * gcc.target/i386/pr32219-2.c: Likewise.
>         * gcc.target/i386/pr32219-3.c: Likewise.
>         * gcc.target/i386/pr32219-4.c: Likewise.
>         * gcc.target/i386/pr32219-5.c: Likewise.
>         * gcc.target/i386/pr32219-6.c: Likewise.
>         * gcc.target/i386/pr32219-7.c: Likewise.
>         * gcc.target/i386/pr32219-8.c: Likewise.
>         * gcc.target/i386/pr64317.c: Expect GOTOFF relocation instead
>         of GOT relocation.
> ---
>  gcc/cgraphunit.c                          |  4 +-
>  gcc/testsuite/gcc.dg/visibility-22.c      | 17 ++++++
>  gcc/testsuite/gcc.dg/visibility-23.c      | 15 +++++
>  gcc/testsuite/gcc.target/i386/pr32219-1.c | 16 ++++++
>  gcc/testsuite/gcc.target/i386/pr32219-2.c | 16 ++++++
>  gcc/testsuite/gcc.target/i386/pr32219-3.c | 17 ++++++
>  gcc/testsuite/gcc.target/i386/pr32219-4.c | 17 ++++++
>  gcc/testsuite/gcc.target/i386/pr32219-5.c | 16 ++++++
>  gcc/testsuite/gcc.target/i386/pr32219-6.c | 16 ++++++
>  gcc/testsuite/gcc.target/i386/pr32219-7.c | 17 ++++++
>  gcc/testsuite/gcc.target/i386/pr32219-8.c | 17 ++++++
>  gcc/testsuite/gcc.target/i386/pr64317.c   |  2 +-
>  gcc/varasm.c                              | 95 ++++++++++++++++++++++++++++++-
>  13 files changed, 260 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/visibility-22.c
>  create mode 100644 gcc/testsuite/gcc.dg/visibility-23.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-1.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-3.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-4.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-5.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-6.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-7.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-8.c
>
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 35b244e..71367a3 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -792,8 +792,10 @@ varpool_node::finalize_decl (tree decl)
>
>    if (node->definition)
>      return;
> -  notice_global_symbol (decl);
> +  /* Set definition first before calling notice_global_symbol so that
> +     it is available to notice_global_symbol.  */
>    node->definition = true;
> +  notice_global_symbol (decl);
>    if (TREE_THIS_VOLATILE (decl) || DECL_PRESERVE_P (decl)
>        /* Traditionally we do not eliminate static variables when not
>          optimizing and when not doing toplevel reoder.  */
> diff --git a/gcc/testsuite/gcc.dg/visibility-22.c b/gcc/testsuite/gcc.dg/visibility-22.c
> new file mode 100644
> index 0000000..52f59be
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/visibility-22.c
> @@ -0,0 +1,17 @@
> +/* PR target/32219 */
> +/* { dg-do run } */
> +/* { dg-require-visibility "" } */
> +/* { dg-options "-O2 -fPIC" { target fpic } } */
> +/* This test requires support for undefined weak symbols.  This support
> +   is not available on hppa*-*-hpux*.  The test is skipped rather than
> +   xfailed to suppress the warning that would otherwise arise.  */
> +/* { dg-skip-if "" { "hppa*-*-hpux*" "*-*-aix*" "*-*-darwin*" } "*" { "" } }  */
> +
> +extern void foo () __attribute__((weak,visibility("hidden")));
> +int
> +main()
> +{
> +  if (foo)
> +    foo ();
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/visibility-23.c b/gcc/testsuite/gcc.dg/visibility-23.c
> new file mode 100644
> index 0000000..0fa9ef4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/visibility-23.c
> @@ -0,0 +1,15 @@
> +/* PR target/32219 */
> +/* { dg-do compile } */
> +/* { dg-require-visibility "" } */
> +/* { dg-final { scan-hidden "foo" } } */
> +/* { dg-options "-O2 -fPIC" { target fpic } } */
> +/* { dg-skip-if "" { "hppa*-*-hpux*" "*-*-aix*" "*-*-darwin*" } "*" { "" } }  */
> +
> +extern void foo () __attribute__((weak,visibility("hidden")));
> +int
> +main()
> +{
> +  if (foo)
> +    foo ();
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-1.c b/gcc/testsuite/gcc.target/i386/pr32219-1.c
> new file mode 100644
> index 0000000..5bd80a0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-1.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpie" } */
> +
> +/* Common symbol with -fpie.  */
> +int xxx;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-2.c b/gcc/testsuite/gcc.target/i386/pr32219-2.c
> new file mode 100644
> index 0000000..0cf2eb5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-2.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpic" } */
> +
> +/* Common symbol with -fpic.  */
> +int xxx;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-3.c b/gcc/testsuite/gcc.target/i386/pr32219-3.c
> new file mode 100644
> index 0000000..911f2a5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-3.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpie" } */
> +
> +/* Weak common symbol with -fpie.  */
> +__attribute__((weak))
> +int xxx;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-4.c b/gcc/testsuite/gcc.target/i386/pr32219-4.c
> new file mode 100644
> index 0000000..3d43439
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-4.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpic" } */
> +
> +/* Weak common symbol with -fpic.  */
> +__attribute__((weak))
> +int xxx;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-5.c b/gcc/testsuite/gcc.target/i386/pr32219-5.c
> new file mode 100644
> index 0000000..ee7442e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-5.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpie" } */
> +
> +/* Initialized symbol with -fpie.  */
> +int xxx = -1;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-6.c b/gcc/testsuite/gcc.target/i386/pr32219-6.c
> new file mode 100644
> index 0000000..f261433
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-6.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpic" } */
> +
> +/* Initialized symbol with -fpic.  */
> +int xxx = -1;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-7.c b/gcc/testsuite/gcc.target/i386/pr32219-7.c
> new file mode 100644
> index 0000000..12aaf72
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-7.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpie" } */
> +
> +/* Weak initialized symbol with -fpie.  */
> +__attribute__((weak))
> +int xxx = -1;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr32219-8.c b/gcc/testsuite/gcc.target/i386/pr32219-8.c
> new file mode 100644
> index 0000000..2e4fba0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr32219-8.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpic" } */
> +
> +/* Weak initialized symbol with -fpic.  */
> +__attribute__((weak))
> +int xxx = -1;
> +
> +int
> +foo ()
> +{
> +  return xxx;
> +}
> +
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr64317.c b/gcc/testsuite/gcc.target/i386/pr64317.c
> index 33f5b5d..32969fc 100644
> --- a/gcc/testsuite/gcc.target/i386/pr64317.c
> +++ b/gcc/testsuite/gcc.target/i386/pr64317.c
> @@ -1,7 +1,7 @@
>  /* { dg-do compile { target { *-*-linux* && ia32 } } } */
>  /* { dg-options "-O2 -fpie" } */
>  /* { dg-final { scan-assembler "addl\[ \\t\]+\[$\]_GLOBAL_OFFSET_TABLE_, %ebx" } } */
> -/* { dg-final { scan-assembler "movl\[ \\t\]+c@GOT\[(\]%ebx\[)\]" } } */
> +/* { dg-final { scan-assembler "movl\[ \\t\]+c@GOTOFF\[(\]%ebx\[)\]" } } */
>  /* { dg-final { scan-assembler-not "movl\[ \\t\]+\[0-9]+\[(\]%esp\[)\], %ebx" } } */
>  long c;
>
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index eb65b1f..b7ef575 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -6802,13 +6802,101 @@ resolution_local_p (enum ld_plugin_symbol_resolution resolution)
>           || resolution == LDPR_RESOLVED_EXEC);
>  }
>
> +static bool
> +default_binds_local_p_2 (const_tree exp, int shlib)
> +{
> +  bool local_p;
> +  bool resolved_locally = false;
> +  bool resolved_to_local_def = false;
> +
> +  /* With resolution file in hands, take look into resolutions.
> +     We can't just return true for resolved_locally symbols,
> +     because dynamic linking might overwrite symbols
> +     in shared libraries.  */
> +  if (TREE_CODE (exp) == VAR_DECL && TREE_PUBLIC (exp)
> +      && (TREE_STATIC (exp) || DECL_EXTERNAL (exp)))
> +    {
> +      varpool_node *vnode = varpool_node::get (exp);
> +      /* If not building shared library, common or initialized symbols
> +        are also resolved locally, regardless they are weak or not.  */
> +      if (vnode)
> +       {
> +         if ((!shlib && vnode->definition)
> +             || vnode->in_other_partition
> +             || resolution_local_p (vnode->resolution))
> +           resolved_locally = true;
> +         if (resolution_to_local_definition_p (vnode->resolution))
> +           resolved_to_local_def = true;
> +       }
> +    }
> +  else if (TREE_CODE (exp) == FUNCTION_DECL && TREE_PUBLIC (exp))
> +    {
> +      struct cgraph_node *node = cgraph_node::get (exp);
> +      if (node
> +         && (resolution_local_p (node->resolution) || node->in_other_partition))
> +       resolved_locally = true;
> +      if (node
> +         && resolution_to_local_definition_p (node->resolution))
> +       resolved_to_local_def = true;
> +    }
> +
> +  /* A non-decl is an entry in the constant pool.  */
> +  if (!DECL_P (exp))
> +    local_p = true;
> +  /* Weakrefs may not bind locally, even though the weakref itself is always
> +     static and therefore local.  Similarly, the resolver for ifunc functions
> +     might resolve to a non-local function.
> +     FIXME: We can resolve the weakref case more curefuly by looking at the
> +     weakref alias.  */
> +  else if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp))
> +          || (TREE_CODE (exp) == FUNCTION_DECL
> +              && lookup_attribute ("ifunc", DECL_ATTRIBUTES (exp))))
> +    local_p = false;
> +  /* Static variables are always local.  */
> +  else if (! TREE_PUBLIC (exp))
> +    local_p = true;
> +  /* A variable is local if the user has said explicitly that it will
> +     be and it is resolved or defined locally, not compiling for PIC or
> +     not weak.  */
> +  else if ((DECL_VISIBILITY_SPECIFIED (exp)
> +           || resolved_to_local_def)
> +          && (resolved_locally
> +              || !flag_pic
> +              || !DECL_EXTERNAL (exp)
> +              || !DECL_WEAK (exp))
> +          && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
> +    local_p = true;
> +  /* Variables defined outside this object might not be local.  */
> +  else if (DECL_EXTERNAL (exp) && !resolved_locally)
> +    local_p = false;
> +  /* If defined in this object and visibility is not default, must be
> +     local.  */
> +  else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
> +    local_p = true;
> +  /* Default visibility weak data can be overridden by a strong symbol
> +     in another module and so are not local.  */
> +  else if (DECL_WEAK (exp)
> +          && !resolved_locally)
> +    local_p = false;
> +  /* If PIC, then assume that any global name can be overridden by
> +     symbols resolved from other modules.  */
> +  else if (shlib)
> +    local_p = false;
> +  /* Otherwise we're left with initialized (or non-common) global data
> +     which is of necessity defined locally.  */
> +  else
> +    local_p = true;
> +
> +  return local_p;
> +}
> +
>  /* Assume ELF-ish defaults, since that's pretty much the most liberal
>     wrt cross-module name binding.  */
>
>  bool
>  default_binds_local_p (const_tree exp)
>  {
> -  return default_binds_local_p_1 (exp, flag_shlib);
> +  return default_binds_local_p_2 (exp, flag_shlib);
>  }
>
>  bool
> @@ -7445,9 +7533,10 @@ default_elf_asm_output_external (FILE *file ATTRIBUTE_UNUSED,
>  {
>    /* We output the name if and only if TREE_SYMBOL_REFERENCED is
>       set in order to avoid putting out names that are never really
> -     used. */
> +     used.   Always output visibility specified in the source.  */
>    if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl))
> -      && targetm.binds_local_p (decl))
> +      && (DECL_VISIBILITY_SPECIFIED (decl)
> +         || targetm.binds_local_p (decl)))
>      maybe_assemble_visibility (decl);
>  }
>
> --
> 2.1.0
>
diff mbox

Patch

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 35b244e..71367a3 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -792,8 +792,10 @@  varpool_node::finalize_decl (tree decl)
 
   if (node->definition)
     return;
-  notice_global_symbol (decl);
+  /* Set definition first before calling notice_global_symbol so that
+     it is available to notice_global_symbol.  */
   node->definition = true;
+  notice_global_symbol (decl);
   if (TREE_THIS_VOLATILE (decl) || DECL_PRESERVE_P (decl)
       /* Traditionally we do not eliminate static variables when not
 	 optimizing and when not doing toplevel reoder.  */
diff --git a/gcc/testsuite/gcc.dg/visibility-22.c b/gcc/testsuite/gcc.dg/visibility-22.c
new file mode 100644
index 0000000..52f59be
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/visibility-22.c
@@ -0,0 +1,17 @@ 
+/* PR target/32219 */
+/* { dg-do run } */
+/* { dg-require-visibility "" } */
+/* { dg-options "-O2 -fPIC" { target fpic } } */
+/* This test requires support for undefined weak symbols.  This support
+   is not available on hppa*-*-hpux*.  The test is skipped rather than
+   xfailed to suppress the warning that would otherwise arise.  */
+/* { dg-skip-if "" { "hppa*-*-hpux*" "*-*-aix*" "*-*-darwin*" } "*" { "" } }  */
+
+extern void foo () __attribute__((weak,visibility("hidden")));
+int
+main()
+{
+  if (foo)
+    foo ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/visibility-23.c b/gcc/testsuite/gcc.dg/visibility-23.c
new file mode 100644
index 0000000..0fa9ef4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/visibility-23.c
@@ -0,0 +1,15 @@ 
+/* PR target/32219 */
+/* { dg-do compile } */
+/* { dg-require-visibility "" } */
+/* { dg-final { scan-hidden "foo" } } */
+/* { dg-options "-O2 -fPIC" { target fpic } } */
+/* { dg-skip-if "" { "hppa*-*-hpux*" "*-*-aix*" "*-*-darwin*" } "*" { "" } }  */
+
+extern void foo () __attribute__((weak,visibility("hidden")));
+int
+main()
+{
+  if (foo)
+    foo ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-1.c b/gcc/testsuite/gcc.target/i386/pr32219-1.c
new file mode 100644
index 0000000..5bd80a0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-1.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Common symbol with -fpie.  */
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-2.c b/gcc/testsuite/gcc.target/i386/pr32219-2.c
new file mode 100644
index 0000000..0cf2eb5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-2.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Common symbol with -fpic.  */
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-3.c b/gcc/testsuite/gcc.target/i386/pr32219-3.c
new file mode 100644
index 0000000..911f2a5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-3.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Weak common symbol with -fpie.  */
+__attribute__((weak))
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-4.c b/gcc/testsuite/gcc.target/i386/pr32219-4.c
new file mode 100644
index 0000000..3d43439
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-4.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Weak common symbol with -fpic.  */
+__attribute__((weak))
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-5.c b/gcc/testsuite/gcc.target/i386/pr32219-5.c
new file mode 100644
index 0000000..ee7442e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-5.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Initialized symbol with -fpie.  */
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-6.c b/gcc/testsuite/gcc.target/i386/pr32219-6.c
new file mode 100644
index 0000000..f261433
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-6.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Initialized symbol with -fpic.  */
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-7.c b/gcc/testsuite/gcc.target/i386/pr32219-7.c
new file mode 100644
index 0000000..12aaf72
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-7.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Weak initialized symbol with -fpie.  */
+__attribute__((weak))
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-8.c b/gcc/testsuite/gcc.target/i386/pr32219-8.c
new file mode 100644
index 0000000..2e4fba0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-8.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Weak initialized symbol with -fpic.  */
+__attribute__((weak))
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr64317.c b/gcc/testsuite/gcc.target/i386/pr64317.c
index 33f5b5d..32969fc 100644
--- a/gcc/testsuite/gcc.target/i386/pr64317.c
+++ b/gcc/testsuite/gcc.target/i386/pr64317.c
@@ -1,7 +1,7 @@ 
 /* { dg-do compile { target { *-*-linux* && ia32 } } } */
 /* { dg-options "-O2 -fpie" } */
 /* { dg-final { scan-assembler "addl\[ \\t\]+\[$\]_GLOBAL_OFFSET_TABLE_, %ebx" } } */
-/* { dg-final { scan-assembler "movl\[ \\t\]+c@GOT\[(\]%ebx\[)\]" } } */
+/* { dg-final { scan-assembler "movl\[ \\t\]+c@GOTOFF\[(\]%ebx\[)\]" } } */
 /* { dg-final { scan-assembler-not "movl\[ \\t\]+\[0-9]+\[(\]%esp\[)\], %ebx" } } */
 long c;
 
diff --git a/gcc/varasm.c b/gcc/varasm.c
index eb65b1f..b7ef575 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6802,13 +6802,101 @@  resolution_local_p (enum ld_plugin_symbol_resolution resolution)
 	  || resolution == LDPR_RESOLVED_EXEC);
 }
 
+static bool
+default_binds_local_p_2 (const_tree exp, int shlib)
+{
+  bool local_p;
+  bool resolved_locally = false;
+  bool resolved_to_local_def = false;
+
+  /* With resolution file in hands, take look into resolutions.
+     We can't just return true for resolved_locally symbols,
+     because dynamic linking might overwrite symbols
+     in shared libraries.  */
+  if (TREE_CODE (exp) == VAR_DECL && TREE_PUBLIC (exp)
+      && (TREE_STATIC (exp) || DECL_EXTERNAL (exp)))
+    {
+      varpool_node *vnode = varpool_node::get (exp);
+      /* If not building shared library, common or initialized symbols
+	 are also resolved locally, regardless they are weak or not.  */
+      if (vnode)
+	{
+	  if ((!shlib && vnode->definition)
+	      || vnode->in_other_partition
+	      || resolution_local_p (vnode->resolution))
+	    resolved_locally = true;
+	  if (resolution_to_local_definition_p (vnode->resolution))
+	    resolved_to_local_def = true;
+	}
+    }
+  else if (TREE_CODE (exp) == FUNCTION_DECL && TREE_PUBLIC (exp))
+    {
+      struct cgraph_node *node = cgraph_node::get (exp);
+      if (node
+	  && (resolution_local_p (node->resolution) || node->in_other_partition))
+	resolved_locally = true;
+      if (node
+	  && resolution_to_local_definition_p (node->resolution))
+	resolved_to_local_def = true;
+    }
+
+  /* A non-decl is an entry in the constant pool.  */
+  if (!DECL_P (exp))
+    local_p = true;
+  /* Weakrefs may not bind locally, even though the weakref itself is always
+     static and therefore local.  Similarly, the resolver for ifunc functions
+     might resolve to a non-local function.
+     FIXME: We can resolve the weakref case more curefuly by looking at the
+     weakref alias.  */
+  else if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp))
+	   || (TREE_CODE (exp) == FUNCTION_DECL
+	       && lookup_attribute ("ifunc", DECL_ATTRIBUTES (exp))))
+    local_p = false;
+  /* Static variables are always local.  */
+  else if (! TREE_PUBLIC (exp))
+    local_p = true;
+  /* A variable is local if the user has said explicitly that it will
+     be and it is resolved or defined locally, not compiling for PIC or
+     not weak.  */
+  else if ((DECL_VISIBILITY_SPECIFIED (exp)
+	    || resolved_to_local_def)
+	   && (resolved_locally
+	       || !flag_pic
+	       || !DECL_EXTERNAL (exp)
+	       || !DECL_WEAK (exp))
+	   && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
+    local_p = true;
+  /* Variables defined outside this object might not be local.  */
+  else if (DECL_EXTERNAL (exp) && !resolved_locally)
+    local_p = false;
+  /* If defined in this object and visibility is not default, must be
+     local.  */
+  else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
+    local_p = true;
+  /* Default visibility weak data can be overridden by a strong symbol
+     in another module and so are not local.  */
+  else if (DECL_WEAK (exp)
+	   && !resolved_locally)
+    local_p = false;
+  /* If PIC, then assume that any global name can be overridden by
+     symbols resolved from other modules.  */
+  else if (shlib)
+    local_p = false;
+  /* Otherwise we're left with initialized (or non-common) global data
+     which is of necessity defined locally.  */
+  else
+    local_p = true;
+
+  return local_p;
+}
+
 /* Assume ELF-ish defaults, since that's pretty much the most liberal
    wrt cross-module name binding.  */
 
 bool
 default_binds_local_p (const_tree exp)
 {
-  return default_binds_local_p_1 (exp, flag_shlib);
+  return default_binds_local_p_2 (exp, flag_shlib);
 }
 
 bool
@@ -7445,9 +7533,10 @@  default_elf_asm_output_external (FILE *file ATTRIBUTE_UNUSED,
 {
   /* We output the name if and only if TREE_SYMBOL_REFERENCED is
      set in order to avoid putting out names that are never really
-     used. */
+     used.   Always output visibility specified in the source.  */
   if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl))
-      && targetm.binds_local_p (decl))
+      && (DECL_VISIBILITY_SPECIFIED (decl)
+	  || targetm.binds_local_p (decl)))
     maybe_assemble_visibility (decl);
 }