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

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, jakehehrlich, MaskRay.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.
mcgrathr added a comment.

I'm not sure what all tests this should have.  It was surprisingly easy to implement and seems to work perfectly.  I've done a Fuchsia build with it that shows binaries that look as they should by eyeball, though I haven't yet tested it at runtime.


mcgrathr added a comment.

Oh, and perhaps I'll update that comment since we do know why ELF specified RELA in the first place, which is for general-case reloc types for machines not like x86, where some reloc types don't apply to locations that store as many bits as the full address size that the addend can be in the general case.  When you have TEXTRELs, then any dynamic reloc can also be such a type.  For newer ABIs that never allowed TEXTRELs, I think people just assumed that the encoding for dynamic relocs should match the encoding for ET_REL relocs without thinking about it deeply.  In the ABIs we're concerned with today, the only narrow reloc type that might appear as a dynamic reloc is ABS32 in ELFCLASS64, but limiting addends to 32 bits is no more limiting than the rules of the small memory model, so you just wouldn't use rela->rel encoding with large memory model (or you could punt only for specific programs when you hit an out of range addend for real, which seems unlikely).  (A similar constraint has always been tolerated for R_ARM_MOV[WT]* relocs where EM_ARM is canonically REL rather than RELA and so addends used in relocs for movw/movt instructions are limited to 16 bits though the address computation is always a full 32 bit computation before the reloc stores the high or low 16 bits.)


This rewrites RELA to REL as well as generating RELR.  This is a
more compact representation for dynamic linker ABIs that support
both REL and RELA encodings (such as Fuchsia's).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79664

Files:
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/Options.td


Index: lld/ELF/Options.td
===================================================================
--- lld/ELF/Options.td
+++ lld/ELF/Options.td
@@ -286,7 +286,7 @@
 
 defm pack_dyn_relocs:
   Eq<"pack-dyn-relocs", "Pack dynamic relocations in the given format">,
-  MetaVarName<"[none,android,relr,android+relr]">;
+  MetaVarName<"[none,android,relr,rel+relr,android+relr]">;
 
 defm use_android_relr_tags: B<"use-android-relr-tags",
     "Use SHT_ANDROID_RELR / DT_ANDROID_RELR* tags instead of SHT_RELR / DT_RELR*",
Index: lld/ELF/Driver.cpp
===================================================================
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -734,18 +734,24 @@
   return {BuildIdKind::None, {}};
 }
 
-static std::pair<bool, bool> getPackDynRelocs(opt::InputArgList &args) {
+static std::tuple<bool, bool, bool> getPackDynRelocs(opt::InputArgList &args) {
   StringRef s = args.getLastArgValue(OPT_pack_dyn_relocs, "none");
   if (s == "android")
-    return {true, false};
+    return {true, false, false};
   if (s == "relr")
-    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};
+    return {true, true, false};
 
   if (s != "none")
     error("unknown -pack-dyn-relocs format: " + s);
-  return {false, false};
+  return {false, false, false};
 }
 
 static void readCallGraph(MemoryBufferRef mb) {
@@ -1118,8 +1124,8 @@
 
   std::tie(config->buildId, config->buildIdVector) = getBuildId(args);
 
-  std::tie(config->androidPackDynRelocs, config->relrPackDynRelocs) =
-      getPackDynRelocs(args);
+  std::tie(config->androidPackDynRelocs, config->relrPackDynRelocs,
+           config->relPackDynRelocs) = getPackDynRelocs(args);
 
   if (auto *arg = args.getLastArg(OPT_symbol_ordering_file)){
     if (args.hasArg(OPT_call_graph_ordering_file))
@@ -1213,12 +1219,20 @@
   // different mechanisms in the first place, but this is how the spec is
   // defined.
   //
-  // You cannot choose which one, Rel or Rela, you want to use. Instead each
-  // ABI defines which one you need to use. The following expression expresses
-  // that.
+  // The psABI for each processor specifies the canonical one to use.  The
+  // following expression expresses that.  But actual compatibility depends on
+  // the dynamic linker implementation (some can handle either format), so when
+  // targetting a particular OS environment with known dynamic linking ABI
+  // support, a contrary choice can be made.  The REL format is more compact,
+  // and when TEXTRELs are not supported, all the dynamic relocation types that
+  // can be emitted encode their in-section addends straightforwardly with
+  // sufficient range for all cases supported by the small memory model (or
+  // never have addends at all, such as GLOB_DAT and JMP_SLOT).
   config->isRela = m == EM_AARCH64 || m == EM_AMDGPU || m == EM_HEXAGON ||
                    m == EM_PPC || m == EM_PPC64 || m == EM_RISCV ||
                    m == EM_X86_64;
+  if (config->relPackDynRelocs)
+    config->isRela = false;
 
   // If the output uses REL relocations we must store the dynamic relocation
   // addends to the output sections. We also store addends for RELA relocations
Index: lld/ELF/Config.h
===================================================================
--- lld/ELF/Config.h
+++ lld/ELF/Config.h
@@ -187,6 +187,7 @@
   bool printGcSections;
   bool printIcfSections;
   bool relocatable;
+  bool relPackDynRelocs;
   bool relrPackDynRelocs;
   bool saveTemps;
   llvm::Optional<uint32_t> shuffleSectionSeed;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79664.262995.patch
Type: text/x-patch
Size: 3887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200509/0420ba1e/attachment.bin>