Produce backtraces for miri internals #280
Conversation
before we built in debug mode for testing, but then installed miri, which builds in release mode. So we built in release mode anyway but tested slowly in debug mode
src/librustc_mir/interpret/error.rs
Outdated
| @@ -192,7 +228,7 @@ impl<'tcx> Error for EvalError<'tcx> { | |||
| TypeNotPrimitive(_) => | |||
| "expected primitive type, got nonprimitive", | |||
| ReallocatedWrongMemoryKind(_, _) => | |||
| "tried to reallocate memory from one kind to another", | |||
| "tried to EvalErrorKindreallocate memory from one kind to another", | |||
There was a problem hiding this comment.
This looks like an accidental edit.
There was a problem hiding this comment.
regex replacements :/ good catch
| #[macro_export] | ||
| macro_rules! err { | ||
| ($($tt:tt)*) => { Err($crate::interpret::EvalErrorKind::$($tt)*.into()) }; | ||
| } |
There was a problem hiding this comment.
What about moving this to error.rs? I think it'd make more sense to have it there.
There was a problem hiding this comment.
macro ordering is important. If we move it to error.rs, we need to ensure that the mod error is the first one in the list, otherwise the modules above don't get the macro.
There was a problem hiding this comment.
Oh, I see. I did not know that. I got used to ordering not mattering in Rust. ;)
tests/compiletest.rs
Outdated
| #[cfg(debug_assertions)] | ||
| const MIRI_PATH: &str = "target/debug/miri"; | ||
| #[cfg(not(debug_assertions))] | ||
| const MIRI_PATH: &str = "target/release/miri"; |
There was a problem hiding this comment.
Can't you use -C debug-assertions to enable debug assertions in a release build? In that case this may use an outdated binary.
There was a problem hiding this comment.
I don't know how else to make this distinction
There was a problem hiding this comment.
I pushed a commit that hopefully should fix this. (#rust IRC channel was very helpful :) )
src/librustc_mir/interpret/error.rs
Outdated
| }); | ||
| }); | ||
| true | ||
| }); |
There was a problem hiding this comment.
The backtrace crate also has this type which just captures the entire backtrace at once:
http://alexcrichton.com/backtrace-rs/backtrace/struct.Backtrace.html
Why did you end up doing the more manual capturing here?
There was a problem hiding this comment.
because... reasons... I'll use that one instead xD
tests/compiletest.rs
Outdated
| #[cfg(not(debug_assertions))] | ||
| const MIRI_PATH: &str = "target/release/miri"; | ||
| fn miri_path() -> String { | ||
| format!("target/{}/miri", env!("PROFILE")) |
There was a problem hiding this comment.
You can do this with const and concat!("target/", env!("PROFILE"), "/miri").
tests/compiletest.rs
Outdated
| fn miri_path() -> String { | ||
| format!("target/{}/miri", env!("PROFILE")) | ||
| const fn miri_path() -> &'static str { | ||
| concat!("target/", env!("PROFILE"), "/miri") |
There was a problem hiding this comment.
You don't need a const fn. You can use a plain const.
|
This has incurred a HUGE performance regression in validation mode. There, we frequently generate an |
|
Probably resolving symbols takes a while; this could involve loading debug info and whatnot. I suppose we will, after all, have to switch to the more low-level API you used first. |
|
Let's get rid of the captures then. They aren't pretty anyway. |
|
I resolve symbols on the low level API, too. Alternatively we can just capture the stack frame pointers, but I think fixing the captures is cleaner |
|
Wait but capturing them was the entire point wasn't it? Also see rust-lang/backtrace-rs#30. We can delay resolving until printing. |
|
To further improve performance, we could check RUST_BACKTRACE and only actually capture and print anything if it is non-empty. |
|
@oli-obk |
|
I am now going back to @oli-obk's original approach, plus checking RUST_BACKTRACE, plus delaying symbol resolution until printing. |
|
@eddyb error-chain is hard to integrate with our system due to the extensibility of the error via the machine. I have not found a way to make error-chain create a generic error type |
|
See #283. |
fixes #251