Skip to content

Commit 6a9fb2d

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 6a9fb2d

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)