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

Display in test fails with SIGILL (illegal instruction) #69558

Closed
SnowyCoder opened this issue Feb 28, 2020 · 2 comments · Fixed by #69959
Closed

Display in test fails with SIGILL (illegal instruction) #69558

SnowyCoder opened this issue Feb 28, 2020 · 2 comments · Fixed by #69959
Labels
A-libtest Area: #[test] related C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@SnowyCoder
Copy link

I tried this code (using cargo test):

use std::fmt::{Display, Formatter};
use std::fmt;
 
pub struct A(Vec<u32>);
 
impl Display for A {
    fn fmt(&self, _f: &mut Formatter<'_>) -> fmt::Result {
       self.0[0];
       Ok(())
   }
}
 
#[test]
fn main() {
   let a = A(vec![]);
   eprintln!("{}", a);
}

I expected to see this happen:
The thread should've panicked with 'Index out of bounds: the len is 0 but the index is 0'

Instead, this happened:
Thread crashes with SIGILL (illegal instruction)

Meta

This bug works in stable, beta and nightly.
It also does the same in rust playground.
Here is my cpu info (I figured that it might be useful given the nature of the bug).
The bug doesn't appear if run from main, nor if run directly (without passing from the Display impl).

rustc --version --verbose:

rustc 1.41.1 (f3e1a954d 2020-02-24)
binary: rustc
commit-hash: f3e1a954d2ead4e2fc197c7da7d71e6c61bad196
commit-date: 2020-02-24
host: x86_64-unknown-linux-gnu
release: 1.41.1
LLVM version: 9.0
Backtrace

running 1 test
thread panicked while processing panic. aborting.
error: test failed, to rerun pass '--lib'

Caused by:
process didn't exit successfully: /path/to/proj/target/debug/deps/proj-296b7cb53c6210d6 (signal: 4, SIGILL: illegal instruction)

@SnowyCoder SnowyCoder added the C-bug Category: This is a bug. label Feb 28, 2020
@jonas-schievink
Copy link
Contributor

This seems to be due to the eprintln!. Using format! instead works fine.

@jonas-schievink jonas-schievink added A-libtest Area: #[test] related T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 28, 2020
@Mark-Simulacrum Mark-Simulacrum added the I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. label Feb 28, 2020
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Feb 28, 2020

I can reproduce. This seems to happen if LOCAL_STDERR is initialized (e.g., with set_panic) which leads us to this code. Note that the RefCell there is borrowed so that we can write to it, but then when we go to panic we try to temporarily take the value out. This borrows the RefCell mutably, again, leading to a panic. Since this happens within an existing panic, we abort the process.

Smaller example:

#![feature(set_stdio, print_internals)]

use std::fmt::{Display, Formatter};
use std::fmt;
use std::io::set_panic;

pub struct A;

impl Display for A {
    fn fmt(&self, _f: &mut Formatter<'_>) -> fmt::Result {
        panic!();
   }
}

fn main() {
    set_panic(Some(Box::new(Vec::new())));
    std::io::_eprint(format_args!("{}", A));
}

alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 13, 2020
This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 14, 2020
…Mark-Simulacrum

std: Don't abort process when printing panics in tests

This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 17, 2020
This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 17, 2020
…Mark-Simulacrum

std: Don't abort process when printing panics in tests

This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
Centril added a commit to Centril/rust that referenced this issue Mar 19, 2020
…Mark-Simulacrum

std: Don't abort process when printing panics in tests

This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
Centril added a commit to Centril/rust that referenced this issue Mar 19, 2020
…Mark-Simulacrum

std: Don't abort process when printing panics in tests

This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
@bors bors closed this as completed in d5b6a20 Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: #[test] related C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants