This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] [aarch64] Fix handling of stubs to weak and section symbols
- From: "Han Shen via binutils" <binutils at sourceware dot org>
- To: Eric Christopher <echristo at gmail dot com>
- Cc: Cary Coutant <ccoutant at gmail dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>, Doug Kwan (關振德) <dougkwan at google dot com>
- Date: Tue, 20 Jun 2017 15:09:45 -0700
- Subject: Re: [PATCH] [aarch64] Fix handling of stubs to weak and section symbols
- Authentication-results: sourceware.org; auth=none
- References: <CALehDX584XS7h=5VW+1fFJW7=dbp8Y9Vqj7k_=FienxK2email@example.com>
- Reply-to: Han Shen <shenhan at google dot com>
On Mon, Jun 19, 2017 at 5:49 PM, Eric Christopher <firstname.lastname@example.org> wrote:
> 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.
Yeah, good catch. This seems affect handling STT_GNU_IFUNC.
> + // 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.
Yeah, if Doug wrote the lines, he must know it better :)
> 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?
> 2017-06-19 Eric Christopher <email@example.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.