[COFF] Add minimal support for /guard:cf

[rnk]

Summary

This patch adds some initial support for Windows control flow guard. At
the end of the day, the linker needs to synthesize a table of RVAs very
similar to the structured exception handler table (/safeseh).

Both /safeseh and /guard:cf take sections of symbol table indices
(.sxdata and .gfids$y) and turn them into RVA tables referenced by the
load config struct in the CRT through special symbols.

[pcc]

lld/COFF/InputFiles.h

191 ↗

(On Diff #131631)

Can there really be more than one .gfids$y section? I don't see why that would be needed.

[pcc]

lld/COFF/InputFiles.h

191 ↗

(On Diff #131631)

Hmm, never mind, I see that we can get multiple sections like this:

void a(), b();
__declspec(selectany) void* f = a;
__declspec(selectany) void* g = b;

[ruiu]

lld/COFF/Chunks.cpp

459 ↗

(On Diff #131631)

Please avoid auto even if writing a real type is a bit too lengthy. If you want to use auto, please do something like this so that a real type is explicit in a local context.

for (const auto &P : Syms) {
  Defined *Sym = P.first;
  ....
}

lld/COFF/DriverUtils.cpp

132–136 ↗

(On Diff #131631)

You can use StringRef::equals_lower.

lld/COFF/InputFiles.h

104–109 ↗

(On Diff #131631)

Since these enums are used only by ObjFile, I'd move this into that class (and make them a non-enum class.)

142–149 ↗

(On Diff #131631)

Actually, I'd even use 1 and 0x800 instead of defining them as constants, as they are used only once and already factored out well.

lld/COFF/Writer.cpp

874 ↗

(On Diff #131631)

Can you write a function comment to give a big picture? Once it becomes clear as to what this function should create, the details becomes obvious.

[amccarth]

Look nice! A couple nits.

lld/COFF/Chunks.h

326 ↗

(On Diff #131631)

Using a std::pair rather than a struct with named fields makes the usage of these (in the .cpp file) a bit harder to follow.

lld/test/COFF/gfids-gc.s

60 ↗

(On Diff #131631)

This just checks that there are at least two entries in the GuardFidTable, but not whether there're exactly two. (You'd need CHECK-GC-NEXT, right?) Was that intentional?

I supposed checking that they're are the _right_ two is a bit of overkill.

lld/test/COFF/gfids-icf.s

6 ↗

(On Diff #131631)

The other test was a little easier to understand because it showed the approximate source code. If that's infeasible here, could you extend the the comment to say something like, "Identical comdat folding will eliminate icf2 because it's the same as icf1."

(I'm not even sure that's exactly true. Is there a guarantee about _which_ copy will be eliminated or just that one of them will?)

[rnk]

lld/COFF/Chunks.h

326 ↗

(On Diff #131631)

I implemented this, but it meant I had to implement DenseMapInfo type traits, which uglified things a fair amount. I think this is OK, but what do you think?

lld/COFF/InputFiles.h

191 ↗

(On Diff #131631)

Yep.

lld/COFF/Writer.cpp

874 ↗

(On Diff #131631)

I did this.

One other consideration here is that this is Yet Another loop over all input object files. If this work shows up in profiling, we should consider deleting this set and making it a bit on Symbol just like WrittenToSymtab.

lld/test/COFF/gfids-gc.s

60 ↗

(On Diff #131631)

There's a GuardCFFunctionCount: 2 line above, but -NEXT here is probably also appropriate.

lld/test/COFF/gfids-icf.s

6 ↗

(On Diff #131631)

This should be clearer!

[rnk]

• address feedback

[amccarth]

LGTM.

[ruiu]

Generally looking pretty good. Minor comments.

lld/COFF/Writer.cpp

874 ↗

(On Diff #131631)

I'm not too worried about a loop that loops over input files because the number of input files isn't that big. Of course your mileage may vary -- if each iteration of the loop is heavy, it would appear in the profiler. Let's see how it works.

868 ↗

(On Diff #132911)

Can you write a function comment for this function so that readers can get a big picture?

924 ↗

(On Diff #132911)

Ditto

938 ↗

(On Diff #132911)

Likewise, toString(C) is probably better.

940 ↗

(On Diff #132911)

toString(File) is better because it generates a user-friendly string even for object files in archive files.

949 ↗

(On Diff #132911)

ulittle32_t is a small object so I wouldn't use it through a reference. I'd just use uint32_t here.

[rnk]

lld/COFF/Writer.cpp

938 ↗

(On Diff #132911)

As discussed, this overload doesn't exist

[rnk]

• address feedback

[ruiu]

LGTM

lld/COFF/Writer.cpp

949 ↗

(On Diff #132911)

Is this done?

[rnk]

Thanks!

lld/COFF/Writer.cpp

949 ↗

(On Diff #132911)

Yes, I think I forgot to quit the editor prompt when I uploaded the next patch.

COFF/Chunks.h

public:
   size_t getSize() const override;
   void getBaserels(std::vector<Baserel> *Res) override;
   void writeTo(uint8_t *Buf) const override;
 
 private:
   Defined *Sym;
 };
 
-// Windows-specific.
-// A chunk for SEH table which contains RVAs of safe exception handler
-// functions. x86-only.
-class SEHTableChunk : public Chunk {
+// Duplicate RVAs are not allowed in RVA tables, so unique symbols by chunk and
+// offset into the chunk. Order does not matter as the RVA table will be sorted
+// later.
+struct ChunkAndOffset {
+  Chunk *InputChunk;
+  uint32_t Offset;
+
+  struct DenseMapInfo {
+    static ChunkAndOffset getEmptyKey() {
+      return {llvm::DenseMapInfo<Chunk *>::getEmptyKey(), 0};
+    }
+    static ChunkAndOffset getTombstoneKey() {
+      return {llvm::DenseMapInfo<Chunk *>::getTombstoneKey(), 0};
+    }
+    static unsigned getHashValue(const ChunkAndOffset &CO) {
+      return llvm::DenseMapInfo<std::pair<Chunk *, uint32_t>>::getHashValue(
+          {CO.InputChunk, CO.Offset});
+    }
+    static bool isEqual(const ChunkAndOffset &LHS, const ChunkAndOffset &RHS) {
+      return LHS.InputChunk == RHS.InputChunk && LHS.Offset == RHS.Offset;
+    }
+  };
+};
+
+using SymbolRVASet = llvm::DenseSet<ChunkAndOffset>;
+
+// Table which contains symbol RVAs. Used for /safeseh and /guard:cf.
+class RVATableChunk : public Chunk {
 public:
-  explicit SEHTableChunk(std::set<Defined *> S) : Syms(std::move(S)) {}
+  explicit RVATableChunk(SymbolRVASet S) : Syms(std::move(S)) {}
   size_t getSize() const override { return Syms.size() * 4; }
   void writeTo(uint8_t *Buf) const override;
 
 private:
-  std::set<Defined *> Syms;
+  SymbolRVASet Syms;
 };
 
 // Windows-specific.
 // This class represents a block in .reloc section.
 // See the PE/COFF spec 5.6 for details.
 class BaserelChunk : public Chunk {
 public:
   BaserelChunk(uint32_t Page, Baserel *Begin, Baserel *End);

 };
 
 void applyMOV32T(uint8_t *Off, uint32_t V);
 void applyBranch24T(uint8_t *Off, int32_t V);
 
 } // namespace coff
 } // namespace lld
 
+namespace llvm {
+template <>
+struct DenseMapInfo<lld::coff::ChunkAndOffset>
+    : lld::coff::ChunkAndOffset::DenseMapInfo {};
+}
+
 #endif

COFF/Chunks.cpp

 void LocalImportChunk::writeTo(uint8_t *Buf) const {
   if (Config->is64()) {
     write64le(Buf + OutputSectionOff, Sym->getRVA() + Config->ImageBase);
   } else {
     write32le(Buf + OutputSectionOff, Sym->getRVA() + Config->ImageBase);
   }
 }
 
-void SEHTableChunk::writeTo(uint8_t *Buf) const {
+void RVATableChunk::writeTo(uint8_t *Buf) const {
   ulittle32_t *Begin = reinterpret_cast<ulittle32_t *>(Buf + OutputSectionOff);
   size_t Cnt = 0;
-  for (Defined *D : Syms)
-    Begin[Cnt++] = D->getRVA();
+  for (const ChunkAndOffset &CO : Syms)
+    Begin[Cnt++] = CO.InputChunk->getRVA() + CO.Offset;
   std::sort(Begin, Begin + Cnt);
+  assert(std::unique(Begin, Begin + Cnt) == Begin + Cnt &&
+         "RVA tables should be de-duplicated");
 }
 
 // Windows-specific. This class represents a block in .reloc section.
 // The format is described here.
 //
 // On Windows, each DLL is linked against a fixed base address and
 // usually loaded to that address. However, if there's already another
 // DLL that overlaps, the loader has to relocate it. To do that, DLLs

COFF/Config.h

struct Configuration {
   StringRef Implib;
   std::vector<Export> Exports;
   std::set<std::string> DelayLoads;
   std::map<std::string, int> DLLOrder;
   Symbol *DelayLoadHelper = nullptr;
 
   bool SaveTemps = false;
 
+  // /guard:cf
+  bool GuardCF;
+
   // Used for SafeSEH.
   Symbol *SEHTable = nullptr;
   Symbol *SEHCount = nullptr;
 
   // Used for /opt:lldlto=N
   unsigned LTOOptLevel = 2;
 
   // Used for /opt:lldltojobs=N

COFF/Driver.h

 
 // For /machine option.
 MachineTypes getMachineType(StringRef Arg);
 StringRef machineToStr(MachineTypes MT);
 
 // Parses a string in the form of "<integer>[,<integer>]".
 void parseNumbers(StringRef Arg, uint64_t *Addr, uint64_t *Size = nullptr);
 
+void parseGuard(StringRef Arg);
+
 // Parses a string in the form of "<integer>[.<integer>]".
 // Minor's default value is 0.
 void parseVersion(StringRef Arg, uint32_t *Major, uint32_t *Minor);
 
 // Parses a string in the form of "<subsystem>[,<integer>[.<integer>]]".
 void parseSubsystem(StringRef Arg, WindowsSubsystem *Sys, uint32_t *Major,
                     uint32_t *Minor);
 

COFF/Driver.cpp

void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   // Handle /base
   if (auto *Arg = Args.getLastArg(OPT_base))
     parseNumbers(Arg->getValue(), &Config->ImageBase);
 
   // Handle /stack
   if (auto *Arg = Args.getLastArg(OPT_stack))
     parseNumbers(Arg->getValue(), &Config->StackReserve, &Config->StackCommit);
 
+  // Handle /guard:cf
+  if (auto *Arg = Args.getLastArg(OPT_guard))
+    parseGuard(Arg->getValue());
+
   // Handle /heap
   if (auto *Arg = Args.getLastArg(OPT_heap))
     parseNumbers(Arg->getValue(), &Config->HeapReserve, &Config->HeapCommit);
 
   // Handle /version
   if (auto *Arg = Args.getLastArg(OPT_version))
     parseVersion(Arg->getValue(), &Config->MajorImageVersion,
                  &Config->MinorImageVersion);
if (Config->ImageBase == uint64_t(-1))
     Config->ImageBase = getDefaultImageBase();
 
   Symtab->addSynthetic(mangle("__ImageBase"), nullptr);
   if (Config->Machine == I386) {
     Symtab->addAbsolute("___safe_se_handler_table", 0);
     Symtab->addAbsolute("___safe_se_handler_count", 0);
   }
 
-  // We do not support /guard:cf (control flow protection) yet.
-  // Define CFG symbols anyway so that we can link MSVC 2015 CRT.
   Symtab->addAbsolute(mangle("__guard_fids_count"), 0);
   Symtab->addAbsolute(mangle("__guard_fids_table"), 0);
-  Symtab->addAbsolute(mangle("__guard_flags"), 0x100);
+  Symtab->addAbsolute(mangle("__guard_flags"), 0);
   Symtab->addAbsolute(mangle("__guard_iat_count"), 0);
   Symtab->addAbsolute(mangle("__guard_iat_table"), 0);
   Symtab->addAbsolute(mangle("__guard_longjmp_count"), 0);
   Symtab->addAbsolute(mangle("__guard_longjmp_table"), 0);
   // Needed for MSVC 2017 15.5 CRT.
   Symtab->addAbsolute(mangle("__enclave_config"), 0);
 
   // This code may add new undefined symbols to the link, which may enqueue more
if (Config->Subsystem == IMAGE_SUBSYSTEM_UNKNOWN) {
     Config->Subsystem = inferSubsystem();
     if (Config->Subsystem == IMAGE_SUBSYSTEM_UNKNOWN)
       fatal("subsystem must be defined");
   }
 
   // Handle /safeseh.
   if (Args.hasFlag(OPT_safeseh, OPT_safeseh_no, false)) {
     for (ObjFile *File : ObjFile::Instances)
-      if (!File->SEHCompat)
+      if (!File->hasSafeSEH())
         error("/safeseh: " + File->getName() + " is not compatible with SEH");
     if (errorCount())
       return;
   }
 
   // In MinGW, all symbols are automatically exported if no symbols
   // are chosen to be exported.
   if (Config->DLL && ((Config->MinGW && Config->Exports.empty()) ||

COFF/DriverUtils.cpp

void parseVersion(StringRef Arg, uint32_t *Major, uint32_t *Minor) {
   std::tie(S1, S2) = Arg.split('.');
   if (S1.getAsInteger(0, *Major))
     fatal("invalid number: " + S1);
   *Minor = 0;
   if (!S2.empty() && S2.getAsInteger(0, *Minor))
     fatal("invalid number: " + S2);
 }
 
+void parseGuard(StringRef Arg) {
+  if (Arg.equals_lower("no"))
+    Config->GuardCF = false;
+  else if (Arg.equals_lower("cf"))
+    Config->GuardCF = true;
+  else
+    fatal("invalid argument to /GUARD: " + Arg);
+}
+
 // Parses a string in the form of "<subsystem>[,<integer>[.<integer>]]".
 void parseSubsystem(StringRef Arg, WindowsSubsystem *Sys, uint32_t *Major,
                     uint32_t *Minor) {
   StringRef SysStr, Ver;
   std::tie(SysStr, Ver) = Arg.split(',');
   *Sys = StringSwitch<WindowsSubsystem>(SysStr.lower())
     .Case("boot_application", IMAGE_SUBSYSTEM_WINDOWS_BOOT_APPLICATION)
     .Case("console", IMAGE_SUBSYSTEM_WINDOWS_CUI)

COFF/InputFiles.h

 class ObjFile : public InputFile {
 public:
   explicit ObjFile(MemoryBufferRef M) : InputFile(ObjectKind, M) {}
   static bool classof(const InputFile *F) { return F->kind() == ObjectKind; }
   void parse() override;
   MachineTypes getMachineType() override;
   ArrayRef<Chunk *> getChunks() { return Chunks; }
   ArrayRef<SectionChunk *> getDebugChunks() { return DebugChunks; }
+  ArrayRef<SectionChunk *> getSXDataChunks() { return SXDataChunks; }
+  ArrayRef<SectionChunk *> getGuardFidChunks() { return GuardFidChunks; }
   ArrayRef<Symbol *> getSymbols() { return Symbols; }
 
   // Returns a Symbol object for the SymbolIndex'th symbol in the
   // underlying object file.
   Symbol *getSymbol(uint32_t SymbolIndex) {
     return Symbols[SymbolIndex];
   }
 
   // Returns the underying COFF file.
   COFFObjectFile *getCOFFObj() { return COFFObj.get(); }
 
   static std::vector<ObjFile *> Instances;
 
-  // True if this object file is compatible with SEH.
-  // COFF-specific and x86-only.
-  bool SEHCompat = false;
-
-  // The symbol table indexes of the safe exception handlers.
-  // COFF-specific and x86-only.
-  ArrayRef<llvm::support::ulittle32_t> SXData;
+  // Flags in the absolute @feat.00 symbol if it is present. These usually
+  // indicate if an object was compiled with certain security features enabled
+  // like stack guard, safeseh, /guard:cf, or other things.
+  uint32_t Feat00Flags = 0;
+
+  // True if this object file is compatible with SEH.  COFF-specific and
+  // x86-only. COFF spec 5.10.1. The .sxdata section.
+  bool hasSafeSEH() { return Feat00Flags & 0x1; }
+
+  // True if this file was compiled with /guard:cf.
+  bool hasGuardCF() { return Feat00Flags & 0x800; }
 
   // Pointer to the PDB module descriptor builder. Various debug info records
   // will reference object files by "module index", which is here. Things like
   // source files and section contributions are also recorded here. Will be null
   // if we are not producing a PDB.
   llvm::pdb::DbiModuleDescriptorBuilder *ModuleDBI = nullptr;
 
 private:
private:
 
   // List of all chunks defined by this file. This includes both section
   // chunks and non-section chunks for common symbols.
   std::vector<Chunk *> Chunks;
 
   // CodeView debug info sections.
   std::vector<SectionChunk *> DebugChunks;
 
+  // Chunks containing symbol table indices of exception handlers. Only used for
+  // 32-bit x86.
+  std::vector<SectionChunk *> SXDataChunks;
+
+  // Chunks containing symbol table indices of address taken symbols.  These are
+  // not linked into the final binary when /guard:cf is set.
+  std::vector<SectionChunk *> GuardFidChunks;
+
   // This vector contains the same chunks as Chunks, but they are
   // indexed such that you can get a SectionChunk by section index.
   // Nonexistent section indices are filled with null pointers.
   // (Because section number is 1-based, the first slot is always a
   // null pointer.)
   std::vector<SectionChunk *> SparseChunks;
 
   // This vector contains a list of all symbols defined or referenced by this

COFF/InputFiles.cpp

SectionChunk *ObjFile::readSection(uint32_t SectionNumber,
                                    const coff_aux_section_definition *Def) {
   const coff_section *Sec;
   StringRef Name;
   if (auto EC = COFFObj->getSection(SectionNumber, Sec))
     fatal("getSection failed: #" + Twine(SectionNumber) + ": " + EC.message());
   if (auto EC = COFFObj->getSectionName(Sec, Name))
     fatal("getSectionName failed: #" + Twine(SectionNumber) + ": " +
           EC.message());
-  if (Name == ".sxdata") {
-    ArrayRef<uint8_t> Data;
-    COFFObj->getSectionContents(Sec, Data);
-    if (Data.size() % 4 != 0)
-      fatal(".sxdata must be an array of symbol table indices");
-    SXData = {reinterpret_cast<const ulittle32_t *>(Data.data()),
-              Data.size() / 4};
-    return nullptr;
-  }
+
   if (Name == ".drectve") {
     ArrayRef<uint8_t> Data;
     COFFObj->getSectionContents(Sec, Data);
     Directives = std::string((const char *)Data.data(), Data.size());
     return nullptr;
   }
 
   // Object files may have DWARF debug info or MS CodeView debug info
SectionChunk *ObjFile::readSection(uint32_t SectionNumber,
   auto *C = make<SectionChunk>(this, Sec);
   if (Def)
     C->Checksum = Def->CheckSum;
 
   // CodeView sections are stored to a different vector because they are not
   // linked in the regular manner.
   if (C->isCodeView())
     DebugChunks.push_back(C);
+  else if (Config->GuardCF && Name == ".gfids$y")
+    GuardFidChunks.push_back(C);
+  else if (Name == ".sxdata")
+    SXDataChunks.push_back(C);
   else
     Chunks.push_back(C);
 
   return C;
 }
 
 void ObjFile::readAssociativeDefinition(
     COFFSymbolRef Sym, const coff_aux_section_definition *Def) {
Symbol *S =
         Symtab->addCommon(this, Name, Sym.getValue(), Sym.getGeneric(), C);
     return S;
   }
   if (Sym.isAbsolute()) {
     COFFObj->getSymbolName(Sym, Name);
     // Skip special symbols.
     if (Name == "@comp.id")
       return nullptr;
-    // COFF spec 5.10.1. The .sxdata section.
     if (Name == "@feat.00") {
-      if (Sym.getValue() & 1)
-        SEHCompat = true;
+      Feat00Flags = Sym.getValue();
       return nullptr;
     }
     if (Sym.isExternal())
       return Symtab->addAbsolute(Name, Sym);
     else
       return make<DefinedAbsolute>(Name, Sym);
   }
   int32_t SectionNumber = Sym.getSectionNumber();

COFF/Options.td

 def defaultlib : P<"defaultlib", "Add the library to the list of input files">;
 def delayload : P<"delayload", "Delay loaded DLL name">;
 def entry   : P<"entry", "Name of entry point symbol">;
 def errorlimit : P<"errorlimit",
     "Maximum number of errors to emit before stopping (0 = no limit)">;
 def export  : P<"export", "Export a function">;
 // No help text because /failifmismatch is not intended to be used by the user.
 def failifmismatch : P<"failifmismatch", "">;
+def guard   : P<"guard", "Control flow guard">;
 def heap    : P<"heap", "Size of the heap">;
 def ignore : P<"ignore", "Specify warning codes to ignore">;
 def implib  : P<"implib", "Import library name">;
 def libpath : P<"libpath", "Additional library search path">;
 def linkrepro : P<"linkrepro", "Dump linker invocation and input files for debugging">;
 def lldltocache : P<"lldltocache", "Path to ThinLTO cached object file directory">;
 def lldltocachepolicy : P<"lldltocachepolicy", "Pruning policy for the ThinLTO cache">;
 def lldsavetemps : F<"lldsavetemps">,

COFF/Writer.cpp

 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/Parallel.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/RandomNumberGenerator.h"
 #include <algorithm>
 #include <cstdio>
 #include <map>
 #include <memory>
 #include <utility>
 
 using namespace llvm;
private:
   void createImportTables();
   void createExportTable();
   void assignAddresses();
   void removeEmptySections();
   void createSymbolAndStringTable();
   void openFile(StringRef OutputPath);
   template <typename PEHeaderTy> void writeHeader();
   void createSEHTable(OutputSection *RData);
+  void createGFIDTable(OutputSection *RData);
+  void markSymbolsForRVATable(ObjFile *File,
+                              ArrayRef<SectionChunk *> SymIdxChunks,
+                              SymbolRVASet &TableSymbols);
+  void maybeAddRVATable(OutputSection *RData, SymbolRVASet TableSymbols,
+                        StringRef TableSym, StringRef CountSym);
   void setSectionPermissions();
   void writeSections();
   void writeBuildId();
   void sortExceptionTable();
 
   llvm::Optional<coff_symbol16> createSymbol(Defined *D);
   size_t addEntryToStringTable(StringRef Str);
 
   OutputSection *findSection(StringRef Name);
   OutputSection *createSection(StringRef Name);
   void addBaserels(OutputSection *Dest);
   void addBaserelBlocks(OutputSection *Dest, std::vector<Baserel> &V);
 
   uint32_t getSizeOfInitializedData();
   std::map<StringRef, std::vector<DefinedImportData *>> binImports();
 
   std::unique_ptr<FileOutputBuffer> &Buffer;
   std::vector<OutputSection *> OutputSections;
   std::vector<char> Strtab;
   std::vector<llvm::object::coff_symbol16> OutputSymtab;
   IdataContents Idata;
   DelayLoadContents DelayIdata;
   EdataContents Edata;
-  SEHTableChunk *SEHTable = nullptr;
+  RVATableChunk *GuardFidsTable = nullptr;
+  RVATableChunk *SEHTable = nullptr;
 
   Chunk *DebugDirectory = nullptr;
   std::vector<Chunk *> DebugRecords;
   CVDebugRecordChunk *BuildId = nullptr;
   Optional<codeview::DebugInfo> PreviousBuildId;
   ArrayRef<uint8_t> SectionTable;
 
   uint64_t FileSize;
if (Config->Debug) {
     BuildId = CVChunk;
     DebugRecords.push_back(CVChunk);
 
     RData->addChunk(DebugDirectory);
     for (Chunk *C : DebugRecords)
       RData->addChunk(C);
   }
 
+  // Create SEH table. x86-only.
+  if (Config->Machine == I386)
     createSEHTable(RData);
+
+  // Create the guard function id table if requested.
+  if (Config->GuardCF)
+    createGFIDTable(RData);
 }
 
 // Create .idata section for the DLL-imported symbol table.
 // The format of this section is inherently Windows-specific.
 // IdataContents class abstracted away the details for us,
 // so we just let it create chunks and add them to the section.
 void Writer::createImportTables() {
   if (ImportFile::Instances.empty())
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->GuardCF)
+    PE->DLLCharacteristics |= IMAGE_DLL_CHARACTERISTICS_GUARD_CF;
   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();

 
 void Writer::openFile(StringRef Path) {
   Buffer = CHECK(
       FileOutputBuffer::create(Path, FileSize, FileOutputBuffer::F_executable),
       "failed to open " + Path);
 }
 
 void Writer::createSEHTable(OutputSection *RData) {
-  // Create SEH table. x86-only.
-  if (Config->Machine != I386)
+  SymbolRVASet Handlers;
+  for (ObjFile *File : ObjFile::Instances) {
+    // FIXME: We should error here instead of earlier unless /safeseh:no was
+    // passed.
+    if (!File->hasSafeSEH())
       return;
 
-  std::set<Defined *> Handlers;
+    markSymbolsForRVATable(File, File->getSXDataChunks(), Handlers);
+  }
+
+  maybeAddRVATable(RData, std::move(Handlers), "__safe_se_handler_table",
+                   "__safe_se_handler_count");
+}
+
+// Add a symbol to an RVA set. Two symbols may have the same RVA, but an RVA set
+// cannot contain duplicates. Therefore, the set is uniqued by Chunk and the
+// symbol's offset into that Chunk.
+static void addSymbolToRVASet(SymbolRVASet &RVASet, Defined *S) {
+  Chunk *C = S->getChunk();
+  if (auto *SC = dyn_cast<SectionChunk>(C))
+    C = SC->Repl; // Look through ICF replacement.
+  uint32_t Off = S->getRVA() - (C ? C->getRVA() : 0);
+  RVASet.insert({C, Off});
+}
+
+// Visit all relocations from all section contributions of this object file and
+// mark the relocation target as address-taken.
+static void markSymbolsWithRelocations(ObjFile *File,
+                                       SymbolRVASet &UsedSymbols) {
+  for (Chunk *C : File->getChunks()) {
+    // We only care about live section chunks. Common chunks and other chunks
+    // don't generally contain relocations.
+    SectionChunk *SC = dyn_cast<SectionChunk>(C);
+    if (!SC || !SC->isLive())
+      continue;
+
+    // Look for relocations in this section against symbols in executable output
+    // sections.
+    for (Symbol *Ref : SC->symbols()) {
+      // FIXME: Do further testing to see if the relocation type matters,
+      // especially for 32-bit where taking the address of something usually
+      // uses an absolute relocation instead of a relative one.
+      if (auto *D = dyn_cast_or_null<Defined>(Ref)) {
+        Chunk *RefChunk = D->getChunk();
+        OutputSection *OS = RefChunk ? RefChunk->getOutputSection() : nullptr;
+        if (OS && OS->getPermissions() & IMAGE_SCN_MEM_EXECUTE)
+          addSymbolToRVASet(UsedSymbols, D);
+      }
+    }
+  }
+}
 
+// Create the guard function id table. This is a table of RVAs of all
+// address-taken functions. It is sorted and uniqued, just like the safe SEH
+// table.
+void Writer::createGFIDTable(OutputSection *RData) {
+  SymbolRVASet AddressTakenSyms;
   for (ObjFile *File : ObjFile::Instances) {
-    if (!File->SEHCompat)
-      return;
-    for (uint32_t I : File->SXData)
-      if (Symbol *B = File->getSymbol(I))
-        if (B->isLive())
-          Handlers.insert(cast<Defined>(B));
-  }
+    // If the object was compiled with /guard:cf, the address taken symbols are
+    // in the .gfids$y sections. Otherwise, we approximate the set of address
+    // taken symbols by checking which symbols were used by relocations in live
+    // sections.
+    if (File->hasGuardCF())
+      markSymbolsForRVATable(File, File->getGuardFidChunks(), AddressTakenSyms);
+    else
+      markSymbolsWithRelocations(File, AddressTakenSyms);
+  }
+
+  // Mark the image entry as address-taken.
+  if (Config->Entry)
+    addSymbolToRVASet(AddressTakenSyms, cast<Defined>(Config->Entry));
+
+  maybeAddRVATable(RData, std::move(AddressTakenSyms), "__guard_fids_table",
+                   "__guard_fids_count");
+
+  // Set __guard_flags, which will be used in the load config to indicate that
+  // /guard:cf was enabled.
+  uint32_t GuardFlags = uint32_t(coff_guard_flags::CFInstrumented) |
+                        uint32_t(coff_guard_flags::HasFidTable);
+  Symbol *FlagSym = Symtab->findUnderscore("__guard_flags");
+  cast<DefinedAbsolute>(FlagSym)->setVA(GuardFlags);
+}
+
+// Take a list of input sections containing symbol table indices and add those
+// symbols to an RVA table. The challenge is that symbol RVAs are not known and
+// depend on the table size, so we can't directly build a set of integers.
+void Writer::markSymbolsForRVATable(ObjFile *File,
+                                    ArrayRef<SectionChunk *> SymIdxChunks,
+                                    SymbolRVASet &TableSymbols) {
+  for (SectionChunk *C : SymIdxChunks) {
+    // Skip sections discarded by linker GC. This comes up when a .gfids section
+    // is associated with something like a vtable and the vtable is discarded.
+    // In this case, the associated gfids section is discarded, and we don't
+    // mark the virtual member functions as address-taken by the vtable.
+    if (!C->isLive())
+      continue;
+
+    // Validate that the contents look like symbol table indices.
+    ArrayRef<uint8_t> Data = C->getContents();
+    if (Data.size() % 4 != 0) {
+      warn("ignoring " + C->getSectionName() +
+           " symbol table index section in object " + toString(File));
+      continue;
+    }
+
+    // Read each symbol table index and check if that symbol was included in the
+    // final link. If so, add it to the table symbol set.
+    ArrayRef<ulittle32_t> SymIndices(
+        reinterpret_cast<const ulittle32_t *>(Data.data()), Data.size() / 4);
+    ArrayRef<Symbol *> ObjSymbols = File->getSymbols();
+    for (uint32_t SymIndex : SymIndices) {
+      if (SymIndex >= ObjSymbols.size()) {
+        warn("ignoring invalid symbol table index in section " +
+             C->getSectionName() + " in object " + toString(File));
+        continue;
+      }
+      if (Symbol *S = ObjSymbols[SymIndex]) {
+        if (S->isLive())
+          addSymbolToRVASet(TableSymbols, cast<Defined>(S));
+      }
+    }
+  }
 
-  if (Handlers.empty())
+// Replace the absolute table symbol with a synthetic symbol pointing to
+// TableChunk so that we can emit base relocations for it and resolve section
+// relative relocations.
+void Writer::maybeAddRVATable(OutputSection *RData,
+                              SymbolRVASet TableSymbols,
+                              StringRef TableSym, StringRef CountSym) {
+  if (TableSymbols.empty())
     return;
 
-  SEHTable = make<SEHTableChunk>(Handlers);
-  RData->addChunk(SEHTable);
+  RVATableChunk *TableChunk = make<RVATableChunk>(std::move(TableSymbols));
+  RData->addChunk(TableChunk);
 
-  // Replace the absolute table symbol with a synthetic symbol pointing to the
-  // SEHTable chunk so that we can emit base relocations for it and resolve
-  // section relative relocations.
-  Symbol *T = Symtab->find("___safe_se_handler_table");
-  Symbol *C = Symtab->find("___safe_se_handler_count");
-  replaceSymbol<DefinedSynthetic>(T, T->getName(), SEHTable);
-  cast<DefinedAbsolute>(C)->setVA(SEHTable->getSize() / 4);
+  Symbol *T = Symtab->findUnderscore(TableSym);
+  Symbol *C = Symtab->findUnderscore(CountSym);
+  replaceSymbol<DefinedSynthetic>(T, T->getName(), TableChunk);
+  cast<DefinedAbsolute>(C)->setVA(TableChunk->getSize() / 4);
 }
 
 // Handles /section options to allow users to overwrite
 // section attributes.
 void Writer::setSectionPermissions() {
   for (auto &P : Config->Section) {
     StringRef Name = P.first;
     uint32_t Perm = P.second;

test/COFF/gfids-corrupt.s

+# RUN: llvm-mc -triple x86_64-windows-msvc %s -filetype=obj -o %t.obj
+# RUN: lld-link %t.obj -opt:noref -guard:cf -out:%t.exe -entry:main 2>&1 | FileCheck %s --check-prefix=ERRS
+# RUN: llvm-readobj -file-headers -coff-load-config %t.exe | FileCheck %s
+
+# ERRS: warning: ignoring .gfids$y symbol table index section in object {{.*}}gfids-corrupt{{.*}}
+# ERRS: warning: ignoring invalid symbol table index in section .gfids$y in object {{.*}}gfids-corrupt{{.*}}
+
+# The table is arbitrary, really.
+# CHECK: ImageBase: 0x140000000
+# CHECK: LoadConfig [
+# CHECK:   SEHandlerTable: 0x0
+# CHECK:   SEHandlerCount: 0
+# CHECK:   GuardCFCheckFunction: 0x0
+# CHECK:   GuardCFCheckDispatch: 0x0
+# CHECK:   GuardCFFunctionTable: 0x14000{{.*}}
+# CHECK:   GuardCFFunctionCount: 2
+# CHECK:   GuardFlags: 0x500
+# CHECK:   GuardAddressTakenIatEntryTable: 0x0
+# CHECK:   GuardAddressTakenIatEntryCount: 0
+# CHECK:   GuardLongJumpTargetTable: 0x0
+# CHECK:   GuardLongJumpTargetCount: 0
+# CHECK: ]
+# CHECK:      GuardFidTable [
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT: ]
+
+
+# Indicate that gfids are present.
+        .def     @feat.00; .scl    3; .type   0; .endef
+        .globl  @feat.00
+@feat.00 = 0x800
+
+        .def     f1; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,f1
+        .global f1
+f1:
+        movl $42, %eax
+        retq
+
+        .def     f2; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,f2
+        .global f2
+f2:
+        movl $13, %eax
+        retq
+
+        .section        .data,"dw",one_only,fp1
+        .globl  fp1
+fp1:
+        .quad   f1
+
+        .section        .data,"dw",one_only,fp2
+        .globl  fp2
+fp2:
+        .quad   f2
+
+        .section        .gfids$y,"dr",associative,fp1
+        .symidx f1
+        .byte 0
+
+        .section        .gfids$y,"dr",associative,fp2
+        .symidx f2
+        .long 0x400
+
+        .def     main; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,main
+        .globl  main
+main:
+        callq      *fp1(%rip)
+        callq      *fp2(%rip)
+        xor %eax, %eax
+        retq
+
+        .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

test/COFF/gfids-fallback.s

+# RUN: grep -B99999 [S]PLITMARKER %s | llvm-mc -triple x86_64-windows-msvc -filetype=obj -o %t1.obj
+# RUN: grep -A99999 [S]PLITMARKER %s | llvm-mc -triple x86_64-windows-msvc -filetype=obj -o %t2.obj
+# RUN: lld-link %t1.obj %t2.obj -guard:cf -out:%t.exe -entry:main -opt:noref
+# RUN: llvm-readobj -file-headers -coff-load-config %t.exe | FileCheck %s
+
+# CHECK: ImageBase: 0x140000000
+# CHECK: LoadConfig [
+# CHECK:   SEHandlerTable: 0x0
+# CHECK:   SEHandlerCount: 0
+# CHECK:   GuardCFCheckFunction: 0x0
+# CHECK:   GuardCFCheckDispatch: 0x0
+# CHECK:   GuardCFFunctionTable: 0x14000{{.*}}
+# CHECK:   GuardCFFunctionCount: 3
+# CHECK:   GuardFlags: 0x500
+# CHECK:   GuardAddressTakenIatEntryTable: 0x0
+# CHECK:   GuardAddressTakenIatEntryCount: 0
+# CHECK:   GuardLongJumpTargetTable: 0x0
+# CHECK:   GuardLongJumpTargetCount: 0
+# CHECK: ]
+# CHECK:      GuardFidTable [
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT: ]
+
+
+# Indicate that no gfids are present. All symbols used by relocations in this
+# file will be considered address-taken.
+        .def     @feat.00; .scl    3; .type   0; .endef
+        .globl  @feat.00
+@feat.00 = 0
+
+        .def     main; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,main
+        .globl  main
+main:
+        subq $8, %rsp
+        leaq foo(%rip), %rdx
+        callq bar
+        movl $0, %eax
+        addq $8, %rsp
+        retq
+
+# Should not appear in gfids table.
+        .def     baz; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,baz
+        .globl  baz
+baz:
+        mov $1, %eax
+        retq
+
+        .def     qux; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,qux
+        .globl  qux
+qux:
+        mov $2, %eax
+        retq
+
+        .def     quxx; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,quxx
+        .globl  quxx
+quxx:
+        mov $3, %eax
+        retq
+
+# Load config.
+        .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
+
+# SPLITMARKER
+
+# Indicate that gfids are present. This file does not take any addresses.
+        .def     @feat.00; .scl    3; .type   0; .endef
+        .globl  @feat.00
+@feat.00 = 0x800
+
+        .def     foo; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,foo
+        .global foo
+foo:
+        movl $42, %eax
+        retq
+
+        .def     bar; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,bar
+        .global bar
+bar:
+        movl $13, %eax
+        retq

test/COFF/gfids-gc.s

+# RUN: llvm-mc -triple x86_64-windows-msvc %s -filetype=obj -o %t.obj
+# RUN: lld-link %t.obj -guard:cf -out:%t.exe -opt:noref -entry:main
+# RUN: llvm-readobj -file-headers -coff-load-config %t.exe | FileCheck %s --check-prefix=CHECK-NOGC
+# RUN: lld-link %t.obj -guard:cf -out:%t.exe -opt:ref -entry:main
+# RUN: llvm-readobj -file-headers -coff-load-config %t.exe | FileCheck %s --check-prefix=CHECK-GC
+
+# This assembly is meant to mimic what CL emits for this kind of C code when
+# /Gw (-fdata-sections) is enabled:
+# int f() { return 42; }
+# int g() { return 13; }
+# int (*fp1)() = &f;
+# int (*fp2)() = &g;
+# int main() {
+#   return fp1();
+# }
+# Compile with 'cl -c -guard:cf -Gw -O1' and note the two associative .gfids$y
+# sections.
+
+# Expect 3 entries: main, f, and g.
+
+# CHECK-NOGC: ImageBase: 0x140000000
+# CHECK-NOGC: LoadConfig [
+# CHECK-NOGC:   SEHandlerTable: 0x0
+# CHECK-NOGC:   SEHandlerCount: 0
+# CHECK-NOGC:   GuardCFCheckFunction: 0x0
+# CHECK-NOGC:   GuardCFCheckDispatch: 0x0
+# CHECK-NOGC:   GuardCFFunctionTable: 0x14000{{.*}}
+# CHECK-NOGC:   GuardCFFunctionCount: 3
+# CHECK-NOGC:   GuardFlags: 0x500
+# CHECK-NOGC:   GuardAddressTakenIatEntryTable: 0x0
+# CHECK-NOGC:   GuardAddressTakenIatEntryCount: 0
+# CHECK-NOGC:   GuardLongJumpTargetTable: 0x0
+# CHECK-NOGC:   GuardLongJumpTargetCount: 0
+# CHECK-NOGC: ]
+# CHECK-NOGC:      GuardFidTable [
+# CHECK-NOGC-NEXT:   0x14000{{.*}}
+# CHECK-NOGC-NEXT:   0x14000{{.*}}
+# CHECK-NOGC-NEXT:   0x14000{{.*}}
+# CHECK-NOGC-NEXT: ]
+
+# Expect 2 entries: main and f. fp2 was discarded, so g was only used as a
+# direct call target.
+
+# CHECK-GC: ImageBase: 0x140000000
+# CHECK-GC: LoadConfig [
+# CHECK-GC:   SEHandlerTable: 0x0
+# CHECK-GC:   SEHandlerCount: 0
+# CHECK-GC:   GuardCFCheckFunction: 0x0
+# CHECK-GC:   GuardCFCheckDispatch: 0x0
+# CHECK-GC:   GuardCFFunctionTable: 0x14000{{.*}}
+# CHECK-GC:   GuardCFFunctionCount: 2
+# CHECK-GC:   GuardFlags: 0x500
+# CHECK-GC:   GuardAddressTakenIatEntryTable: 0x0
+# CHECK-GC:   GuardAddressTakenIatEntryCount: 0
+# CHECK-GC:   GuardLongJumpTargetTable: 0x0
+# CHECK-GC:   GuardLongJumpTargetCount: 0
+# CHECK-GC: ]
+# CHECK-GC:      GuardFidTable [
+# CHECK-GC-NEXT:   0x14000{{.*}}
+# CHECK-GC-NEXT:   0x14000{{.*}}
+# CHECK-GC-NEXT: ]
+
+
+# We need @feat.00 to have 0x800 to indicate .gfids are present.
+        .def     @feat.00;
+        .scl    3;
+        .type   0;
+        .endef
+        .globl  @feat.00
+@feat.00 = 0x801
+
+        .def     main;
+        .scl    2;
+        .type   32;
+        .endef
+        .section        .text,"xr",one_only,main
+        .globl  main
+main:
+        # Call g directly so that it is not dead stripped.
+        callq g
+        rex64 jmpq      *fp1(%rip)
+
+        .def     f;
+        .scl    3;
+        .type   32;
+        .endef
+        .section        .text,"xr",one_only,f
+f:
+        movl $42, %eax
+        retq
+
+        .section        .data,"dw",one_only,fp1
+        .globl  fp1
+fp1:
+        .quad   f
+
+        .section        .gfids$y,"dr",associative,fp1
+        .symidx f
+
+# Section GC will remove the following, so 'g' should not be present in the
+# guard fid table.
+
+        .def     g;
+        .scl    3;
+        .type   32;
+        .endef
+        .section        .text,"xr",one_only,g
+g:
+        movl $13, %eax
+        retq
+
+        .section        .data,"dw",one_only,fp2
+        .globl  fp2
+fp2:
+        .quad   g
+
+        .section        .gfids$y,"dr",associative,fp2
+        .symidx g
+
+        .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

test/COFF/gfids-icf.s

+# RUN: llvm-mc -triple x86_64-windows-msvc %s -filetype=obj -o %t.obj
+# RUN: lld-link %t.obj -guard:cf -out:%t.exe -opt:icf -entry:main
+# RUN: llvm-readobj -file-headers -coff-load-config %t.exe | FileCheck %s --check-prefix=CHECK
+
+# This assembly is meant to mimic what CL emits for this kind of C code:
+# int icf1() { return 42; }
+# int icf2() { return 42; }
+# int (*fp1)() = &icf1;
+# int (*fp2)() = &icf2;
+# int main() {
+#   return fp1();
+#   return fp2();
+# }
+
+# 'icf1' and 'icf2' are address taken, but should be merged into one entry.
+# There are two entries in the table because 'main' is included.
+
+# CHECK: ImageBase: 0x140000000
+# CHECK: LoadConfig [
+# CHECK:   SEHandlerTable: 0x0
+# CHECK:   SEHandlerCount: 0
+# CHECK:   GuardCFCheckFunction: 0x0
+# CHECK:   GuardCFCheckDispatch: 0x0
+# CHECK:   GuardCFFunctionTable: 0x14000{{.*}}
+# CHECK:   GuardCFFunctionCount: 2
+# CHECK:   GuardFlags: 0x500
+# CHECK:   GuardAddressTakenIatEntryTable: 0x0
+# CHECK:   GuardAddressTakenIatEntryCount: 0
+# CHECK:   GuardLongJumpTargetTable: 0x0
+# CHECK:   GuardLongJumpTargetCount: 0
+# CHECK: ]
+# CHECK:      GuardFidTable [
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT: ]
+
+
+# Indicate that gfids are present.
+        .def     @feat.00; .scl    3; .type   0; .endef
+        .globl  @feat.00
+@feat.00 = 0x800
+
+        .def     icf1; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,icf1
+        .global icf1
+icf1:
+        movl $42, %eax
+        retq
+
+        .def     icf2; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,icf2
+        .global icf2
+icf2:
+        movl $42, %eax
+        retq
+
+# Take their two addresses.
+        .data
+        .globl  fp1
+fp1:
+        .quad   icf1
+        .globl  fp2
+fp2:
+        .quad   icf2
+
+        .section        .gfids$y,"dr"
+        .symidx icf1
+        .symidx icf2
+
+        .def     main; .scl    2; .type   32; .endef
+        .section        .text,"xr",one_only,main
+        .globl  main
+main:
+        callq      *fp1(%rip)
+        callq      *fp2(%rip)
+        xor %eax, %eax
+        retq
+
+        .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