[lld-macho] Flesh out STABS implementation

This addresses a lot of the comments in {D89257}. Ideally it'd have been
done in the same diff, but the commits in between make that difficult.

This diff implements:
* N_GSYM and N_STSYM, the STABS for global and static symbols
* Has the STABS reflect the section IDs of their referent symbols
* Ensures we don't fail when encountering absolute symbols or files with
  no debug info
* Sorts STABS symbols by file to minimize the number of N_OSO entries

Reviewed By: clayborg

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

4 files changed, 74 insertions, 52 deletions

diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 19d531cc5028..8a451d0dc217 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -76,6 +76,7 @@ using namespace lld::macho;
 
 std::vector<InputFile *> macho::inputFiles;
 std::unique_ptr<TarWriter> macho::tar;
+int InputFile::idCount = 0;
 
 // Open a given file path and return it as a memory-mapped file.
 Optional<MemoryBufferRef> macho::readFile(StringRef path) {
diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index f7fd77dacff5..6ca9a40a49b0 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -65,13 +65,15 @@ public:
   std::vector<Symbol *> symbols;
   ArrayRef<llvm::MachO::section_64> sectionHeaders;
   std::vector<SubsectionMap> subsections;
+  // Provides an easy way to sort InputFiles deterministically.
+  const int id;
 
 protected:
   InputFile(Kind kind, MemoryBufferRef mb)
-      : mb(mb), fileKind(kind), name(mb.getBufferIdentifier()) {}
+      : mb(mb), id(idCount++), fileKind(kind), name(mb.getBufferIdentifier()) {}
 
   InputFile(Kind kind, const llvm::MachO::InterfaceFile &interface)
-      : fileKind(kind), name(saver.save(interface.getPath())) {}
+      : id(idCount++), fileKind(kind), name(saver.save(interface.getPath())) {}
 
   void parseSections(ArrayRef<llvm::MachO::section_64>);
 
@@ -85,6 +87,8 @@ protected:
 private:
   const Kind fileKind;
   const StringRef name;
+
+  static int idCount;
 };
 
 // .o file
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index ae66961310c5..6ba067bbc9e5 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -617,22 +617,71 @@ void SymtabSection::emitObjectFileStab(ObjFile *file) {
   stabs.emplace_back(std::move(stab));
 }
 
-void SymtabSection::emitFunStabs(Defined *defined) {
-  {
-    StabsEntry stab(MachO::N_FUN);
-    stab.sect = 1;
-    stab.strx = stringTableSection.addString(defined->getName());
-    stab.value = defined->getVA();
-    stabs.emplace_back(std::move(stab));
+void SymtabSection::emitEndFunStab(Defined *defined) {
+  StabsEntry stab(MachO::N_FUN);
+  // FIXME this should be the size of the symbol. Using the section size in
+  // lieu is only correct if .subsections_via_symbols is set.
+  stab.value = defined->isec->getSize();
+  stabs.emplace_back(std::move(stab));
+}
+
+void SymtabSection::emitStabs() {
+  std::vector<Defined *> symbolsNeedingStabs;
+  for (const SymtabEntry &entry :
+       concat<SymtabEntry>(localSymbols, externalSymbols)) {
+    Symbol *sym = entry.sym;
+    if (auto *defined = dyn_cast<Defined>(sym)) {
+      if (defined->isAbsolute())
+        continue;
+      InputSection *isec = defined->isec;
+      ObjFile *file = dyn_cast_or_null<ObjFile>(isec->file);
+      if (!file || !file->compileUnit)
+        continue;
+      symbolsNeedingStabs.push_back(defined);
+    }
   }
 
-  {
-    StabsEntry stab(MachO::N_FUN);
-    // FIXME this should be the size of the symbol. Using the section size in
-    // lieu is only correct if .subsections_via_symbols is set.
-    stab.value = defined->isec->getSize();
-    stabs.emplace_back(std::move(stab));
+  llvm::stable_sort(symbolsNeedingStabs, [&](Defined *a, Defined *b) {
+    return a->isec->file->id < b->isec->file->id;
+  });
+
+  // Emit STABS symbols so that dsymutil and/or the debugger can map address
+  // regions in the final binary to the source and object files from which they
+  // originated.
+  InputFile *lastFile = nullptr;
+  for (Defined *defined : symbolsNeedingStabs) {
+    InputSection *isec = defined->isec;
+    ObjFile *file = dyn_cast<ObjFile>(isec->file);
+    assert(file);
+
+    if (lastFile == nullptr || lastFile != file) {
+      if (lastFile != nullptr)
+        emitEndSourceStab();
+      lastFile = file;
+
+      emitBeginSourceStab(file->compileUnit);
+      emitObjectFileStab(file);
+    }
+
+    StabsEntry symStab;
+    symStab.sect = defined->isec->parent->index;
+    symStab.strx = stringTableSection.addString(defined->getName());
+    symStab.value = defined->getVA();
+
+    // XXX is it right to assume that all symbols in __text are function
+    // symbols?
+    if (isec->name == "__text") {
+      symStab.type = MachO::N_FUN;
+      stabs.emplace_back(std::move(symStab));
+      emitEndFunStab(defined);
+    } else {
+      symStab.type = defined->isExternal() ? MachO::N_GSYM : MachO::N_STSYM;
+      stabs.emplace_back(std::move(symStab));
+    }
   }
+
+  if (!stabs.empty())
+    emitEndSourceStab();
 }
 
 void SymtabSection::finalizeContents() {
@@ -663,41 +712,7 @@ void SymtabSection::finalizeContents() {
     }
   }
 
-  InputFile *lastFile = nullptr;
-  for (Symbol *sym : symtab->getSymbols()) {
-    // Emit STABS symbols so that dsymutil and/or the debugger can map address
-    // regions in the final binary to the source and object files from which
-    // they originated.
-    if (auto *defined = dyn_cast<Defined>(sym)) {
-      if (defined->isAbsolute())
-        continue;
-
-      InputSection *isec = defined->isec;
-      // XXX is it right to assume that all symbols in __text are function
-      // symbols?
-      if (isec->name == "__text") {
-        ObjFile *file = dyn_cast<ObjFile>(isec->file);
-        assert(file);
-        if (!file->compileUnit)
-          continue;
-
-        if (lastFile == nullptr || lastFile != file) {
-          if (lastFile != nullptr)
-            emitEndSourceStab();
-          lastFile = file;
-
-          emitBeginSourceStab(file->compileUnit);
-          emitObjectFileStab(file);
-        }
-        emitFunStabs(defined);
-      }
-      // TODO emit stabs for non-function symbols too
-    }
-  }
-
-  if (!stabs.empty())
-    emitEndSourceStab();
-
+  emitStabs();
   uint32_t symtabIndex = stabs.size();
   for (const SymtabEntry &entry :
        concat<SymtabEntry>(localSymbols, externalSymbols, undefinedSymbols)) {
diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index 56f4bf66ce23..0f3775b11b47 100644
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -410,12 +410,13 @@ struct SymtabEntry {
 };
 
 struct StabsEntry {
-  uint8_t type;
+  uint8_t type = 0;
   uint32_t strx = 0;
   uint8_t sect = 0;
   uint16_t desc = 0;
   uint64_t value = 0;
 
+  StabsEntry() = default;
   explicit StabsEntry(uint8_t type) : type(type) {}
 };
 
@@ -440,7 +441,8 @@ private:
   void emitBeginSourceStab(llvm::DWARFUnit *compileUnit);
   void emitEndSourceStab();
   void emitObjectFileStab(ObjFile *);
-  void emitFunStabs(Defined *);
+  void emitEndFunStab(Defined *);
+  void emitStabs();
 
   StringTableSection &stringTableSection;
   // STABS symbols are always local symbols, but we represent them with special