[PATCH] D63121: [ELF] Make the rule to create relative relocations in a writable section stricter

[Peter Smith via Phabricator via llvm-commits]

peter.smith added inline comments.


================
Comment at: lld/trunk/ELF/Relocations.cpp:922
   if (CanWrite) {
-    // R_GOT refers to a position in the got, even if the symbol is preemptible.
-    bool IsPreemptibleValue = Sym.IsPreemptible && Expr != R_GOT;
-
-    if (!IsPreemptibleValue) {
+    if ((!Sym.IsPreemptible && Type == Target->SymbolicRel) || Expr == R_GOT) {
+      // If this is a symbolic relocation to a non-preemptable symbol, or an
----------------
I'm not sure that there is always just one relocation per Target that is a SymbolicRel. On Arm there are a few implementation defined relocations like R_ARM_TARGET1 that can be R_ARM_ABS32 or R_ARM_REL32 depending on the platform. I think that this needs to be a callback to Target so that more than one relocation can apply.


================
Comment at: lld/trunk/ELF/Relocations.cpp:927
       addRelativeReloc(&Sec, Offset, &Sym, Addend, Expr, Type);
       return;
     } else if (RelType Rel = Target->getDynRel(Type)) {
----------------
Can we guarantee that Sym is in the dynamic symbol table after we have narrowed the relocation types accepted?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63121