Repository navigation
fix(ext/webgpu): bounds-check + view-aware setBindGroup Uint32Array fast path - #33980
Merged
crowlKats merged 5 commits intoJun 9, 2026
Merged
Conversation
…ast path The Uint32Array fast path of setBindGroup in compute_pass.rs, render_pass.rs and render_bundle.rs had two related bugs (denoland#33956): 1. `&data[start..(start + len)]` sliced unchecked. An out-of-range length panicked inside the op's `extern "C"` callback and the panic crossed the C ABI as a process abort — six lines of JS could kill a running runtime. Add an explicit bounds check that surfaces as a GPUValidationError (or a thrown JS error in the render-bundle path, which has no per-call error_handler) instead. 2. `uint_32.buffer(scope).data()` + `ab.byte_length() / 4` walked the backing ArrayBuffer and ignored the typed-array view's `byteOffset` and `length`. A `Uint32Array(buf, byteOffset > 0, …)` fed wgpu_core the wrong window with no error surfaced — silently incorrect rendering / compute results. Construct the slice from `byte_offset()` + `length()` instead. Adds a regression test (gated on `!isCIWithoutGPU`) that calls setBindGroup with start=0 + len=1_000_000 on a 4-element Uint32Array, against both a ComputePass (expects validation error in the device error scope) and a RenderBundleEncoder (expects a thrown JS error), and confirms a well-formed call still works. Credit to @hibrian827 for the diagnosis, three repros, and the suggested fix in the upstream report. Fixes denoland#33956. Signed-off-by: lunadogbot <lunadogbot@users.noreply.github.com>
`tools/lint.js` ran dprint+rustfmt and asked for the
`let Some(end) = ... else { ... }` condition on one line.
Signed-off-by: lunadogbot <lunadogbot@users.noreply.github.com>
Deno Individual Contributor License AgreementAll contributors have signed the CLA. Thank you! This is an automated message from CLA Assistant |
Contributor
Author
|
CI is green (134 success, 2 skipped, 0 failures). Ready for review when you have a moment, @bartlomieju. Same CLA blocker as #33978: |
crowlKats
self-requested a review
May 14, 2026 17:00
crowlKats
approved these changes
Jun 9, 2026
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.
The
Uint32Arrayfast path ofsetBindGroupincompute_pass.rs,render_pass.rsandrender_bundle.rshad two related bugs (#33956):&data[start..(start + len)]sliced unchecked. An out-of-range length panicked inside the op'sextern "C"callback, and the panic crossed the C ABI as a process abort — six lines of JS could kill a running runtime. This PR adds an explicit bounds check that surfaces as aGPUValidationError(or a thrown JS error in the render-bundle path, which has no per-callerror_handler) instead.uint_32.buffer(scope).data()+ab.byte_length() / 4walked the backingArrayBufferand ignored the typed-array view'sbyteOffsetandlength. AUint32Array(buf, byteOffset > 0, …)fedwgpu_corethe wrong window with no error surfaced — silently incorrect rendering / compute results. The slice is now constructed frombyte_offset()+length().Adds a regression test (gated on
!isCIWithoutGPU, like the other webgpu tests) that:setBindGroup(…, new Uint32Array(4), 0, 1_000_000)against both aComputePass(expects a validation error in the device error scope) and aRenderBundleEncoder(expects a thrown JS error), anddeno fmt --checkanddeno lintare clean on the changed files; the Rust changes will get their fmt/clippy from CI.Out of scope: a view-window behavioural test (
Uint32Array(buf, byteOffset=16, length=4)producing the right wgpu binding) needs a full pipeline + buffer + dispatch + readback to observe; the reporter's standalone Rust harness in #33956 demonstrates the semantics of the fix.Credit to @hibrian827 for the diagnosis, three reproducers, and the suggested fix in the upstream report.
Fixes #33956.
@bartlomieju
🤖 Filed by
agor(workerwork2-2, botlunadogbot). Tracking claim: bartlomieju/agent-job-board#24. (Note:@lunadogbotstill needs CLA acceptance from the operator side, same as #33978.)