[PATCH] D41046: [ELF] Make overlapping output sections an error

[Alexander Richardson via llvm-commits]

On 15 December 2017 at 04:15, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> Alexander Richardson via Phabricator <reviews at reviews.llvm.org> writes:
>
>> arichardson updated this revision to Diff 126272.
>> arichardson added a comment.
>>
>> Speed up lld invocation in test/ELF/many-alloc-sections.s lld invocation from 290s to <1s by not checking empty sections
>
> What happens if the sections are 1 byte long? What is the performance
> impact that you get with this patch when linknig the linux kernel?


With 1 byte and my patch takes 1.08-1.14 seconds vs 1.04-1.12 for LLD
without the patch. This is not a very thorough test, I linked it 20
times and measured using `/usr/bin/time` but it seems to me like the
overhead is barely measurable even for a crazy test with 32768 output
sections. With a normal file that less than 40 output sections I see
any noticeable overhead.
I doubt the performance impact for the linux kernel is even measurable
since it has 32 output sections and looping over those 3 times and
comparing the offsets to the next one should just be noise compared to
the total link time.

The reason it was so slow before is that it was comparing every
section to every one that follows. I added a break in the nested loop
once it can't be an overlapping section anymore so in almost every
case (unless we actually have an overlap that nested loop will only
run once. Removing empty ones also speeds it up quite a lot since if
they are zero size we would have to keep comparing until the next
non-zero size section. In this synthetic test case that would mean
comparing almost all of them. I doubt any real binary would have
thousands of zero size sections.

Should I update the many-alloc-sections.s test case to also check 1
byte sections instead of only zero size sections to make it a bit more
interesting?


Furthermore, I could disable the check if there is no custom linker
script and we can assume that the internal address assignment
algorithm works as expected. Would that make the patch more
acceptable?


I'm not sure it makes sense to check for overlapping input sections
since the offsets should be assigned sensibly there anyway. Also there
are way more input sections than output sections so adding a check for
every input section (especially if you compile with
-ffunction-sections) would add noticeable overhead even if it is just
a very simple check.