Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest #84734

tmandry · 2021-04-30T04:29:59Z

For the Fuchsia platform we build libstd with panic=abort and would like a way to run tests with that enabled. This adds low-level support for this directly to compiletest.

In the future I'd like to add high-level support in rustbuild, e.g. having target-specific flags that allow configuring a panic strategy. (Side note: It would be nice if we could also build multiple configurations for the same target, but I'm getting ahead of myself.)

This plus #84500 have everything that's needed to get ui tests passing on fuchsia targets.

Part of #84766. Note that this change only includes the header on tests which need an unwinder to build, not those which need it to run.

r? @Mark-Simulacrum

bjorn3 · 2021-04-30T17:54:49Z

Are you sure those are all the tests that need it? For cg_clif I get a much larger set of tests: https://github.com/bjorn3/rustc_codegen_cranelift/blob/6d30315d055e92464026c2ac0d7f3ad8b02539b6/scripts/test_rustc_tests.sh#L13-L16 and https://github.com/bjorn3/rustc_codegen_cranelift/blob/6d30315d055e92464026c2ac0d7f3ad8b02539b6/scripts/test_rustc_tests.sh#L24-L41

tmandry · 2021-04-30T18:10:20Z

@bjorn3 ah it's because I disabled running any of the UI tests. I should build a panic=abort libstd on linux and get the full set that way. Good to know there's another use case for this, though..

src/tools/compiletest/src/main.rs

Mark-Simulacrum · 2021-05-06T02:03:39Z

This seems good to me; I imagine it may make sense to add the support and then actually annotate all the tests in followup PRs. I left a nit, but please make sure to squash it into the existing commits.

With regards to future extensions (e.g., multiple target variants, etc.) I think support for configuring the panic strategy desired seems OK, but allowing one to build multiple 'identical' targets but with different panic strategies or other attributes is probably best left out of scope for now. It may become (more) desirable as we gain further support for build-std or similar features, where it may make sense to "prebuild" std for a variety of configurations, so that users can opt-in via some standard mechanism to those builds, perhaps before we allow them to actually build std themselves.

I just need this until rustbuild supports -Cpanic=abort std directly.

Mark-Simulacrum · 2021-05-06T02:57:24Z

Let me know what strategy you want to take with the annotations, but r=me on this first draft of you want to followup with those

tmandry · 2021-05-06T03:24:47Z

@Mark-Simulacrum Yes, I'd prefer to do those in a follow-up. I also want to make sure that these have a way of being tested in CI (probably as part of the wasm target)

@bors r=Mark-Simulacrum

bors · 2021-05-06T03:24:48Z

📌 Commit 947ad58 has been approved by Mark-Simulacrum

…=Mark-Simulacrum Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest For the Fuchsia platform we build libstd with `panic=abort` and would like a way to run tests with that enabled. This adds low-level support for this directly to compiletest. In the future I'd like to add high-level support in rustbuild, e.g. having target-specific flags that allow configuring a panic strategy. (Side note: It would be nice if we could also build multiple configurations for the same target, but I'm getting ahead of myself.) This plus rust-lang#84500 have everything that's needed to get ui tests passing on fuchsia targets. Part of rust-lang#84766. Note that this change only includes the header on tests which need an unwinder to _build_, not those which need it to _run_. r? `@Mark-Simulacrum`

…=Mark-Simulacrum Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest For the Fuchsia platform we build libstd with `panic=abort` and would like a way to run tests with that enabled. This adds low-level support for this directly to compiletest. In the future I'd like to add high-level support in rustbuild, e.g. having target-specific flags that allow configuring a panic strategy. (Side note: It would be nice if we could also build multiple configurations for the same target, but I'm getting ahead of myself.) This plus rust-lang#84500 have everything that's needed to get ui tests passing on fuchsia targets. Part of rust-lang#84766. Note that this change only includes the header on tests which need an unwinder to _build_, not those which need it to _run_. r? ``@Mark-Simulacrum``

…=Mark-Simulacrum Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest For the Fuchsia platform we build libstd with `panic=abort` and would like a way to run tests with that enabled. This adds low-level support for this directly to compiletest. In the future I'd like to add high-level support in rustbuild, e.g. having target-specific flags that allow configuring a panic strategy. (Side note: It would be nice if we could also build multiple configurations for the same target, but I'm getting ahead of myself.) This plus rust-lang#84500 have everything that's needed to get ui tests passing on fuchsia targets. Part of rust-lang#84766. Note that this change only includes the header on tests which need an unwinder to _build_, not those which need it to _run_. r? ```@Mark-Simulacrum```

