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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

> 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.

I learned that `-z` is reserved for ELF specific options in GNU ld. So, `-z` would indeed be more suitable. `-z rel` may be a good name. Are you going to create a feature request on GNU ld side:) ?

> 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.

I even posted a musl patch <https://www.openwall.com/lists/musl/2019/03/06/5> last year, but it could not be accepted because RELR is not in the standard. (https://groups.google.com/forum/#!topic/generic-abi/9OO5vhxb00Y "Ongoing Maintenance of the gABI" might make the situation better.)

`-z rel` does not need to be coupled with RELR and it has its own usefulness which can be easily justified. Dynamic relocations usually have 0 addend. GLOB_DAT, J[U]MP_SLOT and COPY always have 0 addend and do not need to read the implicit addend. The symbolic absolute relocation type usually has 0 addend. Unfortunately FreeBSD rtld and glibc ld.so are structured in the way that supporting both REL and RELA is not out-of-the-box.



================
Comment at: lld/ELF/Driver.cpp:743
-    return {false, true};
+    return {false, true, false};
+  // Note there is no plain "rel" option here since rela->rel rewriting is
+  // motivated by space savings and relr is the biggest win for that, so "rel"
+  // alone is more likely to be a typo for "relr" than an intentional selection
+  // of rela->rel without relr.
+  if (s == "rel+relr")
+    return {false, true, true};
   if (s == "android+relr")
-    return {true, true};
----------------
grimar wrote:
> arichardson wrote:
> > grimar wrote:
> > > aside: I wonder why we support "android+relr", but not "android,relr". The latter form looks more natural to me.
> > I agree. How about allowing comma-separated list where the order does not matter?
> It would look better to me.
Note, in compiler drivers (GCC+clang), -Wl, does not have escapes for commas, so commas in the linker generally does not work well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79664