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.

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

Miri gives an error when resolving strings #15

Closed
jyn514 opened this issue Dec 3, 2019 · 8 comments
Closed

Miri gives an error when resolving strings #15

jyn514 opened this issue Dec 3, 2019 · 8 comments

Comments

@jyn514
Copy link

jyn514 commented Dec 3, 2019

Given the following simple use of string_interner, miri gives the error trying to reborrow for SharedReadOnly, but parent tag <2253> does not have an appropriate item in the borrow stack.

I am not quite sure what this error means, but I am concerned about possible UB in string_interner. Here is a link to a similar issue in owning-ref: rust-lang/unsafe-code-guidelines#194

use string_interner::StringInterner;

fn main() {
    let mut strings = StringInterner::default();
    let str_ref = strings.get_or_intern("hello");
    let maybe_resolved = strings.resolve(str_ref);
    println!("{}", maybe_resolved.unwrap());
}
Full miri backtrace
error: Miri evaluation error: trying to reborrow for SharedReadOnly, but parent tag <2253> does not have an appropriate item in the borrow stack
    --> /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/liballoc/boxed.rs:1041:9
     |
1041 |         &**self
     |         ^^^^^^^ Miri evaluation error: trying to reborrow for SharedReadOnly, but parent tag <2253> does not have an appropriate item in the borrow stack
     |
     = note: inside call to `<std::boxed::Box<str> as std::convert::AsRef<str>>::as_ref` at /home/joshua/.cargo/registry/src/gitpro.ttaallkk.top-1ecc6299db9ec823/string-interner-0.7.1/src/lib.rs:367:30
     = note: inside call to closure at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/option.rs:449:29
     = note: inside call to `std::option::Option::<&std::boxed::Box<str>>::map::<&str, [closure@DefId(14:114 ~ string_interner[4f21]::{{impl}}[12]::resolve[0]::{{closure}}[0])]>` at /home/joshua/.cargo/registry/src/gitpro.ttaallkk.top-1ecc6299db9ec823/string-interner-0.7.1/src/lib.rs:365:9
note: inside call to `string_interner::StringInterner::<string_interner::Sym>::resolve` at src/bin.rs:6:26
    --> src/bin.rs:6:26
     |
6    |     let maybe_resolved = strings.resolve(str_ref);
     |                          ^^^^^^^^^^^^^^^^^^^^^^^^
     = note: inside call to `main` at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:61:34
     = note: inside call to closure at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:48:73
     = note: inside call to closure at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/sys_common/backtrace.rs:136:5
     = note: inside call to `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@DefId(1:6021 ~ std[7e82]::rt[0]::lang_start_internal[0]::{{closure}}[0]::{{closure}}[0]) 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:48:13
     = note: inside call to closure at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:287:40
     = note: inside call to `std::panicking::try::do_call::<[closure@DefId(1:6020 ~ std[7e82]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:282:5
     = note: inside call to `std::panicking::try::<i32, [closure@DefId(1:6020 ~ std[7e82]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe]>` at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panic.rs:395:9
     = note: inside call to `std::panic::catch_unwind::<[closure@DefId(1:6020 ~ std[7e82]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:47:25
     = note: inside call to `std::rt::lang_start_internal` at /home/joshua/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:61:5
     = note: inside call to `std::rt::lang_start::<()>`
@Robbepop
Copy link
Owner

Robbepop commented Dec 3, 2019

That's interesting. Thank you for the info.
What does miri say about the pin branch of string_interner?
I feel that it is time for the next round of improvements for string_interner, also in safety perspectives.
The string_interner is using self-referential str for some internal optimizations. Maybe miri found some unsoundness in their usage. I haven't read through the whole issue you linked but it also might be a problem with stacked borrows and self referential structs.

@jyn514
Copy link
Author

jyn514 commented Dec 3, 2019

Interesting, miri does not give any error for the Pin branch.

@jyn514
Copy link
Author

jyn514 commented Dec 3, 2019

string_interner is using self-referential str

Yeah I don't think miri likes that very much: rust-lang/unsafe-code-guidelines#148

@Robbepop
Copy link
Owner

Robbepop commented Dec 3, 2019

Interesting, miri does not give any error for the Pin branch.

That's also interesting because the pin branch is also self-referential, however by making use of the Pin utilities. We did not merge this into master because we had to remove certain APIs that did not work well but also the pin branch by now is pretty old.

@jyn514
Copy link
Author

jyn514 commented Dec 3, 2019

BTW you can test with miri locally like this:

$ rustup component add --toolchain nightly miri
$ cargo +nightly miri run

@jyn514
Copy link
Author

jyn514 commented Mar 27, 2020

Any progress on fixing this?

@jyn514
Copy link
Author

jyn514 commented Mar 27, 2020

Oops, I missed #17. I can confirm that PR fixes the miri evaluation error :)

@Robbepop
Copy link
Owner

This should have been resolved with v0.8. :)
Closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants