Skip to content

[LLD][COFF] Handle --start-lib/--end-lib group in the same way as other archives #136496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[LLD][COFF] Handle --start-lib/--end-lib group in the same way as other archives #136496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c541eee
[LLD][COFF] Handle --start-lib/--end-lib group in the same way as oth…
aganea Apr 20, 2025
a782451
Change the asserts into warning and add more test coverage
aganea May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 84 additions & 40 deletions lld/COFF/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,28 +200,13 @@ static bool compatibleMachineType(COFFLinkerContext &ctx, MachineTypes mt) {
}
}

void LinkerDriver::addFile(InputFile *file) {
Log(ctx) << "Reading " << toString(file);
if (file->lazy) {
if (auto *f = dyn_cast<BitcodeFile>(file))
f->parseLazy();
else
cast<ObjFile>(file)->parseLazy();
} else {
file->parse();
if (auto *f = dyn_cast<ObjFile>(file)) {
ctx.objFileInstances.push_back(f);
} else if (auto *f = dyn_cast<BitcodeFile>(file)) {
if (ltoCompilationDone) {
Err(ctx) << "LTO object file " << toString(file)
<< " linked in after "
"doing LTO compilation.";
}
f->symtab.bitcodeFileInstances.push_back(f);
} else if (auto *f = dyn_cast<ImportFile>(file)) {
ctx.importFileInstances.push_back(f);
}
void LinkerDriver::addFile(InputFile *file, CmdLineArchive *inCmdLineArchive) {
if (inCmdLineArchive) {
inCmdLineArchive->addInputFile(file); // schedule for lazy parsing
return;
}
Log(ctx) << "Reading " << toString(file);
file->maybeParse();

MachineTypes mt = file->getMachineType();
// The ARM64EC target must be explicitly specified and cannot be inferred.
Expand Down Expand Up @@ -259,17 +244,29 @@ MemoryBufferRef LinkerDriver::takeBuffer(std::unique_ptr<MemoryBuffer> mb) {
}

void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
bool wholeArchive, bool lazy) {
bool wholeArchive,
CmdLineArchive *inCmdLineArchive) {
StringRef filename = mb->getBufferIdentifier();

MemoryBufferRef mbref = takeBuffer(std::move(mb));

auto maybePrintWarning = [&](StringRef type, StringRef message) {
if (inCmdLineArchive)
Warn(ctx) << type << " file provided between "
<< inCmdLineArchive->startLibArg << "/"
<< inCmdLineArchive->endLibArg << " " << message;
};

// File type is detected by contents, not by file extension.
switch (identify_magic(mbref.getBuffer())) {
case file_magic::windows_resource:
maybePrintWarning(".res", "will not be lazy");
resources.push_back(mbref);
break;
case file_magic::archive:
// FIXME: We could later support --start-lib/--end-lib groups, to allow for
// "extending" an existing archive/LIB.
maybePrintWarning(".lib/.a", "has no effect");
if (wholeArchive) {
std::unique_ptr<Archive> file =
CHECK(Archive::create(mbref), filename + ": failed to parse archive");
Expand All @@ -284,13 +281,14 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
addFile(make<ArchiveFile>(ctx, mbref));
break;
case file_magic::bitcode:
addFile(BitcodeFile::create(ctx, mbref, "", 0, lazy));
addFile(BitcodeFile::create(ctx, mbref, "", 0), inCmdLineArchive);
break;
case file_magic::coff_object:
case file_magic::coff_import_library:
addFile(ObjFile::create(ctx, mbref, lazy));
addFile(ObjFile::create(ctx, mbref), inCmdLineArchive);
break;
case file_magic::pdb:
maybePrintWarning(".pdb", "will not be lazy");
addFile(make<PDBInputFile>(ctx, mbref));
break;
case file_magic::coff_cl_gl_object:
Expand All @@ -299,6 +297,7 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
break;
case file_magic::pecoff_executable:
if (ctx.config.mingw) {
maybePrintWarning(".dll", "will not be lazy");
addFile(make<DLLFile>(ctx.symtab, mbref));
break;
}
Expand All @@ -315,7 +314,9 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
}
}

void LinkerDriver::enqueuePath(StringRef path, bool wholeArchive, bool lazy) {
void LinkerDriver::enqueuePath(
StringRef path, bool wholeArchive,
std::optional<std::shared_ptr<CmdLineArchive *>> inCmdLineArchive) {
auto future = std::make_shared<std::future<MBErrPair>>(
createFutureForFile(std::string(path)));
std::string pathStr = std::string(path);
Expand Down Expand Up @@ -354,7 +355,8 @@ void LinkerDriver::enqueuePath(StringRef path, bool wholeArchive, bool lazy) {
else
Err(ctx) << msg << "; did you mean '" << nearest << "'";
} else
ctx.driver.addBuffer(std::move(mb), wholeArchive, lazy);
ctx.driver.addBuffer(std::move(mb), wholeArchive,
inCmdLineArchive ? **inCmdLineArchive : nullptr);
});
}

Expand All @@ -373,8 +375,7 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,
if (magic == file_magic::coff_object) {
obj = ObjFile::create(ctx, mb);
} else if (magic == file_magic::bitcode) {
obj = BitcodeFile::create(ctx, mb, parentName, offsetInArchive,
/*lazy=*/false);
obj = BitcodeFile::create(ctx, mb, parentName, offsetInArchive);
} else if (magic == file_magic::coff_cl_gl_object) {
Err(ctx) << mb.getBufferIdentifier()
<< ": is not a native COFF file. Recompile without /GL?";
Expand Down Expand Up @@ -494,7 +495,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
break;
case OPT_defaultlib:
if (std::optional<StringRef> path = findLibIfNew(arg->getValue()))
enqueuePath(*path, false, false);
enqueuePath(*path);
break;
case OPT_entry:
if (!arg->getValue()[0])
Expand Down Expand Up @@ -2172,37 +2173,78 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
return false;
};

// Store start_lib/end_lib arguments in order to render dignostics in the
// same way the flags are written on the command line.
llvm::opt::Arg *startLibArg = nullptr;
llvm::opt::Arg *endLibArg = nullptr;
auto endLibSpelling = [&]() {
return endLibArg ? endLibArg->getSpelling()
: lld::args::getOptionSpellingLikeArg(
ctx.optTable, OPT_end_lib, startLibArg, ctx.saver);
};

// Create a list of input files. These can be given as OPT_INPUT options
// and OPT_wholearchive_file options, and we also need to track OPT_start_lib
// and OPT_end_lib.
{
llvm::TimeTraceScope timeScope2("Parse & queue inputs");
bool inLib = false;
std::optional<std::shared_ptr<CmdLineArchive *>> inCmdLineArchive;
auto close = [&]() {
enqueueTask([=]() {
assert(inCmdLineArchive);
if (CmdLineArchive *a = **inCmdLineArchive)
a->maybeParse();
});
};

for (auto *arg : args) {
switch (arg->getOption().getID()) {
case OPT_end_lib:
if (!inLib)
if (!inCmdLineArchive) {
Err(ctx) << "stray " << arg->getSpelling();
inLib = false;
} else {
endLibArg = arg;
close();
inCmdLineArchive = std::nullopt;
}
break;
case OPT_start_lib:
if (inLib)
if (inCmdLineArchive) {
Err(ctx) << "nested " << arg->getSpelling();
inLib = true;
} else {
startLibArg = arg;
inCmdLineArchive = std::make_shared<CmdLineArchive *>();
enqueueTask([&, inCmdLineArchive, startLibArg, endLibArg]() {
// In is important to create a fake archive here so that we
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// In is important to create a fake archive here so that we
// It is important to create a fake archive here so that we

// remember its placement on the command-line. This will be
// later needed to resolve symbols in the archive order required
// by the MSVC specification.
**inCmdLineArchive = make<CmdLineArchive>(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of the PR is this line. We need, in a subsequent PR, to associate ObjFiles with CmdLineArchive and make it record an Index of where it is on the command line.

ctx.symtab, MemoryBufferRef({}, "<cmdline-lib>"),
startLibArg->getSpelling(), endLibSpelling());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, endLibSpelling() will return the last -end-lib spelling, not necessarily the one corresponding to the current instance.

I'm not entirely sure where this is headed (so feel free to disregard this suggestion), but I think the logic might be easier to follow if we created the CmdLineArchive instance immediately. Instead of storing startLibArg and endLibArg separately, these could be assigned directly within the object as we go. I understand the index will need to be tracked later, but that could likely be set in the task queued by close(). It probably doesn’t need to be handled in the constructor.

});
}
break;
case OPT_wholearchive_file:
if (std::optional<StringRef> path = findFileIfNew(arg->getValue()))
enqueuePath(*path, true, inLib);
enqueuePath(*path, true, inCmdLineArchive);
break;
case OPT_INPUT:
if (std::optional<StringRef> path = findFileIfNew(arg->getValue()))
enqueuePath(*path, isWholeArchive(*path), inLib);
enqueuePath(*path, isWholeArchive(*path), inCmdLineArchive);
break;
default:
// Ignore other options.
break;
}
}
if (inCmdLineArchive) {
StringRef startLib = startLibArg->getSpelling();
Warn(ctx) << startLib << " without " << endLibSpelling()
<< "\nNOTE: all files provided after " << startLib
<< " were lazy.";
close();
}
}

// Read all input files given via the command line.
Expand Down Expand Up @@ -2236,7 +2278,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// addWinSysRootLibSearchPaths(), which is why they are in a separate loop.
for (auto *arg : args.filtered(OPT_defaultlib))
if (std::optional<StringRef> path = findLibIfNew(arg->getValue()))
enqueuePath(*path, false, false);
enqueuePath(*path);
run();
if (errorCount())
return;
Expand Down Expand Up @@ -2553,9 +2595,11 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {

if (args.hasArg(OPT_include_optional)) {
// Handle /includeoptional
for (auto *arg : args.filtered(OPT_include_optional))
if (isa_and_nonnull<LazyArchive>(symtab.find(arg->getValue())))
for (auto *arg : args.filtered(OPT_include_optional)) {
Symbol *sym = ctx.symtab.find(arg->getValue());
if (sym && (isa<LazyArchive>(sym) || isa<LazyObject>(sym)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a test case for that.

symtab.addGCRoot(arg->getValue());
}
}
});
} while (run());
Expand Down Expand Up @@ -2720,7 +2764,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// /manifestdependency: enables /manifest unless an explicit /manifest:no is
// also passed.
if (config->manifest == Configuration::Embed)
addBuffer(createManifestRes(), false, false);
addBuffer(createManifestRes());
else if (config->manifest == Configuration::SideBySide ||
(config->manifest == Configuration::Default &&
!config->manifestDependencies.empty()))
Expand Down
20 changes: 12 additions & 8 deletions lld/COFF/Driver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/TarWriter.h"
#include "llvm/WindowsDriver/MSVCPaths.h"
#include <future>
#include <memory>
#include <optional>
#include <set>
Expand Down Expand Up @@ -80,26 +81,31 @@ class LinkerDriver {

void linkerMain(llvm::ArrayRef<const char *> args);

void addFile(InputFile *file);
void addFile(InputFile *file, CmdLineArchive *inCmdLineArchive = nullptr);

void addClangLibSearchPaths(const std::string &argv0);

// Used by ArchiveFile to enqueue members.
void enqueueArchiveMember(const Archive::Child &c, const Archive::Symbol &sym,
StringRef parentName);

void enqueuePDB(StringRef Path) { enqueuePath(Path, false, false); }
void enqueuePDB(StringRef Path) { enqueuePath(Path); }

MemoryBufferRef takeBuffer(std::unique_ptr<MemoryBuffer> mb);

void enqueuePath(StringRef path, bool wholeArchive, bool lazy);
// Schedule a input file for reading.
void enqueuePath(StringRef path, bool wholeArchive = false,
std::optional<std::shared_ptr<CmdLineArchive *>>
inCmdLineArchive = std::nullopt);

void pullArm64ECIcallHelper();

// Returns a list of chunks of selected symbols.
std::vector<Chunk *> getChunks() const;

std::unique_ptr<llvm::TarWriter> tar; // for /linkrepro

void pullArm64ECIcallHelper();
bool ltoCompilationDone = false;

private:
// Searches a file from search paths.
Expand Down Expand Up @@ -169,8 +175,8 @@ class LinkerDriver {

std::set<std::string> visitedLibs;

void addBuffer(std::unique_ptr<MemoryBuffer> mb, bool wholeArchive,
bool lazy);
void addBuffer(std::unique_ptr<MemoryBuffer> mb, bool wholeArchive = false,
CmdLineArchive *inCmdLineArchive = nullptr);
void addArchiveBuffer(MemoryBufferRef mbref, StringRef symName,
StringRef parentName, uint64_t offsetInArchive);

Expand Down Expand Up @@ -258,8 +264,6 @@ class LinkerDriver {
// Create export thunks for exported and patchable Arm64EC function symbols.
void createECExportThunks();
void maybeCreateECExportThunk(StringRef name, Symbol *&sym);

bool ltoCompilationDone = false;
};

// Create enum with OPT_xxx values for each option in Options.td
Expand Down
Loading