-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add friendly error message for unsupported system page sizes #25050
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.
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
base: main
Are you sure you want to change the base?
Conversation
WebKit/JavaScriptCore uses hardcoded compile-time constants for maximum page sizes that vary by architecture. These constants are defined in WebKit's PageBlock.h (CeilingOnPageSize): - 4 KB: Windows, x86, x86_64, ARM, ARM64, RISC-V64 - 16 KB: Darwin/macOS, PlayStation, MIPS, MIPS64, LoongArch64, Linux ARM64 - 64 KB: PowerPC, or when USE(64KB_PAGE_BLOCK) is set If the actual system page size exceeds these limits, WebKit will crash with an assertion failure. This typically happens on: - Linux ARM64 systems with 64 KB pages (RHEL, Oracle Linux, etc.) - Systems configured with large pages for performance This commit adds an early check on startup that detects unsupported page sizes and exits with a friendly, actionable error message before WebKit has a chance to crash. The message explains: 1. What the problem is (page size mismatch) 2. Why it happens (WebKit hardcoded limits) 3. Possible solutions (use different system, reconfigure kernel, or build with USE(64KB_PAGE_BLOCK)) The check runs immediately after crash handler initialization to provide the best user experience. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughThe PR adds runtime validation of system page size in Changes
Pre-merge checks✅ Passed checks (2 passed)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main.zig(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{cpp,zig}
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
src/**/*.{cpp,zig}: Usebun bdorbun run build:debugto build debug versions for C++ and Zig source files; creates debug build at./build/debug/bun-debug
Run tests usingbun bd test <test-file>with the debug build; never usebun testdirectly as it will not include your changes
Execute files usingbun bd <file> <...args>; never usebun <file>directly as it will not include your changes
Enable debug logs for specific scopes usingBUN_DEBUG_$(SCOPE)=1environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required
Files:
src/main.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Use
bun.Output.scoped(.${SCOPE}, .hidden)for creating debug logs in Zig codeImplement core functionality in Zig, typically in its own directory in
src/
src/**/*.zig: Zig source code should be organized insrc/*.zigfor core runtime, JavaScript bindings, and package manager
In Zig code, be careful with allocators and use defer for cleanup to manage memory properly
UseBUN_DEBUG_QUIET_LOGS=1to disable debug logging, orBUN_DEBUG_<scopeName>=1to enable specific Output.scoped calls in debug builds
Runbun run zig:check-allto compile the Zig code on all platforms when making platform-specific changes
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/main.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
**/*.zig: Expose generated bindings in Zig structs usingpub const js = JSC.Codegen.JS<ClassName>with trait conversion methods:toJS,fromJS, andfromJSDirect
Use consistent parameter nameglobalObjectinstead ofctxin Zig constructor and method implementations
Usebun.JSError!JSValuereturn type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup usingdeinit()method that releases resources, followed byfinalize()called by the GC that invokesdeinit()and frees the pointer
UseJSC.markBinding(@src())in finalize methods for debugging purposes before callingdeinit()
For methods returning cached properties in Zig, declare external C++ functions usingextern fnandcallconv(JSC.conv)calling convention
Implement getter functions with naming patternget<PropertyName>in Zig that acceptthisandglobalObjectparameters and returnJSC.JSValue
Access JavaScript CallFrame arguments usingcallFrame.argument(i), check argument count withcallFrame.argumentCount(), and getthiswithcallFrame.thisValue()
For reference-counted objects, use.deref()in finalize instead ofdestroy()to release references to other JS objects
Files:
src/main.zig
src/**/*.{ts,zig,cpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use absolute paths in file operations
Files:
src/main.zig
src/**/*.{ts,zig}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid shell commands in code - don't use
findorgrep; use Bun's Glob and built-in tools instead
Files:
src/main.zig
🧠 Learnings (6)
📓 Common learnings
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
📚 Learning: 2025-11-24T18:35:39.196Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.196Z
Learning: Applies to **/js_*.zig : Use `bun.JSError!JSValue` for proper error propagation in JavaScript bindings
Applied to files:
src/main.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/main.zig
📚 Learning: 2025-11-24T18:36:33.049Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.049Z
Learning: Applies to src/**/*.zig : Run `bun run zig:check-all` to compile the Zig code on all platforms when making platform-specific changes
Applied to files:
src/main.zig
📚 Learning: 2025-11-24T18:36:08.548Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.548Z
Learning: Applies to **/*.zig : Use `bun.JSError!JSValue` return type for Zig methods and constructors to enable proper error handling and exception propagation
Applied to files:
src/main.zig
📚 Learning: 2025-09-02T17:41:07.869Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/memory.zig:60-76
Timestamp: 2025-09-02T17:41:07.869Z
Learning: In bun's memory utilities, when handling const pointers in deinit operations, prefer compile-time errors over silent skipping to avoid hard-to-find memory leaks. Users expect explicit failures rather than silent omissions in memory management.
Applied to files:
src/main.zig
🔇 Additional comments (1)
src/main.zig (1)
91-95: Code placement and logic are correct; proceed with requested verification steps locallyThe code inspection confirms:
- Placement:
checkPageSizeSupport()is called at line 94 inmain(), immediately aftercrash_handler.init()at line 89 (only comments between). This achieves the intended "fail fast before touching JSC/libuv" behavior.- Architecture mapping: The compile-time logic correctly distinguishes 16KB (macOS, AARCH64 Linux, PS4/PS5, MIPS), 64KB (PowerPC), and 4KB (Windows, x86, ARM, RISC-V) page sizes, with a conservative 64KB fallback.
- Runtime check: Compares actual page size via
std.heap.defaultQueryPageSize()against the expected maximum and exits with code 1 and a helpful error message on mismatch.- Syntax & logic: No errors detected; the Zig code is valid and sound.
The sandbox environment cannot execute
bun run zig:check-allor runtime tests. Please follow the verification steps in the original review comment locally:
- Run
bun run zig:check-allto verify multi-target compilation.- Build with
bun bdon representative platforms (Linux x64, Linux ARM64, macOS, Windows).- Test on a known unsupported configuration (e.g., Linux ARM64 with 64 KB pages) to confirm the exit behavior.
| /// Check if the system page size is supported by the WebKit/JavaScriptCore engine. | ||
| /// WebKit uses compile-time constants for page sizes that vary by architecture: | ||
| /// - 4 KB: Windows, x86, x86_64, ARM, ARM64, RISC-V64 | ||
| /// - 16 KB: Darwin/macOS, PlayStation, MIPS, MIPS64, LoongArch64, Linux ARM64 | ||
| /// - 64 KB: PowerPC variants, or when USE(64KB_PAGE_BLOCK) is set | ||
| /// | ||
| /// If the actual system page size exceeds these hardcoded limits, WebKit will crash. | ||
| /// This function provides a friendly error message before that happens. | ||
| fn checkPageSizeSupport() void { | ||
| // Determine the expected maximum page size based on the target architecture | ||
| // This mirrors the logic in WebKit's PageBlock.h (CeilingOnPageSize) | ||
| const arch = @import("builtin").target.cpu.arch; | ||
| const os_tag = @import("builtin").target.os.tag; | ||
|
|
||
| const expected_max_page_size: usize = comptime blk: { | ||
| // 16 KB architectures | ||
| if (os_tag == .macos or | ||
| os_tag == .ps4 or os_tag == .ps5 or | ||
| arch == .mips or arch == .mips64 or arch == .mips64el or arch == .mipsel or | ||
| arch == .loongarch64 or | ||
| (os_tag == .linux and arch.isAARCH64())) | ||
| { | ||
| break :blk 16 * 1024; // 16 KB | ||
| } | ||
|
|
||
| // 64 KB architectures | ||
| if (arch == .powerpc or arch == .powerpc64 or arch == .powerpc64le) { | ||
| break :blk 64 * 1024; // 64 KB | ||
| } | ||
|
|
||
| // 4 KB architectures (most common) | ||
| if (os_tag == .windows or | ||
| arch.isX86() or | ||
| arch == .arm or arch == .armeb or arch.isAARCH64() or | ||
| arch == .riscv64) | ||
| { | ||
| break :blk 4 * 1024; // 4 KB | ||
| } | ||
|
|
||
| // Unknown architecture - be conservative | ||
| break :blk 64 * 1024; // 64 KB | ||
| }; | ||
|
|
||
| // Get the actual system page size at runtime | ||
| const actual_page_size = @import("std").heap.defaultQueryPageSize(); | ||
|
|
||
| if (actual_page_size > expected_max_page_size) { | ||
| const kb_actual = actual_page_size / 1024; | ||
| const kb_expected = expected_max_page_size / 1024; | ||
|
|
||
| @import("bun").Output.prettyErrorln("<r><red>error<r>: Unsupported system page size", .{}); | ||
| @import("bun").Output.prettyErrorln("", .{}); | ||
| @import("bun").Output.prettyErrorln("Your system is configured with a page size of <b>{d} KB<r>, but Bun's JavaScript", .{kb_actual}); | ||
| @import("bun").Output.prettyErrorln("engine (based on WebKit/JavaScriptCore) was built to support a maximum page", .{}); | ||
| @import("bun").Output.prettyErrorln("size of <b>{d} KB<r> for this architecture.", .{kb_expected}); | ||
| @import("bun").Output.prettyErrorln("", .{}); | ||
| @import("bun").Output.prettyErrorln("This typically happens on systems configured with non-standard page sizes, such as:", .{}); | ||
| @import("bun").Output.prettyErrorln(" - Linux systems with 64 KB pages on ARM64 (RHEL, Oracle Linux, etc.)", .{}); | ||
| @import("bun").Output.prettyErrorln(" - Systems using large pages for performance tuning", .{}); | ||
| @import("bun").Output.prettyErrorln("", .{}); | ||
| @import("bun").Output.prettyErrorln("<b>Possible solutions:<r>", .{}); | ||
| @import("bun").Output.prettyErrorln(" 1. Use a system with a standard page size configuration", .{}); | ||
| @import("bun").Output.prettyErrorln(" 2. Reconfigure your kernel to use a smaller page size", .{}); | ||
| @import("bun").Output.prettyErrorln(" 3. Build Bun from source with USE(64KB_PAGE_BLOCK) enabled", .{}); | ||
| @import("bun").Output.prettyErrorln("", .{}); | ||
| @import("bun").Output.prettyErrorln("For more information, visit: <cyan>https://bun.sh/docs/project/development<r>", .{}); | ||
| @import("bun").Output.flush(); | ||
| @import("bun").Global.exit(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Wire expected_max_page_size to USE(64KB_PAGE_BLOCK) and double‑check early Output usage
The overall shape of checkPageSizeSupport() looks good and mirrors the WebKit CeilingOnPageSize logic by OS/arch, but there are two important points to address:
-
Missing override for
USE(64KB_PAGE_BLOCK)builds- The comments and user message explicitly reference
USE(64KB_PAGE_BLOCK), and “Possible solution Copy source lines when generating error messages #3” tells users they can rebuild Bun with that flag. - Right now,
expected_max_page_sizeis derived only fromarchandos_tag, so for e.g.linux/aarch64you always get16 * 1024, regardless of how WebKit/JavaScriptCore was actually compiled. - If someone does rebuild with
USE(64KB_PAGE_BLOCK)enabled, WebKit’sCeilingOnPageSizewill be 64 KB, but this check will still cap at 16 KB and will:- Reject a 64 KB page‑size system even though the engine supports it.
- Print a misleading “maximum page size of {d} KB” value in the error text.
- You should add a compile‑time override that bumps
expected_max_page_sizeto 64 KB when the WebKit build is configured withUSE(64KB_PAGE_BLOCK)(or your equivalent build flag), e.g. by consulting acomptimebool exported from your build options and short‑circuiting thecomptime blk:
- The comments and user message explicitly reference
- const expected_max_page_size: usize = comptime blk: {
- const expected_max_page_size: usize = comptime blk: {
-
// If WebKit was built with 64 KB page blocks, mirror that here. -
if (comptime use_64kb_page_block_flag) { -
break :blk 64 * 1024; -
} -
// 16 KB architectures if (os_tag == .macos or os_tag == .ps4 or os_tag == .ps5 or arch == .mips or arch == .mips64 or arch == .mips64el or arch == .mipsel or arch == .loongarch64 or (os_tag == .linux and arch.isAARCH64())) { break :blk 16 * 1024; // 16 KB } // ...Replace `use_64kb_page_block_flag` with whatever compile‑time hook you already expose from your WebKit/JSC build configuration. Without this wiring, solution (3) in the error message is misleading.
-
Using
bun.OutputbeforeOutput.Source.Stdio.init()checkPageSizeSupport()calls@import("bun").Output.prettyErrorln/flushbeforemain()has executedOutput.Source.Stdio.init()and installed the usualdefer Output.flush();.- This is probably fine if
prettyErrorlnonly depends on the raw stderr handle, but if any Output internals assumeSource.Stdiohas been initialized, you could get platform‑specific weirdness (or missing text) exactly in the error path you’re trying to improve. - Please verify this path on at least Windows and macOS in addition to Linux; if initialization is required, consider:
- Either initializing the minimal Output backend needed for stderr before calling
checkPageSizeSupport(), or - Falling back to a simpler
std.debug.print/std.io.getStdErr()message in this one early‑startup error path.
- Either initializing the minimal Output backend needed for stderr before calling
-
Minor maintainability nits (optional)
- Since you already have
const std = @import("std"); const _bun = @import("bun");at the bottom, you could reuse them here instead of repeating@import("std")/@import("bun")within the function. This would make the imports more consistent across the file. - It might also be helpful to include the detected
arch/os_tagin the error output (even just one line) to simplify debugging for users on unusual targets.
- Since you already have
---
🏁 Script executed:
```shell
#!/bin/bash
# Query 1: Search for USE(64KB_PAGE_BLOCK) or 64KB page block related configuration
echo "=== Searching for USE(64KB_PAGE_BLOCK) and related flags ==="
rg -i "64kb|use.*page.*block|page.*size.*block" -n -B2 -A2 | head -50
Length of output: 66031
🏁 Script executed:
#!/bin/bash
# Query 2: Examine main() function and Output.Source.Stdio.init() call order
echo "=== Checking main() function structure and Output initialization ==="
rg -n "pub fn main|Output.Source.Stdio.init|crash_handler.init" src/main.zig | head -30Length of output: 162
🏁 Script executed:
#!/bin/bash
# Query 3: Check imports at bottom of src/main.zig
echo "=== Checking imports in src/main.zig ==="
tail -50 src/main.zig | head -30Length of output: 903
🏁 Script executed:
#!/bin/bash
# Query 4: Search for similar early-startup checks using bun.Output.prettyErrorln
echo "=== Searching for other early-startup error paths ==="
rg "prettyErrorln.*error|Output.prettyErrorln" -n -B3 -A3 src/ | head -80Length of output: 6215
🏁 Script executed:
#!/bin/bash
# Query 5: Check if checkPageSizeSupport() exists and where it's called
echo "=== Checking checkPageSizeSupport call site in main() ==="
rg -n "checkPageSizeSupport" src/main.zig -B2 -A2Length of output: 636
🏁 Script executed:
#!/bin/bash
# Query 6: Search build configuration for page size or 64KB related settings
echo "=== Searching build.zig for page size configuration ==="
rg -i "64kb|page.*block|page.*size" build.zig 2>/dev/null | head -30 || echo "Not found in build.zig"Length of output: 55
---
🏁 Script executed:
```shell
#!/bin/bash
# Query: Check file-level imports and top of src/main.zig
echo "=== Checking file-level imports in src/main.zig ==="
head -20 src/main.zig
Length of output: 723
🏁 Script executed:
#!/bin/bash
# Query: Look for where std and bun are imported at file level
echo "=== Looking for @import declarations in src/main.zig ==="
rg "@import" src/main.zig -n | head -20Length of output: 1941
🏁 Script executed:
#!/bin/bash
# Query: Verify Output.Source.Stdio is initialized properly before prettyErrorln
echo "=== Checking Output initialization and dependencies ==="
rg -n "Output.Source.Stdio|Output.prettyErrorln|Output.flush" src/main.zigLength of output: 1818
Wire USE(64KB_PAGE_BLOCK) compile-time flag and defer Output usage until after Stdio.init()
Verification confirms two significant issues that require addressing:
-
Missing
USE(64KB_PAGE_BLOCK)wiring (lines 21, 80)- The error message at line 80 tells users to "Build Bun from source with USE(64KB_PAGE_BLOCK) enabled," but the
expected_max_page_sizelogic has no compile-time mechanism to detect or respect this flag. - If someone rebuilds WebKit with
USE(64KB_PAGE_BLOCK)enabled, the check will incorrectly reject 64 KB page-size systems (e.g., Linux ARM64 with 64 KB pages) and print a misleading max page-size value. - Add a compile-time constant (e.g.,
use_64kb_page_block) that, when true, immediately returns 64 KB as the expected maximum, bypassing the arch/OS logic.
- The error message at line 80 tells users to "Build Bun from source with USE(64KB_PAGE_BLOCK) enabled," but the
-
Early
Output.prettyErrorlncalls beforeStdio.init()(lines 67–83)checkPageSizeSupport()is called at line 94, butOutput.Source.Stdio.init()doesn't run until line 129.- Calling
@import("bun").Output.prettyErrorln()and.flush()before the output backend is initialized risks silent failures or missing text on non-Linux platforms. - Either initialize the minimal Output backend before
checkPageSizeSupport(), or fall back tostd.debug.print()/std.io.getStdErr()for this early error path.
-
Minor: Repeated
@importcalls (lines 28–83)- The function uses
@import("std"),@import("bun"), and@import("builtin")inline rather than reusing file-level imports. - Extract
const std = @import("std");andconst bun = @import("bun");at the file level (per coding guidelines) and reuse them to reduce redundancy.
- The function uses
As noted in the learnings, when making platform-specific changes to Zig code, run bun run zig:check-all to verify correctness across all platforms.
🤖 Prompt for AI Agents
In src/main.zig around lines 17–86: the page-size check doesn't honor a
compile-time USE(64KB_PAGE_BLOCK) flag, emits prettyErrorln output before
Stdio.init(), and repeats inline @import calls; fix by adding a comptime boolean
(e.g., use_64kb_page_block) that, when true, makes expected_max_page_size return
64 * 1024 immediately, update the check to use that constant, replace early
bun.Output.prettyErrorln calls with a fallback that writes to std.err (or ensure
the minimal Output backend is initialized before calling checkPageSizeSupport),
and move repeated imports to file-level consts (const std = @import("std");
const bun = @import("bun"); const builtin = @import("builtin");) and reuse them
throughout.
Summary
WebKit/JavaScriptCore uses hardcoded compile-time constants for maximum page sizes that vary by architecture. If the actual system page size exceeds these limits, WebKit will crash with an assertion failure.
This PR adds an early check on startup that detects unsupported page sizes and exits with a friendly, actionable error message before WebKit has a chance to crash.
Background
WebKit's
PageBlock.hdefinesCeilingOnPageSizewith these limits:USE(64KB_PAGE_BLOCK)is setAs the WebKit source comments note: "All of these values are going to be incorrect on systems configured to use larger than normal page size, so on such systems it is expected that WebKit will crash until this value is changed and recompiled. Sorry."
This commonly affects:
Changes
checkPageSizeSupport()function insrc/main.zigthat mirrors WebKit's page size logicmain()Example Error Output
Testing
bun bd🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]