Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 6707bf0

Browse files
committed
Auto merge of #143002 - Enselic:tests-ui-run-fail-exit-vs-signal, r=petrochenkov
tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) And introduce two new directives for ui tests: * `run-crash` * `run-fail-or-crash` Normally a `run-fail` ui test like tests that panic shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal/crash however. Introduce and use `run-crash` for those tests. Note that Windows crashes are not handled by signals but by certain high bits set on the process exit code. Example exit code for crash on Windows: `0xc000001d` (`STATUS_ILLEGAL_INSTRUCTION`). Because of this, we define "crash" on all platforms as "not exit with success and not exit with a regular failure code in the range 1..=127". Some tests behave differently on different targets: * Targets without unwind support will abort (crash) instead of exit with failure code 101 after panicking. As a special case, allow crashes for `run-fail` tests for such targets. * Different sanitizer implementations handle detected memory problems differently. Some abort (crash) the process while others exit with failure code 1. Introduce and use `run-fail-or-crash` for such tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [x] **Q:** what about on Windows? **A:** we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix. - [x] test all permutations of actual vs expected **Done:** See #143002 (comment). - [x] Handle targets without unwind support - [x] Add `run-fail-or-crash` for some sanitizer tests - [x] remote-test-client. See #143448 ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various try-job: armhf-gnu
2 parents ee3a078 + e1d4f2a commit 6707bf0

File tree

78 files changed

+196
-101
lines changed

Some content is hidden

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

78 files changed

+196
-101
lines changed

‎src/doc/rustc-dev-guide/src/tests/directives.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,10 @@ expectations](ui.md#controlling-passfail-expectations).
7575
| `check-fail` | Building (no codegen) should fail | `ui`, `crashes` | N/A |
7676
| `build-pass` | Building should pass | `ui`, `crashes`, `codegen`, `incremental` | N/A |
7777
| `build-fail` | Building should fail | `ui`, `crashes` | N/A |
78-
| `run-pass` | Running the test binary should pass | `ui`, `crashes`, `incremental` | N/A |
79-
| `run-fail` | Running the test binary should fail | `ui`, `crashes` | N/A |
78+
| `run-pass` | Program must exit with code `0` | `ui`, `crashes`, `incremental` | N/A |
79+
| `run-fail` | Program must exit with code `1..=127` | `ui`, `crashes` | N/A |
80+
| `run-crash` | Program must crash | `ui` | N/A |
81+
| `run-fail-or-crash` | Program must `run-fail` or `run-crash` | `ui` | N/A |
8082
| `ignore-pass` | Ignore `--pass` flag | `ui`, `crashes`, `codegen`, `incremental` | N/A |
8183
| `dont-check-failure-status` | Don't check exact failure status (i.e. `1`) | `ui`, `incremental` | N/A |
8284
| `failure-status` | Check | `ui`, `crashes` | Any `u16` |

‎src/doc/rustc-dev-guide/src/tests/ui.md‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ even run the resulting program. Just add one of the following
448448
- `//@ build-pass` — compilation and linking should succeed but do
449449
not run the resulting binary.
450450
- `//@ run-pass` — compilation should succeed and running the resulting
451-
binary should also succeed.
451+
binary should make it exit with code 0 which indicates success.
452452
- Fail directives:
453453
- `//@ check-fail` — compilation should fail (the codegen phase is skipped).
454454
This is the default for UI tests.
@@ -457,10 +457,20 @@ even run the resulting program. Just add one of the following
457457
- First time is to ensure that the compile succeeds without the codegen phase
458458
- Second time is to ensure that the full compile fails
459459
- `//@ run-fail` — compilation should succeed, but running the resulting
460-
binary should fail.
461-
462-
For `run-pass` and `run-fail` tests, by default the output of the program itself
463-
is not checked.
460+
binary should make it exit with a code in the range `1..=127` which
461+
indicates regular failure. On targets without unwind support, crashes
462+
are also accepted.
463+
- `//@ run-crash` — compilation should succeed, but running the resulting
464+
binary should fail with a crash. Crashing is defined as "not exiting with
465+
a code in the range `0..=127`". Example on Linux: Termination by `SIGABRT`
466+
or `SIGSEGV`. Example on Windows: Exiting with the code for
467+
`STATUS_ILLEGAL_INSTRUCTION` (`0xC000001D`).
468+
- `//@ run-fail-or-crash` — compilation should succeed, but running the
469+
resulting binary should either `run-fail` or `run-crash`. Useful if a test
470+
crashes on some targets but just fails on others.
471+
472+
For `run-pass`. `run-fail`, `run-crash` and `run-fail-or-crash` tests, by
473+
default the output of the program itself is not checked.
464474

465475
If you want to check the output of running the program, include the
466476
`check-run-results` directive. This will check for a `.run.stderr` and

‎src/tools/compiletest/src/common.rs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,37 @@ string_enum! {
8888
}
8989
}
9090

91+
string_enum! {
92+
#[derive(Clone, Copy, PartialEq, Debug, Hash)]
93+
pub enum RunResult {
94+
Pass => "run-pass",
95+
Fail => "run-fail",
96+
Crash => "run-crash",
97+
}
98+
}
99+
100+
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
101+
pub enum RunFailMode {
102+
/// Running the program must make it exit with a regular failure exit code
103+
/// in the range `1..=127`. If the program is terminated by e.g. a signal
104+
/// the test will fail.
105+
Fail,
106+
/// Running the program must result in a crash, e.g. by `SIGABRT` or
107+
/// `SIGSEGV` on Unix or on Windows by having an appropriate NTSTATUS high
108+
/// bit in the exit code.
109+
Crash,
110+
/// Running the program must either fail or crash. Useful for e.g. sanitizer
111+
/// tests since some sanitizer implementations exit the process with code 1
112+
/// to in the face of memory errors while others abort (crash) the process
113+
/// in the face of memory errors.
114+
FailOrCrash,
115+
}
116+
91117
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
92118
pub enum FailMode {
93119
Check,
94120
Build,
95-
Run,
121+
Run(RunFailMode),
96122
}
97123

98124
string_enum! {

‎src/tools/compiletest/src/directives.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use camino::{Utf8Path, Utf8PathBuf};
99
use semver::Version;
1010
use tracing::*;
1111

12-
use crate::common::{Config, Debugger, FailMode, PassMode, TestMode};
12+
use crate::common::{Config, Debugger, FailMode, PassMode, RunFailMode,TestMode};
1313
use crate::debuggers::{extract_cdb_version, extract_gdb_version};
1414
use crate::directives::auxiliary::{AuxProps, parse_and_update_aux};
1515
use crate::directives::needs::CachedNeedsConditions;
@@ -654,7 +654,13 @@ impl TestProps {
654654
Some(FailMode::Build)
655655
} else if config.parse_name_directive(ln, "run-fail") {
656656
check_ui("run");
657-
Some(FailMode::Run)
657+
Some(FailMode::Run(RunFailMode::Fail))
658+
} else if config.parse_name_directive(ln, "run-crash") {
659+
check_ui("run");
660+
Some(FailMode::Run(RunFailMode::Crash))
661+
} else if config.parse_name_directive(ln, "run-fail-or-crash") {
662+
check_ui("run");
663+
Some(FailMode::Run(RunFailMode::FailOrCrash))
658664
} else {
659665
None
660666
};
@@ -1007,7 +1013,9 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
10071013
"regex-error-pattern",
10081014
"remap-src-base",
10091015
"revisions",
1016+
"run-crash",
10101017
"run-fail",
1018+
"run-fail-or-crash",
10111019
"run-flags",
10121020
"run-pass",
10131021
"run-rustfix",

‎src/tools/compiletest/src/runtest.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use regex::{Captures, Regex};
1616
use tracing::*;
1717

1818
use crate::common::{
19-
CompareMode, Config, Debugger, FailMode, PassMode, TestMode,TestPaths,TestSuite,
20-
UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT, UI_STDERR, UI_STDOUT, UI_SVG,
19+
CompareMode, Config, Debugger, FailMode, PassMode, RunFailMode,RunResult,TestMode,TestPaths,
20+
TestSuite,UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT, UI_STDERR, UI_STDOUT, UI_SVG,
2121
UI_WINDOWS_SVG, expected_output_path, incremental_dir, output_base_dir, output_base_name,
2222
output_testname_unique,
2323
};
@@ -282,7 +282,8 @@ impl<'test> TestCx<'test> {
282282
fn should_run(&self, pm: Option<PassMode>) -> WillExecute {
283283
let test_should_run = match self.config.mode {
284284
TestMode::Ui
285-
if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) =>
285+
if pm == Some(PassMode::Run)
286+
|| matches!(self.props.fail_mode, Some(FailMode::Run(_))) =>
286287
{
287288
true
288289
}

‎src/tools/compiletest/src/runtest/ui.rs‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use rustfix::{Filter, apply_suggestions, get_suggestions_from_json};
66
use tracing::debug;
77

88
use super::{
9-
AllowUnused, Emit, FailMode, LinkToAux, PassMode, TargetLocation,TestCx,TestOutput,
10-
Truncated, UI_FIXED, WillExecute,
9+
AllowUnused, Emit, FailMode, LinkToAux, PassMode, RunFailMode,RunResult,TargetLocation,
10+
TestCx,TestOutput,Truncated, UI_FIXED, WillExecute,
1111
};
1212
use crate::json;
1313

@@ -140,12 +140,53 @@ impl TestCx<'_> {
140140
&proc_res,
141141
);
142142
}
143+
let code = proc_res.status.code();
144+
let run_result = if proc_res.status.success() {
145+
RunResult::Pass
146+
} else if code.is_some_and(|c| c >= 1 && c <= 127) {
147+
RunResult::Fail
148+
} else {
149+
RunResult::Crash
150+
};
151+
// Help users understand why the test failed by including the actual
152+
// exit code and actual run result in the failure message.
153+
let pass_hint = format!("code={code:?} so test would pass with `{run_result}`");
143154
if self.should_run_successfully(pm) {
144-
if !proc_res.status.success() {
145-
self.fatal_proc_rec("test run failed!", &proc_res);
155+
if run_result != RunResult::Pass {
156+
self.fatal_proc_rec(
157+
&format!("test did not exit with success! {pass_hint}"),
158+
&proc_res,
159+
);
160+
}
161+
} else if self.props.fail_mode == Some(FailMode::Run(RunFailMode::Fail)) {
162+
// If the test is marked as `run-fail` but do not support
163+
// unwinding we allow it to crash, since a panic will trigger an
164+
// abort (crash) instead of unwind (exit with code 101).
165+
let crash_ok = !self.config.can_unwind();
166+
if run_result != RunResult::Fail && !(crash_ok && run_result == RunResult::Crash) {
167+
let err = if crash_ok {
168+
format!(
169+
"test did not exit with failure or crash (`{}` can't unwind)! {pass_hint}",
170+
self.config.target
171+
)
172+
} else {
173+
format!("test did not exit with failure! {pass_hint}")
174+
};
175+
self.fatal_proc_rec(&err, &proc_res);
146176
}
147-
} else if proc_res.status.success() {
148-
self.fatal_proc_rec("test run succeeded!", &proc_res);
177+
} else if self.props.fail_mode == Some(FailMode::Run(RunFailMode::Crash)) {
178+
if run_result != RunResult::Crash {
179+
self.fatal_proc_rec(&format!("test did not crash! {pass_hint}"), &proc_res);
180+
}
181+
} else if self.props.fail_mode == Some(FailMode::Run(RunFailMode::FailOrCrash)) {
182+
if run_result != RunResult::Fail && run_result != RunResult::Crash {
183+
self.fatal_proc_rec(
184+
&format!("test did not exit with failure or crash! {pass_hint}"),
185+
&proc_res,
186+
);
187+
}
188+
} else {
189+
unreachable!("run_ui_test() must not be called if the test should not run");
149190
}
150191

151192
self.get_output(&proc_res)

‎tests/ui/contracts/contract-attributes-generics.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
//@ [unchk_pass] run-pass
66
//@ [chk_pass] run-pass
77
//
8-
//@ [chk_fail_pre] run-fail
9-
//@ [chk_fail_post] run-fail
10-
//@ [chk_const_fail] run-fail
8+
//@ [chk_fail_pre] run-crash
9+
//@ [chk_fail_post] run-crash
10+
//@ [chk_const_fail] run-crash
1111
//
1212
//@ [unchk_pass] compile-flags: -Zcontract-checks=no
1313
//

‎tests/ui/contracts/contract-attributes-nest.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
//@ [unchk_fail_post] run-pass
66
//@ [chk_pass] run-pass
77
//
8-
//@ [chk_fail_pre] run-fail
9-
//@ [chk_fail_post] run-fail
8+
//@ [chk_fail_pre] run-crash
9+
//@ [chk_fail_post] run-crash
1010
//
1111
//@ [unchk_pass] compile-flags: -Zcontract-checks=no
1212
//@ [unchk_fail_pre] compile-flags: -Zcontract-checks=no

‎tests/ui/contracts/contract-attributes-tail.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
//@ [unchk_fail_post] run-pass
66
//@ [chk_pass] run-pass
77
//
8-
//@ [chk_fail_pre] run-fail
9-
//@ [chk_fail_post] run-fail
8+
//@ [chk_fail_pre] run-crash
9+
//@ [chk_fail_post] run-crash
1010
//
1111
//@ [unchk_pass] compile-flags: -Zcontract-checks=no
1212
//@ [unchk_fail_pre] compile-flags: -Zcontract-checks=no

‎tests/ui/contracts/contract-captures-via-closure-copy.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//@ run-fail
1+
//@ run-crash
22
//@ compile-flags: -Zcontract-checks=yes
33

44
#![feature(contracts)]

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /