diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp --- a/lld/COFF/DLL.cpp +++ b/lld/COFF/DLL.cpp @@ -19,6 +19,7 @@ #include "DLL.h" #include "Chunks.h" +#include "SymbolTable.h" #include "llvm/Object/COFF.h" #include "llvm/Support/Endian.h" #include "llvm/Support/Path.h" @@ -653,9 +654,18 @@ auto *c = make(extName, 0); names.push_back(make(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(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(8)); names.push_back(make(8)); diff --git a/lld/COFF/ICF.cpp b/lld/COFF/ICF.cpp --- a/lld/COFF/ICF.cpp +++ b/lld/COFF/ICF.cpp @@ -131,7 +131,7 @@ 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); diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h --- a/lld/COFF/InputFiles.h +++ b/lld/COFF/InputFiles.h @@ -144,6 +144,7 @@ ArrayRef getDebugChunks() { return debugChunks; } ArrayRef getSXDataChunks() { return sxDataChunks; } ArrayRef getGuardFidChunks() { return guardFidChunks; } + ArrayRef getGuardIATChunks() { return guardIATChunks; } ArrayRef getGuardLJmpChunks() { return guardLJmpChunks; } ArrayRef getSymbols() { return symbols; } @@ -285,9 +286,11 @@ // 32-bit x86. std::vector 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 guardFidChunks; + std::vector guardIATChunks; std::vector guardLJmpChunks; // This vector contains a list of all symbols defined or referenced by this diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -280,6 +280,8 @@ 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") diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h --- a/lld/COFF/Symbols.h +++ b/lld/COFF/Symbols.h @@ -353,6 +353,13 @@ 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 = nullptr; }; // This class represents a symbol for a jump table entry which jumps diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp --- a/lld/COFF/Writer.cpp +++ b/lld/COFF/Writer.cpp @@ -227,6 +227,9 @@ void markSymbolsForRVATable(ObjFile *file, ArrayRef symIdxChunks, SymbolRVASet &tableSymbols); + void getSymbolsFromSections(ObjFile *file, + ArrayRef symIdxChunks, + std::vector &symbols); void maybeAddRVATable(SymbolRVASet tableSymbols, StringRef tableSym, StringRef countSym); void setSectionPermissions(); @@ -607,8 +610,9 @@ createImportTables(); createSections(); - createMiscChunks(); appendImportThunks(); + // Import thunks must be added before the Control Flow Guard tables are added. + createMiscChunks(); createExportTable(); mergeSections(); removeUnusedSections(); @@ -1628,6 +1632,8 @@ // table. void Writer::createGuardCFTables() { SymbolRVASet addressTakenSyms; + SymbolRVASet giatsRVASet; + std::vector giatsSymbols; SymbolRVASet longJmpTargets; for (ObjFile *file : ObjFile::instances) { // If the object was compiled with /guard:cf, the address taken symbols @@ -1637,6 +1643,8 @@ // 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); @@ -1651,6 +1659,16 @@ 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(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) @@ -1659,6 +1677,10 @@ 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", @@ -1675,11 +1697,11 @@ } // 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 symIdxChunks, - SymbolRVASet &tableSymbols) { + std::vector &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. @@ -1697,7 +1719,7 @@ } // 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 symIndices( reinterpret_cast(data.data()), data.size() / 4); ArrayRef objSymbols = file->getSymbols(); @@ -1709,12 +1731,24 @@ } if (Symbol *s = objSymbols[symIndex]) { if (s->isLive()) - addSymbolToRVASet(tableSymbols, cast(s)); + symbols.push_back(cast(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 symIdxChunks, + SymbolRVASet &tableSymbols) { + std::vector syms; + getSymbolsFromSections(file, symIdxChunks, syms); + + for (Symbol *s : syms) + addSymbolToRVASet(tableSymbols, cast(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. diff --git a/lld/test/COFF/giats.s b/lld/test/COFF/giats.s new file mode 100644 --- /dev/null +++ b/lld/test/COFF/giats.s @@ -0,0 +1,117 @@ +# 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 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 diff --git a/llvm/include/llvm/MC/MCObjectFileInfo.h b/llvm/include/llvm/MC/MCObjectFileInfo.h --- a/llvm/include/llvm/MC/MCObjectFileInfo.h +++ b/llvm/include/llvm/MC/MCObjectFileInfo.h @@ -215,6 +215,7 @@ MCSection *XDataSection = nullptr; MCSection *SXDataSection = nullptr; MCSection *GFIDsSection = nullptr; + MCSection *GIATsSection = nullptr; MCSection *GLJMPSection = nullptr; // XCOFF specific sections @@ -397,6 +398,7 @@ 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 diff --git a/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h b/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h --- a/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h +++ b/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h @@ -24,6 +24,7 @@ /// Target of directive emission. AsmPrinter *Asm; std::vector LongjmpTargets; + MCSymbol *lookupImpSymbol(const MCSymbol *Sym); public: WinCFGuard(AsmPrinter *A); diff --git a/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp b/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp --- a/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp @@ -7,7 +7,7 @@ //===----------------------------------------------------------------------===// // // 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. // //===----------------------------------------------------------------------===// @@ -17,8 +17,8 @@ #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" @@ -78,20 +78,50 @@ return false; } +MCSymbol *WinCFGuard::lookupImpSymbol(const MCSymbol *Sym) { + if (Sym->getName().startswith("__imp_")) + return nullptr; + return Asm->OutContext.lookupSymbol(Twine("__imp_") + Sym->getName()); +} + void WinCFGuard::endModule() { const Module *M = Asm->MMI->getModule(); - std::vector Functions; - for (const Function &F : *M) - if (isPossibleIndirectCallTarget(&F)) - Functions.push_back(&F); - if (Functions.empty() && LongjmpTargets.empty()) + std::vector GFIDsEntries; + std::vector GIATsEntries; + for (const Function &F : *M) { + if (isPossibleIndirectCallTarget(&F)) { + // If F is a dllimport and has an "__imp_" symbol already defined, add the + // "__imp_" symbol to the .giats section. + if (F.hasDLLImportStorageClass()) { + if (MCSymbol *impSym = lookupImpSymbol(Asm->getSymbol(&F))) { + GIATsEntries.push_back(impSym); + } + } + // Add the function's symbol to the .gfids section. + // Note: For dllimport functions, MSVC sometimes does not add this symbol + // to the .gfids section, but only adds the corresponding "__imp_" symbol + // to the .giats section. Here we always add the symbol to the .gfids + // section, since this does not introduce security risks. + GFIDsEntries.push_back(Asm->getSymbol(&F)); + } + } + + if (GFIDsEntries.empty() && GIATsEntries.empty() && LongjmpTargets.empty()) return; + + // Emit the symbol index of each GFIDs entry to form the .gfids section. auto &OS = *Asm->OutStreamer; OS.SwitchSection(Asm->OutContext.getObjectFileInfo()->getGFIDsSection()); - for (const Function *F : Functions) - OS.EmitCOFFSymbolIndex(Asm->getSymbol(F)); + for (const MCSymbol *S : GFIDsEntries) + OS.EmitCOFFSymbolIndex(S); + + // Emit the symbol index of each GIATs entry to form the .giats section. + OS.SwitchSection(Asm->OutContext.getObjectFileInfo()->getGIATsSection()); + for (const MCSymbol *S : GIATsEntries) { + OS.EmitCOFFSymbolIndex(S); + } - // Emit the symbol index of each longjmp target. + // Emit the symbol index of each longjmp target to form the .gljmp section. OS.SwitchSection(Asm->OutContext.getObjectFileInfo()->getGLJMPSection()); for (const MCSymbol *S : LongjmpTargets) { OS.EmitCOFFSymbolIndex(S); diff --git a/llvm/lib/MC/MCObjectFileInfo.cpp b/llvm/lib/MC/MCObjectFileInfo.cpp --- a/llvm/lib/MC/MCObjectFileInfo.cpp +++ b/llvm/lib/MC/MCObjectFileInfo.cpp @@ -754,6 +754,11 @@ 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, diff --git a/llvm/test/CodeGen/WinCFGuard/cfguard-giats.ll b/llvm/test/CodeGen/WinCFGuard/cfguard-giats.ll new file mode 100644 --- /dev/null +++ b/llvm/test/CodeGen/WinCFGuard/cfguard-giats.ll @@ -0,0 +1,44 @@ +; RUN: llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s +; Control Flow Guard is currently only available on Windows + +declare dllimport i32 @target_func1() +declare dllimport i32 @target_func2() +declare dllimport i32 @target_func3() +@ptrs = dso_local local_unnamed_addr global [2 x void ()*] [void ()* bitcast (i32 ()* @target_func2 to void ()*), void ()* bitcast (i32 ()* @target_func3 to void ()*)], align 16 + +; Test address-taken functions from imported DLLs are correctly added to the +; Guard Address-Taken IAT Entry (.giats) and Guard Function ID (.gfids) sections. +define i32 @func_cf_giats1() { +entry: + ; Since it is a dllimport, target_func1 will be represented as "__imp_target_func1" when it is + ; stored in the function pointer. Therefore, the .giats section must contain "__imp_target_func1". + ; Unlike MSVC, we also have "target_func1" in the .gfids section, since this is not a security risk. + %func_ptr = alloca i32 ()*, align 8 + store i32 ()* @target_func1, i32 ()** %func_ptr, align 8 + %0 = load i32 ()*, i32 ()** %func_ptr, align 8 + %1 = call i32 %0() + ; target_func2 is called directly from a global array, so should only appear in the .gfids section. + %2 = load i32 ()*, i32 ()** bitcast ([2 x void ()*]* @ptrs to i32 ()**), align 8 + %3 = call i32 %2() + ; target_func3 is called both via a stored function pointer (as with target_func1) and via a gloabl + ; array (as with target_func2), so "target_func3" must appear in .gfids and "__imp_target_func3" in .giats. + store i32 ()* @target_func3, i32 ()** %func_ptr, align 8 + %4 = load i32 ()*, i32 ()** %func_ptr, align 8 + %5 = call i32 %4() + %6 = load i32 ()*, i32 ()** bitcast (void ()** getelementptr inbounds ([2 x void ()*], [2 x void ()*]* @ptrs, i64 0, i64 1) to i32 ()**), align 8 + %7 = call i32 %6() + ret i32 %5 +} + +; CHECK-LABEL: .section .gfids$y,"dr" +; CHECK-NEXT: .symidx target_func1 +; CHECK-NEXT: .symidx target_func2 +; CHECK-NEXT: .symidx target_func3 +; CHECK-NOT: .symidx +; CHECK-LABEL: .section .giats$y,"dr" +; CHECK-NEXT: .symidx __imp_target_func1 +; CHECK-NEXT: .symidx __imp_target_func3 +; CHECK-NOT: .symidx + +!llvm.module.flags = !{!0} +!0 = !{i32 2, !"cfguard", i32 2} diff --git a/llvm/tools/llvm-readobj/COFFDumper.cpp b/llvm/tools/llvm-readobj/COFFDumper.cpp --- a/llvm/tools/llvm-readobj/COFFDumper.cpp +++ b/llvm/tools/llvm-readobj/COFFDumper.cpp @@ -67,6 +67,8 @@ 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; }; @@ -807,6 +809,11 @@ } } + 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); @@ -891,6 +898,9 @@ Conf->GuardRFVerifyStackPointerFunctionPointer); W.printHex("HotPatchTableOffset", Conf->HotPatchTableOffset); + Tables.GuardIatTableVA = Conf->GuardAddressTakenIatEntryTable; + Tables.GuardIatTableCount = Conf->GuardAddressTakenIatEntryCount; + Tables.GuardLJmpTableVA = Conf->GuardLongJumpTargetTable; Tables.GuardLJmpTableCount = Conf->GuardLongJumpTargetCount; }