Bug 24295 - [4.1 Regression] Xorg broken, #pragma weak foo = bar no longer causes bar to be referenced
Summary: [4.1 Regression] Xorg broken, #pragma weak foo = bar no longer causes bar to ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.1.0
: P2 critical
Target Milestone: 4.1.0
Assignee: Alexandre Oliva
URL:
Keywords: wrong-code
Depends on:
Blocks: 24446
  Show dependency treegraph
 
Reported: 2005-10-10 10:47 UTC by Richard Biener
Modified: 2005-10-20 23:14 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.0.3
Known to fail: 4.1.0
Last reconfirmed: 2005-10-20 18:35:38


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2005-10-10 10:47:21 UTC
Consider

static const double foo_local = 1.0;
#pragma weak foo = foo_local

with unit-at-a-time we get rid of the foo_local variable (because it is unused
apart form the weak decl).  This is a regression from 4.0 where we correctly
mark foo_local as referenced.

This breaks building Xorg.  rth fixed it for 4.0, somehow the bug came back.
Comment 1 Andrew Pinski 2005-10-10 12:18:19 UTC
Confirmed, be failing since at least 20050827.
Comment 2 Richard Biener 2005-10-10 12:19:37 UTC
See PR19031 where this problem was fixed for 4.0.0.
Comment 3 Andrew Pinski 2005-10-10 13:55:52 UTC
Failing started before 20050801.
Comment 4 Andrew Pinski 2005-10-10 14:11:32 UTC
And has been failing since 20050701.

