Miri reports stacked borrowing error in unsafe cast from element to array · Issue #24 · dtolnay/ryu · GitHub
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.

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

Miri reports stacked borrowing error in unsafe cast from element to array #24

Closed
brson opened this issue Apr 22, 2020 · 2 comments · Fixed by #25
Closed

Miri reports stacked borrowing error in unsafe cast from element to array #24

brson opened this issue Apr 22, 2020 · 2 comments · Fixed by #25

Comments

@brson
Copy link

brson commented Apr 22, 2020 •
edited
Loading

While testing tikv with miri I came across a report about a stacked borrowing violation in ryu, which we use via serde_json.

The error can be triggered against commit 90e4688 with miri from nightly-2020-04-17 by running:

cargo +nightly-2020-04-17 miri test

which outputs the error

running 12 tests
error: Undefined Behavior: no item granting write access to tag <untagged> found in borrow stack.
    --> /home/ubuntu/.rustup/toolchains/nightly-2020-04-17-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/intrinsics.rs:2001:5
     |
2001 |     copy_nonoverlapping(src, dst, count)
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item granting write access to tag <untagged> found in borrow stack.
     |
     = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimenta
l
     = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

     = note: inside `std::intrinsics::copy_nonoverlapping::<u8>` at /home/ubuntu/.rustup/toolchains/nightly-2020-04-17-x86_64-unknown-linux-gn
u/lib/rustlib/src/rust/src/libcore/intrinsics.rs:2001:5
     = note: inside `ryu::raw::format64` at /home/ubuntu/ryu/src/pretty/mod.rs:69:9
     = note: inside `<f64 as ryu::buffer::Sealed>::write_to_ryu_buffer` at /home/ubuntu/ryu/src/buffer/mod.rs:197:9
     = note: inside `ryu::Buffer::format_finite::<f64>` at /home/ubuntu/ryu/src/buffer/mod.rs:88:21
     = note: inside `ryu::Buffer::format::<f64>` at /home/ubuntu/ryu/src/buffer/mod.rs:65:13

The cause seems to be this code:

     #[inline]
     #[cfg(maybe_uninit)]
     fn first_byte_pointer_mut(&mut self) -> *mut u8 {
        self.bytes[0].as_mut_ptr()
     }

which takes a pointer to the first element of an array, then later treats it as a pointer to an array.

I am not sure whether this is truly UB, but this diff silences miri:

     #[inline]
     #[cfg(maybe_uninit)]
     fn first_byte_pointer_mut(&mut self) -> *mut u8 {
-        self.bytes[0].as_mut_ptr()
+        self.bytes.as_mut_ptr() as *mut u8
     }

Unfortunately, after the patch miri fails to terminate.

I am unsure if this is a bug or not. cc @RalfJung
@dtolnay dtolnay mentioned this issue Apr 23, 2020
Merged
@dtolnay
Copy link
Owner

dtolnay commented Apr 23, 2020

Fixed in 1.0.4.

Regarding failing to terminate, I think our tests are just slow.

brson added a commit to brson/tikv that referenced this issue Apr 23, 2020
@brson
*: update ryu to 1.0.4
409115c 
This fixes some undefined behavior, but there's probably no practical impact.

See dtolnay/ryu#24

Signed-off-by: Brian Anderson <andersrb@gmail.com>
@brson brson mentioned this issue Apr 23, 2020
Merged
@RalfJung
Copy link

I am not sure whether this is truly UB

Well, it violates our experimental aliasing model. This particular concern (whether that kind of restriction for raw pointers is desirable) is tracked at rust-lang/unsafe-code-guidelines#134.

Unfortunately, after the patch miri fails to terminate.

Yeah, Miri is really slow, so that's probably it. I often have to reduce iteration counts with cfg(miri) drastically when adding Miri support to a test suite.

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

Successfully merging a pull request may close this issue.

Fix stacked borrow violation and add miri CI dtolnay/ryu
3 participants
@brson @RalfJung @dtolnay