rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.72k stars 12.64k forks source link

Implementing Drop leads to SIGILL: Illegal instruction in tests but not in normal run #82850

Open bluescreen303 opened 3 years ago

bluescreen303 commented 3 years ago

I tried this code:

pub struct Droppy<T, F: FnMut(&mut T)> {
    inner: T,
    callback: F,
}

impl<T, F: FnMut(&mut T)> Drop for Droppy<T, F> {
    fn drop(&mut self) {
        (self.callback)(&mut self.inner);
    }
}

impl<T, F: FnMut(&mut T)> Droppy<T, F> {
    pub fn new(inner: T, callback: F) -> Self {
        Droppy { inner, callback }
    }
    pub fn force(&mut self) {
        (self.callback)(&mut self.inner);
    }
}

fn main(){
    let mut d = Droppy::new(42, |_| assert_eq!(1,2));
    d.force();
}

#[test]
fn droppy() {
    main()
}

running this with cargo run works fine. I get an assertion failure because 1 isn't 2.

But when I run cargo test

I expected to see this happen:

assertion failure because 1 isn't 2

Instead, this happened:

running 1 test
thread panicked while panicking. aborting.
error: test failed, to rerun pass '--bin broken-case'

Caused by:
  process didn't exit successfully: `.../target/debug/deps/broken_case-77184cc870a0dee1` (signal: 4, SIGILL: illegal instruction)

Meta

rustc --version --verbose:

rustc 1.50.0 (cb75ad5db 2021-02-10)
binary: rustc
commit-hash: cb75ad5db02783e8b0222fee363c5f63f7e2cf5b
commit-date: 2021-02-10
host: x86_64-unknown-linux-gnu
release: 1.50.0
jonas-schievink commented 3 years ago

I'm getting the same result (plus backtrace and 2 panic messages) with cargo run. How can I reproduce this?

ehuss commented 3 years ago

I'm able to reproduce with the steps provided. I think this is an artifact of how output is captured. If you run cargo test -- --nocapture, you should get the equivalent output as cargo run. The test harness doesn't have the opportunity to report the original panic because of the panic-in-panic causes the process to terminate too early.

slightlyoutofphase commented 3 years ago

I'm getting the same result (plus backtrace and 2 panic messages) with cargo run. How can I reproduce this?

Copying and pasting their example directly into the playground and just clicking "test" without changing any settings at all seems to give the sigill error.

rukai commented 3 years ago

This issue is bothering me too.

To be clear the real annoyance here isnt that a sigill occurs, its that we cant see the tracktraces and panic messages when doing cargo test. But we can see the stack traces just fine when doing cargo run.

It occurs on any double panic, so a simpler way to reproduce this is:

struct PanicOnDrop {
}

impl Drop for PanicOnDrop {
    fn drop(&mut self) {
        panic!("dropped");
    }
}

fn main() {
    let _panic_on_drop = PanicOnDrop { };
    panic!("first panic!");
}

#[cfg(test)]
mod tests {
    use main;
    #[test]
    fn foo() {
        main();
    }
}

As mentioned by ehuss cargo test --nocapture works around the issue, but I think it is a bug that we cannot see the stacktraces without knowing to use this magic flag.

ComputerDruid commented 2 years ago

I just hit this issue debugging a flaky CI failure in one of my tests. It's very unfortunate that the backtrace is lost in this case. Could libtest be installing a panic hook that bypasses output capturing if the panic is going to lead to an abort?