Maybe we need a machine which just builds a distro over and over.  I know Apple has a such a beast for Mac OS X.
Comment 5 Andrew Pinski 2005-10-10 14:32:38 UTC
Happens in 20050601 also.
Comment 6 Andrew Pinski 2005-10-10 14:43:03 UTC
Happens also in 20050501.
Comment 7 Andrew Pinski 2005-10-10 14:57:02 UTC
Fails with 20050401 also.
Comment 8 Andrew Pinski 2005-10-10 15:08:27 UTC
Works in 20050320.
Comment 9 Andrew Pinski 2005-10-10 15:15:32 UTC
CCing Honza since the tree profiling branch merge happened during that time.
Comment 10 Andrew Pinski 2005-10-10 15:27:26 UTC
Works with 20050321.
Comment 11 Andrew Pinski 2005-10-10 15:37:14 UTC
Works on 20050325.
Comment 12 Andrew Pinski 2005-10-10 15:37:55 UTC
(In reply to comment #11)
> Works on 20050325.

The top of the changelog at this time is:
2005-03-24  Kazu Hirata  <kazu@cs.umass.edu>

        * emit-rtl.c (reverse_comparison): Remove.
        * rtl.h: Remove the corresponding prototype.

Comment 13 Andrew Pinski 2005-10-10 15:50:36 UTC
Works with 20050328.
Comment 14 Andrew Pinski 2005-10-10 15:52:20 UTC
(In reply to comment #13)
> Works with 20050328.
Note the top of the changelog is:
2005-03-27  Steven Bosscher  <stevenb@suse.de>

        * vax-protos.h (vax_output_int_move, vax_output_int_add,
        vax_output_conditional_branch): New prototypes.


I don't think this was caused by the merge of the tree-profiling after all.
Comment 15 Andrew Pinski 2005-10-10 15:57:44 UTC
I am starting to think the following patch caused it:
-2005-03-29  Jakub Jelinek  <jakub@redhat.com>
-
-       PR middle-end/20622
-       * cgraph.h (struct cgraph_varpool_node): Add alias field.
-       * cgraph.c (cgraph_varpool_assemble_pending_decls): Don't call
-       assemble_variable on aliases.
-       * varasm.c (assemble_alias): Set node->alias.
-       * toplev.c (wrapup_global_declarations): Don't call
-       rest_of_decl_compilation on aliases again.
Comment 16 Janis Johnson 2005-10-10 23:31:49 UTC
A regression hunt identified this patch from hubicka on 2005-03-30:

  http://gcc.gnu.org/ml/gcc-cvs/2005-03/msg01487.html
Comment 17 Steven Bosscher 2005-10-18 22:24:55 UTC
We want to cgraph_varpool_mark_needed_node the rhs of the #pragma weak, but by the time we get to maybe_apply_pending_pragma_weaks, we only have the identifier, and AFAICT no way to find the appropriate symbol that we want to mark as needed.

Here, I've just made the pragma handler force the rhs of the #pragma weak into the assembler file.  Without force_output, the "unreferenced" rhs will still be dropped from the cgraph by cgraph_varpool_remove_unreferenced_decls, and there is no way to tell from there that this variable is needed, other than forcing it to be written out.  This is a hack, but the way we find out about necessary nodes is so complicated that I don't fully understand how this is supposed to be fixed.

I think fixing this completely requires that we keep a list of DECLs instead of IDENTIFIER_NODEs for the pending #pragma weak list, so that we can mark the DECL of pending pragmas from maybe_apply_pragma_weak.  Or maybe there is another way to get the DECL for alias_id there?


Index: c-pragma.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-pragma.c,v
retrieving revision 1.91
diff -u -3 -p -r1.91 c-pragma.c
--- c-pragma.c  19 Jul 2005 20:19:12 -0000      1.91
+++ c-pragma.c  18 Oct 2005 22:23:57 -0000
@@ -36,6 +36,7 @@ Software Foundation, 51 Franklin Street,
 #include "tm_p.h"
 #include "vec.h"
 #include "target.h"
+#include "cgraph.h"

 #define GCC_BAD(gmsgid) \
   do { warning (OPT_Wpragmas, gmsgid); return; } while (0)
@@ -310,7 +311,7 @@ maybe_apply_pending_pragma_weaks (void)
       alias_id = TREE_PURPOSE (t);
       id = TREE_VALUE (t);

-      if (TREE_VALUE (t) == NULL)
+      if (id == NULL)
        continue;

       decl = build_decl (FUNCTION_DECL, alias_id, default_function_type);
@@ -330,6 +331,7 @@ handle_pragma_weak (cpp_reader * ARG_UNU
 {
   tree name, value, x, decl;
   enum cpp_ttype t;
+  struct cgraph_varpool_node *node;

   value = 0;

@@ -354,6 +356,15 @@ handle_pragma_weak (cpp_reader * ARG_UNU
     }
   else
     pending_weaks = tree_cons (name, value, pending_weaks);
+
+  decl = identifier_global_value (value);
+  if (decl && DECL_P (decl))
+    {
+      /* Force DECL into the assembler output no matter what.  */
+      node = cgraph_varpool_node (decl);
+      cgraph_varpool_mark_needed_node (node);
+      node->force_output = true;
+    }
 }
 #else
 void
Comment 18 Richard Biener 2005-10-19 09:31:40 UTC
+  decl = identifier_global_value (value);

Make that

+  decl = decl ? identifier_global_value (value) : NULL_TREE;

and it even survives bootstrap.
Comment 19 Richard Biener 2005-10-19 09:32:08 UTC
err, s/decl/value/ of course.
Comment 20 GCC Commits 2005-10-20 19:30:29 UTC
Subject: Bug 24295

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	aoliva@gcc.gnu.org	2005-10-20 19:30:23

Modified files:
	gcc/testsuite  : ChangeLog 
	gcc            : ChangeLog cgraphunit.c varasm.c 
	gcc/testsuite/g++.old-deja/g++.abi: vtable2.C 
Added files:
	gcc/testsuite/gcc.dg: attr-alias-3.c 
	gcc/testsuite/gcc.dg/weak: weak-14.c 

Log message:
	gcc/ChangeLog:
	PR middle-end/24295
	* cgraphunit.c (cgraph_varpool_remove_unreferenced_decls): Mark
	alias targets.
	* varasm.c (find_decl_and_mark_needed): After cgraph global info
	is ready, stop marking functions, but still mark variables.
	gcc/testsuite/ChangeLog:
	PR middle-end/24295
	* g++.old-deja/g++.abi/vtable2.C: Do not introduce external
	declarations with the same names as thunks' alias targets, use
	aliases instead.
	* gcc.dg/attr-alias-3.c: New test.
	* gcc.dg/weak/weak-14.c, gcc.dg/weak/weak-14a.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.6219&r2=1.6220
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.10193&r2=2.10194
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cgraphunit.c.diff?cvsroot=gcc&r1=1.129&r2=1.130
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/varasm.c.diff?cvsroot=gcc&r1=1.533&r2=1.534
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.old-deja/g++.abi/vtable2.C.diff?cvsroot=gcc&r1=1.7&r2=1.8
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/attr-alias-3.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/weak/weak-14.c.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 21 Alexandre Oliva 2005-10-20 19:32:05 UTC
Fixed.
Comment 22 GCC Commits 2005-10-25 18:59:26 UTC
Subject: Bug 24295

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	aoliva@gcc.gnu.org	2005-10-25 18:59:21

Modified files:
	gcc/testsuite  : ChangeLog 
	gcc/testsuite/g++.old-deja/g++.abi: vtable2.C 

Log message:
	PR middle-end/24295, PR testsuite/24477
	* g++.old-deja/g++.abi/vtable2.C: Require alias for now.  Will be
	removed when weakref hits the tree.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.6252&r2=1.6253
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.old-deja/g++.abi/vtable2.C.diff?cvsroot=gcc&r1=1.8&r2=1.9