Skip to content

Disable libpas JIT heap on Windows ARM64 - #31137

Open
sosukesuzuki wants to merge 1 commit into
mainfrom
claude/disable-libpas-jit-heap-windows-arm64
Open

Disable libpas JIT heap on Windows ARM64#31137
sosukesuzuki wants to merge 1 commit into
mainfrom
claude/disable-libpas-jit-heap-windows-arm64

Conversation

@sosukesuzuki

Copy link
Copy Markdown
Contributor

What does this PR do?

Disables the libpas JIT heap on Windows ARM64 by pointing WEBKIT_VERSION at the preview build of oven-sh/WebKit#234.

Since #30705 upgraded WebKit (which pulled in upstream af63dbd0b5d "[libpas] Enable JIT-heap on Windows"), Windows ARM64 CI has been intermittently crashing with illegal instruction on DFG worker threads — observed across test/js/node/test/parallel/*.js and other random tests in 19+ CI builds since build 55719.

The crash is a PAS_ASSERT firing inside jit_heap_try_allocate -> pas_segregated_heap_ensure_allocator_index. On Windows ARM64, libpas's __builtin_trap() compiles to brk #1, which the OS reports as STATUS_ILLEGAL_INSTRUCTION rather than a breakpoint, so the assertion failure surfaces as a flaky illegal-instruction panic.

Windows x64 has not exhibited the same failure across the same range of CI builds (55400–56380, zero illegal-instruction panics), so the WebKit-side change keeps the libpas JIT heap enabled on x64 and falls back to the legacy MetaAllocator path on Windows ARM64 only, until the assertion can be root-caused.

Notes

  • This points at the preview build autobuild-preview-pr-234-c8f7dc7f. Once Disable libpas JIT heap on Windows ARM64 WebKit#234 merges, WEBKIT_VERSION should be updated to the final merge SHA.
  • The rest of the upstream af63dbd0b5d change (the pas_mmap_capability -> pas_page_flags refactor and the executable-protection plumbing in pas_page_malloc) remains intact; with ENABLE_LIBPAS_JIT_HEAP off on ARM64, jit_heap_config is unused there and pas_page_flag_executable is never set, so those paths are inert.

How did you verify your code works?

  • Disable libpas JIT heap on Windows ARM64 WebKit#234 CI is green across all platforms.
  • Windows ARM64 + x64 lanes in this PR's CI should be the real signal — the goal is for Windows ARM64 test runs to stop hitting the illegal-instruction panic, while x64 behavior is unchanged.

Windows ARM64 has been intermittently crashing with 'illegal instruction'
on DFG worker threads since the WebKit upgrade in #30705 pulled in
upstream af63dbd0b5d ([libpas] Enable JIT-heap on Windows). The crash is
a PAS_ASSERT inside jit_heap_try_allocate ->
pas_segregated_heap_ensure_allocator_index; on ARM64 Windows libpas's
__builtin_trap() compiles to brk #1, which the OS reports as
STATUS_ILLEGAL_INSTRUCTION rather than a breakpoint, so the assertion
failure surfaces as a flaky illegal-instruction panic.

Windows x64 has not exhibited the same failure across the same range of
CI builds, so oven-sh/WebKit#234 disables the libpas JIT heap on Windows
ARM64 only and falls back to the legacy MetaAllocator path there. This
points WEBKIT_VERSION at that PR's preview build until it merges.
@github-actions

Copy link
Copy Markdown
Contributor

Found 2 issues this PR may fix:

  1. Crash in DFG JIT: Illegal instruction in pas_utils.h (WTF::Vector<DFG::AbstractValue> resize) #29412 - Crash in DFG JIT with illegal instruction in pas_utils.h — same libpas JIT heap assertion path (PAS_ASSERT) that this PR disables on Windows ARM64
  2. webkit: re-add -DUSE_SYSTEM_MALLOC=ON to Windows prebuilts #28101 - Request to re-add -DUSE_SYSTEM_MALLOC=ON to Windows prebuilts due to bmalloc/libpas crashes — this PR's targeted disabling of ENABLE_LIBPAS_JIT_HEAP on Windows ARM64 is a partial fix for the same class of issues

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #29412
Fixes #28101

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR updates the WEBKIT_VERSION constant in the WebKit build dependency configuration from a commit hash to an autobuild-preview PR tag, changing the prebuilt release identifier used by the build system.

Changes

WebKit Version Update

