Skip to content

Commit

Permalink
Revert "Reland [CFGuard] Add address-taken IAT tables and delay-load …
Browse files Browse the repository at this point in the history 
…support"

This broke both Firefox and Chromium (PR47905) due to what seems like dllimport
function not being handled correctly.

> 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.
>
> Reviewed By: rnk
>
> Differential Revision: https://reviews.llvm.org/D87544

This reverts commit cfd8481.
  • Loading branch information
@zmodem
zmodem committed Nov 11, 2020
1 parent 138189e commit 418f18c
Show file tree
Hide file tree
Showing 12 changed files with 18 additions and 259 deletions.
10 changes: 0 additions & 10 deletions lld/COFF/DLL.cpp
View file
Expand Up @@ -19,7 +19,6 @@

#include "DLL.h"
#include "Chunks.h"
#include "SymbolTable.h"
#include "llvm/Object/COFF.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/Path.h"
Expand Down Expand Up @@ -654,18 +653,9 @@ void DelayLoadContents::create(Defined *h) {
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));
Expand Down
2 changes: 1 addition & 1 deletion lld/COFF/ICF.cpp
View file
Expand Up @@ -131,7 +131,7 @@ bool ICF::assocEquals(const SectionChunk *a, const SectionChunk *b) {
auto considerForICF = [](const SectionChunk &assoc) {
StringRef Name = assoc.getSectionName();
return !(Name.startswith(".debug") || Name == ".gfids$y" ||
Name == ".giats$y" || Name == ".gljmp$y");
Name == ".gljmp$y");
};
auto ra = make_filter_range(a->children(), considerForICF);
auto rb = make_filter_range(b->children(), considerForICF);
Expand Down
2 changes: 0 additions & 2 deletions lld/COFF/InputFiles.cpp
View file
Expand Up @@ -280,8 +280,6 @@ SectionChunk *ObjFile::readSection(uint32_t sectionNumber,
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")
Expand Down
7 changes: 2 additions & 5 deletions lld/COFF/InputFiles.h
View file
Expand Up @@ -144,7 +144,6 @@ class ObjFile : public InputFile {
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; }

Expand Down Expand Up @@ -286,11 +285,9 @@ class ObjFile : public InputFile {
// 32-bit x86.
std::vector<SectionChunk *> sxDataChunks;

// 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.
// 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.
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
Expand Down
7 changes: 0 additions & 7 deletions lld/COFF/Symbols.h
View file
Expand Up @@ -353,13 +353,6 @@ class DefinedImportData : public Defined {
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
Expand Down
46 changes: 6 additions & 40 deletions lld/COFF/Writer.cpp
View file
Expand Up @@ -227,9 +227,6 @@ class Writer {
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();
Expand Down Expand Up @@ -610,9 +607,8 @@ void Writer::run() {

createImportTables();
createSections();
appendImportThunks();
// Import thunks must be added before the Control Flow Guard tables are added.
createMiscChunks();
appendImportThunks();
createExportTable();
mergeSections();
removeUnusedSections();
Expand Down Expand Up @@ -1632,8 +1628,6 @@ static void markSymbolsWithRelocations(ObjFile *file,
// 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
Expand All @@ -1643,8 +1637,6 @@ void Writer::createGuardCFTables() {
// 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);
Expand All @@ -1659,16 +1651,6 @@ void Writer::createGuardCFTables() {
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)
Expand All @@ -1677,10 +1659,6 @@ void Writer::createGuardCFTables() {
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",
Expand All @@ -1697,11 +1675,11 @@ void Writer::createGuardCFTables() {
}

// Take a list of input sections containing symbol table indices and add those
// symbols to a vector. The challenge is that symbol RVAs are not known and
// 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::getSymbolsFromSections(ObjFile *file,
void Writer::markSymbolsForRVATable(ObjFile *file,
ArrayRef<SectionChunk *> symIdxChunks,
std::vector<Symbol *> &symbols) {
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.
Expand All @@ -1719,7 +1697,7 @@ void Writer::getSymbolsFromSections(ObjFile *file,
}

// Read each symbol table index and check if that symbol was included in the
// final link. If so, add it to the vector of symbols.
// 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();
Expand All @@ -1731,24 +1709,12 @@ void Writer::getSymbolsFromSections(ObjFile *file,
}
if (Symbol *s = objSymbols[symIndex]) {
if (s->isLive())
symbols.push_back(cast<Symbol>(s));
addSymbolToRVASet(tableSymbols, cast<Defined>(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.
Expand Down
117 changes: 0 additions & 117 deletions lld/test/COFF/giats.s
View file

This file was deleted.

2 changes: 0 additions & 2 deletions llvm/include/llvm/MC/MCObjectFileInfo.h
View file
Expand Up @@ -215,7 +215,6 @@ class MCObjectFileInfo {
MCSection *XDataSection = nullptr;
MCSection *SXDataSection = nullptr;
MCSection *GFIDsSection = nullptr;
MCSection *GIATsSection = nullptr;
MCSection *GLJMPSection = nullptr;

// XCOFF specific sections
Expand Down Expand Up @@ -398,7 +397,6 @@ class MCObjectFileInfo {
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
Expand Down

0 comments on commit 418f18c

Please sign in to comment.