PATCH: PR 528: .set is broken for weak symbols

PATCH: PR 528: .set is broken for weak symbols

[H. J. Lu]

On Mon, Nov 08, 2004 at 03:59:40PM -0800, H. J. Lu wrote:
> This patch
> 
> http://sources.redhat.com/ml/binutils/2004-11/msg00098.html
> 
> breaks .set, which should only copy the value, not other flags. See
> 
> http://sources.redhat.com/bugzilla/show_bug.cgi?id=528
> 
> Please fix it now.
> 
> 

This patch fixes the problem for me. I have 2 questions:

1. Should .set change the behavior on weak symbols on any platforms?
Does gcc expect that?
2. The ChangeLog entry has

2004-11-08  Aaron W. LaFramboise <aaron98wiridge9@aaronwl.com>
 
        * symbols.c (any_external_name): Define.
        (resolve_symbol_value): Do not convert weak symbols.
				^^^^^^^

Is this a typo? From the code, it seems that weak symbols are
converted.


H.J.
----
2004-11-08  H.J. Lu  <hongjiu.lu@intel.com>

	PR 528
	* symbols.c (resolve_symbol_value): Convert weak symbols only
	for Windows PECOFF.
	(symbol_equated_reloc_p): Don't equate weaks when relocating
	only for Windows PECOFF.

--- gas/symbols.c.set	2004-11-08 09:43:00.000000000 -0800
+++ gas/symbols.c	2004-11-08 16:18:44.988697868 -0800
@@ -995,7 +995,7 @@ resolve_symbol_value (symbolS *symp)
 	     relocation to be against the symbol to which this symbol
 	     is equated.  */
 	  if (! S_IS_DEFINED (add_symbol)
-#if defined(BFD_ASSEMBLER) || defined(S_IS_WEAK)
+#if defined (OBJ_COFF) && defined (TE_PE) && (defined(BFD_ASSEMBLER) || defined(S_IS_WEAK))
 	      || S_IS_WEAK (add_symbol)
 #endif
 	      || S_IS_COMMON (add_symbol))
