-
Notifications
You must be signed in to change notification settings - Fork 40
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
Undefined behaviours in ContextStack #71
Comments
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 |
I can try preparing a fix to this in a PR. |
Hi! In
ContextStack::current
initialises the context stack by pointingroot.parent
toroot
itself:generator-rs/src/rt.rs
Line 173 in 177935c
root.parent
is later dereferenced:generator-rs/src/rt.rs
Line 185 in 177935c
This seems to be an undefined behaviour as
p
borrows fromroot
, which is then mutated whenroot.parent
is set.In
ContextStack::push_context
, ifroot.parent
androot
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
A simplified reproduction: https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=26f583e11b335b98cc4847340a99aa97
MIRI reports
The text was updated successfully, but these errors were encountered: