[PATCH] D102296: [ELF] getRelocatedSection: remove the check for ET_REL object file

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D102296#2778439 <https://reviews.llvm.org/D102296#2778439>, @Amir wrote:

> @jhenderson: thanks for the suggestion. I'm going to add the Object unit test for this change.
> The motivation is the use of relocations by BOLT for function reordering optimization (specifically, to find references to functions). We recommend that users keep relocations in the final binary: https://github.com/facebookincubator/BOLT#usage

I think I follow now, thanks. Dynamic relocations should probably still be omitted by this lookup, i.e. `getRelocatedSection` only deals with static relocations. Not all dynamic relocation sections have an associated section, so this function will fail on them.

> llvm-dwarfdump is relying on getRelocatedSection() to return section_end() for ELF files of types other than relocatable objects. We've changed the function to return relocatable section for other types of ELF files. As a result, llvm-dwarfdump started re-processing relocations for sections that already had relocations applied, e.g. in executable files, and this resulted in wrong values reported.

It seems to me that for Elf_Rel relocations, you can't really reapply them, because the addend has been overwritten by the final patched value. So, as I understand it, the only way you can reliably know how to reapply relocations is with Elf_Rela relocations. I think this means you need to change llvm-dwarfdump too, to only reapply relocations for ET_REL objects? (It's harmless to do so for other ET_* kinds, iff the relocation types are Elf_Rela of course, but it's pointless work).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102296/new/

https://reviews.llvm.org/D102296