-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 " | ||
"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) {} | ||
|
||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
if (sym.getName() == "__imp_DllMain" || sym.getName() == "DllMain") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
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 |
There was a problem hiding this comment.
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 importedDllMain
, not an issue about ourselves exportingDllMain
.)