Bug 28744 - externally_visible attribute not effective with prior declaration of symbol.
externally_visible attribute not effective with prior declaration of symbol.
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: c
4.1.1
: P3 normal
: ---
Assigned To: Jan Hubicka
http://gcc.gnu.org/ml/gcc-patches/200...
:
Depends on:
Blocks: 28779
  Show dependency treegraph
 
Reported: 2006-08-15 20:01 UTC by David Woodhouse
Modified: 2012-03-24 22:07 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-08-15 20:06:09


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Woodhouse 2006-08-15 20:01:41 UTC
$ cat foo.c
extern char *bar1 __attribute__((externally_visible));
char *bar1;

extern char *bar2 __attribute__((externally_visible));
char *bar2 __attribute__((externally_visible));

extern char *bar3;
char *bar3 __attribute__((externally_visible));

char *bar4 __attribute__((externally_visible));

char *bar5;
extern char *bar5 __attribute__((externally_visible));
 $ gcc -O -fwhole-program -c -o foo.o foo.c
foo.c:1: warning: ‘externally_visible’ attribute have effect only on public objects
foo.c:4: warning: ‘externally_visible’ attribute have effect only on public objects
foo.c:13: warning: ‘externally_visible’ attribute have effect only on public objects
 $ nm -a foo.o | grep bar
00000004 C bar4

All five of these variables ought to be externally visible.
Note -fwhole-program.

Seen with:
(ppc) gcc version 4.1.1 20060525 (Red Hat 4.1.1-1.pr27898)
(ppc->i686) gcc version 4.1.0
Comment 1 Andrew Pinski 2006-08-15 21:04:24 UTC
I think this is really a dup of bug 25795 which was fixed for 4.2.0.
Comment 2 Andrew Pinski 2006-08-15 21:08:29 UTC
And yes this is a dup of bug 25795.

*** This bug has been marked as a duplicate of 25795 ***
Comment 3 Jakub Jelinek 2006-08-15 21:18:35 UTC
It is only partly a dup, handle_externally_visible_attribute still needs fixing,
as the source below is IMHO valid and shouldn't produce any warnings (and all
5 barN's should be externally visible, not just bet[2-4] as with the trunk ATM).
Comment 4 Jakub Jelinek 2006-08-16 10:23:09 UTC
Reopening, as the remaining part needs to be fixed too.
Comment 5 Jakub Jelinek 2006-08-16 13:17:32 UTC
Another patch in http://gcc.gnu.org/ml/gcc-patches/2006-08/msg00537.html
Comment 6 David Woodhouse 2006-08-16 18:11:43 UTC
I don't think it's working right even with the new patch. I'll verify tomorrow, and make sure I have all the correct patches included in my build. The point at which I copied my ppc-cross-i386 'cc1' into /usr/libexec/gcc/ppc64-redhat-linux/4.1.1/cc1 and then wondered why native GCC wouldn't work any more was probably the point at which I should stop working on this for today.

My hack at http://david.woodhou.se/pr27844.patch is working around the problem for now, so I have least been able to link a kernel.

I'm building a Linux kernel with http://david.woodhou.se/linux-combine-build.patch applied. It shouldn't matter much, but the git tree it's applied to is git://git.infradead.org/~jcrouse/geode.git and the config is http://david.woodhou.se/olpc-config

The failure mode with GCC 4.1.0, the patch from http://gcc.gnu.org/ml/gcc-patches/2006-07/msg00993.html and also the patch in comment #5 is that some functions aren't available in the final link:

kernel/built-in.o: In function `register_irq_proc':
/pmac/git/geode/kernel/irq/proc.c:128: undefined reference to `proc_mkdir'
Comment 7 David Woodhouse 2006-08-17 09:16:12 UTC
The one with proc_mkdir was because the EXPORT_SYMBOL is in a different file to the original function -- although my version was working correctly, I'm willing to deal with that case.

The symbol 'proc_root' is also missing though. Simple test case:

 $ cat foo.c
extern int foo;

int __attribute__((externally_visible)) *bar(void)
{
        return &foo;
}
pmac /home/dwmw2 $ cat bar.c

int foo __attribute__((externally_visible)) = 42;
 $ gcc -fwhole-program --combine -Os -c -ofoo.o foo.c bar.c
 $ nm -a foo.o 00000000 T bar 00000000 b .bss
00000000 n .comment
00000000 d .data
00000000 d foo.1277
00000000 a foo.c
00000000 n .note.GNU-stack
00000000 t .text

Note 'd foo.1277' instead of the expected 'D foo'. Giving foo.c and bar.c in the opposite order on the command line 'fixes' this.
Comment 8 Jakub Jelinek 2006-08-17 11:52:36 UTC
Subject: Bug 28744

Author: jakub
Date: Thu Aug 17 11:52:26 2006
New Revision: 116222

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116222
Log:
	PR c/28744
	* cgraph.h (struct cgraph_node): Remove externally_visible
	bitfield.
	* cgraphunit.c (process_function_and_variable_attributes): Set
	local.externally_visible rather than externally_visible.

	PR c/28744
	* c-common.c (handle_externally_visible_attribute): First look
	at TREE_CODE and only if it is function or var decl, check for
	non-public objects.  Don't warn for DECL_EXTERNAL.
	* cgraphunit.c (process_function_and_variable_attributes): Warn
	if externally_visible attribute is used on non-public object.

	* gcc.dg/attr-externally-visible-1.c: New test.
	* gcc.dg/attr-externally-visible-2.c: New test.
	* g++.dg/parse/attr-externally-visible-1.C: New test.
	* g++.dg/parse/attr-externally-visible-2.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/parse/attr-externally-visible-1.C
    trunk/gcc/testsuite/g++.dg/parse/attr-externally-visible-2.C
    trunk/gcc/testsuite/gcc.dg/attr-externally-visible-1.c
    trunk/gcc/testsuite/gcc.dg/attr-externally-visible-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-common.c
    trunk/gcc/cgraph.h
    trunk/gcc/cgraphunit.c
    trunk/gcc/testsuite/ChangeLog

Comment 9 Jakub Jelinek 2006-08-17 12:20:22 UTC
The problem mentioned in #7 is not specific to variables, foo.c:
extern int foo (void);

__attribute__((externally_visible)) void *bar(void)
{
  return foo;
}
bar.c:
__attribute__((externally_visible)) int foo (void)
{
}

results in foo not being externally visible in the end.
The problem seems to be on the process_function_and_variable_attributes side,
it only analyzes newly added cgraph_nodes and cgraph_varpool_nodes.  But, if a prior TU already created such nodes (just DECL_EXTERNALs), then even if they
were defined in the current TU, process_function_and_variable_attributes won't
see them as new.
Always walking the whole pools might be too expensive on the other side.
Is there any reason why process_function_and_variable_attributes is called
at the end of each TU rather than when all TUs were already parsed?
Comment 10 David Woodhouse 2006-08-18 11:11:57 UTC
I've hacked around this for now by reverting the patch for PR25795 and doing this instead:

