book icon indicating copy to clipboard operation
book copied to clipboard

Listing 19-03 is potentially UB under Stacked Borrows

Open nico-abram opened this issue 3 years ago • 4 comments

  • [X] I have checked the latest main branch to see if this has already been fixed
  • [X] I have searched existing issues and pull requests for duplicates

URL to the section(s) of the book with this problem: https://doc.rust-lang.org/book/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer

Description of the problem: Running the listing

fn main() {
    let mut num = 5;

    let r1 = &num as *const i32;
    let r2 = &mut num as *mut i32;

    unsafe {
        println!("r1 is: {}", *r1);
        println!("r2 is: {}", *r2);
    }
}

Under MIRI with MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-check-number-validity -Zmiri-tag-raw-pointers" results in the following:

error: Undefined Behavior: trying to reborrow for SharedReadOnly at alloc766, but parent tag <1658> does not have an appropriate item in the borrow stack
 --> src\main.rs:9:31
  |
9 |         println!("r1 is: {}", *r1);
  |                               ^^^ trying to reborrow for SharedReadOnly at alloc766, but parent tag <1658> does not have an appropriate item in the borrow stack
  |
  = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
  = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

  = note: inside `main` at src\main.rs:9:31
  = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:227:5
  = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys_common\backtrace.rs:123:18
  = note: inside closure at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:145:18
  = 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 D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:259:13
  = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:406:40
  = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:370:19
  = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:133:14
  = note: inside closure at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:128:48
  = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:406:40
  = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:370:19
  = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:133:14
  = note: inside `std::rt::lang_start_internal` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:128:20
  = note: inside `std::rt::lang_start::<()>` at D:\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:144:17
  = note: this error originates in the macro `$crate::format_args_nl` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

Suggested fix: I'm not sure. The following fixes that specific problem:


fn main() {
    let mut num = 5;

    let r2 = &mut num as *mut i32;
    let r1 = unsafe { &*r2 as *const i32 };

    unsafe {
        println!("r1 is: {}", *r1);
        println!("r2 is: {}", *r2);
    }
}

But I'm not sure if it preserves the original meaning

Possibly related: https://github.com/rust-lang/unsafe-code-guidelines/issues/133

nico-abram avatar Jan 21 '22 03:01 nico-abram

You don’t need any flags anymore. I believe since tag-raw-pointers is now on by default AFAIR.

steffahn avatar Aug 11 '22 19:08 steffahn

Hi, I just found this issue.

I think a better fix would be to use (and explain) std::ptr::addr_of[_mut]!. See this playground for an example. Note that the original issue also reproduce using Tools > Miri on the playground.

The problem here is that the temporary &mut invalidates the reference used to create r1, and as such also invalidates the r1 pointer. Deriving r2 from r1 or using std::ptr::addr_of_mut avoid this problem.

I also think that this is a great opportunity to warn and/or illustrate what can go wrong with unsafe, and maybe introduce miri. IMO, the best fix would be to leave the listing as is, explain and illustrate with miri why it's wrong, then provide a correct version.

kellda avatar Aug 27 '23 08:08 kellda