Rollup merge of rust-lang#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 rust-lang#142304, rust-lang#142886) protection against the regression in rust-lang#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 rust-lang#143002 (comment).
- [x] Handle targets without unwind support
- [x] Add `run-fail-or-crash` for some sanitizer tests
- [x] remote-test-client. See rust-lang#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

Author: fmease

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

@@ -75,8 +75,10 @@ expectations](ui.md#controlling-passfail-expectations).
 | `check-fail`                | Building (no codegen) should fail           | `ui`, `crashes`                           | N/A             |
 | `build-pass`                | Building should pass                        | `ui`, `crashes`, `codegen`, `incremental` | N/A             |
 | `build-fail`                | Building should fail                        | `ui`, `crashes`                           | N/A             |
-| `run-pass`                  | Running the test binary should pass         | `ui`, `crashes`, `incremental`            | N/A             |
-| `run-fail`                  | Running the test binary should fail         | `ui`, `crashes`                           | N/A             |
+| `run-pass`                  | Program must exit with code `0`             | `ui`, `crashes`, `incremental`            | N/A             |
+| `run-fail`                  | Program must exit with code `1..=127`       | `ui`, `crashes`                           | N/A             |
+| `run-crash`                 | Program must crash                          | `ui`                                      | N/A             |
+| `run-fail-or-crash`         | Program must `run-fail` or `run-crash`      | `ui`                                      | N/A             |
 | `ignore-pass`               | Ignore `--pass` flag                        | `ui`, `crashes`, `codegen`, `incremental` | N/A             |
 | `dont-check-failure-status` | Don't check exact failure status (i.e. `1`) | `ui`, `incremental`                       | N/A             |
 | `failure-status`            | Check                                       | `ui`, `crashes`                           | Any `u16`       |

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

@@ -448,7 +448,7 @@ even run the resulting program. Just add one of the following
   - `//@ build-pass` — compilation and linking should succeed but do
     not run the resulting binary.
   - `//@ run-pass` — compilation should succeed and running the resulting
-    binary should also succeed.
+    binary should make it exit with code 0 which indicates success.
 - Fail directives:
   - `//@ check-fail` — compilation should fail (the codegen phase is skipped).
     This is the default for UI tests.
@@ -457,10 +457,20 @@ even run the resulting program. Just add one of the following
     - First time is to ensure that the compile succeeds without the codegen phase
     - Second time is to ensure that the full compile fails
   - `//@ run-fail` — compilation should succeed, but running the resulting
-    binary should fail.
-
-For `run-pass` and `run-fail` tests, by default the output of the program itself
-is not checked.
+    binary should make it exit with a code in the range `1..=127` which
+    indicates regular failure. On targets without unwind support, crashes
+    are also accepted.
+  - `//@ run-crash` — compilation should succeed, but running the resulting
+    binary should fail with a crash. Crashing is defined as "not exiting with
+    a code in the range `0..=127`". Example on Linux: Termination by `SIGABRT`
+    or `SIGSEGV`. Example on Windows: Exiting with the code for
+    `STATUS_ILLEGAL_INSTRUCTION` (`0xC000001D`).
+  - `//@ run-fail-or-crash` — compilation should succeed, but running the
+    resulting binary should either `run-fail` or `run-crash`. Useful if a test
+    crashes on some targets but just fails on others.
+
+For `run-pass`. `run-fail`, `run-crash` and `run-fail-or-crash` tests, by
+default the output of the program itself is not checked.
 
 If you want to check the output of running the program, include the
 `check-run-results` directive. This will check for a `.run.stderr` and

src/tools/compiletest/src/common.rs

@@ -88,11 +88,37 @@ string_enum! {
     }
 }
 
+string_enum! {
+    #[derive(Clone, Copy, PartialEq, Debug, Hash)]
+    pub enum RunResult {
+        Pass => "run-pass",
+        Fail => "run-fail",
+        Crash => "run-crash",
+    }
+}
+
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum RunFailMode {
+    /// Running the program must make it exit with a regular failure exit code
+    /// in the range `1..=127`. If the program is terminated by e.g. a signal
+    /// the test will fail.
+    Fail,
+    /// Running the program must result in a crash, e.g. by `SIGABRT` or
+    /// `SIGSEGV` on Unix or on Windows by having an appropriate NTSTATUS high
+    /// bit in the exit code.
+    Crash,
+    /// Running the program must either fail or crash. Useful for e.g. sanitizer
+    /// tests since some sanitizer implementations exit the process with code 1
+    /// to in the face of memory errors while others abort (crash) the process
+    /// in the face of memory errors.
+    FailOrCrash,
+}
+
 #[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
 pub enum FailMode {
     Check,
     Build,
-    Run,
+    Run(RunFailMode),
 }
 
 string_enum! {

src/tools/compiletest/src/directives.rs

@@ -9,7 +9,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 use semver::Version;
 use tracing::*;
 
-use crate::common::{Config, Debugger, FailMode, PassMode, TestMode};
+use crate::common::{Config, Debugger, FailMode, PassMode, RunFailMode, TestMode};
 use crate::debuggers::{extract_cdb_version, extract_gdb_version};
 use crate::directives::auxiliary::{AuxProps, parse_and_update_aux};
 use crate::directives::needs::CachedNeedsConditions;
@@ -654,7 +654,13 @@ impl TestProps {
             Some(FailMode::Build)
         } else if config.parse_name_directive(ln, "run-fail") {
             check_ui("run");
-            Some(FailMode::Run)
+            Some(FailMode::Run(RunFailMode::Fail))
+        } else if config.parse_name_directive(ln, "run-crash") {
+            check_ui("run");
+            Some(FailMode::Run(RunFailMode::Crash))
+        } else if config.parse_name_directive(ln, "run-fail-or-crash") {
+            check_ui("run");
+            Some(FailMode::Run(RunFailMode::FailOrCrash))
         } else {
             None
         };
@@ -1007,7 +1013,9 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
     "regex-error-pattern",
     "remap-src-base",
     "revisions",
+    "run-crash",
     "run-fail",
+    "run-fail-or-crash",
     "run-flags",
     "run-pass",
     "run-rustfix",

src/tools/compiletest/src/runtest.rs

@@ -16,8 +16,8 @@ use regex::{Captures, Regex};
 use tracing::*;
 
 use crate::common::{
-    CompareMode, Config, Debugger, FailMode, PassMode, TestMode, TestPaths, TestSuite,
-    UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT, UI_STDERR, UI_STDOUT, UI_SVG,
+    CompareMode, Config, Debugger, FailMode, PassMode, RunFailMode, RunResult, TestMode, TestPaths,
+    TestSuite, UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT, UI_STDERR, UI_STDOUT, UI_SVG,
     UI_WINDOWS_SVG, expected_output_path, incremental_dir, output_base_dir, output_base_name,
     output_testname_unique,
 };
@@ -282,7 +282,8 @@ impl<'test> TestCx<'test> {
     fn should_run(&self, pm: Option<PassMode>) -> WillExecute {
         let test_should_run = match self.config.mode {
             TestMode::Ui
-                if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) =>
+                if pm == Some(PassMode::Run)
+                    || matches!(self.props.fail_mode, Some(FailMode::Run(_))) =>
             {
                 true
             }

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

@@ -6,8 +6,8 @@ use rustfix::{Filter, apply_suggestions, get_suggestions_from_json};
 use tracing::debug;
 
 use super::{
-    AllowUnused, Emit, FailMode, LinkToAux, PassMode, TargetLocation, TestCx, TestOutput,
-    Truncated, UI_FIXED, WillExecute,
+    AllowUnused, Emit, FailMode, LinkToAux, PassMode, RunFailMode, RunResult, TargetLocation,
+    TestCx, TestOutput, Truncated, UI_FIXED, WillExecute,
 };
 use crate::json;
 
@@ -140,12 +140,53 @@ impl TestCx<'_> {
                     &proc_res,
                 );
             }
+            let code = proc_res.status.code();
+            let run_result = if proc_res.status.success() {
+                RunResult::Pass
+            } else if code.is_some_and(|c| c >= 1 && c <= 127) {
+                RunResult::Fail
+            } else {
+                RunResult::Crash
+            };
+            // Help users understand why the test failed by including the actual
+            // exit code and actual run result in the failure message.
+            let pass_hint = format!("code={code:?} so test would pass with `{run_result}`");
             if self.should_run_successfully(pm) {
-                if !proc_res.status.success() {
-                    self.fatal_proc_rec("test run failed!", &proc_res);
+                if run_result != RunResult::Pass {
+                    self.fatal_proc_rec(
+                        &format!("test did not exit with success! {pass_hint}"),
+                        &proc_res,
+                    );
+                }
+            } else if self.props.fail_mode == Some(FailMode::Run(RunFailMode::Fail)) {
+                // If the test is marked as `run-fail` but do not support
+                // unwinding we allow it to crash, since a panic will trigger an
+                // abort (crash) instead of unwind (exit with code 101).
+                let crash_ok = !self.config.can_unwind();
+                if run_result != RunResult::Fail && !(crash_ok && run_result == RunResult::Crash) {
+                    let err = if crash_ok {
+                        format!(
+                            "test did not exit with failure or crash (`{}` can't unwind)! {pass_hint}",
+                            self.config.target
+                        )
+                    } else {
+                        format!("test did not exit with failure! {pass_hint}")
+                    };
+                    self.fatal_proc_rec(&err, &proc_res);
                 }
-            } else if proc_res.status.success() {
-                self.fatal_proc_rec("test run succeeded!", &proc_res);
+            } else if self.props.fail_mode == Some(FailMode::Run(RunFailMode::Crash)) {
+                if run_result != RunResult::Crash {
+                    self.fatal_proc_rec(&format!("test did not crash! {pass_hint}"), &proc_res);
+                }
+            } else if self.props.fail_mode == Some(FailMode::Run(RunFailMode::FailOrCrash)) {
+                if run_result != RunResult::Fail && run_result != RunResult::Crash {
+                    self.fatal_proc_rec(
+                        &format!("test did not exit with failure or crash! {pass_hint}"),
+                        &proc_res,
+                    );
+                }
+            } else {
+                unreachable!("run_ui_test() must not be called if the test should not run");
             }
 
             self.get_output(&proc_res)

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

@@ -5,9 +5,9 @@
 //@ [unchk_pass] run-pass
 //@ [chk_pass] run-pass
 //
-//@ [chk_fail_pre] run-fail
-//@ [chk_fail_post] run-fail
-//@ [chk_const_fail] run-fail
+//@ [chk_fail_pre] run-crash
+//@ [chk_fail_post] run-crash
+//@ [chk_const_fail] run-crash
 //
 //@ [unchk_pass] compile-flags: -Zcontract-checks=no
 //

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

@@ -5,8 +5,8 @@
 //@ [unchk_fail_post] run-pass
 //@ [chk_pass] run-pass
 //
-//@ [chk_fail_pre] run-fail
-//@ [chk_fail_post] run-fail
+//@ [chk_fail_pre] run-crash
+//@ [chk_fail_post] run-crash
 //
 //@ [unchk_pass] compile-flags: -Zcontract-checks=no
 //@ [unchk_fail_pre] compile-flags: -Zcontract-checks=no

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

@@ -5,8 +5,8 @@
 //@ [unchk_fail_post] run-pass
 //@ [chk_pass] run-pass
 //
-//@ [chk_fail_pre] run-fail
-//@ [chk_fail_post] run-fail
+//@ [chk_fail_pre] run-crash
+//@ [chk_fail_post] run-crash
 //
 //@ [unchk_pass] compile-flags: -Zcontract-checks=no
 //@ [unchk_fail_pre] compile-flags: -Zcontract-checks=no

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

@@ -1,4 +1,4 @@
-//@ run-fail
+//@ run-crash
 //@ compile-flags: -Zcontract-checks=yes
 
 #![feature(contracts)]