Upgrade WebKit to 6d586e29#31724
Conversation
|
Updated 3:01 PM PT - Jun 2nd, 2026
❌ @dylan-conway, your commit c17326d has 8 failures in
🧪 To try this PR locally: bunx bun-pr 31724That installs a local version of the PR into your bun-31724 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR updates the ChangesWebKit version update
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — routine WebKit pin bump with thorough range notes; the inline nit about the now-stale windows-baseline LTO gate in config.ts is worth a follow-up but non-blocking.
Extended reasoning...
Overview
This PR changes a single line: the WEBKIT_VERSION constant in scripts/build/deps/webkit.ts, bumping the pinned oven-sh/WebKit commit from 963f8758 to 6d586e29. The range is 6 fork-local commits with no upstream WebKit merge, and the PR description enumerates each one along with the standard bump checks (no JSType.h changes → no JSType.rs update; all prebuiltSuffix() tarball names present in the published release).
Security risks
None introduced by the bun-side diff itself — it only changes which prebuilt tarball is fetched from the oven-sh/WebKit GitHub releases (same trusted origin, same fetch mechanism). The pulled-in WebKit commits are bug fixes (UB in double→int conversions, a debug-assert bound), a release-build code-size reduction (compile out JIT disassembler), and CI/packaging changes (new Windows ASAN and baseline-LTO artifacts, xwin update). Nothing in the range touches auth, crypto, or permission surfaces.
Level of scrutiny
Version-pin bumps are explicitly in the "mechanical" category, and WebKit bumps are routine maintenance in this repo. The real validation is CI (build #59960), which exercises the new prebuilts across platforms. The author has already done the manual checks that CI cannot (JSType.h diff, release-asset presence). I treated this as low-scrutiny on the bun side while spot-checking that the constant is the only consumer-visible change.
Other factors
The bug-hunter flagged one nit: commit c787a5a7 in this range adds the windows-amd64-baseline-lto artifact, so the hard lto = false override for (windows && baseline) at scripts/build/config.ts:769 — and the two comments justifying it — are now stale. I verified those lines still exist as described. This is a missed-optimization / stale-comment issue, not a correctness regression (builds still succeed, just without LTO for that one config), so it does not block approval and can be cleaned up in a follow-up.
| // importing), and the Windows ICU data table filtered + per-item zstd | ||
| // compressed (lazily decompressed via bun_icu_decompress.cpp). | ||
| export const WEBKIT_VERSION = "963f8758c29e965471c191668d5776a1a1b014b6"; | ||
| export const WEBKIT_VERSION = "6d586e293f008f0e74e5697611a379b1b24815c9"; |
There was a problem hiding this comment.
🟡 This bump includes c787a5a7 ("windows cross: add the amd64-baseline ThinLTO variant"), so bun-webkit-windows-amd64-baseline-lto.tar.gz now exists in the pinned release. However, scripts/build/config.ts:769 still hard-forces lto = false for (windows && baseline), and the comments at config.ts:746-748 and config.ts:767-768 still claim the pinned WEBKIT_VERSION lacks/predates the -baseline-lto variant — both are now factually wrong. Consider dropping || baseline from the gate (or at minimum updating the comments) alongside this bump.
Extended reasoning...
What the issue is
scripts/build/config.ts contains a hard override that forces LTO off for Windows baseline builds, with an inline rationale tied directly to the pinned WebKit version:
// config.ts:765-771
// Windows arm64 / baseline: same — oven-sh/WebKit ships no
// bun-webkit-windows-arm64-lto (LLVM's CodeView emitter aborts on ARM64
// NEON tuple registers during LTO codegen), and the pinned WEBKIT_VERSION
// predates the -baseline-lto variant.
if ((asan && lto) || abi === "android" || (windows && (arm64 || baseline))) {
lto = false;
}And earlier:
// config.ts:746-748
// Resolved early because the LTO defaults below need it (the windows
// -baseline WebKit prebuilt has no -lto variant).
const baseline = partial.baseline ?? false;Both comments assert that the pinned WebKit release lacks a windows-amd64-baseline-lto artifact. That was true at 963f8758, but this PR bumps WEBKIT_VERSION to 6d586e29, whose range includes commit c787a5a7 "windows cross: add the amd64-baseline ThinLTO variant". The release autobuild-6d586e29… therefore does publish bun-webkit-windows-amd64-baseline-lto.tar.gz.
Step-by-step proof
- Before this PR,
WEBKIT_VERSION = "963f8758…". Theautobuild-963f8758…release has no…-windows-amd64-baseline-lto.tar.gz, so the gate atconfig.ts:769is necessary — without it,prebuiltSuffix()would emit-baseline-ltoand the download would 404. - This PR changes
WEBKIT_VERSIONto"6d586e29…". Per the PR description, that range includesc787a5a7"windows cross: add the amd64-baseline ThinLTO variant", and the linked release page now contains the-baseline-ltoasset. config.tsis not touched by this PR, so line 769 still reads(windows && (arm64 || baseline))and line 768 still says "the pinned WEBKIT_VERSION predates the -baseline-lto variant".- A user runs a Windows amd64 baseline cross build with an explicit
--lto=on. The conditionwindows && baselinematches, soltois silently forced tofalse.prebuiltSuffix()(webkit.ts:62-74) then returns-baselineinstead of-baseline-lto, fetching the non-LTO artifact even though the LTO one exists. - The PR's own check — "all tarball names
prebuiltSuffix()produces are present in the release" — cannot catch this, because the gate makes the-baseline-ltosuffix unreachable from any config.
Why nothing else justifies the gate
The ThinLTO miscompile concern at config.ts:750-760 only removes Windows from the LTO default (ltoDefault); explicit --lto=on is still permitted for non-baseline Windows x64. So the only documented reason for the baseline-specific hard override is the missing artifact — and that reason is invalidated by this exact bump. The arm64 half of the gate has its own separate (still-valid) rationale about the CodeView emitter.
Impact
No correctness regression — builds still succeed and produce a working binary. The effects are:
- An explicit
--lto=onfor Windows amd64 baseline is silently downgraded with no warning, even though the matching prebuilt now exists. - Two comments in
config.tsmake a factual claim about the pinnedWEBKIT_VERSIONthat becomes false the moment this PR lands, which will confuse the next person reading this code.
Suggested fix
Alongside this bump, either:
- Drop
|| baselinefrom the(windows && (arm64 || baseline))clause atconfig.ts:769and update both comments accordingly, or - At minimum, rewrite the comments at
config.ts:746-748andconfig.ts:767-768so they no longer claim the pinned version lacks the artifact (and explain why the gate remains, if it should).
…bKit LTO flags
parseInt results, Map keys, and switch-immediate scrutinees at or above
2^31 came back as sign-wrapped int32s on official Linux x64 release
builds once the call site tiered up to the DFG: JSC had out-of-range
double->int casts (undefined behavior) in the round-trip checks that
decide int32 boxing/dispatch, and the LLVM 22 LTO backend (rust-lld,
via the cross-language LTO pipeline) folds those checks into bare
integrality tests. parseInt("80000000", 16) === -2147483648 after
~10k hot calls, persisting for the life of the process; Map keys
(issue #31080) and switch dispatch had the same failure mode. Fresh
processes replayed clean because lower tiers box through the hardened
paths.
The source fixes landed in oven-sh/WebKit#244 and reached main with the
WebKit upgrade in #31724. This adds the regression coverage: tests that
hammer all three paths past JIT tier-up (they fail in ~130 ms on the
last broken canary and run on CI's Linux x64 release lanes, which build
with LTO — debug builds cannot exhibit the fold).
Also forward the artifact builders' LTO flags (-flto=full
-fwhole-program-vtables -fforce-emit-vtables) to local Linux/FreeBSD
WebKit builds: with plain -flto=thin, --webkit=local --lto=on fails to
link with 'inconsistent LTO Unit splitting' because bun-profile links
with -fwhole-program-vtables. This is how the fix was validated
end-to-end before prebuilt artifacts existed.
Fixes #31080
3 participants
Bumps
WEBKIT_VERSIONfrom963f8758to6d586e29, the current head of oven-sh/WebKit main. The prebuilt releaseautobuild-6d586e293f008f0e74e5697611a379b1b24815c9is published with all platform tarballs.This range is a fast-forward of 6 fork-local commits — no upstream WebKit merge:
27f4e7a9Fix UB in double-to-int conversions across JSC (Fix UB in double-to-int conversions across JSC WebKit#244)f18cf9c2FreeList::forEach: bound the interval assert by MarkedBlock::blockSize (FreeList::forEach: bound the interval assert by MarkedBlock::blockSize WebKit#247)48460d87Compile out the JIT disassembler in release builds (Compile out the JIT disassembler in release builds WebKit#241)6d586e29windows cross: add the bun-webkit-windows-amd64-asan variant (windows cross: add the bun-webkit-windows-amd64-asan prebuilt WebKit#240)c787a5a7windows cross: add the amd64-baseline ThinLTO variantc591a185windows: update xwin to 0.9.0 (windows: update xwin to 0.9.0 WebKit#242)Checks:
Source/JavaScriptCore/runtime/JSType.hchanges in the range, so nosrc/jsc/JSType.rsupdate needed.bun-webkit-windows-amd64-asan.tar.gzasset; all tarball namesprebuiltSuffix()produces are present in the release.