COFF: Stop emitting a non-standard COFF symbol table into PEs.

Now that our support for PDB emission is reasonably good, there is
no longer a need to emit a COFF symbol table.

Also fix a bug where we would fail to emit a string table for long
section names if /debug was not specified.

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

lld/COFF/Driver.cpp

@@ -1012,8 +1012,6 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
       Args.hasFlag(OPT_allowisolation, OPT_allowisolation_no, true);
   Config->NxCompat = Args.hasFlag(OPT_nxcompat, OPT_nxcompat_no, true);
   Config->TerminalServerAware = Args.hasFlag(OPT_tsaware, OPT_tsaware_no, true);
-  if (Args.hasArg(OPT_nosymtab))
-    Config->WriteSymtab = false;
 
   Config->MapFile = getMapFile(Args);
 

lld/COFF/Options.td

@@ -120,7 +120,6 @@ def help_q : Flag<["/?", "-?"], "">, Alias<help>;
 def debug_dwarf : F<"debug:dwarf">;
 def export_all_symbols : F<"export-all-symbols">;
 def nopdb : F<"nopdb">, HelpText<"Disable PDB generation for DWARF users">;
-def nosymtab : F<"nosymtab">;
 def lldmingw : F<"lldmingw">;
 def msvclto : F<"msvclto">;
 def output_def : Joined<["/", "-"], "output-def:">;

lld/COFF/Writer.cpp

@@ -118,7 +118,7 @@ class Writer {
   void createExportTable();
   void assignAddresses();
   void removeEmptySections();
-  void createSymbolAndStringTable();
+  void createStringTable();
   void openFile(StringRef OutputPath);
   template <typename PEHeaderTy> void writeHeader();
   void createSEHTable(OutputSection *RData);
@@ -127,9 +127,6 @@ class Writer {
   void writeBuildId();
   void sortExceptionTable();
 
-  llvm::Optional<coff_symbol16> createSymbol(Defined *D);
-  size_t addEntryToStringTable(StringRef Str);
-
   OutputSection *findSection(StringRef Name);
   OutputSection *createSection(StringRef Name);
   void addBaserels(OutputSection *Dest);
@@ -154,7 +151,7 @@ class Writer {
   ArrayRef<uint8_t> SectionTable;
 
   uint64_t FileSize;
-  uint32_t PointerToSymbolTable = 0;
+  uint32_t PointerToStringTable = 0;
   uint64_t SizeOfImage;
   uint64_t SizeOfHeaders;
 };
@@ -293,7 +290,7 @@ void Writer::run() {
   assignAddresses();
   removeEmptySections();
   setSectionPermissions();
-  createSymbolAndStringTable();
+  createStringTable();
 
   // We must do this before opening the output file, as it depends on being able
   // to read the contents of the existing output file.
@@ -470,72 +467,7 @@ void Writer::removeEmptySections() {
     Sec->SectionIndex = Idx++;
 }
 
-size_t Writer::addEntryToStringTable(StringRef Str) {
-  assert(Str.size() > COFF::NameSize);
-  size_t OffsetOfEntry = Strtab.size() + 4; // +4 for the size field
-  Strtab.insert(Strtab.end(), Str.begin(), Str.end());
-  Strtab.push_back('\0');
-  return OffsetOfEntry;
-}
-
-Optional<coff_symbol16> Writer::createSymbol(Defined *Def) {
-  // Relative symbols are unrepresentable in a COFF symbol table.
-  if (isa<DefinedSynthetic>(Def))
-    return None;
-
-  // Don't write dead symbols or symbols in codeview sections to the symbol
-  // table.
-  if (!Def->isLive())
-    return None;
-  if (auto *D = dyn_cast<DefinedRegular>(Def))
-    if (D->getChunk()->isCodeView())
-      return None;
-
-  coff_symbol16 Sym;
-  StringRef Name = Def->getName();
-  if (Name.size() > COFF::NameSize) {
-    Sym.Name.Offset.Zeroes = 0;
-    Sym.Name.Offset.Offset = addEntryToStringTable(Name);
-  } else {
-    memset(Sym.Name.ShortName, 0, COFF::NameSize);
-    memcpy(Sym.Name.ShortName, Name.data(), Name.size());
-  }
-
-  if (auto *D = dyn_cast<DefinedCOFF>(Def)) {
-    COFFSymbolRef Ref = D->getCOFFSymbol();
-    Sym.Type = Ref.getType();
-    Sym.StorageClass = Ref.getStorageClass();
-  } else {
-    Sym.Type = IMAGE_SYM_TYPE_NULL;
-    Sym.StorageClass = IMAGE_SYM_CLASS_EXTERNAL;
-  }
-  Sym.NumberOfAuxSymbols = 0;
-
-  switch (Def->kind()) {
-  case Symbol::DefinedAbsoluteKind:
-    Sym.Value = Def->getRVA();
-    Sym.SectionNumber = IMAGE_SYM_ABSOLUTE;
-    break;
-  default: {
-    uint64_t RVA = Def->getRVA();
-    OutputSection *Sec = nullptr;
-    for (OutputSection *S : OutputSections) {
-      if (S->getRVA() > RVA)
-        break;
-      Sec = S;
-    }
-    Sym.Value = RVA - Sec->getRVA();
-    Sym.SectionNumber = Sec->SectionIndex;
-    break;
-  }
-  }
-  return Sym;
-}
-
-void Writer::createSymbolAndStringTable() {
-  if (!Config->Debug || !Config->WriteSymtab)
-    return;
-
+void Writer::createStringTable() {
   // Name field in the section table is 8 byte long. Longer names need
   // to be written to the string table. First, construct string table.
   for (OutputSection *Sec : OutputSections) {
@@ -549,31 +481,19 @@ void Writer::createSymbolAndStringTable() {
     // to libunwind.
     if ((Sec->getPermissions() & IMAGE_SCN_MEM_DISCARDABLE) == 0)
       continue;
-    Sec->setStringTableOff(addEntryToStringTable(Name));
+    Sec->setStringTableOff(Strtab.size() + 4); // +4 for the size field
+    Strtab.insert(Strtab.end(), Name.begin(), Name.end());
+    Strtab.push_back('\0');
   }
 
-  for (ObjFile *File : ObjFile::Instances) {
-    for (Symbol *B : File->getSymbols()) {
-      auto *D = dyn_cast<Defined>(B);
-      if (!D || D->WrittenToSymtab)
-        continue;
-      D->WrittenToSymtab = true;
-
-      if (Optional<coff_symbol16> Sym = createSymbol(D))
-        OutputSymtab.push_back(*Sym);
-    }
-  }
+  if (Strtab.empty())
+    return;
 
   OutputSection *LastSection = OutputSections.back();
-  // We position the symbol table to be adjacent to the end of the last section.
-  uint64_t FileOff = LastSection->getFileOff() +
-                     alignTo(LastSection->getRawSize(), SectorSize);
-  if (!OutputSymtab.empty()) {
-    PointerToSymbolTable = FileOff;
-    FileOff += OutputSymtab.size() * sizeof(coff_symbol16);
-  }
-  FileOff += Strtab.size() + 4;
-  FileSize = alignTo(FileOff, SectorSize);
+  // We position the string table to be adjacent to the end of the last section.
+  PointerToStringTable = LastSection->getFileOff() +
+                         alignTo(LastSection->getRawSize(), SectorSize);
+  FileSize = alignTo(PointerToStringTable + Strtab.size() + 4, SectorSize);
 }
 
 // Visits all sections to assign incremental, non-overlapping RVAs and
@@ -760,22 +680,18 @@ template <typename PEHeaderTy> void Writer::writeHeader() {
   SectionTable = ArrayRef<uint8_t>(
       Buf - OutputSections.size() * sizeof(coff_section), Buf);
 
-  if (OutputSymtab.empty())
+  // The string table normally follows the symbol table, but because we always
+  // emit an empty symbol table, the string table appears at the location of the
+  // symbol table.
+  COFF->PointerToSymbolTable = PointerToStringTable;
+  COFF->NumberOfSymbols = 0;
+  if (Strtab.empty())
     return;
 
-  COFF->PointerToSymbolTable = PointerToSymbolTable;
-  uint32_t NumberOfSymbols = OutputSymtab.size();
-  COFF->NumberOfSymbols = NumberOfSymbols;
-  auto *SymbolTable = reinterpret_cast<coff_symbol16 *>(
-      Buffer->getBufferStart() + COFF->PointerToSymbolTable);
-  for (size_t I = 0; I != NumberOfSymbols; ++I)
-    SymbolTable[I] = OutputSymtab[I];
-  // Create the string table, it follows immediately after the symbol table.
-  // The first 4 bytes is length including itself.
-  Buf = reinterpret_cast<uint8_t *>(&SymbolTable[NumberOfSymbols]);
-  write32le(Buf, Strtab.size() + 4);
-  if (!Strtab.empty())
-    memcpy(Buf + 4, Strtab.data(), Strtab.size());
+  auto *StringTable = Buffer->getBufferStart() + PointerToStringTable;
+  // Create the string table. The first 4 bytes is length including itself.
+  write32le(StringTable, Strtab.size() + 4);
+  memcpy(StringTable + 4, Strtab.data(), Strtab.size());
 }
 
 void Writer::openFile(StringRef Path) {

lld/test/COFF/delayimports32.test

@@ -73,15 +73,15 @@ BASEREL-NEXT: ]
 
 DISASM:      202b:      68 20 10 40 00  pushl   $4198432
 DISASM-NEXT: 2030:      68 00 40 40 00  pushl   $4210688
-DISASM-NEXT: 2035:      e8 c6 ff ff ff  calll   -58 <_main@0>
+DISASM-NEXT: 2035:      e8 c6 ff ff ff  calll   -58 <.text>
 DISASM-NEXT: 203a:      5a      popl    %edx
 DISASM-NEXT: 203b:      59      popl    %ecx
 DISASM-NEXT: 203c:      ff e0   jmpl    *%eax
 DISASM-NEXT: 203e:      51      pushl   %ecx
 DISASM-NEXT: 203f:      52      pushl   %edx
 DISASM-NEXT: 2040:      68 24 10 40 00  pushl   $4198436
 DISASM-NEXT: 2045:      68 00 40 40 00  pushl   $4210688
-DISASM-NEXT: 204a:      e8 b1 ff ff ff  calll   -79 <_main@0>
+DISASM-NEXT: 204a:      e8 b1 ff ff ff  calll   -79 <.text>
 DISASM-NEXT: 204f:      5a      popl    %edx
 DISASM-NEXT: 2050:      59      popl    %ecx
 DISASM-NEXT: 2051:      ff e0   jmpl    *%eax

lld/test/COFF/long-section-name.test

@@ -1,6 +1,8 @@
 # RUN: yaml2obj < %s > %t.obj
-# RUN: lld-link /debug /out:%t.exe /entry:main %t.obj
+# RUN: lld-link /out:%t.exe /entry:main %t.obj
 # RUN: llvm-readobj -sections %t.exe | FileCheck %s
+# RUN: lld-link /debug /out:%t2.exe /entry:main %t.obj
+# RUN: llvm-readobj -sections %t2.exe | FileCheck %s
 
 # CHECK: Name: .eh_fram (
 # CHECK: Name: .data_long_section_name

lld/test/COFF/lto-opt-level.ll

@@ -1,10 +1,10 @@
 ; RUN: llvm-as -o %t.obj %s
-; RUN: lld-link /out:%t0.exe /entry:main /subsystem:console /opt:lldlto=0 /debug %t.obj
-; RUN: llvm-nm %t0.exe | FileCheck --check-prefix=CHECK-O0 %s
-; RUN: lld-link /out:%t2.exe /entry:main /subsystem:console /opt:lldlto=2 /debug %t.obj
-; RUN: llvm-nm %t2.exe | FileCheck --check-prefix=CHECK-O2 %s
-; RUN: lld-link /out:%t2a.exe /entry:main /subsystem:console /debug %t.obj
-; RUN: llvm-nm %t2a.exe | FileCheck --check-prefix=CHECK-O2 %s
+; RUN: lld-link /out:%t0.exe /entry:main /subsystem:console /opt:lldlto=0 /lldmap:%t0.map %t.obj
+; RUN: FileCheck --check-prefix=CHECK-O0 %s < %t0.map
+; RUN: lld-link /out:%t2.exe /entry:main /subsystem:console /opt:lldlto=2 /lldmap:%t2.map %t.obj
+; RUN: FileCheck --check-prefix=CHECK-O2 %s < %t2.map
+; RUN: lld-link /out:%t2a.exe /entry:main /subsystem:console /lldmap:%t2a.map %t.obj
+; RUN: FileCheck --check-prefix=CHECK-O2 %s < %t2a.map
 
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc"