Skip to content

Commit 1725edc

Browse files
committed
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
1 parent 50e77f1 commit 1725edc

File tree

314 files changed

+836
-860
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

314 files changed

+836
-860
lines changed

Cargo.lock

+3
Original file line numberDiff line numberDiff line change
@@ -3305,6 +3305,9 @@ dependencies = [
33053305
[[package]]
33063306
name = "run_make_support"
33073307
version = "0.0.0"
3308+
dependencies = [
3309+
"wasmparser",
3310+
]
33083311

33093312
[[package]]
33103313
name = "rust-demangler"

config.example.toml

+10
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,16 @@
838838
# See that option for more info.
839839
#codegen-backends = rust.codegen-backends (array)
840840

841+
# This is a "runtool" to pass to `compiletest` when executing tests. Tests will
842+
# execute this tool where the binary-to-test is passed as an argument. Can
843+
# be useful for situations such as when WebAssembly is being tested and a
844+
# runtime needs to be configured.
845+
#
846+
# This configuration is a space-separated list of arguments so `foo bar` would
847+
# execute the program `foo` with the first argument as `bar` and the second
848+
# argument as the test binary.
849+
#runtime = <none> (string)
850+
841851
# =============================================================================
842852
# Distribution options
843853
#

library/test/src/console.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -323,10 +323,11 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Resu
323323
// Prevent the usage of `Instant` in some cases:
324324
// - It's currently not supported for wasm targets.
325325
// - We disable it for miri because it's not available when isolation is enabled.
326-
let is_instant_supported =
327-
!cfg!(target_family = "wasm") && !cfg!(target_os = "zkvm") && !cfg!(miri);
326+
let is_instant_unsupported = (cfg!(target_family = "wasm") && !cfg!(target_os = "wasi"))
327+
|| cfg!(target_os = "zkvm")
328+
|| cfg!(miri);
328329

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

src/bootstrap/src/core/build_steps/test.rs

+7-12
Original file line numberDiff line numberDiff line change
@@ -1656,8 +1656,8 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
16561656
// ensure that `libproc_macro` is available on the host.
16571657
builder.ensure(compile::Std::new(compiler, compiler.host));
16581658

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

@@ -1970,6 +1970,8 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
19701970

19711971
if builder.remote_tested(target) {
19721972
cmd.arg("--remote-test-client").arg(builder.tool_exe(Tool::RemoteTestClient));
1973+
} else if let Some(tool) = builder.runtool(target) {
1974+
cmd.arg("--runtool").arg(tool);
19731975
}
19741976

19751977
if suite != "mir-opt" {
@@ -2505,20 +2507,13 @@ fn prepare_cargo_test(
25052507
dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
25062508
cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
25072509

2508-
if target.contains("emscripten") {
2509-
cargo.env(
2510-
format!("CARGO_TARGET_{}_RUNNER", envify(&target.triple)),
2511-
builder.config.nodejs.as_ref().expect("nodejs not configured"),
2512-
);
2513-
} else if target.starts_with("wasm32") {
2514-
let node = builder.config.nodejs.as_ref().expect("nodejs not configured");
2515-
let runner = format!("{} {}/src/etc/wasm32-shim.js", node.display(), builder.src.display());
2516-
cargo.env(format!("CARGO_TARGET_{}_RUNNER", envify(&target.triple)), &runner);
2517-
} else if builder.remote_tested(target) {
2510+
if builder.remote_tested(target) {
25182511
cargo.env(
25192512
format!("CARGO_TARGET_{}_RUNNER", envify(&target.triple)),
25202513
format!("{} run 0", builder.tool_exe(Tool::RemoteTestClient).display()),
25212514
);
2515+
} else if let Some(tool) = builder.runtool(target) {
2516+
cargo.env(format!("CARGO_TARGET_{}_RUNNER", envify(&target.triple)), tool);
25222517
}
25232518

25242519
cargo

src/bootstrap/src/core/config/config.rs

+3
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,7 @@ pub struct Target {
578578
pub musl_libdir: Option<PathBuf>,
579579
pub wasi_root: Option<PathBuf>,
580580
pub qemu_rootfs: Option<PathBuf>,
581+
pub runtool: Option<String>,
581582
pub no_std: bool,
582583
pub codegen_backends: Option<Vec<Interned<String>>>,
583584
}
@@ -1140,6 +1141,7 @@ define_config! {
11401141
qemu_rootfs: Option<String> = "qemu-rootfs",
11411142
no_std: Option<bool> = "no-std",
11421143
codegen_backends: Option<Vec<String>> = "codegen-backends",
1144+
runtool: Option<String> = "runtool",
11431145
}
11441146
}
11451147

@@ -1858,6 +1860,7 @@ impl Config {
18581860
target.musl_libdir = cfg.musl_libdir.map(PathBuf::from);
18591861
target.wasi_root = cfg.wasi_root.map(PathBuf::from);
18601862
target.qemu_rootfs = cfg.qemu_rootfs.map(PathBuf::from);
1863+
target.runtool = cfg.runtool;
18611864
target.sanitizers = cfg.sanitizers;
18621865
target.profiler = cfg.profiler;
18631866
target.rpath = cfg.rpath;

src/bootstrap/src/lib.rs

+38
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,44 @@ impl Build {
13531353
|| env::var_os("TEST_DEVICE_ADDR").is_some()
13541354
}
13551355

1356+
/// Returns an optional "runtool" to pass to `compiletest` when executing
1357+
/// test binaries.
1358+
///
1359+
/// An example of this would be a WebAssembly runtime when testing the wasm
1360+
/// targets.
1361+
fn runtool(&self, target: TargetSelection) -> Option<String> {
1362+
let configured_runtool =
1363+
self.config.target_config.get(&target).and_then(|t| t.runtool.as_ref()).map(|p| &**p);
1364+
if let Some(runtool) = configured_runtool {
1365+
return Some(runtool.to_owned());
1366+
}
1367+
1368+
if target.starts_with("wasm") && target.contains("wasi") {
1369+
self.default_wasi_runtool()
1370+
} else {
1371+
None
1372+
}
1373+
}
1374+
1375+
/// When a `runtool` configuration is not provided and a WASI-looking target
1376+
/// is being tested this is consulted to prove the environment to see if
1377+
/// there's a runtime already lying around that seems reasonable to use.
1378+
fn default_wasi_runtool(&self) -> Option<String> {
1379+
let mut finder = crate::core::sanity::Finder::new();
1380+
1381+
// Look for Wasmtime, and for its default options be sure to disable
1382+
// its caching system since we're executing quite a lot of tests and
1383+
// ideally shouldn't pollute the cache too much.
1384+
if let Some(path) = finder.maybe_have("wasmtime") {
1385+
if let Ok(mut path) = path.into_os_string().into_string() {
1386+
path.push_str(" run -C cache=n --dir .");
1387+
return Some(path);
1388+
}
1389+
}
1390+
1391+
None
1392+
}
1393+
13561394
/// Returns the root of the "rootfs" image that this target will be using,
13571395
/// if one was configured.
13581396
///

src/ci/docker/host-x86_64/test-various/Dockerfile

+14-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ FROM ubuntu:22.04
33
ARG DEBIAN_FRONTEND=noninteractive
44
RUN apt-get update && apt-get install -y --no-install-recommends \
55
clang-11 \
6+
llvm-11 \
67
g++ \
78
make \
89
ninja-build \
@@ -38,10 +39,14 @@ WORKDIR /
3839
COPY scripts/sccache.sh /scripts/
3940
RUN sh /scripts/sccache.sh
4041

42+
COPY host-x86_64/dist-various-2/build-wasi-toolchain.sh /tmp/
43+
RUN /tmp/build-wasi-toolchain.sh
44+
4145
ENV RUST_CONFIGURE_ARGS \
4246
--musl-root-x86_64=/usr/local/x86_64-linux-musl \
4347
--set build.nodejs=/node-v18.12.0-linux-x64/bin/node \
44-
--set rust.lld
48+
--set rust.lld \
49+
--set target.wasm32-wasip1.wasi-root=/wasm32-wasip1
4550

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

53-
ENV WASM_TARGETS=wasm32-unknown-unknown
58+
RUN curl -L https://github.com/bytecodealliance/wasmtime/releases/download/v18.0.2/wasmtime-v18.0.2-x86_64-linux.tar.xz | \
59+
tar -xJ
60+
ENV PATH "$PATH:/wasmtime-v18.0.2-x86_64-linux"
61+
62+
ENV WASM_TARGETS=wasm32-wasip1
5463
ENV WASM_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $WASM_TARGETS \
5564
tests/run-make \
5665
tests/ui \
@@ -59,7 +68,9 @@ ENV WASM_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $WASM_T
5968
tests/codegen \
6069
tests/assembly \
6170
library/core
62-
ENV CC_wasm32_unknown_unknown=clang-11
71+
ENV CC_wasm32_wasip1=clang-11 \
72+
CFLAGS_wasm32_wasip1="--sysroot /wasm32-wasip1" \
73+
AR_wasm32_wasip1=llvm-ar-11
6374

6475
ENV NVPTX_TARGETS=nvptx64-nvidia-cuda
6576
ENV NVPTX_SCRIPT python3 /checkout/x.py --stage 2 test --host='' --target $NVPTX_TARGETS \

src/etc/wasm32-shim.js

-24
This file was deleted.

src/tools/compiletest/src/common.rs

+9
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,15 @@ impl Config {
451451
self.target_cfg().panic == PanicStrategy::Unwind
452452
}
453453

454+
pub fn has_threads(&self) -> bool {
455+
// Wasm targets don't have threads unless `-threads` is in the target
456+
// name, such as `wasm32-wasip1-threads`.
457+
if self.target.starts_with("wasm") {
458+
return self.target.contains("threads");
459+
}
460+
true
461+
}
462+
454463
pub fn has_asm_support(&self) -> bool {
455464
static ASM_SUPPORTED_ARCHS: &[&str] = &[
456465
"x86", "x86_64", "arm", "aarch64", "riscv32",

src/tools/compiletest/src/header.rs

+1
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
787787
"needs-sanitizer-shadow-call-stack",
788788
"needs-sanitizer-support",
789789
"needs-sanitizer-thread",
790+
"needs-threads",
790791
"needs-unwind",
791792
"needs-xray",
792793
"no-prefer-dynamic",

src/tools/compiletest/src/header/needs.rs

+5
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ pub(super) fn handle_needs(
8484
condition: config.run_enabled(),
8585
ignore_reason: "ignored when running the resulting test binaries is disabled",
8686
},
87+
Need {
88+
name: "needs-threads",
89+
condition: config.has_threads(),
90+
ignore_reason: "ignored on targets without threading support",
91+
},
8792
Need {
8893
name: "needs-unwind",
8994
condition: config.can_unwind(),

0 commit comments

Comments
 (0)