[lld-macho] Don't fold subsections with symbols at nonzero offsets

Symbols occur at non-zero offsets in a subsection if they are
`.alt_entry` symbols, or if `.subsections_via_symbols` is omitted.

It doesn't seem like ld64 supports folding those subsections either.
Moreover, supporting this it makes `foldIdentical` a lot more
complicated to implement. The existing implementation has some
questionable behavior around STABS omission -- if a section with an
non-zero offset symbol was folded into one without, we would omit the
STABS entry for the non-zero offset symbol.

I will be following up with a diff that makes `foldIdentical` zero out
the symbol sizes for folded symbols. Again, this is much easier to
implement if we don't have to worry about non-zero offsets.

Reviewed By: #lld-macho, oontvoo

Differential Revision: https://reviews.llvm.org/D136000

diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp
index 4f4b8fd..bc3dd99 100644 (file)
--- a/lld/MachO/ICF.cpp
+++ b/lld/MachO/ICF.cpp
@@ -425,8 +425,8 @@ void macho::foldIdenticalSections(bool onlyCfStrings) {
     bool isFoldable = (!onlyCfStrings || isCfStringSection(isec)) &&
                       (isCodeSection(isec) || isFoldableWithAddendsRemoved ||
                        isGccExceptTabSection(isec)) &&
-                      !isec->keepUnique && !isec->shouldOmitFromOutput() &&
-                      hasFoldableFlags;
+                      !isec->keepUnique && !isec->hasAltEntry &&
+                      !isec->shouldOmitFromOutput() && hasFoldableFlags;
     if (isFoldable) {
       foldable.push_back(isec);
       for (Defined *d : isec->symbols)

diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index afd0e28..e542fe2 100644 (file)
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -854,6 +854,7 @@ void ObjFile::parseSymbols(ArrayRef<typename LP::section> sectionHeaders,
       //   4. If we have a literal section (e.g. __cstring and __literal4).
       if (!subsectionsViaSymbols || symbolOffset == 0 ||
           sym.n_desc & N_ALT_ENTRY || !isa<ConcatInputSection>(isec)) {
+        isec->hasAltEntry = symbolOffset != 0;
         symbols[symIndex] = createDefined(sym, name, isec, symbolOffset,
                                           symbolSize, forceHidden);
         continue;

diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index ca5c752..398a5c2 100644 (file)
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -141,31 +141,15 @@ void ConcatInputSection::foldIdentical(ConcatInputSection *copy) {
   for (auto &copySym : copy->symbols)
     copySym->wasIdenticalCodeFolded = true;
 
-  // Merge the sorted vectors of symbols together.
-  auto it = symbols.begin();
-  for (auto copyIt = copy->symbols.begin(); copyIt != copy->symbols.end();) {
-    if (it == symbols.end()) {
-      symbols.push_back(*copyIt++);
-      it = symbols.end();
-    } else if ((*it)->value > (*copyIt)->value) {
-      std::swap(*it++, *copyIt);
-    } else {
-      ++it;
-    }
-  }
+  symbols.insert(symbols.end(), copy->symbols.begin(), copy->symbols.end());
   copy->symbols.clear();
 
   // Remove duplicate compact unwind info for symbols at the same address.
   if (symbols.empty())
     return;
-  it = symbols.begin();
-  uint64_t v = (*it)->value;
-  for (++it; it != symbols.end(); ++it) {
-    Defined *d = *it;
-    if (d->value == v)
-      d->unwindEntry = nullptr;
-    else
-      v = d->value;
+  for (auto it = symbols.begin() + 1; it != symbols.end(); ++it) {
+    assert((*it)->value == 0);
+    (*it)->unwindEntry = nullptr;
   }
 }
 

diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 1a97068..5a6a205 100644 (file)
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -64,11 +64,12 @@ public:
 protected:
   InputSection(Kind kind, const Section &section, ArrayRef<uint8_t> data,
                uint32_t align)
-      : sectionKind(kind), align(align), data(data), section(section) {}
+      : sectionKind(kind), keepUnique(false), hasAltEntry(false), align(align),
+        data(data), section(section) {}
 
   InputSection(const InputSection &rhs)
-      : sectionKind(rhs.sectionKind), align(rhs.align), data(rhs.data),
-        section(rhs.section) {}
+      : sectionKind(rhs.sectionKind), keepUnique(false), hasAltEntry(false),
+        align(rhs.align), data(rhs.data), section(rhs.section) {}
 
   Kind sectionKind;
 
@@ -77,7 +78,10 @@ public:
   bool isFinal = false;
   // keep the address of the symbol(s) in this section unique in the final
   // binary ?
-  bool keepUnique = false;
+  bool keepUnique : 1;
+  // Does this section have symbols at offsets other than zero? (NOTE: only
+  // applies to ConcatInputSections.)
+  bool hasAltEntry : 1;
   uint32_t align = 1;
 
   OutputSection *parent = nullptr;

diff --git a/lld/test/MachO/icf.s b/lld/test/MachO/icf.s
index 4325102..03b4743 100644 (file)
--- a/lld/test/MachO/icf.s
+++ b/lld/test/MachO/icf.s
@@ -25,7 +25,7 @@
 # CHECK: [[#%x,DYLIB_REF_4:]]               l     F __TEXT,__text _dylib_ref_4
 # CHECK: [[#%x,ALT:]]                       l     F __TEXT,__text _alt
 # CHECK: [[#%x,WITH_ALT_ENTRY:]]            l     F __TEXT,__text _with_alt_entry
-# CHECK: [[#%x,WITH_ALT_ENTRY]]             l     F __TEXT,__text _no_alt_entry
+# CHECK: [[#%x,NO_ALT_ENTRY:]]              l     F __TEXT,__text _no_alt_entry
 # CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_2:]] l     F __TEXT,__text _defined_ref_with_addend_1
 # CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_2]]  l     F __TEXT,__text _defined_ref_with_addend_2
 # CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_3:]] l     F __TEXT,__text _defined_ref_with_addend_3
@@ -74,7 +74,7 @@
 # CHECK: callq 0x[[#%x,DYLIB_REF_4]]               <_dylib_ref_4>
 # CHECK: callq 0x[[#%x,ALT]]                       <_alt>
 # CHECK: callq 0x[[#%x,WITH_ALT_ENTRY]]            <_with_alt_entry>
-# CHECK: callq 0x[[#%x,WITH_ALT_ENTRY]]            <_with_alt_entry>
+# CHECK: callq 0x[[#%x,NO_ALT_ENTRY]]              <_no_alt_entry>
 # CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_2]] <_defined_ref_with_addend_2>
 # CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_2]] <_defined_ref_with_addend_2>
 # CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_3]] <_defined_ref_with_addend_3>
@@ -161,8 +161,7 @@ _dylib_ref_4:
   mov ___nan + 1@GOTPCREL(%rip), %rax
   callq ___inf + 1
 
-## We can merge two sections even if one of them has an alt entry. Just make
-## sure we don't merge the alt entry symbol with a regular symbol.
+## Sections with alt entries cannot be merged.
 .alt_entry _alt
 _with_alt_entry:
   movq $3132, %rax