[COFF] Set the IMAGE_DLL_CHARACTERISTICS_NO_SEH flag automatically

[mstorsjo]

Summary

This (at least roughly?) seems to match how link.exe sets it.

[rnk]

Does link.exe set this flag if there are no handlers in .sxdata? This flag seems like it was added as a way to mark DLLs that don't have _load_config_used as not having any exception handlers for /safeseh. Maybe we should just set this flag implicitly if there were no handlers in .sxdata and _load_config_used never resolved.

[mstorsjo]

In D41252#955735, @rnk wrote:

Does link.exe set this flag if there are no handlers in .sxdata?

It doesn't seem so - a plain C hello world exe won't have it set.

This flag seems like it was added as a way to mark DLLs that don't have _load_config_used as not having any exception handlers for /safeseh. Maybe we should just set this flag implicitly if there were no handlers in .sxdata and _load_config_used never resolved.

If we choose to do that, we'd need to make this a no-op flag in the mingw driver instead - I've just run across some code that seems to set this flag (for unknown reasons).

[rnk]

In D41252#955741, @mstorsjo wrote:

In D41252#955735, @rnk wrote:

Does link.exe set this flag if there are no handlers in .sxdata?

It doesn't seem so - a plain C hello world exe won't have it set.

Well, it will link the CRT, which has _except_handler4. You would need to make a no CRT test case and check that. It's probably simplest with a DLL with a custom entry point. You can also use llvm-readobj -coff-load-config to see that it doesn't have a load config directory.

This flag seems like it was added as a way to mark DLLs that don't have _load_config_used as not having any exception handlers for /safeseh. Maybe we should just set this flag implicitly if there were no handlers in .sxdata and _load_config_used never resolved.

If we choose to do that, we'd need to make this a no-op flag in the mingw driver instead - I've just run across some code that seems to set this flag (for unknown reasons).

Makes sense.

[mstorsjo]

In D41252#955754, @rnk wrote:

In D41252#955741, @mstorsjo wrote:

In D41252#955735, @rnk wrote:

Does link.exe set this flag if there are no handlers in .sxdata?

It doesn't seem so - a plain C hello world exe won't have it set.

Well, it will link the CRT, which has _except_handler4. You would need to make a no CRT test case and check that. It's probably simplest with a DLL with a custom entry point. You can also use llvm-readobj -coff-load-config to see that it doesn't have a load config directory.

Oh, indeed. Yes, with such a DLL, I get the flag set automatically.

[mstorsjo]

In D41252#955735, @rnk wrote:

Does link.exe set this flag if there are no handlers in .sxdata? This flag seems like it was added as a way to mark DLLs that don't have _load_config_used as not having any exception handlers for /safeseh. Maybe we should just set this flag implicitly if there were no handlers in .sxdata and _load_config_used never resolved.

So I guess this would make sense and match link.exe. Do you happen to know how this relates to SEH based exception handling in (GCC based) MinGW btw? When I build an executable (or DLL) with mingw-gcc that has SEH exceptions, llvm-readobj -coff-load-config doesn't show anything at all for such a binary. So if we were to link such binaries with LLD (which I don't do yet because libcxxabi lacks support for SEH), wouldn't we end up setting this flag for them as well?

[mstorsjo]

Split out the MinGW part, made it set the flag automatically based on Reid's suggestion.

[rnk]

In D41252#955769, @mstorsjo wrote:

In D41252#955735, @rnk wrote:

Does link.exe set this flag if there are no handlers in .sxdata? This flag seems like it was added as a way to mark DLLs that don't have _load_config_used as not having any exception handlers for /safeseh. Maybe we should just set this flag implicitly if there were no handlers in .sxdata and _load_config_used never resolved.

So I guess this would make sense and match link.exe. Do you happen to know how this relates to SEH based exception handling in (GCC based) MinGW btw? When I build an executable (or DLL) with mingw-gcc that has SEH exceptions, llvm-readobj -coff-load-config doesn't show anything at all for such a binary. So if we were to link such binaries with LLD (which I don't do yet because libcxxabi lacks support for SEH), wouldn't we end up setting this flag for them as well?

I suspect that this flag is only meaningful for x86 images, and we should only ever set it for x86 images that have no exception handlers and no load config. GCC's SEH support is x64-only, so this should never interfere with that.

[mstorsjo]

Only set the flag on x86.

[rnk]

In D41252#957037, @mstorsjo wrote:

Only set the flag on x86.

Can you confirm that this is consistent with MSVC? If so, looks good!

COFF/Writer.cpp

692 ↗

