Skip to content

Commit 2a92a1e

Browse files
Rollup merge of rust-lang#158807 - Vastargazing:tests/cstr-clone-into-alloc-error, r=RalfJung
Add regression test for CString::clone_into unwind safety Regression test for the panic-safety fix in rust-lang#155707 rust-lang#70201 gave `<CStr as ToOwned>::clone_into` a path that moved the target `CString`'s buffer out (leaving it empty) before growing a `Vec`; if that growth's allocation failed and unwound, the target was left without its nul terminator, which is UB. rust-lang#155707 fixed this but didn't add a test. The failing allocator here is only honored under Miri - in a normal build a `#[global_allocator]` in a library test doesn't intercept the reallocation inside `CString::clone_into` (it lives in libstd, linked `-C prefer-dynamic`). So the test passes as a regular test and does the real check under Miri, with the unwind assertion gated on `cfg!(miri)`; it runs under Miri in CI via the library-tests-under-Miri job. This started as rust-lang/miri#5157, but per @RalfJung a std regression test belongs with the standard library. r? @RalfJung
2 parents 4267818 + 6a9fb2d commit 2a92a1e

2 files changed

Lines changed: 76 additions & 0 deletions

File tree

library/alloctests/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ rand_xorshift = "0.4.0"
2222
name = "alloctests"
2323
path = "tests/lib.rs"
2424

25+
[[test]]
26+
name = "c_str_alloc_error"
27+
path = "tests/c_str_alloc_error.rs"
28+
2529
[[test]]
2630
name = "vec_deque_alloc_error"
2731
path = "tests/vec_deque_alloc_error.rs"
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
//! Regression test for the panic-safety fix in rust-lang/rust#155707.
2+
//!
3+
//! rust-lang/rust#70201 gave `<CStr as ToOwned>::clone_into` a path that moved the
4+
//! target `CString`'s buffer out before growing a `Vec`; if that growth's allocation
5+
//! failed and unwound, the target was left without its nul terminator. This only
6+
//! reproduces under Miri: in a normal build the `#[global_allocator]` below can't
7+
//! intercept the reallocation inside `CString::clone_into` (it lives in libstd, which
8+
//! library tests link with `-C prefer-dynamic`), so as a regular test it just checks
9+
//! the happy path.
10+
11+
// Disabled under Miri on Windows: a `#[global_allocator]` wrapping `System` trips
12+
// Stacked Borrows there, and it affects libtest's own allocations, not just this test
13+
// (so `#[ignore]` would not be enough). See <https://github.com/rust-lang/miri/issues/2104>.
14+
#![cfg(not(all(miri, windows)))]
15+
#![feature(alloc_error_hook)]
16+
17+
use std::alloc::{GlobalAlloc, Layout, System, set_alloc_error_hook};
18+
use std::ffi::CString;
19+
use std::panic::{AssertUnwindSafe, catch_unwind};
20+
use std::sync::atomic::{AtomicBool, Ordering};
21+
22+
// Once armed, the first allocation of 8 bytes or more fails and disarms, so the
23+
// reallocation inside `clone_into`'s grow path fails while the runtime's own
24+
// smaller allocations keep succeeding.
25+
struct OneShotFailingAlloc;
26+
27+
static ARMED: AtomicBool = AtomicBool::new(false);
28+
29+
unsafe impl GlobalAlloc for OneShotFailingAlloc {
30+
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
31+
if layout.size() >= 8 && ARMED.swap(false, Ordering::SeqCst) {
32+
return core::ptr::null_mut();
33+
}
34+
unsafe { System.alloc(layout) }
35+
}
36+
37+
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
38+
unsafe { System.dealloc(ptr, layout) }
39+
}
40+
41+
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
42+
if new_size >= 8 && ARMED.swap(false, Ordering::SeqCst) {
43+
return core::ptr::null_mut();
44+
}
45+
unsafe { System.realloc(ptr, layout, new_size) }
46+
}
47+
}
48+
49+
#[global_allocator]
50+
static ALLOC: OneShotFailingAlloc = OneShotFailingAlloc;
51+
52+
#[test]
53+
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
54+
fn clone_into_alloc_failure_leaves_target_valid() {
55+
set_alloc_error_hook(|_| panic!("alloc error"));
56+
57+
let src = CString::new("a fairly long value").unwrap();
58+
let mut target = CString::new("x").unwrap();
59+
60+
ARMED.store(true, Ordering::SeqCst);
61+
// Under Miri the failing allocator is honored, so this reallocation unwinds; in a
62+
// normal build the allocator can't intercept it and `clone_into` just succeeds.
63+
let res = catch_unwind(AssertUnwindSafe(|| src.as_c_str().clone_into(&mut target)));
64+
ARMED.store(false, Ordering::SeqCst);
65+
66+
if cfg!(miri) {
67+
assert!(res.is_err(), "clone_into should have unwound on the alloc failure");
68+
}
69+
// Either way `target` must still end in its nul terminator. Before the fix the Miri
70+
// unwind left it empty (also caught as a bad write in `CString`'s destructor).
71+
assert_eq!(target.as_bytes_with_nul().last(), Some(&0));
72+
}

0 commit comments

Comments
 (0)