Skip to content

[LLD][COFF] Make unresolved symbol search behavior compliant with MSVC link.exe #85290

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 14 commits into
base: main
Choose a base branch
from
Open

[LLD][COFF] Make unresolved symbol search behavior compliant with MSVC link.exe #85290

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d5c4adf
[LLD][COFF] Align unresolved symbols search behavior with MSVC link.exe
aganea Mar 14, 2024
b1149e1
Revert unneeded changes
aganea Mar 14, 2024
6c2a777
Test cmd-line libraries
aganea Mar 14, 2024
ad59e48
LLD release notes
aganea Mar 15, 2024
b115c60
Add another egde case where symbols could be pulled/resolved from two…
aganea Mar 15, 2024
9d910c7
Missing assert for cmd-line archives
aganea Mar 15, 2024
865dc26
Support the new behavior with /DEFAULTLIB on the cmd-line or in
aganea Mar 18, 2024
114870b
Fix chaining with pending load symbols
aganea Mar 19, 2024
0ece320
Add /DEFAULTLIB tests
aganea Apr 8, 2024
4d6fcd8
Review fixes
aganea Apr 8, 2024
87a90c3
More review fixes
aganea Apr 8, 2024
c2e7c78
clang-format
aganea Apr 8, 2024
0f5d47e
Typo
aganea Apr 8, 2024
baeca96
Fix lazy node casting in the presence of wrapped symbols (MinGW -wrap)
aganea Jun 17, 2024
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
53 changes: 40 additions & 13 deletions lld/COFF/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ MemoryBufferRef LinkerDriver::takeBuffer(std::unique_ptr<MemoryBuffer> mb) {
}

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

MemoryBufferRef mbref = takeBuffer(std::move(mb));
Expand All @@ -213,11 +214,11 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
ctx.symtab.addFile(make<ArchiveFile>(ctx, mbref));
break;
case file_magic::bitcode:
ctx.symtab.addFile(make<BitcodeFile>(ctx, mbref, "", 0, lazy));
ctx.symtab.addFile(make<BitcodeFile>(ctx, mbref, "", 0, lazy, parent));
break;
case file_magic::coff_object:
case file_magic::coff_import_library:
ctx.symtab.addFile(make<ObjFile>(ctx, mbref, lazy));
ctx.symtab.addFile(make<ObjFile>(ctx, mbref, lazy, parent));
break;
case file_magic::pdb:
ctx.symtab.addFile(make<PDBInputFile>(ctx, mbref));
Expand All @@ -242,7 +243,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, bool lazy,
std::optional<std::shared_future<ArchiveFile *>> parent) {
auto future = std::make_shared<std::future<MBErrPair>>(
createFutureForFile(std::string(path)));
std::string pathStr = std::string(path);
Expand Down Expand Up @@ -281,13 +284,15 @@ void LinkerDriver::enqueuePath(StringRef path, bool wholeArchive, bool lazy) {
else
error(msg + "; did you mean '" + nearest + "'");
} else
ctx.driver.addBuffer(std::move(mb), wholeArchive, lazy);
ctx.driver.addBuffer(std::move(mb), wholeArchive, lazy,
parent ? parent->get() : nullptr);
});
}

void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,
StringRef parentName,
uint64_t offsetInArchive) {
uint64_t offsetInArchive,
ArchiveFile *parent) {
file_magic magic = identify_magic(mb.getBuffer());
if (magic == file_magic::coff_import_library) {
InputFile *imp = make<ImportFile>(ctx, mb);
Expand All @@ -298,10 +303,10 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,

InputFile *obj;
if (magic == file_magic::coff_object) {
obj = make<ObjFile>(ctx, mb);
obj = make<ObjFile>(ctx, mb, /*lazy=*/false, parent);
} else if (magic == file_magic::bitcode) {
obj =
make<BitcodeFile>(ctx, mb, parentName, offsetInArchive, /*lazy=*/false);
obj = make<BitcodeFile>(ctx, mb, parentName, offsetInArchive,
/*lazy=*/false, parent);
} else if (magic == file_magic::coff_cl_gl_object) {
error(mb.getBufferIdentifier() +
": is not a native COFF file. Recompile without /GL?");
Expand All @@ -318,7 +323,8 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,

void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
const Archive::Symbol &sym,
StringRef parentName) {
StringRef parentName,
ArchiveFile *parent) {

auto reportBufferError = [=](Error &&e, StringRef childName) {
fatal("could not get the buffer for the member defining symbol " +
Expand All @@ -335,7 +341,7 @@ void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
enqueueTask([=]() {
llvm::TimeTraceScope timeScope("Archive: ", mb.getBufferIdentifier());
ctx.driver.addArchiveBuffer(mb, toCOFFString(ctx, sym), parentName,
offsetInArchive);
offsetInArchive, parent);
});
return;
}
Expand All @@ -356,7 +362,15 @@ void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
// used as the buffer identifier.
ctx.driver.addArchiveBuffer(takeBuffer(std::move(mbOrErr.first)),
toCOFFString(ctx, sym), "",
/*OffsetInArchive=*/0);
/*OffsetInArchive=*/0, parent);
});
}

void LinkerDriver::enqueueLazyFile(InputFile *file) {
enqueueTask([=]() {
// Once it has been enqued, it cannot be lazy anymore.
file->lazy = false;
ctx.symtab.addFile(file);
});
}

Expand Down Expand Up @@ -2111,25 +2125,38 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
{
llvm::TimeTraceScope timeScope2("Parse & queue inputs");
bool inLib = false;
std::optional<std::shared_future<ArchiveFile *>> inLibArchive;
for (auto *arg : args) {
switch (arg->getOption().getID()) {
case OPT_end_lib:
if (!inLib)
error("stray " + arg->getSpelling());
inLib = false;
inLibArchive = std::nullopt;
break;
case OPT_start_lib:
if (inLib)
error("nested " + arg->getSpelling());
inLib = true;
// In 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.
{
auto a = std::make_shared<std::promise<ArchiveFile *>>();
Copy link
Member Author

@aganea aganea Mar 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Review note: this is made a shared_ptr<promise> simply because lambdas cannot capture non-CopyConstructible object instances, because of https://timsong-cpp.github.io/cppwp/n4659/func.wrap.func.con#7

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is nitty code golf, but can this be implemented with an explicit lambda capture and std::move? This SO pattern is what I was thinking of.

Maybe the task gets copied around more, so if not, then the shared_ptr is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did try several things, including what you're suggesting, as well as the std::bind trick, but nothing else works, besides the share_ptr above.

inLibArchive = a->get_future().share();
enqueueTask([=] {
a->set_value(
make<ArchiveFile>(ctx, MemoryBufferRef({}, "<cmdline-lib>")));
});
}
break;
case OPT_wholearchive_file:
if (std::optional<StringRef> path = findFileIfNew(arg->getValue()))
enqueuePath(*path, true, inLib);
break;
case OPT_INPUT:
if (std::optional<StringRef> path = findFileIfNew(arg->getValue()))
enqueuePath(*path, isWholeArchive(*path), inLib);
enqueuePath(*path, isWholeArchive(*path), inLib, inLibArchive);
break;
default:
// Ignore other options.
Expand Down
21 changes: 15 additions & 6 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 @@ -91,13 +92,20 @@ class LinkerDriver {

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

void enqueuePDB(StringRef Path) { enqueuePath(Path, false, false); }
void enqueuePDB(StringRef Path) {
enqueuePath(Path, false, false, /*parent=*/std::nullopt);
}

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

void enqueuePath(StringRef path, bool wholeArchive, bool lazy);
void enqueuePath(
StringRef path, bool wholeArchive, bool lazy,
std::optional<std::shared_future<ArchiveFile *>> parent = std::nullopt);

void enqueueLazyFile(InputFile *file);

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

Expand Down Expand Up @@ -182,10 +190,11 @@ class LinkerDriver {
StringRef findDefaultEntry();
WindowsSubsystem inferSubsystem();

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

void enqueueTask(std::function<void()> task);
bool run();
Expand Down
13 changes: 8 additions & 5 deletions lld/COFF/InputFiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,12 @@ static bool ignoredSymbolName(StringRef name) {
}

ArchiveFile::ArchiveFile(COFFLinkerContext &ctx, MemoryBufferRef m)
: InputFile(ctx, ArchiveKind, m) {}
: InputFile(ctx, ArchiveKind, m, /*lazy=*/true) {
static unsigned Order = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be a member of COFFLinkerContext.

CmdLineIndex = Order++;
}

void ArchiveFile::parse() {
void ArchiveFile::parseLazy() {
// Parse a MemoryBufferRef as an archive file.
file = CHECK(Archive::create(mb), this);

Expand All @@ -115,7 +118,7 @@ void ArchiveFile::addMember(const Archive::Symbol &sym) {
if (!seen.insert(c.getChildOffset()).second)
return;

ctx.driver.enqueueArchiveMember(c, sym, getName());
ctx.driver.enqueueArchiveMember(c, sym, getName(), this);
}

std::vector<MemoryBufferRef> lld::coff::getArchiveMembers(Archive *file) {
Expand Down Expand Up @@ -1000,8 +1003,8 @@ void ImportFile::parse() {

BitcodeFile::BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef mb,
StringRef archiveName, uint64_t offsetInArchive,
bool lazy)
: InputFile(ctx, BitcodeKind, mb, lazy) {
bool lazy, ArchiveFile *parent)
: InputFile(ctx, BitcodeKind, mb, lazy), parent(parent) {
std::string path = mb.getBufferIdentifier().str();
if (ctx.config.thinLTOIndexOnly)
path = replaceThinLTOSuffix(mb.getBufferIdentifier(),
Expand Down
26 changes: 19 additions & 7 deletions lld/COFF/InputFiles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ class InputFile {
enum Kind {
ArchiveKind,
ObjectKind,
LazyObjectKind,
PDBKind,
ImportKind,
BitcodeKind,
Expand Down Expand Up @@ -105,31 +104,38 @@ class InputFile {

public:
// True if this is a lazy ObjFile or BitcodeFile.
bool lazy = false;
bool lazy;
};

// .lib or .a file.
class ArchiveFile : public InputFile {
public:
explicit ArchiveFile(COFFLinkerContext &ctx, MemoryBufferRef m);
static bool classof(const InputFile *f) { return f->kind() == ArchiveKind; }
void parse() override;
void parse() override{};
void parseLazy();

// Enqueues an archive member load for the given symbol. If we've already
// enqueued a load for the same archive member, this function does nothing,
// which ensures that we don't load the same member more than once.
void addMember(const Archive::Symbol &sym);

private:
std::unique_ptr<Archive> file;

// The order this archive was seen on the cmd-line. This is later needed for
// resolving undefined symbols in archive OBJs.
uint32_t CmdLineIndex;

private:
llvm::DenseSet<uint64_t> seen;
};

// .obj or .o file. This may be a member of an archive file.
class ObjFile : public InputFile {
public:
explicit ObjFile(COFFLinkerContext &ctx, MemoryBufferRef m, bool lazy = false)
: InputFile(ctx, ObjectKind, m, lazy) {}
explicit ObjFile(COFFLinkerContext &ctx, MemoryBufferRef m, bool lazy = false,
ArchiveFile *parent = nullptr)
: InputFile(ctx, ObjectKind, m, lazy), parent(parent) {}
static bool classof(const InputFile *f) { return f->kind() == ObjectKind; }
void parse() override;
void parseLazy();
Expand Down Expand Up @@ -182,6 +188,9 @@ class ObjFile : public InputFile {
// True if this file was compiled with /guard:ehcont.
bool hasGuardEHCont() { return feat00Flags & 0x4000; }

// Whether this Obj buffer is part of an archive.
ArchiveFile *parent;

// Pointer to the PDB module descriptor builder. Various debug info records
// will reference object files by "module index", which is here. Things like
// source files and section contributions are also recorded here. Will be null
Expand Down Expand Up @@ -369,14 +378,17 @@ class BitcodeFile : public InputFile {
public:
explicit BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef mb,
StringRef archiveName, uint64_t offsetInArchive,
bool lazy);
bool lazy = false, ArchiveFile *parent = nullptr);
~BitcodeFile();
static bool classof(const InputFile *f) { return f->kind() == BitcodeKind; }
ArrayRef<Symbol *> getSymbols() { return symbols; }
MachineTypes getMachineType() override;
void parseLazy();
std::unique_ptr<llvm::lto::InputFile> obj;

// Whether this bitcode buffer is part of an archive.
ArchiveFile *parent;

private:
void parse() override;

Expand Down
Loading