Skip to content

release/20.x: Backport [LLD][COFF] Disallow importing DllMain from import libraries (#146610) #146699

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

Open
wants to merge 2 commits into
base: release/20.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -313,6 +313,7 @@ struct Configuration {
bool warnDebugInfoUnusable = true;
bool warnLongSectionNames = true;
bool warnStdcallFixup = true;
bool warnExportedDllMain = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an ABI break.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is an installed header though?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the field in this backport.

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 @@ -1625,6 +1625,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 @@ -117,6 +117,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) {}

Expand All @@ -140,8 +169,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.
if (sym.getName() == "__imp_DllMain" || sym.getName() == "DllMain") {
if (fixupDllMain(ctx, file.get(), sym, skipDllMain))
continue;
}
ctx.symtab.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