Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unsafe vector assignment operations during reallocations #5107

Closed
wants to merge 3 commits into from

Conversation

winner245
Copy link

Current implementations of vector assignment operations, including the copy, move, initializer_list-assignment operators, as well as all overloads of the assign functions and the C++23 assign_range function, are unsafe during reallocations triggered by assignments. This unsafety can lead to a broken vector with all elements erased, as reported in issue #5106.

The issue arises because all the assignment functions ultimately execute the following code during reallocations on assignments:

_Clear_and_reserve_geometric(_Newsize);
if constexpr (_Nothrow_construct) {
    _Mylast = _Uninitialized_{copy,fill,move}_n(_Myfirst, _Newsize, _Val, _Al);
    _ASAN_VECTOR_CREATE;
} else {
    _ASAN_VECTOR_CREATE_GUARD;
    _Mylast = _Uninitialized_{copy,fill,move}_n(_Myfirst, _Newsize, _Val, _Al);
}

This sequence is unsafe because the destruction of the old vector occurs strictly before the construction of the new vector elements. If any exception is raised during the subsequent construction of new elements, the original vector is already cleared and there is no way to recover it.

This patch fixes #5106 using a copy-and-swap idiom, ensuring that the vector remains intact in the event of exceptions during vector reallocations in assignments. In other words, this patch enhances the copy/initializer_list-assignment operators, the assign overloads, and C++23 assign_range of std::vector to provide strong exception guarantees during vector reallocations. However, it should be noted that in general, these assignment operations cannot always guarantee strong exception safety.

@winner245 winner245 requested a review from a team as a code owner November 21, 2024 17:57
@winner245
Copy link
Author

@microsoft-github-policy-service agree

@StephanTLavavej StephanTLavavej added enhancement Something can be improved decision needed We need to choose something before working on this and removed decision needed We need to choose something before working on this labels Nov 21, 2024
@StephanTLavavej
Copy link
Member

Thanks for looking into this! As explained in #5106 (comment), we're going to close this PR without merging, as attempting to strengthen EH guarantees beyond what the Standard requires is an explicit non-goal of our project. (We would additionally have correctness and performance concerns about the specific code changes here - for example, copy-and-swap is an expensive allocating operation, whereas copy assignment can reuse the existing buffer if it has sufficient capacity, so there's a big difference.)

@winner245
Copy link
Author

Thank you for your feedback and insights. I understand and respect your concerns about the standard consistency and performance impact. Thank you for your time and consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Unsafe vector assignment operations during reallocations
2 participants