Skip to content

check.py: take the bench directory off sys.path; direction-labelled jit-stats gate; re-record 43 baselines - #1034

Merged
youknowone merged 4 commits into
mainfrom
perf-bridge
Aug 4, 2026
Merged

check.py: take the bench directory off sys.path; direction-labelled jit-stats gate; re-record 43 baselines#1034
youknowone merged 4 commits into
mainfrom
perf-bridge

Conversation

@youknowone

@youknowone youknowone commented Aug 4, 2026

Copy link
Copy Markdown
Owner

What the cross-platform jit-stats red actually was

sys.path[0] is the script's own directory, and pyre/bench/synth holds over a
thousand entries. The import machinery's scan of that entry is itself a Python
loop, and at that size it crosses the 1039 compile threshold — so a fixture with
no loop anywhere in its body still recorded loops_compiled=1, produced by a
single import statement.

Decomposed in the real bench directory:

script loops_compiled
pass 0
from types import SimpleNamespace 1
import re, collections, struct, json 1
the whole simple_namespace_type.py fixture 1

And varying only the directory a fixture runs from, the fixture itself unchanged:

files in the script's directory loops_compiled
1 / 50 / 200 / 400 0
800 1
the real synth/ 1

It is invariant to PYPY_GC_NURSERY across 1MB..16MB, so it is not the
collection schedule.

Path handling differs per platform, so that ambient loop landed on either side of
the threshold depending on the host. Every jit-stats diff CI reported was a
uniform ±1 on loops_compiled — 80 of 80 on windows, 82 of 86 on ubuntu, and
none at all on macOS, where the baselines were recorded. Setting the flag on
macOS reproduces the Linux numbers exactly: arith_int_bool 8 → 7 with
guard_failures 2213 → 2211, gc_deque_backing_list 6 → 5 / 204 → 203,
struct_pack_unpack 2 → 1 / 2 → 1.

The change

pyre_env sets PYTHONSAFEPATH. An explicit value in the environment still
wins, and pyre reads it as a presence flag, so passing it empty reaches the old
behaviour for an A/B.

-P and not -I: the latter also implies -E, which drops PYTHONIOENCODING
and so changes how the child resolves its stdio encoding — not something to fold
into a run whose stdout is being diffed against an oracle.

The wasm guest has no environment at all, so the variable is resolved host-side
and handed over through a new pyre_set_safe_path export, alongside the existing
pyre_set_script_path. A module built without the export is left on its previous
behaviour. No wasm baseline moves under the flag — that backend never had the
sensitivity — but the plumbing keeps the three backends on one contract.

All 423 bench .py files were checked for sibling and relative imports; there
are none, so nothing depends on the entry being there.

Baselines

40 of the 754 native baselines move — the same 20 fixtures per backend CI listed
— on loops_compiled and guard_failures only, all downwards.

Three wasm baselines are re-recorded for an unrelated reason: they carry a drift
already red on main at this base sha, and two of the three include a
loops_aborted rise, so they are worth a look on their own.

list_append_write_barrier_gc  loops_aborted 0->1, loops_compiled 1->12,
                              bridges_compiled 1->5, guard_failures 200->1335
pickle_terminal_raise_resume  loops_compiled 74->73
recursion_memo_branch         loops_aborted 1->2, loops_compiled 2->3,
                              bridges_compiled 11->14, guard_failures 2227->3083

Also here

The jit-stats gate now labels a move by direction — regressed for a change
toward more aborts or fewer compiled loops, improved for the other way, and
anything ambiguous as regressed. Both still fail; the split is for reading a
log, not for deciding one. A fixture whose counters do not reproduce across a
re-run is reported unstable and not gated on, so a genuinely variable counter
cannot be laundered into a baseline.

