[CFGuard] Add address-taken IAT tables and delay-load support

[ajpaverd]

Summary

This patch adds support for creating Guard Address-Taken IAT Entry Tables (.giats$y sections) in object files, matching the behavior of MSVC. These contain lists of address-taken imported functions, which are used by the linker to create the final GIATS table.
Additionally, if any DLLs are delay-loaded, the linker must look through the .giats tables and add the respective load thunks of address-taken imports to the GFIDS table, as these are also valid call targets.

[rnk]

lld/COFF/Writer.cpp

1682

Can we set either ProtectDelayLoadIAT or DelayLoadIATSection now, or does that take more work?

1761

These two methods are essentially the same up until here. Your version seems more general. Can you rewrite markSymbolsForRVATable in terms of this utility? This code shouldn't be hot, I don't think we need to worry about the cost of an extra temporary vector of address-taken symbols.

[ajpaverd]

Rewrite markSymbolsForRVATable() in terms of getSymbolsFromSections()

lld/COFF/Writer.cpp

1682

I think this will take more work (e.g. the latter requires moving the Delayload import table into its own .didat section).

[rnk]

lgtm

[aeubanks]

I believe this is causing https://crbug.com/1134215:

ninja -t msvc -e environment.x86 -- ..\..\third_party\llvm-build\Release+Asserts\bin\lld-link.exe /nologo -libpath:..\..\third_party\llvm-build\Release+Asserts\lib\clang\12.0.0\lib\windows -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\VC\Tools\MSVC\14.26.28801\lib\x86 -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\win_sdk\Lib\10.0.19041.0\um\x86 -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\win_sdk\Lib\10.0.19041.0\ucrt\x86 -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\VC\Tools\MSVC\14.26.28801\atlmfc\lib\x86 /IMPLIB:./browser_switcher_bho.dll.lib /DLL /OUT:./browser_switcher_bho.dll /PDB:./browser_switcher_bho.dll.pdb @./browser_switcher_bho.dll.rsp
unknown symbol kind
UNREACHABLE executed at C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Symbols.h:443!
PLEASE submit a bug report to https://crbug.com and run tools/clang/scripts/process_crashreports.py (only works inside Google) which will upload a report and include the crash backtrace.
Stack dump:
0.	Program arguments: ..\..\third_party\llvm-build\Release+Asserts\bin\lld-link.exe /nologo -libpath:..\..\third_party\llvm-build\Release+Asserts\lib\clang\12.0.0\lib\windows -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\VC\Tools\MSVC\14.26.28801\lib\x86 -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\win_sdk\Lib\10.0.19041.0\um\x86 -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\win_sdk\Lib\10.0.19041.0\ucrt\x86 -libpath:..\..\third_party\depot_tools\win_toolchain\vs_files\a687d8e2e4114d9015eb550e1b156af21381faac\VC\Tools\MSVC\14.26.28801\atlmfc\lib\x86 /IMPLIB:./browser_switcher_bho.dll.lib /DLL /OUT:./browser_switcher_bho.dll /PDB:./browser_switcher_bho.dll.pdb @./browser_switcher_bho.dll.rsp 
 #0 0x00007ff7471970b5 HandleAbort C:\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\lib\Support\Windows\Signals.inc:408:0
 #1 0x00007ff7495b0063 raise minkernel\crts\ucrt\src\appcrt\misc\signal.cpp:547:0
 #2 0x00007ff7495aa9d0 abort minkernel\crts\ucrt\src\appcrt\startup\abort.cpp:71:0
 #3 0x00007ff74716d416 llvm::llvm_unreachable_internal(char const *, char const *, unsigned int) C:\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\lib\Support\ErrorHandling.cpp:213:0
 #4 0x00007ff7471f067c lld::coff::Defined::getChunk(void) C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Symbols.h:443:0
 #5 0x00007ff747244292 addSymbolToRVASet C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Writer.cpp:1542:0
 #6 0x00007ff747245187 `anonymous namespace'::Writer::createGuardCFTables C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Writer.cpp:1655:0
 #7 0x00007ff747246283 `anonymous namespace'::Writer::createMiscChunks C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Writer.cpp:988:0
 #8 0x00007ff74724bbb8 `anonymous namespace'::Writer::run C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Writer.cpp:614:0
 #9 0x00007ff74724d439 lld::coff::writeResult(void) C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Writer.cpp:311:0
