Re: PATCH: PR middle-end/20218: Can't use __attribute__ ((visibility ("hidden"))) to hide a symbol

["H. J. Lu" <hjl at lucon dot org>]

On Tue, Dec 05, 2006 at 03:57:04PM -0800, H. J. Lu wrote:
> Here is the updated patch. It removes process_pending_assemble_externals
> call in cgraph_optimize and uses ASM_OUTPUT_EXTERNAL to output hidden
> references or definitions.
> 
> I changed maybe_assemble_visibility to return int so that we only
> generate ".globl" directive for hidden references, which is needed
> for Intel ia64 assembler.
> 
> 

Here is an upated patch. The difference is it removes call to
process_pending_assemble_externals only for ELF targets.


H.J.
----
2006-12-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/17982
	PR middle-end/20218
	* cgraphunit.c (cgraph_optimize): Don't call
	process_pending_assemble_externals for ELF targets.

	* config/elfos.h (ASM_OUTPUT_EXTERNAL): New.

	* config/ia64/hpux.h (TARGET_ASM_FILE_END): Removed.

	* config/ia64/ia64.c (ia64_asm_output_external): Rewritten.
	(ia64_hpux_add_extern_decl): Removed.
	(ia64_hpux_file_end): Likewise.
	(extern_func_list): Likewise.
	(extern_func_head): Likewise.

	* output.h (assemble_external): Update comments.
	(default_elf_asm_output_external): New.
	(maybe_assemble_visibility): New.

	* varasm.c (maybe_assemble_visibility): Make it extern and
	return int.
	(default_elf_asm_output_external): New.

--- gcc/cgraphunit.c.global	2006-12-02 09:32:10.000000000 -0800
+++ gcc/cgraphunit.c	2006-12-06 06:10:58.000000000 -0800
@@ -1536,7 +1536,12 @@ cgraph_optimize (void)
       return;
     }
 
+  /* FIXME: For ELF targets, we want to process undefined symbols later
+     so that we can check if they are referenced or not.  Should we do
+     the same for all targets?  */
+#ifndef OBJECT_FORMAT_ELF
   process_pending_assemble_externals ();
+#endif
 
   /* Frontend may output common variables after the unit has been finalized.
      It is safe to deal with them here as they are always zero initialized.  */
--- gcc/config/elfos.h.global	2006-12-02 09:32:10.000000000 -0800
+++ gcc/config/elfos.h	2006-12-05 15:16:22.000000000 -0800
@@ -499,3 +499,13 @@ Boston, MA 02110-1301, USA.  */
         fprintf ((FILE), "\"\n");					\
     }									\
   while (0)
+
+/* A C statement (sans semicolon) to output to the stdio stream STREAM
+   any text necessary for declaring the name of an external symbol
+   named NAME whch is referenced in this compilation but not defined.
+   It is needed to properly support non-default visibility.  */
+
+#ifndef ASM_OUTPUT_EXTERNAL
+#define ASM_OUTPUT_EXTERNAL(FILE, DECL, NAME) \
+  default_elf_asm_output_external (FILE, DECL, NAME)
+#endif
--- gcc/config/ia64/hpux.h.global	2006-10-27 10:09:07.000000000 -0700
+++ gcc/config/ia64/hpux.h	2006-12-05 15:16:22.000000000 -0800
@@ -144,10 +144,6 @@ do {								\
    definitions, so do not use them in gthr-posix.h.  */
 #define GTHREAD_USE_WEAK 0
 
-/* Put out the needed function declarations at the end.  */
-
-#define TARGET_ASM_FILE_END ia64_hpux_file_end
-
 #undef CTORS_SECTION_ASM_OP
 #define CTORS_SECTION_ASM_OP  "\t.section\t.init_array,\t\"aw\",\"init_array\""
 
--- gcc/config/ia64/ia64.c.global	2006-11-16 09:57:21.000000000 -0800
+++ gcc/config/ia64/ia64.c	2006-12-05 15:16:22.000000000 -0800
@@ -255,10 +255,6 @@ static section *ia64_rwreloc_select_rtx_
 						 unsigned HOST_WIDE_INT)
      ATTRIBUTE_UNUSED;
 static unsigned int ia64_section_type_flags (tree, const char *, int);
-static void ia64_hpux_add_extern_decl (tree decl)
-     ATTRIBUTE_UNUSED;
-static void ia64_hpux_file_end (void)
-     ATTRIBUTE_UNUSED;
 static void ia64_init_libfuncs (void)
      ATTRIBUTE_UNUSED;
 static void ia64_hpux_init_libfuncs (void)
@@ -5021,49 +5017,6 @@ ia64_secondary_reload_class (enum reg_cl
 }
 
 
-/* Emit text to declare externally defined variables and functions, because
-   the Intel assembler does not support undefined externals.  */
-
-void
-ia64_asm_output_external (FILE *file, tree decl, const char *name)
-{
-  int save_referenced;
-
-  /* GNU as does not need anything here, but the HP linker does need
-     something for external functions.  */
-
-  if (TARGET_GNU_AS
-      && (!TARGET_HPUX_LD
-	  || TREE_CODE (decl) != FUNCTION_DECL
-	  || strstr (name, "__builtin_") == name))
-    return;
-
-  /* ??? The Intel assembler creates a reference that needs to be satisfied by
-     the linker when we do this, so we need to be careful not to do this for
-     builtin functions which have no library equivalent.  Unfortunately, we
-     can't tell here whether or not a function will actually be called by
-     expand_expr, so we pull in library functions even if we may not need
-     them later.  */
-  if (! strcmp (name, "__builtin_next_arg")
-      || ! strcmp (name, "alloca")
-      || ! strcmp (name, "__builtin_constant_p")
-      || ! strcmp (name, "__builtin_args_info"))
-    return;
-
-  if (TARGET_HPUX_LD)
-    ia64_hpux_add_extern_decl (decl);
-  else
-    {
-      /* assemble_name will set TREE_SYMBOL_REFERENCED, so we must save and
-         restore it.  */
-      save_referenced = TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl));
-      if (TREE_CODE (decl) == FUNCTION_DECL)
-        ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "function");
-      (*targetm.asm_out.globalize_label) (file, name);
-      TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)) = save_referenced;
-    }
-}
-
 /* Parse the -mfixed-range= option string.  */
 
 static void
@@ -9174,55 +9127,33 @@ ia64_hpux_function_arg_padding (enum mac
    return DEFAULT_FUNCTION_ARG_PADDING (mode, type);
 }
 
-/* Linked list of all external functions that are to be emitted by GCC.
-   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.  */
-
-struct extern_func_list GTY(())
-{
-  struct extern_func_list *next;
-  tree decl;
-};
-
-static GTY(()) struct extern_func_list *extern_func_head;
-
-static void
-ia64_hpux_add_extern_decl (tree decl)
-{
-  struct extern_func_list *p = ggc_alloc (sizeof (struct extern_func_list));
-
-  p->decl = decl;
-  p->next = extern_func_head;
-  extern_func_head = p;
-}
-
-/* Print out the list of used global functions.  */
+/* Emit text to declare externally defined variables and functions, because
+   the Intel assembler does not support undefined externals.  */
 
-static void
-ia64_hpux_file_end (void)
+void
+ia64_asm_output_external (FILE *file, tree decl, const char *name)
 {
-  struct extern_func_list *p;
-
-  for (p = extern_func_head; p; p = p->next)
-    {
-      tree decl = p->decl;
-      tree id = DECL_ASSEMBLER_NAME (decl);
-
-      gcc_assert (id);
-
-      if (!TREE_ASM_WRITTEN (decl) && TREE_SYMBOL_REFERENCED (id))
-        {
-	  const char *name = XSTR (XEXP (DECL_RTL (decl), 0), 0);
-
-	  TREE_ASM_WRITTEN (decl) = 1;
-	  (*targetm.asm_out.globalize_label) (asm_out_file, name);
-	  fputs (TYPE_ASM_OP, asm_out_file);
-	  assemble_name (asm_out_file, name);
-	  fprintf (asm_out_file, "," TYPE_OPERAND_FMT "\n", "function");
-        }
+  /* 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. */
+  if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
+    {
+      /* maybe_assemble_visibility will return 1 if the assembler
+	 visibility directive is outputed.  */
+      int need_visibility = ((*targetm.binds_local_p) (decl)
+			     && maybe_assemble_visibility (decl));
+
+      /* GNU as does not need anything here, but the HP linker does
+	 need something for external functions.  */
+      if ((TARGET_HPUX_LD || !TARGET_GNU_AS)
+	  && TREE_CODE (decl) == FUNCTION_DECL)
+	{
+	  ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "function");
+	  (*targetm.asm_out.globalize_label) (file, name);
+	}
+      else if (need_visibility && !TARGET_GNU_AS)
+	(*targetm.asm_out.globalize_label) (file, name);
     }
-
-  extern_func_head = 0;
 }
 
 /* Set SImode div/mod functions, init_integral_libfuncs only initializes
--- gcc/output.h.global	2006-10-17 07:32:38.000000000 -0700
+++ gcc/output.h	2006-12-05 15:16:22.000000000 -0800
@@ -204,9 +204,9 @@ extern void assemble_variable (tree, int
    DONT_OUTPUT_DATA is from assemble_variable.  */
 extern void align_variable (tree decl, bool dont_output_data);
 
-/* Output something to declare an external symbol to the assembler.
-   (Most assemblers don't need this, so we normally output nothing.)
-   Do nothing if DECL is not external.  */
+/* Queue for outputing something to declare an external symbol to the
+   assembler.  (Most assemblers don't need this, so we normally output
+   nothing.)  Do nothing if DECL is not external.  */
 extern void assemble_external (tree);
 
 /* Assemble code to leave SIZE bytes of zeros.  */
@@ -619,6 +619,10 @@ extern void default_file_start (void);
 extern void file_end_indicate_exec_stack (void);
 extern bool default_valid_pointer_mode (enum machine_mode);
 
+extern void default_elf_asm_output_external (FILE *file, tree,
+					     const char *);
+extern int maybe_assemble_visibility (tree);
+
 extern int default_address_cost (rtx);
 
 /* dbxout helper functions */
--- gcc/varasm.c.global	2006-10-17 07:32:38.000000000 -0700
+++ gcc/varasm.c	2006-12-06 06:05:43.000000000 -0800
@@ -126,7 +126,6 @@ static unsigned HOST_WIDE_INT array_size
 static unsigned min_align (unsigned, unsigned);
 static void output_constructor (tree, unsigned HOST_WIDE_INT, unsigned int);
 static void globalize_decl (tree);
-static void maybe_assemble_visibility (tree);
 #ifdef BSS_SECTION_ASM_OP
 #ifdef ASM_OUTPUT_BSS
 static void asm_output_bss (FILE *, tree, const char *,
@@ -5071,13 +5070,18 @@ default_assemble_visibility (tree decl, 
 
 /* A helper function to call assemble_visibility when needed for a decl.  */
 
-static void
+int
 maybe_assemble_visibility (tree decl)
 {
   enum symbol_visibility vis = DECL_VISIBILITY (decl);
 
   if (vis != VISIBILITY_DEFAULT)
-    targetm.asm_out.visibility (decl, vis);
+    {
+      targetm.asm_out.visibility (decl, vis);
+      return 1;
+    }
+  else
+    return 0;
 }
 
 /* Returns 1 if the target configuration supports defining public symbols
@@ -6226,4 +6230,19 @@ output_object_blocks (void)
   htab_traverse (object_block_htab, output_object_block_htab, NULL);
 }
 
+/* Emit text to declare externally defined symbols. It is needed to
+   properly support non-default visibility.  */
+void
+default_elf_asm_output_external (FILE *file ATTRIBUTE_UNUSED,
+				 tree decl,
+				 const char *name 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. */
+  if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl))
+      && targetm.binds_local_p (decl))
+    maybe_assemble_visibility (decl);
+}
+
 #include "gt-varasm.h"