Re: GOLD handling of weak symbols (including x86 vs. ARM)

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Cary Coutant <ccoutant@google.com> writes:
>> However, over in needs_dynamic_reloc, I see this:
>>
>> ? ?// A reference to an undefined symbol from an executable should be
>> ? ?// statically resolved to 0, and does not need a dynamic relocation.
>> ? ?// This matches gnu ld behavior.
>> ? ?if (this->is_undefined() && !parameters->options().shared())
>> ? ? ?return false;
>>
>> So I think we've already decided that we are screwed in this case.
>>
>> And that means that we should remove these lines from use_plt_offset:
>> ? ?if (this->is_weak_undefined())
>> ? ? ?return true;
>> since we've decided that such symbols should be statically resolved to
>> 0.
>>
>> That seems weird to me but perhaps it makes sense in some universe.
>
> I think those lines from use_plt_offset predate the change in
> needs_dynamic_reloc. Here are the two patches that changed
> needs_dynamic_reloc:
>
> http://sourceware.org/ml/binutils/2008-04/msg00019.html
> http://sourceware.org/ml/binutils/2008-04/msg00269.html
>
> The first thread has a good discussion of what the behavior should be.

Ah, I knew I must have missed a thread somewhere, thanks.  Seems like
we came to the same conclusion this time round, which is a good sign. :-)

> It looks to me like I should have also changed use_plt_offset at the
> time. A test case of the form "if (&foo != 0) foo()" probably would
> have caught that.

weak_undef_test.cc already has one of those.  I think the problem is
that it is only run for native linkers, so probably doesn't get much
coverage on targets like ARM that are more often compiled cross.
(x86 works with both -fno-pic (hard-coded 0) and -fPIC (dynamic
overrides allowed); x86 only fails if you add @PLT to the -fno-pic
output by hand.)

Unfortunately, just removing:

 ? ?if (this->is_weak_undefined())
 ? ? ?return true;

regresses weak_plt on x86_64.  We rely completely on use_plt_offset() to
determine whether a relocation uses a PLT, so removing the check means
that even:

	call    foo@PLT

won't use a PLT for a weak undefined foo in executables.  (Recall that
BFD LD does use a PLT in this case, and rightly so.)

What I found a little confusing is that use_plt_offset() unconditionally
passes FUNCTION_CALL to needs_dynamic_reloc(), even though most calls to
use_plt_offset() are made for all relocation types, not just branch/call
relocations.  Is FUNCTION_CALL supposed to mean something else in this
context?

Perhaps we should turn the use_plt_offset() argument into the same
Reference_flags set as needs_dynamic_reloc(), then get the callers of
use_plt_offset() to set FUNCTION_CALL only for branch/call relocs.
That would let us restrict:

 ? ?if (this->is_weak_undefined())
 ? ? ?return true;

to cases where FUNCTION_CALL is set.  Unfortunately, this will touch
all backends, so I thought I'd better ask for comments before coding it up.

If that it is the way to go, should we have a function in each backend
that returns the Reference_flags for a given reloc type?

Richard