Layer / File(s) Summary
WebKit version tag update
scripts/build/deps/webkit.ts
WEBKIT_VERSION constant updated from 3167a44fb92c268c83f09b232b38a9f3e7f9655a to autobuild-preview-pr-234-c8f7dc7f, affecting prebuilt URL construction and identity computations.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Disable libpas JIT heap on Windows ARM64' directly describes the main objective of the PR, which is to disable the libpas JIT heap specifically on Windows ARM64 by updating WEBKIT_VERSION.
Description check ✅ Passed The PR description fully covers both required template sections with comprehensive explanations of what the PR does and how it was verified, exceeding the template requirements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/build/deps/webkit.ts`:
- Line 6: The WEBKIT_VERSION constant is pointing to a temporary preview build
that lacks macOS and Windows artifacts; do not merge as-is — wait for
oven-sh/WebKit#234 to land and then update the WEBKIT_VERSION export (symbol:
WEBKIT_VERSION) to the final merge SHA string for that PR so the build system
can fetch artifacts for all platforms; replace the current
"autobuild-preview-..." value with the final commit/merge SHA once available and
ensure the change is committed before merging.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c1d6c0c8-2e42-4cbf-9e61-d58fb8fc8e39

📥 Commits

Reviewing files that changed from the base of the PR and between a43a01b and b90db22.

📒 Files selected for processing (1)
  • scripts/build/deps/webkit.ts

* From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "3167a44fb92c268c83f09b232b38a9f3e7f9655a";
export const WEBKIT_VERSION = "autobuild-preview-pr-234-c8f7dc7f";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify that the preview build URLs are accessible for common platform combinations.
# Expected: HTTP 200 or 302 redirect for each artifact URL.

version="autobuild-preview-pr-234-c8f7dc7f"

# Test a representative subset of platform/arch/variant combinations
artifacts=(
  "bun-webkit-linux-amd64-lto.tar.gz"
  "bun-webkit-linux-arm64-lto.tar.gz"
  "bun-webkit-macos-amd64-lto.tar.gz"
  "bun-webkit-macos-arm64-lto.tar.gz"
  "bun-webkit-windows-amd64-lto.tar.gz"
  "bun-webkit-windows-arm64-lto.tar.gz"
)

base_url="https://github.com/oven-sh/WebKit/releases/download/${version}"

for artifact in "${artifacts[@]}"; do
  url="${base_url}/${artifact}"
  echo "Checking: ${url}"
  http_code=$(curl -s -o /dev/null -w "%{http_code}" -L -I "${url}")
  if [[ "${http_code}" =~ ^(200|302)$ ]]; then
    echo "${http_code}"
  else
    echo "${http_code} (artifact may not exist)"
  fi
done

Repository: oven-sh/bun

Length of output: 987


The preview build is incomplete — missing artifacts for macOS and Windows platforms.

Verification confirms the preview build artifacts are unavailable for macOS (both amd64 and arm64) and Windows (both amd64 and arm64), returning HTTP 404. Only Linux artifacts (amd64 and arm64) are accessible. Since the build system requires artifacts for all platforms, this version cannot be used until the preview release is complete.

Additionally, per the PR objectives, update this to the final merge SHA once oven-sh/WebKit#234 lands—do not merge with the temporary preview version.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/build/deps/webkit.ts` at line 6, The WEBKIT_VERSION constant is
pointing to a temporary preview build that lacks macOS and Windows artifacts; do
not merge as-is — wait for oven-sh/WebKit#234 to land and then update the
WEBKIT_VERSION export (symbol: WEBKIT_VERSION) to the final merge SHA string for
that PR so the build system can fetch artifacts for all platforms; replace the
current "autobuild-preview-..." value with the final commit/merge SHA once
available and ensure the change is committed before merging.

* From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "3167a44fb92c268c83f09b232b38a9f3e7f9655a";
export const WEBKIT_VERSION = "autobuild-preview-pr-234-c8f7dc7f";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Heads-up: pointing WEBKIT_VERSION at a tag name instead of a SHA breaks scripts/sync-webkit-source.ts for the local-WebKit workflow — git rev-parse HEAD can never equal the tag string so the early-exit at line 17 is dead, and git pull on main won't auto-follow a preview tag pointing at an unmerged PR commit, so git checkout autobuild-preview-pr-234-c8f7dc7f at line 24 will fail with pathspec ... did not match on existing clones. Prebuilt/CI is fine (the autobuild- prefix is already handled in prebuiltUrl) and the PR notes this pin is temporary, so probably just worth mentioning that local-mode contributors will need a manual git fetch --tags until the merge SHA lands.

