[lld] Disable section merging only for non-trivial relocations

pranavk · pranavk · commit adef51a1b78a · 2025-05-13T16:20:28.000-07:00
diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp
@@ -77,9 +77,11 @@
 #include "InputFiles.h"
 #include "LinkerScript.h"
 #include "OutputSections.h"
+#include "Relocations.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"
+#include "Target.h"
 #include "llvm/ADT/EquivalenceClasses.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Object/ELF.h"
@@ -105,18 +107,23 @@ template <class ELFT> class ICF {
   void segregate(size_t begin, size_t end, uint32_t eqClassBase, bool constant);
 
   template <class RelTy>
-  bool constantEq(const InputSection *a, Relocs<RelTy> relsA,
-                  const InputSection *b, Relocs<RelTy> relsB);
+  bool constantEq(InputSection *a, Relocs<RelTy> relsA, InputSection *b,
+                  Relocs<RelTy> relsB);
 
   template <class RelTy>
   bool variableEq(const InputSection *a, Relocs<RelTy> relsA,
                   const InputSection *b, Relocs<RelTy> relsB);
 
-  bool equalsConstant(const InputSection *a, const InputSection *b);
+  bool equalsConstant(InputSection *a, InputSection *b);
   bool equalsVariable(const InputSection *a, const InputSection *b);
 
   size_t findBoundary(size_t begin, size_t end);
 
+  // A relocation with side-effects is considered non-trivial. Eg: relocation
+  // creates GOT entry or TLS slot.
+  template <class RelTy>
+  bool isTrivialRelocation(InputSection *a, Symbol &s, RelTy reloc);
+
   void forEachClassRange(size_t begin, size_t end,
                          llvm::function_ref<void(size_t, size_t)> fn);
 
@@ -235,11 +242,30 @@ void ICF<ELFT>::segregate(size_t begin, size_t end, uint32_t eqClassBase,
   }
 }
 
+template <class ELFT>
+template <class RelTy>
+bool ICF<ELFT>::isTrivialRelocation(InputSection *a, Symbol &s, RelTy reloc) {
+  OffsetGetter getter(*a);
+  uint64_t offset = getter.get(ctx, reloc.r_offset);
+  RelExpr expr = ctx.target->getRelExpr(reloc.getType(ctx.arg.isMips64EL), s,
+                                        a->content().data() + offset);
+
+  if (needsGot(expr) || needsTls(s, expr))
+    return false;
+  return true;
+}
+
+// Two symbols referenced by relocations can be merged together safely
+// when their addends are same.
+static bool canMergeSymbols(uint64_t addA, uint64_t addB) {
+  return addA == addB;
+}
+
 // Compare two lists of relocations.
 template <class ELFT>
 template <class RelTy>
