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

[WebGPU] RenderBundleEncoder::endCurrentICB may assert in ASAN builds on invalid bundle encoder #28668

Conversation

mwyrzykowski
Copy link
Contributor

@mwyrzykowski mwyrzykowski commented May 16, 2024

21e320d

[WebGPU] RenderBundleEncoder::endCurrentICB may assert in ASAN builds on invalid bundle encoder
https://bugs.webkit.org/show_bug.cgi?id=274271
<radar://128064942>

Reviewed by Dan Glastonbury.

There is an ASAN assert in Vector::grow(sz) to ensure sz >= the current size,
which will not be true when replaying commands from an invalid RenderBundleEncoder.

But we shouldn't replay commands from an invalid RenderBundleEncoder in the first place,
so return early if the encoder is invalid.

* LayoutTests/fast/webgpu/fuzz-274271-expected.txt: Added.
* LayoutTests/fast/webgpu/fuzz-274271.html: Added.
* LayoutTests/TestExpectations:
Add regression test.

* Source/WebGPU/WebGPU/RenderBundleEncoder.h:
(WebGPU::RenderBundleEncoder::isValid const): Deleted.
* Source/WebGPU/WebGPU/RenderBundleEncoder.mm:
(WebGPU::RenderBundleEncoder::endCurrentICB):
(WebGPU::RenderBundleEncoder::isValid const):
(WebGPU::RenderBundleEncoder::replayCommands):

Canonical link: https://commits.webkit.org/278997@main

0abeda4

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2   πŸ§ͺ wincairo-tests
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ webkitpy ❌ πŸ§ͺ ios-wk2-wpt ❌ πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-cairo
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  πŸ§ͺ jsc-arm64 βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ api-gtk
  πŸ›  πŸ§ͺ merge βœ… πŸ›  watch βœ… πŸ›  jsc-armv7
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-armv7-tests

@mwyrzykowski mwyrzykowski self-assigned this May 16, 2024
@mwyrzykowski mwyrzykowski added the WebGPU For bugs in WebGPU label May 16, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-RenderBundleEncoderendCurrentICB-may-assert-in-ASAN-builds-on-invalid-bundle-encoder branch from 1fc727f to bfdd297 Compare May 16, 2024 19:11
@mwyrzykowski mwyrzykowski requested a review from djg May 17, 2024 23:19
Copy link
Contributor

@djg djg left a comment

Choose a reason for hiding this comment

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

:shipit:

@mwyrzykowski mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label May 18, 2024
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebGPU-RenderBundleEncoderendCurrentICB-may-assert-in-ASAN-builds-on-invalid-bundle-encoder branch from bfdd297 to a14271a Compare May 18, 2024 16:17
@mwyrzykowski mwyrzykowski removed the merge-queue Applied to send a pull request to merge-queue label May 20, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-RenderBundleEncoderendCurrentICB-may-assert-in-ASAN-builds-on-invalid-bundle-encoder branch from a14271a to 0abeda4 Compare May 20, 2024 15:02
@mwyrzykowski mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label May 20, 2024
@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels May 20, 2024
@mwyrzykowski mwyrzykowski added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels May 20, 2024
… on invalid bundle encoder

https://bugs.webkit.org/show_bug.cgi?id=274271
<radar://128064942>

Reviewed by Dan Glastonbury.

There is an ASAN assert in Vector::grow(sz) to ensure sz >= the current size,
which will not be true when replaying commands from an invalid RenderBundleEncoder.

But we shouldn't replay commands from an invalid RenderBundleEncoder in the first place,
so return early if the encoder is invalid.

* LayoutTests/fast/webgpu/fuzz-274271-expected.txt: Added.
* LayoutTests/fast/webgpu/fuzz-274271.html: Added.
* LayoutTests/TestExpectations:
Add regression test.

* Source/WebGPU/WebGPU/RenderBundleEncoder.h:
(WebGPU::RenderBundleEncoder::isValid const): Deleted.
* Source/WebGPU/WebGPU/RenderBundleEncoder.mm:
(WebGPU::RenderBundleEncoder::endCurrentICB):
(WebGPU::RenderBundleEncoder::isValid const):
(WebGPU::RenderBundleEncoder::replayCommands):

Canonical link: https://commits.webkit.org/278997@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebGPU-RenderBundleEncoderendCurrentICB-may-assert-in-ASAN-builds-on-invalid-bundle-encoder branch from 0abeda4 to 21e320d Compare May 20, 2024 16:57
@webkit-commit-queue
Copy link
Collaborator

Committed 278997@main (21e320d): https://commits.webkit.org/278997@main

Reviewed commits have been landed. Closing PR #28668 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 21e320d into WebKit:main May 20, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebGPU For bugs in WebGPU
Projects
None yet
5 participants