Extended reasoning...

What breaks

scripts/sync-webkit-source.ts is the convenience script for the documented local-WebKit workflow (CONTRIBUTING.md:278 — "Check out the commit hash specified in WEBKIT_VERSION"). It imports WEBKIT_VERSION and assumes it is a 40-char commit SHA. With this PR it is now the release-tag string autobuild-preview-pr-234-c8f7dc7f, which trips two assumptions in the script.

Code path

// scripts/sync-webkit-source.ts
14  const checkedOutCommit = (await Bun.$`git rev-parse HEAD`.text()).trim();
15  const { WEBKIT_VERSION: expectedCommit } = await import("./build/deps/webkit.ts");
17  if (checkedOutCommit == expectedCommit) { ... }   // (1)
21  await Bun.$`git checkout main`;
22  await Bun.$`git pull`;
24  await Bun.$`git checkout ${expectedCommit}`;      // (2)

(1) Dead early-exit. git rev-parse HEAD always returns a 40-char SHA, so comparing it against "autobuild-preview-pr-234-c8f7dc7f" can never succeed. Even when vendor/WebKit is already at the right commit, the script falls through to checkout main → pull → checkout. Minor, but it does extra network work every run.

(2) Checkout failure on existing clones. Line 22's git pull on main only auto-follows tags that point at commits being fetched into main's history (git's default fetch.tagopt behavior). The autobuild-preview-pr-234-* tag points at the head of an unmerged PR branch in oven-sh/WebKit, which is not reachable from main, so the tag is not fetched. Line 24 then runs git checkout autobuild-preview-pr-234-c8f7dc7f against a clone that has neither the tag nor the underlying object, and git fails with error: pathspec 'autobuild-preview-pr-234-c8f7dc7f' did not match any file(s) known to git.

Why nothing else catches this

The only other consumer of WEBKIT_VERSION is prebuiltUrl() in scripts/build/deps/webkit.ts, which already special-cases the autobuild- prefix when constructing the release-asset URL — so prebuilt mode and CI are unaffected. The doc comment at webkit.ts:1-4 and CONTRIBUTING.md still describe the constant as a "commit hash", but sync-webkit-source.ts is the only place that mechanically depends on the SHA shape.

Step-by-step proof

  1. Contributor has an existing vendor/WebKit clone at the previous SHA 3167a44f….
  2. They pull this PR and run bun scripts/sync-webkit-source.ts.
  3. Line 14: checkedOutCommit = "3167a44f…".
  4. Line 15: expectedCommit = "autobuild-preview-pr-234-c8f7dc7f".
  5. Line 17: "3167a44f…" == "autobuild-preview-pr-234-c8f7dc7f" → false.
  6. Line 21–22: checkout main, git pull — fetches main plus tags pointing into main's history; the preview tag points at PR chalk package cannot be imported #234's branch tip, so it is skipped.
  7. Line 24: git checkout autobuild-preview-pr-234-c8f7dc7f → ref unknown → pathspec did not match → script exits non-zero.

A fresh git clone would fetch all tags by default and so would succeed; the failure is specific to existing clones, which is the common case for active contributors.

Impact & severity

  • Only affects the opt-in local-WebKit dev script; CI and the default prebuilt path are fine.
  • The PR description already states the value is temporary pending Disable libpas JIT heap on Windows ARM64 WebKit#234 merging, after which it'll become a SHA again.
  • Workaround is trivial: (cd vendor/WebKit && git fetch --tags) before running the script.

So this is a nit — worth a heads-up since the PR description doesn't mention the local-mode side-effect, but not blocking.

Possible fix

If you want the script to keep working during the preview window, swap line 22 for git fetch --tags origin (or add it before the checkout) and compare against git rev-parse HEAD vs git rev-parse "${expectedCommit}^{commit}" so tag refs and SHAs both resolve. Otherwise, just noting the manual git fetch --tags step in the PR description is probably sufficient given the pin is short-lived.

@robobun

robobun commented May 20, 2026

Copy link
Copy Markdown
Collaborator
Updated 10:23 AM PT - May 20th, 2026

@sosukesuzuki, your commit b90db22 has 1 failures in Build #56421 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 31137

That installs a local version of the PR into your bun-31137 executable, so you can run:

bun-31137 --bun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants