-
Notifications
You must be signed in to change notification settings - Fork 15.6k
[LLD] [COFF] Fix implicit DLL entry point for MinGW #171680
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
Conversation
|
A note for the reviewers: I hardly ever use C/C++ these days, so don't hesitate to point out if something can be done in a better way. |
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-lld-coff Author: Mateusz Mikuła (mati865) ChangesPreviously LLD would always set implicit entry point for DLLs to the symbol that is prefixed with an underscore. However, mingw-w64 defines it without that underscore. This change fixes that by adding a special branch for MinGW. Also, it simplifies tests that use MinGW style DLL entry symbol by skipping entry point argument. Fixes #171441 Full diff: https://github.com/llvm/llvm-project/pull/171680.diff 12 Files Affected:
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 0e528de9c3652..05679255703e9 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1489,6 +1489,14 @@ getVFS(COFFLinkerContext &ctx, const opt::InputArgList &args) {
return nullptr;
}
+StringRef DllDefaultEntryPoint(MachineTypes machine, bool mingw) {
+ if (mingw) {
+ return (machine == I386) ? "_DllMainCRTStartup@12" : "DllMainCRTStartup";
+ } else {
+ return (machine == I386) ? "__DllMainCRTStartup@12" : "_DllMainCRTStartup";
+ }
+}
+
constexpr const char *lldsaveTempsValues[] = {
"resolution", "preopt", "promote", "internalize", "import",
"opt", "precodegen", "prelink", "combinedindex"};
@@ -2408,8 +2416,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
symtab.entry = symtab.addGCRoot(symtab.mangle(arg->getValue()), true);
} else if (!symtab.entry && !config->noEntry) {
if (args.hasArg(OPT_dll)) {
- StringRef s = (config->machine == I386) ? "__DllMainCRTStartup@12"
- : "_DllMainCRTStartup";
+ StringRef s = DllDefaultEntryPoint(config->machine, config->mingw);
symtab.entry = symtab.addGCRoot(s, true);
} else if (config->driverWdm) {
// /driver:wdm implies /entry:_NtProcessStartup
diff --git a/lld/test/COFF/autoimport-arm-code.s b/lld/test/COFF/autoimport-arm-code.s
index 562a1a959e38b..8d7cc7f5cb019 100644
--- a/lld/test/COFF/autoimport-arm-code.s
+++ b/lld/test/COFF/autoimport-arm-code.s
@@ -2,7 +2,7 @@
# RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.thumb\n.text\nDllMainCRTStartup:\nbx lr\n.data\nvariable:\n.long 42" > %t-lib.s
# RUN: llvm-mc -triple=armv7-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
-# RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+# RUN: lld-link -out:%t-lib.dll -dll %t-lib.obj -lldmingw -implib:%t-lib.lib
# RUN: llvm-mc -triple=armv7-windows-gnu %s -filetype=obj -o %t.obj
# RUN: not lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-lib.lib 2>&1 | FileCheck %s
diff --git a/lld/test/COFF/autoimport-arm-data.s b/lld/test/COFF/autoimport-arm-data.s
index 82c66f0989d49..efc19f7f1865d 100644
--- a/lld/test/COFF/autoimport-arm-data.s
+++ b/lld/test/COFF/autoimport-arm-data.s
@@ -2,7 +2,7 @@
# RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.thumb\n.text\nDllMainCRTStartup:\nbx lr\n.data\nvariable:\n.long 42" > %t-lib.s
# RUN: llvm-mc -triple=armv7-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
-# RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+# RUN: lld-link -out:%t-lib.dll -dll %t-lib.obj -lldmingw -implib:%t-lib.lib
# RUN: llvm-mc -triple=armv7-windows-gnu %s -filetype=obj -o %t.obj
# RUN: lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-lib.lib -verbose
diff --git a/lld/test/COFF/autoimport-arm64-code.s b/lld/test/COFF/autoimport-arm64-code.s
index 9f5cc8f894255..b7cddf9bac040 100644
--- a/lld/test/COFF/autoimport-arm64-code.s
+++ b/lld/test/COFF/autoimport-arm64-code.s
@@ -2,7 +2,7 @@
# RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\n.data\nvariable:\n.long 42" > %t-lib.s
# RUN: llvm-mc -triple=aarch64-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
-# RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+# RUN: lld-link -out:%t-lib.dll -dll %t-lib.obj -lldmingw -implib:%t-lib.lib
# RUN: llvm-mc -triple=aarch64-windows-gnu %s -filetype=obj -o %t.obj
# RUN: not lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-lib.lib 2>&1 | FileCheck %s
diff --git a/lld/test/COFF/autoimport-arm64-data.s b/lld/test/COFF/autoimport-arm64-data.s
index 7934e2a969257..f3f16bc5210a4 100644
--- a/lld/test/COFF/autoimport-arm64-data.s
+++ b/lld/test/COFF/autoimport-arm64-data.s
@@ -2,7 +2,7 @@
# RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\n.data\nvariable:\n.long 42" > %t-lib.s
# RUN: llvm-mc -triple=aarch64-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
-# RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+# RUN: lld-link -out:%t-lib.dll -dll %t-lib.obj -lldmingw -implib:%t-lib.lib
# RUN: llvm-mc -triple=aarch64-windows-gnu %s -filetype=obj -o %t.obj
# RUN: lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-lib.lib -verbose
diff --git a/lld/test/COFF/autoimport-debug.s b/lld/test/COFF/autoimport-debug.s
index 1f31a2b9e554f..5865dac8d0d18 100644
--- a/lld/test/COFF/autoimport-debug.s
+++ b/lld/test/COFF/autoimport-debug.s
@@ -10,7 +10,7 @@
## debug info.
# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/lib.s -filetype=obj -o %t.dir/lib.obj
-# RUN: lld-link -out:%t.dir/lib.dll -dll -entry:DllMainCRTStartup %t.dir/lib.obj -lldmingw -implib:%t.dir/lib.lib
+# RUN: lld-link -out:%t.dir/lib.dll -dll %t.dir/lib.obj -lldmingw -implib:%t.dir/lib.lib
# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/main.s -filetype=obj -o %t.dir/main.obj
# RUN: lld-link -lldmingw -out:%t.dir/main.exe -entry:main %t.dir/main.obj %t.dir/lib.lib -opt:noref -debug:dwarf -runtime-pseudo-reloc:no
diff --git a/lld/test/COFF/autoimport-gc.s b/lld/test/COFF/autoimport-gc.s
index fef6c02eba82f..f97fe4f534d27 100644
--- a/lld/test/COFF/autoimport-gc.s
+++ b/lld/test/COFF/autoimport-gc.s
@@ -2,7 +2,7 @@
# RUN: split-file %s %t.dir
# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/lib.s -filetype=obj -o %t.dir/lib.obj
-# RUN: lld-link -out:%t.dir/lib.dll -dll -entry:DllMainCRTStartup %t.dir/lib.obj -lldmingw -implib:%t.dir/lib.lib
+# RUN: lld-link -out:%t.dir/lib.dll -dll %t.dir/lib.obj -lldmingw -implib:%t.dir/lib.lib
# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/main.s -filetype=obj -o %t.dir/main.obj
# RUN: lld-link -lldmingw -out:%t.dir/main.exe -entry:main %t.dir/main.obj %t.dir/lib.lib -opt:ref -debug:dwarf
diff --git a/lld/test/COFF/autoimport-lto.ll b/lld/test/COFF/autoimport-lto.ll
index 94210e43cda6c..9a63d2f232d4e 100644
--- a/lld/test/COFF/autoimport-lto.ll
+++ b/lld/test/COFF/autoimport-lto.ll
@@ -2,7 +2,7 @@
; RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\n.data\nvariable:\n.long 42" > %t-lib.s
; RUN: llvm-mc -triple=x86_64-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
-; RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+; RUN: lld-link -out:%t-lib.dll -dll %t-lib.obj -lldmingw -implib:%t-lib.lib
; RUN: llvm-as -o %t.obj %s
; RUN: lld-link -lldmingw -out:%t.exe -entry:entry %t.obj %t-lib.lib
diff --git a/lld/test/COFF/autoimport-refptr.s b/lld/test/COFF/autoimport-refptr.s
index 4071d6be0809e..b087dd9363553 100644
--- a/lld/test/COFF/autoimport-refptr.s
+++ b/lld/test/COFF/autoimport-refptr.s
@@ -2,7 +2,7 @@
# RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\n.data\nvariable:\n.long 42" > %t-lib.s
# RUN: llvm-mc -triple=x86_64-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
-# RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+# RUN: lld-link -out:%t-lib.dll -dll %t-lib.obj -lldmingw -implib:%t-lib.lib
# RUN: llvm-mc -triple=x86_64-windows-gnu %s -defsym listptrs=1 -filetype=obj -o %t.obj
# RUN: lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-lib.lib -verbose
diff --git a/lld/test/COFF/autoimport-x86.s b/lld/test/COFF/autoimport-x86.s
index 5d7c9c2c3fa58..39f2457a8259d 100644
--- a/lld/test/COFF/autoimport-x86.s
+++ b/lld/test/COFF/autoimport-x86.s
@@ -2,7 +2,7 @@
# RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\n.data\nvariable:\n.long 42" > %t-lib.s
# RUN: llvm-mc -triple=x86_64-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
-# RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+# RUN: lld-link -out:%t-lib.dll -dll %t-lib.obj -lldmingw -implib:%t-lib.lib
# RUN: llvm-mc -triple=x86_64-windows-gnu -defsym listptrs=1 %s -filetype=obj -o %t.obj
# RUN: lld-link -lldmingw -debug:symtab -out:%t.exe -entry:main %t.obj %t-lib.lib -verbose
diff --git a/lld/test/COFF/exclude-all.s b/lld/test/COFF/exclude-all.s
index f1977836b18cf..03dc0ddc6296e 100644
--- a/lld/test/COFF/exclude-all.s
+++ b/lld/test/COFF/exclude-all.s
@@ -2,7 +2,7 @@
# RUN: llvm-mc -triple=i686-windows-gnu %s -filetype=obj -o %t.obj
-# RUN: lld-link -lldmingw -exclude-all-symbols -dll -out:%t.dll -entry:DllMainCRTStartup@12 %t.obj
+# RUN: lld-link -lldmingw -exclude-all-symbols -dll -out:%t.dll %t.obj
# RUN: llvm-readobj --coff-exports %t.dll | FileCheck %s -check-prefix=NO-EXPORTS
# NO-EXPORTS-NOT: Name:
@@ -25,7 +25,7 @@ _dataSym:
# RUN: yaml2obj %p/Inputs/export.yaml -o %t.obj
#
-# RUN: lld-link -safeseh:no -out:%t.dll -dll %t.obj -lldmingw -exclude-all-symbols -output-def:%t.def
+# RUN: lld-link -safeseh:no -out:%t.dll -dll %t.obj -lldmingw -exclude-all-symbols -entry:_DllMainCRTStartup -output-def:%t.def
# RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix=DLLEXPORT %s
# DLLEXPORT: Name: exportfn3
diff --git a/lld/test/COFF/export-all.s b/lld/test/COFF/export-all.s
index cd0a6f5f7b35a..70266f37809ca 100644
--- a/lld/test/COFF/export-all.s
+++ b/lld/test/COFF/export-all.s
@@ -2,7 +2,7 @@
# RUN: llvm-mc -triple=i686-windows-gnu %s -filetype=obj -o %t.obj
-# RUN: lld-link -lldmingw -dll -out:%t.dll -entry:DllMainCRTStartup@12 %t.obj -implib:%t.lib
+# RUN: lld-link -lldmingw -dll -out:%t.dll %t.obj -implib:%t.lib
# RUN: llvm-readobj --coff-exports %t.dll | grep Name: | FileCheck %s
# RUN: llvm-readobj --coff-exports %t.dll | FileCheck %s --check-prefix=CHECK-RVA
# RUN: llvm-readobj %t.lib | FileCheck -check-prefix=IMPLIB %s
@@ -57,7 +57,7 @@ __imp__unexported:
# RUN: yaml2obj %p/Inputs/export.yaml -o %t.obj
#
-# RUN: lld-link -safeseh:no -out:%t.dll -dll %t.obj -lldmingw -export-all-symbols -output-def:%t.def
+# RUN: lld-link -safeseh:no -out:%t.dll -dll %t.obj -lldmingw -export-all-symbols -entry:_DllMainCRTStartup -output-def:%t.def
# RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix=CHECK2 %s
# RUN: cat %t.def | FileCheck -check-prefix=CHECK2-DEF %s
@@ -88,7 +88,7 @@ __imp__unexported:
# RUN: llvm-ar rcs %t.dir/libs/libmingwex.a %t.dir/libs/mingwfunc.o
# RUN: echo -e ".global crtfunc\n.text\ncrtfunc:\nret\n" > %t.dir/libs/crtfunc.s
# RUN: llvm-mc -triple=x86_64-windows-gnu %t.dir/libs/crtfunc.s -filetype=obj -o %t.dir/libs/crt2.o
-# RUN: lld-link -safeseh:no -out:%t.dll -dll -entry:DllMainCRTStartup %t.main.obj -lldmingw %t.dir/libs/crt2.o %t.dir/libs/libmingwex.a -output-def:%t.def
+# RUN: lld-link -safeseh:no -out:%t.dll -dll %t.main.obj -lldmingw %t.dir/libs/crt2.o %t.dir/libs/libmingwex.a -output-def:%t.def
# RUN: echo "EOF" >> %t.def
# RUN: cat %t.def | FileCheck -check-prefix=CHECK-EXCLUDE %s
@@ -99,7 +99,7 @@ __imp__unexported:
# Test that libraries included with -wholearchive: are autoexported, even if
# they are in a library that otherwise normally would be excluded.
-# RUN: lld-link -safeseh:no -out:%t.dll -dll -entry:DllMainCRTStartup %t.main.obj -lldmingw %t.dir/libs/crt2.o -wholearchive:%t.dir/libs/libmingwex.a -output-def:%t.def
+# RUN: lld-link -safeseh:no -out:%t.dll -dll %t.main.obj -lldmingw %t.dir/libs/crt2.o -wholearchive:%t.dir/libs/libmingwex.a -output-def:%t.def
# RUN: echo "EOF" >> %t.def
# RUN: cat %t.def | FileCheck -check-prefix=CHECK-WHOLEARCHIVE %s
|
mstorsjo
left a comment
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.
Looks good in general, just one small nit on the code, and two on the commit message (PR description).
For the commit message, prefixing it with [LLD] [COFF] could be more correct, since this is changing the code for the COFF linker part, even though it is specific to mingw mode use of it.
Also, it could be good to clarify, that when linking is driven from Clang (or GCC), the compiler driver passes an explicit entry point, so this shouldn't change anything for those cases.
For reference, so we know the implicit default is the right one, we can check https://github.com/llvm/llvm-project/blob/llvmorg-21.1.8/clang/lib/Driver/ToolChains/MinGW.cpp#L176-L180 which passes -e DllMainCRTStartup or -e _DllMainCRTStartup@12. https://github.com/llvm/llvm-project/blob/llvmorg-21.1.8/lld/MinGW/Driver.cpp#L235-L243 does strip out the leading underscore on i386, which gets put back by the symtab.mangle(arg->getValue() a few lines above the call to DllDefaultEntryPoint. TL;DR, the code change looks correct.
lld/COFF/Driver.cpp
Outdated
| return nullptr; | ||
| } | ||
|
|
||
| StringRef DllDefaultEntryPoint(MachineTypes machine, bool mingw) { |
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.
This could be static (as long as we're not in an anonymous namespace; the inline diff makes it hard to tell, but the function above seems to be static).
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.
Ah, good catch. I had forgotten that functions in C(++) are global by default.
Previously, LLD would always set the implicit entry point for DLLs to the symbol that is prefixed with an underscore. However, mingw-w64 defines it without that underscore. This change fixes that by adding a special branch for MinGW. Also, it simplifies tests that use MinGW style DLL entry symbol by skipping the entry point argument. Note, tests that use MSVC style DLL entry symbol and LLD in MinGW mode, will now require using explicit entry point. I believe this is sensible. When an explicit entry point is passed, i.e. LLD is called by Clang or GCC, there will be no observable difference.
e2005b1 to
6c536c0
Compare
|
Thank you for the review. I have slightly improved the description and expanded the note with mention of Clang and GCC. |
mstorsjo
left a comment
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.
LGTM, thanks!
|
I don't have any push/write rights, can you merge it for me (unless waiting for another reviewer)? |
|
Yeah; I can merge it later tonight - just giving others a courtesy period to comment further on it (even though it's been up for review for a week) before pushing the button. |
Previously, LLD would always set the implicit entry point for DLLs to the symbol that is prefixed with an underscore. However, mingw-w64 defines it without that underscore. This change fixes that by adding a special branch for MinGW. Also, it simplifies tests that use MinGW style DLL entry symbol by skipping the entry point argument. Note, tests that use MSVC style DLL entry symbol and LLD in MinGW mode, will now require using explicit entry point. I believe this is sensible. When an explicit entry point is passed, i.e. LLD is called by Clang or GCC, there will be no observable difference. Fixes llvm#171441 (cherry picked from commit 2824225)
Previously, LLD would always set the implicit entry point for DLLs to the symbol that is prefixed with an underscore. However, mingw-w64 defines it without that underscore. This change fixes that by adding a special branch for MinGW. Also, it simplifies tests that use MinGW style DLL entry symbol by skipping the entry point argument. Note, tests that use MSVC style DLL entry symbol and LLD in MinGW mode, will now require using explicit entry point. I believe this is sensible. When an explicit entry point is passed, i.e. LLD is called by Clang or GCC, there will be no observable difference. Fixes llvm#171441
Previously, LLD would always set the implicit entry point for DLLs to the symbol that is prefixed with an underscore. However, mingw-w64 defines it without that underscore. This change fixes that by adding a special branch for MinGW. Also, it simplifies tests that use MinGW style DLL entry symbol by skipping the entry point argument. Note, tests that use MSVC style DLL entry symbol and LLD in MinGW mode, will now require using explicit entry point. I believe this is sensible. When an explicit entry point is passed, i.e. LLD is called by Clang or GCC, there will be no observable difference. Fixes llvm#171441
Previously, LLD would always set the implicit entry point for DLLs to the symbol that is prefixed with an underscore. However, mingw-w64 defines it without that underscore.
This change fixes that by adding a special branch for MinGW. Also, it simplifies tests that use MinGW style DLL entry symbol by skipping the entry point argument.
Note, tests that use MSVC style DLL entry symbol and LLD in MinGW mode, will now require using explicit entry point. I believe this is sensible. When an explicit entry point is passed, i.e. LLD is called by Clang or GCC, there will be no observable difference.
Fixes #171441