[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#2769406 <https://reviews.llvm.org/D102296#2769406>, @Amir wrote:

> There are options for adding the test, too:
>
> 1. As a unit test for getRelocatedSection that would check that this function works for executable files with relocations.
> 2. As a test in lld folder that would run llvm-mc; ld.lld with --emit-relocs and then a client of getRelocatedSection (`llvm-objdump -dr` or something similar). Cons: test is unrelated to lld.
> 3. As a prepared ELF binary test for LLVM clients of getRelocatedSection.
>
> What do you think would be an appropriate solution?

You can use yaml2obj to create an ET_EXEC (or whatever) object with static relocations. There are many good examples of using yaml2obj to create an object in places like the llvm-readobj or even the yaml2obj tests themselves - just find one that has relocations and change the type. The fix isn't related to LLD (directly), so I don't think we should test it in the LLD tests (although it might be a good candidate for my planned cross-project-tests  - see D95339 <https://reviews.llvm.org/D95339> and related patches). However, my personal opinion is that as this behaviour change is for the Object library, it should be a new Object unit test, i.e. option 1.

What's the motivation for this change anyway? An executable (whether built with --emit-relocs or not) will already have had its static relocations applied. Applying dynamic relocations seems likely to be impossible, since by their very nature they can't be applied until the program has been loaded in memory/dynamic libraries loaded etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102296