_jitstats_baseline_path resolves a <name>.<backend>.<platform>.jitstats
overlay when one exists. Both overlays this branch had are removed: one froze
guard_failures=637 while the shared baseline already carried 638 and windows
produced 638, which made a fixture fail that passed on main. The mechanism is
kept, the two uses are not — an overlay shadows the shared file permanently, so
it has to be justified against the current value, not a remembered one.

Verification

dynasm 377/377, cranelift 377/377, wasm 373/373 locally, on LLBC
re-extracted at this base.

authored by Claude

Summary by CodeRabbit

  • Chores

    • Updated benchmark statistics across multiple benchmarks.
  • Improvements

    • Enhanced JIT-stat validation with improved regression and improvement classification, instability detection, and platform-specific baselines.
    • Added safe-path mode support for WASM environments via environment variable forwarding.
    • Improved diagnostic reporting for compiled-loop context and distinction between regressed, improved, and unstable metrics.

…at do not

reproduce, and resolve a per-platform baseline

Three changes to the jit-stats gate.

Direction. `_jit_stats_change` now returns (regressions, improvements) instead
of one string, classified per counter: a rise in the badness fields or in
guard_failures is a regression, a fall in loops_compiled is a regression, and
the opposite moves are gains. bridges_compiled is in neither list, so both its
directions report as regressions — a fall to 0 is the dead-bridge case when
guards still fail and a gain when they stopped (list_length_hint_validate fell
4 -> 0 as guard_failures fell 828 -> 1), and a rise is either wider coverage or
a guard storm. Both outcomes still fail; the labels are REGRESSED and IMPROVED,
and each carries the --snapshot command that re-records it.

Reproducibility. A fixture that disagrees with its baseline is now re-run
JITSTATS_STABILITY_RUNS times. If a repeat of the same binary reports different
counters, the comparison is reported as UNSTABLE and not gated. Measured on this
tree: two runs of one pyre-dynasm binary disagreed by loops_compiled +2 on five
unrelated fixtures. Instability is measured per invocation, not annotated per
fixture.

Per-platform baselines. `_jitstats_baseline_path` prefers
<name>.<backend>.<sys.platform>.jitstats when it exists. windows-latest
cranelift reports closure_per_call guard_failures 416 and
recursive_call_frame_relocation 637 where macos-latest and ubuntu-24.04 both
report 415 and 638, on two independent runs, with every other counter equal and
that host's dynasm leg passing 371/371. Those two overlays are added here; the
shared files still gate the other two platforms exactly.

Assisted-by: Claude
The overlay recorded guard_failures=637, while the windows cranelift run
produces 638 — the value the shared baseline already carries. The overlay
only made the fixture fail on windows.

Assisted-by: Claude
…-stats baselines

`sys.path[0]` is the script's own directory, and `pyre/bench/synth` holds over a
thousand entries. The import machinery's scan of that entry is itself a Python
loop, and at that size it crosses the 1039 compile threshold — so a fixture with
no loop anywhere in its body still recorded `loops_compiled=1`, produced by a
single `import` statement. Varying only the directory a fixture runs from, the
fixture itself unchanged: 400 files compile nothing, 800 compile that loop. It
is invariant to PYPY_GC_NURSERY from 1MB to 16MB.

Path handling differs per platform, so that ambient loop landed on either side of
the threshold depending on the host: every jit-stats diff CI reported was a
uniform +-1 on `loops_compiled`, 80 of 80 on windows and 82 of 86 on ubuntu, with
macOS — where the baselines were recorded — reporting none.

`pyre_env` now sets PYTHONSAFEPATH, and an explicit value in the environment
still wins, so passing it empty reaches the old behaviour. `-P` alone rather than
`-I`, which also implies `-E` and would change how the child resolves its stdio
encoding while its stdout is being diffed against an oracle.

The wasm guest has no environment, so the variable is resolved host-side and
handed over through a new `pyre_set_safe_path` export, alongside the existing
`pyre_set_script_path`. A module without the export is left on its previous
behaviour.

