Address some of Fangrui's review comments

Author: pranavk

lld/ELF/ICF.cpp

@@ -245,10 +245,13 @@ 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);
+  // For our use cases, we can get by without calculating exact location within
+  // the section, and just use fake location array. We need to ensure validity
+  // for loc[-1] to loc[3] as various targets' getRelExpr() reference them.
+  std::array<uint8_t, 5> fakeLocArray;
+  uint8_t *fakeLoc = fakeLocArray.data() + 1;
   RelExpr expr = ctx.target->getRelExpr(reloc.getType(ctx.arg.isMips64EL), s,
-                                        a->content().data() + offset);
+                                        fakeLoc);
 
   if (needsGot(expr) || needsTls(s, expr))
     return false;
@@ -412,7 +415,7 @@ bool ICF<ELFT>::variableEq(InputSection *secA, Relocs<RelTy> ra,
     // getting merged into each other (done later in ICF). We do this as
     // post-ICF passes cannot handle duplicates when iterating over local
     // symbols. There are also assertions that prevent this.
-    if ((!da->isGlobal() || !db->isGlobal()) &&
+    if ((da->isLocal() || db->isLocal()) &&
         !isTrivialRelocation(secA, sa, *rai))
       return false;
 

lld/ELF/InputSection.h

@@ -514,49 +514,6 @@ 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;

lld/ELF/Relocations.cpp

@@ -371,6 +371,49 @@ 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 {

lld/ELF/Target.cpp

@@ -26,7 +26,6 @@
 #include "Target.h"
 #include "InputFiles.h"
 #include "OutputSections.h"
-#include "Relocations.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"