[PATCH] D11023: COFF: Emit a symbol table if /debug is specified

[Rui Ueyama]

Can you add a test for the symbol table? I don't think long-section-name test covers the new feature.


================
Comment at: COFF/InputFiles.cpp:149
@@ -148,3 +148,3 @@
     }
-    if (Name.startswith(".debug"))
+    if (!Config->Debug && Name.startswith(".debug"))
       continue;
----------------
Add a comment saying that we want to preserve DWARF debug sections only when /debug is on.

================
Comment at: COFF/Writer.cpp:288
@@ +287,3 @@
+      RoundUpToAlignment(LastSection->getRawSize(), FileAlignment);
+  if (Config->Debug) {
+    // Name field in the section table is 8 byte long. Longer names need
----------------
Return early by inverting the condition.

================
Comment at: COFF/Writer.cpp:301
@@ +300,3 @@
+      for (SymbolBody *B : File->getSymbols()) {
+        if (auto *D = dyn_cast<DefinedRegular>(B)) {
+          if (!D->isLive())
----------------
I prefer

  auto *D = dyn_cast<...
  if (!D || !D->isLive())
    continue;

to reduce indentation level.

================
Comment at: COFF/Writer.cpp:317-320
@@ +316,6 @@
+          if (Name.size() > COFF::NameSize) {
+            Sym.Name.Offset.Zeroes = 0;
+            Sym.Name.Offset.Offset = Strtab.size() + 4; // +4 for the size field
+            Strtab.insert(Strtab.end(), Name.begin(), Name.end());
+            Strtab.push_back('\0');
+          } else {
----------------
Make this a separate member function.

================
Comment at: COFF/Writer.cpp:328-329
@@ +327,4 @@
+          Sym.SectionNumber = SymSec->SectionIndex;
+          Sym.StorageClass = IMAGE_SYM_CLASS_NULL;
+          Sym.NumberOfAuxSymbols = 0;
+          OutputSymtab.push_back(Sym);
----------------
If these actual values are 0, you don't need to set.

================
Comment at: COFF/Writer.cpp:492
@@ -443,17 +491,3 @@
 
-  // Write string table if we need to. The string table immediately
-  // follows the symbol table, so we create a dummy symbol table
-  // first. The symbol table contains one dummy symbol.
-  if (Strtab.empty())
-    return;
-  COFF->PointerToSymbolTable = Buf - Buffer->getBufferStart();
-  COFF->NumberOfSymbols = 1;
-  auto *SymbolTable = reinterpret_cast<coff_symbol16 *>(Buf);
-  Buf += sizeof(*SymbolTable);
-  // (Set 4 to make the dummy symbol point to the first string table
-  // entry, so that tools to print out symbols don't read NUL bytes.)
-  SymbolTable->Name.Offset.Offset = 4;
-  // Then create the symbol table. The first 4 bytes is length
-  // including itself.
-  write32le(Buf, Strtab.size() + 4);
-  memcpy(Buf + 4, Strtab.data(), Strtab.size());
+  if (!OutputSymtab.empty()) {
+    OutputSection *LastSection = OutputSections.back();
----------------
Return early.

================
Comment at: COFF/Writer.cpp:504
@@ +503,3 @@
+    Buf = reinterpret_cast<uint8_t *>(&SymbolTable[NumberOfSymbols]);
+    // Then create the string table. The first 4 bytes is length
+    // including itself.
----------------
Update comment ("then" no longer makes sense.)

================
Comment at: COFF/Writer.h:64
@@ -65,2 +63,3 @@
 
+  uint32_t SectionIndex = 0;
 private:
----------------
Add a comment saying that this is 1-based section index.

================
Comment at: COFF/Writer.h:65
@@ -66,2 +64,3 @@
+  uint32_t SectionIndex = 0;
 private:
   StringRef Name;
----------------
Add a blank line before this line.


http://reviews.llvm.org/D11023