UB in `From<String>` for `BoxedString`

dragazo commented 1 year ago

I ran the existing test suite with miri, which encountered undefined behavior in From<String> for BoxedString. Specifically, the error is on the line:

unsafe { allocator.grow(ptr, old_layout, Self::layout_for(cap)) }

And the output from miri is as follows:

test test::tests::fail ... error: Undefined Behavior: attempting a read access using <210053> at alloc55337[0x20], but that tag does not exist in the borrow stack for this location
   --> /home/devin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:228:17
    |
228 |                 ptr::copy_nonoverlapping(ptr.as_ptr(), new_ptr.as_mut_ptr(), old_size);
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                 |
    |                 attempting a read access using <210053> at alloc55337[0x20], but that tag does not exist in the borrow stack for this location
    |                 this error occurs as part of an access at alloc55337[0x0..0x21]
    |
    = 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: <210053> was created by a SharedReadWrite retag at offsets [0x0..0x20]
   --> src/boxed.rs:181:59
    |
181 |                 let ptr = unsafe { NonNull::new_unchecked(s.as_mut_ptr()) };
    |                                                           ^^^^^^^^^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `std::alloc::Global::grow_impl` at /home/devin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:228:17: 228:87
    = note: inside `<std::alloc::Global as std::alloc::Allocator>::grow` at /home/devin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:266:18: 266:68
note: inside `<boxed::BoxedString as std::convert::From<std::string::String>>::from`
   --> src/boxed.rs:187:30
    |
187 |                     unsafe { allocator.grow(ptr, old_layout, Self::layout_for(cap)) }
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: inside `<std::string::String as std::convert::Into<boxed::BoxedString>>::into` at /home/devin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:716:9: 716:22
note: inside `<SmartString<config::Compact> as std::convert::From<std::string::String>>::from`
   --> src/lib.rs:679:30
    |
679 |             Self::from_boxed(string.into())
    |                              ^^^^^^^^^^^^^
    = note: inside `<std::string::String as std::convert::Into<SmartString<config::Compact>>>::into` at /home/devin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:716:9: 716:22
note: inside `test::tests::fail`
   --> src/test.rs:515:51
    |
515 |         let control_smart: SmartString<Compact> = control.into();
    |                                                   ^^^^^^^^^^^^^^
note: inside closure
   --> src/test.rs:509:15
    |
508 |     #[test]
    |     ------- in this procedural macro expansion
509 |     fn fail() {
    |               ^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

MolotovCherry commented 9 months ago

This appears to still be a problem. I have a similar Miri UB detection for this code:

use scraper::Html;
use smartstring::{LazyCompact, SmartString};

fn main() {
    let a = Html::parse_document("<html></html>");
    let s: String = a.html(); // ~ Fine
    let _: SmartString<LazyCompact> = s.into(); // ~ Not fine
}

Cargo.toml

[package]
name = "boo"
version = "0.1.0"
edition = "2021"

[dependencies]
smartstring = "1.0.1"
scraper = "0.18.1"

error: Undefined Behavior: attempting a read access using <26426> at alloc8296[0x27], but that tag does not exist in the borrow stack for this location
   --> ~\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\alloc.rs:228:17
    |
228 |                 ptr::copy_nonoverlapping(ptr.as_ptr(), new_ptr.as_mut_ptr(), old_size);
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                 |
    |                 attempting a read access using <26426> at alloc8296[0x27], but that tag does not exist in the borrow stack for this location
    |                 this error occurs as part of an access at alloc8296[0x0..0x40]
    |
    = 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: <26426> was created by a SharedReadWrite retag at offsets [0x0..0x27]
   --> src\main.rs:7:39
    |
7   |     let _: SmartString<LazyCompact> = s.into();
    |                                       ^^^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `std::alloc::Global::grow_impl` at ~\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\alloc.rs:228:17: 228:87
    = note: inside `<std::alloc::Global as std::alloc::Allocator>::grow` at ~\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\alloc.rs:266:18: 266:68
    = note: inside `<smartstring::boxed::BoxedString as std::convert::From<std::string::String>>::from` at ~\.cargo\registry\src\index.crates.io-6f17d22bba15001f\smartstring-1.0.1\src\boxed.rs:187:30: 187:84
    = note: inside `<std::string::String as std::convert::Into<smartstring::boxed::BoxedString>>::into` at ~\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\convert\mod.rs:757:9: 757:22
    = note: inside `<smartstring::SmartString<smartstring::LazyCompact> as std::convert::From<std::string::String>>::from` at ~\.cargo\registry\src\index.crates.io-6f17d22bba15001f\smartstring-1.0.1\src\lib.rs:679:30: 679:43
    = note: inside `<std::string::String as std::convert::Into<smartstring::SmartString<smartstring::LazyCompact>>>::into` at ~\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\convert\mod.rs:757:9: 757:22

MolotovCherry commented 9 months ago

Tested PR #34, which appears to fix the UB I was experiencing

bodil / smartstring

UB in `From<String>` for `BoxedString` #49