What is this weird behavior when using unsafe and pointers in constructor?

Question

I'm running into a this weird issue where the program succeeds or fails based on if a put my unsafe block in the constructor or not. Not sure if it is a bug or I am just getting lucky in the latter or I'm just missing something important.

Failing case:

fn main() {
    let mut xc = Bob::default();
    let mut xp = Bob::default();
    let mut x = Bob::new(Some(&mut xc), Some(&mut xp));

    unsafe {
        (&mut *x.child.unwrap()).string = String::from("child");
        (&mut *xp.child.unwrap()).string = String::from("middle");
    }
    println!("{:?}", xc);
    println!("{:?}", x);
}

#[derive(Debug)]
struct Bob {
    child: Option<*mut Bob>,
    parent: Option<*mut Bob>,
    string: String,
}
impl Bob {
    fn new(child: Option<*mut Bob>, parent: Option<*mut Bob>) -> Bob {
        let mut this = Bob {
            child,
            parent,
            string: String::from("Hello")
        };
        if child.is_some(){
            unsafe { child.unwrap().as_mut().unwrap().parent = Some(&mut this);}
        }
        if parent.is_some(){
            unsafe { parent.unwrap().as_mut().unwrap().child = Some(&mut this);}
        }
        this
    }
}

impl Default for Bob {
    fn default() -> Bob {
        Bob {
            child: None,
            parent: None,
            string: String::from("Hello")
        }
    }
}

