Skip to content

Fix insert() leaving an undestroyable container when a constructor throws - #75

Merged
martinus merged 1 commit into
mainfrom
fix-68-insert-exception-safety
Jul 27, 2026
Merged

Fix insert() leaving an undestroyable container when a constructor throws#75
martinus merged 1 commit into
mainfrom
fix-68-insert-exception-safety

Conversation

@martinus

Copy link
Copy Markdown
Owner

Fixes #68.

make_uninitialized_space() shifts the elements at pos out of the way and immediately counts the resulting gap in size(). Until the caller constructs into that gap the container claims raw memory, so a throwing element constructor left it in a state that could not even be destroyed:

size=2 capacity=8
inserting 3 copies, the 2nd throws...
caught: copy
after: size=5
ERROR: AddressSanitizer: heap-use-after-free
  #6 ThrowOnCopy::~ThrowOnCopy()
  #7 std::_Destroy<ThrowOnCopy>(ThrowOnCopy*)

size() says 5 while slots 0 and 1 hold objects std::uninitialized_fill_n had already destroyed on its way out.

Fix

All five callers (emplace, both single inserts, the count insert, the forward-iterator range insert) now go through fill_uninitialized_space(), which closes the gap again if the fill throws.

close_gap() has two modes:

  • relocating T cannot throw — shift the tail back down; the container ends up exactly as it started.
  • otherwise — drop the tail. Shorter, but valid and destroys cleanly.

std::move_if_noexcept, the usual way to preserve elements of a throwing-move type, is deliberately not used: close_gap runs mid-unwind, and a throwing copy there would be a second exception in flight and std::terminate.

That split matches the standard. [sequence.reqmts] only requires the strong guarantee for a middle insert when moving cannot throw, and the basic guarantee otherwise.

Tests

test/unit/insert_exception_safety.cpp sweeps sizes 0-9 × every position × counts 1-5 × first/last throw point, across insert(pos, count, v), insert(pos, first, last), insert(pos, v) and emplace, in both storage modes and across the reallocating boundary. For the nothrow-move type it asserts the contents are byte-identical to before; for the throwing-move type asan and the leak checker do the asserting.

Reverting the guard reproduces the heap-use-after-free immediately.

Measured cost

Reviewed the codegen rather than assuming. For svector<int, 8> the try/catch is fully elided — identical .text size (5851 bytes), no landing pads, no .gcc_except_table entries. For svector<std::string, 8> the TU grows 3.9%, almost all of it one out-of-line close_gap per T — it is not templated on the lambda, so the five call sites share it. fill_uninitialized_space inlines away entirely at -O3.

Not fixed here

make_uninitialized_space_new() has the same shape of bug on the growth path: if a move throws partway through relocating into the new storage, the elements already moved are leaked. Confirmed with asan and filed as #74, along with the deeper redesign that would remove this whole family of problems.

…rows

make_uninitialized_space() shifts the elements at pos out of the way and
immediately counts the new gap in size(). Until the caller constructs into that
gap the container claims raw memory, so a throwing element constructor left it
in a state that could not even be destroyed: the destructor ran over slots that
were never built, or over slots the filling algorithm had already cleaned up,
which is a double destroy. asan reports heap-use-after-free.

All five callers now go through fill_uninitialized_space(), which closes the
gap again if the fill throws. When relocating a T cannot throw, close_gap()
shifts the tail back down and the container ends up exactly as it started.
Otherwise it drops the tail, which is shorter but valid and destroys cleanly.
std::move_if_noexcept is not an option there, a copy could throw a second
exception during unwinding.

That matches what the standard asks for: [sequence.reqmts] only requires the
strong guarantee for a middle insert when moving cannot throw, and the basic
guarantee otherwise.

Closes #68

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@martinus
martinus merged commit a377c26 into main Jul 27, 2026
6 checks passed
@martinus
martinus deleted the fix-68-insert-exception-safety branch July 27, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

insert() commits the new size before constructing, so a throwing element ctor causes a double destroy

1 participant