[PATCH] [aarch64] Fix handling of stubs to weak and section symbols

[Eric Christopher <echristo at gmail dot com>]

Hi Cary, Han,

This patch has 3 notable changes:

-         symval.set_output_value(this->plt_section()->address()
-                                 + gsym->plt_offset());
+         symval.set_output_value(this->plt_address_for_global(gsym));

which is just a boring fix to update to use the correct API. Looks
like it didn't get updated with the others.

+  // We don't create stubs for undefined symbols, but do for weak.
+  if (gsym
+      && !gsym->use_plt_offset(arp->reference_flags())
+      && gsym->is_undefined())

which is a small fix on top of my previous one for undefined symbols
to match the associated code in scan_reloc_for_stub by including stubs
for undefined weak symbols.

and finally:

-      // If symbol is a section symbol, we don't know the actual type of
-      // destination.  Give up.
-      if (psymval->is_section_symbol())
-       continue;

which appears to have been pulled from the arm port when the aarch64
port was created. I looked and this was put into the arm port when
stub creation was first put in with no other comments. I can't think
of a reason why we wouldn't want to be able to have stubs to section
symbols - at least not as long as they're STT_FUNC or in separate
sections (these are all likely to be both). I think the original idea
was the return if we didn't know if the symbol was STT_FUNC, but not
entirely positive (cc'ing Doug in case someone sees this discussion in
the future and he wants to chime in). It might be possible to put a
suitable assert or verification into the stub creation in order to
verify that all of the bits of the ABI are satisfied:

The target symbol has type STT_FUNC.
Or, the target symbol and relocated place are in separate sections
input to the linker.
Or, the target symbol is undefined (external to the link unit).

but it's a little harder to figure out the best when/where since a lot
of this code is spread out.


At any rate, I'm convinced that this is safe given that we're now able
to link and execute a bunch of code that wouldn't work before on some
fairly extensive internal testing.

Thoughts? OK?

-eric

2017-06-19  Eric Christopher  <echristo@gmail.com>

* aarch64.cc (scan_reloc_for_stub): Use plt_address_for_global to
calculate the symbol value.
(scan_reloc_section_for_stubs): Allow stubs to be created for
section symbols.
(maybe_apply_stub): Handle creating stubs for weak symbols to
match the code in scan_reloc_for_stub.

Attachment: aarch64.diff.txt
Description: Text document