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

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

Open
bluescreen303 opened this issue Mar 6, 2021 · 5 comments
Labels
A-libtest Area: `#[test]` / the `test` library C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bluescreen303
Copy link

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
@bluescreen303 bluescreen303 added the C-bug Category: This is a bug. label Mar 6, 2021
@jonas-schievink
Copy link
Contributor

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

@ehuss
Copy link
Contributor

ehuss commented Mar 6, 2021

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
Copy link
Contributor

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
Copy link
Contributor

rukai commented Aug 12, 2021

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
Copy link
Contributor

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?

@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-libtest Area: `#[test]` / the `test` library and removed needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. labels Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: `#[test]` / the `test` library C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: No status
Development

No branches or pull requests

8 participants