(On Diff #127166)

nit: I'd test the Machine first to short-circuit out of the symbol lookup for x64.

[mstorsjo]

In D41252#957049, @rnk wrote:

In D41252#957037, @mstorsjo wrote:

Only set the flag on x86.

Can you confirm that this is consistent with MSVC? If so, looks good!

Yes, confirmed that MSVC only sets it on x86.

COFF/Writer.cpp

692 ↗

(On Diff #127166)

Will do before pushing.

lld/trunk/COFF/Writer.cpp

template <typename PEHeaderTy> void Writer::writeHeader() {
   if (Config->HighEntropyVA)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA;
   if (!Config->AllowBind)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NO_BIND;
   if (Config->NxCompat)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NX_COMPAT;
   if (!Config->AllowIsolation)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NO_ISOLATION;
+  if (Config->Machine == I386 && !SEHTable &&
+      !Symtab->findUnderscore("_load_config_used"))
+    PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NO_SEH;
   if (Config->TerminalServerAware)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE;
   PE->NumberOfRvaAndSize = NumberfOfDataDirectory;
   if (OutputSection *Text = findSection(".text")) {
     PE->BaseOfCode = Text->getRVA();
     PE->SizeOfCode = Text->getRawSize();
   }
   PE->SizeOfInitializedData = getSizeOfInitializedData();

lld/trunk/test/COFF/hello32.test

 HEADER-NEXT:   MinorOperatingSystemVersion: 0
 HEADER-NEXT:   MajorImageVersion: 0
 HEADER-NEXT:   MinorImageVersion: 0
 HEADER-NEXT:   MajorSubsystemVersion: 6
 HEADER-NEXT:   MinorSubsystemVersion: 0
 HEADER-NEXT:   SizeOfImage: 20480
 HEADER-NEXT:   SizeOfHeaders: 512
 HEADER-NEXT:   Subsystem: IMAGE_SUBSYSTEM_WINDOWS_CUI (0x3)
-HEADER-NEXT:   Characteristics [ (0x9140)
+HEADER-NEXT:   Characteristics [ (0x9540)
 HEADER-NEXT:     IMAGE_DLL_CHARACTERISTICS_APPCONTAINER (0x1000)
 HEADER-NEXT:     IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE (0x40)
+HEADER-NEXT:     IMAGE_DLL_CHARACTERISTICS_NO_SEH (0x400)
 HEADER-NEXT:     IMAGE_DLL_CHARACTERISTICS_NX_COMPAT (0x100)
 HEADER-NEXT:     IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE (0x8000)
 HEADER-NEXT:   ]
 HEADER-NEXT:   SizeOfStackReserve: 1048576
 HEADER-NEXT:   SizeOfStackCommit: 4096
 HEADER-NEXT:   SizeOfHeapReserve: 1048576
 HEADER-NEXT:   SizeOfHeapCommit: 4096
 HEADER-NEXT:   NumberOfRvaAndSize: 16

lld/trunk/test/COFF/safeseh.s

 # RUN: llvm-mc -triple i686-windows-msvc %s -filetype=obj -o %t.obj
 # RUN: lld-link %t.obj -safeseh -out:%t.exe -opt:noref -entry:main
-# RUN: llvm-readobj -coff-basereloc -coff-load-config %t.exe | FileCheck %s --check-prefix=CHECK-NOGC
+# RUN: llvm-readobj -coff-basereloc -coff-load-config -file-headers %t.exe | FileCheck %s --check-prefix=CHECK-NOGC
 # RUN: lld-link %t.obj -safeseh -out:%t.exe -opt:ref -entry:main
-# RUN: llvm-readobj -coff-basereloc -coff-load-config %t.exe | FileCheck %s --check-prefix=CHECK-GC
+# RUN: llvm-readobj -coff-basereloc -coff-load-config -file-headers %t.exe | FileCheck %s --check-prefix=CHECK-GC
 
 # __safe_se_handler_table needs to be relocated against ImageBase.
 # check that the relocation is present.
+# CHECK-NOGC-NOT: IMAGE_DLL_CHARACTERISTICS_NO_SEH
 # CHECK-NOGC: BaseReloc [
 # CHECK-NOGC:   Entry {
 # CHECK-NOGC:     Type: HIGHLOW
 # CHECK-NOGC: LoadConfig [
 # CHECK-NOGC:   Size: 0x48
 # CHECK-NOGC:   SEHandlerTable: 0x401048
 # CHECK-NOGC:   SEHandlerCount: 1
 # CHECK-NOGC: ]
 # CHECK-NOGC: SEHTable [
 # CHECK-NOGC-NEXT:   0x402006
 # CHECK-NOGC-NEXT: ]
 
 # Without the SEH table, the address is absolute, so check that we do
 # not have a relocation for it.
+# CHECK-GC-NOT: IMAGE_DLL_CHARACTERISTICS_NO_SEH
 # CHECK-GC: BaseReloc [
 # CHECK-GC-NEXT: ]
 # CHECK-GC: LoadConfig [
 # CHECK-GC:   Size: 0x48
 # CHECK-GC:   SEHandlerTable: 0x0
 # CHECK-GC:   SEHandlerCount: 0
 # CHECK-GC: ]
 # CHECK-GC-NOT: SEHTable