40 of the 754 native baselines move — the same 20 fixtures per backend CI listed
— on `loops_compiled` and `guard_failures` only, all downwards. No wasm baseline
moves under the flag.

Three wasm baselines are re-recorded for an unrelated reason: they carry a drift
that is already red on main at this base sha, and two of the three include a
`loops_aborted` rise.

  list_append_write_barrier_gc  loops_aborted 0->1, loops_compiled 1->12,
                                bridges_compiled 1->5, guard_failures 200->1335
  pickle_terminal_raise_resume  loops_compiled 74->73
  recursion_memo_branch         loops_aborted 1->2, loops_compiled 2->3,
                                bridges_compiled 11->14, guard_failures 2227->3083

dynasm 377/377, cranelift 377/377 and wasm 373/373 pass locally.

Assisted-by: Claude
The overlay was added for a windows-only `guard_failures` 415 -> 416 on
cranelift, before the bench directory was taken off `sys.path`. Whether that
delta survives the flag cannot be measured here, and an overlay that is no longer
needed does not fail quietly: it shadows the shared baseline permanently, so a
stale value becomes a failure main does not have.

Removing it leaves at worst the delta main already reports, which is diagnosable
from the windows log; the overlay can be restored against a value read back from
it.

Assisted-by: Claude
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The change adds safe-path propagation for Wasm execution, expands JIT-stat validation with stability and directional classifications, and updates synthetic benchmark baselines for new JIT results.

Changes

JIT validation and safe-path execution

Layer / File(s) Summary
Safe-path propagation to Wasm
pyre/check.py, pyre/pyre-wasm-runner/src/main.rs, pyre/pyre-wasm/src/lib.rs
Child environments set PYTHONSAFEPATH. The runner forwards the setting through pyre_set_safe_path. The Wasm host conditionally omits the script directory from sys.path.
JIT-stat classification and stability checks
pyre/check.py
JIT-stat changes are classified as regressions or improvements. Changed fixtures run again to detect instability. Platform-specific baselines take precedence.
JIT-stat result reporting
pyre/check.py
Backend results and final summaries distinguish regressions, improvements, unstable fixtures, missing baselines, and absent statistic lines.
Synthetic JIT-stat baselines
pyre/bench/synth/*.jitstats
Benchmark baselines update guard failures, bridges compiled, aborted loops, and compiled loops for Cranelift, DynASM, and Wasm runs.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Check
  participant WasmRunner
  participant WasmHost
  Check->>WasmRunner: Set PYTHONSAFEPATH=1
  WasmRunner->>WasmHost: Call pyre_set_safe_path(1)
  WasmHost->>WasmHost: Suppress script-directory sys.path entry
Loading
sequenceDiagram
  participant Check
  participant Backend
  participant JITStats
  Check->>Backend: Execute changed fixture
  Backend->>JITStats: Produce counters
  Check->>Backend: Repeat fixture
  Backend->>JITStats: Produce repeat counters
  Check->>Check: Classify the result
Loading

Possibly related PRs

Poem

A rabbit checked the loops at dawn,
And found some guards had hopped along.
Safe paths guide the Wasm way,
New stats mark each run today.
“Re-run the truth!” the rabbit sings,
While tidy counters sprout small wings.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: safe-path handling, direction-labelled JIT-stat gating, and baseline updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf-bridge

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.

❤️ Share

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6f768e2cb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pyre/check.py
Comment on lines +1285 to +1289
repeats = self._jitstats_repeats(backend, script, timeout)
drifted = next(
(s for s in repeats or () if s != jitstats), None
)
if drifted is not None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep stable regressions gated when another counter fluctuates

If one counter has a repeatable regression while any unrelated counter varies between the initial run and a repeat, this branch marks the entire fixture unstable and records it as a pass. For example, a stable loops_aborted 0 -> 1 can be hidden by ordinary guard_failures jitter, even when every repeat retains the new abort. Compare stability per changed field and continue failing fields whose baseline delta persists rather than discarding the whole snapshot.

AGENTS.md reference: AGENTS.md:L238-L246

Useful? React with 👍 / 👎.

Comment thread pyre/pyre-wasm/src/lib.rs
Comment on lines +818 to +819
pub extern "C" fn pyre_set_safe_path(enabled: u32) {
super::SAFE_PATH.with(|f| f.set(enabled != 0));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Propagate safe_path to the interpreter runtime flags

When the wasm runner receives nonempty PYTHONSAFEPATH, this setter updates only the new TLS cell used by run_python_impl; it never updates importing::SYS_SAFE_PATH, which is what safe_path_flag() and sys.flags.safe_path read. Consequently the script directory is suppressed while sys.flags.safe_path still reports False and other safe-path-dependent interpreter behavior remains disabled, diverging from the native launcher. Route the setting through the existing interpreter-owned runtime flag instead of maintaining a second TLS value.

AGENTS.md reference: AGENTS.md:L148-L162

Useful? React with 👍 / 👎.

internal_compile_panics=0
loops_aborted=0
loops_compiled=1
loops_aborted=1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not bless the unexplained wasm abort regression

This baseline update makes the gate accept loops_aborted rising from 0 to 1, alongside guard_failures rising from 200 to 1335, even though this commit contains no corresponding wasm/JIT change or root-cause explanation; the commit message explicitly calls the drift unrelated. The same masking occurs in recursion_memo_branch.wasm.jitstats (loops_aborted 1 -> 2). Leave these baselines unchanged until the regression is explained or fixed, otherwise future checks treat the degraded behavior as healthy.

AGENTS.md reference: AGENTS.md:L238-L246

Useful? React with 👍 / 👎.

@youknowone
youknowone merged commit 073d9e5 into main Aug 4, 2026
7 of 8 checks passed
@youknowone
youknowone deleted the perf-bridge branch August 4, 2026 15:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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/check.py`:
- Around line 1284-1290: Filter out absent repeat snapshots before selecting a
drifted result in the regression/improvement handling around _jitstats_repeats,
so None entries do not terminate next() and later differing snapshots are
detected. Add test coverage for repeats containing [None, changed_snapshot],
ensuring the changed snapshot is reported as drift.

In `@pyre/pyre-wasm-runner/src/main.rs`:
- Around line 379-391: The PYTHONSAFEPATH handling must be enforced on every
Wasm execution path, not silently skipped when the module lacks
pyre_set_safe_path. Update the logic around pyre_set_safe_path and both engine
branches to require the setter when safe path is enabled, propagate an error if
unavailable or ineffective, and ensure this validation occurs before invoking
pyre_run_python.

In `@pyre/pyre-wasm/src/lib.rs`:
- Around line 519-525: Move SAFE_PATH out of thread-local storage and store the
startup setting on the Wasm execution owner or pass it through
PyExecutionContext. Update the public pyre_set_safe_path ABI path and
run_python_impl to read and propagate this owned execution state, preserving the
existing safe-path behavior for wasm-host builds.
🪄 Autofix

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 Plus

Run ID: 1cc6ff24-2dc1-4617-bf9a-dad9b89b0393

📥 Commits

Reviewing files that changed from the base of the PR and between 12c2f33 and c6f768e.

📒 Files selected for processing (46)
  • pyre/bench/synth/arith_int_bool.cranelift.jitstats
  • pyre/bench/synth/arith_int_bool.dynasm.jitstats
  • pyre/bench/synth/ast_compile_roundtrip.cranelift.jitstats
  • pyre/bench/synth/ast_compile_roundtrip.dynasm.jitstats
  • pyre/bench/synth/gc_deque_backing_list.cranelift.jitstats
  • pyre/bench/synth/gc_deque_backing_list.dynasm.jitstats
  • pyre/bench/synth/imp_lock_rlock_semantics.cranelift.jitstats
  • pyre/bench/synth/imp_lock_rlock_semantics.dynasm.jitstats
  • pyre/bench/synth/import_from_name_path.cranelift.jitstats
  • pyre/bench/synth/import_from_name_path.dynasm.jitstats
  • pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats
  • pyre/bench/synth/inline_freevar_after_mayforce.dynasm.jitstats
  • pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats
  • pyre/bench/synth/module_getattr_descr_error.cranelift.jitstats
  • pyre/bench/synth/module_getattr_descr_error.dynasm.jitstats
  • pyre/bench/synth/module_getattr_surrogate_cls.cranelift.jitstats
  • pyre/bench/synth/module_getattr_surrogate_cls.dynasm.jitstats
  • pyre/bench/synth/operator_set_inplace_ops.cranelift.jitstats
  • pyre/bench/synth/operator_set_inplace_ops.dynasm.jitstats
  • pyre/bench/synth/pickle_ctor_args.cranelift.jitstats
  • pyre/bench/synth/pickle_ctor_args.dynasm.jitstats
  • pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats
  • pyre/bench/synth/print_stdout_redirect.cranelift.jitstats
  • pyre/bench/synth/print_stdout_redirect.dynasm.jitstats
  • pyre/bench/synth/pypy_dict_primitives_nonbinding.cranelift.jitstats
  • pyre/bench/synth/pypy_dict_primitives_nonbinding.dynasm.jitstats
  • pyre/bench/synth/recursion_memo_branch.wasm.jitstats
  • pyre/bench/synth/seqiter_tuple_error_parity.cranelift.jitstats
  • pyre/bench/synth/seqiter_tuple_error_parity.dynasm.jitstats
  • pyre/bench/synth/simple_namespace_type.cranelift.jitstats
  • pyre/bench/synth/simple_namespace_type.dynasm.jitstats
  • pyre/bench/synth/sre_pattern_methods.cranelift.jitstats
  • pyre/bench/synth/sre_pattern_methods.dynasm.jitstats
  • pyre/bench/synth/sre_wasm_min.cranelift.jitstats
  • pyre/bench/synth/sre_wasm_min.dynasm.jitstats
  • pyre/bench/synth/sre_wasm_min1.cranelift.jitstats
  • pyre/bench/synth/sre_wasm_min1.dynasm.jitstats
  • pyre/bench/synth/str_encode_text_codec.cranelift.jitstats
  • pyre/bench/synth/str_encode_text_codec.dynasm.jitstats
  • pyre/bench/synth/struct_pack_unpack.cranelift.jitstats
  • pyre/bench/synth/struct_pack_unpack.dynasm.jitstats
  • pyre/bench/synth/type_dotted_name.cranelift.jitstats
  • pyre/bench/synth/type_dotted_name.dynasm.jitstats
  • pyre/check.py
  • pyre/pyre-wasm-runner/src/main.rs
  • pyre/pyre-wasm/src/lib.rs

Comment thread pyre/check.py
Comment on lines +1284 to +1290
if regressions or improvements:
repeats = self._jitstats_repeats(backend, script, timeout)
drifted = next(
(s for s in repeats or () if s != jitstats), None
)
if drifted is not None:
moved = _jit_stats_change(jitstats, drifted)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Skip absent repeat snapshots before selecting drift.

Line 1286 uses None both as the no-drift sentinel and as a possible repeat snapshot. If the first repeat has no [jit-stats] line, it ends next() and prevents a later differing repeat from being checked. The fixture can then be gated instead of reported unstable.

Filter missing repeat snapshots before selecting drift. Add coverage for [None, changed_snapshot].

Proposed fix
                 drifted = next(
-                    (s for s in repeats or () if s != jitstats), None
+                    (
+                        snapshot
+                        for snapshot in repeats or ()
+                        if snapshot is not None and snapshot != jitstats
+                    ),
+                    None,
                 )
📝 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.

Suggested change
if regressions or improvements:
repeats = self._jitstats_repeats(backend, script, timeout)
drifted = next(
(s for s in repeats or () if s != jitstats), None
)
if drifted is not None:
moved = _jit_stats_change(jitstats, drifted)
if regressions or improvements:
repeats = self._jitstats_repeats(backend, script, timeout)
drifted = next(
(
snapshot
for snapshot in repeats or ()
if snapshot is not None and snapshot != jitstats
),
None,
)
if drifted is not None:
moved = _jit_stats_change(jitstats, drifted)
🤖 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/check.py` around lines 1284 - 1290, Filter out absent repeat snapshots
before selecting a drifted result in the regression/improvement handling around
_jitstats_repeats, so None entries do not terminate next() and later differing
snapshots are detected. Add test coverage for repeats containing [None,
changed_snapshot], ensuring the changed snapshot is reported as drift.

Comment on lines +379 to +391
// `-P` / PYTHONSAFEPATH, resolved host-side: the guest's environment is
// permanently empty, so the variable has to be handed over explicitly or it
// would read as unset there while working natively. A presence flag, matching
// `pyrex::resolve_safe_path` — any non-empty value enables it, `"0"`
// included, and an empty value counts as unset. Absent on a module predating
// the export, leaving the guest on its default of seeding `sys.path[0]`.
if std::env::var_os("PYTHONSAFEPATH").is_some_and(|value| !value.is_empty()) {
if let Ok(set_safe_path) =
instance.get_typed_func::<u32, ()>(&mut store, "pyre_set_safe_path")
{
set_safe_path.call(&mut store, 1)?;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 'WasmEngine::Wasmi|wasmi_host::run|PYTHONSAFEPATH|pyre_set_safe_path' pyre/pyre-wasm-runner/src/main.rs
rg -n -C 6 'SAFE_PATH|SCRIPT_PATH|add_sys_path|pyre_set_safe_path' pyre/pyre-wasm/src/lib.rs

Repository: youknowone/pyre

Length of output: 7647


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== wasmi_host outline =="
ast-grep outline pyre/pyre-wasm-runner/src/wasmi_host.rs --view compact || true

echo "== wasmi_host relevant symbols =="
rg -n -C 8 'run\(|inspect\(|pyre_set_safe_path|pyre_set_script_path|wasmi|WasmModule|Memory|Table|Function' pyre/pyre-wasm-runner/src/wasmi_host.rs

echo "== main safe path/script path call context =="
sed -n '340,420p' pyre/pyre-wasm-runner/src/main.rs

Repository: youknowone/pyre

Length of output: 12823


Security And Privacy (CWE-426): Untrusted Search Path

Reachability: External · Exploitability: Moderate

Reachability path
● Entry
  pyre/pyre-wasm/src/lib.rs:818
  pyre_set_safe_path
│
▼
● Hop
  pyre/check.py:1019
  Check: Measure each timed interpreter/backend's empty-program user-CPU cost. Runs an empty script STARTUP_SAMPLES times per interpreter and reco…
│
▼
● Sink
  pyre/pyre-wasm-runner/src/main.rs

Enforce PYTHONSAFEPATH before running either Wasm engine.

Both engine paths currently continue when a module lacks the new pyre_set_safe_path export. When the caller enables safe path, sys.path[0] is still seeded for modules that do not export the setter; call it on every execution path and fail before pyre_run_python if the selected module cannot enforce the requested policy.

🤖 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-wasm-runner/src/main.rs` around lines 379 - 391, The PYTHONSAFEPATH
handling must be enforced on every Wasm execution path, not silently skipped
when the module lacks pyre_set_safe_path. Update the logic around
pyre_set_safe_path and both engine branches to require the setter when safe path
is enabled, propagate an error if unavailable or ineffective, and ensure this
validation occurs before invoking pyre_run_python.

Comment thread pyre/pyre-wasm/src/lib.rs
Comment on lines +519 to +525
/// `-P` / PYTHONSAFEPATH, which suppresses the `sys.path[0]` entry. The
/// guest has no environment, so the embedder passes the resolved flag in
/// through `pyre_set_safe_path` rather than the variable being read here.
/// Only the native-host binding seeds that entry — the browser build has no
/// filesystem to seed it from.
#[cfg(feature = "wasm-host")]
static SAFE_PATH: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 6 'thread_local!|SCRIPT_PATH|SAFE_PATH|pyre_set_script_path|pyre_set_safe_path|run_python_impl' pyre/pyre-wasm/src/lib.rs
rg -n -C 4 'resolve_safe_path|PYTHONSAFEPATH|pymain_sys_path_add_path0' .

Repository: youknowone/pyre

Length of output: 29272


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== pyre/pyre-wasm/src/lib.rs relevant outline =="
ast-grep outline pyre/pyre-wasm/src/lib.rs --view compact 2>/dev/null | sed -n '1,220p' || true

echo "== pyre/pyre-interpreter/src/importing.rs SYS_SAFE_PATH and safe_path_flag =="
sed -n '1,80p;380,435p;1848,1872p' pyre/pyre-interpreter/src/importing.rs

echo "== pyre/pyrex/src/lib.rs LaunchFlags / resolve_safe_path =="
sed -n '1,260p' pyre/pyrex/src/lib.rs

echo "== safe_path usages in pyre-interpreter/src/importing.rs =="
rg -n -C 3 'safe_path|safe_path_flag|SYS_SAFE_PATH|add_sys_path_0|add_sys_path' pyre/pyre-interpreter/src/importing.rs

Repository: youknowone/pyre

Length of output: 22135


Move SAFE_PATH out of TLS.

SAFE_PATH is interpreter startup state selected by the public WASM host ABI and used by run_python_impl. The upstream Python implementation maps -P / PYTHONSAFEPATH into interpreter configuration; store it on the Wasm execution owner or stage it via PyExecutionContext instead of thread_local! with no upstream citation or exceptional-TLS justification.

🤖 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-wasm/src/lib.rs` around lines 519 - 525, Move SAFE_PATH out of
thread-local storage and store the startup setting on the Wasm execution owner
or pass it through PyExecutionContext. Update the public pyre_set_safe_path ABI
path and run_python_impl to read and propagate this owned execution state,
preserving the existing safe-path behavior for wasm-host builds.

Source: Coding guidelines

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit c6f768e).
Updated: 2026-08-04T15:55:23.323Z

Files in the reviewed diff
pyre/check.py
pyre/pyre-wasm-runner/src/main.rs
pyre/pyre-wasm/src/lib.rs

1. Regressions to PyPy parity introduced by this patch

None.

2. Other mismatches introduced by this patch

  • pyre/pyre-wasm/src/lib.rs:818 ↔ pypy/interpreter/app_main.py:749 — the wasm setter stores safe-path only in private SAFE_PATH; it suppresses sys.path[0] at pyre/pyre-wasm/src/lib.rs:572, but does not update the interpreter runtime flag. PyPy sets the single safe_path option, which is exposed as sys.flags.safe_path (pypy/module/sys/app.py:176). Thus with PYTHONSAFEPATH=1, wasm can report sys.flags.safe_path == False while omitting the script directory.

3. Pre-existing mismatches (already present before this patch)

None.

4. Structural adaptations

  • pyre/pyre-wasm-runner/src/main.rs:385 ↔ pypy/interpreter/app_main.py:749 — forwarding PYTHONSAFEPATH through the pyre_set_safe_path wasm ABI is a necessary wasm host/guest adaptation: PyPy reads its process environment directly, while the guest intentionally has no environment. The non-empty-value semantics, including "0" being enabled, match PyPy.

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.

1 participant