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

Allow rust staticlib to work with MSVC's /WHOLEARCHIVE #129257

Merged
merged 2 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ dependencies = [

[[package]]
name = "ar_archive_writer"
version = "0.4.0"
version = "0.4.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "de11a9d32db3327f981143bdf699ade4d637c6887b13b97e6e91a9154666963c"
checksum = "01667f6f40216b9a0b2945e05fed5f1ad0ab6470e69cb9378001e37b1c0668e4"
dependencies = [
"object 0.36.2",
]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2021"

[dependencies]
# tidy-alphabetical-start
ar_archive_writer = "0.4.0"
ar_archive_writer = "0.4.2"
arrayvec = { version = "0.7", default-features = false }
bitflags = "2.4.1"
cc = "1.0.90"
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_codegen_ssa/src/back/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,11 @@ pub trait ArchiveBuilderBuilder {
&exports,
machine,
!sess.target.is_like_msvc,
/*comdat=*/ false,
// Enable compatibility with MSVC's `/WHOLEARCHIVE` flag.
// Without this flag a duplicate symbol error would be emitted
// when linking a rust staticlib using `/WHOLEARCHIVE`.
// See #129020
true,
ChrisDenton marked this conversation as resolved.
Show resolved Hide resolved
) {
sess.dcx()
.emit_fatal(ErrorCreatingImportLibrary { lib_name, error: error.to_string() });
Expand Down
1 change: 1 addition & 0 deletions tests/run-make/msvc-wholearchive/c.c
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// This page is intentionally left blank
4 changes: 4 additions & 0 deletions tests/run-make/msvc-wholearchive/dll.def
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
LIBRARY dll
EXPORTS
hello
number
52 changes: 52 additions & 0 deletions tests/run-make/msvc-wholearchive/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//! This is a regression test for #129020
//! It ensures we can use `/WHOLEARCHIVE` to link a rust staticlib into DLL
//! using the MSVC linker

//@ only-msvc
Copy link
Contributor

@mati865 mati865 Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice (but not required) to also cover windows-gnullvm that has the same issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not tried gnullvm specifically but the llvm linker has another issue which I think needs solving on their end first (which is why even the msvc test skips lld-link).

.idata$4 should not refer to special section 0

LLVM seems to not properly recognise .idata$4 and .idata$5 as special unless they're specifically in short import libraries. incidentally this also makes it break on MSVC long import libraries. Though since they haven't been in use since the 90's I guess there's not been much pressure to support it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should open a new issue to track this case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? How do we usually track upstream issues?

Copy link
Member

@jieyouxu jieyouxu Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a special label for waiting on LLVM: S-waiting-for-llvm for describing the dragon is eepy (and better if it links to a llvm upstream issue)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though since they haven't been in use since the 90's I guess there's not been much pressure to support it.

ld.bfd used by windows-gnu still uses them as it still cannot output short import libraries 😉

// Reason: this is testing the MSVC linker

use std::path::PathBuf;

use run_make_support::{cc, cmd, env_var, extra_c_flags, rustc};

fn main() {
// Build the staticlib
rustc().crate_type("staticlib").input("static.rs").output("static.lib").run();
// Build an empty object to pass to the linker.
cc().input("c.c").output("c.obj").args(["-c"]).run();

// Find the C toolchain's linker.
let mut linker = PathBuf::from(env_var("CC"));
let linker_flavour = if linker.file_stem().is_some_and(|s| s == "cl") {
linker.set_file_name("link.exe");
"msvc"
} else if linker.file_stem().is_some_and(|s| s == "clang-cl") {
linker.set_file_name("lld-link.exe");
"llvm"
} else {
panic!("unknown C toolchain");
};

// As a sanity check, make sure this works without /WHOLEARCHIVE.
// Otherwise the actual test failure may be caused by something else.
cmd(&linker)
.args(["c.obj", "./static.lib", "-dll", "-def:dll.def", "-out:dll.dll"])
.args(extra_c_flags())
.run();

// FIXME(@ChrisDenton): this doesn't currently work with llvm's lld-link for other reasons.
// May need LLVM patches.
if linker_flavour == "msvc" {
// Link in the staticlib using `/WHOLEARCHIVE` and produce a DLL.
cmd(&linker)
.args([
"c.obj",
"-WHOLEARCHIVE:./static.lib",
"-dll",
"-def:dll.def",
"-out:dll_whole_archive.dll",
])
.args(extra_c_flags())
.run();
}
}
9 changes: 9 additions & 0 deletions tests/run-make/msvc-wholearchive/static.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#[no_mangle]
pub extern "C" fn hello() {
println!("Hello world!");
}

#[no_mangle]
pub extern "C" fn number() -> u32 {
42
}
Loading