Skip to content

Commit 418f18c

Browse files
committed
Revert "Reland [CFGuard] Add address-taken IAT tables and delay-load 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.
1 parent 138189e commit 418f18c

File tree

12 files changed

+18
-259
lines changed

12 files changed

+18
-259
lines changed

lld/COFF/DLL.cpp

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
#include "DLL.h"
2121
#include "Chunks.h"
22-
#include "SymbolTable.h"
2322
#include "llvm/Object/COFF.h"
2423
#include "llvm/Support/Endian.h"
2524
#include "llvm/Support/Path.h"
@@ -654,18 +653,9 @@ void DelayLoadContents::create(Defined *h) {
654653
auto *c = make<HintNameChunk>(extName, 0);
655654
names.push_back(make<LookupChunk>(c));
656655
hintNames.push_back(c);
657-
// Add a syntentic symbol for this load thunk, using the "__imp_load"
658-
// prefix, in case this thunk needs to be added to the list of valid
659-
// call targets for Control Flow Guard.
660-
StringRef symName = saver.save("__imp_load_" + extName);
661-
s->loadThunkSym =
662-
cast<DefinedSynthetic>(symtab->addSynthetic(symName, t));
663656
}
664657
}
665658
thunks.push_back(tm);
666-
StringRef tmName =
667-
saver.save("__tailMerge_" + syms[0]->getDLLName().lower());
668-
symtab->addSynthetic(tmName, tm);
669659
// Terminate with null values.
670660
addresses.push_back(make<NullChunk>(8));
671661
names.push_back(make<NullChunk>(8));

lld/COFF/ICF.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ bool ICF::assocEquals(const SectionChunk *a, const SectionChunk *b) {
131131
auto considerForICF = [](const SectionChunk &assoc) {
132132
StringRef Name = assoc.getSectionName();
133133
return !(Name.startswith(".debug") || Name == ".gfids$y" ||
134-
Name == ".giats$y" || Name == ".gljmp$y");
134+
Name == ".gljmp$y");
135135
};
136136
auto ra = make_filter_range(a->children(), considerForICF);
137137
auto rb = make_filter_range(b->children(), considerForICF);

