[LLD] [COFF] Omit IMAGE_SYM_CLASS_LABEL symbols from the PE symbol table

[mstorsjo]

Summary

Also omit section definition symbols. This matches the condition for
symbols to omit from map files.

If we start producing lots of IMAGE_SYM_CLASS_LABEL symbols due to
D113865, it inflates the size of mingw style default linked binaries
(with dwarf debug info and a regular PE symbol table) a fair bit
(by 14% in one test).

By omitting these symbols, the size remains the same as before, and
by omitting the section definition symbols we further shrink the
mingw style unstripped binaries by 14% in one test.

[rnk]

Seems reasonable, just style nits

lld/COFF/Writer.cpp

1213–1214

If dc will only be used in the if, it's preferred to declare the variable inside the if condition.

1213–1219

Please keep this grouped with the check, so it's clear that it's one operation to ensure that each symbol is written at most once.

[mstorsjo]

Adjusted the code as suggested.

[mstorsjo]

lld/COFF/Writer.cpp

1213–1214

Sure, thanks.

1213–1219

Ok, will do. I was a bit divided over this while writing the initial version, as it also seems counterintuitive to set a flag "written" while we might still bail out and not write it. But setting the flag avoids needlessly checking the storage class multiple times (and keeps the flag check more together).

[rnk]

lgtm

lld/COFF/Writer.cpp

void Writer::createSymbolAndStringTable() {
   }
 
   if (config->debugDwarf || config->debugSymtab) {
     for (ObjFile *file : ctx.objFileInstances) {
       for (Symbol *b : file->getSymbols()) {
         auto *d = dyn_cast_or_null<Defined>(b);
         if (!d || d->writtenToSymtab)
           continue;
         d->writtenToSymtab = true;
+        if (auto *dc = dyn_cast_or_null<DefinedCOFF>(d)) {
+          COFFSymbolRef symRef = dc->getCOFFSymbol();
+          if (symRef.isSectionDefinition() ||
+              symRef.getStorageClass() == COFF::IMAGE_SYM_CLASS_LABEL)
+            continue;
+        }
 
         if (Optional<coff_symbol16> sym = createSymbol(d))
           outputSymtab.push_back(*sym);
       }
     }
   }
 
   if (outputSymtab.empty() && strtab.empty())

lld/test/COFF/strtab-size.s

 sym187:
 sym188:
 sym189:
 sym190:
 sym191:
 sym192:
 sym193:
 sym194:
+sym195:
   ret

lld/test/COFF/symtab.test

 # RUN: yaml2obj %s -o %t.obj
 # RUN: lld-link /debug:dwarf /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
 # RUN: llvm-readobj --symbols %t.exe | FileCheck %s
 # RUN: lld-link /debug:dwarf /opt:noref /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
 # RUN: llvm-readobj --symbols %t.exe | FileCheck %s
 # RUN: lld-link /debug:symtab /opt:noref /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
 # RUN: llvm-readobj --symbols %t.exe | FileCheck %s
 
 # RUN: lld-link /debug /out:%t.exe /entry:main %t.obj %p/Inputs/std64.lib
 # RUN: llvm-readobj --symbols %t.exe | FileCheck -check-prefix=NO %s
 
 # CHECK:      Symbols [
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: .text
-# CHECK-NEXT:     Value: 0
-# CHECK-NEXT:     Section: .text (1)
-# CHECK-NEXT:     BaseType: Null (0x0)
-# CHECK-NEXT:     ComplexType: Null (0x0)
-# CHECK-NEXT:     StorageClass: Static (0x3)
-# CHECK-NEXT:     AuxSymbolCount: 0
-# CHECK-NEXT:   }
-# CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: .text2
-# CHECK-NEXT:     Value: 0
-# CHECK-NEXT:     Section: .text (1)
-# CHECK-NEXT:     BaseType: Null (0x0)
-# CHECK-NEXT:     ComplexType: Null (0x0)
-# CHECK-NEXT:     StorageClass: Static (0x3)
-# CHECK-NEXT:     AuxSymbolCount: 0
-# CHECK-NEXT:   }
-# CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: .data
-# CHECK-NEXT:     Value: 0
-# CHECK-NEXT:     Section: .data (3)
-# CHECK-NEXT:     BaseType: Null (0x0)
-# CHECK-NEXT:     ComplexType: Null (0x0)
-# CHECK-NEXT:     StorageClass: Static (0x3)
-# CHECK-NEXT:     AuxSymbolCount: 0
-# CHECK-NEXT:   }
-# CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Name: MessageBoxA
 # CHECK-NEXT:     Value: 80
 # CHECK-NEXT:     Section: .text (1)
 # CHECK-NEXT:     BaseType: Null (0x0)
 # CHECK-NEXT:     ComplexType: Null (0x0)
 # CHECK-NEXT:     StorageClass: External (0x2)
 # CHECK-NEXT:     AuxSymbolCount: 0
 # CHECK-NEXT:   }
- Name:            abs_symbol
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
   - Name:            __ImageBase
     Value:           0
     SectionNumber:   0
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            .Ltemp_symbol
+    Value:           1
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_LABEL
 ...