[LLD] [MinGW] Implement the --no-seh flag

[mstorsjo]

Summary

Previously this flag was just ignored. If set, set the IMAGE_DLL_CHARACTERISTICS_NO_SEH bit, regardless of the normal safeSEH machinery.

In mingw configurations, the safeSEH bit might not be set in e.g. object files built from handwritten assembly, making it impossible to use the normal safeseh flag. As mingw setups don't generally use SEH on 32 bit x86 at all, it should be fine to set that flag bit though - hook up the existing GNU ld flag for controlling that.

@hans - This isn't a regression fix, but a fairly safe new option, that might be important for VLC (it might fix https://code.videolan.org/videolan/vlc-winrt/-/issues/303, issues in certifying VLC built with llvm-mingw, due to this flag missing).

[mstorsjo]

Actually, this does fix a regression in one sense. Prior to LLD 9, the IMAGE_DLL_CHARACTERISTICS_NO_SEH flag did end up set even in mingw configurations, so ignoring the --no-seh option was just fine. But for LLD 9, the safeSEH mechanism implementation was improved, and this flag no longer was set in mingw configurations.

[mstorsjo]

Updated the patch; the previous one was made on top of another local patch.

[amccarth]

This is probably fine, be I have an inline comment with some questions.

lld/COFF/Writer.cpp

1396

Should setNoSEHCharacteristic and config->noSEH always agree? What happens when they don't?

setNoSEHCharacteristic is set in Writer::createSEHTable when there are no SEH handlers. Should this logical-OR happen there?

What happens if the user chooses --noSEH but there are SEH handlers?

[mstorsjo]

lld/COFF/Writer.cpp

1396

Should setNoSEHCharacteristic and config->noSEH always agree? What happens when they don't?

setNoSEHCharacteristic is set in Writer::createSEHTable when there are no SEH handlers. Should this logical-OR happen there?

We generally don't want to call createSEHTable in mingw mode. That function errors out on any input object file without the @feat marker indicating support for safe SEH (which happens for e.g. most handwritten mingw asm sources).

