Add regression test for CString::clone_into unwind safety by Vastargazing · Pull Request #158807 · rust-lang/rust

Vastargazing · 2026-07-05T11:53:39Z

Regression test for the panic-safety fix in #155707

#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. #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

rustbot · 2026-07-05T11:53:44Z

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

RalfJung · 2026-07-05T14:53:23Z

LGTM, thanks :)

@bors r+ rollup

rust-bors · 2026-07-05T14:53:26Z

📌 Commit 735cc3d has been approved by RalfJung

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 2. This pull request will be tested once the tree is reopened.

@RalfJung

…-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

@RalfJung

…-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

Rollup of 20 pull requests Successful merges: - #158377 (add `-Zforce-intrinsic-fallback` flag) - #158642 (Clarify some interning details) - #158743 (Look for cdb location in the registry first) - #158775 (bootstrap: only encode RUSTFLAGS when a flag contains a space) - #158782 (Add and use cfg(target_has_threads) to enforce no_thread impl usage) - #158785 (hook intrinsic-test into aarch64-gnu) - #157734 (Stabilize `local_key_cell_update`) - #158183 (std: allocate less memory in `current_exe` for OpenBSD) - #158671 (Move tests batch 17) - #158730 (Update `FIXME(static_mut_refs)` comments) - #158752 (Reorganize `tests/ui/issues` [18/N]) - #158755 (Use `ThinVec` more in the AST) - #158757 (Fix incorrect tracking issue for `read_le`/`read_be`) - #158765 (Fix ICE on non-ident path in `doc(auto_cfg values)`) - #158771 (library: expand HashSet::extract_if coverage) - #158772 (rustc-dev-guide subtree update) - #158776 (fix: emit diagnostic for AVR target without target-cpu) - #158786 (Add regression test for builtin attr macro values) - #158807 (Add regression test for CString::clone_into unwind safety) - #158825 (Fix typo)

jhpratt · 2026-07-06T08:32:35Z

Look like you've got some UB in the test.

@bors r- #158837 (comment)

rust-bors · 2026-07-06T08:32:45Z

This pull request was unapproved.

View changes since this unapproval

Vastargazing · 2026-07-07T04:57:52Z

Thanks. The UB is in libtest's own teardown, not the test body: with a #[global_allocator] installed, libtest frees its internal allocations (the mpmc results channel) through it, and on Windows System::dealloc reads the allocation header just before the pointer, which trips Stacked Borrows since the freed Box's provenance doesn't cover the header. It fires even with 0 tests run - it's inherent to wrapping System in a global allocator under Miri on Windows, independent of the test itself.

Two ways to resolve it, whichever you'd prefer:

keep it here and only install the global allocator + run the test when not all(miri, target_os = "windows") - Miri's default allocator on Windows, Linux Miri still runs it and catches the bug; or
move it back to a Miri ui-test (fn main, no libtest harness) - this is what Add regression test for CString::clone_into unwind safety miri#5157 was, which passed the full Miri matrix including Windows.

Happy to do either - which do you prefer?

RalfJung · 2026-07-07T11:07:30Z

Disabling the test on Windows with a link to rust-lang/miri#2104 seems good.

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.

Vastargazing · 2026-07-08T04:33:24Z

Done - disabled under Miri on Windows via crate-level #![cfg(not(all(miri, windows)))] rather than #[ignore], since the global allocator hits the issue even when the test doesn't run. Linked rust-lang/miri#2104. Force-pushed.

@rustbot ready

RalfJung · 2026-07-08T06:57:35Z

@bors try jobs=x86_64-msvc-ext2

…r, r=<try> Add regression test for CString::clone_into unwind safety try-job: x86_64-msvc-ext2

rust-bors · 2026-07-08T08:59:18Z

☀️ Try build successful (CI)
Build commit: 4cd977a (4cd977a8b0ecf2e638bd2ae15174885b423ef890)
Base parent: 99576cf (99576cf2ebc870c305225e9856b9423228872c56)

RalfJung · 2026-07-08T09:16:51Z

@bors r+ rollup

rust-bors · 2026-07-08T09:16:54Z

📌 Commit 6a9fb2d has been approved by RalfJung

It is now in the queue for this repository.

@RalfJung

…-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

@RalfJung

…-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

…uwer Rollup of 18 pull requests Successful merges: - #158871 (add relnotes for 1.97.0) - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158655 (Fix coroutine MIR saved local remapping) - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`) - #158920 (Update wasm-component-ld to 0.5.26) - #158926 (wrapping_sh* methods: clarify underspecified reference) - #158927 (add core test run with `-Zforce-intrinsic-fallback`) - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`) - #158807 (Add regression test for CString::clone_into unwind safety) - #158862 (Fix the span for parameter suggestion ) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic) - #158902 (add codegen test for range length bound propagation) - #158913 (Update `browser-ui-test` version to `0.24.1`) - #158935 (std: support real fd methods on Emscripten) - #158951 (Merge three `MaxUniverse`s into one)

@RalfJung

…-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

@RalfJung

…-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

…uwer Rollup of 25 pull requests Successful merges: - #158871 (add relnotes for 1.97.0) - #158968 (stdarch subtree update) - #154445 (rustdoc: Represent `--output-format=json` coverage and ir differently) - #156370 (Reject linked dylib EII default overrides) - #157153 (allow `Allocator`s to be used as `#[global_allocator]`s) - #158495 (Rename HAS_CT_PROJECTION to HAS_CONST_ALIAS) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158655 (Fix coroutine MIR saved local remapping) - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`) - #158912 (Introduce new bootstrap config section for PGO configuration) - #158920 (Update wasm-component-ld to 0.5.26) - #158926 (wrapping_sh* methods: clarify underspecified reference) - #158927 (add core test run with `-Zforce-intrinsic-fallback`) - #158932 (Do not build the compiler when invoking `x perf compare`) - #158937 (Emit the emscripten entry point as `__main_argc_argv`) - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`) - #156548 ( Library support for aarch64-unknown-linux-pauthtest target) - #158307 (CI job for parallel frontend ui tests) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158741 (Simplify `Option::into_flat_iter` signature) - #158807 (Add regression test for CString::clone_into unwind safety) - #158862 (Fix the span for parameter suggestion ) - #158883 (tests: fix enum-match.rs to handle LLVM 23)

@RalfJung

…-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

Rollup of 23 pull requests Successful merges: - #158968 (stdarch subtree update) - #154445 (rustdoc: Represent `--output-format=json` coverage and ir differently) - #158495 (Rename HAS_CT_PROJECTION to HAS_CONST_ALIAS) - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`) - #158870 (std: merge the unix-like io::error modules into one file) - #158920 (Update wasm-component-ld to 0.5.26) - #158926 (wrapping_sh* methods: clarify underspecified reference) - #158927 (add core test run with `-Zforce-intrinsic-fallback`) - #158932 (Do not build the compiler when invoking `x perf compare`) - #158937 (Emit the emscripten entry point as `__main_argc_argv`) - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`) - #156144 (Better docs for PartialEq (includes macro rename)) - #156548 ( Library support for aarch64-unknown-linux-pauthtest target) - #157995 (`Vec::dedup_by` docs explicit function argument order) - #158307 (CI job for parallel frontend ui tests) - #158741 (Simplify `Option::into_flat_iter` signature) - #158807 (Add regression test for CString::clone_into unwind safety) - #158862 (Fix the span for parameter suggestion ) - #158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic) - #158902 (add codegen test for range length bound propagation) - #158913 (Update `browser-ui-test` version to `0.24.1`) - #158935 (std: support real fd methods on Emscripten) - #158978 (Add regression test for too-big by-value ABI args)

@RalfJung

Rollup merge of #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 #155707 #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. #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

