Repository navigation
Conversation
Resize replaced the owned allocation but left m_active_p on the freed block and m_capacity at the old size. Update both, in the main source and the Rust vendored copy, and add BufResize unit tests. Bump the fuzz seed 14 -> 15. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Buf::Resizegrows an owned buffer by allocating a larger block, copying, and freeing the old one, but it only updatesm_owned_p.m_active_pkeeps pointing at the freed block, andm_capacitykeeps the old size.For example, after
Buf buf(2); buf.Resize(4);:buf.Capacity()still returns 2, andbuf.data()still returns the freed block.buf.Resize(8)throws "Buffer is already swapped", becausem_owned_p != m_active_p.Appendstill checks the old capacity. With assertions disabled, it proceeds through the stale active pointer.Relevance and scope
Buf::Resizeis part of the exportedBufAPI. Our search of the upstream source at the tested revision found no production callers of this method. This PR corrects its behavior when called directly; we have not identified an affected application or established external usage. The regression tests demonstrate the API defect, rather than an observed failure in FastLanes' current internal workflows.What changed
src/cor/lyt/buf.cpp: after replacing the owned allocation, also setm_active_pandm_capacity.rust/vendor/fastlanes/src/cor/lyt/buf.cpp: the same two assignments, so the Rust crate does not keep the bug. Following Add MSVC and GCC support & add+improve CI Coverage (incl DEBUG runs) #69, only the affected vendored file is patched rather than running a fullupdate-fastlanes-srcrefresh.test/src/unit_tests/buf_resize_test.cpp(new, registered inunit_test): five tests covering growth, repeated growth, append after growth, a non-growth request, and resizing a swapped buffer.test/src/quick_fuzz_tests/fuzz_config.json:base_seed14 → 15 (make bump).Unchanged: the used-byte count, the copy length (
m_capacitybytes), the early return for non-growth, and the exception when resizing a swapped buffer.Testing
On Linux:
unit_testbuilt from scratch with GCC 15.2.0, Release.BufResize.*5/5 passed, and the full binary passed 102/102 with no skips.buf.cppchange reverted, the three growth tests fail and the two controls pass (Clang 21.1.8).buf.cppreports a use-after-free when readingdata()after growth on the original code, and is clean with the fix.cargo build --releaseoffls-rscompiles the patched vendored source with Clang 21.1.8. It finished with no errors (10 minutes, serial jobs).make verify(seed),scripts/header_check.py --check, and clang-format 14 on the changed C++ files all pass.Not run locally: the other C++ suites (dataset, expression, reader, primitive, quick-fuzz), Python, and the Rust example, which needs libc++. The full Clang build of the test target stopped inside GoogleTest on
-Wcharacter-conversionwith Clang 21, before reaching this change. I'm relying on CI for those.This change was developed with AI coding agents, which wrote the tests, ran the checks above, and drafted this description.