[lld] r347623 - [COFF] Add exported functions to gfids table for /guard:cf

[Reid Kleckner via llvm-commits]

Author: rnk
Date: Mon Nov 26 17:50:17 2018
New Revision: 347623

URL: http://llvm.org/viewvc/llvm-project?rev=347623&view=rev
Log:
[COFF] Add exported functions to gfids table for /guard:cf

Summary:
MSVC does this, and we should to.

The .gfids table is a table of RVAs, so it's impossible for a DLL to
indicate that an imported symbol is address taken. Therefore, exports
appear to be listed as address taken by the DLL that exports them.

This fixes an issue that Firefox ran into here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1485016#c12

In Firefox, the export directive came from a .def file, but we need to
do this for any kind of export.

Reviewers: dmajor, hans, amccarth, alex

Subscribers: llvm-commits

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

Added:
    lld/trunk/test/COFF/gfids-export.s
Modified:
    lld/trunk/COFF/Writer.cpp

Modified: lld/trunk/COFF/Writer.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Writer.cpp?rev=347623&r1=347622&r2=347623&view=diff
==============================================================================
--- lld/trunk/COFF/Writer.cpp (original)
+++ lld/trunk/COFF/Writer.cpp Mon Nov 26 17:50:17 2018
@@ -1304,6 +1304,25 @@ static void addSymbolToRVASet(SymbolRVAS
   RVASet.insert({C, Off});
 }
 
+// Given a symbol, add it to the GFIDs table if it is a live, defined, function
+// symbol in an executable section.
+static void maybeAddAddressTakenFunction(SymbolRVASet &AddressTakenSyms,
+                                         Symbol *S) {
+  auto *D = dyn_cast_or_null<DefinedCOFF>(S);
+
+  // Ignore undefined symbols and references to non-functions (e.g. globals and
+  // labels).
+  if (!D ||
+      D->getCOFFSymbol().getComplexType() != COFF::IMAGE_SYM_DTYPE_FUNCTION)
+    return;
+
+  // Mark the symbol as address taken if it's in an executable section.
+  Chunk *RefChunk = D->getChunk();
+  OutputSection *OS = RefChunk ? RefChunk->getOutputSection() : nullptr;
+  if (OS && OS->Header.Characteristics & IMAGE_SCN_MEM_EXECUTE)
+    addSymbolToRVASet(AddressTakenSyms, D);
+}
+
 // Visit all relocations from all section contributions of this object file and
 // mark the relocation target as address-taken.
 static void markSymbolsWithRelocations(ObjFile *File,
@@ -1322,17 +1341,7 @@ static void markSymbolsWithRelocations(O
         continue;
 
       Symbol *Ref = SC->File->getSymbol(Reloc.SymbolTableIndex);
-      if (auto *D = dyn_cast_or_null<DefinedCOFF>(Ref)) {
-        if (D->getCOFFSymbol().getComplexType() != COFF::IMAGE_SYM_DTYPE_FUNCTION)
-          // Ignore relocations against non-functions (e.g. labels).
-          continue;
-
-        // Mark the symbol if it's in an executable section.
-        Chunk *RefChunk = D->getChunk();
-        OutputSection *OS = RefChunk ? RefChunk->getOutputSection() : nullptr;
-        if (OS && OS->Header.Characteristics & IMAGE_SCN_MEM_EXECUTE)
-          addSymbolToRVASet(UsedSymbols, D);
-      }
+      maybeAddAddressTakenFunction(UsedSymbols, Ref);
     }
   }
 }
@@ -1359,7 +1368,11 @@ void Writer::createGuardCFTables() {
 
   // Mark the image entry as address-taken.
   if (Config->Entry)
-    addSymbolToRVASet(AddressTakenSyms, cast<Defined>(Config->Entry));
+    maybeAddAddressTakenFunction(AddressTakenSyms, Config->Entry);
+
+  // Mark exported symbols in executable sections as address-taken.
+  for (Export &E : Config->Exports)
+    maybeAddAddressTakenFunction(AddressTakenSyms, E.Sym);
 
   // Ensure sections referenced in the gfid table are 16-byte aligned.
   for (const ChunkAndOffset &C : AddressTakenSyms)

Added: lld/trunk/test/COFF/gfids-export.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/gfids-export.s?rev=347623&view=auto
==============================================================================
--- lld/trunk/test/COFF/gfids-export.s (added)
+++ lld/trunk/test/COFF/gfids-export.s Mon Nov 26 17:50:17 2018
@@ -0,0 +1,44 @@
+# REQUIRES: x86
+# RUN: llvm-mc -triple x86_64-pc-win32 %s -filetype=obj -o %t.obj
+# RUN: lld-link %t.obj -guard:cf -dll -out:%t.dll -noentry
+# RUN: llvm-readobj -coff-load-config %t.dll | FileCheck %s --check-prefix=CHECK
+
+# There should be a single entry in the table for the exported symbol.
+#
+# CHECK: GuardFidTable [
+# CHECK-NEXT: 0x180001000
+# CHECK-NEXT: ]
+
+        .def    func_export; .scl    2; .type   32; .endef
+        .globl func_export
+        .section       .text,"xr",one_only,func_export
+        .p2align 4
+func_export:
+        movl $1, %eax
+        .globl label_export
+label_export:
+        movl $2, %eax
+        ret
+
+        .data
+        .globl data_export
+data_export:
+        .long 42
+
+        .section .drectve,"dr"
+        .ascii " /EXPORT:func_export"
+        .ascii " /EXPORT:label_export"
+        .ascii " /EXPORT:data_export"
+
+
+# Load configuration directory entry (winnt.h _IMAGE_LOAD_CONFIG_DIRECTORY64).
+# The linker will define the __guard_* symbols.
+        .section .rdata,"dr"
+.globl _load_config_used
+_load_config_used:
+        .long 256
+        .fill 124, 1, 0
+        .quad __guard_fids_table
+        .quad __guard_fids_count
+        .long __guard_flags
+        .fill 128, 1, 0