Bob { child: None, parent: Some(0x37b76ff7f8), string: "child" }
Bob { child: Some(0x37b76ff8b8), parent: Some(0x37b76ff8f0), string: "thread 'main' panicked at 'failed printing to stdout: Windows stdio in console mode does not support writing non-UTF-8 byte sequences', library\std\src\io\stdio.rs:1019:9
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library\std\src\panicking.rs:593
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library\core\src\panicking.rs:67
   2: std::io::stdio::print_to
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library\std\src\io\stdio.rs:1019
   3: std::io::stdio::_print
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library\std\src\io\stdio.rs:1096
   4: rust_allocators::main
             at .\src\main.rs:14
   5: core::ops::function::FnOnce::call_once<void (*)(),tuple$<> >
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225\library\core\src\ops\function.rs:250
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: process didn't exit successfully: `target\debug\rust-allocators.exe` (exit code: 0xc0000374, STATUS_HEAP_CORRUPTION)

Process finished with exit code -1073740940 (0xC0000374)

Working alternative:

fn main() {
    let mut xc = Bob::default();
    let mut xp = Bob::default();
    let mut x = Bob::new(Some(&mut xc), Some(&mut xp));
    xp.child = Some(&mut x);
    // unsafe { Some(&mut xp as *mut Bob).unwrap().as_mut().unwrap().child = Some(&mut x);} // ALSO WORKS


    unsafe {
        (&mut *x.child.unwrap()).string = String::from("child");
        (&mut *xp.child.unwrap()).string = String::from("middle");
    }

    println!("{:?}", xc);
    println!("{:?}", x);
}

#[derive(Debug)]
struct Bob {
    child: Option<*mut Bob>,
    parent: Option<*mut Bob>,
    string: String,
}
impl Bob {
    fn new(child: Option<*mut Bob>, parent: Option<*mut Bob>) -> Bob {
        let mut this = Bob {
            child,
            parent,
            string: String::from("Hello")
        };
        if child.is_some(){
            unsafe { child.unwrap().as_mut().unwrap().parent = Some(&mut this);}
        }
        //if parent.is_some(){
        //    unsafe { parent.unwrap().as_mut().unwrap().child = Some(&mut this);}
        //}
        this
    }
}

impl Default for Bob {
    fn default() -> Bob {
        Bob {
            child: None,
            parent: None,
            string: String::from("Hello")
        }
    }
}

Bob { child: None, parent: Some(0x990ecff628), string: "child" }
Bob { child: Some(0x990ecff708), parent: Some(0x990ecff740), string: "middle" }

I expected both cases to work.

Yes, you indeed are missing something important - the very purpose of Rust! Not only you're absolutely ignoring error handling, but also just returning a dangling pointer to the stack. You need to get to know both Rust and systems programming in general a little bit better before experimenting this way. In this case, as @cafce25 pointed out, you can use heap, but it would a mere crutch in terms of Rust. This entire logic needs to be rewritten. Without raw pointers when possible. — Miiao
– Miiao, Commented Sep 10, 2023 at 13:53
@drewtato: I would note that Miri checks are not the end-all be-all, if anything, the existence of both the (original) Stacked Borrows model and the (new) Tree Borrows model shows that the exact semantics are still up for debate. As such, I would not advise treating Miri as an absolute. If it passes Miri, it's a good sign, if it doesn't, it's a bad sign, but it could pass Miri and still crash and it could not pass Miri and still not result in UB. — Matthieu M.
– Matthieu M., Commented Sep 11, 2023 at 12:43
As mentioned by drewtato, there's a fantastic tool called Miri which is very helpful -- though not absolute -- and points out a lot of likely flaws in unsafe code. You can install it with rustup +nightly component add miri, and then run cargo miri test to run your codes unit-tests under miri. — Matthieu M.
– Matthieu M., Commented Sep 11, 2023 at 12:45

cafce25 · Accepted Answer · 2023-09-10 06:10:47Z

3

The pointer to this taken here

unsafe { child.unwrap().as_mut().unwrap().parent = Some(&mut this);}

is pointing to a function local variable, it becomes dangling as soon as you return from that function! Dereferencing it afterwards is undefined behavior. Returning a value from a function moves it, so it's address might change.

And while miri puts it in other words, it does say the same thing:

error: Undefined Behavior: dereferencing pointer failed: alloc993 has been freed, so this pointer is dangling
  --> src/main.rs:8:9
   |
8  |         (&mut *xp.child.unwrap()).string = String::from("middle");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ dereferencing pointer failed: alloc993 has been freed, so this pointer is dangling
   |
   = 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
help: alloc993 was allocated here:
  --> src/main.rs:22:13
   |
22 |         let mut this = Bob {
   |             ^^^^^^^^
help: alloc993 was deallocated here:
  --> src/main.rs:34:5
   |
34 |     }
   |     ^
   = note: BACKTRACE (of the first span):
   = note: inside `main` at src/main.rs:8:9: 8:34

The second version works because you create the pointer and don't subsequently move the values so you don't access any dangling pointers, the parent pointer of xc is still dangling, but you don't access it so that's fine.

There is a way to pull this off, we have to ensure

the actual value doesn't move in memory when we return — we can Box::pin it
it can't be moved afterwards — add a PhantomPinned so it doesn't implement Unpin

So change to this:

use core::marker::PhantomPinned;
use core::pin::Pin;

#[derive(Default, Debug)]
struct Bob {
    child: Option<*mut Bob>,
    parent: Option<*mut Bob>,
    string: String,
    _marker: PhantomPinned,
}

impl Bob {
    fn new(child: Option<*mut Bob>, parent: Option<*mut Bob>) -> Pin<Box<Bob>> {
        let mut this = Box::pin(Bob {
            child,
            parent,
            string: String::from("Hello"),
            _marker: PhantomPinned,
        });

        if child.is_some() {
            let this = this.as_mut();
            unsafe {
                child.unwrap().as_mut().unwrap().parent = Some(this.get_unchecked_mut());
            }
        }
        if parent.is_some() {
            let this = this.as_mut();
            unsafe {
                parent.unwrap().as_mut().unwrap().child = Some(this.get_unchecked_mut());
            }
        }
        this
    }
}

edited Sep 10, 2023 at 6:10

answered Sep 10, 2023 at 5:07

cafce25

29.9k5 gold badges49 silver badges68 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

mcmah309 Over a year ago

Amazing explanation and solution, thanks a lot! I didn't realize that "moves" may or may not (this is not a language constraint) result in a stack copy and de-initialization of old stack data, which may cause dangling references. Sounds like you should always use pinning if using raw pointers in these cases. Some additional references: users.rust-lang.org/t/why-do-values-need-to-be-moved/55971/27 reddit.com/r/rust/comments/qpaifu/…

jthulhu Over a year ago

@mcmah309 the problem is not about pinning, it's about allocating on the stack or allocating on the heap. If a variable is allocated on the stack, then any pointer to it will become invalid as soon as the frame it lives in is destroyed (in Rust as in any other language, that's just how the stack works), which happens when the function returns.

mcmah309 Over a year ago

@jthulhu learning here, so bear with me. But I don't believe this is about stack vs heap, but rather how ownership is transfered behind the scenes. Your answer suggests to me if I created a variable inside a function and returned it that would be an issue. If I don't use pinning and just use a Box the code fails.

Matthieu M. Over a year ago

@jthulhu: I wouldn't frame it on stack vs heap. While you are correct that returning the value moves it from the current frame to the parent frame (well, may move it), you can put a value on the heap and still move it...

Collectives™ on Stack Overflow

What is this weird behavior when using unsafe and pointers in constructor?

1 Answer 1

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related