llvm dev - Nov 2017 - [RFC] Making .eh_frame more linker-friendly

I performed tests basing on first diff of https://reviews.llvm.org/D40352.
(Creates .eh_frame for each .text.*, sets SHF_LINK_ORDER and .sh_link of created
.eh_frame section to point to corresponding .text.)

With use of GNU ld (GNU Binutils) 2.29.51.20171006 it reports errors when
linking sample apps:
~/LLVM/Release/bin/clang++ test.cpp -ffunction-sections -o test.o
/usr/local/bin/ld: .eh_frame has both ordered [`.eh_frame' in
/tmp/test-dbc52e.o] and unordered
                   [`.eh_frame' in
/usr/lib/gcc/x86_64-linux-gnu/5.4.1/../../../x86_64-linux-gnu/crt1.o] sections
/usr/local/bin/ld: final link failed: Bad value

Looks it's code explicitly restricts mixing sections with link order flag
and without:
https://github.com/gittup/binutils/blob/gittup/bfd/elflink.c#L9991

With GNU gold (GNU Binutils 2.29.51.20171006) 1.14 have an assert:
~/LLVM/Release/bin/clang++ test.cpp -ffunction-sections -o test.o
/usr/local/bin/ld: internal error in layout_eh_frame_section, at
.././../gold/object.cc:1309
It is that place:
https://github.com/gittup/binutils/blob/gittup/gold/object.cc#L1372
Did not investigate it, but it looks it is place
(https://sourceware.org/ml/binutils/2009-06/msg00097.html)
mentioned in comment for
https://marc.info/?l=llvm-commits&m=144683596826489.

LLD ~head fails here
https://github.com/llvm-mirror/lld/blob/master/ELF/InputFiles.cpp#L392 as
we are trying to do cast to InputSection, though .eh_frame is not regular
InputSection but EhInputSection.

Basing on all above I supposed it should be fine to not emit SHF_LINK_ORDER
flags for start.
I changed implementation so that it emits multiple .eh_frames and sets sh_link
field for
SHT_X86_64_UNWIND sections, but don't set the SHF_LINK_ORDER flag. In that
way only
x64 target is affected and SHT_X86_64_UNWIND sections are known to be .eh_frame
so can be
handled on linker side not by name, what is also nice for start it seems.
(Uploaded current version as diff 2 here: https://reviews.llvm.org/D40352)

With such change LLD and bfd links fine, and I was able to test clang link time.
Clang does not
use exceptions, but x64 ABI still requires .eh_frame sections for functions, and
result objects
contains many of them so I think it is reasonable test. What I did: built clang
with and without
patch, used result compiler binaries for producing 2 sets of release clang
objects
(flags were -fPIC -std=c++11 -ffunction-sections -fdata-sections), used them for
testing link time
with use of ld.bfd and LLD.

Results (30 runs for each):
1) ld.bfd link time before patch: 2,922310802 seconds time elapsed ( +-  0,13% )
   Size: 80 667 832 bytes
2) ld.bfd link time after patch: 3,773565826 seconds time elapsed ( +-  0,13% )
   Size: 80 667 832 bytes
3) LLD link time before patch: 0,419247946 seconds time elapsed ( +-  0,48% )
   Size: 80 738 240 bytes
4) LLD link time after patch: 0,460139012 seconds time elapsed ( +-  0,44% )
   Size: 80 738 240 bytes

There is no difference in size (that is probably expected as linkers do
deduplication optimization, so looks right for me),
slowdown for bfd is about 29%, for LLD about 9.7%.

What we gonna do next ? My plan is to prepare demo patch for LLD to stop parsing
.eh_frames
for GC step and benchmark the results with use of -gc-sections. It also can show
amount of LLD
code reduction/simplification we can have.


George.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171123/84fc462e/attachment.html>

>In that way only x64 target is affected and SHT_X86_64_UNWIND sections are
known to be .eh_frame so can be
>handled on linker side not by name, what is also nice for start it seems.
>(Uploaded current version as diff 2 here: https://reviews.llvm.org/D40352)
Taking this back. If we will emit multiple eh_frame  we anyways anyways will
need either use some flag or
handle ".eh_frame" by name for other targets, so doing that for
SHT_X86_64_UNWIND did not have much sence.


George.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171123/b3587fba/attachment-0001.html>

>What we gonna do next ? My plan is to prepare demo patch for LLD to stop
parsing .eh_frames
>for GC step and benchmark the results with use of -gc-sections. It also can
show amount of LLD
>code reduction/simplification we can have.
>
>George.

Demo patch for LLD that stops parsing .eh_frame during GC stage

is: https://reviews.llvm.org/D40484. With it LLD GC code is slightly simpler.

I tested it both together with lib/mc patch for emiting multiple .eh_frames

(https://reviews.llvm.org/D40352) and along, it looks it has no any visible
effect on

perfomance by itself.


George.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171127/a0c58acf/attachment.html>

> With GNU gold (GNU Binutils 2.29.51.20171006) 1.14 have an assert:
> ~/LLVM/Release/bin/clang++ test.cpp -ffunction-sections -o test.o
> /usr/local/bin/ld: internal error in layout_eh_frame_section, at
> .././../gold/object.cc:1309
> It is that place:
> https://github.com/gittup/binutils/blob/gittup/gold/object.cc#L1372
> Did not investigate it, but it looks it is place
> (https://sourceware.org/ml/binutils/2009-06/msg00097.html)
> mentioned in comment for
> https://marc.info/?l=llvm-commits&m=144683596826489.
I've committed a patch in gold that should fix this problem:

   https://sourceware.org/ml/binutils/2017-11/msg00541.html

Can you try gold again with this patch applied? You should at least
get a little further.

If it still doesn't work, could I trouble you for a sample object file?

-cary

>> With GNU gold (GNU Binutils 2.29.51.20171006) 1.14 have an assert:
>> ~/LLVM/Release/bin/clang++ test.cpp -ffunction-sections -o test.o
>> /usr/local/bin/ld: internal error in layout_eh_frame_section, at
>> .././../gold/object.cc:1309
>> It is that place:
>> https://github.com/gittup/binutils/blob/gittup/gold/object.cc#L1372
>> Did not investigate it, but it looks it is place
>> (https://sourceware.org/ml/binutils/2009-06/msg00097.html)
>> mentioned in comment for
>> https://marc.info/?l=llvm-commits&m=144683596826489.
>
>I've committed a patch in gold that should fix this problem:
>
>   https://sourceware.org/ml/binutils/2017-11/msg00541.html
>
>Can you try gold again with this patch applied? You should at least
>get a little further.
>
>If it still doesn't work, could I trouble you for a sample object file?
>
>-cary
I'll try it soon and return with results, thanks !

Just a small clarification: your commit message saying that "LLVM is
experimenting with placing
.eh_frame sections in the COMDAT group", that is true for original
experiment which faced this gold issue
 (https://marc.info/?l=llvm-commits&m=144683596826489), current approach I
am experimented
with (https://reviews.llvm.org/D40352) sets sh_link of .eh_frame to
corresponding .text section instead.
Though anyways issue looks to be that gold did not like to have multiple
.eh_frame sections and that
is what both approaches faced with and your patch seeems fixes that.

George.