[PATCH] elf: Support DT_RELR relative relocation format [BZ #27924]

[Fangrui Song]

On 2021-10-08, H.J. Lu wrote:
>On Fri, Oct 8, 2021 at 10:37 AM Fangrui Song <maskray@google.com> wrote:
>>
>> On 2021-10-08, H.J. Lu wrote:
>> >On Thu, Oct 7, 2021 at 11:58 PM Fangrui Song via Binutils
>> ><binutils@sourceware.org> wrote:
>> >>
>> >> PIC objects (especially PIE and symbolic shared objects) usually have many
>> >> relative relocations. In 2017/2018, SHT_RELR/DT_RELR was proposed on
>> >> https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
>> >> ("Proposal for a new section type SHT_RELR") and welcomed by many parties
>> >> (including Solaris). This packed format can typically save 95% dynamic
>> >> relocation section size for PIE. The vaddr size of a PIE can be 10% smaller.
>> >>
>> >> * Chrome OS folks have carried a local patch for a while (latest version:
>> >>   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
>> >>   I.e. this feature has been battle tested.
>> >> * Android bionic supports DT_RELR.
>> >> * The Linux kernel has supported CONFIG_RELR since 2019-08
>> >>   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
>> >> * A musl patch (by me) exists but is not applied:
>> >>   https://www.openwall.com/lists/musl/2019/03/06/3
>> >>
>> >> I believe upstream glibc should support DT_RELR to benefit all Linux
>> >> distributions.
>> >>
>> >> As of linker support (to the best of my knowledge):
>> >>
>> >> * LLD support DT_RELR.
>> >> * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
>> >>   has a gold patch.
>> >> * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
>> >>
>> >> I wish that GNU ld and gold maintainers can implement the feature as well :)
>> >
>> >This feature should be implemented in GNU linker first before getting
>> >into glibc.
>> >
>> >
>> >H.J.
>>
>> Why? I think the situation is quite different from other features where
>> we generally want producers before consumers.  The feature is validated
>> by multiple parties and has multiple independent implementations and has
>> an implementation in a glibc supported linker (LLD).
>
>This feature should be validated by the default glibc linker.   Otherwise,
>things can go wrong unnoticed.

I wish you did not get overly cautious on this matter:)

The original ChromeOS patch
https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32/0004-sys-libs-glibc-add-support-for-SHT_RELR-sections.patch
is indeed intrusive and may cause maintenance problem in my view
(with my recent experience working on the loader).
Its elf_machine_relr_relative callback and its change to elf/do-rel.h
make me concerned.

This patch is clean and touches just one place. The core logic is just
the 26-line ELF_DYNAMIC_DO_RELR in an isolated place.
The ELF_DYNAMIC_RELOCATE framework hasn't change since 1995.
The new logic doesn't touch symbol resolution, so it will require
minimum maintenance burden.

Since I put up the patch, I certainly sign up for maintaining this piece
of code in case of a rare break.

>> Landing into glibc first can do GNU linker folks a favor: they don't
>> need to apply the glibc patch themselves to validate correctness of the
>> linker change.
>
>This has never been a real issue.  We prefer to test linker correctness
>independent of host, library and/or CPU.

May I ask you, Alan, or Nick, or other GNU ld maintainers to take up the work? :)

Sorry that I cannot deal with the sheer mount of GNU ld complexity by
myself. I will be happy to do validation, though.

I can give one hint that since .relr.dyn is sensitive to the relocated
locations' addresses care needs  to be taken to avoid size oscillation
especially on architectures which need range extension
thunks(veneers/stub groups).  A nice approach is to never shrink the
size of .relr.dyn (https://reviews.llvm.org/D67164)

https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/0006-gold-readelf-add-experimental-support-for-SHT_RELR-s.patch
mixes gold and readelf patches. Perhaps a maintainer wants to split it.
(--experimental-use-relr may be better spelled as --pack-dyn-relocs=relr
to be consistent with LLD. If you insist that ELF specific options
should use -z relr I am fine as well.)

(I just checked: Rahul Chaudhry <rahulchaudhry@google.com> no longer
works at Google so the email address will not work.)