Warn when an output section name is longer than 8 characters

[rnk]

Summary

Microsoft's dumpbin tool cannot handle such PE files. LLVM tools and GNU
tools can, and use this to encode long section names like ".debug_info",
which is commonly used for DWARF. Don't do this in mingw mode, since
these are commonly used.

PR43754

[ruiu]

LGTM

[MaskRay]

lld/COFF/Driver.cpp

1198

Delete {}

[mstorsjo]

Microsoft's dumpbin tool cannot handle such PE files.

This had me quite surprised, as I'm sure it was able to handle them just fine.

By testing a few different versions of such tools I had lying around, I see that this worked just fine up to MSVC 2019 16.1, but in 16.2 it changed to error out with "LINK : fatal error LNK1328: missing string table". I'm a bit unsure about whether this really is intended behaviour, or just a bug.

This change in itself seems fine though.

As a heads up for slightly related matters, I'm considering implementing more options for controlling when and how the long section names are emitted (requested at https://github.com/mstorsjo/llvm-mingw/issues/56). GNU ld has options --enable-long-section-names and --disable-long-section-names (where I think disabled was the default in some ancient past, but enable is the default these days, and Rust build tooling includes the --enable-long-section-names flag, which lld/MinGW doesn't recognize yet). Haven't yet entirely decided on whether we should make those options override our current default logic (include long name if debug is enabled, except for runtime mapped sections like .eh_frame where libunwind needs to be able to locate the section name), or just make them override the defaults for non-runtime-mapped sections (like .debug_* and the rust metadata section).

[mstorsjo]

In D69594#1726649, @mstorsjo wrote:

Microsoft's dumpbin tool cannot handle such PE files.

This had me quite surprised, as I'm sure it was able to handle them just fine.

By testing a few different versions of such tools I had lying around, I see that this worked just fine up to MSVC 2019 16.1, but in 16.2 it changed to error out with "LINK : fatal error LNK1328: missing string table". I'm a bit unsure about whether this really is intended behaviour, or just a bug.

FYI, I found a existing bug report for this exact issue, https://developercommunity.visualstudio.com/content/problem/777863/dumpbin-regression-in-since-toolset-1422.html, which is marked as fixed, pending release. So apparently the dumpbin misbehaviour is just a regression.

lld/COFF/Config.h

struct Configuration {
   bool terminalServerAware = true;
   bool largeAddressAware = false;
   bool highEntropyVA = false;
   bool appContainer = false;
   bool mingw = false;
   bool warnMissingOrderSymbol = true;
   bool warnLocallyDefinedImported = true;
   bool warnDebugInfoUnusable = true;
+  bool warnLongSectionNames = true;
   bool incremental = true;
   bool integrityCheck = false;
   bool killAt = false;
   bool repro = false;
   bool swaprunCD = false;
   bool swaprunNet = false;
   bool thinLTOEmitImportsFiles;
   bool thinLTOIndexOnly;
 };
 
 extern Configuration *config;
 
 } // namespace coff
 } // namespace lld
 
 #endif

lld/COFF/Driver.cpp

for (auto *arg : args.filtered(OPT_ignore)) {
     StringRef(arg->getValue()).split(vec, ',');
     for (StringRef s : vec) {
       if (s == "4037")
         config->warnMissingOrderSymbol = false;
       else if (s == "4099")
         config->warnDebugInfoUnusable = false;
       else if (s == "4217")
         config->warnLocallyDefinedImported = false;
+      else if (s == "longsections")
+        config->warnLongSectionNames = false;
       // Other warning numbers are ignored.
     }
   }
 
   // Handle /out
   if (auto *arg = args.getLastArg(OPT_out))
     config->outputFile = arg->getValue();
 
   // Handle /verbose
   if (args.hasArg(OPT_verbose))
     config->verbose = true;
   errorHandler().verbose = config->verbose;
 
   // Handle /force or /force:unresolved
void LinkerDriver::link(ArrayRef<const char *> argsArr) {
   for (auto *arg : args.filtered(OPT_swaprun))
     parseSwaprun(arg->getValue());
   config->terminalServerAware =
       !config->dll && args.hasFlag(OPT_tsaware, OPT_tsaware_no, true);
   config->debugDwarf = debug == DebugKind::Dwarf;
   config->debugGHashes = debug == DebugKind::GHash;
   config->debugSymtab = debug == DebugKind::Symtab;
 
+  // Don't warn about long section names, such as .debug_info, for mingw or when
+  // -debug:dwarf is requested.
+  if (config->mingw || config->debugDwarf)
+    config->warnLongSectionNames = false;
+
   config->mapFile = getMapFile(args);
 
   if (config->incremental && args.hasArg(OPT_profile)) {
     warn("ignoring '/incremental' due to '/profile' specification");
     config->incremental = false;
   }
 
   if (config->incremental && args.hasArg(OPT_order)) {

lld/COFF/Writer.cpp

void Writer::createSymbolAndStringTable() {
   // solution where discardable sections have long names preserved and
   // non-discardable sections have their names truncated, to ensure that any
   // section which is mapped at runtime also has its name mapped at runtime.
   for (OutputSection *sec : outputSections) {
     if (sec->name.size() <= COFF::NameSize)
       continue;
     if ((sec->header.Characteristics & IMAGE_SCN_MEM_DISCARDABLE) == 0)
       continue;
+    if (config->warnLongSectionNames) {
+      warn("section name " + sec->name +
+           " is longer than 8 characters and will use a non-standard string "
+           "table");
+    }
     sec->setStringTableOff(addEntryToStringTable(sec->name));
   }
 
   if (config->debugDwarf || config->debugSymtab) {
     for (ObjFile *file : ObjFile::instances) {
       for (Symbol *b : file->getSymbols()) {
         auto *d = dyn_cast_or_null<Defined>(b);
         if (!d || d->writtenToSymtab)

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

 # RUN: yaml2obj < %s > %t.obj
-# RUN: lld-link /out:%t.exe /entry:main %t.obj
+# RUN: lld-link /out:%t.exe /entry:main %t.obj 2>&1 | FileCheck %s --check-prefix=WARN
 # RUN: llvm-readobj --sections %t.exe | FileCheck %s
-# RUN: lld-link /debug /out:%t2.exe /entry:main %t.obj
+# RUN: lld-link /debug /out:%t2.exe /entry:main %t.obj 2>&1 | FileCheck %s --check-prefix=WARN
 # RUN: llvm-readobj --sections %t2.exe | FileCheck %s
+#
+# No warnings in mingw mode or with ignore flag.
+# RUN: lld-link /out:%t.exe /entry:main %t.obj /ignore:longsections 2>&1 | FileCheck %s --check-prefix=IGNORE --allow-empty
+# RUN: lld-link /out:%t.exe /entry:main %t.obj -lldmingw 2>&1 | FileCheck %s --check-prefix=IGNORE --allow-empty
+# RUN: lld-link /out:%t.exe /entry:main %t.obj -debug:dwarf 2>&1 | FileCheck %s --check-prefix=IGNORE --allow-empty
+
+# WARN: warning: section name .data_long_section_name is longer than 8 characters and will use a non-standard string table
+# WARN: warning: section name .text_long_section_name is longer than 8 characters and will use a non-standard string table
+
+# IGNORE-NOT: warning:
 
 # CHECK: Name: .eh_fram (
 # CHECK: Name: .data_long_section_name
 # CHECK: Name: .text_long_section_name
 
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_AMD64