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 safe code due to address space exhaustion (?) #2769

Closed
saethlin opened this issue Jan 26, 2023 · 3 comments · Fixed by rust-lang/rust#107756
Closed

Miri reports UB in safe code due to address space exhaustion (?) #2769

saethlin opened this issue Jan 26, 2023 · 3 comments · Fixed by rust-lang/rust#107756

Comments

@saethlin
Copy link
Member

saethlin commented Jan 26, 2023

On a 32-bit target, Miri says this program encounters UB:

fn main() {
    for _ in 0..4 {
        let a = [0u8; 1024 * 1024 * 1024];
        drop(&a[..]);
    }
}
error: Undefined Behavior: overflowing in-bounds pointer arithmetic
 --> src/main.rs:4:15
  |
4 |         drop(&a[..]);
  |               ^ overflowing in-bounds pointer arithmetic
  |
  = 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 `main` at src/main.rs:4:15: 4:16

The program above is a bit slow to execute but it gets the job done without any unsafe. Using Vec::with_capacity is faster.

I feel like this shouldn't be possible? Or at least we shouldn't report UB?


The Vec version is this:

fn main() {
    for _ in 0..4 {
        let mut a: Vec<u8> = Vec::with_capacity(1024 * 1024 * 1024);
        drop(a.spare_capacity_mut().as_ptr());
    }
}
error: Undefined Behavior: overflowing in-bounds pointer arithmetic
 --> src/main.rs:4:14
  |
4 |         drop(a.spare_capacity_mut().as_ptr());
  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflowing in-bounds pointer arithmetic
  |
  = 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 `main` at src/main.rs:4:14: 4:45

In this case I think it's much more clear that the last allocation should fail

@RalfJung
Copy link
Member

Oh fun, I didn't think it was actually possible to exhaust the address space in Miri...

However the corresponding arithmetic in alloc_base_addr should lead to ICEs, not nice UB errors. Can you get us a more precise error, with an unpruned backtrace?

@saethlin
Copy link
Member Author

The base address of the allocation is fine. The problem is that there isn't enough address space after the base address for the size of the allocation. I modified the demo to make it clearer I think:

fn main() {
    for _ in 0..4 {
        let a = [0u8; 1024 * 1024 * 1024];
        println!("{:p}", &a[0] as *const u8);
        drop(&a[..]);
    }
}
$ MIRIFLAGS=-Zmiri-backtrace=full cargo +nightly miri run --target=i686-unknown-linux-gnu
Preparing a sysroot for Miri (target: i686-unknown-linux-gnu)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/i686-unknown-linux-gnu/debug/scratch`
0x25e2e
0x4002bfb6
0x80031038
0xc0035fc6
error: Undefined Behavior: overflowing in-bounds pointer arithmetic
 --> src/main.rs:5:15
  |
5 |         drop(&a[..]);
  |               ^ overflowing in-bounds pointer arithmetic
  |
  = 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 `main` at src/main.rs:5:15: 5:16
  = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
  = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:121:18: 121:21
  = note: inside closure at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:166:18: 166:82
  = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:287:13: 287:31
  = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:483:40: 483:43
  = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:447:19: 447:81
  = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:140:14: 140:33
  = note: inside closure at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:148:48: 148:73
  = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:483:40: 483:43
  = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:447:19: 447:81
  = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:140:14: 140:33
  = note: inside `std::rt::lang_start_internal` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:148:20: 148:98
  = note: inside `std::rt::lang_start::<()>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:165:17: 170:6

error: aborting due to previous error

@RalfJung
Copy link
Member

This code computes the at-the-end address of the allocation:

global_state.next_base_addr = base_addr.checked_add(max(size.bytes(), 1)).unwrap();

However this seems to be doing a u64 overflow check instead of a target-usize overflow check.

RalfJung pushed a commit to RalfJung/miri that referenced this issue Feb 9, 2023
miri: fix ICE when running out of address space

Fixes rust-lang#2769
r? `@oli-obk`

I didn't add a test since that requires oli-obk/ui_test#38 (host must be 64bit and target 32bit). Also the test takes ~30s, so I am not sure if we want to have it in the test suite?
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.

2 participants