Rollup of 23 pull requests Successful merges: - rust-lang/rust#158968 (stdarch subtree update) - rust-lang/rust#154445 (rustdoc: Represent `--output-format=json` coverage and ir differently) - rust-lang/rust#158495 (Rename HAS_CT_PROJECTION to HAS_CONST_ALIAS) - rust-lang/rust#158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`) - rust-lang/rust#158870 (std: merge the unix-like io::error modules into one file) - rust-lang/rust#158920 (Update wasm-component-ld to 0.5.26) - rust-lang/rust#158926 (wrapping_sh* methods: clarify underspecified reference) - rust-lang/rust#158927 (add core test run with `-Zforce-intrinsic-fallback`) - rust-lang/rust#158932 (Do not build the compiler when invoking `x perf compare`) - rust-lang/rust#158937 (Emit the emscripten entry point as `__main_argc_argv`) - rust-lang/rust#151379 (Stabilize `VecDeque::retain_back` from `truncate_front`) - rust-lang/rust#156144 (Better docs for PartialEq (includes macro rename)) - rust-lang/rust#156548 ( Library support for aarch64-unknown-linux-pauthtest target) - rust-lang/rust#157995 (`Vec::dedup_by` docs explicit function argument order) - rust-lang/rust#158307 (CI job for parallel frontend ui tests) - rust-lang/rust#158741 (Simplify `Option::into_flat_iter` signature) - rust-lang/rust#158807 (Add regression test for CString::clone_into unwind safety) - rust-lang/rust#158862 (Fix the span for parameter suggestion ) - rust-lang/rust#158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic) - rust-lang/rust#158902 (add codegen test for range length bound propagation) - rust-lang/rust#158913 (Update `browser-ui-test` version to `0.24.1`) - rust-lang/rust#158935 (std: support real fd methods on Emscripten) - rust-lang/rust#158978 (Add regression test for too-big by-value ABI args)

Zalathar · 2026-09-03T06:42:38Z

Possible flaky failure of this test in PR CI: #161517 (comment)

Vastargazing · 2026-09-04T06:08:33Z

thanks for flagging this

this is very likely a race in c_str_alloc_error: its allocator can fail a libtest allocation while allocation-failure injection is active

that matches the observed CI failure, though the log has no backtrace to confirm it conclusively

@RalfJung, would you be willing to review and be assigned to a follow-up PR?

RalfJung · 2026-09-04T06:29:49Z

I'd prefer if someone else could review this, I'm pretty swamped at the moment.

rustbot assigned RalfJung Jul 5, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 5, 2026

Vastargazing mentioned this pull request Jul 5, 2026

Add regression test for CString::clone_into unwind safety rust-lang/miri#5157

Closed

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 5, 2026

JonathanBrouwer mentioned this pull request Jul 5, 2026

Rollup of 16 pull requests #158828

Closed

jhpratt mentioned this pull request Jul 6, 2026

Rollup of 20 pull requests #158837

Closed

rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 6, 2026

Vastargazing force-pushed the tests/cstr-clone-into-alloc-error branch from 735cc3d to 6a9fb2d Compare July 8, 2026 04:01

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 8, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jul 8, 2026

Auto merge of #158807 - Vastargazing:tests/cstr-clone-into-alloc-erro…

4cd977a

…r, r=<try> Add regression test for CString::clone_into unwind safety try-job: x86_64-msvc-ext2

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 8, 2026

GuillaumeGomez mentioned this pull request Jul 8, 2026

Rollup of 16 pull requests #158953

Closed

JonathanBrouwer mentioned this pull request Jul 8, 2026

Rollup of 18 pull requests #158958

Closed

JonathanBrouwer mentioned this pull request Jul 8, 2026

Rollup of 25 pull requests #158973

Closed

JonathanBrouwer mentioned this pull request Jul 8, 2026

Rollup of 25 pull requests #158977

Closed

This was referenced Jul 9, 2026

Rollup of 25 pull requests #158989

Closed

Rollup of 23 pull requests #158991

Merged

rust-bors Bot merged commit 1b30851 into rust-lang:main Jul 9, 2026
14 checks passed

rustbot added this to the 1.99.0 milestone Jul 9, 2026

Vastargazing deleted the tests/cstr-clone-into-alloc-error branch July 11, 2026 05:43

Uh oh!

Uh oh!

Conversation

Vastargazing commented Jul 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 5, 2026

Uh oh!

RalfJung commented Jul 5, 2026

Uh oh!

rust-bors Bot commented Jul 5, 2026

Uh oh!

jhpratt commented Jul 6, 2026

Uh oh!

rust-bors Bot commented Jul 6, 2026

Uh oh!

Vastargazing commented Jul 7, 2026

Uh oh!

RalfJung commented Jul 7, 2026

Uh oh!

Vastargazing commented Jul 8, 2026

Uh oh!

RalfJung commented Jul 8, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Jul 8, 2026

Uh oh!

RalfJung commented Jul 8, 2026 via email

Uh oh!

rust-bors Bot commented Jul 8, 2026

Uh oh!

Uh oh!

Zalathar commented Sep 3, 2026

Uh oh!

Vastargazing commented Sep 4, 2026

Uh oh!

RalfJung commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Vastargazing commented Jul 5, 2026 •

edited by rustbot

Loading