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

[Merged by Bors] - Fixes dropping empty BlobVec #2295

Closed
wants to merge 5 commits into from
Closed

[Merged by Bors] - Fixes dropping empty BlobVec #2295

wants to merge 5 commits into from

Conversation

thebluefish
Copy link
Contributor

@thebluefish thebluefish commented Jun 2, 2021

When dropping the data, we originally only checked the size of an individual item instead of the size of the allocation. However with a capacity of 0, we attempt to deallocate a pointer which was not the result of allocation. That is, an item of Layout { size_: 8, align_: 8 } produces an array of Layout { size_: 0, align_: 8 } when capacity = 0.

Fixes #2294

@DJMcNab
Copy link
Member

DJMcNab commented Jun 2, 2021

If this fixes #2294, consider making the reference a proper 'fixes reference' of the form 'fixes #2294'

@mockersf mockersf added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels Jun 2, 2021
@mockersf
Copy link
Member

mockersf commented Jun 2, 2021

LGTM once formatted 👍

@NathanSWard
Copy link
Contributor

NathanSWard commented Jun 2, 2021

Can you add a test where the type has Layout { size_: 8, align_: 8 } to ensure it doesn't cause the SIGSEGV as #2294 describes?

Co-authored-by: Nathan Ward <43621845+NathanSWard@users.noreply.github.com>
@NathanSWard NathanSWard added the P-High This is particularly urgent, and deserves immediate attention label Jun 2, 2021
Co-authored-by: Nathan Ward <43621845+NathanSWard@users.noreply.github.com>
@NathanSWard NathanSWard added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 2, 2021
@cart
Copy link
Member

cart commented Jun 2, 2021

Looks good to me! Thanks for the quick fix!

@cart
Copy link
Member

cart commented Jun 2, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jun 2, 2021
When dropping the data, we originally only checked the size of an individual item instead of the size of the allocation. However with a capacity of 0, we attempt to deallocate a pointer which was not the result of allocation. That is, an item of `Layout { size_: 8, align_: 8 }` produces an array of `Layout { size_: 0, align_: 8 }` when `capacity = 0`.

Fixes #2294
@bors bors bot changed the title Fixes dropping empty BlobVec [Merged by Bors] - Fixes dropping empty BlobVec Jun 2, 2021
@bors bors bot closed this Jun 2, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
When dropping the data, we originally only checked the size of an individual item instead of the size of the allocation. However with a capacity of 0, we attempt to deallocate a pointer which was not the result of allocation. That is, an item of `Layout { size_: 8, align_: 8 }` produces an array of `Layout { size_: 0, align_: 8 }` when `capacity = 0`.

Fixes bevyengine#2294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior P-High This is particularly urgent, and deserves immediate attention S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using spawn_bundle with an empty vector causes a sigsegv on destruction.
6 participants