Skip to content

Commit

Permalink
Re-land [LLD] Remove global state in lldCommon
Browse files Browse the repository at this point in the history 
Move all variables at file-scope or function-static-scope into a hosting structure (lld::CommonLinkerContext) that lives at lldMain()-scope. Drivers will inherit from this structure and add their own global state, in the same way as for the existing COFFLinkerContext.

See discussion in https://lists.llvm.org/pipermail/llvm-dev/2021-June/151184.html

The previous land f860fe3 caused issues in https://lab.llvm.org/buildbot/#/builders/123/builds/8383, fixed by 22ee510.

Differential Revision: https://reviews.llvm.org/D108850
  • Loading branch information
@aganea
aganea committed Jan 20, 2022
1 parent 57ebfea commit 83d59e0
Show file tree
Hide file tree
Showing 49 changed files with 522 additions and 400 deletions.
7 changes: 7 additions & 0 deletions clang/lib/Driver/ToolChains/MSVC.cpp
View file
Expand Up @@ -47,7 +47,14 @@
// Make sure this comes before MSVCSetupApi.h
#include <comdef.h>

#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wnon-virtual-dtor"
#endif
#include "MSVCSetupApi.h"
#ifdef __clang__
#pragma clang diagnostic pop
#endif
#include "llvm/Support/COM.h"
_COM_SMARTPTR_TYPEDEF(ISetupConfiguration, __uuidof(ISetupConfiguration));
_COM_SMARTPTR_TYPEDEF(ISetupConfiguration2, __uuidof(ISetupConfiguration2));
Expand Down
3 changes: 2 additions & 1 deletion lld/COFF/COFFLinkerContext.h
View file
Expand Up @@ -15,12 +15,13 @@
#include "InputFiles.h"
#include "SymbolTable.h"
#include "Writer.h"
#include "lld/Common/CommonLinkerContext.h"
#include "lld/Common/Timer.h"

namespace lld {
namespace coff {

class COFFLinkerContext {
class COFFLinkerContext : public CommonLinkerContext {
public:
COFFLinkerContext();
COFFLinkerContext(const COFFLinkerContext &) = delete;
Expand Down
3 changes: 1 addition & 2 deletions lld/COFF/Chunks.cpp
View file
Expand Up @@ -12,7 +12,6 @@
#include "SymbolTable.h"
#include "Symbols.h"
#include "Writer.h"
#include "lld/Common/ErrorHandler.h"
#include "llvm/ADT/Twine.h"
#include "llvm/BinaryFormat/COFF.h"
#include "llvm/Object/COFF.h"
Expand Down Expand Up @@ -430,7 +429,7 @@ void SectionChunk::sortRelocations() {
return;
warn("some relocations in " + file->getName() + " are not sorted");
MutableArrayRef<coff_relocation> newRelocs(
bAlloc.Allocate<coff_relocation>(relocsSize), relocsSize);
bAlloc().Allocate<coff_relocation>(relocsSize), relocsSize);
memcpy(newRelocs.data(), relocsData, relocsSize * sizeof(coff_relocation));
llvm::sort(newRelocs, cmpByVa);
setRelocs(newRelocs);
Expand Down
4 changes: 2 additions & 2 deletions lld/COFF/DLL.cpp
View file
Expand Up @@ -659,14 +659,14 @@ void DelayLoadContents::create(COFFLinkerContext &ctx, Defined *h) {
// 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);
StringRef symName = saver().save("__imp_load_" + extName);
s->loadThunkSym =
cast<DefinedSynthetic>(ctx.symtab.addSynthetic(symName, t));
}
}
thunks.push_back(tm);
StringRef tmName =
saver.save("__tailMerge_" + syms[0]->getDLLName().lower());
saver().save("__tailMerge_" + syms[0]->getDLLName().lower());
ctx.symtab.addSynthetic(tmName, tm);
// Terminate with null values.
addresses.push_back(make<NullChunk>(8));
Expand Down
70 changes: 27 additions & 43 deletions lld/COFF/Driver.cpp
View file
Expand Up @@ -19,9 +19,7 @@
#include "Writer.h"
#include "lld/Common/Args.h"
#include "lld/Common/Driver.h"
#include "lld/Common/ErrorHandler.h"
#include "lld/Common/Filesystem.h"
#include "lld/Common/Memory.h"
#include "lld/Common/Timer.h"
#include "lld/Common/Version.h"
#include "llvm/ADT/Optional.h"
Expand Down Expand Up @@ -63,36 +61,22 @@ namespace coff {
std::unique_ptr<Configuration> config;
std::unique_ptr<LinkerDriver> driver;

bool link(ArrayRef<const char *> args, bool canExitEarly, raw_ostream &stdoutOS,
raw_ostream &stderrOS) {
lld::stdoutOS = &stdoutOS;
lld::stderrOS = &stderrOS;
bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
llvm::raw_ostream &stderrOS, bool exitEarly, bool disableOutput) {
// This driver-specific context will be freed later by lldMain().
auto *ctx = new COFFLinkerContext;

errorHandler().cleanupCallback = []() {
freeArena();
};

errorHandler().logName = args::getFilenameWithoutExe(args[0]);
errorHandler().errorLimitExceededMsg =
"too many errors emitted, stopping now"
" (use /errorlimit:0 to see all errors)";
errorHandler().exitEarly = canExitEarly;
stderrOS.enable_colors(stderrOS.has_colors());
ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput);
ctx->e.logName = args::getFilenameWithoutExe(args[0]);
ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now"
" (use /errorlimit:0 to see all errors)";

COFFLinkerContext ctx;
config = std::make_unique<Configuration>();
driver = std::make_unique<LinkerDriver>(ctx);
driver = std::make_unique<LinkerDriver>(*ctx);

driver->linkerMain(args);

// Call exit() if we can to avoid calling destructors.
if (canExitEarly)
exitLld(errorCount() ? 1 : 0);

bool ret = errorCount() == 0;
if (!canExitEarly)
errorHandler().reset();
return ret;
return errorCount() == 0;
}

// Parse options of the form "old;new".
Expand Down Expand Up @@ -162,7 +146,7 @@ static std::future<MBErrPair> createFutureForFile(std::string path) {
static StringRef mangle(StringRef sym) {
assert(config->machine != IMAGE_FILE_MACHINE_UNKNOWN);
if (config->machine == I386)
return saver.save("_" + sym);
return saver().save("_" + sym);
return sym;
}

Expand Down Expand Up @@ -358,9 +342,9 @@ void LinkerDriver::parseDirectives(InputFile *file) {
Export exp = parseExport(e);
if (config->machine == I386 && config->mingw) {
if (!isDecorated(exp.name))
exp.name = saver.save("_" + exp.name);
exp.name = saver().save("_" + exp.name);
if (!exp.extName.empty() && !isDecorated(exp.extName))
exp.extName = saver.save("_" + exp.extName);
exp.extName = saver().save("_" + exp.extName);
}
exp.directives = true;
config->exports.push_back(exp);
Expand Down Expand Up @@ -442,11 +426,11 @@ StringRef LinkerDriver::doFindFile(StringRef filename) {
SmallString<128> path = dir;
sys::path::append(path, filename);
if (sys::fs::exists(path.str()))
return saver.save(path.str());
return saver().save(path.str());
if (!hasExt) {
path.append(".obj");
if (sys::fs::exists(path.str()))
return saver.save(path.str());
return saver().save(path.str());
}
}
return filename;
Expand Down Expand Up @@ -483,7 +467,7 @@ StringRef LinkerDriver::doFindLibMinGW(StringRef filename) {

SmallString<128> s = filename;
sys::path::replace_extension(s, ".a");
StringRef libName = saver.save("lib" + s.str());
StringRef libName = saver().save("lib" + s.str());
return doFindFile(libName);
}

Expand All @@ -492,7 +476,7 @@ StringRef LinkerDriver::doFindLib(StringRef filename) {
// Add ".lib" to Filename if that has no file extension.
bool hasExt = filename.contains('.');
if (!hasExt)
filename = saver.save(filename + ".lib");
filename = saver().save(filename + ".lib");
StringRef ret = doFindFile(filename);
// For MinGW, if the find above didn't turn up anything, try
// looking for a MinGW formatted library name.
Expand Down Expand Up @@ -525,7 +509,7 @@ void LinkerDriver::addLibSearchPaths() {
Optional<std::string> envOpt = Process::GetEnv("LIB");
if (!envOpt.hasValue())
return;
StringRef env = saver.save(*envOpt);
StringRef env = saver().save(*envOpt);
while (!env.empty()) {
StringRef path;
std::tie(path, env) = env.split(';');
Expand Down Expand Up @@ -873,8 +857,8 @@ static void parseModuleDefs(StringRef path) {
driver->takeBuffer(std::move(mb));

if (config->outputFile.empty())
config->outputFile = std::string(saver.save(m.OutputFile));
config->importName = std::string(saver.save(m.ImportName));
config->outputFile = std::string(saver().save(m.OutputFile));
config->importName = std::string(saver().save(m.ImportName));
if (m.ImageBase)
config->imageBase = m.ImageBase;
if (m.StackReserve)
Expand Down Expand Up @@ -902,14 +886,14 @@ static void parseModuleDefs(StringRef path) {
// DLL instead. This is supported by both MS and GNU linkers.
if (!e1.ExtName.empty() && e1.ExtName != e1.Name &&
StringRef(e1.Name).contains('.')) {
e2.name = saver.save(e1.ExtName);
e2.forwardTo = saver.save(e1.Name);
e2.name = saver().save(e1.ExtName);
e2.forwardTo = saver().save(e1.Name);
config->exports.push_back(e2);
continue;
}
e2.name = saver.save(e1.Name);
e2.extName = saver.save(e1.ExtName);
e2.aliasTarget = saver.save(e1.AliasTarget);
e2.name = saver().save(e1.Name);
e2.extName = saver().save(e1.ExtName);
e2.aliasTarget = saver().save(e1.AliasTarget);
e2.ordinal = e1.Ordinal;
e2.noname = e1.Noname;
e2.data = e1.Data;
Expand Down Expand Up @@ -1906,9 +1890,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
Export e = parseExport(arg->getValue());
if (config->machine == I386) {
if (!isDecorated(e.name))
e.name = saver.save("_" + e.name);
e.name = saver().save("_" + e.name);
if (!e.extName.empty() && !isDecorated(e.extName))
e.extName = saver.save("_" + e.extName);
e.extName = saver().save("_" + e.extName);
}
config->exports.push_back(e);
}
Expand Down
24 changes: 12 additions & 12 deletions lld/COFF/DriverUtils.cpp
View file
Expand Up @@ -48,17 +48,17 @@ const uint16_t RT_MANIFEST = 24;

class Executor {
public:
explicit Executor(StringRef s) : prog(saver.save(s)) {}
void add(StringRef s) { args.push_back(saver.save(s)); }
void add(std::string &s) { args.push_back(saver.save(s)); }
void add(Twine s) { args.push_back(saver.save(s)); }
void add(const char *s) { args.push_back(saver.save(s)); }
explicit Executor(StringRef s) : prog(saver().save(s)) {}
void add(StringRef s) { args.push_back(saver().save(s)); }
void add(std::string &s) { args.push_back(saver().save(s)); }
void add(Twine s) { args.push_back(saver().save(s)); }
void add(const char *s) { args.push_back(saver().save(s)); }

void run() {
ErrorOr<std::string> exeOrErr = sys::findProgramByName(prog);
if (auto ec = exeOrErr.getError())
fatal("unable to find " + prog + " in PATH: " + ec.message());
StringRef exe = saver.save(*exeOrErr);
StringRef exe = saver().save(*exeOrErr);
args.insert(args.begin(), exe);

if (sys::ExecuteAndWait(args[0], args) != 0)
Expand Down Expand Up @@ -636,14 +636,14 @@ static StringRef killAt(StringRef sym, bool prefix) {
sym = sym.substr(0, sym.find('@', 1));
if (!sym.startswith("@")) {
if (prefix && !sym.startswith("_"))
return saver.save("_" + sym);
return saver().save("_" + sym);
return sym;
}
// For fastcall, remove the leading @ and replace it with an
// underscore, if prefixes are used.
sym = sym.substr(1);
if (prefix)
sym = saver.save("_" + sym);
sym = saver().save("_" + sym);
return sym;
}

Expand Down Expand Up @@ -854,7 +854,7 @@ opt::InputArgList ArgParser::parse(ArrayRef<const char *> argv) {
argv.data() + argv.size());
if (!args.hasArg(OPT_lldignoreenv))
addLINK(expandedArgv);
cl::ExpandResponseFiles(saver, getQuotingStyle(args), expandedArgv);
cl::ExpandResponseFiles(saver(), getQuotingStyle(args), expandedArgv);
args = optTable.ParseArgs(makeArrayRef(expandedArgv).drop_front(),
missingIndex, missingCount);

Expand Down Expand Up @@ -901,7 +901,7 @@ ParsedDirectives ArgParser::parseDirectives(StringRef s) {
// Handle /EXPORT and /INCLUDE in a fast path. These directives can appear for
// potentially every symbol in the object, so they must be handled quickly.
SmallVector<StringRef, 16> tokens;
cl::TokenizeWindowsCommandLineNoCopy(s, saver, tokens);
cl::TokenizeWindowsCommandLineNoCopy(s, saver(), tokens);
for (StringRef tok : tokens) {
if (tok.startswith_insensitive("/export:") ||
tok.startswith_insensitive("-export:"))
Expand All @@ -914,7 +914,7 @@ ParsedDirectives ArgParser::parseDirectives(StringRef s) {
// already copied quoted arguments for us, so those do not need to be
// copied again.
bool HasNul = tok.end() != s.end() && tok.data()[tok.size()] == '\0';
rest.push_back(HasNul ? tok.data() : saver.save(tok).data());
rest.push_back(HasNul ? tok.data() : saver().save(tok).data());
}
}

Expand Down Expand Up @@ -948,7 +948,7 @@ void ArgParser::addLINK(SmallVector<const char *, 256> &argv) {

std::vector<const char *> ArgParser::tokenize(StringRef s) {
SmallVector<const char *, 16> tokens;
cl::TokenizeWindowsCommandLine(s, saver, tokens);
cl::TokenizeWindowsCommandLine(s, saver(), tokens);
return std::vector<const char *>(tokens.begin(), tokens.end());
}

Expand Down
26 changes: 13 additions & 13 deletions lld/COFF/InputFiles.cpp
View file
Expand Up @@ -15,8 +15,6 @@
#include "SymbolTable.h"
#include "Symbols.h"
#include "lld/Common/DWARF.h"
#include "lld/Common/ErrorHandler.h"
#include "lld/Common/Memory.h"
#include "llvm-c/lto.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Triple.h"
Expand Down Expand Up @@ -905,7 +903,7 @@ ObjFile::getVariableLocation(StringRef var) {
Optional<std::pair<std::string, unsigned>> ret = dwarf->getVariableLoc(var);
if (!ret)
return None;
return std::make_pair(saver.save(ret->first), ret->second);
return std::make_pair(saver().save(ret->first), ret->second);
}

// Used only for DWARF debug info, which is not common (except in MinGW
Expand Down Expand Up @@ -940,8 +938,8 @@ void ImportFile::parse() {
fatal("broken import library");

// Read names and create an __imp_ symbol.
StringRef name = saver.save(StringRef(buf + sizeof(*hdr)));
StringRef impName = saver.save("__imp_" + name);
StringRef name = saver().save(StringRef(buf + sizeof(*hdr)));
StringRef impName = saver().save("__imp_" + name);
const char *nameStart = buf + sizeof(coff_import_header) + name.size() + 1;
dllName = std::string(StringRef(nameStart));
StringRef extName;
Expand Down Expand Up @@ -995,11 +993,12 @@ BitcodeFile::BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef mb,
// into consideration at LTO time (which very likely causes undefined
// symbols later in the link stage). So we append file offset to make
// filename unique.
MemoryBufferRef mbref(
mb.getBuffer(),
saver.save(archiveName.empty() ? path
: archiveName + sys::path::filename(path) +
utostr(offsetInArchive)));
MemoryBufferRef mbref(mb.getBuffer(),
saver().save(archiveName.empty()
? path
: archiveName +
sys::path::filename(path) +
utostr(offsetInArchive)));

obj = check(lto::InputFile::create(mbref));
}
Expand Down Expand Up @@ -1035,6 +1034,7 @@ FakeSectionChunk ltoDataSectionChunk(&ltoDataSection.section);
} // namespace

void BitcodeFile::parse() {
llvm::StringSaver &saver = lld::saver();
std::vector<std::pair<Symbol *, bool>> comdat(obj->getComdatTable().size());
for (size_t i = 0; i != obj->getComdatTable().size(); ++i)
// FIXME: Check nodeduplicate
Expand Down Expand Up @@ -1156,11 +1156,11 @@ void DLLFile::parse() {
s->nameType = ImportNameType::IMPORT_NAME;

if (coffObj->getMachine() == I386) {
s->symbolName = symbolName = saver.save("_" + symbolName);
s->symbolName = symbolName = saver().save("_" + symbolName);
s->nameType = ImportNameType::IMPORT_NAME_NOPREFIX;
}

StringRef impName = saver.save("__imp_" + symbolName);
StringRef impName = saver().save("__imp_" + symbolName);
ctx.symtab.addLazyDLLSymbol(this, s, impName);
if (code)
ctx.symtab.addLazyDLLSymbol(this, s, symbolName);
Expand All @@ -1179,7 +1179,7 @@ void DLLFile::makeImport(DLLFile::Symbol *s) {

size_t impSize = s->dllName.size() + s->symbolName.size() + 2; // +2 for NULs
size_t size = sizeof(coff_import_header) + impSize;
char *buf = bAlloc.Allocate<char>(size);
char *buf = bAlloc().Allocate<char>(size);
memset(buf, 0, size);
char *p = buf;
auto *imp = reinterpret_cast<coff_import_header *>(p);
Expand Down

0 comments on commit 83d59e0

Please sign in to comment.