H.J., This last version of the patch bootstraps and passes the test suite without regressions on x86_64-apple-darwin14.
https://gcc.gnu.org/ml/gcc-testresults/2015-02/msg00912.html Thanks for fixing this. Jack On Sat, Feb 7, 2015 at 11:45 AM, H.J. Lu <hjl.to...@gmail.com> wrote: > On Sat, Feb 07, 2015 at 07:56:06AM -0800, H.J. Lu 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. >> > > Here is an updated patch. > > > H.J. > From d010cd1ddf866f8e10fe7ad2cf483b5a872bc6ea Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.to...@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 renames > default_binds_local_p_1 to default_binds_local_p_2 and implements the > new behavior in default_binds_local_p_2 controlled by a variable. > The old behavior remains with 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_1): Renamed to ... > (default_binds_local_p_2): This. 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_binds_local_p_1): Call 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 | 61 > +++++++++++++++++++++---------- > 13 files changed, 209 insertions(+), 22 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..e4bc2c1 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -6802,17 +6802,8 @@ resolution_local_p (enum ld_plugin_symbol_resolution > resolution) > || resolution == LDPR_RESOLVED_EXEC); > } > > -/* 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); > -} > - > -bool > -default_binds_local_p_1 (const_tree exp, int shlib) > +static bool > +default_binds_local_p_2 (const_tree exp, bool shlib, bool weak_dominate) > { > bool local_p; > bool resolved_locally = false; > @@ -6826,11 +6817,18 @@ default_binds_local_p_1 (const_tree exp, int shlib) > && (TREE_STATIC (exp) || DECL_EXTERNAL (exp))) > { > varpool_node *vnode = varpool_node::get (exp); > - if (vnode && (resolution_local_p (vnode->resolution) || > vnode->in_other_partition)) > - resolved_locally = true; > - if (vnode > - && resolution_to_local_definition_p (vnode->resolution)) > - resolved_to_local_def = true; > + /* If not building shared library, common or initialized symbols > + are also resolved locally, regardless they are weak or not if > + weak_dominate is true. */ > + if (vnode) > + { > + if ((weak_dominate && !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)) > { > @@ -6859,9 +6857,15 @@ default_binds_local_p_1 (const_tree exp, int shlib) > else if (! TREE_PUBLIC (exp)) > local_p = true; > /* A variable is local if the user has said explicitly that it will > - be. */ > + be and it is resolved or defined locally, not compiling for PIC, > + not weak or weak_dominate is false. */ > else if ((DECL_VISIBILITY_SPECIFIED (exp) > || resolved_to_local_def) > + && (!weak_dominate > + || 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. */ > @@ -6881,8 +6885,9 @@ default_binds_local_p_1 (const_tree exp, int shlib) > else if (shlib) > local_p = false; > /* Uninitialized COMMON variable may be unified with symbols > - resolved from other modules. */ > + resolved from other modules unless weak_dominate is true. */ > else if (DECL_COMMON (exp) > + && !weak_dominate > && !resolved_locally > && (DECL_INITIAL (exp) == NULL > || (!in_lto_p && DECL_INITIAL (exp) == error_mark_node))) > @@ -6895,6 +6900,21 @@ default_binds_local_p_1 (const_tree exp, int shlib) > 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_2 (exp, flag_shlib != 0, true); > +} > + > +bool > +default_binds_local_p_1 (const_tree exp, int shlib) > +{ > + return default_binds_local_p_2 (exp, shlib != 0, false); > +} > + > /* Return true when references to DECL must bind to current definition in > final executable. > > @@ -7445,9 +7465,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 >