Repository navigation
CString being noalias is a footgun #136770
Description
Internally, String is implemented as a Vec<u8>. However, CString is implemented as a Box<[u8]>.
This means that CString is currently treated as noalias, like Box (but not Vec) is. (See rust-lang/unsafe-code-guidelines#326.) This means that there are additional rules of how pointers pointing into the CString buffer can be used, and violating these rules lead to UB. As far as I can tell, the fact that this applies to CString is not documented anywhere.
I don't know if this has produced any issues in practice, but I believe that this is an undesirable footgun that should be eliminated (e.g., by changing CString to use NonNull<[u8]> instead), especially since CString is intended to be turned into a pointer and passed to FFI, potentially resulting in subtle UB that Miri can't detect.
For example, consider the following code, which is a stand-in for how a program with C FFI might use CString:
use std::ffi::{CString, c_char, CStr};
fn main() {
let my_string = CString::new("abc").unwrap();
let ptr: *const c_char = my_string.as_ptr();
// Suppose that `ptr` is given to some C FFI function here
let _moved_my_string = my_string;
// Suppose that `ptr` is re-obtained from C FFI here
let _my_str = unsafe { CStr::from_ptr(ptr) };
}This code might seem intuitively fine to many Rust users, but it causes UB according to Miri.
Error from Miri
error: Undefined Behavior: attempting a read access using <2513> at alloc1005[0x0], but that tag does not exist in the borrow stack for this location
--> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ffi/c_str.rs:740:22
|
740 | unsafe { strlen(s) }
| ^^^^^^^^^
| |
| attempting a read access using <2513> at alloc1005[0x0], but that tag does not exist in the borrow stack for this location
| this error occurs as part of an access at alloc1005[0x0..0x1]
|
= 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: <2513> was created by a SharedReadOnly retag at offsets [0x0..0x4]
--> src/main.rs:5:30
|
5 | let ptr: *const c_char = my_string.as_ptr();
| ^^^^^^^^^^^^^^^^^^
help: <2513> was later invalidated at offsets [0x0..0x4] by a Unique retag (of a reference/box inside this compound value)
--> src/main.rs:7:28
|
7 | let _moved_my_string = my_string;
| ^^^^^^^^^
= note: BACKTRACE (of the first span):
= note: inside `core::ffi::c_str::strlen::runtime` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ffi/c_str.rs:740:22: 740:31
= note: inside `core::ffi::c_str::strlen` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/intrinsics/mod.rs:3886:9: 3886:61
= note: inside `std::ffi::CStr::from_ptr::<'_>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ffi/c_str.rs:267:28: 267:39
note: inside `main`
--> src/main.rs:9:28
|
9 | let _my_str = unsafe { CStr::from_ptr(ptr) };
| ^^^^^^^^^^^^^^^^^^^
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
(It's debatable whether CString not having extra capacity is good or not, but that's a separate issue.)
@rustbot labels +A-FFI +A-box +T-libs-api +T-opsem
Meta
The Miri error was from running the code on the playground with rust version 1.86.0-nightly (2025-02-08 43ca9d18e333797f0aa3).
Activity
the8472 commentedon Feb 9, 2025
Only by making assumptions about
as_ptr(). If you usedslice.as_ptr()and then tried to move the thing the slice was borrowing from that'd be incorrect. So one could equally say "you moved the thing you were borrowing from, of course that's UB". This is the general rule.Vec::as_ptris the exception and explicitly documented so.If you base your assumptions on the exceptional case instead of the general case then you'll run into problems... so that intuition seems wrong.
theemathas commentedon Feb 9, 2025
@the8472
Vec::as_ptr's documentation says:This doesn't actually say anything about noalias. In particular, it doesn't say whether you're allowed to move the
Vecor not.the8472 commentedon Feb 9, 2025
Hrm right, I'm not sure if the extra grace that Vec provides is guaranteed or just a "let's not gratuitously break people who relied on this". But my point stands, the norm is that moving invalidates and you need an exception to do this. If your intuition is that this is generally ok then the intuition needs fixing.
I think a better argument might be saying that this is useful and asking for a guarantee. But that'd need a better demonstration what this is useful for, why does it get moved?
hanna-kruppe commentedon Feb 9, 2025
See also rust-lang/rfcs#3712
the8472 commentedon Feb 9, 2025
At least
CStringdoes explicitly document that you get the ptr through deref, which means it's derived from a borrow.joboet commentedon Feb 12, 2025
For what it's worth, I've found an instance of
stdcode that is unsound because of this. TheCommandimplementation uses this code to generate an environment array for the spawned process:rust/library/std/src/sys/pal/unix/process/process_common.rs
Lines 417 to 419 in 021fb9c
Because the
CStringis moved, the provenance of the pointers in the array will be invalidated. Unfortunately, this wasn't caught by miri since it doesn't support process spawning...theemathas commentedon Feb 13, 2025
I've created a new Zulip topic on this issue.
traviscross commentedon Feb 13, 2025
For what it's worth, this fails Stacked Borrows but is not necessarily UB as the exact model for Rust is still undetermined.
This is, e.g., accepted by Tree Borrows.
Playground link
cc @RalfJung
RalfJung commentedon Feb 13, 2025
Good catch! We have generally strived to keep
stdcompatible with Stacked Borrows. Well, I have strived to do so, and so far my patches were accepted, and we are gating PRs on Miri running the std test suite with Stacked Borrows. But Miri can't runCommandso this case got missed.Cc @rust-lang/opsem
theemathas commentedon May 31, 2025
The specific unsoundness in std seems to have been fixed by #138896. The potential footgun with CString in general remains though.