Skip to content
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

Fix import lib generation under MinGW #438

Merged
merged 2 commits into from
Jan 12, 2025

Conversation

kleisauke
Copy link
Contributor

Override import_name after parsing the .def file, as -Wl,--output-def, in LLVM and Binutils does not include a library name in the generated .def file.

cargo-c/src/target.rs

Lines 112 to 118 in c37aec4

} else if os == "windows" && env == "gnu" {
// This is only set up to work on GNU toolchain versions of Rust
lines.push(format!(
"-Wl,--output-def,{}",
target_dir.join(format!("{lib_name}.def")).display()
));
}

https://github.com/llvm/llvm-project/blob/llvmorg-19.1.6/lld/COFF/MinGW.cpp#L173-L192
https://github.com/bminor/binutils-gdb/blob/binutils-2_43_1/binutils/dlltool.c#L1813-L1872

Also bump implib while we are here, to ensure the import library works when targeting i686-pc-windows-gnu{,llvm}.

@sdroege
Copy link
Contributor

sdroege commented Jan 12, 2025

CC @amyspark

@lu-zero lu-zero merged commit 851aab5 into lu-zero:master Jan 12, 2025
17 checks passed
@kleisauke kleisauke deleted the fix-import-lib-mingw branch January 13, 2025 09:13
@lazka
Copy link
Contributor

lazka commented Feb 11, 2025

A bit of a long shot, but we are getting crashes with the new import libs since updating to 0.10.9, see msys2/MINGW-packages#23358. I tried reverting this PR, but then it fails at dynamic link time (do I need to downgrade rust too? edit: no, doesn't help)

@lu-zero
Copy link
Owner

lu-zero commented Feb 11, 2025

If you could bisect the releases to find the last one working would be great, even better if you can can make a smaller reproducer.

@lazka
Copy link
Contributor

lazka commented Feb 12, 2025

I just went back all versions, the last working version was 0.10.4, then linking is broken in general until 0.10.9, but then things crash at runtime.

@lazka
Copy link
Contributor

lazka commented Feb 12, 2025

Since the last working importlib is much simpler than the broken one, my guess is that #404 could be related

@lu-zero
Copy link
Owner

lu-zero commented Feb 12, 2025

Can you please compare the two outputs and tell me what changed?

@lazka
Copy link
Contributor

lazka commented Feb 12, 2025

I'm afraid I don't know enough about this to help much (I haven't figured out why calling liq_version() crashes for starters):

Both llvm and gnu linker result in the same crash, so I assume the linker isn't at fault.

@lazka
Copy link
Contributor

lazka commented Feb 12, 2025

I've created a (smaller) reproducer: https://github.com/lazka/cargo-c-crash

Let me know if I should create a separate issue for this.

edit: I've worked around this issue my using the toolchain's dlltool to create an importlib instead of using cargo-c

edit2: I've created a separate: #443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants