ATTRIBUTE_WEAK not working in crtstuff.c?

ATTRIBUTE_WEAK not working in crtstuff.c?

[Raja R Harinath]

Hi,

For a couple of weeks, I've been getting shared libraries with
non-weak references to __register_frame_info_bases when compiled with
a GCC built off the trunk.

  $ nm --dynamic libfreetype.so
  [snip]
           w _Jv_RegisterClasses
  00049f1c A __bss_start
           U __ctype_b
           w __cxa_finalize
           U __deregister_frame_info_bases
           U __fxstat
           w __gmon_start__
           U __register_frame_info_bases
           U __sigsetjmp

Notice the weak reference to __cxa_finalize and the strong reference
to __register_frame_info_bases.  In crtstuff.c, both are marked with
ATTRIBUTE_WEAK, but __register_frame_info_bases has a declaration
without ATTRIBUTE_WEAK in unwind-dw2-fde.h, which is included in
crtstuff.c.  The comment in crtstuff.c says:

  /* We do not want to add the weak attribute to the declarations of these
     routines in unwind-dw2-fde.h because that will cause the definition of
     these symbols to be weak as well.

     This exposes a core issue, how to handle creating weak references vs
     how to create weak definitions.  Either we have to have the definition
     of TARGET_WEAK_ATTRIBUTE be conditional in the shared header files or
     have a second declaration if we want a function's references to be weak,
     but not its definition.

     Making TARGET_WEAK_ATTRIBUTE conditional seems like a good solution until
     one thinks about scaling to larger problems -- ie, the condition under
     which TARGET_WEAK_ATTRIBUTE is active will eventually get far too
     complicated.

     So, we take an approach similar to #pragma weak -- we have a second
     declaration for functions that we want to have weak references.

     Neither way is particularly good.  */

It seems to me, on a cursory glance, that this approach of adding a
weak attribute to a second declaration isn't working.

- Hari
-- 
Raja R Harinath ------------------------------ harinath@cs.umn.edu
"When all else fails, read the instructions."      -- Cahn's Axiom
"Our policy is, when in doubt, do the right thing."   -- Roy L Ash

Re: ATTRIBUTE_WEAK not working in crtstuff.c?

[Alan Modra]

Hmm, we put the new decl on weakdecls list, but then never use the
new one so the !TREE_USED test in weak_finish prevents the asm
.weak being emitted.  This is one way to fix the problem.

gcc/ChangeLog
	* c-decl.c (duplicate_decls): Call declare_weak on olddecl.

diff -urpN -xCVS -x*~ gcc-ppc64-31.orig/gcc/c-decl.c gcc-ppc64-31/gcc/c-decl.c
--- gcc-ppc64-31.orig/gcc/c-decl.c	Wed Apr  3 09:00:04 2002
+++ gcc-ppc64-31/gcc/c-decl.c	Thu Apr 18 17:14:08 2002
@@ -1421,6 +1421,9 @@ duplicate_decls (newdecl, olddecl, diffe
 	    }
 	}
 
+      if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+	declare_weak (olddecl);
+
       DECL_ATTRIBUTES (newdecl)
 	= (*targetm.merge_decl_attributes) (olddecl, newdecl);
     }

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: ATTRIBUTE_WEAK not working in crtstuff.c?

[Franz Sirl]

At 09:52 18.04.2002, Alan Modra wrote:
>Hmm, we put the new decl on weakdecls list, but then never use the
>new one so the !TREE_USED test in weak_finish prevents the asm
>.weak being emitted.  This is one way to fix the problem.
>
>gcc/ChangeLog
>         * c-decl.c (duplicate_decls): Call declare_weak on olddecl.
>
>diff -urpN -xCVS -x*~ gcc-ppc64-31.orig/gcc/c-decl.c gcc-ppc64-31/gcc/c-decl.c
>--- gcc-ppc64-31.orig/gcc/c-decl.c      Wed Apr  3 09:00:04 2002
>+++ gcc-ppc64-31/gcc/c-decl.c   Thu Apr 18 17:14:08 2002
>@@ -1421,6 +1421,9 @@ duplicate_decls (newdecl, olddecl, diffe
>             }
>         }
>
>+      if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
>+       declare_weak (olddecl);
>+
>        DECL_ATTRIBUTES (newdecl)
>         = (*targetm.merge_decl_attributes) (olddecl, newdecl);
>      }

Well, this cures only part of the problem, check the discussions on 
SYMBOL_REF_WEAK shortly before gcc-3.0 was released.