In practice, that doesn't happen as the safeseh option defaults to false in mingw mode, and the lld-link level /safeseh option isn't set by any mingw driver option (unless it's given -Xlink=/safeseh to manually set lld-link level options from the mingw driver).

What happens if the user chooses --noSEH but there are SEH handlers?

In that case, the image could end up lying about this aspect.

Mingw mode compilers don't use SEH at all in i386 mode at the moment though, so the mingw driver level --no-seh flag should be pretty safe for users. And it matches what GNU ld does in this case; if the option is set, the capability flag is set, without verification.

For msvc mode users, the lld-link level /noseh option that is added here is undocumented and private, and thus not meant to be used by users directly - only by the mingw driver.

[hans]

In D84701#2177048, @mstorsjo wrote:

Actually, this does fix a regression in one sense. Prior to LLD 9, the IMAGE_DLL_CHARACTERISTICS_NO_SEH flag did end up set even in mingw configurations, so ignoring the --no-seh option was just fine. But for LLD 9, the safeSEH mechanism implementation was improved, and this flag no longer was set in mingw configurations.

Thanks, will merge once it lands.

[amccarth]

LGTM!

[hans]

In D84701#2177968, @hans wrote:

In D84701#2177048, @mstorsjo wrote:

Actually, this does fix a regression in one sense. Prior to LLD 9, the IMAGE_DLL_CHARACTERISTICS_NO_SEH flag did end up set even in mingw configurations, so ignoring the --no-seh option was just fine. But for LLD 9, the safeSEH mechanism implementation was improved, and this flag no longer was set in mingw configurations.

Thanks, will merge once it lands.

70b2872f4810569173c7042c51333d83deb16d88

lld/COFF/Config.h

struct Configuration {
 
   // /guard:cf
   GuardCFLevel guardCF = GuardCFLevel::Off;
 
   // Used for SafeSEH.
   bool safeSEH = false;
   Symbol *sehTable = nullptr;
   Symbol *sehCount = nullptr;
+  bool noSEH = false;
 
   // Used for /opt:lldlto=N
   unsigned ltoo = 2;
 
   // Used for /opt:lldltojobs=N
   std::string thinLTOJobs;
   // Used for /opt:lldltopartitions=N
   unsigned ltoPartitions = 1;

lld/COFF/Driver.cpp

void LinkerDriver::link(ArrayRef<const char *> argsArr) {
   // not we assume x64.
   if (config->machine == IMAGE_FILE_MACHINE_UNKNOWN) {
     warn("/machine is not specified. x64 is assumed");
     config->machine = AMD64;
   }
   config->wordsize = config->is64() ? 8 : 4;
 
   // Handle /safeseh, x86 only, on by default, except for mingw.
-  if (config->machine == I386 &&
-      args.hasFlag(OPT_safeseh, OPT_safeseh_no, !config->mingw))
-    config->safeSEH = true;
+  if (config->machine == I386) {
+    config->safeSEH = args.hasFlag(OPT_safeseh, OPT_safeseh_no, !config->mingw);
+    config->noSEH = args.hasArg(OPT_noseh);
+  }
 
   // Handle /functionpadmin
   for (auto *arg : args.filtered(OPT_functionpadmin, OPT_functionpadmin_opt))
     parseFunctionPadMin(arg, config->machine);
 
   if (tar)
     tar->append("response.txt",
                 createResponseFile(args, filePaths,

lld/COFF/Options.td

 def export_all_symbols : F<"export-all-symbols">;
 defm demangle : B<"demangle",
     "Demangle symbols in output (default)",
     "Do not demangle symbols in output">;
 def include_optional : Joined<["/", "-", "/?", "-?"], "includeoptional:">,
     HelpText<"Add symbol as undefined, but allow it to remain undefined">;
 def kill_at : F<"kill-at">;
 def lldmingw : F<"lldmingw">;
+def noseh : F<"noseh">;
 def output_def : Joined<["/", "-", "/?", "-?"], "output-def:">;
 def pdb_source_path : P<"pdbsourcepath",
     "Base path used to make relative source file path absolute in PDB">;
 def rsp_quoting : Joined<["--"], "rsp-quoting=">,
   HelpText<"Quoting style for response files, 'windows' (default) or 'posix'">;
 def start_lib : F<"start-lib">,
   HelpText<"Starts group of objects treated as if they were in a library">;
 def thinlto_emit_imports_files :

lld/COFF/Writer.cpp

template <typename PEHeaderTy> void Writer::writeHeader() {
   if (config->nxCompat)
     pe->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NX_COMPAT;
   if (!config->allowIsolation)
     pe->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NO_ISOLATION;
   if (config->guardCF != GuardCFLevel::Off)
     pe->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_GUARD_CF;
   if (config->integrityCheck)
     pe->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_FORCE_INTEGRITY;
-  if (setNoSEHCharacteristic)
+  if (setNoSEHCharacteristic || config->noSEH)
     pe->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NO_SEH;
   if (config->terminalServerAware)
     pe->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE;
   pe->NumberOfRvaAndSize = numberOfDataDirectory;
   if (textSec->getVirtualSize()) {
     pe->BaseOfCode = textSec->getRVA();
     pe->SizeOfCode = textSec->getRawSize();
   }

lld/MinGW/Driver.cpp

bool mingw::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   if (args.hasArg(OPT_export_all_symbols))
     add("-export-all-symbols");
   if (args.hasArg(OPT_large_address_aware))
     add("-largeaddressaware");
   if (args.hasArg(OPT_kill_at))
     add("-kill-at");
   if (args.hasArg(OPT_appcontainer))
     add("-appcontainer");
+  if (args.hasArg(OPT_no_seh))
+    add("-noseh");
 
   if (args.getLastArgValue(OPT_m) != "thumb2pe" &&
       args.getLastArgValue(OPT_m) != "arm64pe" && !args.hasArg(OPT_dynamicbase))
     add("-dynamicbase:no");
 
   if (args.hasFlag(OPT_no_insert_timestamp, OPT_insert_timestamp, false))
     add("-timestamp:0");
 

lld/MinGW/Options.td

defm major_subsystem_version: EqLong<"major-subsystem-version",
      "Set the OS and subsystem major version">;
 defm map: Eq<"Map", "Output a linker map">;
 defm minor_os_version: EqLong<"minor-os-version",
      "Set the OS and subsystem minor version">;
 defm minor_subsystem_version: EqLong<"minor-subsystem-version",
      "Set the OS and subsystem minor version">;
 def no_insert_timestamp: F<"no-insert-timestamp">,
     HelpText<"Don't include PE header timestamp">;
+def no_seh: F<"no-seh">, HelpText<"Set the 'no SEH' flag in the executable">;
 def no_whole_archive: F<"no-whole-archive">,
     HelpText<"No longer include all object files for following archives">;
 def large_address_aware: Flag<["--"], "large-address-aware">,
     HelpText<"Enable large addresses">;
 def no_gc_sections: F<"no-gc-sections">, HelpText<"Don't remove unused sections">;
 def o: JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
   HelpText<"Path to file to write output">;
 defm out_implib: Eq<"out-implib", "Import library name">;

 def: F<"build-id">;
 def: F<"disable-auto-image-base">;
 def: F<"enable-auto-image-base">;
 def: F<"end-group">;
 def: Flag<["--"], "full-shutdown">;
 def: F<"high-entropy-va">;
 def: S<"major-image-version">;
 def: S<"minor-image-version">;
-def: F<"no-seh">;
 def: F<"nxcompat">;
 def: F<"pic-executable">;
 def: S<"plugin">;
 def: J<"plugin=">;
 def: S<"plugin-opt">;
 def: J<"plugin-opt=">;
 def: J<"sysroot">;
 def: F<"start-group">;
 def: F<"tsaware">;

lld/test/COFF/noseh.s

+# REQUIRES: x86
+# RUN: llvm-mc -triple i686-w64-mingw32 %s -filetype=obj -o %t.obj
+# RUN: lld-link -lldmingw %t.obj -out:%t.exe -entry:main
+# RUN: llvm-readobj --file-headers %t.exe | FileCheck %s --check-prefix=DEFAULT
+# RUN: lld-link -lldmingw %t.obj -out:%t.noseh.exe -entry:main -noseh
+# RUN: llvm-readobj --file-headers %t.noseh.exe | FileCheck %s --check-prefix=NOSEH
+
+# DEFAULT: Characteristics [
+# DEFAULT-NOT:   IMAGE_DLL_CHARACTERISTICS_NO_SEH
+# DEFAULT: ]
+
+# NOSEH: Characteristics [
+# NOSEH:   IMAGE_DLL_CHARACTERISTICS_NO_SEH
+# NOSEH: ]
+
+        .text
+        .globl  _main
+_main:
+        ret

lld/test/MinGW/driver.test

 RUN: ld.lld -### -m i386pep foo.o -file-alignment=0x1000 | FileCheck -check-prefix FILEALIGN %s
 FILEALIGN: -filealign:0x1000
 
 RUN: ld.lld -### -m i386pep foo.o --section-alignment 0x2000 | FileCheck -check-prefix ALIGN %s
 RUN: ld.lld -### -m i386pep foo.o -section-alignment 0x2000 | FileCheck -check-prefix ALIGN %s
 RUN: ld.lld -### -m i386pep foo.o --section-alignment=0x2000 | FileCheck -check-prefix ALIGN %s
 RUN: ld.lld -### -m i386pep foo.o -section-alignment=0x2000 | FileCheck -check-prefix ALIGN %s
 ALIGN: -align:0x2000
+
+RUN: ld.lld -### -m i386pe foo.o -no-seh | FileCheck -check-prefix NOSEH %s
+RUN: ld.lld -### -m i386pe foo.o --no-seh | FileCheck -check-prefix NOSEH %s
+NOSEH: -noseh