Skip to content

Commit 9e0ddf7

Browse files
committed
fix(build): the import-library and .def spellings follow the target ABI
Windows CI again, one layer further in: lld-link: warning: ignoring unknown argument '--out-implib' lld-link: error: could not open 'bin/mathkit-shared.lib': no such file Clang targeting the MSVC ABI speaks the GNU DIALECT while driving lld-link, so a dialect-keyed spelling handed the MSVC linker a MinGW flag. The `.def` had the same problem from the other side: `/DEF:` was added only to the MSVC-dialect rule, so a clang-driven MSVC-ABI link generated the file and never passed it. Three flags in this PR made the identical mistake — `-fPIC`, the import library, and `/DEF:` — and it is always the same one: asking which COMPILER when the question is which TARGET. So the spelling leaves the dialect table entirely and becomes `pe_link_flag`, which reads the target triple and wraps in `-Wl,` unless the linker is invoked directly. The dialect table now says, where the entry used to be, why it cannot answer this. Verified in both directions locally: a MinGW target still gets `-Wl,--out-implib,` and no def edge; ELF and Mach-O are untouched.
1 parent 9bdb284 commit 9e0ddf7

2 files changed

Lines changed: 47 additions & 30 deletions

File tree

src/build/ninja_backend.cppm

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,31 @@ std::string join_flags(const std::vector<std::string>& flags) {
207207
// default install name is the path it was LINKED at, so a package built in
208208
// /tmp/build-xyz records /tmp/build-xyz and cannot be relocated — which is
209209
// every distributed dylib. `@rpath/<file>` is the only default that travels.
210+
// A PE link flag, spelled for the TARGET ABI and wrapped for the driver.
211+
//
212+
// ⚠️ NOT a dialect-table entry, and Windows CI is why. Clang targeting the MSVC
213+
// ABI speaks the GNU DIALECT while driving lld-link, so a dialect-keyed spelling
214+
// handed it `-Wl,--out-implib,` and lld-link answered `warning: ignoring unknown
215+
// argument '--out-implib'` followed by `could not open '…lib': no such file`.
216+
// Three flags in this PR made the same mistake — `-fPIC`, the import library,
217+
// and `/DEF:` — and the mistake is always the same one: asking which COMPILER
218+
// when the question is which TARGET.
219+
//
220+
// `sep` is `LinkStyle::SeparateLinker`, i.e. link.exe invoked directly, where
221+
// the flag needs no `-Wl,` wrapper. Everything else goes through a compiler
222+
// driver.
223+
std::string pe_link_flag(const BuildPlan& plan, bool sep,
224+
std::string_view msvcForm, std::string_view gnuForm,
225+
std::string_view path)
226+
{
227+
const auto t = mcpp::toolchain::triple::parse(plan.toolchain.targetTriple);
228+
const bool msvcAbi = t ? t->is_msvc_env()
229+
: mcpp::toolchain::is_msvc_target(plan.toolchain);
230+
if (!msvcAbi) return std::string(gnuForm) + std::string(path);
231+
auto flag = std::string(msvcForm) + std::string(path);
232+
return sep ? flag : "-Wl," + flag;
233+
}
234+
210235
std::string shared_soname_flag(const LinkUnit& lu, const BuildPlan& plan) {
211236
if (lu.kind != LinkUnit::SharedLibrary) return "";
212237
const auto t = mcpp::toolchain::triple::parse(plan.toolchain.targetTriple);
@@ -1058,7 +1083,7 @@ std::string emit_ninja_string(const BuildPlan& plan) {
10581083
link_rule("cxx_archive", std::string(dial.archiveCmd), "AR");
10591084
link_rule("cxx_shared",
10601085
"$cxx -shared $in -o $out $ldflags $soname_flag "
1061-
"$implib_flag $unit_ldflags",
1086+
"$implib_flag $def_flag $unit_ldflags",
10621087
"SHARED");
10631088
// mcpp#426: a link unit with no C++ translation unit in it is
10641089
// linked by the C driver. `g++` appends `-lstdc++` unconditionally,
@@ -1071,7 +1096,7 @@ std::string emit_ninja_string(const BuildPlan& plan) {
10711096
"$cc $in -o $out $c_ldflags $unit_ldflags", "LINK");
10721097
link_rule("c_shared",
10731098
"$cc -shared $in -o $out $c_ldflags $soname_flag "
1074-
"$implib_flag $unit_ldflags",
1099+
"$implib_flag $def_flag $unit_ldflags",
10751100
"SHARED");
10761101
}
10771102
}
@@ -1853,20 +1878,21 @@ std::string emit_ninja_string(const BuildPlan& plan) {
18531878
// agrees with — the name belongs to plan.cppm's import_library_for, and
18541879
// this is how it gets to the command. The SPELLING belongs to the
18551880
// dialect table, same as `archiveRemoveArg`.
1881+
const bool sepLinker =
1882+
dial.linkStyle == mcpp::toolchain::CommandDialect::LinkStyle::SeparateLinker;
18561883
if (!lu.importLibrary.empty()) {
1857-
std::string arg{ dial.sharedImportLibArg };
1858-
// `{}` or nothing: a row without the placeholder cannot say WHERE to
1859-
// write, so emitting its bare text would hand the linker a flag with
1860-
// no argument. Skipping is the honest reading of an empty row, and
1861-
// the implicit output above then fails loudly as a missing file
1862-
// rather than quietly linking against a stale one.
1863-
if (auto at = arg.find("{}"); at != std::string::npos) {
1864-
arg.replace(at, 2, escape_ninja_path(lu.importLibrary));
1865-
out_line += " implib_flag = " + arg + "\n";
1866-
}
1884+
out_line += " implib_flag = " + pe_link_flag(
1885+
plan, sepLinker, "/IMPLIB:", "-Wl,--out-implib,",
1886+
escape_ninja_path(lu.importLibrary)) + "\n";
1887+
}
1888+
if (!lu.defFile.empty()) {
1889+
// `/DEF:` on both sides: it is a link.exe/lld-link flag, and the
1890+
// only targets that reach here are MSVC-ABI ones (MinGW auto-exports
1891+
// and gets no def edge at all).
1892+
out_line += " def_flag = " + pe_link_flag(
1893+
plan, sepLinker, "/DEF:", "/DEF:",
1894+
escape_ninja_path(lu.defFile)) + "\n";
18671895
}
1868-
if (!lu.defFile.empty())
1869-
out_line += " def_flag = /DEF:" + escape_ninja_path(lu.defFile) + "\n";
18701896
{
18711897
// Per-unit C++ runtime link, by ROLE. The kind→role map is the
18721898
// only place that knows a TestBinary runs on the build machine

src/toolchain/dialect.cppm

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,14 @@ struct CommandDialect {
107107
std::string_view archiveRemoveArg; // "d" needs no {} | "/REMOVE:{}"
108108
bool archiveRemoveTakesArchiveFirst = true;
109109

110-
// How the linker is told where to write a shared library's IMPORT LIBRARY —
111-
// the archive of stubs a PE consumer links against, as opposed to the `.dll`
112-
// the loader opens. `{}` is the path.
110+
// ⚠️ THE IMPORT-LIBRARY AND `.def` SPELLINGS ARE NOT HERE, on purpose.
113111
//
114-
// Emitted only when the TARGET has import libraries at all (PE); on ELF and
115-
// Mach-O the shared library is its own link input and there is nothing to
116-
// write. So this being non-empty in both rows is not a contradiction: the
117-
// rows describe how to SAY it, and the target decides whether to.
118-
std::string_view sharedImportLibArg; // "-Wl,--out-implib,{}" | "/IMPLIB:{}"
112+
// They were, keyed on the dialect, and that is wrong in a way Windows CI
113+
// demonstrated: clang targeting the MSVC ABI speaks the GNU DIALECT while
114+
// driving LLD-LINK, so it was handed `-Wl,--out-implib,` and answered
115+
// `lld-link: warning: ignoring unknown argument '--out-implib'`. The
116+
// spelling follows the target ABI, which this table does not know — see
117+
// ninja_backend's pe_link_flag.
119118
};
120119

121120
// Dialect lookup. GCC / Clang / MinGW → gnu; MSVC → msvc.
@@ -219,10 +218,6 @@ constexpr CommandDialect kGnuDialect{
219218
// `ar d <archive> <member>...` — one verb, then every member.
220219
.archiveRemoveArg = "d",
221220
.archiveRemoveTakesArchiveFirst = true,
222-
// ld/lld: `--out-implib` is what makes a PE shared library linkable at all.
223-
// Without it mingw writes only the .dll, consumers link the .dll directly,
224-
// and that works — until the same package is consumed by any other linker.
225-
.sharedImportLibArg = "-Wl,--out-implib,{}",
226221
};
227222

228223
// Native cl.exe. Unreachable in builds until the MSVC backend lands
@@ -258,10 +253,6 @@ constexpr CommandDialect kMsvcDialect{
258253
// reported with the command that produced it rather than swallowed.
259254
.archiveRemoveArg = "/REMOVE:{}",
260255
.archiveRemoveTakesArchiveFirst = false,
261-
// link.exe writes one whether asked or not; naming it explicitly is how the
262-
// path stays the one plan.cppm chose, instead of the linker's `$out`-derived
263-
// guess (`foo.dll.lib`) that nothing else in mcpp agrees with.
264-
.sharedImportLibArg = "/IMPLIB:{}",
265256
};
266257

267258
} // namespace

0 commit comments

Comments
 (0)