-bool ICF<ELFT>::constantEq(const InputSection *secA, Relocs<RelTy> ra,
-                           const InputSection *secB, Relocs<RelTy> rb) {
+bool ICF<ELFT>::constantEq(InputSection *secA, Relocs<RelTy> ra,
+                           InputSection *secB, Relocs<RelTy> rb) {
   if (ra.size() != rb.size())
     return false;
   auto rai = ra.begin(), rae = ra.end(), rbi = rb.begin();
@@ -287,13 +313,14 @@ bool ICF<ELFT>::constantEq(const InputSection *secA, Relocs<RelTy> ra,
     // Relocations referring to InputSections are constant-equal if their
     // section offsets are equal.
     if (isa<InputSection>(da->section)) {
-      // Our symbol folding logic later merges symbols in two folded sections
-      // We should not merge sections in the first place if their symbols
-      // cannot be merged together.
-      bool canMergeSymbols = addA == addB;
-      if (da->value + addA == db->value + addB && canMergeSymbols)
+      if (da->value + addA == db->value + addB) {
+        // For non-trivial relocations, if we cannot merge symbols together,
+        // we must not merge them.
+        if (!isTrivialRelocation(secA, sa, *rai) &&
+            !canMergeSymbols(addA, addB))
+          return false;
         continue;
-      return false;
+      }
     }
 
     // Relocations referring to MergeInputSections are constant-equal if their
@@ -319,7 +346,7 @@ bool ICF<ELFT>::constantEq(const InputSection *secA, Relocs<RelTy> ra,
 // Compare "non-moving" part of two InputSections, namely everything
 // except relocation targets.
 template <class ELFT>
-bool ICF<ELFT>::equalsConstant(const InputSection *a, const InputSection *b) {
+bool ICF<ELFT>::equalsConstant(InputSection *a, InputSection *b) {
   if (a->flags != b->flags || a->getSize() != b->getSize() ||
       a->content() != b->content())
     return false;
@@ -338,26 +365,27 @@ bool ICF<ELFT>::equalsConstant(const InputSection *a, const InputSection *b) {
              : constantEq(a, ra.relas, b, rb.relas);
 }
 
-template <class RelTy>
-static SmallVector<Symbol *> getReloc(const InputSection *sec,
-                                      Relocs<RelTy> relocs) {
-  SmallVector<Symbol *> syms;
+template <class ELFT, class RelTy>
+static SmallVector<std::pair<Symbol *, uint64_t>>
+getReloc(const InputSection *sec, Relocs<RelTy> relocs) {
+  SmallVector<std::pair<Symbol *, uint64_t>> syms;
   for (auto ri = relocs.begin(), re = relocs.end(); ri != re; ++ri) {
     Symbol &sym = sec->file->getRelocTargetSym(*ri);
-    syms.push_back(&sym);
+    syms.emplace_back(&sym, getAddend<ELFT>(*ri));
   }
   return syms;
 }
 
 template <class ELFT>
-static SmallVector<Symbol *> getRelocTargetSyms(const InputSection *sec) {
+static SmallVector<std::pair<Symbol *, uint64_t>>
+getRelocTargetSyms(const InputSection *sec) {
   const RelsOrRelas<ELFT> rel = sec->template relsOrRelas<ELFT>();
   if (rel.areRelocsCrel())
-    return getReloc(sec, rel.crels);
+    return getReloc<ELFT>(sec, rel.crels);
   if (rel.areRelocsRel())
-    return getReloc(sec, rel.rels);
+    return getReloc<ELFT>(sec, rel.rels);
 
-  return getReloc(sec, rel.relas);
+  return getReloc<ELFT>(sec, rel.relas);
 }
 
 // Compare two lists of relocations. Returns true if all pairs of
@@ -572,19 +600,21 @@ template <class ELFT> void ICF<ELFT>::run() {
     if (end - begin == 1)
       return;
     print() << "selected section " << sections[begin];
-    SmallVector<Symbol *> syms = getRelocTargetSyms<ELFT>(sections[begin]);
+    SmallVector<std::pair<Symbol *, uint64_t>> syms =
+        getRelocTargetSyms<ELFT>(sections[begin]);
     for (size_t i = begin + 1; i < end; ++i) {
       print() << "  removing identical section " << sections[i];
       sections[begin]->replace(sections[i]);
-      SmallVector<Symbol *> replacedSyms =
+      SmallVector<std::pair<Symbol *, uint64_t>> replacedSyms =
           getRelocTargetSyms<ELFT>(sections[i]);
       assert(syms.size() == replacedSyms.size() &&
              "Should have same number of syms!");
       for (size_t i = 0; i < syms.size(); i++) {
-        if (syms[i] == replacedSyms[i] || !syms[i]->isGlobal() ||
-            !replacedSyms[i]->isGlobal())
+        if (syms[i].first == replacedSyms[i].first ||
+            !syms[i].first->isGlobal() || !replacedSyms[i].first->isGlobal() ||
+            !canMergeSymbols(syms[i].second, replacedSyms[i].second))
           continue;
-        symbolEquivalence.unionSets(syms[i], replacedSyms[i]);
+        symbolEquivalence.unionSets(syms[i].first, replacedSyms[i].first);
       }
 
       // At this point we know sections merged are fully identical and hence
diff --git a/lld/ELF/InputSection.h b/lld/ELF/InputSection.h
@@ -514,6 +514,49 @@ class SyntheticSection : public InputSection {
   }
 };
 
+class OffsetGetter {
+public:
+  OffsetGetter() = default;
+  explicit OffsetGetter(InputSectionBase &sec) {
+    if (auto *eh = dyn_cast<EhInputSection>(&sec)) {
+      cies = eh->cies;
+      fdes = eh->fdes;
+      i = cies.begin();
+      j = fdes.begin();
+    }
+  }
+
+  // Translates offsets in input sections to offsets in output sections.
+  // Given offset must increase monotonically. We assume that Piece is
+  // sorted by inputOff.
+  uint64_t get(Ctx &ctx, uint64_t off) {
+    if (cies.empty())
+      return off;
+
+    while (j != fdes.end() && j->inputOff <= off)
+      ++j;
+    auto it = j;
+    if (j == fdes.begin() || j[-1].inputOff + j[-1].size <= off) {
+      while (i != cies.end() && i->inputOff <= off)
+        ++i;
+      if (i == cies.begin() || i[-1].inputOff + i[-1].size <= off) {
+        Err(ctx) << ".eh_frame: relocation is not in any piece";
+        return 0;
+      }
+      it = i;
+    }
+
+    // Offset -1 means that the piece is dead (i.e. garbage collected).
+    if (it[-1].outputOff == -1)
+      return -1;
+    return it[-1].outputOff + (off - it[-1].inputOff);
+  }
+
+private:
+  ArrayRef<EhSectionPiece> cies, fdes;
+  ArrayRef<EhSectionPiece>::iterator i, j;
+};
+
 inline bool isStaticRelSecType(uint32_t type) {
   return type == llvm::ELF::SHT_RELA || type == llvm::ELF::SHT_CREL ||
          type == llvm::ELF::SHT_REL;
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
@@ -127,29 +127,6 @@ void elf::reportRangeError(Ctx &ctx, uint8_t *loc, int64_t v, int n,
   }
 }
 
-// Build a bitmask with one bit set for each 64 subset of RelExpr.
-static constexpr uint64_t buildMask() { return 0; }
-
-template <typename... Tails>
-static constexpr uint64_t buildMask(int head, Tails... tails) {
-  return (0 <= head && head < 64 ? uint64_t(1) << head : 0) |
-         buildMask(tails...);
-}
-
-// Return true if `Expr` is one of `Exprs`.
-// There are more than 64 but less than 128 RelExprs, so we divide the set of
-// exprs into [0, 64) and [64, 128) and represent each range as a constant
-// 64-bit mask. Then we decide which mask to test depending on the value of
-// expr and use a simple shift and bitwise-and to test for membership.
-template <RelExpr... Exprs> static bool oneof(RelExpr expr) {
-  assert(0 <= expr && (int)expr < 128 &&
-         "RelExpr is too large for 128-bit mask!");
-
-  if (expr >= 64)
-    return (uint64_t(1) << (expr - 64)) & buildMask((Exprs - 64)...);
-  return (uint64_t(1) << expr) & buildMask(Exprs...);
-}
-
 static RelType getMipsPairType(RelType type, bool isLocal) {
   switch (type) {
   case R_MIPS_HI16:
@@ -196,15 +173,6 @@ static bool needsPlt(RelExpr expr) {
                RE_PPC64_CALL_PLT>(expr);
 }
 
-bool lld::elf::needsGot(RelExpr expr) {
-  return oneof<R_GOT, RE_AARCH64_AUTH_GOT, RE_AARCH64_AUTH_GOT_PC, R_GOT_OFF,
-               RE_MIPS_GOT_LOCAL_PAGE, RE_MIPS_GOT_OFF, RE_MIPS_GOT_OFF32,
-               RE_AARCH64_GOT_PAGE_PC, RE_AARCH64_AUTH_GOT_PAGE_PC,
-               RE_AARCH64_AUTH_GOT_PAGE_PC, R_GOT_PC, R_GOTPLT,
-               RE_AARCH64_GOT_PAGE, RE_LOONGARCH_GOT, RE_LOONGARCH_GOT_PAGE_PC>(
-      expr);
-}
-
 // True if this expression is of the form Sym - X, where X is a position in the
 // file (PC, or GOT for example).
 static bool isRelExpr(RelExpr expr) {
@@ -403,49 +371,6 @@ template <class ELFT> static void addCopyRelSymbol(Ctx &ctx, SharedSymbol &ss) {
 //
 // For sections other than .eh_frame, this class doesn't do anything.
 namespace {
-class OffsetGetter {
-public:
-  OffsetGetter() = default;
-  explicit OffsetGetter(InputSectionBase &sec) {
-    if (auto *eh = dyn_cast<EhInputSection>(&sec)) {
-      cies = eh->cies;
-      fdes = eh->fdes;
-      i = cies.begin();
-      j = fdes.begin();
-    }
-  }
-
-  // Translates offsets in input sections to offsets in output sections.
-  // Given offset must increase monotonically. We assume that Piece is
-  // sorted by inputOff.
-  uint64_t get(Ctx &ctx, uint64_t off) {
-    if (cies.empty())
-      return off;
-
-    while (j != fdes.end() && j->inputOff <= off)
-      ++j;
-    auto it = j;
-    if (j == fdes.begin() || j[-1].inputOff + j[-1].size <= off) {
-      while (i != cies.end() && i->inputOff <= off)
-        ++i;
-      if (i == cies.begin() || i[-1].inputOff + i[-1].size <= off) {
-        Err(ctx) << ".eh_frame: relocation is not in any piece";
-        return 0;
-      }
-      it = i;
-    }
-
-    // Offset -1 means that the piece is dead (i.e. garbage collected).
-    if (it[-1].outputOff == -1)
-      return -1;
-    return it[-1].outputOff + (off - it[-1].inputOff);
-  }
-
-private:
-  ArrayRef<EhSectionPiece> cies, fdes;
-  ArrayRef<EhSectionPiece>::iterator i, j;
-};
-
 // This class encapsulates states needed to scan relocations for one
 // InputSectionBase.
 class RelocationScanner {
@@ -1593,7 +1518,7 @@ void RelocationScanner::scanOne(typename Relocs<RelTy>::const_iterator &i) {
   //
   // Some RISCV TLSDESC relocations reference a local NOTYPE symbol,
   // but we need to process them in handleTlsRelocation.
-  if (sym.isTls() || oneof<R_TLSDESC_PC, R_TLSDESC_CALL>(expr)) {
+  if (needsTls(sym, expr)) {
     if (unsigned processed =
             handleTlsRelocation(expr, type, offset, sym, addend)) {
       i += processed - 1;
diff --git a/lld/ELF/Relocations.h b/lld/ELF/Relocations.h
@@ -9,6 +9,7 @@
 #ifndef LLD_ELF_RELOCATIONS_H
 #define LLD_ELF_RELOCATIONS_H
 
+#include "Symbols.h"
 #include "lld/Common/LLVM.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
@@ -18,7 +19,6 @@
 namespace lld::elf {
 struct Ctx;
 class Defined;
-class Symbol;
 class InputSection;
 class InputSectionBase;
 class OutputSection;
@@ -355,6 +355,29 @@ inline Relocs<RelTy> sortRels(Relocs<RelTy> rels,
   return rels;
 }
 
+// Build a bitmask with one bit set for each 64 subset of RelExpr.
+constexpr uint64_t buildMask() { return 0; }
+
+template <typename... Tails>
+constexpr uint64_t buildMask(int head, Tails... tails) {
+  return (0 <= head && head < 64 ? uint64_t(1) << head : 0) |
+         buildMask(tails...);
+}
+
+// Return true if `Expr` is one of `Exprs`.
+// There are more than 64 but less than 128 RelExprs, so we divide the set of
+// exprs into [0, 64) and [64, 128) and represent each range as a constant
+// 64-bit mask. Then we decide which mask to test depending on the value of
+// expr and use a simple shift and bitwise-and to test for membership.
+template <RelExpr... Exprs> static bool oneof(RelExpr expr) {
+  assert(0 <= expr && (int)expr < 128 &&
+         "RelExpr is too large for 128-bit mask!");
+
+  if (expr >= 64)
+    return (uint64_t(1) << (expr - 64)) & buildMask((Exprs - 64)...);
+  return (uint64_t(1) << expr) & buildMask(Exprs...);
+}
+
 template <bool is64>
 inline Relocs<llvm::object::Elf_Crel_Impl<is64>>
 sortRels(Relocs<llvm::object::Elf_Crel_Impl<is64>> rels,
@@ -367,7 +390,19 @@ RelocationBaseSection &getIRelativeSection(Ctx &ctx);
 // Returns true if Expr refers a GOT entry. Note that this function returns
 // false for TLS variables even though they need GOT, because TLS variables uses
 // GOT differently than the regular variables.
-bool needsGot(RelExpr expr);
+inline bool needsGot(RelExpr expr) {
+  return oneof<R_GOT, RE_AARCH64_AUTH_GOT, RE_AARCH64_AUTH_GOT_PC, R_GOT_OFF,
+               RE_MIPS_GOT_LOCAL_PAGE, RE_MIPS_GOT_OFF, RE_MIPS_GOT_OFF32,
+               RE_AARCH64_GOT_PAGE_PC, RE_AARCH64_AUTH_GOT_PAGE_PC,
+               RE_AARCH64_AUTH_GOT_PAGE_PC, R_GOT_PC, R_GOTPLT,
+               RE_AARCH64_GOT_PAGE, RE_LOONGARCH_GOT, RE_LOONGARCH_GOT_PAGE_PC>(
+      expr);
+}
+
+inline bool needsTls(Symbol &s, RelExpr expr) {
+  return s.isTls() || oneof<R_TLSDESC_PC, R_TLSDESC_CALL>(expr);
+}
+
 } // namespace lld::elf
 
 #endif
diff --git a/lld/ELF/Target.cpp b/lld/ELF/Target.cpp
@@ -26,6 +26,7 @@
 #include "Target.h"
 #include "InputFiles.h"
 #include "OutputSections.h"
+#include "Relocations.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"
diff --git a/lld/test/ELF/icf-addend.s b/lld/test/ELF/icf-addend.s
