Skip to content

[LLD][COFF] Fix importing DllMain from import libraries #146610

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.

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

Merged
merged 1 commit into from
Jul 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions lld/COFF/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ struct Configuration {
bool warnDebugInfoUnusable = true;
bool warnLongSectionNames = true;
bool warnStdcallFixup = true;
bool warnExportedDllMain = true;
bool incremental = true;
bool integrityCheck = false;
bool killAt = false;
Expand Down
2 changes: 2 additions & 0 deletions lld/COFF/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1643,6 +1643,8 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
config->warnLocallyDefinedImported = false;
else if (s == "longsections")
config->warnLongSectionNames = false;
else if (s == "exporteddllmain")
config->warnExportedDllMain = false;
// Other warning numbers are ignored.
}
}
Expand Down
40 changes: 39 additions & 1 deletion lld/COFF/InputFiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,35 @@ static coff_symbol_generic *cloneSymbol(COFFSymbolRef sym) {
}
}

// Skip importing DllMain thunks from import libraries.
static bool fixupDllMain(COFFLinkerContext &ctx, llvm::object::Archive *file,
const Archive::Symbol &sym, bool &skipDllMain) {
if (skipDllMain)
return true;
const Archive::Child &c =
CHECK(sym.getMember(), file->getFileName() +
": could not get the member for symbol " +
toCOFFString(ctx, sym));
MemoryBufferRef mb =
CHECK(c.getMemoryBufferRef(),
file->getFileName() +
": could not get the buffer for a child buffer of the archive");
if (identify_magic(mb.getBuffer()) == file_magic::coff_import_library) {
if (ctx.config.warnExportedDllMain) {
// We won't place DllMain symbols in the symbol table if they are
// coming from a import library. This message can be ignored with the flag
// '/ignore:exporteddllmain'
Warn(ctx)
<< file->getFileName()
<< ": skipping exported DllMain symbol [exporteddllmain]\nNOTE: this "
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I'm a little late to the party here, but I think the option naming (and likewise the internal variables, but they're less effort to change later) feels a bit backwards here.

From the point of view of this link, there's an imported DllMain that we're ignoring. (This import lib entry probably exists because some DLL erroneously does export it, yes, but from the point of view of this linking, the issue is about an imported DllMain, not an issue about ourselves exporting DllMain.)

"might be a mistake when the DLL/library was produced.";
}
skipDllMain = true;
return true;
}
return false;
}

ArchiveFile::ArchiveFile(COFFLinkerContext &ctx, MemoryBufferRef m)
: InputFile(ctx.symtab, ArchiveKind, m) {}

Expand Down Expand Up @@ -176,8 +205,17 @@ void ArchiveFile::parse() {
}

// Read the symbol table to construct Lazy objects.
for (const Archive::Symbol &sym : file->symbols())
bool skipDllMain = false;
for (const Archive::Symbol &sym : file->symbols()) {
// If the DllMain symbol was exported by mistake, skip importing it
// otherwise we might end up with a import thunk in the final binary which
// is wrong.
Copy link
Member

Choose a reason for hiding this comment

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

I find this comment a bit hand-wavy. Obviously, calling another DLL's DllMain rathern than our own is not going to be doing the right thing - it's not so much about an import thunk or so, right? So I'd rather just say this:

// If an import library provides the DllMain symbol, skip importing it, as we should be using our own DllMain, not another DLL's DllMain.

if (sym.getName() == "__imp_DllMain" || sym.getName() == "DllMain") {
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be missing the relevant prefix handling for i386?

if (fixupDllMain(ctx, file.get(), sym, skipDllMain))
continue;
}
archiveSymtab->addLazyArchive(this, sym);
}
}

// Returns a buffer pointing to a member file containing a given symbol.
Expand Down
57 changes: 57 additions & 0 deletions lld/test/COFF/exported-dllmain.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
REQUIRES: x86
RUN: split-file %s %t.dir && cd %t.dir

RUN: llvm-mc -filetype=obj -triple=x86_64-windows a.s -o a.obj

RUN: llvm-mc -filetype=obj -triple=x86_64-windows b1.s -o b1.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows b2.s -o b2.obj

### This is the line where our problem occurs. Here, we export the DllMain symbol which shouldn't happen normally.
RUN: lld-link b1.obj b2.obj -out:b.dll -dll -implib:b.lib -entry:DllMain -export:bar -export:DllMain

RUN: llvm-mc -filetype=obj -triple=x86_64-windows c.s -o c.obj
RUN: lld-link -lib c.obj -out:c.lib

### Later, if b.lib is provided before other libs/objs that export DllMain statically, we previously were using the dllimported DllMain from b.lib, which is wrong.
RUN: lld-link a.obj b.lib c.lib -dll -out:out.dll -entry:DllMain 2>&1 | FileCheck -check-prefix=WARN %s
RUN: lld-link a.obj b.lib c.lib -dll -out:out.dll -entry:DllMain -ignore:exporteddllmain 2>&1 | FileCheck -check-prefix=IGNORED --allow-empty %s
RUN: llvm-objdump --private-headers -d out.dll | FileCheck -check-prefix=DISASM %s

WARN: lld-link: warning: b.lib: skipping exported DllMain symbol [exporteddllmain]
IGNORED-NOT: lld-link: warning: b.lib: skipping exported DllMain symbol [exporteddllmain]

DISASM: The Import Tables:
DISASM: DLL Name: b.dll
DISASM-NOT: DllMain
DISASM: bar
DISASM: Disassembly of section .text:
DISASM-EMPTY:
DISASM: b8 01 00 00 00 movl $0x1, %eax
DISASM-NEXT: c3 retq

#--- a.s
.text
.globl foo
foo:
call *__imp_bar(%rip)
ret

#--- b1.s
.text
.globl bar
bar:
ret

#--- b2.s
.text
.globl DllMain
DllMain:
xor %eax, %eax
ret

#--- c.s
.text
.globl DllMain
DllMain:
movl $1, %eax
ret
Loading