re PR rtl-optimization/32219 (optimizer causes wrong code in pic/hidden/weak symbol checking.)

PR rtl/32219

gcc/
	* cgraphunit.c (cgraph_node::finalize_function): Set definition
	before notice_global_symbol.
	(varpool_node::finalize_decl): Likewise.
	* varasm.c (default_binds_local_p_2): Rename from
	default_binds_local_p_1, add weak_dominate argument.  Use direct
	returns instead of assigning to local variable.  Unify varpool and
	cgraph paths via symtab_node.  Reject undef weak variables before
	testing visibility.  Reorder tests for simplicity.
	(default_binds_local_p): Use default_binds_local_p_2.
	(default_binds_local_p_1): Likewise.
	(decl_binds_to_current_def_p): Unify varpool and cgraph paths
	via symtab_node.
	(default_elf_asm_output_external): Emit visibility when specified.
gcc/testsuite/
	* gcc.dg/visibility-22.c: New test.
	* gcc.dg/visibility-23.c: New test.
	* gcc.target/i386/pr32219-1.c: New test.
	* gcc.target/i386/pr32219-2.c: New test.
	* gcc.target/i386/pr32219-3.c: New test.
	* gcc.target/i386/pr32219-4.c: New test.
	* gcc.target/i386/pr32219-5.c: New test.
	* gcc.target/i386/pr32219-6.c: New test.
	* gcc.target/i386/pr32219-7.c: New test.
	* gcc.target/i386/pr32219-8.c: New test.
	* gcc.target/i386/pr64317.c: Expect GOTOFF, not GOT.

Co-Authored-By: Richard Henderson <rth@redhat.com>

From-SVN: r220674

gcc/ChangeLog

+2015-02-12  H.J. Lu  <hongjiu.lu@intel.com>
+	    Richard Henderson  <rth@redhat.com>
+
+	PR rtl/32219
+	* cgraphunit.c (cgraph_node::finalize_function): Set definition
+	before notice_global_symbol.
+	(varpool_node::finalize_decl): Likewise.
+	* varasm.c (default_binds_local_p_2): Rename from
+	default_binds_local_p_1, add weak_dominate argument.  Use direct
+	returns instead of assigning to local variable.  Unify varpool and
+	cgraph paths via symtab_node.  Reject undef weak variables before
+	testing visibility.  Reorder tests for simplicity.
+	(default_binds_local_p): Use default_binds_local_p_2.
+	(default_binds_local_p_1): Likewise.
+	(decl_binds_to_current_def_p): Unify varpool and cgraph paths
+	via symtab_node.
+	(default_elf_asm_output_external): Emit visibility when specified.
+
 2015-02-13  Alan Modra  <amodra@gmail.com>
 
 	* config/rs6000/rs6000.c (rs6000_emit_epilogue): Fix typo in

gcc/cgraphunit.c

@@ -442,8 +442,10 @@ cgraph_node::finalize_function (tree decl, bool no_collect)
       node->local.redefined_extern_inline = true;
     }
 
-  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);
   node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
 
   /* With -fkeep-inline-functions we are keeping all inline functions except
@@ -803,8 +805,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.  */

gcc/testsuite/ChangeLog

+2015-02-12  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR rtl/32219
+	* gcc.dg/visibility-22.c: New test.
+	* gcc.dg/visibility-23.c: New test.
+	* gcc.target/i386/pr32219-1.c: New test.
+	* gcc.target/i386/pr32219-2.c: New test.
+	* gcc.target/i386/pr32219-3.c: New test.
+	* gcc.target/i386/pr32219-4.c: New test.
+	* gcc.target/i386/pr32219-5.c: New test.
+	* gcc.target/i386/pr32219-6.c: New test.
+	* gcc.target/i386/pr32219-7.c: New test.
+	* gcc.target/i386/pr32219-8.c: New test.
+	* gcc.target/i386/pr64317.c: Expect GOTOFF, not GOT.
+
 2015-02-12  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
 
 	PR libgfortran/57822

gcc/testsuite/gcc.dg/visibility-22.c

+/* 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;
+}

gcc/testsuite/gcc.dg/visibility-23.c

+/* 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;
+}

gcc/testsuite/gcc.target/i386/pr32219-1.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr32219-2.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr32219-3.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr32219-4.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr32219-5.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr32219-6.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr32219-7.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr32219-8.c

+/* { 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 } } } */

gcc/testsuite/gcc.target/i386/pr64317.c

 /* { 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;
 

gcc/varasm.c

@@ -6802,97 +6802,96 @@ 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;
-  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 (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;
-    }
-  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;
+    return 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))
+  if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp))
 	   || (TREE_CODE (exp) == FUNCTION_DECL
 	       && lookup_attribute ("ifunc", DECL_ATTRIBUTES (exp))))
-    local_p = false;
+    return 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.  */
-  else if ((DECL_VISIBILITY_SPECIFIED (exp)
-	    || resolved_to_local_def)
-	   && 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 (! TREE_PUBLIC (exp))
+    return true;
+
+  /* With resolution file in hand, take look into resolutions.
+     We can't just return true for resolved_locally symbols,
+     because dynamic linking might overwrite symbols
+     in shared libraries.  */
+  bool resolved_locally = false;
+  bool defined_locally = false;
+  if (symtab_node *node = symtab_node::get (exp))
+    {
+      if (node->definition || node->in_other_partition)
+	{
+	  defined_locally = true;
+	  resolved_locally = (weak_dominate && !shlib);
+	}
+      if (resolution_to_local_definition_p (node->resolution))
+	defined_locally = resolved_locally = true;
+      else if (resolution_local_p (node->resolution))
+	resolved_locally = true;
+    }
+
+  /* Undefined weak symbols are never defined locally.  */
+  if (DECL_WEAK (exp) && !defined_locally)
+    return false;
+
+  /* A symbol is local if the user has said explicitly that it will be,
+     or if we have a definition for the symbol.  We cannot infer visibility
+     for undefined symbols.  */
+  if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT
+      && (DECL_VISIBILITY_SPECIFIED (exp) || defined_locally))
+    return true;
+
   /* If PIC, then assume that any global name can be overridden by
      symbols resolved from other modules.  */
-  else if (shlib)
-    local_p = false;
+  if (shlib)
+    return false;
+
+  /* Variables defined outside this object might not be local.  */
+  if (DECL_EXTERNAL (exp) && !resolved_locally)
+    return false;
+
+  /* Non-dominant weak symbols are not defined locally.  */
+  if (DECL_WEAK (exp) && !resolved_locally)
+    return false;
+
   /* Uninitialized COMMON variable may be unified with symbols
      resolved from other modules.  */
-  else if (DECL_COMMON (exp)
-	   && !resolved_locally
-	   && (DECL_INITIAL (exp) == NULL
-	       || (!in_lto_p && DECL_INITIAL (exp) == error_mark_node)))
-    local_p = false;
+  if (DECL_COMMON (exp)
+      && !resolved_locally
+      && (DECL_INITIAL (exp) == NULL
+	  || (!in_lto_p && DECL_INITIAL (exp) == error_mark_node)))
+    return false;
+
   /* Otherwise we're left with initialized (or non-common) global data
      which is of necessity defined locally.  */
-  else
-    local_p = true;
+  return true;
+}
+
+/* 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);
+}
 
-  return local_p;
+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
@@ -6914,22 +6913,14 @@ decl_binds_to_current_def_p (const_tree decl)
     return false;
   if (!TREE_PUBLIC (decl))
     return true;
+
   /* When resolution is available, just use it.  */
-  if (TREE_CODE (decl) == VAR_DECL
-      && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
+  if (symtab_node *node = symtab_node::get (decl))
     {
-      varpool_node *vnode = varpool_node::get (decl);
-      if (vnode
-	  && vnode->resolution != LDPR_UNKNOWN)
-	return resolution_to_local_definition_p (vnode->resolution);
-    }
-  else if (TREE_CODE (decl) == FUNCTION_DECL)
-    {
-      struct cgraph_node *node = cgraph_node::get (decl);
-      if (node
-	  && node->resolution != LDPR_UNKNOWN)
+      if (node->resolution != LDPR_UNKNOWN)
 	return resolution_to_local_definition_p (node->resolution);
     }
+
   /* Otherwise we have to assume the worst for DECL_WEAK (hidden weaks
      binds locally but still can be overwritten), DECL_COMMON (can be merged
      with a non-common definition somewhere in the same module) or
@@ -7449,9 +7440,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);
 }