Skip to content

[NFCI][ELF] Merge AgainstSymbol and AgainstSymbolWithTargetVA #150795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

jrtc27
Copy link
Collaborator

@jrtc27 jrtc27 commented Jul 26, 2025

The former is just a special case of the latter, ignoring the expr and
always just using the addend. If we use R_ADDEND as expr (which
previously had no effect, and so was misleadingly R_ABS not R_ADDEND in
all but one use) then we don't need to maintain this as a separate case.

This just leaves MipsMultiGotPage as a special case; the only difference
between the other two Kind values is what needsDynSymIndex returns.

Created using spr 1.3.5
@llvmbot
Copy link
Member

llvmbot commented Jul 26, 2025

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Jessica Clarke (jrtc27)

Changes

The former is just a special case of the latter, ignoring the expr and
always just using the addend. If we use R_ADDEND as expr (which
previously had no effect, and so was misleadingly R_ABS not R_ADDEND in
all but one use) then we don't need to maintain this as a separate case.

This just leaves MipsMultiGotPage as a special case; the only difference
between the other two Kind values is what needsDynSymIndex returns.


Full diff: https://github.com/llvm/llvm-project/pull/150795.diff

3 Files Affected:

  • (modified) lld/ELF/Relocations.cpp (+8-6)
  • (modified) lld/ELF/SyntheticSections.cpp (+5-8)
  • (modified) lld/ELF/SyntheticSections.h (+3-9)
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index ed53228359f86..7579def5865bb 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -885,10 +885,12 @@ static void addPltEntry(Ctx &ctx, PltSection &plt, GotPltSection &gotPlt,
                         RelocationBaseSection &rel, RelType type, Symbol &sym) {
   plt.addEntry(sym);
   gotPlt.addEntry(sym);
-  rel.addReloc({type, &gotPlt, sym.getGotPltOffset(ctx),
-                sym.isPreemptible ? DynamicReloc::AgainstSymbol
-                                  : DynamicReloc::AddendOnly,
-                sym, 0, R_ABS});
+  if (sym.isPreemptible)
+    rel.addReloc({type, &gotPlt, sym.getGotPltOffset(ctx),
+                  DynamicReloc::AgainstSymbol, sym, 0, R_ADDEND});
+  else
+    rel.addReloc({type, &gotPlt, sym.getGotPltOffset(ctx),
+                  DynamicReloc::AddendOnly, sym, 0, R_ABS});
 }
 
 void elf::addGotEntry(Ctx &ctx, Symbol &sym) {
@@ -899,7 +901,7 @@ void elf::addGotEntry(Ctx &ctx, Symbol &sym) {
   if (sym.isPreemptible) {
     ctx.mainPart->relaDyn->addReloc({ctx.target->gotRel, ctx.in.got.get(), off,
                                      DynamicReloc::AgainstSymbol, sym, 0,
-                                     R_ABS});
+                                     R_ADDEND});
     return;
   }
 
@@ -921,7 +923,7 @@ static void addGotAuthEntry(Ctx &ctx, Symbol &sym) {
   if (sym.isPreemptible) {
     ctx.mainPart->relaDyn->addReloc({R_AARCH64_AUTH_GLOB_DAT, ctx.in.got.get(),
                                      off, DynamicReloc::AgainstSymbol, sym, 0,
-                                     R_ABS});
+                                     R_ADDEND});
     return;
   }
 
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 690e9bf2d7d8b..6bb3aa2e5e745 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -1065,9 +1065,9 @@ void MipsGotSection::build() {
       // for the TP-relative offset as we don't know how much other data will
       // be allocated before us in the static TLS block.
       if (s->isPreemptible || ctx.arg.shared)
-        ctx.mainPart->relaDyn->addReloc(
-            {ctx.target->tlsGotRel, this, offset,
-             DynamicReloc::AgainstSymbolWithTargetVA, *s, 0, R_ABS});
+        ctx.mainPart->relaDyn->addReloc({ctx.target->tlsGotRel, this, offset,
+                                         DynamicReloc::AgainstSymbol, *s, 0,
+                                         R_ABS});
     }
     for (std::pair<Symbol *, size_t> &p : got.dynTlsSymbols) {
       Symbol *s = p.first;
@@ -1648,14 +1648,11 @@ uint64_t DynamicReloc::getOffset() const {
 int64_t DynamicReloc::computeAddend(Ctx &ctx) const {
   switch (kind) {
   case AddendOnly:
-  case AgainstSymbolWithTargetVA: {
+  case AgainstSymbol: {
     uint64_t ca = inputSec->getRelocTargetVA(
         ctx, Relocation{expr, type, 0, addend, sym}, getOffset());
     return ctx.arg.is64 ? ca : SignExtend64<32>(ca);
   }
-  case AgainstSymbol:
-    assert(sym != nullptr);
-    return addend;
   case MipsMultiGotPage:
     assert(sym == nullptr);
     return getMipsPageAddr(outputSec->addr) + addend;
@@ -1698,7 +1695,7 @@ void RelocationBaseSection::addAddendOnlyRelocIfNonPreemptible(
   // No need to write an addend to the section for preemptible symbols.
   if (sym.isPreemptible)
     addReloc({dynType, &isec, offsetInSec, DynamicReloc::AgainstSymbol, sym, 0,
-              R_ABS});
+              R_ADDEND});
   else
     addReloc(DynamicReloc::AddendOnly, dynType, isec, offsetInSec, sym, 0,
              R_ABS, addendRelType);
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index b72436372fbc1..5a2437c7961df 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -424,13 +424,9 @@ class DynamicReloc {
     /// Useful for various relative and TLS relocations (e.g. R_X86_64_TPOFF64).
     AddendOnly,
     /// The resulting dynamic relocation references symbol #sym from the dynamic
-    /// symbol table and uses #addend as the value of computeAddend(ctx).
+    /// symbol table and uses InputSection::getRelocTargetVA() for the final
+    /// addend.
     AgainstSymbol,
-    /// The resulting dynamic relocation references symbol #sym from the dynamic
-    /// symbol table and uses InputSection::getRelocTargetVA() + #addend for the
-    /// final addend. It can be used for relocations that write the symbol VA as
-    // the addend (e.g. R_MIPS_TLS_TPREL64) but still reference the symbol.
-    AgainstSymbolWithTargetVA,
     /// This is used by the MIPS multi-GOT implementation. It relocates
     /// addresses of 64kb pages that lie inside the output section.
     MipsMultiGotPage,
@@ -458,9 +454,7 @@ class DynamicReloc {
 
   uint64_t getOffset() const;
   uint32_t getSymIndex(SymbolTableBaseSection *symTab) const;
-  bool needsDynSymIndex() const {
-    return kind == AgainstSymbol || kind == AgainstSymbolWithTargetVA;
-  }
+  bool needsDynSymIndex() const { return kind == AgainstSymbol; }
 
   /// Computes the addend of the dynamic relocation. Note that this is not the
   /// same as the #addend member variable as it may also include the symbol

@jrtc27
Copy link
Collaborator Author

jrtc27 commented Jul 26, 2025

#150798. Screwed up spr...

@jrtc27 jrtc27 closed this Jul 26, 2025
@jrtc27 jrtc27 deleted the users/jrtc27/spr/nfcielf-merge-againstsymbol-and-againstsymbolwithtargetva branch July 26, 2025 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants