From 8f1a28f1902806077c95ff8292192e8d6d3fa821 Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk@google.com>
Date: Wed, 18 Apr 2018 22:37:10 +0000
Subject: [PATCH] [COFF] Mark images with no exception handlers for /safeseh

Summary:
DLLs and executables with no exception handlers need to be marked with
IMAGE_DLL_CHARACTERISTICS_NO_SEH, even if they have a load config.

Discovered here when building Chromium with LLD on Windows:
https://crbug.com/833951

Reviewers: ruiu, mstorsjo

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D45778

llvm-svn: 330300
---
 lld/COFF/Writer.cpp             | 16 +++++++++++----
 lld/test/COFF/safeseh-notable.s | 43 +++++++++++++++++++++++++++++++++++++++++
 lld/test/COFF/safeseh.s         | 12 +++++++++---
 3 files changed, 64 insertions(+), 7 deletions(-)
 create mode 100644 lld/test/COFF/safeseh-notable.s

diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index de64022..13356a9 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -185,8 +185,7 @@ private:
   IdataContents Idata;
   DelayLoadContents DelayIdata;
   EdataContents Edata;
-  RVATableChunk *GuardFidsTable = nullptr;
-  RVATableChunk *SEHTable = nullptr;
+  bool SetNoSEHCharacteristic = false;
 
   DebugDirectoryChunk *DebugDirectory = nullptr;
   std::vector<Chunk *> DebugRecords;
@@ -828,8 +827,7 @@ template <typename PEHeaderTy> void Writer::writeHeader() {
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NO_ISOLATION;
   if (Config->GuardCF != GuardCFLevel::Off)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_GUARD_CF;
-  if (Config->Machine == I386 && !SEHTable &&
-      !Symtab->findUnderscore("_load_config_used"))
+  if (SetNoSEHCharacteristic)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_NO_SEH;
   if (Config->TerminalServerAware)
     PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE;
@@ -934,6 +932,10 @@ void Writer::openFile(StringRef Path) {
 }
 
 void Writer::createSEHTable() {
+  // Set the no SEH characteristic on x86 binaries unless we find exception
+  // handlers.
+  SetNoSEHCharacteristic = true;
+
   SymbolRVASet Handlers;
   for (ObjFile *File : ObjFile::Instances) {
     // FIXME: We should error here instead of earlier unless /safeseh:no was
@@ -944,6 +946,12 @@ void Writer::createSEHTable() {
     markSymbolsForRVATable(File, File->getSXDataChunks(), Handlers);
   }
 
+  // Remove the "no SEH" characteristic if all object files were built with
+  // safeseh, we found some exception handlers, and there is a load config in
+  // the object.
+  SetNoSEHCharacteristic =
+      Handlers.empty() || !Symtab->findUnderscore("_load_config_used");
+
   maybeAddRVATable(std::move(Handlers), "__safe_se_handler_table",
                    "__safe_se_handler_count");
 }
diff --git a/lld/test/COFF/safeseh-notable.s b/lld/test/COFF/safeseh-notable.s
new file mode 100644
index 0000000..1f00cb2
--- /dev/null
+++ b/lld/test/COFF/safeseh-notable.s
@@ -0,0 +1,43 @@
+# RUN: llvm-mc -triple i686-windows-msvc %s -filetype=obj -o %t.obj
+# RUN: lld-link %t.obj -safeseh -out:%t.exe -entry:main
+# RUN: llvm-readobj -file-headers %t.exe | FileCheck %s
+
+# This object lacks a _load_config_used global, so we set
+# IMAGE_DLL_CHARACTERISTICS_NO_SEH even though there is an exception handler.
+# This is a more secure default. If someone wants to use a CRT without a load
+# config and they want to use 32-bit SEH, they will need to provide a
+# safeseh-compatible load config.
+
+# CHECK-LABEL: Characteristics [
+# CHECK:   IMAGE_DLL_CHARACTERISTICS_NO_SEH
+# CHECK: ]
+
+# CHECK-LABEL:  DataDirectory {
+# CHECK:    LoadConfigTableRVA: 0x0
+# CHECK:    LoadConfigTableSize: 0x0
+# CHECK:  }
+
+# CHECK-NOT: LoadConfig
+# CHECK-NOT: SEHTable
+
+        .def     @feat.00;
+        .scl    3;
+        .type   0;
+        .endef
+        .globl  @feat.00
+@feat.00 = 1
+
+        .text
+        .def     _main; .scl    2; .type   32; .endef
+        .globl  _main
+_main:
+        pushl $_my_handler
+        movl $42, %eax
+        popl %ecx
+        ret
+
+        .def     _my_handler; .scl    3; .type   32; .endef
+_my_handler:
+        ret
+
+.safeseh _my_handler
diff --git a/lld/test/COFF/safeseh.s b/lld/test/COFF/safeseh.s
index 77a7cd3..4860122 100644
--- a/lld/test/COFF/safeseh.s
+++ b/lld/test/COFF/safeseh.s
@@ -6,6 +6,7 @@
 
 # __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 {
@@ -19,9 +20,14 @@
 # CHECK-NOGC-NEXT:   0x401006
 # 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
+# If we enable GC, the exception handler should be removed, and we should add
+# the DLL characteristic flag that indicates that there are no exception
+# handlers in this DLL. The exception handler table in the load config should
+# be empty and there should be no relocations for it.
+#
+# CHECK-GC: Characteristics [
+# CHECK-GC:   IMAGE_DLL_CHARACTERISTICS_NO_SEH
+# CHECK-GC: ]
 # CHECK-GC: BaseReloc [
 # CHECK-GC-NEXT: ]
 # CHECK-GC: LoadConfig [
-- 
2.7.4