[lld-macho][nfc] Comments and style fixes

Added some comments (particularly around finalize() and
finalizeContents()) as well as doing some rephrasing / grammar fixes for
existing comments.

Also did some minor style fixups, such as by putting methods together in
a class definition and having fields of similar types next to each
other.

Reviewed By: #lld-macho, oontvoo

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

diff --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
index c293334..dbfc05a 100644 (file)
--- a/lld/MachO/Driver.h
+++ b/lld/MachO/Driver.h
@@ -81,9 +81,8 @@ public:
       notFounds.insert(path.str());
   }
 
-  // Writes the dependencies to specified path.
-  // The content is sorted by its Op Code, then within each section,
-  // alphabetical order.
+  // Writes the dependencies to specified path. The content is first sorted by
+  // OpCode and then by the filename (in alphabetical order).
   void write(llvm::StringRef version,
              const llvm::SetVector<InputFile *> &inputs,
              llvm::StringRef output);

diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 6a4a4fd..0b661c8 100644 (file)
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -177,6 +177,7 @@ public:
 
   void parseLoadCommands(MemoryBufferRef mb);
   void parseReexports(const llvm::MachO::InterfaceFile &interface);
+  bool isReferenced() const { return numReferencedSymbols > 0; }
 
   static bool classof(const InputFile *f) { return f->kind() == DylibKind; }
 
@@ -187,21 +188,17 @@ public:
   uint32_t compatibilityVersion = 0;
   uint32_t currentVersion = 0;
   int64_t ordinal = 0; // Ordinal numbering starts from 1, so 0 is a sentinel
+  unsigned numReferencedSymbols = 0;
   RefState refState;
   bool reexport = false;
   bool forceNeeded = false;
   bool forceWeakImport = false;
   bool deadStrippable = false;
   bool explicitlyLinked = false;
-
-  unsigned numReferencedSymbols = 0;
-
-  bool isReferenced() const { return numReferencedSymbols > 0; }
-
   // An executable can be used as a bundle loader that will load the output
   // file being linked, and that contains symbols referenced, but not
   // implemented in the bundle. When used like this, it is very similar
-  // to a Dylib, so we re-used the same class to represent it.
+  // to a dylib, so we've used the same class to represent it.
   bool isBundleLoader;
 
 private:

diff --git a/lld/MachO/OutputSection.h b/lld/MachO/OutputSection.h
index eb55485..51f39dd 100644 (file)
--- a/lld/MachO/OutputSection.h
+++ b/lld/MachO/OutputSection.h
@@ -58,13 +58,23 @@ public:
   // Unneeded sections are omitted entirely (header and body).
   virtual bool isNeeded() const { return true; }
 
-  virtual void finalize() {
-    // TODO investigate refactoring synthetic section finalization logic into
-    // overrides of this function.
-  }
+  // The implementations of this method can assume that it is only called right
+  // before addresses get assigned to this particular OutputSection. In
+  // particular, this means that it gets called only after addresses have been
+  // assigned to output sections that occur earlier in the output binary.
+  // Naturally, this means different sections' finalize() methods cannot execute
+  // concurrently with each other. As such, avoid using this method for
+  // operations that do not require this strict sequential guarantee.
+  //
+  // Operations that need to occur late in the linking process, but which do not
+  // need the sequential guarantee, should be named `finalizeContents()`. See
+  // e.g. LinkEditSection::finalizeContents() and
+  // CStringSection::finalizeContents().
+  virtual void finalize() {}
 
   virtual void writeTo(uint8_t *buf) const = 0;
 
+  // Handle section$start$ and section$end$ symbols.
   void assignAddressesToStartEndSymbols();
 
   StringRef name;

diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index 49b68c7..12e422b 100644 (file)
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -62,6 +62,8 @@ public:
     align = target->wordSize;
   }
 
+  // Implementations of this method can assume that the regular (non-__LINKEDIT)
+  // sections already have their addresses assigned.
   virtual void finalizeContents() {}
 
   // Sections in __LINKEDIT are special: their offsets are recorded in the

diff --git a/lld/MachO/UnwindInfoSection.cpp b/lld/MachO/UnwindInfoSection.cpp
index 49af2f6..8b1e357 100644 (file)
--- a/lld/MachO/UnwindInfoSection.cpp
+++ b/lld/MachO/UnwindInfoSection.cpp
@@ -392,7 +392,7 @@ UnwindInfoSectionImpl<Ptr>::findLsdaReloc(ConcatInputSection *isec) const {
 }
 
 // Scan the __LD,__compact_unwind entries and compute the space needs of
-// __TEXT,__unwind_info and __TEXT,__eh_frame
+// __TEXT,__unwind_info and __TEXT,__eh_frame.
 template <class Ptr> void UnwindInfoSectionImpl<Ptr>::finalize() {
   if (symbols.empty())
     return;

diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 2c0794e..851cb3d 100644 (file)
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -1108,8 +1108,10 @@ template <class LP> void Writer::run() {
   treatSpecialUndefineds();
   if (config->entry && !isa<Undefined>(config->entry))
     prepareBranchTarget(config->entry);
+
   // Canonicalization of all pointers to InputSections should be handled by
-  // these two methods.
+  // these two scan* methods. I.e. from this point onward, for all live
+  // InputSections, we should have `isec->canonical() == isec`.
   scanSymbols();
   scanRelocations();
 
@@ -1119,6 +1121,8 @@ template <class LP> void Writer::run() {
 
   if (in.stubHelper->isNeeded())
     in.stubHelper->setup();
+  // At this point, we should know exactly which output sections are needed,
+  // courtesy of scanSymbols() and scanRelocations().
   createOutputSections<LP>();
 
   // After this point, we create no new segments; HOWEVER, we might
@@ -1146,11 +1150,10 @@ void macho::resetWriter() { LCDylib::resetInstanceCount(); }
 
 void macho::createSyntheticSections() {
   in.header = make<MachHeaderSection>();
-  if (config->dedupLiterals) {
+  if (config->dedupLiterals)
     in.cStringSection = make<DeduplicatedCStringSection>();
-  } else {
+  else
     in.cStringSection = make<CStringSection>();
-  }
   in.wordLiteralSection =
       config->dedupLiterals ? make<WordLiteralSection>() : nullptr;
   in.rebase = make<RebaseSection>();