Skip to content

[LLD][COFF] Follow up comments on pr146610 #147152

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 3 commits into
base: main
Choose a base branch
from
+83 −17
Open

[LLD][COFF] Follow up comments on pr146610 #147152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a4b6b7
Follow up comments on pr146610
aganea Jul 5, 2025
25f3b01
Address review comments especially i386 support
aganea Jul 17, 2025
312fd39
Revert some of the previous changes
aganea Jul 17, 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
2 changes: 1 addition & 1 deletion lld/COFF/Config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ struct Configuration {
bool warnDebugInfoUnusable = true;
bool warnLongSectionNames = true;
bool warnStdcallFixup = true;
bool warnExportedDllMain = true;
bool warnImportedDllMain = true;
bool incremental = true;
bool integrityCheck = false;
bool killAt = false;
Expand Down
4 changes: 2 additions & 2 deletions lld/COFF/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1643,8 +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;
else if (s == "importeddllmain")
config->warnImportedDllMain = false;
// Other warning numbers are ignored.
}
}
Expand Down
30 changes: 19 additions & 11 deletions lld/COFF/InputFiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ 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 " +
Expand All @@ -128,13 +126,13 @@ static bool fixupDllMain(COFFLinkerContext &ctx, llvm::object::Archive *file,
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) {
if (ctx.config.warnImportedDllMain) {
// 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'
// '/ignore:importeddllmain'
Warn(ctx)
<< file->getFileName()
<< ": skipping exported DllMain symbol [exporteddllmain]\nNOTE: this "
<< ": skipping imported DllMain symbol [importeddllmain]\nNOTE: this "
"might be a mistake when the DLL/library was produced.";
}
skipDllMain = true;
Expand Down Expand Up @@ -204,14 +202,24 @@ void ArchiveFile::parse() {
}
}

// Read the symbol table to construct Lazy objects.
bool skipDllMain = false;
StringRef mangledDllMain, impMangledDllMain;

// The calls below will fail if we haven't set the machine type yet. Instead
// of failing, it is preferable to skip this "imported DllMain" check if we
// don't know the machine type at this point.
if (!file->isEmpty() && ctx.config.machine != IMAGE_FILE_MACHINE_UNKNOWN) {
mangledDllMain = archiveSymtab->mangle("DllMain");
impMangledDllMain = uniqueSaver().save("__imp_" + mangledDllMain);
}

// Read the symbol table to construct Lazy objects.
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))
// 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 (!mangledDllMain.empty() && (sym.getName() == mangledDllMain ||
sym.getName() == impMangledDllMain)) {
if (skipDllMain || fixupDllMain(ctx, file.get(), sym, skipDllMain))
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this change in itself - why do we need to check if skipDllMain already was set? As far as I can see - the effect is to skip any (even a regular object) DllMain if we already saw an import object DllMain in the same archive. But if the objects would happen to be iterated over in a different order, that wouldn’t happen. Is this a functional aspect of this feature?

Copy link
Member Author

@aganea aganea Jul 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It is simply to avoid calling twice fixupDllMain() and avoid displaying the message twice for each LIB. This is because we iterate on the __imp_ symbols as well:

> dumpbin /all "C:\Program Files (x86)\Windows Kits\10\Lib\10.0.26100.0\um\x64\xinput.lib" | findstr DllMain
      936 DllMain
      936 __imp_DllMain
        4 DllMain
        4 __imp_DllMain
  Symbol name  : DllMain
             1    DllMain

continue;
}
archiveSymtab->addLazyArchive(this, sym);
Expand Down
58 changes: 58 additions & 0 deletions lld/test/COFF/imported-dllmain-i386.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
REQUIRES: x86
RUN: split-file %s %t.dir && cd %t.dir

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

RUN: llvm-mc -filetype=obj -triple=i386-windows b1.s -o b1.obj
RUN: llvm-mc -filetype=obj -triple=i386-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 -safeseh:no

RUN: llvm-mc -filetype=obj -triple=i386-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 -safeseh:no 2>&1 | FileCheck -check-prefix=WARN %s
RUN: lld-link a.obj b.lib c.lib -dll -out:out.dll -entry:DllMain -ignore:importeddllmain -safeseh:no 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 imported DllMain symbol [importeddllmain]
IGNORED-NOT: lld-link: warning: b.lib: skipping imported DllMain symbol [importeddllmain]

DISASM: The Import Tables:
DISASM: DLL Name: b.dll
DISASM-NOT: DllMain
DISASM: bar
DISASM: Disassembly of section .text:
DISASM: b0 01 movb $0x1, %al
DISASM-NEXT: c3 retl

#--- a.s
.text
.globl _foo
_foo:
call *__imp__bar
ret

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

#--- b2.s
.intel_syntax noprefix
.text
.globl _DllMain
_DllMain:
xor al, al
ret

#--- c.s
.intel_syntax noprefix
.text
.globl _DllMain
_DllMain:
mov al, 1
ret
6 changes: 3 additions & 3 deletions lld/test/COFF/exported-dllmain.test → lld/test/COFF/imported-dllmain.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ 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: lld-link a.obj b.lib c.lib -dll -out:out.dll -entry:DllMain -ignore:importeddllmain 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]
WARN: lld-link: warning: b.lib: skipping imported DllMain symbol [importeddllmain]
IGNORED-NOT: lld-link: warning: b.lib: skipping imported DllMain symbol [importeddllmain]

DISASM: The Import Tables:
DISASM: DLL Name: b.dll
Expand Down
Loading