@@ -2239,7 +2239,7 @@ symbol_equated_reloc_p (symbolS *s)
      resolve_symbol_value to flag expression syms that have been
      equated.  */
   return (s->sy_value.X_op == O_symbol
-#if defined(BFD_ASSEMBLER) || defined(S_IS_WEAK)
+#if defined (OBJ_COFF) && defined (TE_PE) && (defined(BFD_ASSEMBLER) || defined(S_IS_WEAK))
 	  && ! S_IS_WEAK (s)
 #endif
 	  && ((s->sy_resolved && s->sy_value.X_op_symbol != NULL)

Re: PATCH: PR 528: .set is broken for weak symbols

[Alan Modra]

On Mon, Nov 08, 2004 at 04:30:25PM -0800, H. J. Lu wrote:
> 	PR 528
> 	* symbols.c (resolve_symbol_value): Convert weak symbols only
> 	for Windows PECOFF.
> 	(symbol_equated_reloc_p): Don't equate weaks when relocating
> 	only for Windows PECOFF.

Please commit if you haven't done so already, and close pr 528.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: PR 528: .set is broken for weak symbols

[Aaron W. LaFramboise]

H. J. Lu wrote:

> On Mon, Nov 08, 2004 at 03:59:40PM -0800, H. J. Lu wrote:
> 
>>This patch
>>
>>http://sources.redhat.com/ml/binutils/2004-11/msg00098.html
>>
>>breaks .set, which should only copy the value, not other flags. See
>>
>>http://sources.redhat.com/bugzilla/show_bug.cgi?id=528

Sorry for the breakage.  I don't understand exactly whats going wrong
here, though.  See below.

.set doesn't just copy value, though; its used to equate symbols.  For
example, __attribute__((weak, alias ("..."))) uses .set for most (all?)
platforms.

I don't understand why it is correct for your testcase foo.o to not
generate a relocation; if weak symbol bar is overriden in another
object, the operand to "jmp" will not be overriden, despite having been
equated to the weak symbol being overriden.

This would mean that local symbols equated to the weak symbol would
resolve to a different function than non-local symbols equated to the
same weak symbol, which seems very wrong to me.  In addition, it means
that a weak function might actually be used, despite being overridden by
a strong function.  This is also suprising, as one would expect that a
weak function overriden by a strong one would never be used.

> 1. Should .set change the behavior on weak symbols on any platforms?
> Does gcc expect that?

I think GCC does expect that .set will do more than just copy the value
in the case of weak symbols.

> 2. The ChangeLog entry has
> 
> 2004-11-08  Aaron W. LaFramboise <aaron98wiridge9@aaronwl.com>
>  
>         * symbols.c (any_external_name): Define.
>         (resolve_symbol_value): Do not convert weak symbols.
> 				^^^^^^^
> Is this a typo? From the code, it seems that weak symbols are
> converted.

Yes, you're right; this is a typo.

> 	* symbols.c (resolve_symbol_value): Convert weak symbols only
> 	for Windows PECOFF.

This conversion is necessary for the same reason conversion is necessary
with undefined symbols: since the equated-to symbol is weak, the equated
symbol's value will be incorrect if the equated-to weak symbol is
overriden.  In other words, symbol A is equated to weak symbol B, and a
relocation against A is not converted to be against B, if B is
overriden, the relocation couldn't possibly come out right after fixup.

Why is this not the case with ELF?

> 	(symbol_equated_reloc_p): Don't equate weaks when relocating
> 	only for Windows PECOFF.

This is necessary for similar reasons as the above, but opposite.  Since
a weak symbol may later be overridden, and thus no longer equated, it is
not safe to assume that it is equated.  In other words, weak symbol C is
equated to symbol D.  If the reloc code assumes that C is equated to D,
then that relocation is against D.  If C is later overridden, the
relocation will be incorrect, as the fixup will be against D, which
won't be the same as C.

Why is this not the case with ELF?


Thanks for your time,

Aaron W. LaFramboise

Re: PATCH: PR 528: .set is broken for weak symbols

[H. J. Lu]

On Tue, Nov 09, 2004 at 12:48:57AM -0600, Aaron W. LaFramboise wrote:
> H. J. Lu wrote:
> 
> > On Mon, Nov 08, 2004 at 03:59:40PM -0800, H. J. Lu wrote:
> > 
> >>This patch
> >>
> >>http://sources.redhat.com/ml/binutils/2004-11/msg00098.html
> >>
> >>breaks .set, which should only copy the value, not other flags. See
> >>
> >>http://sources.redhat.com/bugzilla/show_bug.cgi?id=528
> 
> Sorry for the breakage.  I don't understand exactly whats going wrong
> here, though.  See below.
> 
> .set doesn't just copy value, though; its used to equate symbols.  For
> example, __attribute__((weak, alias ("..."))) uses .set for most (all?)
> platforms.
> 
> I don't understand why it is correct for your testcase foo.o to not
> generate a relocation; if weak symbol bar is overriden in another
> object, the operand to "jmp" will not be overriden, despite having been
> equated to the weak symbol being overriden.
> 
> This would mean that local symbols equated to the weak symbol would
> resolve to a different function than non-local symbols equated to the
> same weak symbol, which seems very wrong to me.  In addition, it means
> that a weak function might actually be used, despite being overridden by
> a strong function.  This is also suprising, as one would expect that a
> weak function overriden by a strong one would never be used.
> 

.set should just copy value and type. Be default, the new symbol is
always local. BTW,

	.weak foo

implies

	.global foo

> > 1. Should .set change the behavior on weak symbols on any platforms?
> > Does gcc expect that?
> 
> I think GCC does expect that .set will do more than just copy the value
> in the case of weak symbols.

I don't think so. Otherwise, there is no need to use ".set". It can use
the original symbol instead.

> 
> > 2. The ChangeLog entry has
> > 
> > 2004-11-08  Aaron W. LaFramboise <aaron98wiridge9@aaronwl.com>
> >  
> >         * symbols.c (any_external_name): Define.
> >         (resolve_symbol_value): Do not convert weak symbols.
> > 				^^^^^^^
> > Is this a typo? From the code, it seems that weak symbols are
> > converted.
> 
> Yes, you're right; this is a typo.
> 
> > 	* symbols.c (resolve_symbol_value): Convert weak symbols only
> > 	for Windows PECOFF.
> 
> This conversion is necessary for the same reason conversion is necessary
> with undefined symbols: since the equated-to symbol is weak, the equated
> symbol's value will be incorrect if the equated-to weak symbol is
> overriden.  In other words, symbol A is equated to weak symbol B, and a
> relocation against A is not converted to be against B, if B is
> overriden, the relocation couldn't possibly come out right after fixup.
> 
> Why is this not the case with ELF?
> 
> > 	(symbol_equated_reloc_p): Don't equate weaks when relocating
> > 	only for Windows PECOFF.
> 
> This is necessary for similar reasons as the above, but opposite.  Since
> a weak symbol may later be overridden, and thus no longer equated, it is
> not safe to assume that it is equated.  In other words, weak symbol C is
> equated to symbol D.  If the reloc code assumes that C is equated to D,
> then that relocation is against D.  If C is later overridden, the
> relocation will be incorrect, as the fixup will be against D, which
> won't be the same as C.
> 
> Why is this not the case with ELF?

The key is ".set" is not used to equate symbols. It is used to set a
symbol with the same value and type. If the symbol doesn't exist
before, the new symbol will be local.


H.J.

Re: PATCH: PR 528: .set is broken for weak symbols

[Aaron W. LaFramboise]

H. J. Lu wrote:

> The key is ".set" is not used to equate symbols. It is used to set a
> symbol with the same value and type. If the symbol doesn't exist
> before, the new symbol will be local.

I am not so sure this is absolutely correct though.  Consider:

        .text
        .global a
        .set a, b
        jmp a

RELOCATION RECORDS FOR [.text]:
OFFSET   TYPE              VALUE
00000001 DISP32            b

Note that the two symbols are equated with .set and that the relocation
is adjusted from being against a to being against b.

In my mind, a weak symbol is semantically much more like an undefined
symbol than a defined symbol.  For weak symbols, like undefined symbols,
we don't actually know the final value until the final link.
Specifically, I think a weak symbol (even a defined weak symbol) is just
a special case of an undefined symbol, in which special semantics are
invoked if the symbol is not strongly defined rather than just failing.

So, it seems contradictory and arbitrary that .set for .weak doesn't
have similar semantics as .set for undefined symbols.

In any case, PECOFF doesn't have the distinction between defweak and
undefweak, with final semantics being a little bit of both.  It makes a
lot of sense, for PECOFF, for weak symbols to have the semantics I
described, so I think that the current handling is correct.  (I don't
like having an #ifdef TE_PE in the target-independent part of gas
though; how can this be fixed?)

Also, I fixed the typo in the ChangeLog:
<http://sources.redhat.com/ml/binutils-cvs/2004-11/msg00071.html>.

Aaron W. LaFramboise

Re: PATCH: PR 528: .set is broken for weak symbols

[H. J. Lu]

On Tue, Nov 09, 2004 at 03:30:54PM -0600, Aaron W. LaFramboise wrote:
> H. J. Lu wrote:
> 
> > The key is ".set" is not used to equate symbols. It is used to set a
> > symbol with the same value and type. If the symbol doesn't exist
> > before, the new symbol will be local.
> 
> I am not so sure this is absolutely correct though.  Consider:
> 
>         .text
>         .global a
>         .set a, b
>         jmp a
> 
> RELOCATION RECORDS FOR [.text]:
> OFFSET   TYPE              VALUE
> 00000001 DISP32            b
> 
> Note that the two symbols are equated with .set and that the relocation
> is adjusted from being against a to being against b.

That is because b is undefined.

> 
> In my mind, a weak symbol is semantically much more like an undefined
> symbol than a defined symbol.  For weak symbols, like undefined symbols,
> we don't actually know the final value until the final link.
> Specifically, I think a weak symbol (even a defined weak symbol) is just
> a special case of an undefined symbol, in which special semantics are
> invoked if the symbol is not strongly defined rather than just failing.
> 

You are talking different weak symbol. See

http://www.sco.com/developers/gabi/latest/ch4.symtab.html

for weak symbols in ELF.

> So, it seems contradictory and arbitrary that .set for .weak doesn't
> have similar semantics as .set for undefined symbols.
> 
> In any case, PECOFF doesn't have the distinction between defweak and
> undefweak, with final semantics being a little bit of both.  It makes a
> lot of sense, for PECOFF, for weak symbols to have the semantics I
> described, so I think that the current handling is correct.  (I don't
> like having an #ifdef TE_PE in the target-independent part of gas
> though; how can this be fixed?)
> 

You can define

#define TC_CONVERT_WEAK .....

or something like that and default it to the old semantics. You can
do it differently for PECOFF.


H.J.