Skip to content

Size in bytes to clobber is not aligned on 32 bit OMGIRGenerator's prepareForTailCallImpl #54173

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
webkit-commit-queue merged 1 commit into from
Nov 25, 2025
Merged

Size in bytes to clobber is not aligned on 32 bit OMGIRGenerator's prepareForTailCallImpl #54173

webkit-commit-queue merged 1 commit into from
Nov 25, 2025

Conversation

@ioannad
Copy link
Contributor

@ioannad ioannad commented Nov 19, 2025 •
edited by webkit-early-warning-system
Loading

9f179c4

Size in bytes to clobber is not aligned on 32 bit OMGIRGenerator's prepareForTailCallImpl
https://bugs.webkit.org/show_bug.cgi?id=302776

Reviewed by Justin Michaud.

This bug causes a build failure on 32 bit systems.

Define the stackSlotsToClobber as a multiple of stackAlignmentBytes() to ensure that the stackBytesToClobber are always aligned on the stack.

* Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp:
(JSC::Wasm::prepareForTailCallImpl):

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

76bce51

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   🧪 api-mac-debug ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim   🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge   🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 tv   🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@ioannad ioannad requested a review from a team as a code owner November 19, 2025 12:59
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 19, 2025 •
edited
Loading

EWS run on previous version of this PR (hash ec5babc)

Details

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ⏳ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

justinmichaud
justinmichaud reviewed Nov 19, 2025
View reviewed changes
Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp Outdated
jit.pushPair(tmp, tmp);
jit.move(MacroAssembler::TrustedImm32(0xBFFF), tmp);
constexpr int stackSlotsToClobber = 50;
// Choose a number of stack slots to clobber that is aligned.
Copy link
Contributor

@justinmichaud justinmichaud Nov 19, 2025

Choose a reason for hiding this comment

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

I don't think we need the comment

Copy link
Contributor Author

@ioannad ioannad Nov 21, 2025

Choose a reason for hiding this comment

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

Removed.

@ioannad ioannad force-pushed the eng/Size-in-bytes-to-clobber-is-not-aligned-on-32-bit-OMGIRGenerator-s-prepareForTailCallImpl branch from ec5babc to 7abdcf6 Compare November 21, 2025 11:17
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 7abdcf6)

Details

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
❌ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@justinmichaud justinmichaud added the merge-queue Applied to send a pull request to merge-queue label Nov 21, 2025
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Nov 21, 2025

Commit message contains (OOPS!), blocking PR #54173. Details: Build #27791

@webkit-commit-queue webkit-commit-queue 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 Nov 21, 2025
@ioannad ioannad force-pushed the eng/Size-in-bytes-to-clobber-is-not-aligned-on-32-bit-OMGIRGenerator-s-prepareForTailCallImpl branch from 7abdcf6 to 76bce51 Compare November 25, 2025 10:10
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 25, 2025 •
edited
Loading

EWS run on current version of this PR (hash 76bce51)

Details

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   🧪 api-mac-debug ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim   🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge   🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 tv   🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@aoikonomopoulos aoikonomopoulos 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 Nov 25, 2025
@ioannad @aoikonomopoulos
Size in bytes to clobber is not aligned on 32 bit OMGIRGenerator's pr…
9f179c4 
…epareForTailCallImpl

https://bugs.webkit.org/show_bug.cgi?id=302776

Reviewed by Justin Michaud.

This bug causes a build failure on 32 bit systems.

Define the stackSlotsToClobber as a multiple of stackAlignmentBytes() to ensure that the stackBytesToClobber are always aligned on the stack.

* Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp:
(JSC::Wasm::prepareForTailCallImpl):

Canonical link: https://commits.webkit.org/303530@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Size-in-bytes-to-clobber-is-not-aligned-on-32-bit-OMGIRGenerator-s-prepareForTailCallImpl branch from 76bce51 to 9f179c4 Compare November 25, 2025 10:57
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Nov 25, 2025

Committed 303530@main (9f179c4): https://commits.webkit.org/303530@main

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

@webkit-commit-queue webkit-commit-queue merged commit 9f179c4 into WebKit:main Nov 25, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 25, 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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ioannad @webkit-early-warning-system @webkit-commit-queue @justinmichaud @aoikonomopoulos