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

run-make: audit the ignore-{windows,msvc,windows-msvc} tests #128602

Open
7 of 28 tasks
jieyouxu opened this issue Aug 3, 2024 · 10 comments
Open
7 of 28 tasks

run-make: audit the ignore-{windows,msvc,windows-msvc} tests #128602

jieyouxu opened this issue Aug 3, 2024 · 10 comments
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Aug 3, 2024

There are several ignore-msvc/ignore-windows/ignore-windows-msvc Makefile and rmake.rs (tests that are ported to pure Rust files) run-make tests that have those ignore-*s because of several common challenges:

  1. Can't figure out how to build the proper e.g. import lib for dynamic lib to satisfy link.exe.
  2. Can't figure out the msvc build invocations to generate the desired output artifacts.
  3. Debug info or object symbol differences versus mingw or linux or apple.
  4. Weird appearance of unexpected symbols or lack of expected symbols.

It would be great and super helpful if Windows experts could take a look at them, and see if some of the ignore-*s can be resolved or otherwise how we can expand the tests to cover msvc as well, or have proper reasons why they must be ignore-*'d. The test suite is quite tricky to run on Windows, see https://rustc-dev-guide.rust-lang.org/tests/running.html?highlight=run-make#windows, seems to require msys2 + make, binutils, diffutils for the remaining Makefiles.

ignore-msvc:

ignore-windows-msvc:

ignore-windows:

  • tests/run-make/naked-symbol-visibility/rmake.rs
  • tests/run-make/dep-info/Makefile
  • tests/run-make/short-ice/rmake.rs
  • tests/run-make/incr-add-rust-src-component/Makefile (symlink)
  • tests/run-make/extern-fn-reachable/Makefile
  • tests/run-make/dep-info/Makefile
  • tests/run-make/dep-info-spaces/Makefile
  • tests/run-make/lto-avoid-object-duplication/rmake.rs
  • tests/run-make/translation/Makefile (symlink)
  • tests/run-make/native-link-modifier-bundle/Makefile
  • tests/run-make/textrel-on-minimal-lib/rmake.rs
  • tests/run-make/remap-path-prefix-dwarf/Makefile
  • tests/run-make/libs-through-symlinks/Makefile (symlink)
  • tests/run-make/redundant-libs/Makefile
  • tests/run-make/reproducible-build-2/rmake.rs (the symlink part might be possible with needs-symlink, unsure about reproducible paths)
  • tests/run-make/fmt-write-bloat/rmake.rs (no_std test) (run-make: explaing why fmt-write-bloat is ignore-windows #128807)
@jieyouxu jieyouxu added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-run-make Area: port run-make Makefiles to rmake.rs labels Aug 3, 2024
@rustbot

This comment was marked as outdated.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 3, 2024
@jieyouxu
Copy link
Member Author

jieyouxu commented Aug 3, 2024

If the Windows experts could take a look when they have time, it would be very helpful :3
@rustbot ping windows

@rustbot
Copy link
Collaborator

rustbot commented Aug 3, 2024

Hey Windows Group! This bug has been identified as a good "Windows candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @albertlarsan68 @arlosi @ChrisDenton @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @nico-abram @retep998 @sivadeilra @wesleywiser

@rustbot rustbot added the O-windows Operating system: Windows label Aug 3, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 3, 2024
@jieyouxu
Copy link
Member Author

jieyouxu commented Aug 3, 2024

You can find the current run-make tests that have ignore-msvc with https://github.com/search?q=repo%3Arust-lang%2Frust%20ignore-msvc%20path%3Atests%2Frun-make&type=code as well as ignore-windows-msvc with https://github.com/search?q=repo%3Arust-lang%2Frust+ignore-windows-msvc+path%3Atests%2Frun-make&type=code.

I'm happy to help answer questions about the rmake.rs setup :3

@ChrisDenton
Copy link
Member

I very quickly did one in #128603. It uses a library function rather than nm (using llvm-nm would also have been an option).

@ChrisDenton
Copy link
Member

ChrisDenton commented Aug 3, 2024

I don't have time to fix it atm but zero-extend-abi-param-passing looks like another easy one. It just needs two things.

A version of build_native_static_lib that optimizes the output. Maybe called build_native_static_lib_optimized or something. This would be exactly the same as build_native_static_lib but would also set -O3 for non-msvc and /O2 for msvc.

param_passing.rs should also be changed to use #[link(name = "bad", kind = "static")]:

Or the #[link] line could be removed entirely as it's set in the call to rustc.

@ChrisDenton
Copy link
Member

Ok I've done the above now as well as the other non-Makefile tests. I've not looked at the Makefile tests yet since I'd prefer to wait until they've been ported to pure Rust as I find it much easier to work on Rust tests without the added layer of emulation.

@jieyouxu jieyouxu changed the title run-make: audit the ignore-msvc tests run-make: audit the ignore-{windows,msvc,windows-msvc} tests Aug 5, 2024
@mati865
Copy link
Contributor

mati865 commented Oct 27, 2024

Somewhat related, do you have an idea how to make test run on windows-gnullvm and msvc only?
If I write:

//@ only-windows
//@ ignore-windows-gnu // or just ignore-gnu

Then gnullvm will be also excluded. There are few things (like CFGuad) that will probsbly never work with mingw-w64+GNU targets but are supported on mingw-w64+LLVM (not yet within Rust).

@jieyouxu
Copy link
Member Author

@mati865 I think this is a problem with how the windows-gnu matcher in compiletest works, do you mind filing a new issue (i.e. windows-gnu also means ignore gnullvm and there's no way to specifiy running windows-gnu but skipping gnu-llvm)?

@mati865
Copy link
Contributor

mati865 commented Oct 28, 2024

Sure, I'll do it later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants