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

[int3]

Summary

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.

[int3]

lld/MachO/Driver.h

85

I felt that the overload of the word "section" here made things confusing

[oontvoo]

LG thanks!

lld/MachO/Driver.h

public:
   explicit DependencyTracker(llvm::StringRef path);
 
   // Adds the given path to the set of not-found files.
   inline void logFileNotFound(const Twine &path) {
     if (active)
       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);
 
 private:
   enum DepOpCode : uint8_t {
     // Denotes the linker version.
     Version = 0x00,

lld/MachO/InputFiles.h

public:
   explicit DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
                      bool isBundleLoader = false);
   explicit DylibFile(const llvm::MachO::InterfaceFile &interface,
                      DylibFile *umbrella = nullptr,
                      bool isBundleLoader = false);
 
   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; }
 
   StringRef installName;
   DylibFile *exportingFile = nullptr;
   DylibFile *umbrella;
   SmallVector<StringRef, 2> rpaths;
   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:
   bool handleLDSymbol(StringRef originalName);
   void handleLDPreviousSymbol(StringRef name, StringRef originalName);
   void handleLDInstallNameSymbol(StringRef name, StringRef originalName);
   void handleLDHideSymbol(StringRef name, StringRef originalName);
   void checkAppExtensionSafety(bool dylibIsAppExtensionSafe) const;

lld/MachO/OutputSection.h

public:
   // as-is so their file size is the same as their address space size.
   virtual uint64_t getFileSize() const { return getSize(); }
 
   // Hidden sections omit header content, but body content may still be present.
   virtual bool isHidden() const { return false; }
   // 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;
   llvm::TinyPtrVector<Defined *> sectionStartSymbols;
   llvm::TinyPtrVector<Defined *> sectionEndSymbols;
   OutputSegment *parent = nullptr;
   // For output sections that don't have explicit ordering requirements, their
   // output order should be based on the order of the input sections they

lld/MachO/SyntheticSections.h

 // All sections in __LINKEDIT should inherit from this.
 class LinkEditSection : public SyntheticSection {
 public:
   LinkEditSection(const char *segname, const char *name)
       : SyntheticSection(segname, name) {
     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
   // load commands like LC_DYLD_INFO_ONLY and LC_SYMTAB, instead of in section
   // headers.
   bool isHidden() const override final { return true; }
 
   virtual uint64_t getRawSize() const = 0;

lld/MachO/UnwindInfoSection.cpp

return r.offset % sizeof(CompactUnwindEntry<Ptr>) ==
            offsetof(CompactUnwindEntry<Ptr>, lsda);
   });
   if (it == isec->relocs.end())
     return nullptr;
   return &*it;
 }
 
 // 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;
 
   // At this point, the address space for __TEXT,__text has been
   // assigned, so we can relocate the __LD,__compact_unwind entries
   // into a temporary buffer. Relocation is necessary in order to sort
   // the CU entries by function address. Sorting is necessary so that

lld/MachO/Writer.cpp

void Writer::writeOutputFile() {
   if (auto e = buffer->commit())
     error("failed to write to the output file: " + toString(std::move(e)));
 }
 
 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();
 
   // Do not proceed if there was an undefined symbol.
   if (errorCount())
     return;
 
   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
   // yet create branch-range extension thunks for architectures whose
   // hardware call instructions have limited range, e.g., ARM(64).
   // The thunks are created as InputSections interspersed among
   // the ordinary __TEXT,_text InputSections.
   sortSegmentsAndSections();

 }
 
 template <class LP> void macho::writeResult() { Writer().run<LP>(); }
 
 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>();
   in.binding = make<BindingSection>();
   in.weakBinding = make<WeakBindingSection>();
   in.lazyBinding = make<LazyBindingSection>();
   in.exports = make<ExportSection>();
   in.got = make<GotSection>();