Auto merge of #143002 - Enselic:tests-ui-run-fail-exit-vs-signal, r=<try>

tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok)

Normally a `run-fail` ui test 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 however. Introduce and use `run-fail-without-exit-code` for those 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
- [ ] what about on Windows?
- [ ] also update docs at https://rustc-dev-guide.rust-lang.org/tests/directives.html#controlling-outcome-expectations
- [ ] clean up the code

### 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: x86_64-msvc-1
try-job: x86_64-msvc-2

Author: rust-bors[bot]

src/tools/compiletest/src/common.rs

@@ -110,11 +110,22 @@ string_enum! {
     }
 }
 
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum RunFailMode {
+    /// Running the program must exit with an exit code >= 1 && <= 127. If the
+    /// program is terminated by a signal (or the exit code indicates abnormal
+    /// exit) the test will fail.
+    FailureExitCode,
+    /// The running program must have crashed, e.g. by SIGABRT on Unix or
+    /// Windows by setting an appropriate NTSTATUS high bit in the exit code.
+    Crash,
+}
+
 #[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
 pub enum FailMode {
     Check,
     Build,
-    Run,
+    Run(RunFailMode),
 }
 
 string_enum! {

src/tools/compiletest/src/directive-list.rs

@@ -241,6 +241,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
     "regex-error-pattern",
     "remap-src-base",
     "revisions",
+    "run-crash",
     "run-fail",
     "run-flags",
     "run-pass",

src/tools/compiletest/src/header.rs

@@ -9,7 +9,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 use semver::Version;
 use tracing::*;
 
-use crate::common::{Config, Debugger, FailMode, Mode, PassMode};
+use crate::common::{Config, Debugger, FailMode, Mode, PassMode, RunFailMode};
 use crate::debuggers::{extract_cdb_version, extract_gdb_version};
 use crate::errors::ErrorKind;
 use crate::executor::{CollectedTestDesc, ShouldPanic};
@@ -655,7 +655,10 @@ impl TestProps {
             Some(FailMode::Build)
         } else if config.parse_name_directive(ln, "run-fail") {
             check_ui("run");
-            Some(FailMode::Run)
+            Some(FailMode::Run(RunFailMode::FailureExitCode))
+        } else if config.parse_name_directive(ln, "run-crash") {
+            check_ui("run");
+            Some(FailMode::Run(RunFailMode::Crash))
         } else {
             None
         };

src/tools/compiletest/src/runtest.rs

@@ -17,10 +17,10 @@ use tracing::*;
 
 use crate::common::{
     Assembly, Codegen, CodegenUnits, CompareMode, Config, CoverageMap, CoverageRun, Crashes,
-    DebugInfo, Debugger, FailMode, Incremental, MirOpt, PassMode, Pretty, RunMake, Rustdoc,
-    RustdocJs, RustdocJson, TestPaths, UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT,
-    UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, incremental_dir,
-    output_base_dir, output_base_name, output_testname_unique,
+    DebugInfo, Debugger, FailMode, Incremental, MirOpt, PassMode, Pretty, RunFailMode, RunMake,
+    Rustdoc, RustdocJs, RustdocJson, TestPaths, UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR,
+    UI_RUN_STDOUT, UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path,
+    incremental_dir, output_base_dir, output_base_name, output_testname_unique,
 };
 use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff};
 use crate::errors::{Error, ErrorKind, load_errors};
@@ -277,7 +277,11 @@ impl<'test> TestCx<'test> {
 
     fn should_run(&self, pm: Option<PassMode>) -> WillExecute {
         let test_should_run = match self.config.mode {
-            Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => true,
+            Ui if pm == Some(PassMode::Run)
+                || matches!(self.props.fail_mode, Some(FailMode::Run(_))) =>
+            {
+                true
+            }
             MirOpt if pm == Some(PassMode::Run) => true,
             Ui | MirOpt => false,
             mode => panic!("unimplemented for mode {:?}", mode),
@@ -296,6 +300,13 @@ impl<'test> TestCx<'test> {
         }
     }
 
+    fn must_have_exit_code(&self) -> bool {
+        match self.config.mode {
+            Ui => self.props.fail_mode == Some(FailMode::Run(RunFailMode::FailureExitCode)),
+            mode => panic!("unimplemented for mode {:?}", mode),
+        }
+    }
+
     fn should_compile_successfully(&self, pm: Option<PassMode>) -> bool {
         match self.config.mode {
             RustdocJs => true,

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

@@ -140,14 +140,45 @@ impl TestCx<'_> {
                     &proc_res,
                 );
             }
+            #[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+            enum DetailedExitStatus {
+                ExitWithSuccess,
+                ExitWithFailure { code: i32 },
+                Crashed { code: Option<i32> },
+            }
+            fn code_is_proper_failure_exit_code(code: i32) -> bool {
+                code >= 1 && code <= 127
+            }
+            let code = proc_res.status.code();
+            let actual_status = if proc_res.status.success() {
+                DetailedExitStatus::ExitWithSuccess
+            } else if let Some(code) = code
+                && code_is_proper_failure_exit_code(code)
+            {
+                DetailedExitStatus::ExitWithFailure { code }
+            } else {
+                DetailedExitStatus::Crashed { code }
+            };
             if self.should_run_successfully(pm) {
-                if !proc_res.status.success() {
-                    self.fatal_proc_rec("test run failed!", &proc_res);
+                if actual_status != DetailedExitStatus::ExitWithSuccess {
+                    self.fatal_proc_rec(
+                        &format!("test run failed! got {actual_status:?}"),
+                        &proc_res,
+                    );
                 }
-            } else if proc_res.status.success() {
-                self.fatal_proc_rec("test run succeeded!", &proc_res);
+            } else if self.must_have_exit_code() {
+                if !matches!(actual_status, DetailedExitStatus::ExitWithFailure { .. }) {
+                    self.fatal_proc_rec(
+                        &format!("test did not exit with failure code! got {actual_status:?}"),
+                        &proc_res,
+                    );
+                }
+            } else if !matches!(actual_status, DetailedExitStatus::Crashed { .. }) {
+                self.fatal_proc_rec(
+                    &format!("test did not crash! got {actual_status:?}"),
+                    &proc_res,
+                );
             }
-
             self.get_output(&proc_res)
         } else {
             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)]

tests/ui/contracts/contract-const-fn.rs

@@ -8,8 +8,8 @@
 //
 //@ [all_pass] run-pass
 //
-//@ [runtime_fail_pre] run-fail
-//@ [runtime_fail_post] run-fail
+//@ [runtime_fail_pre] run-crash
+//@ [runtime_fail_post] run-crash
 //
 //@ [all_pass] compile-flags: -Zcontract-checks=yes
 //@ [runtime_fail_pre] compile-flags: -Zcontract-checks=yes