8000 Undefined behaviours in ContextStack · Issue #71 · Xudong-Huang/generator-rs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Undefined behaviours in ContextStack #71

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.

Sign up for GitHub

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

Jump to bottom
Open
jasonyu1996 opened this issue Nov 27, 2024 · 3 comments
Open

Undefined behaviours in ContextStack #71

jasonyu1996 opened this issue Nov 27, 2024 · 3 comments

Comments

@jasonyu1996
Copy link

Hi! In ContextStack::current initialises the context stack by pointing root.parent to root itself:

generator-rs/src/rt.rs

Line 173 in 177935c

root.parent = p; // init top to current

root.parent is later dereferenced:

generator-rs/src/rt.rs

Line 185 in 177935c

unsafe { &mut *root.parent }

This seems to be an undefined behaviour as p borrows from root, which is then mutated when root.parent is set.
In ContextStack::push_context, if root.parent and root point to the same location, we get aliasing mutable references, which would also be an undefined behaviour:

generator-rs/src/rt.rs

Line 207 in 177935c

pub fn push_context(&self, ctx: *mut Context) {

A simplified reproduction: https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=26f583e11b335b98cc4847340a99aa97

MIRI reports

   Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.91s
     Running `/playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/playground`
error: Undefined Behavior: trying to retag from <1816> for SharedReadOnly permission at alloc923[0x0], but that tag does not exist in the borrow stack for this location
 --> src/main.rs:8:22
  |
8 |     let _ = unsafe { &*node.n };
  |                      ^^^^^^^^
  |                      |
  |                      trying to retag from <1816> for SharedReadOnly permission at alloc923[0x0], but that tag does not exist in the borrow stack for this location
  |                      this error occurs as part of retag at alloc923[0x0..0x8]
  |
  = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows 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
help: <1816> was created by a SharedReadWrite retag at offsets [0x0..0x8]
 --> src/main.rs:7:14
  |
7 |     node.n = &mut *node as *mut Node;
  |              ^^^^^^^^^^
help: <1816> was later invalidated at offsets [0x0..0x8] by a write access
 --> src/main.rs:7:5
  |
7 |     node.n = &mut *node as *mut Node;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = note: BACKTRACE (of the first span):
  = note: inside `main` at src/main.rs:8:22: 8:30

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error
@Xudong-Huang
Copy link
Owner
Xudong-Huang commented Nov 28, 2024 •
edited
Loading

The unsafe code ligic should be correct. I think this is a miri false alarm for this lib. Also the unsafe code is internally used and user can't get mulitiple mutable refs.

@jasonyu1996
Copy link
Author

The unsafe code ligic should be correct. I think this is a miri false alarm for this lib. Also the unsafe code is internally used and user can't get mulitiple mutable refs.

The logic is correct indeed but still it's an undefined behaviour which happens to produce the desired behaviour under the current compiler implementation. It's not guaranteed that the compiler won't introduce optimisations that break the code. So it's a ticking bomb.

An example of this type of undefined behaviour already causing miscompilation under the current compiler: https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=3c1c7f860b4a042eb88025c75f83837a. Release and debug produce different outcomes.

Nomicon includes a section about this type of issue: https://doc.rust-lang.org/nomicon/aliasing.html

@jasonyu1996
Copy link
Author

I can try preparing a fix to this in a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jasonyu1996 @Xudong-Huang
0