--- gcc/c-decl.c~	2006-01-19 23:48:07.000000000 +0000
+++ gcc/c-decl.c	2006-08-15 21:43:43.000000000 +0100
@@ -1660,6 +1660,25 @@ merge_decls (tree newdecl, tree olddecl,
   if (TREE_DEPRECATED (newdecl))
     TREE_DEPRECATED (olddecl) = 1;
 
+  if (TREE_CODE (newdecl) == FUNCTION_DECL)
+    {
+      struct cgraph_node *n = cgraph_node (newdecl);
+      if (n->local.externally_visible)
+	{
+	   struct cgraph_node *o = cgraph_node (olddecl);
+	   o->local.externally_visible = true;
+	}
+    }
+  else if (TREE_CODE (newdecl) == VAR_DECL)
+    {
+      struct cgraph_varpool_node *n = cgraph_varpool_node (newdecl);
+      if (n->externally_visible)
+	{
+	  struct cgraph_varpool_node *o = cgraph_varpool_node (olddecl);
+	  o->externally_visible = true;
+	}
+    }
+
   /* Keep source location of definition rather than declaration and of
      prototype rather than non-prototype unless that prototype is
      built-in.  */
--- gcc/c-common.c~	2006-08-18 10:35:10.000000000 +0100
+++ gcc/c-common.c	2006-08-18 10:52:18.000000000 +0100
@@ -4243,7 +4243,7 @@ handle_externally_visible_attribute (tre
 {
   tree node = *pnode;
 
-  if ((!TREE_STATIC (node) && TREE_CODE (node) != FUNCTION_DECL)
+  if (0 && (!TREE_STATIC (node) && TREE_CODE (node) != FUNCTION_DECL)
       || !TREE_PUBLIC (node))
     {
       warning (OPT_Wattributes,
Comment 11 Jan Hubicka 2006-08-20 12:42:32 UTC
Subject: Re:  externally_visible attribute not effective with prior declaration of symbol.

> Is there any reason why process_function_and_variable_attributes is called
> at the end of each TU rather than when all TUs were already parsed?

The reason is that we do unreachable function removal after each unit
(to conserve memory) and for that we do need to process USED attributes
(not really the externally_visible as those are used only in
cgraph_optimize).  Keeping handling of USED attributes at TU basis,
while moving externally_visible to global basis would not completely
work, since USED attributes in whole program mode can be used for public
variables too, pretty much as externally_visible is used in the
testcase.

I guess only solution is to process all TU local objects at the end of
each TU and all global objects at the cgraph_optimize stage.  I will
post the patch for this.

Thank you for looking into those dead ends!
Honza
Comment 12 Bernhard Reutner-Fischer 2006-08-20 12:53:17 UTC
(In reply to comment #11)
> Subject: Re:  externally_visible attribute not effective with prior declaration
> of symbol.
> 
> > Is there any reason why process_function_and_variable_attributes is called
> > at the end of each TU rather than when all TUs were already parsed?
> 
> The reason is that we do unreachable function removal after each unit
> (to conserve memory) and for that we do need to process USED attributes
> (not really the externally_visible as those are used only in
> cgraph_optimize).  Keeping handling of USED attributes at TU basis,
> while moving externally_visible to global basis would not completely
> work, since USED attributes in whole program mode can be used for public
> variables too, pretty much as externally_visible is used in the
> testcase.

If this is really true, then there are several bugs (in the FEs?) because there
are numerous occurances where referenced_vars_insert() is called with
TREE_USED(to) == 0

Should there be an assertion that only TREE_USED() > 0 are valid targets for
insertion in/after dfa?
> 
> I guess only solution is to process all TU local objects at the end of
> each TU and all global objects at the cgraph_optimize stage.  I will
> post the patch for this.
> 
> Thank you for looking into those dead ends!
> Honza
> 
Comment 13 Jan Hubicka 2006-08-20 12:59:00 UTC
Subject: Re:  externally_visible attribute not effective with prior declaration of symbol.

> 
> If this is really true, then there are several bugs (in the FEs?) because there
> are numerous occurances where referenced_vars_insert() is called with
> TREE_USED(to) == 0
> 
> Should there be an assertion that only TREE_USED() > 0 are valid targets for
> insertion in/after dfa?

I am not quite convinced there is necesarily a problem, since from
frontend point of view all public variables are automatically used, so
the whole thing matters only for the cgraph code were we start to
differentiate -fwhole-program mode from non-whole-program...

Honza
Comment 14 Jan Hubicka 2006-08-20 13:12:07 UTC
Subject: Re:  externally_visible attribute not effective with prior declaration of symbol.

Hi,
this is patch I am testing now.  Can you think of way to break it
(again? :))
The whole thing is a lot more sliperly than I would like it to be...

Honza

Index: cgraphunit.c
===================================================================
*** cgraphunit.c	(revision 116257)
--- cgraphunit.c	(working copy)
*************** cgraph_analyze_function (struct cgraph_n
*** 965,975 ****
     is valid.
  
     So, we walk the nodes at the end of the translation unit, applying the
!    attributes at that point.  */
  
  static void
  process_function_and_variable_attributes (struct cgraph_node *first,
!                                           struct cgraph_varpool_node *first_var)
  {
    struct cgraph_node *node;
    struct cgraph_varpool_node *vnode;
--- 985,1002 ----
     is valid.
  
     So, we walk the nodes at the end of the translation unit, applying the
!    attributes at that point. 
!    
!    The local variables needs to be walked on the end of each compilation unit
!    (to allow dead function/variable removal), while the global variables needs
!    to be handled on the end of compilation to allow flags to be declared only
!    in one of units.  The GLOBAL is used to specify whether local or global
!    variables shall be processed.  */
  
  static void
  process_function_and_variable_attributes (struct cgraph_node *first,
!                                           struct cgraph_varpool_node *first_var,
! 					  bool global)
  {
    struct cgraph_node *node;
    struct cgraph_varpool_node *vnode;
*************** process_function_and_variable_attributes
*** 977,982 ****
--- 1004,1012 ----
    for (node = cgraph_nodes; node != first; node = node->next)
      {
        tree decl = node->decl;
+       if (global != (DECL_COMDAT (decl)
+ 		     || (TREE_PUBLIC (decl) && !DECL_EXTERNAL (decl))))
+ 	continue;
        if (lookup_attribute ("used", DECL_ATTRIBUTES (decl)))
  	{
  	  mark_decl_referenced (decl);
*************** process_function_and_variable_attributes
*** 1000,1005 ****
--- 1030,1037 ----
    for (vnode = cgraph_varpool_nodes; vnode != first_var; vnode = vnode->next)
      {
        tree decl = vnode->decl;
+       if (global != (DECL_COMDAT (decl) || TREE_PUBLIC (decl)))
+ 	continue;
        if (lookup_attribute ("used", DECL_ATTRIBUTES (decl)))
  	{
  	  mark_decl_referenced (decl);
*************** cgraph_finalize_compilation_unit (void)
*** 1052,1058 ****
      }
  
    timevar_push (TV_CGRAPH);
!   process_function_and_variable_attributes (first_analyzed, first_analyzed_var);
    cgraph_varpool_analyze_pending_decls ();
    if (cgraph_dump_file)
      {
--- 1085,1092 ----
      }
  
    timevar_push (TV_CGRAPH);
!   process_function_and_variable_attributes (first_analyzed, first_analyzed_var,
! 		  			    false);
    cgraph_varpool_analyze_pending_decls ();
    if (cgraph_dump_file)
      {
*************** cgraph_optimize (void)
*** 1505,1512 ****
  
    timevar_push (TV_CGRAPHOPT);
    if (!quiet_flag)
!     fprintf (stderr, "Performing intraprocedural optimizations\n");
  
    cgraph_function_and_variable_visibility ();
    if (cgraph_dump_file)
      {
--- 1540,1548 ----
  
    timevar_push (TV_CGRAPHOPT);
    if (!quiet_flag)
!     fprintf (stderr, "Performing interprocedural optimizations\n");
  
+   process_function_and_variable_attributes (NULL, NULL, true);
    cgraph_function_and_variable_visibility ();
    if (cgraph_dump_file)
      {
Comment 15 Bernhard Reutner-Fischer 2006-08-20 13:15:34 UTC
(In reply to comment #13)
> Subject: Re:  externally_visible attribute not effective with prior declaration
> of symbol.
> 
> > 
> > If this is really true, then there are several bugs (in the FEs?) because there
> > are numerous occurances where referenced_vars_insert() is called with
> > TREE_USED(to) == 0
> > 
> > Should there be an assertion that only TREE_USED() > 0 are valid targets for
> > insertion in/after dfa?
> 
> I am not quite convinced there is necesarily a problem, since from
> frontend point of view all public variables are automatically used, so
> the whole thing matters only for the cgraph code were we start to
> differentiate -fwhole-program mode from non-whole-program...

For publics, i'd agree, but still there are clearly private funcs that have TREE_USED == 0, like:

../../../src/gcc-4.2/libiberty/regex.c:4445: warning:  referenced_var_insert(): 
'' == 0

Where
4441: static reg_errcode_t
4442: byte_compile_range (unsigned int range_start_char, const char **p_ptr,
4443:                    const char *pend, RE_TRANSLATE_TYPE translate,
4444:                    reg_syntax_t syntax, unsigned char *b)
4445: {

Aren't these bugs since they clearly are not public and TREE_USED is 0 when referenced_vars_insert is called on them?

Why are public funcs not marked USED?
Comment 16 Alexandre Pereira Nunes 2008-01-15 18:03:39 UTC
vanilla gcc 4.2.2 seems to compile this testcase ok (all five symbols are emmited and externally visible, no warnings)
Comment 17 Jan Hubicka 2012-03-24 22:07:55 UTC
I believe this is fully solved now with late processing of externally_visible and removal of --combine