[PATCH] D79664: [ELF] Support --pack-dyn-relocs=rel+relr

Sat May 9 14:22:03 PDT 2020

mcgrathr added a comment.

In D79664#2028262 <https://reviews.llvm.org/D79664#2028262>, @MaskRay wrote:

> I haven't thought much on this yet. First impression: why isn't the RELA->REL toggle a new option? It does not seem to fit with the rest of --pack-dyn-relocs which selects Android packed dynamic relocations or RELR or both.

--pack-dyn-relocs seems exactly appropriate to me since it's about packing the dynamic relocs.  It does RELA->REL conversion just as the existing option does REL(A)->RELR conversion.  But I'm not concerned with the switch syntax.  I'm fine with whatever syntax people prefer.  (For example, I don't at all think it's important to disallow the RELA->REL conversion without RELR support, I just don't care about that mode at all.)  I probably wouldn't have added --pack-dyn-relocs with its string syntax in the first place, but instead just added separate `-z android-relocs` and `-z relr` switches so the new one would be `-z rel` or something.  But given --pack-dyn-relocs exists, it seems like the natural place for this to me now.

> Regarding tests, you can add `llvm-readelf -S` RUN lines to check `.rel.dyn` or `.rela.dyn`. See some newly touched test files.
> 
> musl ld.so supports both REL and RELA. I can probably finish the patch and test it on musl.

Thanks very much!  Fuchsia's current dynamic linker is a (distant) fork from musl and supports both as well.  I haven't looked at musl lately so perhaps it hasn't added RELR support as Fuchsia did (where RELR encoding is the preferred default).  So using and testing with musl (or older musl) might be a good reason to support doing RELA->REL encoding without doing RELR encoding.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79664