[lld] r324221 - [ELF] Implement --[no-]apply-dynamic-relocs option.

[Rafael Avila de Espindola via llvm-commits]

Peter Smith via llvm-commits <llvm-commits at lists.llvm.org> writes:

> Author: psmith
> Date: Mon Feb  5 02:15:08 2018
> New Revision: 324221
>
> URL: http://llvm.org/viewvc/llvm-project?rev=324221&view=rev
> Log:
> [ELF] Implement --[no-]apply-dynamic-relocs option.
>
> 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.

For what it is worth it, the freebsd dynamic linker used to have the
same bug:

https://reviews.freebsd.org/D9180

> Modified: lld/trunk/ELF/Driver.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=324221&r1=324220&r2=324221&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Driver.cpp (original)
> +++ lld/trunk/ELF/Driver.cpp Mon Feb  5 02:15:08 2018
> @@ -597,6 +597,8 @@ static int parseInt(StringRef S, opt::Ar
>  void LinkerDriver::readConfigs(opt::InputArgList &Args) {
>    Config->AllowMultipleDefinition =
>        Args.hasArg(OPT_allow_multiple_definition) || hasZOption(Args, "muldefs");
> +  Config->ApplyDynamicRelocs = Args.hasFlag(OPT_apply_dynamic_relocs,
> +                                            OPT_no_apply_dynamic_relocs, true);

Please change the default.

It is OK to have a --handle-bug-in-other-tool option, but it should
never be the default so that we reduce the chance of future tools having
the same bug.


> Modified: lld/trunk/ELF/SyntheticSections.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=324221&r1=324220&r2=324221&view=diff
> ==============================================================================
> --- lld/trunk/ELF/SyntheticSections.cpp (original)
> +++ lld/trunk/ELF/SyntheticSections.cpp Mon Feb  5 02:15:08 2018
> @@ -1207,10 +1207,11 @@ void RelocationBaseSection::addReloc(uin
>                                       uint64_t OffsetInSec, bool UseSymVA,
>                                       Symbol *Sym, int64_t Addend, RelExpr Expr,
>                                       RelType Type) {
> -  // REL type relocations don't have addend fields unlike RELAs, and
> -  // their addends are stored to the section to which they are applied.
> -  // So, store addends if we need to.
> -  if (!Config->IsRela && UseSymVA)
> +  // We store the addends for dynamic relocations for both REL and RELA
> +  // relocations for compatibility with GNU Linkers. There is some system
> +  // software such as the Bionic dynamic linker that uses the addend prior
> +  // to dynamic relocation resolution.
> +  if ((!Config->IsRela || Config->ApplyDynamicRelocs) && UseSymVA)


Please change this to just

if (Config->ApplyDynamicRelocs && UseSymVA)

by making Config->ApplyDynamicRelocs always true if we are not using Elf_Rela.


Cheers,
Rafael