#10 0x00007ff7471f852a lld::coff::LinkerDriver::link(class llvm::ArrayRef<char const *>) C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Driver.cpp:2143:0
#11 0x00007ff7471f8af0 lld::coff::link(class llvm::ArrayRef<char const *>, bool, class llvm::raw_ostream &, class llvm::raw_ostream &) C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\COFF\Driver.cpp:96:0
#12 0x00007ff747158898 lldMain C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\tools\lld\lld.cpp:155:0
#13 0x00007ff747158ce7 main C:\b\s\w\ir\cache\builder\src\third_party\llvm\lld\tools\lld\lld.cpp:211:0
#14 0x00007ff749583a04 __scrt_common_main_seh d:\A01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
#15 0x00007ff9c0982774 (C:\Windows\System32\KERNEL32.DLL+0x12774)
#16 0x00007ff9c0a90d51 (C:\Windows\SYSTEM32\ntdll.dll+0x70d51)

[aeubanks]

Printing out the kind() of the failing symbol, it gives 40, which is nowhere in the range of Symbol::Kind. Reverting for now, let me know if you need more details for fixing this.

[theraven]

We don't normally revert commits for downstream build failures. Please can you submit a test case that triggers this issue?

[aeubanks]

Working on a test case right now.

[aeubanks]

turns out it was a variable not being initialized: https://reviews.llvm.org/D88744

[aeubanks]

In D87544#2308930, @aeubanks wrote:

turns out it was a variable not being initialized: https://reviews.llvm.org/D88744

Whoops, forgot that was on top of this patch. Commented inline in this patch

lld/COFF/Symbols.h

352

this needs to be initialized

[ajpaverd]

Thanks for the diagnostics @aeubanks! Does this now resolve the issue with the Chrome build?

[aeubanks]

When I was looking into it, after initializing loadThunkSym to nullptr, the build step that was failing then succeeded. So that should be good enough.

[ajpaverd]

Always initialize loadThunkSym to nullptr.

[aeubanks]

lgtm, feel free to reland

[ajpaverd]

Thanks @aeubanks! I don't yet have commit access and my colleague is on leave.

[dmajor]

This caused a regression when calling through function pointers in a certain way as seen in sqlite3: https://bugs.llvm.org/show_bug.cgi?id=47905

[hans]

In D87544#2339299, @dmajor wrote:

This caused a regression when calling through function pointers in a certain way as seen in sqlite3: https://bugs.llvm.org/show_bug.cgi?id=47905

Reverted in 418f18c6cdfe56e77669e2f4d3df3bca1020156d due to that.

[ajpaverd]

Correctly handle __imp_ symbols in .giats and .gfids sections

For address-taken dllimport functions with a corresponding __imp_ symbol defined, add the __imp_ symbol to the .giats section. Always add the symbols of address-taken functions to the .gfids section.

[ajpaverd]

@hans and @dmajor thanks for reporting this issue (https://bugs.llvm.org/show_bug.cgi?id=47905).
Do the latest changes resolve this in Chromium and sqlite3?

[hans]

For Chromium, the latest patch seems to fix the issues we saw. Thanks!

[dmajor]

Firefox tests look good as well.

[aeubanks]

would you like me to reland this?

[thakis]

FYI: While the reland didn't break anything, it apparently also didn't fix everything it set out to fix, see https://bugs.chromium.org/p/chromium/issues/detail?id=1132179#c13 and onwards.

lld/COFF/DLL.cpp

 // If you are not particularly interested in linking against Windows
 // DLL, you can skip this file, and you should still be able to
 // understand the rest of the linker.
 //
 //===----------------------------------------------------------------------===//
 
 #include "DLL.h"
 #include "Chunks.h"
+#include "SymbolTable.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Path.h"
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::support::endian;
 using namespace llvm::COFF;
for (DefinedImportData *s : syms) {
       thunks.push_back(t);
       StringRef extName = s->getExternalName();
       if (extName.empty()) {
         names.push_back(make<OrdinalOnlyChunk>(s->getOrdinal()));
       } else {
         auto *c = make<HintNameChunk>(extName, 0);
         names.push_back(make<LookupChunk>(c));
         hintNames.push_back(c);
+        // Add a syntentic symbol for this load thunk, using the "__imp_load"
+        // prefix, in case this thunk needs to be added to the list of valid
+        // call targets for Control Flow Guard.
+        StringRef symName = saver.save("__imp_load_" + extName);
+        s->loadThunkSym =
+            cast<DefinedSynthetic>(symtab->addSynthetic(symName, t));
       }
     }
     thunks.push_back(tm);
+    StringRef tmName =
+        saver.save("__tailMerge_" + syms[0]->getDLLName().lower());
+    symtab->addSynthetic(tmName, tm);
     // Terminate with null values.
     addresses.push_back(make<NullChunk>(8));
     names.push_back(make<NullChunk>(8));
 
     for (int i = 0, e = syms.size(); i < e; ++i)
       syms[i]->setLocation(addresses[base + i]);
     auto *mh = make<NullChunk>(8);
     mh->setAlignment(8);

lld/COFF/ICF.cpp

 
 // Returns true if two sections' associative children are equal.
 bool ICF::assocEquals(const SectionChunk *a, const SectionChunk *b) {
   // Ignore associated metadata sections that don't participate in ICF, such as
   // debug info and CFGuard metadata.
   auto considerForICF = [](const SectionChunk &assoc) {
     StringRef Name = assoc.getSectionName();
     return !(Name.startswith(".debug") || Name == ".gfids$y" ||
-             Name == ".gljmp$y");
+             Name == ".giats$y" || Name == ".gljmp$y");
   };
   auto ra = make_filter_range(a->children(), considerForICF);
   auto rb = make_filter_range(b->children(), considerForICF);
   return std::equal(ra.begin(), ra.end(), rb.begin(), rb.end(),
                     [&](const SectionChunk &ia, const SectionChunk &ib) {
                       return ia.eqClass[cnt % 2] == ib.eqClass[cnt % 2];
                     });
 }

lld/COFF/InputFiles.h

explicit ObjFile(MemoryBufferRef m, std::vector<Symbol *> &&symbols)
       : InputFile(ObjectKind, m), symbols(std::move(symbols)) {}
   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<SectionChunk *> getGuardIATChunks() { return guardIATChunks; }
   ArrayRef<SectionChunk *> getGuardLJmpChunks() { return guardLJmpChunks; }
   ArrayRef<Symbol *> getSymbols() { return symbols; }
 
   ArrayRef<uint8_t> getDebugSection(StringRef secName);
 
   // Returns a Symbol object for the symbolIndex'th symbol in the
   // underlying object file.
   Symbol *getSymbol(uint32_t symbolIndex) {
private:
 
   // 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 and longjmp
-  // targets.  These are not linked into the final binary when /guard:cf is set.
+  // Chunks containing symbol table indices of address taken symbols, address
+  // taken IAT entries, and longjmp targets. These are not linked into the
+  // final binary when /guard:cf is set.
   std::vector<SectionChunk *> guardFidChunks;
+  std::vector<SectionChunk *> guardIATChunks;
   std::vector<SectionChunk *> guardLJmpChunks;
 
   // This vector contains a list of all symbols defined or referenced by this
   // file. They are indexed such that you can get a Symbol by symbol
   // index. Nonexistent indices (which are occupied by auxiliary
   // symbols in the real symbol table) are filled with null pointers.
   std::vector<Symbol *> symbols;
 

lld/COFF/InputFiles.cpp

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 (name == ".gfids$y")
     guardFidChunks.push_back(c);
+  else if (name == ".giats$y")
+    guardIATChunks.push_back(c);
   else if (name == ".gljmp$y")
     guardLJmpChunks.push_back(c);
   else if (name == ".sxdata")
     sxDataChunks.push_back(c);
   else if (config->tailMerge && sec->NumberOfRelocations == 0 &&
            name == ".rdata" && leaderName.startswith("??_C@"))
     // COFF sections that look like string literal sections (i.e. no
     // relocations, in .rdata, leader symbol name matches the MSVC name mangling

lld/COFF/Symbols.h

public:
   Chunk *getChunk() { return file->location; }
   void setLocation(Chunk *addressTable) { file->location = addressTable; }
 
   StringRef getDLLName() { return file->dllName; }
   StringRef getExternalName() { return file->externalName; }
   uint16_t getOrdinal() { return file->hdr->OrdinalHint; }
 
   ImportFile *file;
+
+  // This is a pointer to the synthetic symbol associated with the load thunk
+  // for this symbol that will be called if the DLL is delay-loaded. This is
+  // needed for Control Flow Guard because if this DefinedImportData symbol is a
+  // valid call target, the corresponding load thunk must also be marked as a
+  // valid call target.
+  DefinedSynthetic *loadThunkSym;
 };
 
 // This class represents a symbol for a jump table entry which jumps
 // to a function in a DLL. Linker are supposed to create such symbols
 // without "__imp_" prefix for all function symbols exported from
 // DLLs, so that you can call DLL functions as regular functions with
 // a regular name. A function pointer is given as a DefinedImportData.
 class DefinedImportThunk : public Defined {

lld/COFF/Writer.cpp

private:
   template <typename PEHeaderTy> void writeHeader();
   void createSEHTable();
   void createRuntimePseudoRelocs();
   void insertCtorDtorSymbols();
   void createGuardCFTables();
   void markSymbolsForRVATable(ObjFile *file,
                               ArrayRef<SectionChunk *> symIdxChunks,
                               SymbolRVASet &tableSymbols);
+  void getSymbolsFromSections(ObjFile *file,
+                              ArrayRef<SectionChunk *> symIdxChunks,
+                              std::vector<Symbol *> &symbols);
   void maybeAddRVATable(SymbolRVASet tableSymbols, StringRef tableSym,
                         StringRef countSym);
   void setSectionPermissions();
   void writeSections();
   void writeBuildId();
   void sortSections();
   void sortExceptionTable();
   void sortCRTSectionChunks(std::vector<Chunk *> &chunks);

 }
 
 // The main function of the writer.
 void Writer::run() {
   ScopedTimer t1(codeLayoutTimer);
 
   createImportTables();
   createSections();
-  createMiscChunks();
   appendImportThunks();
+  // Import thunks must be added before the Control Flow Guard tables are added.
+  createMiscChunks();
   createExportTable();
   mergeSections();
   removeUnusedSections();
   finalizeAddresses();
   removeEmptySections();
   assignOutputSectionIndices();
   setSectionPermissions();
   createSymbolAndStringTable();
static void markSymbolsWithRelocations(ObjFile *file,
   }
 }
 
 // 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::createGuardCFTables() {
   SymbolRVASet addressTakenSyms;
+  SymbolRVASet giatsRVASet;
+  std::vector<Symbol *> giatsSymbols;
   SymbolRVASet longJmpTargets;
   for (ObjFile *file : ObjFile::instances) {
     // If the object was compiled with /guard:cf, the address taken symbols
     // are in .gfids$y sections, and the longjmp targets are in .gljmp$y
     // sections. If the object was not compiled with /guard:cf, we assume there
     // were no setjmp targets, and that all code symbols with relocations are
     // possibly address-taken.
     if (file->hasGuardCF()) {
       markSymbolsForRVATable(file, file->getGuardFidChunks(), addressTakenSyms);
+      markSymbolsForRVATable(file, file->getGuardIATChunks(), giatsRVASet);
+      getSymbolsFromSections(file, file->getGuardIATChunks(), giatsSymbols);
       markSymbolsForRVATable(file, file->getGuardLJmpChunks(), longJmpTargets);
     } else {
       markSymbolsWithRelocations(file, addressTakenSyms);
     }
   }
 
   // Mark the image entry as address-taken.
   if (config->entry)
     maybeAddAddressTakenFunction(addressTakenSyms, config->entry);
 
   // Mark exported symbols in executable sections as address-taken.
   for (Export &e : config->exports)
     maybeAddAddressTakenFunction(addressTakenSyms, e.sym);
 
+  // For each entry in the .giats table, check if it has a corresponding load
+  // thunk (e.g. because the DLL that defines it will be delay-loaded) and, if
+  // so, add the load thunk to the address taken (.gfids) table.
+  for (Symbol *s : giatsSymbols) {
+    if (auto *di = dyn_cast<DefinedImportData>(s)) {
+      if (di->loadThunkSym)
+        addSymbolToRVASet(addressTakenSyms, di->loadThunkSym);
+    }
+  }
+
   // Ensure sections referenced in the gfid table are 16-byte aligned.
   for (const ChunkAndOffset &c : addressTakenSyms)
     if (c.inputChunk->getAlignment() < 16)
       c.inputChunk->setAlignment(16);
 
   maybeAddRVATable(std::move(addressTakenSyms), "__guard_fids_table",
                    "__guard_fids_count");
 
