[PATCH] Add a do_assignments hook to ldemul
H.J. Lu
hjl.tools@gmail.com
Thu May 5 16:27:00 GMT 2016
On Sat, Apr 30, 2016 at 4:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Apr 30, 2016 at 7:40 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Apr 29, 2016 at 5:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Apr 29, 2016 at 6:35 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Fri, Apr 29, 2016 at 5:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> 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.
>>>>
>>>> I have a fix for it. But _bfd_elf_is_start_stop still needs fix.
>>>
>>> Here is the updated patch, which doesn't use _bfd_elf_is_start_stop.
>>>
>>> OK for master?
>>>
>>
>> Scan over all input files for each start/stop symbol may be too
>> expensive. Assuming there are fewer start/stop symbols than
>> input files, this patch collects all start/stop symbols first and
>> only scan over all input files once.
>>
>> Comments, feedbacks?
>>
>
> An updated patch.
This patch enables the do_assignments hook only if
CALL_LDEMUL_DO_ASSIGNMENTS is yes.
Any comments, feedbacks?
Thanks.
--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-a-do_assignments-hook-to-ldemul.patch
Type: text/x-patch
Size: 28788 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20160505/d749c196/attachment.bin>
More information about the Binutils
mailing list