Patchwork jason@redhat.com

login
register
mail settings
Submitter Jan Hubicka
Date April 26, 2012, 11:06 a.m.
Message ID <20120426110648.GC26747@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/155250/
State New
Headers show

Comments

Jan Hubicka - April 26, 2012, 11:06 a.m.
Hi,
I would like to remove old code handling aliases.  The catch is that in new
representation I am not accepting aliases from functions to variables or
vice versa.  It is not impossible to add, but it is another weird side
case (for exmaple you can then call variable).

I know of only one case where this is needed and it is the following
testcase:
/* PR 19031 */
/* { dg-do compile } */
/* { dg-require-weak "" } */
/* { dg-require-alias "" } */
/* { dg-options "-funit-at-a-time" } */

/* { dg-final { scan-assembler "xyzzy" } } */

static const int local = 1; 
#pragma weak xyzzy = local

obviously it looks pretty harmless.  Problem here is that xyzzy is not
declared and the code in c-pragma.c then always creates function decl.
The following patch fixes it by looking up the proper declaration.
I also noticed that the code is setting EXTERN flag.  This is wrong:
these are normal aliases and a lot of code is discovering weakrefs
by this flag.

There is one catch however, now we refuse the following:
static const int local = 1; 
#pragma weak xyzzy = local
#pragma weak xyzzy2 = xyzzy
while previously we accepted it.  I am not sure if this makes sense to support.

Such use seems more like a bug than design.  Especially because overwritting
xyzzy by something else will keep xyzzy2 to be alias of "local". At least on
ELF target.

If we want to accept such constructs, it is not big deal to make
maybe_apply_pending_pragma_weaks to iterate until it stabilizes.

Seems to make sense?
Honza
	* c-pragma.c: Include cgraph.h.
	(maybe_apply_pending_pragma_weaks): Lookup target type.
Jason Merrill - April 30, 2012, 8:23 p.m.
This makes sense to me.

Jason
Jan Hubicka - April 30, 2012, 8:38 p.m.
> This makes sense to me.
Thanks (and sorry for bogus subject).
I bootstrapped/regtested the change and tested it indeed lets me build kernel
and Mozilla with the sanity check that there are no function<->variable
aliases.  Is the change OK?

Honza
>
> Jason
Jason Merrill - April 30, 2012, 8:58 p.m.
On 04/30/2012 04:38 PM, Jan Hubicka wrote:
>> This makes sense to me.
> Thanks (and sorry for bogus subject).
> I bootstrapped/regtested the change and tested it indeed lets me build kernel
> and Mozilla with the sanity check that there are no function<->variable
> aliases.  Is the change OK?

OK.

Jason

Patch

Index: c-family/c-pragma.c
===================================================================
--- c-family/c-pragma.c	(revision 186835)
+++ c-family/c-pragma.c	(working copy)
@@ -39,6 +39,7 @@  along with GCC; see the file COPYING3.
 #include "diagnostic.h"
 #include "opts.h"
 #include "plugin.h"
+#include "cgraph.h"
 
 #define GCC_BAD(gmsgid) \
   do { warning (OPT_Wpragmas, gmsgid); return; } while (0)
@@ -311,6 +312,7 @@  maybe_apply_pending_pragma_weaks (void)
   tree alias_id, id, decl;
   int i;
   pending_weak *pe;
+  symtab_node target;
 
   FOR_EACH_VEC_ELT (pending_weak, pending_weaks, i, pe)
     {
@@ -320,13 +322,22 @@  maybe_apply_pending_pragma_weaks (void)
       if (id == NULL)
 	continue;
 
+      target = symtab_node_for_asm (id);
       decl = build_decl (UNKNOWN_LOCATION,
-			 FUNCTION_DECL, alias_id, default_function_type);
+			 target ? TREE_CODE (target->symbol.decl) : FUNCTION_DECL,
+			 alias_id, default_function_type);
 
       DECL_ARTIFICIAL (decl) = 1;
       TREE_PUBLIC (decl) = 1;
-      DECL_EXTERNAL (decl) = 1;
       DECL_WEAK (decl) = 1;
+      if (TREE_CODE (decl) == VAR_DECL)
+	TREE_STATIC (decl) = 1;
+      if (!target)
+	{
+	  error ("%q+D aliased to undefined symbol %qE",
+		 decl, id);
+	  continue;
+	}
 
       assemble_alias (decl, id);
     }