8000 texSubImage2D crashed on iOS16 + WebGL2 + large WASM memory by kkinnunen-apple · Pull Request #9451 · WebKit/WebKit · GitHub
[go: up one dir, main page]
Skip to content

texSubImage2D crashed on iOS16 + WebGL2 + large WASM memory#9451

Merged
webkit-early-warning-system merged 1 commit intoWebKit:mainfrom
kkinnunen-apple:webgl-ipc-texupload-too-big-memory-1
Feb 2, 2023
Merged

texSubImage2D crashed on iOS16 + WebGL2 + large WASM memory#9451
webkit-early-warning-system merged 1 commit intoWebKit:mainfrom
kkinnunen-apple:webgl-ipc-texupload-too-big-memory-1

Conversation

@kkinnunen-apple
Copy link
Contributor
@kkinnunen-apple kkinnunen-apple commented Feb 1, 2023 •
edited by webkit-early-warning-system
Loading

905fdd7

texSubImage2D crashed on iOS16 + WebGL2 + large WASM memory
https://bugs.webkit.org/show_bug.cgi?id=250558
rdar://104305743

Reviewed by Matt Woodrow.

Adjust the input size from the ArrayBufferView byteLength
to the actual image data size texSubImage2D would use.
Fixes crashes where IPC would copy the whole ArrayBufferView into
encoding buffer, causing the WP process size increase.

* Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::validateTexFuncData):

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

1cc3b31

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@kkinnunen-apple kkinnunen-apple self-assigned this Feb 1, 2023
@kkinnunen-apple kkinnunen-apple added the WebGL Bugs in WebKit’s implementation of the WebGL standard. label Feb 1, 2023
@kkinnunen-apple kkinnunen-apple requested review from djg and grorg February 1, 2023 14:51
@webkit-early-warning-system
Copy link
Collaborator
webkit-early-warning-system commented Feb 1, 2023 •
edited
Loading

EWS run on current version of this PR (hash 1cc3b31)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

djg
djg approved these changes Feb 1, 2023
View reviewed changes
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.

🚢

@djg djg added the merge-queue Applied to send a pull request to merge-queue label Feb 2, 2023
@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 Feb 2, 2023
@kkinnunen-apple kkinnunen-apple added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Feb 2, 2023
@kkinnunen-apple
texSubImage2D crashed on iOS16 + WebGL2 + large WASM memory
905fdd7 
https://bugs.webkit.org/show_bug.cgi?id=250558
rdar://104305743

Reviewed by Matt Woodrow.

Adjust the input size from the ArrayBufferView byteLength
to the actual image data size texSubImage2D would use.
Fixes crashes where IPC would copy the whole ArrayBufferView into
encoding buffer, causing the WP process size increase.

* Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::validateTexFuncData):

Canonical link: https://commits.webkit.org/259742@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the webgl-ipc-texupload-too-big-memory-1 branch from 1cc3b31 to 905fdd7 Compare February 2, 2023 08:39
@webkit-commit-queue
Copy link
Collaborator
webkit-commit-queue commented Feb 2, 2023

Committed 259742@main (905fdd7): https://commits.webkit.org/259742@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 905fdd7 into WebKit:main Feb 2, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 2, 2023
@juj
Copy link
juj commented May 12, 2023

Hi @kkinnunen-apple , this bug is critically affecting Unity content. Do you know which version of released Safari would first have a fix for this? (or also, which version of Safari might be the first one that got this regression)

@kkinnunen-apple
Copy link
Contributor Author
kkinnunen-apple commented May 22, 2023

Hi @kkinnunen-apple , this bug is critically affecting Unity content. Do you know which version of released Safari would first have a fix for this? (or also, which version of Safari might be the first one that got this regression)

The fix shipped in iOS 16.4, macOS 12.3, Safari 16.4

@juj juj mentioned this pull request Feb 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@djg djg djg approved these changes

@mattwoodrow mattwoodrow mattwoodrow approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

@grorg grorg Awaiting requested review from grorg

Assignees

@kkinnunen-apple kkinnunen-apple

Labels

WebGL Bugs in WebKit’s implementation of the WebGL standard.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@kkinnunen-apple @webkit-early-warning-system @webkit-commit-queue @juj @djg @mattwoodrow @webkit-ews-buildbot
0