[PATCH] D24728: [ARM][LLD] Add support for .ARM.exidx sections

[Peter Smith via llvm-commits]

Thanks for the comments. I'll update D24728 with a rebased version +
updated test. I'll start a new review with a split patch so that we
can see what it looks like. My preference would be to get ld -r
working in all use cases but executables/shared objects are way more
important right now.

On 27 September 2016 at 15:55, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> Another interesting thing I noticed: it looks like gold doesn't
> implement the order restriction at all. The reason things work is
> because they are concatenated in the same order as the sections they
> point to.
>
> For example, given the script
>
> SECTIONS
> {
>   .text : { test1.o(.text) test3.o(.text) test2.o(.text) }
> }
>
> and a command line of "test1.o test2.o test3.o", the output .ARM.exidx
> will be in the wrong order.
>
> Given all this, I think we should split the patch. A suggestion of
> steps would be:
>
> * Make sure we gc the sections correctly. That is: a .ARM.exidx
> doesn't keep the target live, it is the other way around. We keep it
> if and only if what it points to is kept.
>
> * Make sure we combine .ARM.exidx the same way we combine their target
> sections with -r. For example, if we have
>
> -------------------------------
>         .section        .foo,"ax",%progbits,unique,1
>         .fnstart
>         b       external
>         .cantunwind
>         .fnend
>
>         .section        .foo,"ax",%progbits,unique,2
>         .fnstart
>         b       external
>         .cantunwind
>         .fnend
>
>         .section        .bar,"ax",%progbits,unique,1
>         .fnstart
>         b       external
>         .cantunwind
>         .fnend
>
>         .section        .bar,"ax",%progbits,unique,2
>         .fnstart
>         b       external
>         .cantunwind
>         .fnend
> ------------------------
>
> We should end up with two .ARM.exidx  sections, one for foo and one
> for bar. This is similar to relocations.
>
> * Set the correct link in the output sections.
> * Set other metadata (PT_ARM_EXIDX).
>
> This would put us in the same position as gold: it works short of an
> odd linker script, at which point we can evaluate if we need more.
>
> Cheers,
> Rafael
>
> On 27 September 2016 at 15:25, Peter Smith <peter.smith at linaro.org> wrote:
>> I'll update the patch, some recent changes since I submitted the
>> review have shifted around the addresses so the table starts at a
>> different address.
>>
>> On 27 September 2016 at 14:28, Rafael Espíndola
>> <rafael.espindola at gmail.com> wrote:
>>> I am getting a test failure:
>>>
>>> // CHECK-SCRIPT-NEXT: 114: 1e ff 2f e1 bx lr
>>>
>>> Cheers,
>>> Rafael
>>>
>>>
>>> On 27 September 2016 at 13:40, Rafael Espíndola
>>> <rafael.espindola at gmail.com> wrote:
>>>> On 27 September 2016 at 12:48, Peter Smith <peter.smith at linaro.org> wrote:
>>>>> Thanks for the feedback, unfortunately I'm struggling to find another
>>>>> way that doesn't break the ABI. For better or worse the ARM ABI chose
>>>>> to use SHF_LINK_ORDER so its what I have to work with, and is the only
>>>>> thing I can assume that other linker's have implemented.
>>>>>
>>>>> The general problem I have is that the output of a relocatable link
>>>>> can be fed into other links on any ABI compatible linker with the
>>>>> final combination of .ARM.exidx sections in the final exe/so making
>>>>> sense whichever linker does the final link. To be compatible with
>>>>> other linkers such as ld.bfd and ld.gold two conditions have to hold:
>>>>> 1.) Any .ARM.exidx Output Section needs to have a SHF_LINK_ORDER flag
>>>>> set and a sh_link to the executable section it describes
>>>>> 2.) Regardless of what order the linker combines the sections in the
>>>>> final link, when all .ARM.exidx sections are combined into one section
>>>>> they are in the correct order.
>>>>>
>>>>> The only things that I can think of that satisfies the second constraint are:
>>>>> - Do not combine any .ARM.exidx input section or any section that a
>>>>> .ARM.exidx section has a sh_link to.
>>>>> - For each output section S, combine the .ARM.exidx sections that have
>>>>> sh_links to input sections in S into a single output section.
>>>>>
>>>>> Combining all the .ARM.exidx sections into a single output section and
>>>>> sorting it won't work in all cases. A final link that consumes the
>>>>> relocatable object can change the order of any pair of sections,
>>>>> breaking the sorting done for the relocatable object, and as we can't
>>>>> guarantee that the final linker will resort the table the final output
>>>>> could be incorrect.
>>>>>
>>>>> On the assumption that we want to keep the output of ld -r ABI
>>>>> compatible we will need to produce a relocatable output file that
>>>>> another linker can use SHF_LINK_ORDER to process. It is true that
>>>>> internally to lld we don't need to use the SHF_LINK_ORDER to mechanism
>>>>> to do this, we just need to put it in at the end.
>>>>>
>>>>> The most difficult part is to create the correct number of .ARM.exidx
>>>>> output sections in the output. Adding the SHF_LINK_ORDER and sh_info
>>>>> is relatively simple. There are a number of possible implementations:
>>>>> 1.) The no combination option as implemented by D24728, using a flag
>>>>> to say don't combine.
>>>>> 2.) Use the naming convention of .ARM.exidx.section_name to construct
>>>>> the OutputSections (can sort these as per the final link)
>>>>> 3.) Do an ARM specific renaming pass of input sections such that
>>>>> createOutputSection does the right thing.
>>>>> 4.) Separate all .ARM.exidx sections from the input sections and do a
>>>>> separate ARM specific createOutputSection pass to create the
>>>>> OutputSections.
>>>>>
>>>>> Do you have any thoughts or preferences? I'm a bit stuck right now as
>>>>> I can't change the ABI, and I can't see a way of writing the output
>>>>> file in such a way that another linker will be guaranteed to do the
>>>>> right thing.
>>>>>
>>>>> Peter
>>>>>
>>>>> P.S.
>>>>> If you are vaguely interested, the ARM ABI has a design principle of
>>>>> smart format dumb linker. In practical terms all decisions a linker
>>>>> needs to make for a correct (if not optimal) output are explicit in
>>>>> the ELF metadata. Requiring linkers to sort the exception tables would
>>>>> have broken that principle. I also suspect that the exception table
>>>>> design was heavily influenced by the Itanium ABI which also has
>>>>> exception tables that are ordered using SHF_LINK_ORDER.
>>>>
>>>> Given the hack that .eh_frame is, that seems the correct design choice.
>>>>
>>>> I will take a look at the patch itself, but I think two options should
>>>> have a fairly clean implementation:
>>>>
>>>> * Don't merge sections that is pointed by a SHF_LINK_ORDER section.
>>>> * Assume that the merging is compatible. That is, that if a group of
>>>> SHF_LINK_ORDER sections is merged, the sections they point to are also
>>>> merged. I case where that doesn't happen (because of an odd linker
>>>> script) is considered garbage in, garbage out.
>>>>
>>>> Cheers,
>>>> Rafael