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

aganea · 2025-07-05T15:08:54Z

This is a follow-up PR for post-commit comments in #146610

Changed "exporteddllmain" references into "importeddllmain".
Add support for x86 and test coverage.
Changed a comment to better express why we're skipping importing DllMain.

llvmbot · 2025-07-05T15:09:24Z

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld

Author: Alexandre Ganea (aganea)

Changes

This is a follow-up PR for post-commit comments in #146610

Changed "exporteddllmain" references into "importeddllmain".
Add support for x86 and test coverage.
Changed a comment to better express why we're skipping importing DllMain.

Full diff: https://github.com/llvm/llvm-project/pull/147152.diff

5 Files Affected:

(modified) lld/COFF/Config.h (+1-1)
(modified) lld/COFF/Driver.cpp (+2-2)
(modified) lld/COFF/InputFiles.cpp (+7-7)
(added) lld/test/COFF/imported-dllmain-i386.test (+59)
(renamed) lld/test/COFF/imported-dllmain.test (+3-3)

diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 79b63e5b7236f..9220c847f356d 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -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;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 283aeed1a19cd..0f649ff1cf4f8 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -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.
     }
   }
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 0b7dbea8cdd99..1edd24cdf2dea 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -128,13 +128,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;
@@ -207,10 +207,10 @@ void ArchiveFile::parse() {
   // Read the symbol table to construct Lazy objects.
   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 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" ||
+        sym.getName() == "_DllMain") {
       if (fixupDllMain(ctx, file.get(), sym, skipDllMain))
         continue;
     }
diff --git a/lld/test/COFF/imported-dllmain-i386.test b/lld/test/COFF/imported-dllmain-i386.test
new file mode 100644
index 0000000000000..a9cd94d089e29
--- /dev/null
+++ b/lld/test/COFF/imported-dllmain-i386.test
@@ -0,0 +1,59 @@
+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-EMPTY:
+DISASM-NEXT: 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
diff --git a/lld/test/COFF/exported-dllmain.test b/lld/test/COFF/imported-dllmain.test
similarity index 86%
rename from lld/test/COFF/exported-dllmain.test
rename to lld/test/COFF/imported-dllmain.test
index fcf6ed1005379..fa8579b1b41c5 100644
--- a/lld/test/COFF/exported-dllmain.test
+++ b/lld/test/COFF/imported-dllmain.test
@@ -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

llvmbot · 2025-07-05T15:09:24Z

@llvm/pr-subscribers-lld-coff

Author: Alexandre Ganea (aganea)

Changes

This is a follow-up PR for post-commit comments in #146610

Changed "exporteddllmain" references into "importeddllmain".
Add support for x86 and test coverage.
Changed a comment to better express why we're skipping importing DllMain.

Full diff: https://github.com/llvm/llvm-project/pull/147152.diff

5 Files Affected:

(modified) lld/COFF/Config.h (+1-1)
(modified) lld/COFF/Driver.cpp (+2-2)
(modified) lld/COFF/InputFiles.cpp (+7-7)
(added) lld/test/COFF/imported-dllmain-i386.test (+59)
(renamed) lld/test/COFF/imported-dllmain.test (+3-3)

diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 79b63e5b7236f..9220c847f356d 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -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;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 283aeed1a19cd..0f649ff1cf4f8 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -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.
     }
   }
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 0b7dbea8cdd99..1edd24cdf2dea 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -128,13 +128,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;
@@ -207,10 +207,10 @@ void ArchiveFile::parse() {
   // Read the symbol table to construct Lazy objects.
   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 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" ||
+        sym.getName() == "_DllMain") {
       if (fixupDllMain(ctx, file.get(), sym, skipDllMain))
         continue;
     }
diff --git a/lld/test/COFF/imported-dllmain-i386.test b/lld/test/COFF/imported-dllmain-i386.test
new file mode 100644
index 0000000000000..a9cd94d089e29
--- /dev/null
+++ b/lld/test/COFF/imported-dllmain-i386.test
@@ -0,0 +1,59 @@
+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-EMPTY:
+DISASM-NEXT: 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
diff --git a/lld/test/COFF/exported-dllmain.test b/lld/test/COFF/imported-dllmain.test
similarity index 86%
rename from lld/test/COFF/exported-dllmain.test
rename to lld/test/COFF/imported-dllmain.test
index fcf6ed1005379..fa8579b1b41c5 100644
--- a/lld/test/COFF/exported-dllmain.test
+++ b/lld/test/COFF/imported-dllmain.test
@@ -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

lld/COFF/InputFiles.cpp

mstorsjo · 2025-07-17T19:26:23Z

Ping - it would be great to have this settled and fixed for the 21.x release branch.

aganea · 2025-07-17T19:36:36Z

Ping - it would be great to have this settled and fixed for the 21.x release branch.

Sorry I have been taken with other things, I will update this today.

mstorsjo · 2025-07-18T05:58:22Z

lld/COFF/InputFiles.cpp

+    // 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))


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?

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

mstorsjo

LGTM, thanks! It sounds like a reasonable compromise to just do this if we know the machine type already.

mstorsjo · 2025-07-21T21:13:27Z

/cherry-pick fcacd4e

llvmbot · 2025-07-21T21:19:20Z

/pull-request #149906

This is a follow-up PR for post-commit comments in llvm#146610 - Changed "exporteddllmain" references to "importeddllmain". - Add support for x86 target and test coverage. - Changed a comment to better express why we're skipping importing `DllMain`. (cherry picked from commit fcacd4e)

Follow up comments on pr146610

9a4b6b7

aganea requested review from rnk, mstorsjo and cjacek July 5, 2025 15:08

llvmbot added lld lld:COFF platform:windows labels Jul 5, 2025

aganea mentioned this pull request Jul 5, 2025

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

Merged

mstorsjo reviewed Jul 5, 2025

View reviewed changes

lld/COFF/InputFiles.cpp Outdated Show resolved Hide resolved

aganea added 2 commits July 17, 2025 18:05

Address review comments especially i386 support

25f3b01

Revert some of the previous changes

312fd39

mstorsjo reviewed Jul 18, 2025

View reviewed changes

mstorsjo approved these changes Jul 18, 2025

View reviewed changes

aganea merged commit fcacd4e into llvm:main Jul 21, 2025
9 checks passed

mstorsjo added this to the LLVM 21.x Release milestone Jul 21, 2025

github-project-automation bot added this to LLVM Release Status Jul 21, 2025

github-project-automation bot moved this to Needs Triage in LLVM Release Status Jul 21, 2025

llvmbot moved this from Needs Triage to Done in LLVM Release Status Jul 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

aganea commented Jul 5, 2025

Uh oh!

llvmbot commented Jul 5, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 5, 2025

Uh oh!

Uh oh!

mstorsjo commented Jul 17, 2025

Uh oh!

aganea commented Jul 17, 2025

Uh oh!

mstorsjo Jul 18, 2025

Uh oh!

aganea Jul 18, 2025 •

edited

Loading

Uh oh!

mstorsjo left a comment

Uh oh!

Uh oh!

mstorsjo commented Jul 21, 2025

Uh oh!

llvmbot commented Jul 21, 2025

Uh oh!

Uh oh!

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

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

Conversation

aganea commented Jul 5, 2025

Uh oh!

llvmbot commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 5, 2025

Uh oh!

Uh oh!

mstorsjo commented Jul 17, 2025

Uh oh!

aganea commented Jul 17, 2025

Uh oh!

mstorsjo Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

aganea Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstorsjo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mstorsjo commented Jul 21, 2025

Uh oh!

llvmbot commented Jul 21, 2025

Uh oh!

Uh oh!

llvmbot commented Jul 5, 2025 •

edited

Loading

aganea Jul 18, 2025 •

edited

Loading