This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF] Implement --[no-]apply-dynamic-relocs option.
ClosedPublic

Authored by peter.smith on Feb 1 2018, 7:57 AM.

Details

Summary

When resolving dynamic RELA relocations the addend is taken from the relocation and not the place being relocated. Accordingly lld does not write the addend field to the place like it would for a REL relocation. Unfortunately there is some system software, in particlar dynamic loaders such as Bionic's linker64 that use the value of the place prior to
relocation to find the offset that they have been loaded at. Both gold and bfd control this behavior with the --[no-]apply-dynamic-relocs option.

This change implements the option and defaults it to true for compatibility with gold and bfd. I've chosen the same default (--apply-dynamic-relocs) but I don't have a strong opinion.

I found this out the hard way while trying to see if LLD could be used as the linker for Android AOSP. The resulting image wouldn't boot with LLD linking the Bionic dynamic loader linker64. It turns out that the Bionic loader makes use of the addend in the place prior to relocation:

In Bionic linker_main.cpp (https://android.googlesource.com/platform/bionic/+/master/linker/linker_main.cpp)

extern "C" ElfW(Addr) __linker_init(void* raw_args) {
  KernelArgumentBlock args(raw_args);

  // AT_BASE is set to 0 in the case when linker is run by iself
  // so in order to link the linker it needs to calcuate AT_BASE
  // using information at hand. The trick below takes advantage
  // of the fact that the value of linktime_addr before relocations
  // are run is an offset and this can be used to calculate AT_BASE.
  static uintptr_t linktime_addr = reinterpret_cast<uintptr_t>(&linktime_addr);
  ElfW(Addr) linker_addr = reinterpret_cast<uintptr_t>(&linktime_addr) - linktime_addr;

This results in a R*RELATIVE relocation on linktime_addr with the addend field on RELA containing the address of linktime_addr. If 0 is written to the place this calculation goes wrong and the dynamic linker fails to relocate itself. I've raised https://issuetracker.google.com/issues/72789859 on Bionic to see if there is an alternative mechanism that they can use.

Both gold and recent versions of BFD (Binutils 2.26 https://sourceware.org/ml/binutils/2016-04/msg00407.html) write the addend into the place. Due to another bug in Bionic https://android-review.googlesource.com/c/platform/bionic/+/176902 there is an option --[no-]apply-dynamic-relocs that controls whether the addends are written to the place.

For reference it seems like glibc on aarch64 uses a similar trick, but relies on a different convention.

/* Return the link-time address of _DYNAMIC.  Conveniently, this is the
   first element of the GOT. */
static inline ElfW(Addr) __attribute__ ((unused))
elf_machine_dynamic (void)
{
  extern const ElfW(Addr) _GLOBAL_OFFSET_TABLE_[] attribute_hidden;
  return _GLOBAL_OFFSET_TABLE_[0];
}

/* Return the run-time load address of the shared object.  */

static inline ElfW(Addr) __attribute__ ((unused))
elf_machine_load_address (void)
{
  /* To figure out the load address we use the definition that for any symbol:
     dynamic_addr(symbol) = static_addr(symbol) + load_addr

    _DYNAMIC sysmbol is used here as its link-time address stored in
    the special unrelocated first GOT entry.  */

    extern ElfW(Dyn) _DYNAMIC[] attribute_hidden;
    return (ElfW(Addr)) &_DYNAMIC - elf_machine_dynamic ();
}

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith created this revision.Feb 1 2018, 7:57 AM
ruiu added inline comments.Feb 1 2018, 12:30 PM
ELF/Options.td
52 ↗(On Diff #132397)

Remove the full stop.

179 ↗(On Diff #132397)

Ditto

ELF/SyntheticSections.cpp
1210–1214 ↗(On Diff #132397)

Can you rewrite entire comment to say that we basically store addends both to RELA relocations and to the locations where the relocations are applied, even though it is redundant, because that's what GNU linkers do, and some dynamic linkers such as Bionic's depend on it.

grimar added a subscriber: grimar.Feb 1 2018, 11:00 PM
grimar added inline comments.
test/ELF/dynamic-got-rela.s
6 ↗(On Diff #132397)

Seems it should be:
llvm-readobj -r -s -l -section-data %t2.so

With corresponding change to NOAPPLYDYNREL block below.

Thanks to both for the comments. I've incorporated them in the diff.

ruiu accepted this revision.Feb 2 2018, 12:52 PM

LGTM

ELF/SyntheticSections.cpp
1211 ↗(On Diff #132548)

I'd prefer to add "even though it's redundant" because it'd be the first thing to wonder when someone read this comment.

This revision is now accepted and ready to land.Feb 2 2018, 12:52 PM
This revision was automatically updated to reflect the committed changes.