Skip to content

Fix overflow in RenderLayers::iter_layers#14264

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
Azorlogh:fix-render-layer-overflow
Jul 15, 2024
Merged

Fix overflow in RenderLayers::iter_layers#14264
alice-i-cecile merged 1 commit intobevyengine:mainfrom
Azorlogh:fix-render-layer-overflow

Conversation

@Azorlogh
Copy link
Contributor

Objective

  • Fixes overflow when calling RenderLayers::iter_layers on layers of the form k * 64 - 1
    • Causes a panic in debug mode, and an infinite iterator in release mode

Solution

  • Use u64::checked_shr instead of >>=

Testing

  • Added a test case for this: render_layer_iter_no_overflow

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen P-Crash A sudden unexpected crash S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 14, 2024
@alice-i-cecile alice-i-cecile added this to the 0.14.1 milestone Jul 14, 2024
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 14, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 15, 2024
Merged via the queue into bevyengine:main with commit a79df7b Jul 15, 2024
mockersf pushed a commit that referenced this pull request Aug 2, 2024
# Objective

- Fixes overflow when calling `RenderLayers::iter_layers` on layers of
the form `k * 64 - 1`
- Causes a panic in debug mode, and an infinite iterator in release mode

## Solution

- Use `u64::checked_shr` instead of `>>=`

## Testing

- Added a test case for this: `render_layer_iter_no_overflow`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash 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.

3 participants