Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiletest ice tracking #122997

Merged
merged 13 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1400,6 +1400,8 @@ impl Step for RunMakeSupport {

default_test!(Ui { path: "tests/ui", mode: "ui", suite: "ui" });

default_test!(Crashes { path: "tests/crashes", mode: "crashes", suite: "crashes" });

default_test!(RunPassValgrind {
path: "tests/run-pass-valgrind",
mode: "run-pass-valgrind",
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,7 @@ impl<'a> Builder<'a> {
test::ExpandYamlAnchors,
test::Tidy,
test::Ui,
test::Crashes,
test::RunPassValgrind,
test::Coverage,
test::CoverageMap,
Expand Down
1 change: 1 addition & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ string_enum! {
Assembly => "assembly",
CoverageMap => "coverage-map",
CoverageRun => "coverage-run",
Crashes => "crashes",
}
}

Expand Down
15 changes: 14 additions & 1 deletion src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,16 @@ impl TestProps {
self.incremental = true;
}

if config.mode == Mode::Crashes {
// we don't want to pollute anything with backtrace-files
// also turn off backtraces in order to save some execution
// time on the tests; we only need to know IF it crashes
self.rustc_env = vec![
("RUST_BACKTRACE".to_string(), "0".to_string()),
("RUSTC_ICE".to_string(), "0".to_string()),
];
Comment on lines +588 to +591
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will unset any other possible envs and left only this two, can it be wrong?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why I would have other env vars influencing test outcome 🤔

Copy link
Contributor

@klensy klensy Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it possible to set env vars via "rustc-env" directive, but looks like this will unset it?
https://github.com/rust-lang/rust/pull/122997/files#diff-bf53d26b9ca67283fc825e902a64780b463c153d1b1df0c8e782da0300f08956R452-R457

}

for key in &["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
if let Ok(val) = env::var(key) {
if self.exec_env.iter().find(|&&(ref x, _)| x == key).is_none() {
Expand All @@ -596,7 +606,8 @@ impl TestProps {

fn update_fail_mode(&mut self, ln: &str, config: &Config) {
let check_ui = |mode: &str| {
if config.mode != Mode::Ui {
// Mode::Crashes may need build-fail in order to trigger llvm errors or stack overflows
if config.mode != Mode::Ui && config.mode != Mode::Crashes {
panic!("`{}-fail` header is only supported in UI tests", mode);
}
};
Expand Down Expand Up @@ -625,6 +636,7 @@ impl TestProps {
fn update_pass_mode(&mut self, ln: &str, revision: Option<&str>, config: &Config) {
let check_no_run = |s| match (config.mode, s) {
(Mode::Ui, _) => (),
(Mode::Crashes, _) => (),
(Mode::Codegen, "build-pass") => (),
(Mode::Incremental, _) => {
if revision.is_some() && !self.revisions.iter().all(|r| r.starts_with("cfail")) {
Expand Down Expand Up @@ -757,6 +769,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"ignore-mode-codegen-units",
"ignore-mode-coverage-map",
"ignore-mode-coverage-run",
"ignore-mode-crashes",
"ignore-mode-debuginfo",
"ignore-mode-incremental",
"ignore-mode-js-doc-test",
Expand Down
16 changes: 13 additions & 3 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
"mode",
"which sort of compile tests to run",
"run-pass-valgrind | pretty | debug-info | codegen | rustdoc \
| rustdoc-json | codegen-units | incremental | run-make | ui | js-doc-test | mir-opt | assembly",
| rustdoc-json | codegen-units | incremental | run-make | ui \
| js-doc-test | mir-opt | assembly | crashes",
)
.reqopt(
"",
Expand All @@ -82,7 +83,12 @@ pub fn parse_config(args: Vec<String>) -> Config {
.optopt("", "run", "whether to execute run-* tests", "auto | always | never")
.optflag("", "ignored", "run tests marked as ignored")
.optflag("", "with-debug-assertions", "whether to run tests with `ignore-debug` header")
.optmulti("", "skip", "skip tests matching SUBSTRING. Can be passed multiple times", "SUBSTRING")
.optmulti(
"",
"skip",
"skip tests matching SUBSTRING. Can be passed multiple times",
"SUBSTRING",
)
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
.optflag("", "exact", "filters match exactly")
.optopt(
"",
Expand Down Expand Up @@ -145,7 +151,11 @@ pub fn parse_config(args: Vec<String>) -> Config {
.optflag("", "profiler-support", "is the profiler runtime enabled for this target")
.optflag("h", "help", "show this message")
.reqopt("", "channel", "current Rust channel", "CHANNEL")
.optflag("", "git-hash", "run tests which rely on commit version being compiled into the binaries")
.optflag(
"",
"git-hash",
"run tests which rely on commit version being compiled into the binaries",
)
.optopt("", "edition", "default Rust edition", "EDITION")
.reqopt("", "git-repository", "name of the git repository", "ORG/REPO")
.reqopt("", "nightly-branch", "name of the git branch for nightly", "BRANCH");
Expand Down
29 changes: 26 additions & 3 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::common::{
expected_output_path, UI_EXTENSIONS, UI_FIXED, UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG,
};
use crate::common::{incremental_dir, output_base_dir, output_base_name, output_testname_unique};
use crate::common::{Assembly, Incremental, JsDocTest, MirOpt, RunMake, RustdocJson, Ui};
use crate::common::{Assembly, Crashes, Incremental, JsDocTest, MirOpt, RunMake, RustdocJson, Ui};
use crate::common::{Codegen, CodegenUnits, DebugInfo, Debugger, Rustdoc};
use crate::common::{CompareMode, FailMode, PassMode};
use crate::common::{Config, TestPaths};
Expand Down Expand Up @@ -244,7 +244,7 @@ impl<'test> TestCx<'test> {
/// Code executed for each revision in turn (or, if there are no
/// revisions, exactly once, with revision == None).
fn run_revision(&self) {
if self.props.should_ice && self.config.mode != Incremental {
if self.props.should_ice && self.config.mode != Incremental && self.config.mode != Crashes {
self.fatal("cannot use should-ice in a test that is not cfail");
}
match self.config.mode {
Expand All @@ -263,6 +263,7 @@ impl<'test> TestCx<'test> {
JsDocTest => self.run_js_doc_test(),
CoverageMap => self.run_coverage_map_test(),
CoverageRun => self.run_coverage_run_test(),
Crashes => self.run_crash_test(),
}
}

Expand Down Expand Up @@ -295,6 +296,7 @@ impl<'test> TestCx<'test> {
match self.config.mode {
JsDocTest => true,
Ui => pm.is_some() || self.props.fail_mode > Some(FailMode::Build),
Crashes => false,
Incremental => {
let revision =
self.revision.expect("incremental tests require a list of revisions");
Expand Down Expand Up @@ -359,6 +361,27 @@ impl<'test> TestCx<'test> {
self.check_forbid_output(&output_to_check, &proc_res);
}

fn run_crash_test(&self) {
let pm = self.pass_mode();
let proc_res = self.compile_test(WillExecute::No, self.should_emit_metadata(pm));

if std::env::var("COMPILETEST_VERBOSE_CRASHES").is_ok() {
eprintln!("{}", proc_res.status);
eprintln!("{}", proc_res.stdout);
eprintln!("{}", proc_res.stderr);
eprintln!("{}", proc_res.cmdline);
}

// if a test does not crash, consider it an error
if proc_res.status.success() || matches!(proc_res.status.code(), Some(1 | 0)) {
self.fatal(&format!(
"test no longer crashes/triggers ICE! Please give it a mearningful name, \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't mind me, I'm just passing by

Suggested change
"test no longer crashes/triggers ICE! Please give it a mearningful name, \
"test no longer crashes/triggers ICE! Please give it a meaningful name, \

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think oli already fixed this in a drive-by cleanup somewhere 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, did he? I just saw that in one of this PRs #126316 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember doing that, but it probably hasn't landed yet

add a doc-comment to the start of the test explaining why it exists and \
move it to tests/ui or wherever you see fit."
));
}
}

fn run_rfail_test(&self) {
let pm = self.pass_mode();
let should_run = self.run_if_enabled();
Expand Down Expand Up @@ -2517,7 +2540,7 @@ impl<'test> TestCx<'test> {
rustc.arg("-Cdebug-assertions=no");
}
RunPassValgrind | Pretty | DebugInfo | Rustdoc | RustdocJson | RunMake
| CodegenUnits | JsDocTest => {
| CodegenUnits | JsDocTest | Crashes => {
// do not use JSON output
}
}
Expand Down
1 change: 1 addition & 0 deletions src/tools/opt-dist/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ llvm-config = "{llvm_config}"
"tests/pretty",
"tests/run-pass-valgrind",
"tests/ui",
"tests/crashes",
];
for test_path in env.skipped_tests() {
args.extend(["--skip", test_path]);
Expand Down
17 changes: 17 additions & 0 deletions src/tools/tidy/src/known_bug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//! Tidy check to ensure that tests inside 'tests/crashes' have a '@known-bug' directive.

use crate::walk::*;
use std::path::Path;

pub fn check(filepath: &Path, bad: &mut bool) {
walk(filepath, |path, _is_dir| filter_not_rust(path), &mut |entry, contents| {
let file = entry.path();
if !contents.lines().any(|line| line.starts_with("//@ known-bug: ")) {
tidy_error!(
bad,
"{} crash/ice test does not have a \"//@ known-bug: \" directive",
file.display()
);
}
});
}
1 change: 1 addition & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub mod features;
pub mod fluent_alphabetical;
mod fluent_used;
pub(crate) mod iter_header;
pub mod known_bug;
pub mod mir_opt_tests;
pub mod pal;
pub mod run_make_tests;
Expand Down
2 changes: 2 additions & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ fn main() {
let library_path = root_path.join("library");
let compiler_path = root_path.join("compiler");
let librustdoc_path = src_path.join("librustdoc");
let crashes_path = tests_path.join("crashes");

let args: Vec<String> = env::args().skip(1).collect();
let (cfg_args, pos_args) = match args.iter().position(|arg| arg == "--") {
Expand Down Expand Up @@ -108,6 +109,7 @@ fn main() {
check!(mir_opt_tests, &tests_path, bless);
check!(rustdoc_gui_tests, &tests_path);
check!(rustdoc_css_themes, &librustdoc_path);
check!(known_bug, &crashes_path);

// Checks that only make sense for the compiler.
check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose);
Expand Down
17 changes: 17 additions & 0 deletions tests/crashes/100041.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//@ known-bug: #100041

pub trait WellUnformed {
type RequestNormalize;
}

impl<T: ?Sized> WellUnformed for T {
type RequestNormalize = ();
}

pub fn latent(_: &[<[[()]] as WellUnformed>::RequestNormalize; 0]) {}

pub fn bang() {
latent(&[]);
}

fn main() {}
14 changes: 14 additions & 0 deletions tests/crashes/101036.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ known-bug: #101036
#![feature(generic_const_exprs)]

const fn t<const N: usize>() -> u8 {
N as u8
}

#[repr(u8)]
enum T<const N: u8 = { T::<0>::A as u8 + T::<0>::B as u8 }>
where
[(); N as usize]:
{
A = t::<N>() as u8, B
}
42 changes: 42 additions & 0 deletions tests/crashes/101557.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//@ known-bug: #101557
//@ compile-flags: -Copt-level=0
#![feature(generic_const_exprs)]
use std::marker::PhantomData;

trait Trait {
const CONST: usize;
}

struct A<T: Trait> {
_marker: PhantomData<T>,
}

impl<const N: usize> Trait for [i8; N] {
const CONST: usize = N;
}

impl<const N: usize> From<usize> for A<[i8; N]> {
fn from(_: usize) -> Self {
todo!()
}
}

impl<T: Trait> From<A<[i8; T::CONST]>> for A<T> {
fn from(_: A<[i8; T::CONST]>) -> Self {
todo!()
}
}

fn f<T: Trait>() -> A<T>
where
[(); T::CONST]:,
{
// Usage of `0` is arbitrary
let a = A::<[i8; T::CONST]>::from(0);
A::<T>::from(a)
}

fn main() {
// Usage of `1` is arbitrary
f::<[i8; 1]>();
}
11 changes: 11 additions & 0 deletions tests/crashes/101962.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ known-bug: #101962

#![feature(core_intrinsics)]

pub fn wrapping<T: Copy>(a: T, b: T) {
let _z = core::intrinsics::wrapping_mul(a, b);
}

fn main() {
wrapping(1,2);
}
45 changes: 45 additions & 0 deletions tests/crashes/102047.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
//@ known-bug: #102047

struct Ty1;
struct Ty2;

pub trait Trait<T> {}

pub trait WithAssoc1<'a> {
type Assoc;
}
pub trait WithAssoc2<'a> {
type Assoc;
}

impl<T, U> Trait<for<'a> fn(<T as WithAssoc1<'a>>::Assoc, <U as WithAssoc2<'a>>::Assoc)> for (T, U)
where
T: for<'a> WithAssoc1<'a> + for<'a> WithAssoc2<'a, Assoc = i32>,
U: for<'a> WithAssoc2<'a>,
{
}

impl WithAssoc1<'_> for Ty1 {
type Assoc = ();
}
impl WithAssoc2<'_> for Ty1 {
type Assoc = i32;
}
impl WithAssoc1<'_> for Ty2 {
type Assoc = ();
}
impl WithAssoc2<'_> for Ty2 {
type Assoc = u32;
}

fn foo<T, U, V>()
where
T: for<'a> WithAssoc1<'a>,
U: for<'a> WithAssoc2<'a>,
(T, U): Trait<V>,
{
}

fn main() {
foo::<Ty1, Ty2, _>();
}
14 changes: 14 additions & 0 deletions tests/crashes/102252.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ known-bug: #102252

#![feature(min_specialization, rustc_attrs)]

#[rustc_specialization_trait]
pub trait Trait {}

struct Struct
where
Self: Iterator<Item = <Self as Iterator>::Item>, {}

impl Trait for Struct {}

fn main() {}
Loading
Loading