Skip to content

[NFCI][ELF][Mips] Refactor MipsGotSection to avoid explicit writes #150730

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

Open
wants to merge 3 commits into
base: users/jrtc27/spr/main.nfcielfmips-refactor-mipsgotsection-to-avoid-explicit-writes
Choose a base branch
from

Conversation

jrtc27
Copy link
Collaborator

@jrtc27 jrtc27 commented Jul 26, 2025

Splitting the VA / addend calculations between build and writeTo means
having to keep them in sync and duplicating some of the logic. Move all
such calculations into build, mirroring how the normal non-MIPS code in
Relocations.cpp ensures the addend and initial memory contents are set.

Created using spr 1.3.5
@llvmbot
Copy link
Member

llvmbot commented Jul 26, 2025

@llvm/pr-subscribers-backend-mips

@llvm/pr-subscribers-lld

Author: Jessica Clarke (jrtc27)

Changes

Splitting the VA / addend calculations between build and writeTo means
having to keep them in sync and duplicating some of the logic. For
everything except "page address" relocations, move all such calculations
into build, mirroring how the normal non-MIPS code in Relocations.cpp
ensures the addend and initial memory contents are set.


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

3 Files Affected:

  • (modified) lld/ELF/Arch/Mips.cpp (+2)
  • (modified) lld/ELF/SyntheticSections.cpp (+86-69)
  • (modified) lld/ELF/SyntheticSections.h (+1)
diff --git a/lld/ELF/Arch/Mips.cpp b/lld/ELF/Arch/Mips.cpp
index 91c7f15ae1f1c..edb85fb7d96b3 100644
--- a/lld/ELF/Arch/Mips.cpp
+++ b/lld/ELF/Arch/Mips.cpp
@@ -589,6 +589,7 @@ void MIPS<ELFT>::relocate(uint8_t *loc, const Relocation &rel,
 
   switch (type) {
   case R_MIPS_32:
+  case R_MIPS_REL32:
   case R_MIPS_GPREL32:
   case R_MIPS_TLS_DTPREL32:
   case R_MIPS_TLS_TPREL32:
@@ -597,6 +598,7 @@ void MIPS<ELFT>::relocate(uint8_t *loc, const Relocation &rel,
   case R_MIPS_64:
   case R_MIPS_TLS_DTPREL64:
   case R_MIPS_TLS_TPREL64:
+  case (R_MIPS_64 << 8) | R_MIPS_REL32:
     write64(ctx, loc, val);
     break;
   case R_MIPS_26:
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 0bb00c6d2bcff..2b3245711bae3 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -781,6 +781,10 @@ MipsGotSection::MipsGotSection(Ctx &ctx)
     : SyntheticSection(ctx, ".got", SHT_PROGBITS,
                        SHF_ALLOC | SHF_WRITE | SHF_MIPS_GPREL, 16) {}
 
+void MipsGotSection::addConstant(const Relocation &r) {
+  relocations.push_back(r);
+}
+
 void MipsGotSection::addEntry(InputFile &file, Symbol &sym, int64_t addend,
                               RelExpr expr) {
   FileGot &g = getGot(file);
@@ -1055,16 +1059,23 @@ void MipsGotSection::build() {
     ctx.symAux.back().gotIdx = p.second;
   }
 
-  // Create dynamic relocations.
+  // Create relocations.
+  //
+  // NB: GOT 'page address' entries have their VAs handled in writeTo as they
+  // reference an OutputSection not a Symbol.
   for (FileGot &got : gots) {
-    // Create dynamic relocations for TLS entries.
+    static Undefined dummy(ctx.internalFile, "", STB_LOCAL, 0, 0);
+
+    // Create relocations for TLS entries.
     for (std::pair<Symbol *, size_t> &p : got.tls) {
       Symbol *s = p.first;
       uint64_t offset = p.second * ctx.arg.wordsize;
       // When building a shared library we still need a dynamic relocation
       // 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)
+      if (!s->isPreemptible && !ctx.arg.shared)
+        addConstant({R_TPREL, ctx.target->symbolicRel, offset, 0, s});
+      else
         ctx.mainPart->relaDyn->addAddendOnlyRelocIfNonPreemptible(
             ctx.target->tlsGotRel, *this, offset, *s, ctx.target->symbolicRel);
     }
@@ -1072,57 +1083,98 @@ void MipsGotSection::build() {
       Symbol *s = p.first;
       uint64_t offset = p.second * ctx.arg.wordsize;
       if (s == nullptr) {
-        if (!ctx.arg.shared)
-          continue;
-        ctx.mainPart->relaDyn->addReloc(
-            {ctx.target->tlsModuleIndexRel, this, offset});
+        if (ctx.arg.shared)
+          ctx.mainPart->relaDyn->addReloc(
+              {ctx.target->tlsModuleIndexRel, this, offset});
+        else
+          addConstant({R_ADDEND, ctx.target->symbolicRel, offset, 1, &dummy});
       } else {
         // When building a shared library we still need a dynamic relocation
         // for the module index. Therefore only checking for
         // S->isPreemptible is not sufficient (this happens e.g. for
         // thread-locals that have been marked as local through a linker script)
         if (!s->isPreemptible && !ctx.arg.shared)
-          continue;
-        ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsModuleIndexRel,
-                                              *this, offset, *s);
+          // Write one to the GOT slot.
+          addConstant({R_ADDEND, ctx.target->symbolicRel, offset, 1, s});
+        else
+          ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsModuleIndexRel,
+                                                *this, offset, *s);
+        offset += ctx.arg.wordsize;
         // However, we can skip writing the TLS offset reloc for non-preemptible
         // symbols since it is known even in shared libraries
-        if (!s->isPreemptible)
-          continue;
-        offset += ctx.arg.wordsize;
-        ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsOffsetRel, *this,
-                                              offset, *s);
+        if (s->isPreemptible)
+          ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsOffsetRel, *this,
+                                                offset, *s);
+        else
+          addConstant({R_ABS, ctx.target->tlsOffsetRel, offset, 0, s});
       }
     }
 
-    // Do not create dynamic relocations for non-TLS
-    // entries in the primary GOT.
-    if (&got == primGot)
+    // Primary GOT's local and global relocations are implicit; write out VA as
+    // a constant for both (note MIPS requires the VA be written even for the
+    // global entries).
+    if (&got == primGot) {
+      // Relocations for "global" entries.
+      for (const std::pair<Symbol *, size_t> &p : got.global) {
+        uint64_t offset = p.second * ctx.arg.wordsize;
+        addConstant({R_ABS, ctx.target->relativeRel, offset, 0, p.first});
+      }
+      for (const std::pair<Symbol *, size_t> &p : got.relocs) {
+        uint64_t offset = p.second * ctx.arg.wordsize;
+        addConstant({R_ABS, ctx.target->relativeRel, offset, 0, p.first});
+      }
+
+      // Relocations for "local" entries
+      for (const std::pair<GotEntry, size_t> &p : got.local16) {
+        uint64_t offset = p.second * ctx.arg.wordsize;
+        if (p.first.first == nullptr)
+          addConstant({R_ADDEND, ctx.target->relativeRel, offset,
+                       p.first.second, &dummy});
+        else
+          addConstant({R_ABS, ctx.target->relativeRel, offset, p.first.second,
+                       p.first.first});
+      }
+
       continue;
+    }
 
-    // Dynamic relocations for "global" entries.
+    // Relocations for "global" entries.
     for (const std::pair<Symbol *, size_t> &p : got.global) {
       uint64_t offset = p.second * ctx.arg.wordsize;
       ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->relativeRel, *this,
                                             offset, *p.first);
     }
-    if (!ctx.arg.isPic)
-      continue;
-    // Dynamic relocations for "local" entries in case of PIC.
-    for (const std::pair<const OutputSection *, FileGot::PageBlock> &l :
-         got.pagesMap) {
-      size_t pageCount = l.second.count;
-      for (size_t pi = 0; pi < pageCount; ++pi) {
-        uint64_t offset = (l.second.firstIndex + pi) * ctx.arg.wordsize;
-        ctx.mainPart->relaDyn->addReloc({ctx.target->relativeRel, this, offset,
-                                         l.first, int64_t(pi * 0x10000)});
+    // Relocation-only entries exist as dummy entries for dynamic symbols that
+    // aren't otherwise in the primary GOT, as the ABI requires an entry for
+    // each dynamic symbol. Secondary GOTs have no need for them.
+    assert(got.relocs.empty() &&
+           "Relocation-only entries should only be in the primary GOT");
+
+    // Relocations for "local" entries
+    if (ctx.arg.isPic) {
+      for (const std::pair<const OutputSection *, FileGot::PageBlock> &l :
+           got.pagesMap) {
+        size_t pageCount = l.second.count;
+        for (size_t pi = 0; pi < pageCount; ++pi) {
+          uint64_t offset = (l.second.firstIndex + pi) * ctx.arg.wordsize;
+          ctx.mainPart->relaDyn->addReloc({ctx.target->relativeRel, this,
+                                           offset, l.first,
+                                           int64_t(pi * 0x10000)});
+        }
       }
     }
     for (const std::pair<GotEntry, size_t> &p : got.local16) {
       uint64_t offset = p.second * ctx.arg.wordsize;
-      ctx.mainPart->relaDyn->addReloc({ctx.target->relativeRel, this, offset,
-                                       DynamicReloc::AddendOnlyWithTargetVA,
-                                       *p.first.first, p.first.second, R_ABS});
+      if (p.first.first == nullptr)
+        addConstant({R_ADDEND, ctx.target->relativeRel, offset, p.first.second,
+                     &dummy});
+      else if (!ctx.arg.isPic)
+        addConstant({R_ABS, ctx.target->relativeRel, offset, p.first.second,
+                     p.first.first});
+      else
+        ctx.mainPart->relaDyn->addRelativeReloc(
+            ctx.target->relativeRel, *this, offset, *p.first.first,
+            p.first.second, ctx.target->relativeRel, R_ABS);
     }
   }
 }
@@ -1161,49 +1213,14 @@ void MipsGotSection::writeTo(uint8_t *buf) {
             (uint64_t)1 << (ctx.arg.wordsize * 8 - 1));
   ctx.target->relocateAlloc(*this, buf);
   for (const FileGot &g : gots) {
-    auto write = [&](size_t i, const Symbol *s, int64_t a) {
-      uint64_t va = a;
-      if (s)
-        va = s->getVA(ctx, a);
-      writeUint(ctx, buf + i * ctx.arg.wordsize, va);
-    };
     // Write 'page address' entries to the local part of the GOT.
     for (const std::pair<const OutputSection *, FileGot::PageBlock> &l :
          g.pagesMap) {
       size_t pageCount = l.second.count;
       uint64_t firstPageAddr = getMipsPageAddr(l.first->addr);
       for (size_t pi = 0; pi < pageCount; ++pi)
-        write(l.second.firstIndex + pi, nullptr, firstPageAddr + pi * 0x10000);
-    }
-    // Local, global, TLS, reloc-only  entries.
-    // If TLS entry has a corresponding dynamic relocations, leave it
-    // initialized by zero. Write down adjusted TLS symbol's values otherwise.
-    // To calculate the adjustments use offsets for thread-local storage.
-    // http://web.archive.org/web/20190324223224/https://www.linux-mips.org/wiki/NPTL
-    for (const std::pair<GotEntry, size_t> &p : g.local16)
-      write(p.second, p.first.first, p.first.second);
-    // Write VA to the primary GOT only. For secondary GOTs that
-    // will be done by REL32 dynamic relocations.
-    if (&g == &gots.front())
-      for (const std::pair<Symbol *, size_t> &p : g.global)
-        write(p.second, p.first, 0);
-    for (const std::pair<Symbol *, size_t> &p : g.relocs)
-      write(p.second, p.first, 0);
-    for (const std::pair<Symbol *, size_t> &p : g.tls) {
-      if (!p.first->isPreemptible && !ctx.arg.shared)
-        write(p.second, p.first, -0x7000);
-    }
-    for (const std::pair<Symbol *, size_t> &p : g.dynTlsSymbols) {
-      if (p.first == nullptr && !ctx.arg.shared)
-        write(p.second, nullptr, 1);
-      else if (p.first && !p.first->isPreemptible) {
-        // If we are emitting a shared library with relocations we mustn't write
-        // anything to the GOT here. When using Elf_Rel relocations the value
-        // one will be treated as an addend and will cause crashes at runtime
-        if (!ctx.arg.shared)
-          write(p.second, nullptr, 1);
-        write(p.second + 1, p.first, -0x8000);
-      }
+        writeUint(ctx, buf + (l.second.firstIndex + pi) * ctx.arg.wordsize,
+                  firstPageAddr + pi * 0x10000);
     }
   }
 }
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 7612915b5b1dc..e330f55b74daa 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -203,6 +203,7 @@ class MipsGotSection final : public SyntheticSection {
   // primary and optional multiple secondary GOTs.
   void build();
 
+  void addConstant(const Relocation &r);
   void addEntry(InputFile &file, Symbol &sym, int64_t addend, RelExpr expr);
   void addDynTlsEntry(InputFile &file, Symbol &sym);
   void addTlsIndex(InputFile &file);

@llvmbot
Copy link
Member

llvmbot commented Jul 26, 2025

@llvm/pr-subscribers-lld-elf

Author: Jessica Clarke (jrtc27)

Changes

Splitting the VA / addend calculations between build and writeTo means
having to keep them in sync and duplicating some of the logic. For
everything except "page address" relocations, move all such calculations
into build, mirroring how the normal non-MIPS code in Relocations.cpp
ensures the addend and initial memory contents are set.


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

3 Files Affected:

  • (modified) lld/ELF/Arch/Mips.cpp (+2)
  • (modified) lld/ELF/SyntheticSections.cpp (+86-69)
  • (modified) lld/ELF/SyntheticSections.h (+1)
diff --git a/lld/ELF/Arch/Mips.cpp b/lld/ELF/Arch/Mips.cpp
index 91c7f15ae1f1c..edb85fb7d96b3 100644
--- a/lld/ELF/Arch/Mips.cpp
+++ b/lld/ELF/Arch/Mips.cpp
@@ -589,6 +589,7 @@ void MIPS<ELFT>::relocate(uint8_t *loc, const Relocation &rel,
 
   switch (type) {
   case R_MIPS_32:
+  case R_MIPS_REL32:
   case R_MIPS_GPREL32:
   case R_MIPS_TLS_DTPREL32:
   case R_MIPS_TLS_TPREL32:
@@ -597,6 +598,7 @@ void MIPS<ELFT>::relocate(uint8_t *loc, const Relocation &rel,
   case R_MIPS_64:
   case R_MIPS_TLS_DTPREL64:
   case R_MIPS_TLS_TPREL64:
+  case (R_MIPS_64 << 8) | R_MIPS_REL32:
     write64(ctx, loc, val);
     break;
   case R_MIPS_26:
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 0bb00c6d2bcff..2b3245711bae3 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -781,6 +781,10 @@ MipsGotSection::MipsGotSection(Ctx &ctx)
     : SyntheticSection(ctx, ".got", SHT_PROGBITS,
                        SHF_ALLOC | SHF_WRITE | SHF_MIPS_GPREL, 16) {}
 
+void MipsGotSection::addConstant(const Relocation &r) {
+  relocations.push_back(r);
+}
+
 void MipsGotSection::addEntry(InputFile &file, Symbol &sym, int64_t addend,
                               RelExpr expr) {
   FileGot &g = getGot(file);
@@ -1055,16 +1059,23 @@ void MipsGotSection::build() {
     ctx.symAux.back().gotIdx = p.second;
   }
 
-  // Create dynamic relocations.
+  // Create relocations.
+  //
+  // NB: GOT 'page address' entries have their VAs handled in writeTo as they
+  // reference an OutputSection not a Symbol.
   for (FileGot &got : gots) {
-    // Create dynamic relocations for TLS entries.
+    static Undefined dummy(ctx.internalFile, "", STB_LOCAL, 0, 0);
+
+    // Create relocations for TLS entries.
     for (std::pair<Symbol *, size_t> &p : got.tls) {
       Symbol *s = p.first;
       uint64_t offset = p.second * ctx.arg.wordsize;
       // When building a shared library we still need a dynamic relocation
       // 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)
+      if (!s->isPreemptible && !ctx.arg.shared)
+        addConstant({R_TPREL, ctx.target->symbolicRel, offset, 0, s});
+      else
         ctx.mainPart->relaDyn->addAddendOnlyRelocIfNonPreemptible(
             ctx.target->tlsGotRel, *this, offset, *s, ctx.target->symbolicRel);
     }
@@ -1072,57 +1083,98 @@ void MipsGotSection::build() {
       Symbol *s = p.first;
       uint64_t offset = p.second * ctx.arg.wordsize;
       if (s == nullptr) {
-        if (!ctx.arg.shared)
-          continue;
-        ctx.mainPart->relaDyn->addReloc(
-            {ctx.target->tlsModuleIndexRel, this, offset});
+        if (ctx.arg.shared)
+          ctx.mainPart->relaDyn->addReloc(
+              {ctx.target->tlsModuleIndexRel, this, offset});
+        else
+          addConstant({R_ADDEND, ctx.target->symbolicRel, offset, 1, &dummy});
       } else {
         // When building a shared library we still need a dynamic relocation
         // for the module index. Therefore only checking for
         // S->isPreemptible is not sufficient (this happens e.g. for
         // thread-locals that have been marked as local through a linker script)
         if (!s->isPreemptible && !ctx.arg.shared)
-          continue;
-        ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsModuleIndexRel,
-                                              *this, offset, *s);
+          // Write one to the GOT slot.
+          addConstant({R_ADDEND, ctx.target->symbolicRel, offset, 1, s});
+        else
+          ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsModuleIndexRel,
+                                                *this, offset, *s);
+        offset += ctx.arg.wordsize;
         // However, we can skip writing the TLS offset reloc for non-preemptible
         // symbols since it is known even in shared libraries
-        if (!s->isPreemptible)
-          continue;
-        offset += ctx.arg.wordsize;
-        ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsOffsetRel, *this,
-                                              offset, *s);
+        if (s->isPreemptible)
+          ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->tlsOffsetRel, *this,
+                                                offset, *s);
+        else
+          addConstant({R_ABS, ctx.target->tlsOffsetRel, offset, 0, s});
       }
     }
 
-    // Do not create dynamic relocations for non-TLS
-    // entries in the primary GOT.
-    if (&got == primGot)
+    // Primary GOT's local and global relocations are implicit; write out VA as
+    // a constant for both (note MIPS requires the VA be written even for the
+    // global entries).
+    if (&got == primGot) {
+      // Relocations for "global" entries.
+      for (const std::pair<Symbol *, size_t> &p : got.global) {
+        uint64_t offset = p.second * ctx.arg.wordsize;
+        addConstant({R_ABS, ctx.target->relativeRel, offset, 0, p.first});
+      }
+      for (const std::pair<Symbol *, size_t> &p : got.relocs) {
+        uint64_t offset = p.second * ctx.arg.wordsize;
+        addConstant({R_ABS, ctx.target->relativeRel, offset, 0, p.first});
+      }
+
+      // Relocations for "local" entries
+      for (const std::pair<GotEntry, size_t> &p : got.local16) {
+        uint64_t offset = p.second * ctx.arg.wordsize;
+        if (p.first.first == nullptr)
+          addConstant({R_ADDEND, ctx.target->relativeRel, offset,
+                       p.first.second, &dummy});
+        else
+          addConstant({R_ABS, ctx.target->relativeRel, offset, p.first.second,
+                       p.first.first});
+      }
+
       continue;
+    }
 
-    // Dynamic relocations for "global" entries.
+    // Relocations for "global" entries.
     for (const std::pair<Symbol *, size_t> &p : got.global) {
       uint64_t offset = p.second * ctx.arg.wordsize;
       ctx.mainPart->relaDyn->addSymbolReloc(ctx.target->relativeRel, *this,
                                             offset, *p.first);
     }
-    if (!ctx.arg.isPic)
-      continue;
-    // Dynamic relocations for "local" entries in case of PIC.
-    for (const std::pair<const OutputSection *, FileGot::PageBlock> &l :
-         got.pagesMap) {
-      size_t pageCount = l.second.count;
-      for (size_t pi = 0; pi < pageCount; ++pi) {
-        uint64_t offset = (l.second.firstIndex + pi) * ctx.arg.wordsize;
-        ctx.mainPart->relaDyn->addReloc({ctx.target->relativeRel, this, offset,
-                                         l.first, int64_t(pi * 0x10000)});
+    // Relocation-only entries exist as dummy entries for dynamic symbols that
+    // aren't otherwise in the primary GOT, as the ABI requires an entry for
+    // each dynamic symbol. Secondary GOTs have no need for them.
+    assert(got.relocs.empty() &&
+           "Relocation-only entries should only be in the primary GOT");
+
+    // Relocations for "local" entries
+    if (ctx.arg.isPic) {
+      for (const std::pair<const OutputSection *, FileGot::PageBlock> &l :
+           got.pagesMap) {
+        size_t pageCount = l.second.count;
+        for (size_t pi = 0; pi < pageCount; ++pi) {
+          uint64_t offset = (l.second.firstIndex + pi) * ctx.arg.wordsize;
+          ctx.mainPart->relaDyn->addReloc({ctx.target->relativeRel, this,
+                                           offset, l.first,
+                                           int64_t(pi * 0x10000)});
+        }
       }
     }
     for (const std::pair<GotEntry, size_t> &p : got.local16) {
       uint64_t offset = p.second * ctx.arg.wordsize;
-      ctx.mainPart->relaDyn->addReloc({ctx.target->relativeRel, this, offset,
-                                       DynamicReloc::AddendOnlyWithTargetVA,
-                                       *p.first.first, p.first.second, R_ABS});
+      if (p.first.first == nullptr)
+        addConstant({R_ADDEND, ctx.target->relativeRel, offset, p.first.second,
+                     &dummy});
+      else if (!ctx.arg.isPic)
+        addConstant({R_ABS, ctx.target->relativeRel, offset, p.first.second,
+                     p.first.first});
+      else
+        ctx.mainPart->relaDyn->addRelativeReloc(
+            ctx.target->relativeRel, *this, offset, *p.first.first,
+            p.first.second, ctx.target->relativeRel, R_ABS);
     }
   }
 }
@@ -1161,49 +1213,14 @@ void MipsGotSection::writeTo(uint8_t *buf) {
             (uint64_t)1 << (ctx.arg.wordsize * 8 - 1));
   ctx.target->relocateAlloc(*this, buf);
   for (const FileGot &g : gots) {
-    auto write = [&](size_t i, const Symbol *s, int64_t a) {
-      uint64_t va = a;
-      if (s)
-        va = s->getVA(ctx, a);
-      writeUint(ctx, buf + i * ctx.arg.wordsize, va);
-    };
     // Write 'page address' entries to the local part of the GOT.
     for (const std::pair<const OutputSection *, FileGot::PageBlock> &l :
          g.pagesMap) {
       size_t pageCount = l.second.count;
       uint64_t firstPageAddr = getMipsPageAddr(l.first->addr);
       for (size_t pi = 0; pi < pageCount; ++pi)
-        write(l.second.firstIndex + pi, nullptr, firstPageAddr + pi * 0x10000);
-    }
-    // Local, global, TLS, reloc-only  entries.
-    // If TLS entry has a corresponding dynamic relocations, leave it
-    // initialized by zero. Write down adjusted TLS symbol's values otherwise.
-    // To calculate the adjustments use offsets for thread-local storage.
-    // http://web.archive.org/web/20190324223224/https://www.linux-mips.org/wiki/NPTL
-    for (const std::pair<GotEntry, size_t> &p : g.local16)
-      write(p.second, p.first.first, p.first.second);
-    // Write VA to the primary GOT only. For secondary GOTs that
-    // will be done by REL32 dynamic relocations.
-    if (&g == &gots.front())
-      for (const std::pair<Symbol *, size_t> &p : g.global)
-        write(p.second, p.first, 0);
-    for (const std::pair<Symbol *, size_t> &p : g.relocs)
-      write(p.second, p.first, 0);
-    for (const std::pair<Symbol *, size_t> &p : g.tls) {
-      if (!p.first->isPreemptible && !ctx.arg.shared)
-        write(p.second, p.first, -0x7000);
-    }
-    for (const std::pair<Symbol *, size_t> &p : g.dynTlsSymbols) {
-      if (p.first == nullptr && !ctx.arg.shared)
-        write(p.second, nullptr, 1);
-      else if (p.first && !p.first->isPreemptible) {
-        // If we are emitting a shared library with relocations we mustn't write
-        // anything to the GOT here. When using Elf_Rel relocations the value
-        // one will be treated as an addend and will cause crashes at runtime
-        if (!ctx.arg.shared)
-          write(p.second, nullptr, 1);
-        write(p.second + 1, p.first, -0x8000);
-      }
+        writeUint(ctx, buf + (l.second.firstIndex + pi) * ctx.arg.wordsize,
+                  firstPageAddr + pi * 0x10000);
     }
   }
 }
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 7612915b5b1dc..e330f55b74daa 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -203,6 +203,7 @@ class MipsGotSection final : public SyntheticSection {
   // primary and optional multiple secondary GOTs.
   void build();
 
+  void addConstant(const Relocation &r);
   void addEntry(InputFile &file, Symbol &sym, int64_t addend, RelExpr expr);
   void addDynTlsEntry(InputFile &file, Symbol &sym);
   void addTlsIndex(InputFile &file);

@jrtc27
Copy link
Collaborator Author

jrtc27 commented Jul 26, 2025

The current code is quite crusty and divergent from non-MIPS in API use, but fixing it up like this is quite high-risk, especially given how weird the MIPS GOT is when it comes to the required initial memory state. Is anyone using LLD for MIPS these days who can test this in a wider context? I've tried to be very careful but I would not be surprised if I've made mistakes; our test coverage isn't great.

Created using spr 1.3.5
@MaskRay
Copy link
Member

MaskRay commented Jul 26, 2025

The current code is quite crusty and divergent from non-MIPS in API use, but fixing it up like this is quite high-risk, especially given how weird the MIPS GOT is when it comes to the required initial memory state. Is anyone using LLD for MIPS these days who can test this in a wider context? I've tried to be very careful but I would not be surprised if I've made mistakes; our test coverage isn't great.

It's indeed very difficult to find folks still concerned with MIPS...
I believe only one company is still actively contributing to the MIPS backend in LLVM... @wzssyqa

Besides the ClangBuiltLinux maintainer and the OpenBSD maintainer continue to support MIPS.

@jrtc27
Copy link
Collaborator Author

jrtc27 commented Jul 26, 2025

The current code is quite crusty and divergent from non-MIPS in API use, but fixing it up like this is quite high-risk, especially given how weird the MIPS GOT is when it comes to the required initial memory state. Is anyone using LLD for MIPS these days who can test this in a wider context? I've tried to be very careful but I would not be surprised if I've made mistakes; our test coverage isn't great.

It's indeed very difficult to find folks still concerned with MIPS... I believe only one company is still actively contributing to the MIPS backend in LLVM... @wzssyqa

Besides the ClangBuiltLinux maintainer and the OpenBSD maintainer continue to support MIPS.

This one I don't hugely care about getting in soon as it's (a) meant to be NFC (b) limited to MipsGotSection. It makes the code clearer and more like the normal GOT, but in theory what's there is fine, and the wider LLD internals aren't affected by it.

@brad0
Copy link
Contributor

brad0 commented Jul 28, 2025

It's indeed very difficult to find folks still concerned with MIPS... I believe only one company is still actively contributing to the MIPS backend in LLVM... @wzssyqa

Besides the ClangBuiltLinux maintainer and the OpenBSD maintainer continue to support MIPS.

It's a catch 22 when the linker has never been feature complete / bug free enough to be usable. For OpenBSD it's the only arch we currently don't use as a linker due to issues / bugs.

@jrtc27
Copy link
Collaborator Author

jrtc27 commented Jul 28, 2025

It's indeed very difficult to find folks still concerned with MIPS... I believe only one company is still actively contributing to the MIPS backend in LLVM... @wzssyqa
Besides the ClangBuiltLinux maintainer and the OpenBSD maintainer continue to support MIPS.

It's a catch 22 when the linker has never been feature complete / bug free enough to be usable. For OpenBSD it's the only arch we currently don't use as a linker due to issues / bugs.

Hm, that surprises me. We got it to a good enough point to be usable as the system linker for FreeBSD, before MIPS was dropped as a supported architecture.

@brad0
Copy link
Contributor

brad0 commented Jul 28, 2025

Hm, that surprises me. We got it to a good enough point to be usable as the system linker for FreeBSD, before MIPS was dropped as a supported architecture.

We've run into bugs even with other OS's using the linker as a system linker on other arches too. Anyway, it's a bit frustrating as I can't get details out of the other developers and the group as a whole really does not seem to want to be involved with upstream unless its under their specific conditions.

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.

4 participants