From 7c41177de2d62e456ebe04754d4d86de26527824 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Fri, 18 Aug 2023 19:48:52 +0200 Subject: [PATCH 1/6] [add tests] --- .github/workflows/main.yml | 2 +- test/dynLib-monolith.sh | 16 ++++++++++++++++ test/dynLib-monolith/.no_run | 0 test/dynLib-monolith/.no_test | 0 test/dynLib-monolith/dub.sdl | 15 +++++++++++++++ test/dynLib-monolith/inner_dep/dub.sdl | 4 ++++ .../inner_dep/source/inner_dep/mod.d | 7 +++++++ test/dynLib-monolith/source/library.d | 7 +++++++ test/exe-shared-defaultlib/.no_test | 0 test/exe-shared-defaultlib/dub.sdl | 11 +++++++++++ test/exe-shared-defaultlib/source/app.d | 7 +++++++ 11 files changed, 68 insertions(+), 1 deletion(-) create mode 100755 test/dynLib-monolith.sh create mode 100644 test/dynLib-monolith/.no_run create mode 100644 test/dynLib-monolith/.no_test create mode 100644 test/dynLib-monolith/dub.sdl create mode 100644 test/dynLib-monolith/inner_dep/dub.sdl create mode 100644 test/dynLib-monolith/inner_dep/source/inner_dep/mod.d create mode 100644 test/dynLib-monolith/source/library.d create mode 100644 test/exe-shared-defaultlib/.no_test create mode 100644 test/exe-shared-defaultlib/dub.sdl create mode 100644 test/exe-shared-defaultlib/source/app.d diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f86f2f74d..c33b57367 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -102,7 +102,7 @@ jobs: # FIXME: DMD fails a few tests on Windows; remove them for now if [[ '${{ matrix.dc }}' = dmd* ]]; then # DLL support is lacking - rm -rf test/{1-dynLib-simple,2-dynLib-dep,2-dynLib-with-staticLib-dep} + rm -rf test/{1-dynLib-simple,2-dynLib-dep,2-dynLib-with-staticLib-dep,dynLib-monolith} # Unicode in paths too rm -rf test/issue130-unicode-СНА* # ImportC probably requires set-up MSVC environment variables diff --git a/test/dynLib-monolith.sh b/test/dynLib-monolith.sh new file mode 100755 index 000000000..b13c9a83e --- /dev/null +++ b/test/dynLib-monolith.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash + +DIR=$(dirname "${BASH_SOURCE[0]}") +. "$DIR"/common.sh + +if [[ "$DC" != ldc* ]]; then + echo "Skipping test, needs LDC" + exit 0 +fi + +# enforce a full build (2 static libs, 1 dynamic one) and collect -v output +output=$("$DUB" build -v -f --root "$DIR"/dynLib-monolith) + +if [[ $(grep -c -- '-fvisibility=hidden' <<<"$output") -ne 3 ]]; then + die $LINENO "Didn't find 3 occurrences of '-fvisibility=hidden' in the verbose dub output!" "$output" +fi diff --git a/test/dynLib-monolith/.no_run b/test/dynLib-monolith/.no_run new file mode 100644 index 000000000..e69de29bb diff --git a/test/dynLib-monolith/.no_test b/test/dynLib-monolith/.no_test new file mode 100644 index 000000000..e69de29bb diff --git a/test/dynLib-monolith/dub.sdl b/test/dynLib-monolith/dub.sdl new file mode 100644 index 000000000..90b28d884 --- /dev/null +++ b/test/dynLib-monolith/dub.sdl @@ -0,0 +1,15 @@ +name "dynlib-monolith" +description "A 'monolithic' dynamic library, linking all dependencies statically (except for shared druntime/Phobos) and exporting select symbols explicitly via 'export'" +targetType "dynamicLibrary" +dependency "inner_dep" path="inner_dep" + +# With LDC, `-fvisibility=hidden` is passed down for compiling all dependencies. +dflags "-fvisibility=hidden" platform="ldc" +dflags "-fvisibility=hidden" platform="gdc" +# not supported by DMD + +# With LDC on Windows, code ending up in a DLL is compiled with +# `-fvisibility=public -dllimport=all` by default. So we additionally need +# to override `-dllimport` to use the shared druntime/Phobos DLLs (used by +# default for dynamic libraries), which is also passed down to all deps: +dflags "-dllimport=defaultLibsOnly" platform="windows-ldc" diff --git a/test/dynLib-monolith/inner_dep/dub.sdl b/test/dynLib-monolith/inner_dep/dub.sdl new file mode 100644 index 000000000..4535eaaf1 --- /dev/null +++ b/test/dynLib-monolith/inner_dep/dub.sdl @@ -0,0 +1,4 @@ +name "inner_dep" +description "A static library depending on another static library" +targetType "staticLibrary" +dependency "staticlib-simple" path="../../1-staticLib-simple" diff --git a/test/dynLib-monolith/inner_dep/source/inner_dep/mod.d b/test/dynLib-monolith/inner_dep/source/inner_dep/mod.d new file mode 100644 index 000000000..123866e7f --- /dev/null +++ b/test/dynLib-monolith/inner_dep/source/inner_dep/mod.d @@ -0,0 +1,7 @@ +module inner_dep.mod; + +void innerDepFunction() +{ + import staticlib.app; + entry(); +} diff --git a/test/dynLib-monolith/source/library.d b/test/dynLib-monolith/source/library.d new file mode 100644 index 000000000..80bef3702 --- /dev/null +++ b/test/dynLib-monolith/source/library.d @@ -0,0 +1,7 @@ +module library; + +export void foo() +{ + import inner_dep.mod; + innerDepFunction(); +} diff --git a/test/exe-shared-defaultlib/.no_test b/test/exe-shared-defaultlib/.no_test new file mode 100644 index 000000000..e69de29bb diff --git a/test/exe-shared-defaultlib/dub.sdl b/test/exe-shared-defaultlib/dub.sdl new file mode 100644 index 000000000..734c20ec2 --- /dev/null +++ b/test/exe-shared-defaultlib/dub.sdl @@ -0,0 +1,11 @@ +name "exe-shared-defaultlib" +description "An executable with a statically linked dep, linked against *shared* druntime/Phobos" +targetType "executable" +dependency "staticlib-simple" path="../1-staticLib-simple/" + +# With LDC on Windows, `-link-defaultlib-shared` is passed down for compiling +# staticlib-simple (for an implicit `-dllimport=defaultLibsOnly`). +dflags "-link-defaultlib-shared" platform="ldc" + +# no libphobos2.dll/dylib with DMD on Windows/Mac +dflags "-defaultlib=libphobos2.so" platform="linux-dmd" diff --git a/test/exe-shared-defaultlib/source/app.d b/test/exe-shared-defaultlib/source/app.d new file mode 100644 index 000000000..36aa99b12 --- /dev/null +++ b/test/exe-shared-defaultlib/source/app.d @@ -0,0 +1,7 @@ +module app; +import staticlib.app; + +void main() +{ + entry(); +} From b76ef13fdfc29aeee4de43316d0c08af125b0303 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sat, 19 Aug 2023 02:23:04 +0200 Subject: [PATCH 2/6] [convert test/dynLib-monolith.sh to dynLib-monolith.script.d for Windows support] --- test/dynLib-monolith.script.d | 39 +++++++++++++++++++++++++++++++++++ test/dynLib-monolith.sh | 16 -------------- 2 files changed, 39 insertions(+), 16 deletions(-) create mode 100644 test/dynLib-monolith.script.d delete mode 100755 test/dynLib-monolith.sh diff --git a/test/dynLib-monolith.script.d b/test/dynLib-monolith.script.d new file mode 100644 index 000000000..f31b64380 --- /dev/null +++ b/test/dynLib-monolith.script.d @@ -0,0 +1,39 @@ +/+ dub.json: { + "name": "dynlib-monolith-script" +} +/ +module dynlib_monolith_script; + +import std.algorithm, std.path, std.process, std.stdio; + +int main() +{ + const dc = environment.get("DC", "dmd"); + if (!dc.startsWith("ldc")) + { + writeln("Skipping test, needs LDC"); + return 0; + } + + // enforce a full build (2 static libs, 1 dynamic one) and collect -v output + enum projDir = dirName(__FILE_FULL_PATH__).buildPath("dynLib-monolith"); + const res = execute([environment.get("DUB", "dub"), "build", "-f", "-v", "--root", projDir]); + + int errorOut(string msg) + { + writeln("Error: " ~ msg); + writeln("==========================================================="); + writeln(res.output); + writeln("==========================================================="); + return 1; + } + + if (res.status != 0) + return errorOut("The dub invocation failed:"); + + version (Windows) enum needle = " -fvisibility=hidden -dllimport=defaultLibsOnly"; + else enum needle = " -fvisibility=hidden"; + if (res.output.count(needle) != 3) + return errorOut("Cannot find exactly 3 occurrences of '" ~ needle ~ "' in the verbose dub output:"); + + return 0; +} diff --git a/test/dynLib-monolith.sh b/test/dynLib-monolith.sh deleted file mode 100755 index b13c9a83e..000000000 --- a/test/dynLib-monolith.sh +++ /dev/null @@ -1,16 +0,0 @@ -#!/usr/bin/env bash - -DIR=$(dirname "${BASH_SOURCE[0]}") -. "$DIR"/common.sh - -if [[ "$DC" != ldc* ]]; then - echo "Skipping test, needs LDC" - exit 0 -fi - -# enforce a full build (2 static libs, 1 dynamic one) and collect -v output -output=$("$DUB" build -v -f --root "$DIR"/dynLib-monolith) - -if [[ $(grep -c -- '-fvisibility=hidden' <<<"$output") -ne 3 ]]; then - die $LINENO "Didn't find 3 occurrences of '-fvisibility=hidden' in the verbose dub output!" "$output" -fi From 81c371e587581890e264d619a00a0f09f76b3fa0 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Fri, 18 Aug 2023 19:49:50 +0200 Subject: [PATCH 3/6] LDC: Apply dflags affecting symbol visibility to all deps Example scenarios on Windows: * If an .exe is linked against druntime/Phobos DLLs (e.g., to enable loading other D DLLs, all sharing central druntime/Phobos), all static-lib dub dependencies need to be compiled with `-dllimport=defaultLibsOnly` too [the default with `-link-defaultlib-shared`]. * To make large DLLs with few selective `export`ed symbols work after #2412, the implicit flags need to be overridden - for the DLL itself and all its static-lib dependencies. This can currently only be accomplished by setting the DFLAGS environment variable. This PR enables overriding these `dflags` either in the DLL root project/ config directly, or in some dependency shared by multiple DLLs/ executables. E.g., at Symmetry, we have an .exe with lots of plugin DLLs. All plugins and the .exe have a shared dependency, so adding `dflags "-fvisibility=hidden" "-dllimport=defaultLibsOnly" platform="windows-ldc"` once in the dub.sdl of that shared dependency suffices to compile *everything* with those required flags. --- source/dub/generators/generator.d | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/source/dub/generators/generator.d b/source/dub/generators/generator.d index 88d719ef6..5a04accfe 100644 --- a/source/dub/generators/generator.d +++ b/source/dub/generators/generator.d @@ -459,6 +459,39 @@ class ProjectGenerator configureDependents(*roottarget, targets); visited.clear(); + // LDC: need to pass down dflags affecting symbol visibility, especially on Windows + if (genSettings.platform.compiler == "ldc") + { + const isWindows = genSettings.platform.isWindows(); + bool passDownDFlag(string flag) + { + if (flag.startsWith("--")) + flag = flag[1 .. $]; + return flag.startsWith("-fvisibility=") || (isWindows && + (flag.startsWith("-link-defaultlib-shared") || + flag.startsWith("-dllimport="))); + } + + // all dflags from dependencies have already been added to the root project + auto rootDFlagsToPassDown = roottarget.buildSettings.dflags.filter!passDownDFlag.array; + + if (rootDFlagsToPassDown.length) + { + foreach (name, ref ti; targets) + { + if (&ti != roottarget) + { + import std.range : chain; + ti.buildSettings.dflags = ti.buildSettings.dflags + // remove all existing visibility flags first to reduce duplicates + .filter!(f => !passDownDFlag(f)) + .chain(rootDFlagsToPassDown) + .array; + } + } + } + } + // 4. As an extension to configureDependents we need to copy any injectSourceFiles // in our dependencies (ignoring targetType) void configureDependentsFinalImages(ref TargetInfo ti, TargetInfo[string] targets, ref TargetInfo finalBinaryTarget, size_t level = 0) From 7378056972de3df11cc0a26b15c5ccec3bb09d46 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sat, 19 Aug 2023 04:04:16 +0200 Subject: [PATCH 4/6] [minimally improve comments] --- test/dynLib-monolith/dub.sdl | 10 ++++++---- test/exe-shared-defaultlib/dub.sdl | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/test/dynLib-monolith/dub.sdl b/test/dynLib-monolith/dub.sdl index 90b28d884..4c68861ee 100644 --- a/test/dynLib-monolith/dub.sdl +++ b/test/dynLib-monolith/dub.sdl @@ -1,5 +1,5 @@ name "dynlib-monolith" -description "A 'monolithic' dynamic library, linking all dependencies statically (except for shared druntime/Phobos) and exporting select symbols explicitly via 'export'" +description "A 'monolithic' dynamic library, linking all dependencies statically (except for shared druntime/Phobos) and exporting only select symbols explicitly via 'export'" targetType "dynamicLibrary" dependency "inner_dep" path="inner_dep" @@ -9,7 +9,9 @@ dflags "-fvisibility=hidden" platform="gdc" # not supported by DMD # With LDC on Windows, code ending up in a DLL is compiled with -# `-fvisibility=public -dllimport=all` by default. So we additionally need -# to override `-dllimport` to use the shared druntime/Phobos DLLs (used by -# default for dynamic libraries), which is also passed down to all deps: +# `-fvisibility=public -dllimport=all` by default. So we additionally need to +# override `-dllimport` to dllimport from the shared druntime/Phobos DLLs only +# (used by default for dynamic libraries; with static druntime/Phobos via +# `-link-defaultlib-shared=false`, use `-dllimport=none`). +# This flag is also passed down to all deps. dflags "-dllimport=defaultLibsOnly" platform="windows-ldc" diff --git a/test/exe-shared-defaultlib/dub.sdl b/test/exe-shared-defaultlib/dub.sdl index 734c20ec2..5068e0996 100644 --- a/test/exe-shared-defaultlib/dub.sdl +++ b/test/exe-shared-defaultlib/dub.sdl @@ -4,7 +4,7 @@ targetType "executable" dependency "staticlib-simple" path="../1-staticLib-simple/" # With LDC on Windows, `-link-defaultlib-shared` is passed down for compiling -# staticlib-simple (for an implicit `-dllimport=defaultLibsOnly`). +# all dependencies (for an implicit `-dllimport=defaultLibsOnly`). dflags "-link-defaultlib-shared" platform="ldc" # no libphobos2.dll/dylib with DMD on Windows/Mac From 5974adb2427a68975bea60a3e945fefa320361cb Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Thu, 11 Jan 2024 14:09:59 +0100 Subject: [PATCH 5/6] [add changelog entry] --- changelog/ldc_visibility.dd | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 changelog/ldc_visibility.dd diff --git a/changelog/ldc_visibility.dd b/changelog/ldc_visibility.dd new file mode 100644 index 000000000..f172d70c2 --- /dev/null +++ b/changelog/ldc_visibility.dd @@ -0,0 +1,20 @@ +LDC: Apply dflags affecting symbol visibility to all deps + +The `-fvisibility` (and, on Windows, `-dllimport` and `-link-defaultlib-shared`) +flags in effect for some root project are now passed down to all (direct and indirect) +dependencies of that root project - when using LDC. This is mainly important on +Windows, for executables linked against shared druntime/Phobos, as well as DLLs. + +For Windows executables to be linked against the druntime/Phobos DLLs via +`-link-defaultlib-shared`, this means that all its static-library dub dependencies +are now automatically compiled with `-link-defaultlib-shared` too, thus defaulting to +`-dllimport=defaultLibsOnly`, which is a (Windows-specific) requirement for this +scenario. + +With LDC, code ending up in a Windows DLL is compiled with `-fvisibility=public +-dllimport=all` by default (implicit flags added by dub). This can now be overridden +via e.g. `-fvisibility=hidden -dllimport=defaultLibsOnly` to generate a DLL exporting +only select symbols with explicit `export` visibility and linking all dub deps +statically (but druntime/Phobos dynamically). If you want to additionally link druntime +and Phobos statically into the DLL, use `-link-defaultlib-shared=false +-fvisibility=hidden -dllimport=none` instead. From 547b120cbf5127b5430e4bfd967a727283650c18 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Thu, 11 Jan 2024 14:41:12 +0100 Subject: [PATCH 6/6] [don't pass flags down to shared-library deps] --- source/dub/generators/generator.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/dub/generators/generator.d b/source/dub/generators/generator.d index 5a04accfe..b6d1b05da 100644 --- a/source/dub/generators/generator.d +++ b/source/dub/generators/generator.d @@ -479,7 +479,7 @@ class ProjectGenerator { foreach (name, ref ti; targets) { - if (&ti != roottarget) + if (&ti != roottarget && ti.buildSettings.targetType != TargetType.dynamicLibrary) { import std.range : chain; ti.buildSettings.dflags = ti.buildSettings.dflags