This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Add a record_link_assignments hook to ldemul
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: Nick Clifton <nickc at redhat dot com>, Binutils <binutils at sourceware dot org>
- Date: Fri, 29 Apr 2016 05:30:29 -0700
- Subject: Re: [PATCH] Add a record_link_assignments hook to ldemul
- Authentication-results: sourceware.org; auth=none
- References: <CAMe9rOrA-N+EzYB=_vPMoUrO6_W6Otg54ceuTpH4ThJ1FVCt6A at mail dot gmail dot com> <20160428233808 dot GD18915 at bubble dot grove dot modra dot org> <CAMe9rOoDUUrnSSb-b_CmrvwB1KzaMKRwHxOw8c48TTvQqMcORQ at mail dot gmail dot com> <20160429063520 dot GE18915 at bubble dot grove dot modra dot org>
On Thu, Apr 28, 2016 at 11:35 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 28, 2016 at 05:59:55PM -0700, H.J. Lu wrote:
>> On Thu, Apr 28, 2016 at 4:38 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Thu, Apr 28, 2016 at 02:01:47PM -0700, H.J. Lu wrote:
>> >> + /* Symbol is defined. Check if it is also defined in a regular
>> >> + input file only when it is currently defined in a dynamic
>> >> + object, since otherwise, it can't be a __start_<name> nor
>> >> + __stop_<name> symbol. */
>> >> + if (!h->def_dynamic)
>> >> return NULL;
>> > [snip]
>> >> + if (s != NULL)
>> >> + {
>> >> + h->root.u.undef.section = s;
>> >> + break;
>> >> + }
>> >
>> > You can't set u.undef here on a defined symbol. That's just too ugly,
>> > even if you later set it to undefined. Better to force it
>> > bfd_link_hash_undefined here.
>>
>> Like this?
>
> No, I meant that if you want to make use of the u.undef.section cache,
> then set it to undefined first. If you don't intend to set
> u.undef.section then set to undefined later.
>
>> > This is getting quite messy, and I'm wondering if we even need
>> > _bfd_elf_is_start_stop, except for gc-sections code.
>
> Note that when I wrote _bfd_elf_is_start_stop I thought it might come
> in useful in check_relocs, to prevent __start_* and __stop_* from
> being seen as dynamic. Now that you're running both
> find_statement_assignment and lang_do_assignments before check_relocs,
> doing anything special with _bfd_elf_is_start_stop should not be
> necessary.
__start_* and __stop_* aren't handled properly with shared library:
https://sourceware.org/bugzilla/show_bug.cgi?id=20022
_bfd_elf_is_start_stop needs to check symbols defined in a
shared library.
> find_statement_assignment has given the bfd backend a look at symbols,
> and you've processed the PROVIDE (__start_sec = .) linker script
> statements in lang_do_assignments. So it ought to be possible to make
> __start_sec a def_regular normal symbol by that point. As I said
> before, I'd expect that to work better if find_statement_assignment
> ran before lang_do_assignments. That allows
> bfd_elf_record_link_assignment to force def_dynamic symbols to
> bfd_link_hash_undefined so that ldexp.c code handling PROVIDE doesn't
> need to know ELF specific details.
>
> However, you said that doing it that way didn't work. What didn't
> work, and what needs to change in bfd_elf_record_link_assignment to
> make it work?
The failed testcase is ld-elf/ehdr_start-userdef. We have
/* Only adjust the export class if the symbol was referenced
and not defined, otherwise leave it alone. */
if (h != NULL
&& (h->root.type == bfd_link_hash_new
|| h->root.type == bfd_link_hash_undefined
|| h->root.type == bfd_link_hash_undefweak
|| h->root.type == bfd_link_hash_common))
{
Since this is run before lang_do_assignments, we don't see
__ehdr_start = 0x12345678;
in linker script and get
(gdb) p *h
$2 = {root = {root = {next = 0x0, string = 0x859bad "__ehdr_start",
hash = 78192523}, type = bfd_link_hash_undefined, non_ir_ref = 0,
linker_def = 0, u = {undef = {next = 0x0, abfd = 0x857100, section = 0x0},
def = {next = 0x0, section = 0x857100, value = 0}, i = {next = 0x0,
link = 0x857100, warning = 0x0}, c = {next = 0x0, p = 0x857100,
size = 0}}}, indx = -1, dynindx = -1, got = {refcount = 0, offset = 0,
glist = 0x0, plist = 0x0}, plt = {refcount = 0, offset = 0, glist = 0x0,
plist = 0x0}, size = 0, type = 0, other = 0, target_internal = 0,
ref_regular = 1, def_regular = 0, ref_dynamic = 0, def_dynamic = 0,
ref_regular_nonweak = 1, dynamic_adjusted = 0, needs_copy = 0,
needs_plt = 0, non_elf = 0, versioned = unversioned, forced_local = 0,
dynamic = 0, mark = 0, non_got_ref = 0, dynamic_def = 0,
ref_dynamic_nonweak = 0, pointer_equality_needed = 0, unique_global = 0,
protected_def = 0, dynstr_index = 0, u = {weakdef = 0x0,
elf_hash_value = 0}, verinfo = {verdef = 0x0, vertree = 0x0}, vtable = 0x0}
We get
3: 0000000012345678 0 NOTYPE LOCAL DEFAULT ABS __ehdr_start
instead of
3: 0000000012345678 0 NOTYPE GLOBAL DEFAULT ABS __ehdr_start
in output.
--
H.J.