Rollup of 11 pull requests Successful merges: - rust-lang#84409 (Ensure TLS destructors run before thread joins in SGX) - rust-lang#84500 (Add --run flag to compiletest) - rust-lang#84728 (Add test for suggestion to borrow unsized function parameters) - rust-lang#84734 (Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest) - rust-lang#84755 (Allow using `core::` in intra-doc links within core itself) - rust-lang#84871 (Disallows `#![feature(no_coverage)]` on stable and beta (using standard crate-level gating)) - rust-lang#84872 (Wire up tidy dependency checks for cg_clif) - rust-lang#84896 (Handle incorrect placement of parentheses in trait bounds more gracefully) - rust-lang#84905 (CTFE engine: rename copy → copy_intrinsic, move to intrinsics.rs) - rust-lang#84953 (Remove unneeded call to with_default_session_globals in rustdoc highlight) - rust-lang#84987 (small nits) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned Mark-Simulacrum Apr 30, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2021

This comment has been minimized.

Sign in to view

tmandry force-pushed the compiletest-needs-unwind branch from b01f6a5 to 64b9061 Compare April 30, 2021 04:50

This comment has been minimized.

Sign in to view

tmandry force-pushed the compiletest-needs-unwind branch from 64b9061 to c65ba57 Compare April 30, 2021 05:04

This comment has been minimized.

Sign in to view

tmandry force-pushed the compiletest-needs-unwind branch 2 times, most recently from 4e16d2e to 9cda465 Compare April 30, 2021 06:44

tmandry changed the title ~~Add needs-unwind and support for testing panic=abort libstd to compiletest~~ Add needs-unwind and beginning of support for testing panic=abort std to compiletest Apr 30, 2021

Mark-Simulacrum reviewed May 6, 2021

View reviewed changes

src/tools/compiletest/src/main.rs Outdated Show resolved Hide resolved

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 6, 2021

tmandry added 4 commits May 6, 2021 02:49

compiletest: Add --target-panic, needs-unwind

568d9c5

Add needs-unwind to tests

e1a8ecf

Support multi target-rustcflags for -Zpanic-abort-tests

1993e1a

I just need this until rustbuild supports -Cpanic=abort std directly.

Fix up/ignore failing ui tests on fuchsia

947ad58

tmandry force-pushed the compiletest-needs-unwind branch from 9cda465 to 947ad58 Compare May 6, 2021 02:49

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 6, 2021

Dylan-DPC-zz mentioned this pull request May 6, 2021

Rollup of 16 pull requests #84975

Closed

This was referenced May 6, 2021

Rollup of 11 pull requests #85005

Closed

Rollup of 11 pull requests #85006

Closed

Dylan-DPC-zz mentioned this pull request May 6, 2021

Rollup of 11 pull requests #85014

Merged

bors merged commit 577f1d0 into rust-lang:master May 7, 2021

rustbot added this to the 1.54.0 milestone May 7, 2021

tmandry deleted the compiletest-needs-unwind branch May 7, 2021 03:57

ehuss mentioned this pull request Aug 12, 2022

compiletest: use target cfg instead of hard-coded tables #100260

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest #84734

Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest #84734

tmandry commented Apr 30, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bjorn3 commented Apr 30, 2021 •

edited

Loading

tmandry commented Apr 30, 2021 •

edited

Loading

Mark-Simulacrum commented May 6, 2021

Mark-Simulacrum commented May 6, 2021

tmandry commented May 6, 2021

bors commented May 6, 2021

Add needs-unwind and beginning of support for testing panic=abort std to compiletest #84734

Add needs-unwind and beginning of support for testing panic=abort std to compiletest #84734

Conversation

tmandry commented Apr 30, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bjorn3 commented Apr 30, 2021 • edited Loading

tmandry commented Apr 30, 2021 • edited Loading

Mark-Simulacrum commented May 6, 2021

Mark-Simulacrum commented May 6, 2021

tmandry commented May 6, 2021

bors commented May 6, 2021

Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest #84734

Add `needs-unwind` and beginning of support for testing `panic=abort` std to compiletest #84734

tmandry commented Apr 30, 2021 •

edited

Loading

bjorn3 commented Apr 30, 2021 •

edited

Loading

tmandry commented Apr 30, 2021 •

edited

Loading