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

Improve error message: Unsupported operation vs. undefined behavior #417

Closed
RalfJung opened this issue Jul 27, 2018 · 9 comments · Fixed by #1250
Closed

Improve error message: Unsupported operation vs. undefined behavior #417

RalfJung opened this issue Jul 27, 2018 · 9 comments · Fixed by #1250
Labels
A-interpreter Area: affects the core interpreter C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@RalfJung
Copy link
Member

Someone using miri and getting an error should learn immediately whether this is a problem in their code (UB, e.g. out-of-bounds memory access) or a problem in miri (unsupported operation, e.g. printing a pointer).

Any ideas how we could make that as clear as possible?

@RalfJung
Copy link
Member Author

We could probably also improve the rendering of the stack traces.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 30, 2018

A third category is panics. Maybe our error enum should just have these three variants with sub-enums for the concrete error?

@RalfJung
Copy link
Member Author

There's also resource exhaustion (out of memory, out of time). But I'd put them together with panics, both are just "normal execution behavior".

@oli-obk
Copy link
Contributor

oli-obk commented Jul 30, 2018

Separating them from panics has advantages for rustc figuring out what to report which way. Reporting const eval resource exhaustion as "this will panic at runtime" seems bad

@RalfJung
Copy link
Member Author

Well sure the error message would still look into the inner variant.

@RalfJung
Copy link
Member Author

Related: rust-lang/const-eval#4

@RalfJung RalfJung added the C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement label Nov 17, 2018
@RalfJung RalfJung added the A-interpreter Area: affects the core interpreter label Mar 8, 2019
@lambda
Copy link

lambda commented Mar 28, 2019

An example of this issue from Zulip chat; minified example from @shepmaster:

Trying to make a related example:

fn main() {
    let a: u8 = 42;
    let a_ref = &a;
    let a_ptr = a_ref as *const u8;
    let a_aligned = a_ptr as usize % 16;
    unsafe { a_ptr.sub(a_aligned) };
}
error[E0080]: constant evaluation error: a raw memory access tried to access part of a pointer value as raw bytes
 --> src/main.rs:5:21
  |
5 |     let a_aligned = a_ptr as usize % 16;
  |                     ^^^^^^^^^^^^^^^^^^^ a raw memory access tried to access part of a pointer value as raw bytes
  |
  = note: inside call to `main` at /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:64:34
  = note: inside call to closure at /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:53
  = note: inside call to closure at /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:293:40
  = note: inside call to `std::panicking::try::do_call::<[closure@DefId(1/1:1830 ~ std[82ff]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:289:5
  = note: inside call to `std::panicking::try::<i32, [closure@DefId(1/1:1830 ~ std[82ff]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe]>` at /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panic.rs:388:9
  = note: inside call to `std::panic::catch_unwind::<[closure@DefId(1/1:1830 ~ std[82ff]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:25
  = note: inside call to `std::rt::lang_start_internal` at /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:64:5
  = note: inside call to `std::rt::lang_start::<()>`

And suggestion of the better supported alternative from @oli-obk:

what you usually want to do is to use https://doc.rust-lang.org/std/primitive.slice.html#method.align_to since that uses intrinsics internally to support all the use cases of miri. The low level operation for pointers is https://doc.rust-lang.org/std/primitive.pointer.html#method.align_offset

@RalfJung
Copy link
Member Author

@shepmaster points out that it would also be a good idea for Miri to indicate how "certain" some case of UB is -- like, dereferencing a dangling pointer is definitely UB, but a Stacked Borrows violation is more experimental.

@RalfJung
Copy link
Member Author

Blocked on rust-lang/const-eval#4: once the CTFE error enum contains the information we need, we can think about how to best present that to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants