[PATCH] D77893: [lld] Merge Mach-O input sections

[Jez Ng via Phabricator via llvm-commits]

int3 marked 2 inline comments as done.
int3 added a comment.

Noticed a couple of minor things while rebasing on top of this. I've folded changes for them into D78270: [lld-macho] Support calls to functions in dylibs <https://reviews.llvm.org/D78270>.



================
Comment at: lld/MachO/OutputSection.h:40
+  // Hidden sections omit header content, but body content is still present.
+  virtual bool isHidden() const { return !this->isNeeded(); }
+  // Unneeded sections are omitted entirely (header and body).
----------------
IMO this should just be `return true`. Whether a section is hidden is orthogonal from whether it is needed: hidden sections will never have a header regardless of whether they have a body. (I know we override this method with `return false` for synthetic sections, but regardless I think it's confusing to write it this way for non-synthetic sections.)


================
Comment at: lld/MachO/Writer.cpp:117-118
 
-    if (seg->getSections().empty())
+    if (!seg->isNeeded())
       return;
 
----------------
I think this should stay as `getSections().empty()`, and the check for `isNeeded()` should be moved outside writeTo(). We should just not create LCSegment commands for unneeded segments. The `empty()` check however is still needed because `__LINKEDIT` can be empty.


================
Comment at: lld/MachO/Writer.cpp:368-369
+    for (auto &p : seg->getSections()) {
+      OutputSection *section = p.second;
+      section->writeTo(buf + section->fileOff);
     }
----------------
we should avoid writing unneeded output sections (My stacked diff makes that assumption for one of the stub helper synthetic section)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77893/new/

https://reviews.llvm.org/D77893