[PATCH] D111181: llvm-objcopy][ELF] Don't assume RELA sections are dynamic if they carry the SHF_ALLOC flag

[James Henderson via Phabricator via llvm-commits]

jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

> So let's replace this with a check against the ELF file type, which is more appropriate: partially linked binaries of type ET_REL cannot have dynamic relocation sections, and ET_EXEC/ET_DYN fully linked cannot have the static ones. Note that this requires that reading the section headers is deferred until after we figure out the file type, which also means that finding the executable header offset

Sorry, but this statement is incorrect. See for example LLD's --emit-relocs option: static relocations can be emitted in a fully linked output file. I guess you may have discovered this already though?

The only real way of being able to truly tell whether a relocation section is a dynamic relocation section is if it is referred to by the dynamic table via one of the appropriate tags. We previously didn't go down this route, as it requires doing something non-trivial that llvm-objcopy didn't do before.

Have you got an actual output that demonstrates this issue, or is it merely theoretical? If you do have an actual output, why are the relocation sections SHF_ALLOC, but not dynamic relocations? Such relocations would take up memory space in the output file, not to mention that I suspect some linkers would get very confused by them/throw them away etc.

Please ensure your patch has testing for the new behaviour, that would have failed before this change, and that you upload your diff with full context (use -U999999 if using tools like `patch` to generate the diff). Please also make sure you have used `clang-format` to reformat your changes.


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

https://reviews.llvm.org/D111181