Weak handling vs. multiple declarations vs. #pragma weak vs. ordering is 
currently more or less unspecified. If you look at my initial patch back 
then, there was:

@@ -2028,7 +2031,15 @@ duplicate_decls (newdecl, olddecl, diffe
      }

    /* Merge the storage class information.  */
-  DECL_WEAK (newdecl) |= DECL_WEAK (olddecl);
+  if (!DECL_WEAK (newdecl) && DECL_WEAK (olddecl))
+    declare_weak (newdecl);
+  if (DECL_WEAK (newdecl) && !DECL_WEAK (olddecl))
+    declare_weak (olddecl);
+  if (DECL_WEAK (newdecl) && DECL_RTL (newdecl)
+      && GET_CODE (DECL_RTL (newdecl)) == MEM
+      && XEXP (DECL_RTL (newdecl), 0)
+      && GET_CODE (XEXP (DECL_RTL (newdecl), 0)) == SYMBOL_REF)
+    SYMBOL_REF_WEAK (XEXP (DECL_RTL (newdecl), 0)) = 1;
    /* For functions, static overrides non-static.  */
    if (TREE_CODE (newdecl) == FUNCTION_DECL)
      {

Even that didn't cover all cases IIRC. If someone would come up with some 
testcases specifying the desired semantics of attribute((weak)) vs. #pragma 
weak vs. declaration vs. definition, I'm willing to work on it again. It 
doesn't make much sense to work on it when the desired semantics we want to 
support (!, I think there are cases we should warn about or even error out) 
are largely unspecified or unknown.

Franz.

Re: ATTRIBUTE_WEAK not working in crtstuff.c?

[Richard Henderson]

On Thu, Apr 18, 2002 at 10:29:47AM +0200, Franz Sirl wrote:
> Even that didn't cover all cases IIRC. If someone would come up with some 
> testcases specifying the desired semantics of attribute((weak)) vs. #pragma 
> weak vs. declaration vs. definition, I'm willing to work on it again.

Quite simple: attributes are sticky, and #pragma is a funny name
for an attribute.


r~

Re: ATTRIBUTE_WEAK not working in crtstuff.c?

[Franz Sirl]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

On Thursday 18 April 2002 19:44, Richard Henderson wrote:
> On Thu, Apr 18, 2002 at 10:29:47AM +0200, Franz Sirl wrote:
> > Even that didn't cover all cases IIRC. If someone would come up with some
> > testcases specifying the desired semantics of attribute((weak)) vs.
> > #pragma weak vs. declaration vs. definition, I'm willing to work on it
> > again.
>
> Quite simple: attributes are sticky, and #pragma is a funny name
> for an attribute.

Hehe, when it's so simple, then why there is no warning/error for 8 of 21 
cases below, leading to potential miscompilation due to the missing 
SYMBOL_REF_WEAK (/i in RTL) flag? And these testcases are just a quick 
summary I cobbled together in 5 min, there may be more cases out there.

Only case 3a (with the currently commented out #pragma weak) gives an error.

BTW, does the dg testsuite have something like "scan-rtl-not rtl/-dr 
symbol_ref:" to scan for a naked SYMBOL_REF?

Franz.



[-- Attachment #2: weak.c --]
[-- Type: text/plain, Size: 2621 bytes --]


// test function addresses with __attribute__((weak))
extern void * foo1 (void) __attribute__((weak));
extern void * foo1 (void);
void * foo1 (void)
{
  return (void *)foo1;
}

extern void * foo2 (void);
extern void * foo2 (void) __attribute__((weak));
void * foo2 (void)
{
  return (void *)foo2;
}

extern void * foo3 (void);
void * foo3 (void)
{
  return (void *)foo3;
}
extern void * foo3 (void) __attribute__((weak));



// test function addresses with #pragma weak
#pragma weak foo1a
extern void * foo1a (void);
void * foo1a (void)
{
  return (void *)foo1a;
}

extern void * foo2a (void);
#pragma weak foo2a
void * foo2a (void)
{
  return (void *)foo2a;
}

extern void * foo3a (void);
void * foo3a (void)
{
  return (void *)foo3a;
}
//#pragma weak foo3a




// test variable addresses with __attribute__((weak))
extern int ifoo1b (void) __attribute__((weak));
extern int ifoo1b (void);
void * foo1b (void)
{
  return (void *)&ifoo1b;
}

extern int ifoo2b (void);
extern int ifoo2b (void) __attribute__((weak));
void * foo2b (void)
{
  return (void *)&ifoo2b;
}

extern int ifoo3b (void);
void * foo3b (void)
{
  return (void *)&ifoo3b;
}
extern int ifoo3b (void) __attribute__((weak));

extern int ifoo4b (void) __attribute__((weak));
int ifoo4b (void);
void * foo4b (void)
{
  return (void *)&ifoo4b;
}

int ifoo5b (void);
extern int ifoo5b (void) __attribute__((weak));
void * foo5b (void)
{
  return (void *)&ifoo5b;
}

int ifoo6b (void);
void * foo6b (void)
{
  return (void *)&ifoo6b;
}
extern int ifoo6b (void) __attribute__((weak));

extern int ifoo7b (void);
void * foo7b (void)
{
  return (void *)&ifoo7b;
}
int ifoo7b (void) __attribute__((weak));

extern int ifoo8b (void) __attribute__((weak));
void * foo8b (void)
{
  return (void *)&ifoo8b;
}
int ifoo8b (void);


// test variable addresses with #pragma weak
#pragma weak ifoo1c
extern int ifoo1c (void);
void * foo1c (void)
{
  return (void *)&ifoo1c;
}

extern int ifoo2c (void);
#pragma weak ifoo2c
void * foo2c (void)
{
  return (void *)&ifoo2c;
}

extern int ifoo3c (void);
void * foo3c (void)
{
  return (void *)&ifoo3c;
}
#pragma weak ifoo3c

#pragma weak ifoo4c
int ifoo4c (void);
void * foo4c (void)
{
  return (void *)&ifoo4c;
}

int ifoo5c (void);
#pragma weak ifoo5c
void * foo5c (void)
{
  return (void *)&ifoo5c;
}

int ifoo6c (void);
void * foo6c (void)
{
  return (void *)&ifoo6c;
}
#pragma weak ifoo6c

extern int ifoo7c (void);
void * foo7c (void)
{
  return (void *)&ifoo7c;
}
#pragma weak ifoo7c
int ifoo7c (void);

extern int ifoo8c (void);
void * foo8c (void)
{
  return (void *)&ifoo8c;
}
int ifoo8c (void);
#pragma weak ifoo8c

Re: ATTRIBUTE_WEAK not working in crtstuff.c?

[Richard Henderson]

On Thu, Apr 18, 2002 at 10:37:03PM +0200, Franz Sirl wrote:
> Hehe, when it's so simple, then why there is no warning/error for 8 of 21 
> cases below, leading to potential miscompilation due to the missing 
> SYMBOL_REF_WEAK (/i in RTL) flag?

Because someone broke it at some point in the past and we're just 
noticing?

> BTW, does the dg testsuite have something like "scan-rtl-not rtl/-dr 
> symbol_ref:" to scan for a naked SYMBOL_REF?

Nope.


r~

Re: ATTRIBUTE_WEAK not working in crtstuff.c?

[Alan Modra]

On Thu, Apr 18, 2002 at 10:29:47AM +0200, Franz Sirl wrote:
> Well, this cures only part of the problem, check the discussions on 
> SYMBOL_REF_WEAK shortly before gcc-3.0 was released.

Um, yes.  Missed the var decl case.  cp/decl.c needs the same
treatment too.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: ATTRIBUTE_WEAK not working in crtstuff.c?

[Franz Sirl]

At 22:39 18.04.2002, Richard Henderson wrote:
>On Thu, Apr 18, 2002 at 10:37:03PM +0200, Franz Sirl wrote:
> > Hehe, when it's so simple, then why there is no warning/error for 8 of 21
> > cases below, leading to potential miscompilation due to the missing
> > SYMBOL_REF_WEAK (/i in RTL) flag?
>
>Because someone broke it at some point in the past and we're just
>noticing?

It's more like it _never_ worked 100% correct :-). Remember that 
SYMBOL_REF_WEAK is quite new, before that it usually was enough to tell the 
assembler about weak symbols. Now RTL generation should know about weak 
symbols, but there is really no specification on how 'late' we will accept 
the attaching of the weak attribute or when we should error/warn about 
misusage.

> > BTW, does the dg testsuite have something like "scan-rtl-not rtl/-dr
> > symbol_ref:" to scan for a naked SYMBOL_REF?
>
>Nope.

Is there interest in such a beast? Seems easy enough to do a scanrtl.exp 
based on scanasm.exp, so we would have:

  scan-rtl         <pattern> <rtldump-extension> <flag>
  scan-rtl-not     <pattern> <rtldump-extension> <flag>
  scan-rtl-dem     <pattern> <rtldump-extension> <flag>
  scan-rtl-dem-not <pattern> <rtldump-extension> <flag>

I don't know if the demangle stuff is useful on RTL though.

Franz.