Increase stack size for build.rs to prevent overflow on Windows - #7
Conversation
Windows' default 1 MiB main-thread stack overflows during syn's recursive parsing of the ~90 collected interpreter source files (STATUS_STACK_OVERFLOW 0xc00000fd), failing `cargo build`.
WalkthroughThe build script is refactored to execute the build pipeline on a dedicated worker thread configured with 64MiB stack size. The new ChangesBuild Thread Stack Overflow Prevention
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
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 `@pyre/pyre-jit-trace/build.rs`:
- Around line 16-26: When rejoining the worker thread created by
std::thread::Builder::new().stack_size(...).spawn(real_main), avoid losing the
worker's original panic message by inspecting the Err payload from join()
instead of calling expect directly; on Err, downcast the Box<dyn Any + Send>
payload to &str or String (or format it generically) and re-panic with that
message (or call std::panic::resume_unwind with the original payload) so the
main-thread panic contains the worker's original diagnostic text and backtrace;
locate the join()/.expect("build-script worker panicked") call and replace the
expect with logic that downcasts and re-raises the original payload from the
join() result.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6fae8fbf-fac7-4857-b5dd-0f538c05971f
📒 Files selected for processing (1)
pyre/pyre-jit-trace/build.rs
| // Run on a worker thread with a large stack: on Windows the main | ||
| // thread's default stack is 1 MiB, which `syn`'s recursive parsing | ||
| // of the ~90 collected source files overflows | ||
| // (STATUS_STACK_OVERFLOW 0xc00000fd). | ||
| std::thread::Builder::new() | ||
| .stack_size(64 * 1024 * 1024) | ||
| .spawn(real_main) | ||
| .expect("spawn build-script worker") | ||
| .join() | ||
| .expect("build-script worker panicked"); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Core change LGTM — optional: forward the worker's panic payload for cleaner build output.
The spawn/join pattern is correct: fn real_main satisfies FnOnce() + Send + 'static, the 64 MiB stack directly addresses the documented Windows overflow, and cargo::rerun-if-changed directives emitted from the worker thread reach Cargo correctly because stdout is process-wide and there is no concurrent output.
One cosmetic gap: when the worker panics, Rust's default panic hook already prints the message + backtrace to stderr before join() returns Err, so diagnostic information is preserved. However, the subsequent .expect("build-script worker panicked") emits a second, less-informative panic in the main thread. If you want the re-raised panic to carry the original message, you can downcast the payload:
♻️ Optional: re-raise worker panic payload in the main thread
- .join()
- .expect("build-script worker panicked");
+ .join()
+ .unwrap_or_else(|e| {
+ let msg = e
+ .downcast_ref::<&str>()
+ .copied()
+ .or_else(|| e.downcast_ref::<String>().map(|s| s.as_str()))
+ .unwrap_or("<non-string panic payload>");
+ panic!("build-script worker panicked: {msg}");
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Run on a worker thread with a large stack: on Windows the main | |
| // thread's default stack is 1 MiB, which `syn`'s recursive parsing | |
| // of the ~90 collected source files overflows | |
| // (STATUS_STACK_OVERFLOW 0xc00000fd). | |
| std::thread::Builder::new() | |
| .stack_size(64 * 1024 * 1024) | |
| .spawn(real_main) | |
| .expect("spawn build-script worker") | |
| .join() | |
| .expect("build-script worker panicked"); | |
| } | |
| // Run on a worker thread with a large stack: on Windows the main | |
| // thread's default stack is 1 MiB, which `syn`'s recursive parsing | |
| // of the ~90 collected source files overflows | |
| // (STATUS_STACK_OVERFLOW 0xc00000fd). | |
| std::thread::Builder::new() | |
| .stack_size(64 * 1024 * 1024) | |
| .spawn(real_main) | |
| .expect("spawn build-script worker") | |
| .join() | |
| .unwrap_or_else(|e| { | |
| let msg = e | |
| .downcast_ref::<&str>() | |
| .copied() | |
| .or_else(|| e.downcast_ref::<String>().map(|s| s.as_str())) | |
| .unwrap_or("<non-string panic payload>"); | |
| panic!("build-script worker panicked: {msg}"); | |
| }); | |
| } |
🤖 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 `@pyre/pyre-jit-trace/build.rs` around lines 16 - 26, When rejoining the worker
thread created by std::thread::Builder::new().stack_size(...).spawn(real_main),
avoid losing the worker's original panic message by inspecting the Err payload
from join() instead of calling expect directly; on Err, downcast the Box<dyn Any
+ Send> payload to &str or String (or format it generically) and re-panic with
that message (or call std::panic::resume_unwind with the original payload) so
the main-thread panic contains the worker's original diagnostic text and
backtrace; locate the join()/.expect("build-script worker panicked") call and
replace the expect with logic that downcasts and re-raises the original payload
from the join() result.
…pe __init__ lookup #8 dict arbitrary keys (PyPy dictobject.py parity): - W_DictObject entries: (i64, PyObjectRef) → (PyObjectRef, PyObjectRef) - dict_keys_equal: pointer identity → int equality → str equality - w_dict_lookup/w_dict_store: general PyObjectRef key API - w_dict_getitem_str/w_dict_setitem_str: str key convenience wrappers - All callers updated: build_map, py_getitem, py_setitem, dict methods #6 classmethod binding (PyPy function.py ClassMethod parity): - load_method: classmethod found in instance MRO → bind class (w_type) - load_method: classmethod found on type → bind type (class arg) - Foo.create() now receives cls=Foo as first argument #9 call_type_object __init__ lookup (PyPy typeobject.py parity): - lookup_in_type_mro_pub(w_type, "__init__") instead of py_getattr(w_type) - Avoids ATTR_TABLE shadowing of __init__ on type objects #7 W_TypeObject MRO cache (PyPy typeobject.py parity): - Added mro_w field to W_TypeObject - Computed once at type creation (build_class_inner) - lookup_in_type_mro uses cached MRO when available
#3 rawptrinfo: Add BoxEnv trait with is_virtual_ref() and is_virtual_raw() matching RPython's getptrinfo(box).is_virtual() and getrawptrinfo(box).is_virtual() type-dispatched virtual checks. #4 NONE handling: Removed — _number_boxes no longer handles OpRef::NONE. RPython snapshots never contain null boxes. Callers must filter. #5 Constant identification: BoxEnv::is_const() replaces HashMap lookup. RPython uses isinstance(box, Const); we use a trait method that implementations can back with OpRef range checks or HashMap. #7/#9 Multi-frame + frame boundaries: Snapshot struct with framestack (Vec<SnapshotFrame>). Each frame records jitcode_index, pc, boxes. number() serializes slot_count per frame for rebuild_from_numbering to correctly split tagged values across frames. SimpleBoxEnv: test implementation with constants/replacements/types/ virtuals HashMaps. Production BoxEnv will be implemented by OptContext. 5 tests including multi-frame roundtrip.
Windows' default 1 MiB main-thread stack overflows during syn's recursive parsing of the ~90 collected interpreter source files (STATUS_STACK_OVERFLOW 0xc00000fd), failing `cargo build`.
…fault Bridge chaining (PYRE_WASM_ENABLE_BRIDGES=0 disables): compiled bridges resolve guard exits in-module through a global fail-index space with nested chaining and a REF_HOME_FLOOR-sized ref-home area; declined bridges fall back to host round-trips. WASM_BRIDGES_ENABLED flips to default-true and the dormant-tracer comment block is dropped. CALL_ASSEMBLER (PYRE_WASM_CA=0 disables, now default-on): - alloc/pop frame helpers lower through the residual (i64xn)->i64 call_indirect type family instead of the jit_call trampoline (residual_max_arity forced >= 2 when the CA arm is emitted). - CA callee frames are libc-allocated jitframes registered with the shadow stack and recycled through a LIFO pool, replacing per-call old-gen allocation that forced back-to-back major collections (gc_majors 1704 -> 0 on fib_recursive). - Frames are sized and gcmapped to FRAME_REF_HOME_FLOOR when bridges are enabled, mirroring execute_token. - CA on a loop that already has bridges (or further chaining on a loop whose CA arm ran) is declined (diag slot 14 decl_ca_chain): composing the two on one recursion produces wrong output on recursion_memo_branch/generator_tree_recursion (task #7). Inline fast paths in generated code: - write barrier: test the TRACK_YOUNG_PTRS header flag inline and only call_indirect the helper when set (genop_discard_cond_call_gc_wb). - nursery allocation (PYRE_WASM_INLINE_ALLOC=0 disables): New/ NewWithVtable and constant-length NewArray/NewArrayClear bump nursery_free inline (header word + array length store) and only call the collecting helper on overflow, gated on the new type_alloc_is_plain trait (no destructor, not a weakref), size below max_nursery_object_size, and gc_stress off. Diagnostics: collection_counts on the GC trait (MiniMarkGC override), pyre_gc_minor/major_collections + pyre_jit_set_inline_alloc guest exports, gc_minors/gc_majors in the runner's [jit-stats] line, runner labels for the new MC_DIAG slots. fib_recursive (fib 0..34) 24s -> 2.9s, fannkuch8 13.8s -> 0.56s; wasm suite 168/169 (remaining fail is the pre-existing nbody libm ULP drift). Assisted-by: Claude
…fault Bridge chaining (PYRE_WASM_ENABLE_BRIDGES=0 disables): compiled bridges resolve guard exits in-module through a global fail-index space with nested chaining and a REF_HOME_FLOOR-sized ref-home area; declined bridges fall back to host round-trips. WASM_BRIDGES_ENABLED flips to default-true and the dormant-tracer comment block is dropped. CALL_ASSEMBLER (PYRE_WASM_CA=0 disables, now default-on): - alloc/pop frame helpers lower through the residual (i64xn)->i64 call_indirect type family instead of the jit_call trampoline (residual_max_arity forced >= 2 when the CA arm is emitted). - CA callee frames are libc-allocated jitframes registered with the shadow stack and recycled through a LIFO pool, replacing per-call old-gen allocation that forced back-to-back major collections (gc_majors 1704 -> 0 on fib_recursive). - Frames are sized and gcmapped to FRAME_REF_HOME_FLOOR when bridges are enabled, mirroring execute_token. - CA on a loop that already has bridges (or further chaining on a loop whose CA arm ran) is declined (diag slot 14 decl_ca_chain): composing the two on one recursion produces wrong output on recursion_memo_branch/generator_tree_recursion (task #7). Inline fast paths in generated code: - write barrier: test the TRACK_YOUNG_PTRS header flag inline and only call_indirect the helper when set (genop_discard_cond_call_gc_wb). - nursery allocation (PYRE_WASM_INLINE_ALLOC=0 disables): New/ NewWithVtable and constant-length NewArray/NewArrayClear bump nursery_free inline (header word + array length store) and only call the collecting helper on overflow, gated on the new type_alloc_is_plain trait (no destructor, not a weakref), size below max_nursery_object_size, and gc_stress off. Diagnostics: collection_counts on the GC trait (MiniMarkGC override), pyre_gc_minor/major_collections + pyre_jit_set_inline_alloc guest exports, gc_minors/gc_majors in the runner's [jit-stats] line, runner labels for the new MC_DIAG slots. fib_recursive (fib 0..34) 24s -> 2.9s, fannkuch8 13.8s -> 0.56s; wasm suite 168/169 (remaining fail is the pre-existing nbody libm ULP drift). Assisted-by: Claude
…fault Bridge chaining (PYRE_WASM_ENABLE_BRIDGES=0 disables): compiled bridges resolve guard exits in-module through a global fail-index space with nested chaining and a REF_HOME_FLOOR-sized ref-home area; declined bridges fall back to host round-trips. WASM_BRIDGES_ENABLED flips to default-true and the dormant-tracer comment block is dropped. CALL_ASSEMBLER (PYRE_WASM_CA=0 disables, now default-on): - alloc/pop frame helpers lower through the residual (i64xn)->i64 call_indirect type family instead of the jit_call trampoline (residual_max_arity forced >= 2 when the CA arm is emitted). - CA callee frames are libc-allocated jitframes registered with the shadow stack and recycled through a LIFO pool, replacing per-call old-gen allocation that forced back-to-back major collections (gc_majors 1704 -> 0 on fib_recursive). - Frames are sized and gcmapped to FRAME_REF_HOME_FLOOR when bridges are enabled, mirroring execute_token. - CA on a loop that already has bridges (or further chaining on a loop whose CA arm ran) is declined (diag slot 14 decl_ca_chain): composing the two on one recursion produces wrong output on recursion_memo_branch/generator_tree_recursion (task #7). Inline fast paths in generated code: - write barrier: test the TRACK_YOUNG_PTRS header flag inline and only call_indirect the helper when set (genop_discard_cond_call_gc_wb). - nursery allocation (PYRE_WASM_INLINE_ALLOC=0 disables): New/ NewWithVtable and constant-length NewArray/NewArrayClear bump nursery_free inline (header word + array length store) and only call the collecting helper on overflow, gated on the new type_alloc_is_plain trait (no destructor, not a weakref), size below max_nursery_object_size, and gc_stress off. Diagnostics: collection_counts on the GC trait (MiniMarkGC override), pyre_gc_minor/major_collections + pyre_jit_set_inline_alloc guest exports, gc_minors/gc_majors in the runner's [jit-stats] line, runner labels for the new MC_DIAG slots. fib_recursive (fib 0..34) 24s -> 2.9s, fannkuch8 13.8s -> 0.56s; wasm suite 168/169 (remaining fail is the pre-existing nbody libm ULP drift). Assisted-by: Claude
… + CALL_ASSEMBLER default-on) (#347) * jit-trace: exclude bridge walks from terminate_no_replay store-commit `terminate_no_replay` keeps a Terminate walk's eager list stores (journal-commit) so the loop-free function portal can return the walk's result without replaying it. A bridge Terminate walk does not go through that portal: its caller resumes the guarded region through the blackhole, which re-applies the stores. Keeping them here as well double-applied them. Gate the predicate on `!is_bridge_trace` so bridge walks roll their stores back before the blackhole replay. Assisted-by: Claude * builtins: route hasattr/getattr attribute-name check through checkattrname hasattr/getattr validated the name inline with their own "hasattr()/getattr(): attribute name must be string" messages; route both through checkattrname (operation.py:41-45), which raises the unified "attribute name must be string, not '<type>'" message setattr/delattr already use. Assisted-by: Claude * optimizeopt/heap: treat runtime-minted mutable descrs as written by every non-elidable call A descr minted at runtime (ei_index unset, u32::MAX) is outside the compute_bitstrings universe, so no call's write bitstring can ever name it; a bitcheck miss proves nothing. Treating the miss as "not written" let optheap keep a getfield cache across a store residual and re-read a stale value (module-global cell fold: 'acc = acc + a; acc = acc + b' dropped the first term). Field and array descr loops in force_from_effectinfo now conservatively invalidate mutable out-of-universe descrs on every non-elidable call. Assisted-by: Claude * jit-trace: record word-ABI void helpers through a sized descr with caller-supplied effect jit_store_name_to_namespace / jit_list_append are extern "C" helpers whose C signature returns a dummy machine word (-> i64, value ignored) but were recorded as plain void residuals. Add make_call_descr_void_word_abi (result_type=Void, result_size=8, distinct interning key) and TraceCtx::call_void_typed_word_abi so a signature-exact backend lowering (wasm call_indirect) can select the (i64xn) -> i64 type and drop the result. The new entry point takes a caller-supplied EffectInfo — these helpers write the heap (namespace cells, list storage), and the opcode-default empty write set let optheap CSE a getfield across the call; both call sites record MOST_GENERAL (graphanalyze.py:60 analyze_external_call for an unanalyzed external writer). The call_*_typed record tail is factored into record_call_with_descr (invalidate-before-record, pyjitpl.py:2683-2684). Assisted-by: Claude * codewriter: widen BhSizeSpec/BhDescr vtable to u64 The vtable field is an ob_type pointer captured in the producing process (the opcode_descrs.bin build script) and crosses the build->runtime serialization boundary. Declared usize, a 64-bit host pointer failed bincode's width check when deserialized on a wasm32 runtime. Store it as u64 and cast at the consumers; the value is an opaque identity word either way (real vtables are re-resolved via type_id -> gc_cache). Assisted-by: Claude * metainterp: extend MC_DIAG guard-to-bridge tallies; gated fbw bridge walk prints MC_DIAG grows 8 -> 18 slots: compile_bridge entry/InvalidLoop/ retrace_requested/arity-giveup (8-11), start_bridge_tracing entry and its early-return reasons (12-17), and stack_almost_full()==true (5, formerly reserved). Under the existing fbw debug-abort flag, the full-body walker also prints a bridge-walk epilogue line (commit state + outcome kind) and a note when a bridge STORE_SUBSCR specialization declines to the generic residual. Assisted-by: Claude * backend-wasm: bridge chaining and CALL_ASSEMBLER fast paths, on by default Bridge chaining (PYRE_WASM_ENABLE_BRIDGES=0 disables): compiled bridges resolve guard exits in-module through a global fail-index space with nested chaining and a REF_HOME_FLOOR-sized ref-home area; declined bridges fall back to host round-trips. WASM_BRIDGES_ENABLED flips to default-true and the dormant-tracer comment block is dropped. CALL_ASSEMBLER (PYRE_WASM_CA=0 disables, now default-on): - alloc/pop frame helpers lower through the residual (i64xn)->i64 call_indirect type family instead of the jit_call trampoline (residual_max_arity forced >= 2 when the CA arm is emitted). - CA callee frames are libc-allocated jitframes registered with the shadow stack and recycled through a LIFO pool, replacing per-call old-gen allocation that forced back-to-back major collections (gc_majors 1704 -> 0 on fib_recursive). - Frames are sized and gcmapped to FRAME_REF_HOME_FLOOR when bridges are enabled, mirroring execute_token. - CA on a loop that already has bridges (or further chaining on a loop whose CA arm ran) is declined (diag slot 14 decl_ca_chain): composing the two on one recursion produces wrong output on recursion_memo_branch/generator_tree_recursion (task #7). Inline fast paths in generated code: - write barrier: test the TRACK_YOUNG_PTRS header flag inline and only call_indirect the helper when set (genop_discard_cond_call_gc_wb). - nursery allocation (PYRE_WASM_INLINE_ALLOC=0 disables): New/ NewWithVtable and constant-length NewArray/NewArrayClear bump nursery_free inline (header word + array length store) and only call the collecting helper on overflow, gated on the new type_alloc_is_plain trait (no destructor, not a weakref), size below max_nursery_object_size, and gc_stress off. Diagnostics: collection_counts on the GC trait (MiniMarkGC override), pyre_gc_minor/major_collections + pyre_jit_set_inline_alloc guest exports, gc_minors/gc_majors in the runner's [jit-stats] line, runner labels for the new MC_DIAG slots. fib_recursive (fib 0..34) 24s -> 2.9s, fannkuch8 13.8s -> 0.56s; wasm suite 168/169 (remaining fail is the pre-existing nbody libm ULP drift). Assisted-by: Claude * check.py: allow bounded float divergence for the nbody wasm bench only The wasm float tolerance is now opt-in per bench via run_bench(..., wasm_float_tol=True) rather than applied to every wasm bench. Only nbody is marked; all other wasm benches require byte-identical output. The comparison gate becomes `backend == "wasm" and wasm_float_tol`, threaded from run_bench through _run_backend_bench. Corrects the wasm_outputs_match root-cause note: the divergence is libm's arch-specific pow building blocks, not FMA fusion (FMA hypothesis tested and refuted). Assisted-by: Claude * ci: save rust-cache on main only, restore-only on PRs Adds save-if: ${{ github.ref == 'refs/heads/main' }} to all five Swatinem/rust-cache steps (prepare-charon-llbc, cargo-test, pyre-check, cpython-tests, wasm-build). PR jobs become restore-only and pull main's default-branch cache via restore-keys, so each PR no longer persists its own ~500-600 MB target cache and multiplies the repo past GitHub's 10 GB cache budget. This is the still-applicable part of #209; that PR's other commit (LLBC handoff to wasm-build via artifact) is already superseded on main, where wasm-build restores LLBC through download-artifact. Assisted-by: Claude * check.py: add wasm to default backends when its target is installed DEFAULT_BACKENDS gains "wasm" whenever `rustup target list --installed` reports wasm32-unknown-unknown, the sole extra prerequisite for building the wasm backend (the wasmtime runtime is embedded in pyre-wasm-runner, not an external tool). On a machine without that target the probe returns false and the defaults stay dynasm,cranelift, so a bare check.py still runs only the native backends there. Assisted-by: Claude * optimizeopt/heap: gate out-of-universe descr invalidation on compute_bitstrings_has_run The runtime-minted-descr invalidation added in 6091829 fired on any descr whose effect_idx is the u32::MAX sentinel. In the unit tests compute_bitstrings is skipped, so every cached descr carries u32::MAX and an unrelated CallMayForce write bit wrongly invalidated it, breaking test_call_may_force_uses_effectinfo_to_keep_unaffected_cached_fields, test_call_may_force_keeps_unaffected_variable_index_array_cache and test_default_pipeline_escaping_call_arg_flush_is_selective. Gate the clause on compute_bitstrings_has_run() in both the field and array loops: after bitstrings run a u32::MAX effect_idx genuinely means an out-of-universe runtime descr (invalidate conservatively); before that it just means the fixture never stamped the slot, so the existing identity fallback is the correct arbiter. Assisted-by: Claude * ci: install wasm32 target on the Linux pyre/check.py leg only check.py now adds wasm to its default backends when wasm32-unknown-unknown is installed. The three pyre/check.py jobs share one steps anchor, so a step guarded by runner.os == 'Linux' installs the target on the ubuntu leg only — it builds and runs the wasm backend there, while macOS/Windows keep no wasm32 target and stay on dynasm/cranelift. wasm output is platform-independent, so exercising it on one OS is enough. Assisted-by: Claude
This pull request modifies the build script in
pyre/pyre-jit-trace/build.rsto address stack overflow issues during parsing on Windows. The main change is running the build logic on a worker thread with an increased stack size to prevent overflows when parsing many source files.Build process robustness:
synon Windows. The actual build logic has been moved to a newreal_mainfunction.Summary by CodeRabbit