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

Miri reports UB in the core lib #2839

Closed
r-bk opened this issue Apr 12, 2023 · 11 comments · Fixed by rust-lang/rust#110233
Closed

Miri reports UB in the core lib #2839

r-bk opened this issue Apr 12, 2023 · 11 comments · Fixed by rust-lang/rust#110233

Comments

@r-bk
Copy link

r-bk commented Apr 12, 2023

Started getting this error with recent miri builds:

test arrayvec::traits::index::testing::test_range_index_mut_panics - should panic ... error: Undefined Behavior: unwinding past a stack frame that does not allow unwinding
  --> /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:70:9
   |
70 |         const_eval_select((index, len), slice_end_index_len_fail_ct, slice_end_index_len_fail_rt)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `core::slice::index::slice_end_index_len_fail` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:70:9: 70:98
   = note: inside `<std::ops::Range<usize> as std::slice::SliceIndex<[u64]>>::index_mut` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:422:13: 422:60
   = note: inside `core::slice::index::<impl std::ops::IndexMut<std::ops::Range<usize>> for [u64]>::index_mut` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:31:9: **31:30**

Is there a real UB in the core lib?

@RalfJung
Copy link
Member

RalfJung commented Apr 12, 2023

What is the code to reproduce the problem? If your code calls unsafe core functions the wrong way then it would not be surprising to see UB inside core.

@RalfJung
Copy link
Member

@saethlin seems related to the failures we are seeing in miri-test-libstd.

@r-bk
Copy link
Author

r-bk commented Apr 12, 2023

@RalfJung Here is a simplified example which reproduces the issue:

#[test]
#[should_panic]
fn test_slice_range_index_mut_panics() {
    let mut arr: [u64; 3] = [1, 2, 3];
    let slc: &mut [u64] = &mut arr;
    let _ = &mut slc[1..7];
}

This test case causes the following miri error:

test arrayvec::traits::index::testing::test_slice_range_index_mut_panics - should panic ... error: Undefined Behavior: unwinding past a stack frame that does not allow unwinding
  --> /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:70:9
   |
70 |         const_eval_select((index, len), slice_end_index_len_fail_ct, slice_end_index_len_fail_rt)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `core::slice::index::slice_end_index_len_fail` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:70:9: 70:98
   = note: inside `<std::ops::Range<usize> as std::slice::SliceIndex<[u64]>>::index_mut` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:422:13: 422:60
   = note: inside `core::slice::index::<impl std::ops::IndexMut<std::ops::Range<usize>> for [u64]>::index_mut` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:31:9: 31:30
note: inside `arrayvec::traits::index::testing::test_slice_range_index_mut_panics`
  --> src/arrayvec/traits/index.rs:93:22
   |
93 |         let _ = &mut slc[1..7];
   |                      ^^^^^^^^^
note: inside closure
  --> src/arrayvec/traits/index.rs:90:44
   |
88 |     #[test]
   |     ------- in this procedural macro expansion
89 |     #[should_panic]
90 |     fn test_slice_range_index_mut_panics() {
   |                                            ^
   = note: inside `<[closure@src/arrayvec/traits/index.rs:90:5: 94:6] as std::ops::FnOnce<()>>::call_once - shim` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
   = note: inside `<fn() -> std::result::Result<(), std::string::String> as std::ops::FnOnce<()>>::call_once - shim(fn() -> std::result::Result<(), std::string::String>)` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
   = note: inside `test::__rust_begin_short_backtrace::<std::result::Result<(), std::string::String>, fn() -> std::result::Result<(), std::string::String>>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:656:18: 656:21
   = note: inside closure at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:647:30: 647:61
   = note: inside `<[closure@test::run_test::{closure#1}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
   = note: inside `<std::boxed::Box<dyn std::ops::FnOnce() -> std::result::Result<(), std::string::String> + std::marker::Send> as std::ops::FnOnce<()>>::call_once` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1976:9: 1976:52
   = note: inside `<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() -> std::result::Result<(), std::string::String> + std::marker::Send>> as std::ops::FnOnce<()>>::call_once` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:271:9: 271:19
   = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() -> std::result::Result<(), std::string::String> + std::marker::Send>>, std::result::Result<(), std::string::String>>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:485:40: 485:43
   = note: inside `std::panicking::r#try::<std::result::Result<(), std::string::String>, std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() -> std::result::Result<(), std::string::String> + std::marker::Send>>>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:449:19: 449:81
   = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() -> std::result::Result<(), std::string::String> + std::marker::Send>>, std::result::Result<(), std::string::String>>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:140:14: 140:33
   = note: inside `test::run_test_in_process` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:679:27: 679:65
   = note: inside closure at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:573:39: 581:14
   = note: inside closure at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:600:37: 600:79
   = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@test::run_test::run_test_inner::{closure#1}], ()>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:134:18: 134:21
   = note: inside closure at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:525:17: 525:78
   = note: inside `<std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked_<'_, '_, [closure@test::run_test::run_test_inner::{closure#1}], ()>::{closure#1}::{closure#0}]> as std::ops::FnOnce<()>>::call_once` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:271:9: 271:19
   = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked_<'_, '_, [closure@test::run_test::run_test_inner::{closure#1}], ()>::{closure#1}::{closure#0}]>, ()>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:485:40: 485:43
   = note: inside `std::panicking::r#try::<(), std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked_<'_, '_, [closure@test::run_test::run_test_inner::{closure#1}], ()>::{closure#1}::{closure#0}]>>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:449:19: 449:81
   = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked_<'_, '_, [closure@test::run_test::run_test_inner::{closure#1}], ()>::{closure#1}::{closure#0}]>, ()>` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:140:14: 140:33
   = note: inside closure at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:524:30: 526:16
   = note: inside `<[closure@std::thread::Builder::spawn_unchecked_<'_, '_, [closure@test::run_test::run_test_inner::{closure#1}], ()>::{closure#1}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
   = note: inside `<std::boxed::Box<dyn std::ops::FnOnce()> as std::ops::FnOnce<()>>::call_once` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1976:9: 1976:52
   = note: inside `<std::boxed::Box<std::boxed::Box<dyn std::ops::FnOnce()>> as std::ops::FnOnce<()>>::call_once` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1976:9: 1976:52
   = note: inside `std::sys::unix::thread::Thread::new::thread_start` at /home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/unix/thread.rs:108:17: 108:64
   = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error
Error: aborting due to previous error
error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/home/runner/.rustup/toolchains/nightly-2023-04-12-x86_64-unknown-linux-gnu/bin/cargo-miri runner /home/runner/work/cds/cds/target/miri/x86_64-unknown-linux-gnu/debug/deps/cds-110baade9b65c8ee` (exit status: 1)
Error: The process '/home/runner/.cargo/bin/cargo' failed with exit code 1

@saethlin
Copy link
Member

Interesting!

Here's as far as I can minimize this:

#![feature(core_intrinsics)]

fn main() {
    unsafe {
        core::intrinsics::const_eval_select((), ow_ct, ow_ct)
    }
}

const fn ow_ct() -> ! {
    panic!();
}

And the bug is that we generate this MIR for main, with unwind unreachable on the call:

fn main() -> () {
    let mut _0: ();                      // return place in scope 0 at src/main.rs:3:11: 3:11
    let mut _1: !;                       // in scope 0 at src/main.rs:5:9: 5:62
    scope 1 {
    }

    bb0: {
        _1 = ow_ct() -> unwind unreachable; // scope 1 at src/main.rs:5:9: 5:62
                                         // mir::Constant
                                         // + span: src/main.rs:5:56: 5:61
                                         // + literal: Const { ty: fn() -> ! {ow_ct}, val: Value(<ZST>) }
    }
}

@RalfJung
Copy link
Member

Packing the same code in a main function works fine, so it's got something to do with the test harness. We've seen should_panic tests with similar errors in miri-test-libstd so this is likely the same underlying issue.

@RalfJung
Copy link
Member

And the bug is that we generate this MIR for main, with unwind unreachable on the call:

Probably related to rust-lang/rust#102906 then, Cc @nbdd0121

@saethlin
Copy link
Member

The unwind unreachale is inserted by AbortUnwindingCalls:

-// MIR for `main` before AbortUnwindingCalls
+// MIR for `main` after AbortUnwindingCalls
 
 fn main() -> () {
     let mut _0: ();                      // return place in scope 0 at src/main.rs:3:11: 3:11
@@ -11,7 +11,7 @@ fn main() -> () {
         StorageLive(_1);                 // scope 1 at src/main.rs:5:9: 5:62
         StorageLive(_2);                 // scope 1 at src/main.rs:5:45: 5:47
         _2 = ();                         // scope 1 at src/main.rs:5:45: 5:47
-        _1 = const_eval_select::<(), fn() -> ! {ow_ct}, fn() -> ! {ow_ct}, !>(move _2, ow_ct, ow_ct); // scope 1 at src/main.rs:5:9: 5:62
+        _1 = const_eval_select::<(), fn() -> ! {ow_ct}, fn() -> ! {ow_ct}, !>(move _2, ow_ct, ow_ct) -> unwind unreachable; // scope 1 at src/main.rs:5:9: 5:62
                                          // mir::Constant
                                          // + span: src/main.rs:5:9: 5:44
                                          // + literal: Const { ty: unsafe extern "rust-intrinsic" fn((), fn() -> ! {ow_ct}, fn() -> ! {ow_ct}) -> ! {const_eval_select::<(), fn() -> ! {ow_ct}, fn() -> ! {ow_ct}, !>}, val: Value(<ZST>) }

@nbdd0121
Copy link
Contributor

This seems to be a longstanding bug that's exposed by rust-lang/rust#102906. fn_can_unwind assumes all rust-intrinsic calls to be nounwind.

@nbdd0121
Copy link
Contributor

Related: rust-lang/rust#104451

@saethlin
Copy link
Member

saethlin commented Apr 12, 2023

So is that issue now definitely a soundness issue? It seems very wrong to suggest to a codegen backend that unwinding is unreachable when it is reachable.

@RalfJung
Copy link
Member

Yeah sounds like a soundness issue to me, affecting not just Miri but also regular rustc.

thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jun 1, 2023
Make rust-intrinsic ABI unwindable

Fix #104451, fix rust-lang/miri#2839

r? `@RalfJung`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants