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 segfault in ArrayVec<Vec<T>>::into_vec #601

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Nov 2, 2021

Fixes #599

@philipc
Copy link
Collaborator Author

philipc commented Nov 2, 2021

The new asserts are enough for this to be covered by existing tests. The error was also detected by miri, and this is now fixed.

@philipc philipc merged commit 9fe567d into gimli-rs:master Nov 2, 2021
@philipc philipc deleted the issue-599 branch November 2, 2021 04:39
@cbiffle
Copy link

cbiffle commented Nov 10, 2021

Out of curiosity, I notice that after the fix the code is still constructing a prefix of a slice using from_raw_parts_mut in DerefMut/Deref, and only checking bounds in debug builds. What performance benefit did you measure from skipping the bounds checks in release builds, rather than using bounds-checked prefixing (&mut slice[..len]), which would turn this class of mistake into a panic rather than a segfault?

(Just finished tracking down an intermittent crash in our debugger due to this.)

@philipc
Copy link
Collaborator Author

philipc commented Nov 10, 2021

I didn't check for this PR, but when the ArrayVec implementation was first introduced it had a measurable difference in the existing benchmarks.

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.

Segmentation fault in ArrayVec::clear
2 participants