Skip to content

Commit

Permalink
Auto merge of rust-lang#122036 - alexcrichton:test-wasm-with-wasi, r=…
Browse files Browse the repository at this point in the history
…oli-obk

Test wasm32-wasip1 in CI, not wasm32-unknown-unknown

This commit changes CI to no longer test the `wasm32-unknown-unknown` target and instead test the `wasm32-wasip1` target. There was some discussion of this in a [Zulip thread], and the motivations for this PR are:

* Runtime failures on `wasm32-unknown-unknown` print nothing, meaning all you get is "something failed". In contrast `wasm32-wasip1` can print to stdout/stderr.

* The unknown-unknown target is missing lots of pieces of libstd, and while `wasm32-wasip1` is also missing some pieces (e.g. threads) it's missing fewer pieces. This means that many more tests can be run.

Overall my hope is to improve the debuggability of wasm failures on CI and ideally be a bit less of a maintenance burden.

This commit specifically removes the testing of `wasm32-unknown-unknown` and replaces it with testing of `wasm32-wasip1`. Along the way there were a number of other archiectural changes made as well, including:

* A new `target.*.runtool` option can now be specified in `config.toml` which is passed as `--runtool` to `compiletest`. This is used to reimplement execution of WebAssembly in a less-wasm-specific fashion.

* The default value for `runtool` is an ambiently located WebAssembly runtime found on the system, if any. I've implemented logic for Wasmtime.

* Existing testing support for `wasm32-unknown-unknown` and Emscripten has been removed. I'm not aware of Emscripten testing being run any time recently and otherwise `wasm32-wasip1` is in theory the focus now.

* I've added a new `//@ needs-threads` directive for `compiletest` and classified a bunch of wasm-ignored tests as needing threads. In theory these tests can run on `wasm32-wasi-preview1-threads`, for example.

* I've tried to audit all existing tests that are either `ignore-emscripten` or `ignore-wasm*`. Many now run on `wasm32-wasip1` due to being able to emit error messages, for example. Many are updated with comments as to why they can't run as well.

* The `compiletest` output matching for `wasm32-wasip1` automatically uses "match a subset" mode implemented in `compiletest`. This is because WebAssembly runtimes often add extra information on failure, such as the `unreachable` instruction in `panic!`, which isn't able to be matched against the golden output from native platforms.

* I've ported most existing `run-make` tests that use custom Node.js wrapper scripts to the new run-make-based-in-Rust infrastructure. To do this I added `wasmparser` as a dependency of `run-make-support` for the various wasm tests to use that parse wasm files. The one test that executed WebAssembly now uses `wasmtime`-the-CLI to execute the test instead. I have not ported over an exception-handling test as Wasmtime doesn't implement this yet.

* I've updated the `test` crate to print out timing information for WASI targets as it can do that (gets a previously ignored test now passing).

* The `test-various` image now builds a WASI sysroot for the WASI target and additionally downloads a fixed release of Wasmtime, currently the latest one at 18.0.2, and uses that for testing.

[Zulip thread]: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Have.20wasm.20tests.20ever.20caused.20problems.20on.20CI.3F/near/424317944
  • Loading branch information
bors committed Mar 12, 2024
2 parents 4a0cc88 + cf6d605 commit dc2ffa4
Show file tree
Hide file tree
Showing 236 changed files with 721 additions and 791 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3309,6 +3309,9 @@ dependencies = [
[[package]]
name = "run_make_support"
version = "0.0.0"
dependencies = [
"wasmparser",
]

[[package]]
name = "rust-demangler"
Expand Down
7 changes: 4 additions & 3 deletions library/test/src/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,11 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Resu
// Prevent the usage of `Instant` in some cases:
// - It's currently not supported for wasm targets.
// - We disable it for miri because it's not available when isolation is enabled.
let is_instant_supported =
!cfg!(target_family = "wasm") && !cfg!(target_os = "zkvm") && !cfg!(miri);
let is_instant_unsupported = (cfg!(target_family = "wasm") && !cfg!(target_os = "wasi"))
|| cfg!(target_os = "zkvm")
|| cfg!(miri);

let start_time = is_instant_supported.then(Instant::now);
let start_time = (!is_instant_unsupported).then(Instant::now);
run_tests(opts, tests, |x| on_test_event(&x, &mut st, &mut *out))?;
st.exec_time = start_time.map(|t| TestSuiteExecTime(t.elapsed()));

Expand Down
15 changes: 3 additions & 12 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1657,8 +1657,8 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// ensure that `libproc_macro` is available on the host.
builder.ensure(compile::Std::new(compiler, compiler.host));

// As well as the target, except for plain wasm32, which can't build it
if suite != "mir-opt" && !target.contains("wasm") && !target.contains("emscripten") {
// As well as the target
if suite != "mir-opt" {
builder.ensure(TestHelpers { target });
}

Expand Down Expand Up @@ -2511,16 +2511,7 @@ fn prepare_cargo_test(
dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());

if target.contains("emscripten") {
cargo.env(
format!("CARGO_TARGET_{}_RUNNER", envify(&target.triple)),
builder.config.nodejs.as_ref().expect("nodejs not configured"),
);
} else if target.starts_with("wasm32") {
let node = builder.config.nodejs.as_ref().expect("nodejs not configured");
let runner = format!("{} {}/src/etc/wasm32-shim.js", node.display(), builder.src.display());
cargo.env(format!("CARGO_TARGET_{}_RUNNER", envify(&target.triple)), &runner);
} else if builder.remote_tested(target) {
if builder.remote_tested(target) {
cargo.env(
format!("CARGO_TARGET_{}_RUNNER", envify(&target.triple)),
format!("{} run 0", builder.tool_exe(Tool::RemoteTestClient).display()),
Expand Down
33 changes: 30 additions & 3 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,9 +1361,36 @@ impl Build {
/// An example of this would be a WebAssembly runtime when testing the wasm
/// targets.
fn runner(&self, target: TargetSelection) -> Option<String> {
let target = self.config.target_config.get(&target)?;
let runner = target.runner.as_ref()?;
Some(runner.to_owned())
let configured_runner =
self.config.target_config.get(&target).and_then(|t| t.runner.as_ref()).map(|p| &**p);
if let Some(runner) = configured_runner {
return Some(runner.to_owned());
}

if target.starts_with("wasm") && target.contains("wasi") {
self.default_wasi_runner()
} else {
None
}
}

/// When a `runner` configuration is not provided and a WASI-looking target
/// is being tested this is consulted to prove the environment to see if
/// there's a runtime already lying around that seems reasonable to use.
fn default_wasi_runner(&self) -> Option<String> {
let mut finder = crate::core::sanity::Finder::new();

// Look for Wasmtime, and for its default options be sure to disable
// its caching system since we're executing quite a lot of tests and
// ideally shouldn't pollute the cache too much.
if let Some(path) = finder.maybe_have("wasmtime") {
if let Ok(mut path) = path.into_os_string().into_string() {
path.push_str(" run -C cache=n --dir .");
return Some(path);
}
}

None
}

/// Returns the root of the "rootfs" image that this target will be using,
Expand Down
17 changes: 14 additions & 3 deletions src/ci/docker/host-x86_64/test-various/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ FROM ubuntu:22.04
ARG DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y --no-install-recommends \
clang-11 \
llvm-11 \
g++ \
make \
ninja-build \
Expand Down Expand Up @@ -38,10 +39,14 @@ WORKDIR /
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

COPY host-x86_64/dist-various-2/build-wasi-toolchain.sh /tmp/
RUN /tmp/build-wasi-toolchain.sh

ENV RUST_CONFIGURE_ARGS \
--musl-root-x86_64=/usr/local/x86_64-linux-musl \
--set build.nodejs=/node-v18.12.0-linux-x64/bin/node \
--set rust.lld
--set rust.lld \
--set target.wasm32-wasip1.wasi-root=/wasm32-wasip1

# Some run-make tests have assertions about code size, and enabling debug
# assertions in libstd causes the binary to be much bigger than it would
Expand All @@ -50,7 +55,11 @@ ENV RUST_CONFIGURE_ARGS \
ENV NO_DEBUG_ASSERTIONS=1
ENV NO_OVERFLOW_CHECKS=1

ENV WASM_TARGETS=wasm32-unknown-unknown
RUN curl -L https://github.com/bytecodealliance/wasmtime/releases/download/v18.0.2/wasmtime-v18.0.2-x86_64-linux.tar.xz | \
tar -xJ
ENV PATH "$PATH:/wasmtime-v18.0.2-x86_64-linux"

ENV WASM_TARGETS=wasm32-wasip1
ENV WASM_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $WASM_TARGETS \
tests/run-make \
tests/ui \
Expand All @@ -59,7 +68,9 @@ ENV WASM_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $WASM_T
tests/codegen \
tests/assembly \
library/core
ENV CC_wasm32_unknown_unknown=clang-11
ENV CC_wasm32_wasip1=clang-11 \
CFLAGS_wasm32_wasip1="--sysroot /wasm32-wasip1" \
AR_wasm32_wasip1=llvm-ar-11

ENV NVPTX_TARGETS=nvptx64-nvidia-cuda
ENV NVPTX_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $NVPTX_TARGETS \
Expand Down
24 changes: 0 additions & 24 deletions src/etc/wasm32-shim.js

This file was deleted.

99 changes: 21 additions & 78 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,8 @@ impl<'test> TestCx<'test> {
let expected_coverage_dump = self.load_expected_output(kind);
let actual_coverage_dump = self.normalize_output(&proc_res.stdout, &[]);

let coverage_dump_errors = self.compare_output(
kind,
&actual_coverage_dump,
&expected_coverage_dump,
self.props.compare_output_lines_by_subset,
);
let coverage_dump_errors =
self.compare_output(kind, &actual_coverage_dump, &expected_coverage_dump);

if coverage_dump_errors > 0 {
self.fatal_proc_rec(
Expand Down Expand Up @@ -591,12 +587,8 @@ impl<'test> TestCx<'test> {
self.fatal_proc_rec(&err, &proc_res);
});

let coverage_errors = self.compare_output(
kind,
&normalized_actual_coverage,
&expected_coverage,
self.props.compare_output_lines_by_subset,
);
let coverage_errors =
self.compare_output(kind, &normalized_actual_coverage, &expected_coverage);

if coverage_errors > 0 {
self.fatal_proc_rec(
Expand Down Expand Up @@ -2632,9 +2624,7 @@ impl<'test> TestCx<'test> {
// double the length.
let mut f = self.output_base_dir().join("a");
// FIXME: This is using the host architecture exe suffix, not target!
if self.config.target.contains("emscripten") {
f = f.with_extra_extension("js");
} else if self.config.target.contains("wasm32") {
if self.config.target.starts_with("wasm") {
f = f.with_extra_extension("wasm");
} else if self.config.target.contains("spirv") {
f = f.with_extra_extension("spv");
Expand All @@ -2649,32 +2639,6 @@ impl<'test> TestCx<'test> {
// then split apart its command
let mut args = self.split_maybe_args(&self.config.runner);

// If this is emscripten, then run tests under nodejs
if self.config.target.contains("emscripten") {
if let Some(ref p) = self.config.nodejs {
args.push(p.into());
} else {
self.fatal("emscripten target requested and no NodeJS binary found (--nodejs)");
}
// If this is otherwise wasm, then run tests under nodejs with our
// shim
} else if self.config.target.contains("wasm32") {
if let Some(ref p) = self.config.nodejs {
args.push(p.into());
} else {
self.fatal("wasm32 target requested and no NodeJS binary found (--nodejs)");
}

let src = self
.config
.src_base
.parent()
.unwrap() // chop off `ui`
.parent()
.unwrap(); // chop off `tests`
args.push(src.join("src/etc/wasm32-shim.js").into_os_string());
}

let exe_file = self.make_exe_name();

args.push(exe_file.into_os_string());
Expand Down Expand Up @@ -4079,35 +4043,17 @@ impl<'test> TestCx<'test> {
match output_kind {
TestOutput::Compile => {
if !self.props.dont_check_compiler_stdout {
errors += self.compare_output(
stdout_kind,
&normalized_stdout,
&expected_stdout,
self.props.compare_output_lines_by_subset,
);
errors +=
self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout);
}
if !self.props.dont_check_compiler_stderr {
errors += self.compare_output(
stderr_kind,
&normalized_stderr,
&expected_stderr,
self.props.compare_output_lines_by_subset,
);
errors +=
self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr);
}
}
TestOutput::Run => {
errors += self.compare_output(
stdout_kind,
&normalized_stdout,
&expected_stdout,
self.props.compare_output_lines_by_subset,
);
errors += self.compare_output(
stderr_kind,
&normalized_stderr,
&expected_stderr,
self.props.compare_output_lines_by_subset,
);
errors += self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout);
errors += self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr);
}
}
errors
Expand Down Expand Up @@ -4201,12 +4147,7 @@ impl<'test> TestCx<'test> {
)
});

errors += self.compare_output(
"fixed",
&fixed_code,
&expected_fixed,
self.props.compare_output_lines_by_subset,
);
errors += self.compare_output("fixed", &fixed_code, &expected_fixed);
} else if !expected_fixed.is_empty() {
panic!(
"the `//@ run-rustfix` directive wasn't found but a `*.fixed` \
Expand Down Expand Up @@ -4701,17 +4642,19 @@ impl<'test> TestCx<'test> {
}
}

fn compare_output(
&self,
kind: &str,
actual: &str,
expected: &str,
compare_output_by_lines: bool,
) -> usize {
fn compare_output(&self, kind: &str, actual: &str, expected: &str) -> usize {
if actual == expected {
return 0;
}

// If `compare-output-lines-by-subset` is not explicitly enabled then
// auto-enable it when a `runner` is in use since wrapper tools might
// provide extra output on failure, for example a WebAssembly runtime
// might print the stack trace of an `unreachable` instruction by
// default.
let compare_output_by_lines =
self.props.compare_output_lines_by_subset || self.config.runner.is_some();

let tmp;
let (expected, actual): (&str, &str) = if compare_output_by_lines {
let actual_lines: HashSet<_> = actual.lines().collect();
Expand Down
1 change: 1 addition & 0 deletions src/tools/run-make-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ version = "0.0.0"
edition = "2021"

[dependencies]
wasmparser = "0.118.2"
16 changes: 12 additions & 4 deletions src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ use std::env;
use std::path::{Path, PathBuf};
use std::process::{Command, Output};

pub use wasmparser;

pub fn out_dir() -> PathBuf {
env::var_os("TMPDIR").unwrap().into()
}

fn setup_common_build_cmd() -> Command {
let rustc = env::var("RUSTC").unwrap();
let mut cmd = Command::new(rustc);
cmd.arg("--out-dir")
.arg(env::var("TMPDIR").unwrap())
.arg("-L")
.arg(env::var("TMPDIR").unwrap());
cmd.arg("--out-dir").arg(out_dir()).arg("-L").arg(out_dir());
cmd
}

Expand Down Expand Up @@ -45,6 +48,11 @@ impl RustcInvocationBuilder {
self
}

pub fn args(&mut self, args: &[&str]) -> &mut RustcInvocationBuilder {
self.cmd.args(args);
self
}

#[track_caller]
pub fn run(&mut self) -> Output {
let caller_location = std::panic::Location::caller();
Expand Down
2 changes: 1 addition & 1 deletion tests/assembly/wasm_exceptions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ only-wasm32-bare
//@ only-wasm32
//@ assembly-output: emit-asm
//@ compile-flags: -C target-feature=+exception-handling
//@ compile-flags: -C panic=unwind
Expand Down
1 change: 1 addition & 0 deletions tests/codegen/abi-main-signature-32bit-c-int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// This test is for targets with 32bit c_int only.
//@ ignore-msp430
//@ ignore-avr
//@ ignore-wasi wasi codegens the main symbol differently

fn main() {
}
Expand Down
1 change: 0 additions & 1 deletion tests/codegen/drop.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//@ ignore-wasm32-bare compiled with panic=abort by default
//@ needs-unwind - this test verifies the amount of drop calls when unwinding is used
//@ compile-flags: -C no-prepopulate-passes

Expand Down
1 change: 1 addition & 0 deletions tests/codegen/enum/enum-debug-clike.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

//
//@ ignore-msvc
//@ ignore-wasi wasi codegens the main symbol differently

//@ compile-flags: -g -C no-prepopulate-passes

Expand Down
1 change: 1 addition & 0 deletions tests/codegen/enum/enum-debug-niche.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// This is ignored for the fallback mode on MSVC due to problems with PDB.

//@ ignore-msvc
//@ ignore-wasi wasi codegens the main symbol differently

//@ compile-flags: -g -C no-prepopulate-passes

Expand Down
Loading

0 comments on commit dc2ffa4

Please sign in to comment.