Skip to content

wasm->wasm and wasm->js thunk executable allocation should be JITCompilationMustSucceed #47387

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 30, 2025
Merged

wasm->wasm and wasm->js thunk executable allocation should be JITCompilationMustSucceed #47387

merged 1 commit into from
Jun 30, 2025

Conversation

danlliu
Copy link
Contributor

@danlliu danlliu commented Jun 30, 2025 •
edited by webkit-early-warning-system
Loading

66fbef3

wasm->wasm and wasm->js thunk executable allocation should be JITCompilationMustSucceed
https://bugs.webkit.org/show_bug.cgi?id=295218
rdar://154519254

Reviewed by Justin Michaud and Mark Lam.

This fixes some behavior with Wasm tests where executable allocation
fuzzing leads to Wasm modules exiting early due to a pretend failing
allocation. These thunks aren't very large, and we shouldn't fail to
allocate these unless the entire system is OOM anyway.

* JSTests/wasm/lowExecutableMemory/imports-oom.js:
* Source/JavaScriptCore/wasm/WasmBinding.cpp:
(JSC::Wasm::wasmToWasm):
* Source/JavaScriptCore/wasm/js/WasmToJS.cpp:
(JSC::Wasm::wasmToJS):

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

16b9c33

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc   🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@danlliu danlliu requested a review from a team as a code owner June 30, 2025 19:27
@danlliu danlliu self-assigned this Jun 30, 2025
@danlliu danlliu added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jun 30, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2025 •
edited
Loading

EWS run on current version of this PR (hash 16b9c33)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc   🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

MenloDorian
MenloDorian approved these changes Jun 30, 2025
View reviewed changes
Copy link

@MenloDorian MenloDorian left a comment

Choose a reason for hiding this comment

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

r=me too

@danlliu danlliu added the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
@danlliu
wasm->wasm and wasm->js thunk executable allocation should be JITComp…
66fbef3 
…ilationMustSucceed

https://bugs.webkit.org/show_bug.cgi?id=295218
rdar://154519254

Reviewed by Justin Michaud and Mark Lam.

This fixes some behavior with Wasm tests where executable allocation
fuzzing leads to Wasm modules exiting early due to a pretend failing
allocation. These thunks aren't very large, and we shouldn't fail to
allocate these unless the entire system is OOM anyway.

* JSTests/wasm/lowExecutableMemory/imports-oom.js:
* Source/JavaScriptCore/wasm/WasmBinding.cpp:
(JSC::Wasm::wasmToWasm):
* Source/JavaScriptCore/wasm/js/WasmToJS.cpp:
(JSC::Wasm::wasmToJS):

Canonical link: https://commits.webkit.org/296832@main
@webkit-commit-queue webkit-commit-queue force-pushed the 154519254-wasm-thunk-alloc branch from 16b9c33 to 66fbef3 Compare June 30, 2025 22:22
@webkit-commit-queue
Copy link
Collaborator

Committed 296832@main (66fbef3): https://commits.webkit.org/296832@main

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

@webkit-commit-queue webkit-commit-queue merged commit 66fbef3 into WebKit:main Jun 30, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinmichaud justinmichaud justinmichaud approved these changes

@MenloDorian MenloDorian MenloDorian approved these changes

Assignees

@danlliu danlliu

Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@danlliu @webkit-early-warning-system @webkit-commit-queue @justinmichaud @MenloDorian