Skip to content

Commit 735cc3d

Browse files
committed
Add regression test for CString::clone_into unwind safety
CString::clone_into reuses the target's allocation by moving the buffer into a Vec and growing it. If that growth's allocation fails and the alloc error hook unwinds, the target has to be left as a valid CString, but nothing covered that path. Add a test in library/alloctests that fails the reallocation under a panicking alloc error hook and checks the target stays valid. The failing allocator is only honored under Miri - a global allocator in a library test doesn't intercept libstd's allocation in a normal build - so the unwind assertion is gated on cfg!(miri); the test still runs and passes as a regular test.
1 parent ea088e0 commit 735cc3d

2 files changed

Lines changed: 71 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: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
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+
#![feature(alloc_error_hook)]
11+
12+
use std::alloc::{GlobalAlloc, Layout, System, set_alloc_error_hook};
13+
use std::ffi::CString;
14+
use std::panic::{AssertUnwindSafe, catch_unwind};
15+
use std::sync::atomic::{AtomicBool, Ordering};
16+
17+
// Once armed, the first allocation of 8 bytes or more fails and disarms, so the
18+
// reallocation inside `clone_into`'s grow path fails while the runtime's own
19+
// smaller allocations keep succeeding.
20+
struct OneShotFailingAlloc;
21+
22+
static ARMED: AtomicBool = AtomicBool::new(false);
23+
24+
unsafe impl GlobalAlloc for OneShotFailingAlloc {
25+
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
26+
if layout.size() >= 8 && ARMED.swap(false, Ordering::SeqCst) {
27+
return core::ptr::null_mut();
28+
}
29+
unsafe { System.alloc(layout) }
30+
}
31+
32+
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
33+
unsafe { System.dealloc(ptr, layout) }
34+
}
35+
36+
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
37+
if new_size >= 8 && ARMED.swap(false, Ordering::SeqCst) {
38+
return core::ptr::null_mut();
39+
}
40+
unsafe { System.realloc(ptr, layout, new_size) }
41+
}
42+
}
43+
44+
#[global_allocator]
45+
static ALLOC: OneShotFailingAlloc = OneShotFailingAlloc;
46+
47+
#[test]
48+
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
49+
fn clone_into_alloc_failure_leaves_target_valid() {
50+
set_alloc_error_hook(|_| panic!("alloc error"));
51+
52+
let src = CString::new("a fairly long value").unwrap();
53+
let mut target = CString::new("x").unwrap();
54+
55+
ARMED.store(true, Ordering::SeqCst);
56+
// Under Miri the failing allocator is honored, so this reallocation unwinds; in a
57+
// normal build the allocator can't intercept it and `clone_into` just succeeds.
58+
let res = catch_unwind(AssertUnwindSafe(|| src.as_c_str().clone_into(&mut target)));
59+
ARMED.store(false, Ordering::SeqCst);
60+
61+
if cfg!(miri) {
62+
assert!(res.is_err(), "clone_into should have unwound on the alloc failure");
63+
}
64+
// Either way `target` must still end in its nul terminator. Before the fix the Miri
65+
// unwind left it empty (also caught as a bad write in `CString`'s destructor).
66+
assert_eq!(target.as_bytes_with_nul().last(), Some(&0));
67+
}

0 commit comments

Comments
 (0)