refactor: Harden prevector::change_capacity against capacity < size #35166

Summary

Clamp new_capacity to never go below size() in change_capacity(), enforcing the same invariant that std::vector maintains (capacity >= size).

Without this guard, if change_capacity() is called with new_capacity < size(), two problems arise:

• Indirect-to-direct transition: memcpy copies size() elements into the direct buffer which only holds N elements — a heap buffer overflow.
• Indirect-to-indirect realloc: the buffer shrinks below the number of live elements — subsequent access is out-of-bounds.

While no current caller triggers either case, the 1.5x growth computation (new_size + (new_size >> 1)) wraps around uint32_t when new_size > 0xAAAAAAAA, producing a small value that would hit this path. The clamp prevents this overflow from becoming a heap buffer overflow.

This is the same class of latent issue fixed in a7af72a697 ("prevector::swap: fix (unreached) data corruption").

Test plan

Verified with a standalone test exercising indirect-to-direct transitions, grow+shrink cycles, and the capacity >= size invariant. Existing PrevectorTestInt randomized tests are unaffected (the clamp is a no-op for all valid callers).

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

This looks entirely LLM generated without real understanding from the author, and should be closed.

There is no test attached and the description even mentions "no current caller triggers either case".

This looks entirely LLM generated without real understanding from the author, and should be closed.

There is no test attached and the description even mentions "no current caller triggers either case".

The issue was found by my manual code-review, but of course I used LLM to generate the PR description. Don't you? I didn't include the local test I ran because I'm not sure it fits in the existing prevector test-suite, as there are no public functions that can have the check pass and cause the capacity to not reduce. I can add the test results here, or do you prefer that the test itself to be added as part of the PR?

I didn't include the local test I ran because I'm not sure it fits in the existing prevector test-suite, as there are no public functions that can have the check pass and cause the capacity to not reduce.

So why change it then if the public interface of the class already handles it correctly?

I didn't include the local test I ran because I'm not sure it fits in the existing prevector test-suite, as there are no public functions that can have the check pass and cause the capacity to not reduce.

So why change it then if the public interface of the class already handles it correctly?

Currently, the public interface has an unexploitable integer overflow bug in the 1.5x growth factor. 4 places compute new_size + (new_size >> 1) which wraps uint32_t to a small value when new_size > 0xAAAAAAAA:

insert(iterator, const T&) insert(iterator, size_type, const T&) insert(iterator, InputIterator, InputIterator) emplace_back

If the overflow happens, change_capacity() is called with new_capacity < size(). My thought is that std::vector is immune to this because its allocator would reject the wrapped size, but prevector would happily accept it.

I don't think this is useful? This will just move the UB or misbehaviour further down.

If there ever was a use case to store more than idk 1GiB of data or so, it should be free and more correct to just use u64.

Closing this again. Not only is it clearly LLM generated, but I also don't think it is a good allocation of reviewer time to be working through this, especially given the mentioned defects of this PR.