Fix cross-compiling with dlltool for raw-dylib

[dpaoliello]

Fix for #103939

Issue Details:
When attempting to cross-compile using the raw-dylib feature and the GNU toolchain, rustc would attempt to find a cross-compiling version of dlltool (e.g., i686-w64-mingw32-dlltool). The has two issues 1) on Windows dlltool is always dlltool (no cross-compiling named versions exist) and 2) it only supported compiling to i686 and x86_64 resulting in ARM 32 and 64 compiling as x86_64.

Fix Details:

• On Windows always use the normal dlltool binary.
• Add the ARM64 cross-compiling dlltool name (support for this is coming: https://sourceware.org/bugzilla/show_bug.cgi?id=29964)
• Provide the -m argument to dlltool to indicate the target machine type.

(This is the first of two PRs to fix the remaining issues for the raw-dylib feature (#58713) that is blocking stabilization (#104218))

[rustbot]

r? @nagisa

(rustbot has picked a reviewer for you, use r? to override)

[dpaoliello]

r? @michaelwoerister

[mati865]

This will not work when cross-compiling because on Linux there is no dlltool (since this tool is meant only for Windows targets), only {triple}-dlltool are available:
https://archlinux.org/packages/community/x86_64/mingw-w64-binutils/files/
https://packages.ubuntu.com/kinetic/amd64/binutils-mingw-w64-x86-64/filelist

[dpaoliello]

This will not work when cross-compiling because on Linux there is no dlltool (since this tool is meant only for Windows targets), only {triple}-dlltool are available: https://archlinux.org/packages/community/x86_64/mingw-w64-binutils/files/ https://packages.ubuntu.com/kinetic/amd64/binutils-mingw-w64-x86-64/filelist

Well, that's fun. Also means that the original code was broken if cross-compiling from non-Windows for Windows with the same architecture:

> rustc +nightly test.rs --target x86_64-pc-windows-gnu
error: Error calling dlltool: No such file or directory (os error 2)

error: aborting due to previous error

[michaelwoerister]

Thanks a lot for taking this on, @dpaoliello!

[michaelwoerister]

I'd really like us to have proper tests for this before we merge the PR, at least for the cases:

• x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu
• x86_64-pc-windows-gnu -> i686-pc-windows-gnu
• x86_64-unknown-linux-gnu -> x86_64-pc-windows-gnu
• x86_64-unknown-linux-gnu -> i686-pc-windows-gnu (?)

It seems to be straightforward to cross compiling simple libraries as part of a test, as long as the library does not depend on libcore or libstd (which we don't need here). Here's an example for a UI test:

// build-pass
// only-linux

// Run the test for both i686 and x86_64
// revisions: i686 x86_64
// [i686]compile-flags:--target=i686-pc-windows-gnu
// [x86_64]compile-flags:--target=x86_64-pc-windows-gnu

#![feature(raw_dylib)]
#![feature(no_core, lang_items)]
#![no_std]
#![no_core]
#![crate_type = "lib"]

// This is needed because of #![no_core]:
#[lang = "sized"]
trait Sized {}

// The DLL does not actually exist anywhere, but we don't care.
// We just want rustc to successfully build an import library
// from the extern block.
#[link(name = "extern_1", kind = "raw-dylib")]
extern "C" {
    fn extern_fn_2();
    fn print_extern_variable();
    static mut extern_variable: i32;
    #[link_name = "extern_fn_4"]
    fn extern_fn_4_renamed();
}

pub unsafe fn foo() {
    extern_fn_2();
    print_extern_variable();
    extern_variable = 123;
    extern_fn_4_renamed();
}

This test will fail if it cannot find and invoke the expected dlltool. However, the downside is that we then would require any system running tests to have binutils-mingw-w64-x86-64 installed.

The x86_64-pc-windows-gnu -> *-pc-windows-gnu cases should be easier, since we can make such tests only-x86_64-pc-windows-gnu.

Another thing I noticed is that there might be an additional set of flags needed, as described here:
microsoft/windows-rs@cb8d6a9#diff-82947385e42042ffba4e2ba53c683427146ce3bf9e7c15ef45ad38e624a66444R100-R106

That PR seems to apply them unconditionally (at least on Windows?). Maybe we should add a (run-make) test that does a check as described in the corresponding bug report.

[mati865]

The x86_64-pc-windows-gnu -> *-pc-windows-gnu cases should be easier, since we can make such tests only-x86_64-pc-windows-gnu.

This test would fail on both CI and on user systems because typically only one toolchain is available in PATH (either i686 or x86_64 or AArch64).

[michaelwoerister]

Even if the host only ever is x86_64-pc-windows-gnu?

[mati865]

I mean you could only test x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu and not x86_64-pc-windows-gnu -> i686-pc-windows-gnu.

[dpaoliello]

I'd really like us to have proper tests for this before we merge the PR, at least for the cases:

• x86_64-pc-windows-gnu -> x86_64-pc-windows-gnu
• x86_64-pc-windows-gnu -> i686-pc-windows-gnu
• x86_64-unknown-linux-gnu -> x86_64-pc-windows-gnu
• x86_64-unknown-linux-gnu -> i686-pc-windows-gnu (?)

Done.

This test would fail on both CI and on user systems because typically only one toolchain is available in PATH (either i686 or x86_64 or AArch64).

Ony my local testing it seemed to work - let's see what the CI does...

Another thing I noticed is that there might be an additional set of flags needed, as described here: microsoft/windows-rs@cb8d6a9#diff-82947385e42042ffba4e2ba53c683427146ce3bf9e7c15ef45ad38e624a66444R100-R106

That PR seems to apply them unconditionally (at least on Windows?). Maybe we should add a (run-make) test that does a check as described in the corresponding bug report.

Good catch - added.

[michaelwoerister]

I'm not sure what's going on here. rustc tries to call dlltool (without .exe) which seems to indicate that this branch is hit: https://github.com/rust-lang/rust/pull/108355/files#diff-0629e2ee816d38d7591985254f2e34523289cf29188ff24c5d4fba05ae839de5R449 -- but I don't understand why, since rustc is being called with --target i686-pc-windows-gnu as expected. The test failure doesn't reproduce for me locally either.

[michaelwoerister]

Unrelated to the CI failure, but there seem to be a few small errors in the Makefile that lead to the test passing without actually testing anything.

[mati865]

I'm not sure what's going on here. rustc tries to call dlltool (without .exe) which seems to indicate that this branch is hit: #108355 (files) -- but I don't understand why, since rustc is being called with --target i686-pc-windows-gnu as expected. The test failure doesn't reproduce for me locally either.

Pretty sure the error message doesn't tell the actual executable and just uses hardcoded dlltool but haven't bothered to check.
If I'm right this error would be expected because there is no i686-w64-mingw32-dlltool installed on that runner.

[michaelwoerister]

Yes, indeed -- thanks, @mati865!

That should be fixable by installing the binutils-mingw-w64 package similar to the mingw-check Dockerfile.

But this highlights the problem already mentioned before: If we add the test in its current form, everyone who wants to run tests locally will have to install binutils-mingw-w64. I'm not sure that's acceptable.

There's precedent for ignoring tests when the environment doesn't fulfill certain requirements (e.g. xLTO tests need Clang). We may have to go down that route if we can't find a more elegant way.

[dpaoliello]

Ok, here's my workaround: we try to detect if i686-w64-mingw32-dlltool is present (and use it if it is), otherwise we use the -Zdlltool arg to redirect to x86_64-w64-mingw32-dlltool instead.

[michaelwoerister]

#109073 has been merged, maybe that helps.
@bors retry

[bors]

⌛ Testing commit 94aaac6c6715291c994643dc59f616ca77a913b6 with merge 2eb6699bd74c7f5b3d1b98907bf4b8dada17997d...

[bors]

💔 Test failed - checks-actions

[dpaoliello]

Looks like i686 dlltool.exe can't produce x64 binaries, so I'm ignoring that target triple in the test for now.

[michaelwoerister]

Thanks, @dpaoliello!
@bors r+

[bors]

📌 Commit a90f342 has been approved by michaelwoerister

It is now in the queue for this repository.

[bors]

⌛ Testing commit a90f342 with merge 9a6b0c3...

[bors]

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing 9a6b0c3 to master...

[rust-timer]

Finished benchmarking commit (9a6b0c3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.4%, 3.4%]	1
Regressions ❌ (secondary)	2.0%	[0.9%, 3.4%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	5
All ❌✅ (primary)	3.4%	[3.4%, 3.4%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.6%	[-0.7%, -0.5%]	4
All ❌✅ (primary)	-	-	0