+  // Add the Guard Address Taken IAT Entry Table (.giats).
+  maybeAddRVATable(std::move(giatsRVASet), "__guard_iat_table",
+                   "__guard_iat_count");
+
   // Add the longjmp target table unless the user told us not to.
   if (config->guardCF == GuardCFLevel::Full)
     maybeAddRVATable(std::move(longJmpTargets), "__guard_longjmp_table",
                      "__guard_longjmp_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);
   if (config->guardCF == GuardCFLevel::Full)
     guardFlags |= uint32_t(coff_guard_flags::HasLongJmpTable);
   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
+// symbols to a vector. 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,
+void Writer::getSymbolsFromSections(ObjFile *file,
                                     ArrayRef<SectionChunk *> symIdxChunks,
-                                    SymbolRVASet &tableSymbols) {
+                                    std::vector<Symbol *> &symbols) {
   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->live)
       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.
+    // final link. If so, add it to the vector of symbols.
     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));
-      }
-    }
-  }
+          symbols.push_back(cast<Symbol>(s));
+      }
+
+// Take a list of input sections containing symbol table indices and add those
+// symbols to an RVA table.
+void Writer::markSymbolsForRVATable(ObjFile *file,
+                                    ArrayRef<SectionChunk *> symIdxChunks,
+                                    SymbolRVASet &tableSymbols) {
+  std::vector<Symbol *> syms;
+  getSymbolsFromSections(file, symIdxChunks, syms);
+
+  for (Symbol *s : syms)
+    addSymbolToRVASet(tableSymbols, cast<Defined>(s));
 }
 
 // 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(SymbolRVASet tableSymbols, StringRef tableSym,
                               StringRef countSym) {
   if (tableSymbols.empty())
     return;
 
   RVATableChunk *tableChunk = make<RVATableChunk>(std::move(tableSymbols));
   rdataSec->addChunk(tableChunk);
 
   Symbol *t = symtab->findUnderscore(tableSym);
   Symbol *c = symtab->findUnderscore(countSym);
   replaceSymbol<DefinedSynthetic>(t, t->getName(), tableChunk);
   cast<DefinedAbsolute>(c)->setVA(tableChunk->getSize() / 4);
 }
 
 // MinGW specific. Gather all relocations that are imported from a DLL even
 // though the code didn't expect it to, produce the table that the runtime
 // uses for fixing them up, and provide the synthetic symbols that the
 // runtime uses for finding the table.
 void Writer::createRuntimePseudoRelocs() {
   std::vector<RuntimePseudoReloc> rels;
 
   for (Chunk *c : symtab->getChunks()) {
     auto *sc = dyn_cast<SectionChunk>(c);
     if (!sc || !sc->live)
       continue;

lld/test/COFF/giats.s

+# REQUIRES: x86
+
+# Make a DLL that exports exportfn1.
+# RUN: yaml2obj %p/Inputs/export.yaml -o %basename_t-exp.obj
+# RUN: lld-link /out:%basename_t-exp.dll /dll %basename_t-exp.obj /export:exportfn1 /implib:%basename_t-exp.lib
+
+# Make an object file that imports exportfn1.
+# RUN: llvm-mc -triple x86_64-windows-msvc %s -filetype=obj -o %basename_t.obj
+
+# Check that the Guard address-taken IAT entry tables are propagated to the final executable.
+# RUN: lld-link %basename_t.obj -guard:cf -entry:main -out:%basename_t-nodelay.exe %basename_t-exp.lib
+# RUN: llvm-readobj --file-headers --coff-load-config %basename_t-nodelay.exe | FileCheck %s --check-prefix CHECK
+
+# CHECK: ImageBase: 0x140000000
+# CHECK: LoadConfig [
+# CHECK:   GuardCFFunctionTable: 0x140002114
+# CHECK:   GuardCFFunctionCount: 1
+# CHECK:   GuardFlags: 0x10500
+# CHECK:   GuardAddressTakenIatEntryTable: 0x140002118
+# CHECK:   GuardAddressTakenIatEntryCount: 1
+# CHECK: ]
+# CHECK:      GuardFidTable [
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT: ]
+# CHECK:      GuardIatTable [
+# CHECK-NEXT:   0x14000{{.*}}
+# CHECK-NEXT: ]
+
+
+# Check that the additional load thunk symbol is added to the GFIDs table.
+# RUN: lld-link %basename_t.obj -guard:cf -entry:main -out:%basename_t-delay.exe %basename_t-exp.lib -alternatename:__delayLoadHelper2=main -delayload:%basename_t-exp.dll
+# RUN: llvm-readobj --file-headers --coff-load-config %basename_t-delay.exe | FileCheck %s --check-prefix DELAY-CHECK
+
+# DELAY-CHECK: ImageBase: 0x140000000
+# DELAY-CHECK: LoadConfig [
+# DELAY-CHECK:   GuardCFFunctionTable: 0x140002114
+# DELAY-CHECK:   GuardCFFunctionCount: 2
+# DELAY-CHECK:   GuardFlags: 0x10500
+# DELAY-CHECK:   GuardAddressTakenIatEntryTable: 0x14000211C
+# DELAY-CHECK:   GuardAddressTakenIatEntryCount: 1
+# DELAY-CHECK: ]
+# DELAY-CHECK:      GuardFidTable [
+# DELAY-CHECK-NEXT:   0x14000{{.*}}
+# DELAY-CHECK-NEXT:   0x14000{{.*}}
+# DELAY-CHECK-NEXT: ]
+# DELAY-CHECK:      GuardIatTable [
+# DELAY-CHECK-NEXT:   0x14000{{.*}}
+# DELAY-CHECK-NEXT: ]
+
+
+# This assembly is reduced from C code like:
+# __declspec(noinline)
+# void IndirectCall(BOOL (func)(HANDLE)) {
+#   (*func)(NULL);
+# }
+# int main(int argc, char** argv) {
+#   IndirectCall(exportfn1);
+# }
+
+  .text
+  .def   @feat.00;
+  .scl  3;
+  .type 0;
+  .endef
+  .globl  @feat.00
+.set @feat.00, 2048
+  .def   IndirectCall;  .scl  2;  .type 32; .endef
+  .globl  IndirectCall                    # -- Begin function IndirectCall
+  .p2align  4, 0x90
+IndirectCall:                           # @IndirectCall
+# %bb.0:
+  subq  $40, %rsp
+  movq  %rcx, 32(%rsp)
+  movq  32(%rsp), %rax
+  movq  %rax, %rdx        # This would otherwise have be: movq __guard_dispatch_icall_fptr(%rip), %rdx
+  xorl  %ecx, %ecx
+  callq *%rdx
+  nop
+  addq  $40, %rsp
+  retq
+                                        # -- End function
+  .def   main;  .scl  2;  .type 32; .endef
+  .globl  main                            # -- Begin function main
+  .p2align  4, 0x90
+main:                                   # @main
+# %bb.0:
+  subq  $56, %rsp
+  movq  __imp_exportfn1(%rip), %rax
+  movq  %rdx, 48(%rsp)
+  movl  %ecx, 44(%rsp)
+  movq  %rax, %rcx
+  callq IndirectCall
+  xorl  %eax, %eax
+  addq  $56, %rsp
+  retq
+                                        # -- End function
+  .section  .gfids$y,"dr"
+  .section  .giats$y,"dr"
+  .symidx __imp_exportfn1
+  .section  .gljmp$y,"dr"
+
+# 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 12, 1, 0
+        .quad __guard_iat_table
+        .quad __guard_iat_count
+        .quad __guard_longjmp_table
+        .quad __guard_fids_count
+        .fill 84, 1, 0
 No newline at end of file

llvm/include/llvm/MC/MCObjectFileInfo.h

protected:
   MCSection *ThreadLocalPointerSection = nullptr;
 
   /// COFF specific sections.
   MCSection *DrectveSection = nullptr;
   MCSection *PDataSection = nullptr;
   MCSection *XDataSection = nullptr;
   MCSection *SXDataSection = nullptr;
   MCSection *GFIDsSection = nullptr;
+  MCSection *GIATsSection = nullptr;
   MCSection *GLJMPSection = nullptr;
 
   // XCOFF specific sections
   MCSection *TOCBaseSection = nullptr;
 
 public:
   void InitMCObjectFileInfo(const Triple &TT, bool PIC, MCContext &ctx,
                             bool LargeCodeModel = false);
public:
   }
 
   // COFF specific sections.
   MCSection *getDrectveSection() const { return DrectveSection; }
   MCSection *getPDataSection() const { return PDataSection; }
   MCSection *getXDataSection() const { return XDataSection; }
   MCSection *getSXDataSection() const { return SXDataSection; }
   MCSection *getGFIDsSection() const { return GFIDsSection; }
+  MCSection *getGIATsSection() const { return GIATsSection; }
   MCSection *getGLJMPSection() const { return GLJMPSection; }
 
   // XCOFF specific sections
   MCSection *getTOCBaseSection() const { return TOCBaseSection; }
 
   MCSection *getEHFrameSection() {
     return EHFrameSection;
   }

llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp

 //===-- CodeGen/AsmPrinter/WinCFGuard.cpp - Control Flow Guard Impl ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains support for writing the metadata for Windows Control Flow
-// Guard, including address-taken functions, and valid longjmp targets.
+// Guard, including address-taken functions and valid longjmp targets.
 //
 //===----------------------------------------------------------------------===//
 
 #include "WinCFGuard.h"
 #include "llvm/CodeGen/AsmPrinter.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/IR/Constants.h"
-#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCStreamer.h"
 
 #include <vector>
 
 using namespace llvm;
 
for (const Use &U : FnOrCast->uses()) {
         else
           return true;
       }
     }
   }
   return false;
 }
 
+/// Returns true if this function should be added to the Guard Address Taken IAT
+/// Entry Table (GIATs) instead of the Guard Function ID Table (GFIDs).
+static bool isIATAddressTaken(const Function *F) {
+  if (F->hasDLLImportStorageClass()) {
+    return true;
+  }
+  return false;
+}
+
 void WinCFGuard::endModule() {
   const Module *M = Asm->MMI->getModule();
-  std::vector<const Function *> Functions;
-  for (const Function &F : *M)
-    if (isPossibleIndirectCallTarget(&F))
-      Functions.push_back(&F);
-  if (Functions.empty() && LongjmpTargets.empty())
+  std::vector<const Function *> GFIDsEntries;
+  std::vector<const Function *> GIATsEntries;
+  for (const Function &F : *M) {
+    if (isPossibleIndirectCallTarget(&F)) {
+      if (isIATAddressTaken(&F)) {
+        // If the possible call target is reached via the IAT, add it to the
+        // GIATs table instead of the GFIDs table.
+        GIATsEntries.push_back(&F);
+      } else {
+        // Otherwise add it to the GFIDs table.
+        GFIDsEntries.push_back(&F);
+      }
+    }
+  }
+
+  if (GFIDsEntries.empty() && GIATsEntries.empty() && LongjmpTargets.empty())
     return;
+
+  // Emit the symbol index of each GFIDs entry to form the GFIDs table.
   auto &OS = *Asm->OutStreamer;
   OS.SwitchSection(Asm->OutContext.getObjectFileInfo()->getGFIDsSection());
-  for (const Function *F : Functions)
+  for (const Function *F : GFIDsEntries)
     OS.EmitCOFFSymbolIndex(Asm->getSymbol(F));
 
-  // Emit the symbol index of each longjmp target.
+  // Emit the symbol index of each GIATs entry to form the GIATs table.
+  OS.SwitchSection(Asm->OutContext.getObjectFileInfo()->getGIATsSection());
+  for (const Function *F : GIATsEntries) {
+    OS.EmitCOFFSymbolIndex(Asm->OutContext.getOrCreateSymbol(
+        Twine("__imp_") + Asm->getSymbol(F)->getName()));
+  }
+
+  // Emit the symbol index of each longjmp target to form the GLJMP table.
   OS.SwitchSection(Asm->OutContext.getObjectFileInfo()->getGLJMPSection());
   for (const MCSymbol *S : LongjmpTargets) {
     OS.EmitCOFFSymbolIndex(S);
   }
 }

llvm/lib/MC/MCObjectFileInfo.cpp

void MCObjectFileInfo::initCOFFMCObjectFileInfo(const Triple &T) {
   SXDataSection = Ctx->getCOFFSection(".sxdata", COFF::IMAGE_SCN_LNK_INFO,
                                       SectionKind::getMetadata());
 
   GFIDsSection = Ctx->getCOFFSection(".gfids$y",
                                      COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
                                          COFF::IMAGE_SCN_MEM_READ,
                                      SectionKind::getMetadata());
 
+  GIATsSection = Ctx->getCOFFSection(".giats$y",
+                                     COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+                                         COFF::IMAGE_SCN_MEM_READ,
+                                     SectionKind::getMetadata());
+
   GLJMPSection = Ctx->getCOFFSection(".gljmp$y",
                                      COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
                                          COFF::IMAGE_SCN_MEM_READ,
                                      SectionKind::getMetadata());
 
   TLSDataSection = Ctx->getCOFFSection(
       ".tls$", COFF::IMAGE_SCN_CNT_INITIALIZED_DATA | COFF::IMAGE_SCN_MEM_READ |
                    COFF::IMAGE_SCN_MEM_WRITE,

llvm/test/CodeGen/WinCFGuard/cfguard-giats.ll

+; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s
+; Control Flow Guard is currently only available on Windows
+
+declare dllimport i32 @target_func()
+
+; Test address-taken functions from imported DLLs are added to the 
+; Guard Address-Taken IAT Entry table (.giats).
+define i32 @func_cf_giats() {
+entry:
+  %func_ptr = alloca i32 ()*, align 8
+  store i32 ()* @target_func, i32 ()** %func_ptr, align 8
+  %0 = load i32 ()*, i32 ()** %func_ptr, align 8
+  %1 = call i32 %0()
+  ret i32 %1
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"cfguard", i32 2}
+
+; CHECK-LABEL: .section .giats$y,"dr"
+; CHECK-NEXT:  .symidx __imp_target_func
+; CHECK-NOT:   .symidx
 No newline at end of file

llvm/tools/llvm-readobj/COFFDumper.cpp

 namespace {
 
 struct LoadConfigTables {
   uint64_t SEHTableVA = 0;
   uint64_t SEHTableCount = 0;
   uint32_t GuardFlags = 0;
   uint64_t GuardFidTableVA = 0;
   uint64_t GuardFidTableCount = 0;
+  uint64_t GuardIatTableVA = 0;
+  uint64_t GuardIatTableCount = 0;
   uint64_t GuardLJmpTableVA = 0;
   uint64_t GuardLJmpTableCount = 0;
 };
 
 class COFFDumper : public ObjDumper {
 public:
   friend class COFFObjectDumpDelegate;
   COFFDumper(const llvm::object::COFFObjectFile *Obj, ScopedPrinter &Writer)
if (Tables.GuardFlags & uint32_t(coff_guard_flags::FidTableHasFlags)) {
       };
       printRVATable(Tables.GuardFidTableVA, Tables.GuardFidTableCount, 5,
                     PrintGuardFlags);
     } else {
       printRVATable(Tables.GuardFidTableVA, Tables.GuardFidTableCount, 4);
     }
   }
 
+  if (Tables.GuardIatTableVA) {
+    ListScope LS(W, "GuardIatTable");
+    printRVATable(Tables.GuardIatTableVA, Tables.GuardIatTableCount, 4);
+  }
+
   if (Tables.GuardLJmpTableVA) {
     ListScope LS(W, "GuardLJmpTable");
     printRVATable(Tables.GuardLJmpTableVA, Tables.GuardLJmpTableCount, 4);
   }
 }
 
 template <typename T>
 void COFFDumper::printCOFFLoadConfig(const T *Conf, LoadConfigTables &Tables) {
void COFFDumper::printCOFFLoadConfig(const T *Conf, LoadConfigTables &Tables) {
   W.printHex("DynamicValueRelocTableOffset",
              Conf->DynamicValueRelocTableOffset);
   W.printNumber("DynamicValueRelocTableSection",
                 Conf->DynamicValueRelocTableSection);
   W.printHex("GuardRFVerifyStackPointerFunctionPointer",
              Conf->GuardRFVerifyStackPointerFunctionPointer);
   W.printHex("HotPatchTableOffset", Conf->HotPatchTableOffset);
 
+  Tables.GuardIatTableVA = Conf->GuardAddressTakenIatEntryTable;
+  Tables.GuardIatTableCount = Conf->GuardAddressTakenIatEntryCount;
+
   Tables.GuardLJmpTableVA = Conf->GuardLongJumpTargetTable;
   Tables.GuardLJmpTableCount = Conf->GuardLongJumpTargetCount;
 }
 
 void COFFDumper::printBaseOfDataField(const pe32_header *Hdr) {
   W.printHex("BaseOfData", Hdr->BaseOfData);
 }