lld/COFF/InputFiles.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,6 @@ SectionChunk *ObjFile::readSection(uint32_t sectionNumber,
280280
debugChunks.push_back(c);
281281
else if (name == ".gfids$y")
282282
guardFidChunks.push_back(c);
283-
else if (name == ".giats$y")
284-
guardIATChunks.push_back(c);
285283
else if (name == ".gljmp$y")
286284
guardLJmpChunks.push_back(c);
287285
else if (name == ".sxdata")

lld/COFF/InputFiles.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ class ObjFile : public InputFile {
144144
ArrayRef<SectionChunk *> getDebugChunks() { return debugChunks; }
145145
ArrayRef<SectionChunk *> getSXDataChunks() { return sxDataChunks; }
146146
ArrayRef<SectionChunk *> getGuardFidChunks() { return guardFidChunks; }
147-
ArrayRef<SectionChunk *> getGuardIATChunks() { return guardIATChunks; }
148147
ArrayRef<SectionChunk *> getGuardLJmpChunks() { return guardLJmpChunks; }
149148
ArrayRef<Symbol *> getSymbols() { return symbols; }
150149

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

289-
// Chunks containing symbol table indices of address taken symbols, address
290-
// taken IAT entries, and longjmp targets. These are not linked into the
291-
// final binary when /guard:cf is set.
288+
// Chunks containing symbol table indices of address taken symbols and longjmp
289+
// targets. These are not linked into the final binary when /guard:cf is set.
292290
std::vector<SectionChunk *> guardFidChunks;
293-
std::vector<SectionChunk *> guardIATChunks;
294291
std::vector<SectionChunk *> guardLJmpChunks;
295292

296293
// This vector contains a list of all symbols defined or referenced by this

lld/COFF/Symbols.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -353,13 +353,6 @@ class DefinedImportData : public Defined {
353353
uint16_t getOrdinal() { return file->hdr->OrdinalHint; }
354354

355355
ImportFile *file;
356-
357-
// This is a pointer to the synthetic symbol associated with the load thunk
358-
// for this symbol that will be called if the DLL is delay-loaded. This is
359-
// needed for Control Flow Guard because if this DefinedImportData symbol is a
360-
// valid call target, the corresponding load thunk must also be marked as a
361-
// valid call target.
362-
DefinedSynthetic *loadThunkSym = nullptr;
363356
};
364357

365358
// This class represents a symbol for a jump table entry which jumps

lld/COFF/Writer.cpp

Lines changed: 6 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,6 @@ class Writer {
227227
void markSymbolsForRVATable(ObjFile *file,
228228
ArrayRef<SectionChunk *> symIdxChunks,
229229
SymbolRVASet &tableSymbols);
230-
void getSymbolsFromSections(ObjFile *file,
231-
ArrayRef<SectionChunk *> symIdxChunks,
232-
std::vector<Symbol *> &symbols);
233230
void maybeAddRVATable(SymbolRVASet tableSymbols, StringRef tableSym,
234231
StringRef countSym);
235232
void setSectionPermissions();
@@ -610,9 +607,8 @@ void Writer::run() {
610607

611608
createImportTables();
612609
createSections();
613-
appendImportThunks();
614-
// Import thunks must be added before the Control Flow Guard tables are added.
615610
createMiscChunks();
611+
appendImportThunks();
616612
createExportTable();
617613
mergeSections();
618614
removeUnusedSections();
@@ -1632,8 +1628,6 @@ static void markSymbolsWithRelocations(ObjFile *file,
16321628
// table.
16331629
void Writer::createGuardCFTables() {
16341630
SymbolRVASet addressTakenSyms;
1635-
SymbolRVASet giatsRVASet;
1636-
std::vector<Symbol *> giatsSymbols;
16371631
SymbolRVASet longJmpTargets;
16381632
for (ObjFile *file : ObjFile::instances) {
16391633
// If the object was compiled with /guard:cf, the address taken symbols
@@ -1643,8 +1637,6 @@ void Writer::createGuardCFTables() {
16431637
// possibly address-taken.
16441638
if (file->hasGuardCF()) {
16451639
markSymbolsForRVATable(file, file->getGuardFidChunks(), addressTakenSyms);
1646-
markSymbolsForRVATable(file, file->getGuardIATChunks(), giatsRVASet);
1647-
getSymbolsFromSections(file, file->getGuardIATChunks(), giatsSymbols);
16481640
markSymbolsForRVATable(file, file->getGuardLJmpChunks(), longJmpTargets);
16491641
} else {
16501642
markSymbolsWithRelocations(file, addressTakenSyms);
@@ -1659,16 +1651,6 @@ void Writer::createGuardCFTables() {
16591651
for (Export &e : config->exports)
16601652
maybeAddAddressTakenFunction(addressTakenSyms, e.sym);
16611653

1662-
// For each entry in the .giats table, check if it has a corresponding load
1663-
// thunk (e.g. because the DLL that defines it will be delay-loaded) and, if
1664-
// so, add the load thunk to the address taken (.gfids) table.
1665-
for (Symbol *s : giatsSymbols) {
1666-
if (auto *di = dyn_cast<DefinedImportData>(s)) {
1667-
if (di->loadThunkSym)
1668-
addSymbolToRVASet(addressTakenSyms, di->loadThunkSym);
1669-
}
1670-
}
1671-
16721654
// Ensure sections referenced in the gfid table are 16-byte aligned.
16731655
for (const ChunkAndOffset &c : addressTakenSyms)
16741656
if (c.inputChunk->getAlignment() < 16)
@@ -1677,10 +1659,6 @@ void Writer::createGuardCFTables() {
16771659
maybeAddRVATable(std::move(addressTakenSyms), "__guard_fids_table",
16781660
"__guard_fids_count");
16791661

1680-
// Add the Guard Address Taken IAT Entry Table (.giats).
1681-
maybeAddRVATable(std::move(giatsRVASet), "__guard_iat_table",
1682-
"__guard_iat_count");
1683-
16841662
// Add the longjmp target table unless the user told us not to.
16851663
if (config->guardCF == GuardCFLevel::Full)
16861664
maybeAddRVATable(std::move(longJmpTargets), "__guard_longjmp_table",
@@ -1697,11 +1675,11 @@ void Writer::createGuardCFTables() {
16971675
}
16981676

16991677
// Take a list of input sections containing symbol table indices and add those
1700-
// symbols to a vector. The challenge is that symbol RVAs are not known and
1678+
// symbols to an RVA table. The challenge is that symbol RVAs are not known and
17011679
// depend on the table size, so we can't directly build a set of integers.
1702-
void Writer::getSymbolsFromSections(ObjFile *file,
1680+
void Writer::markSymbolsForRVATable(ObjFile *file,
17031681
ArrayRef<SectionChunk *> symIdxChunks,
1704-
std::vector<Symbol *> &symbols) {
1682+
SymbolRVASet &tableSymbols) {
17051683
for (SectionChunk *c : symIdxChunks) {
17061684
// Skip sections discarded by linker GC. This comes up when a .gfids section
17071685
// is associated with something like a vtable and the vtable is discarded.
@@ -1719,7 +1697,7 @@ void Writer::getSymbolsFromSections(ObjFile *file,
17191697
}
17201698

17211699
// Read each symbol table index and check if that symbol was included in the
1722-
// final link. If so, add it to the vector of symbols.
1700+
// final link. If so, add it to the table symbol set.
17231701
ArrayRef<ulittle32_t> symIndices(
17241702
reinterpret_cast<const ulittle32_t *>(data.data()), data.size() / 4);
17251703
ArrayRef<Symbol *> objSymbols = file->getSymbols();
@@ -1731,24 +1709,12 @@ void Writer::getSymbolsFromSections(ObjFile *file,
17311709
}
17321710
if (Symbol *s = objSymbols[symIndex]) {
17331711
if (s->isLive())
1734-
symbols.push_back(cast<Symbol>(s));
1712+
addSymbolToRVASet(tableSymbols, cast<Defined>(s));
17351713
}
17361714
}
17371715
}
17381716
}
17391717

1740-
// Take a list of input sections containing symbol table indices and add those
1741-
// symbols to an RVA table.
1742-
void Writer::markSymbolsForRVATable(ObjFile *file,
1743-
ArrayRef<SectionChunk *> symIdxChunks,
1744-
SymbolRVASet &tableSymbols) {
1745-
std::vector<Symbol *> syms;
1746-
getSymbolsFromSections(file, symIdxChunks, syms);
1747-
1748-
for (Symbol *s : syms)
1749-
addSymbolToRVASet(tableSymbols, cast<Defined>(s));
1750-
}
1751-
17521718
// Replace the absolute table symbol with a synthetic symbol pointing to
17531719
// tableChunk so that we can emit base relocations for it and resolve section
17541720
// relative relocations.

lld/test/COFF/giats.s

Lines changed: 0 additions & 117 deletions
This file was deleted.

llvm/include/llvm/MC/MCObjectFileInfo.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,6 @@ class MCObjectFileInfo {
215215
MCSection *XDataSection = nullptr;
216216
MCSection *SXDataSection = nullptr;
217217
MCSection *GFIDsSection = nullptr;
218-
MCSection *GIATsSection = nullptr;
219218
MCSection *GLJMPSection = nullptr;
220219

221220
// XCOFF specific sections
@@ -398,7 +397,6 @@ class MCObjectFileInfo {
398397
MCSection *getXDataSection() const { return XDataSection; }
399398
MCSection *getSXDataSection() const { return SXDataSection; }
400399
MCSection *getGFIDsSection() const { return GFIDsSection; }
401-
MCSection *getGIATsSection() const { return GIATsSection; }
402400
MCSection *getGLJMPSection() const { return GLJMPSection; }
403401

404402
// XCOFF specific sections

0 commit comments

Comments
 (0)