Skip to content

Validate binding ranges against buffer size #7911

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
merged 6 commits into from
Jul 10, 2025

Conversation

andyleiserson
Copy link
Contributor

Reprise of #7875, which was reverted in #7905.

This tightens up the validation of binding sizes. I have added a helper on the wgpu_core Buffer type to resolve implicit remainder-of-buffer sizes and validate the binding offset and size against the size of the buffer. This helper is now the preferred way of constructing a hal::BufferBinding. Direct construction is no longer allowed in wgpu_core. There is an unsafe new_unchecked helper for cases where direct construction is necessary.

This fixes a small but critical typo in the previous change: 14a5abd

It also stays even closer to the original behavior for zero-size bindings than the previous change. When we validate a binding with implicit remainder-of-buffer size, we preserve the information that it is sized as such, and resolve it again in the backends.

I've divided the changes into meaningful independent commits. Since #7875 was reverted, the PR diff is against the state prior to those changes. The diff against the state after #7875 may also be informative: https://github.com/gfx-rs/wgpu/compare/e5b03ffa1defc0b760a8c0bd8c54aa9b98c78002...andyleiserson:wgpu:buffer-size-fixes?expand=1.

Testing
Enables some CTS tests that were broken by the previous change.

The following CTS tests are still failing, but are also relevant:

webgpu:api,validation,encoding,cmds,render,draw:vertex_buffer_OOB:*
webgpu:api,validation,encoding,cmds,render,draw:buffer_binding_overlap:*

Squash or Rebase? Undecided

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@andyleiserson andyleiserson requested review from crowlKats and a team as code owners July 10, 2025 00:49
@ErichDonGubler
Copy link
Member

@jimblandy fielded the review for the first iteration of this fix, so I'll tentatively pull him in for the second round.

Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Andy and I talked through this in a meeting, and it looks good to me.

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.

3 participants