jit, stdlib: PR 1187 review follow-ups; cpython_tests: per-host baseline overlay

.github/workflows/pyre-ci.yml

@@ -576,9 +576,9 @@ jobs:
     # leg; waiting for macOS races the Linux upload. This job is the only place
     # CI runs the suite — `pyre/check.py` keeps the stage behind
     # `--cpython-suite`, which no job passes, so its wall time is paid once.
-    # Separately, the baseline is darwin-arm64-specific
-    # (`CPYTHON_SUITE_BASELINE_HOST`); `PLATFORM_GATED` only handles modules
-    # CPython skips wholesale on this host.
+    # Separately, this runner gates against the shared baseline plus its own
+    # `baseline.linux-x86_64.json` overlay; `PLATFORM_GATED` only handles
+    # modules CPython skips wholesale on this host.
     needs: prepare-charon-llbc-linux
     if: ${{ !cancelled() && needs.prepare-charon-llbc-linux.result == 'success' }}
     timeout-minutes: 30

pyre/check.py

@@ -8,7 +8,6 @@
 import difflib
 import math
 import os
-import platform
 import re
 import shutil
 import statistics
@@ -146,11 +145,6 @@ def _detect_pyre_stdlib():
 BENCH_DIR = "pyre/bench"
 SYNTHETIC_BENCH_DIR = "pyre/bench/synth"
 CPYTHON_SUITE_BASELINE = "pyre/cpython_tests/baseline.json"
-# The baseline holds one verdict per module per backend, and dynasm emits
-# arch-specific code, so a verdict only compares against the host it was
-# observed on. `.github/workflows/pyre-ci.yml` pins its CPython suite job to
-# `macos-latest` for the same reason.
-CPYTHON_SUITE_BASELINE_HOST = ("darwin", "arm64")
 # Per module, matching the CI job. `test.test_asyncio` alone needs ~2m47s of
 # wall time, so a smaller per-module limit turns a slow module into a fake
 # regression.
@@ -2430,10 +2424,11 @@ def run_cpython_suite(self):
         specialised-pair subscript fold while the whole synthetic corpus and
         every parity fixture stayed green.
 
-        The baseline records one verdict per module per backend, observed on
-        darwin-arm64, and dynasm's codegen is arch-specific -- so the
-        comparison only means anything there.  On any other host the stage
-        reports that it did not run instead of counting as a pass.
+        The baseline records one verdict per module per backend and follows
+        the host: `run.py` reads a `baseline.<platform>-<machine>.json`
+        overlay before the shared file, so a verdict a host disagrees with
+        (dynasm's codegen is arch-specific) is recorded there rather than
+        making the stage unusable off one machine.
 
         Off by default and reached only through `--cpython-suite`: the suite
         costs more wall time than every other stage here put together, and the
@@ -2449,15 +2444,6 @@ def run_cpython_suite(self):
             print(dim("skip (backend not enabled)"))
             self._append_comparison(backend, name, "-", "-", "skip")
             return
-        host = (sys.platform, platform.machine())
-        if host != CPYTHON_SUITE_BASELINE_HOST:
-            sys.stdout.write(f"    {backend:<10s}")
-            print(dim(
-                f"skip (baseline observed on {'-'.join(CPYTHON_SUITE_BASELINE_HOST)}, "
-                f"host is {'-'.join(host)})"
-            ))
-            self._append_comparison(backend, name, "-", "-", "skip")
-            return
         sys.stdout.write(f"    {backend:<10s}")
         sys.stdout.flush()
         output, elapsed, code, stderr = run_timed(

pyre/cpython_tests/run.py

@@ -37,9 +37,13 @@
 `--strict-baseline` (gates on unrecorded improvements), `--full`, or
 `--update-baseline` to detect them. `SKIP` baseline entries are not run.
 
-Baseline entries carry no platform. `PLATFORM_GATED` excludes modules CPython
-skips wholesale on this host, preventing another host's PASS from becoming a
-false `PASS -> SKIP` regression; it does not make the baseline portable.
+The baseline is a shared file plus a per-host overlay,
+`baseline.<sys.platform>-<machine>.json`, consulted first. Whichever host
+records a (module, backend) first sets the shared verdict; every other host
+writes an entry only where it disagrees, and drops it again once the two
+agree. Two separate mechanisms sit next to this: `PLATFORM_GATED` excludes
+modules CPython skips wholesale on a host, and `KNOWN_SKIPS` is a decision
+about the module rather than the host, so it stays shared.
 
 Usage:
     python3 pyre/cpython_tests/run.py [--backend dynasm|cranelift]
@@ -54,6 +58,7 @@
 import concurrent.futures
 import json
 import os
+import platform
 import signal
 import subprocess
 import sys
@@ -66,6 +71,7 @@
 TESTDIR = ROOT / "lib-python" / "3" / "test"
 STDLIB_VERSION_FILE = ROOT / "lib-python" / "stdlib-version.txt"
 DEFAULT_BASELINE = HERE / "baseline.json"
+HOST_TAG = f"{sys.platform}-{platform.machine()}"
 
 EXE = ".exe" if sys.platform == "win32" else ""
 BIN_NAME = {"dynasm": "pyre-dynasm", "cranelift": "pyre-cranelift"}
@@ -453,11 +459,30 @@ def load_baseline(path: Path) -> dict:
     return json.loads(path.read_text(encoding="utf-8"))
 
 
-def expected_status(baseline: dict, module: str, backend: str) -> str | None:
-    entry = baseline.get("modules", {}).get(module)
-    if entry is None:
-        return None
-    return entry.get(backend) or entry.get("dynasm")
+def host_baseline_path(path: Path) -> Path:
+    """Per-host overlay beside the shared baseline, `baseline.<host>.json`.
+
+    Same shape as the shared file and consulted first, so a host records only
+    the modules it genuinely disagrees with.  The jit-stats baselines overlay
+    on `sys.platform` alone; a verdict also has to separate the architectures
+    within a platform, because the dynasm backend emits different machine code
+    on each and a miscompile is not portable.
+    """
+    return path.with_name(f"{path.stem}.{HOST_TAG}{path.suffix}")
+
+
+def expected_status(baseline: dict, overlay: dict, module: str,
+                    backend: str) -> str | None:
+    """Recorded verdict for `module`, the host overlay winning over the shared
+    file.  Within one file `dynasm` stands in for a backend with no entry."""
+    for source in (overlay, baseline):
+        entry = source.get("modules", {}).get(module)
+        if entry is None:
+            continue
+        status = entry.get(backend) or entry.get("dynasm")
+        if status is not None:
+            return status
+    return None
 
 
 # ── main ─────────────────────────────────────────────────────────────
@@ -518,6 +543,8 @@ def main() -> int:
         return 2
 
     baseline = load_baseline(args.baseline)
+    overlay_path = host_baseline_path(args.baseline)
+    overlay = load_baseline(overlay_path) if overlay_path.exists() else {"modules": {}}
     modules = discover_modules(args.filter)
 
     if args.list:
@@ -558,7 +585,7 @@ def main() -> int:
             off_platform.append((m, gate_reason))
             skipped.append(m)
             continue
-        exp = expected_status(baseline, m, args.backend)
+        exp = expected_status(baseline, overlay, m, args.backend)
         is_skip = (exp == "SKIP") or (m in KNOWN_SKIPS)
         if is_skip and not args.full and not args.update_baseline:
             skipped.append(m)
@@ -571,6 +598,10 @@ def main() -> int:
     print(f"pyre CPython suite — backend={args.backend} mode={args.mode} "
           f"jit={'off' if args.no_jit else 'on'} jobs={args.jobs}")
     print(f"binary: {binary}")
+    overlay_count = len(overlay.get("modules", {}))
+    print(f"baseline: {args.baseline.name} + "
+          + (f"{overlay_path.name} ({overlay_count} host entries)"
+             if overlay_count else f"no {HOST_TAG} overlay"))
     for m, reason in off_platform:
         print(f"  off-platform on {sys.platform}: {m} ({reason})")
     extra = f", {deselected} not gated (non-PASS)" if deselected else ""
@@ -632,7 +663,7 @@ def main() -> int:
     regressions: list[str] = []
     improvements: list[str] = []
     for m, (status, detail) in sorted(results.items()):
-        exp = expected_status(baseline, m, args.backend)
+        exp = expected_status(baseline, overlay, m, args.backend)
         if exp == "PASS" and status != "PASS":
             regressions.append(f"{m}: PASS -> {status}  {detail}")
         elif exp != "PASS" and status == "PASS":
@@ -659,9 +690,11 @@ def main() -> int:
         print(f"\nreport written: {args.report}")
 
     if args.update_baseline:
-        write_baseline(args.baseline, baseline, results, args.backend)
-        print(f"\nbaseline written: {args.baseline} "
-              f"({sum(1 for s, _ in results.values() if s == 'PASS')} PASS recorded)")
+        written = write_baseline(args.baseline, baseline, overlay, results,
+                                 args.backend)
+        recorded = sum(1 for s, _ in results.values() if s == "PASS")
+        for target in written:
+            print(f"\nbaseline written: {target} ({recorded} PASS recorded)")
         return 0
 
     if regressions:
@@ -682,25 +715,68 @@ def main() -> int:
     return 0
 
 
-def write_baseline(path: Path, baseline: dict, results: dict, backend: str) -> None:
+def write_baseline(path: Path, baseline: dict, overlay: dict, results: dict,
+                   backend: str) -> list[Path]:
+    """Record `results`, splitting them between the shared baseline and this
+    host's overlay.  Returns the files actually written.
+
+    A verdict the shared file has never seen establishes the shared answer, so
+    whichever host records first sets it and no host is privileged.  After
+    that a host writes to its own overlay only where it disagrees, and a run
+    that comes back into agreement drops the overlay entry again -- so an
+    overlay never outlives the divergence that created it.
+    """
     modules = baseline.setdefault("modules", {})
+    overlay_modules = overlay.setdefault("modules", {})
     baseline["stdlib_version"] = stdlib_version()
+    overlay_dirty = False
     for m, (status, _detail) in results.items():
         # Defensive: never overwrite another platform's recorded result.
         if platform_gate(m) is not None:
             continue
-        entry = modules.setdefault(m, {})
         # A curated KNOWN_SKIP stays SKIP regardless of what the run observed
         # (it is a "do not run" decision, not a result). Modules absent from
         # `results` (phantom skips that no longer exist) are simply not added.
+        # It is a decision about the module rather than about this host, so it
+        # is shared and never overlaid.
         if m in KNOWN_SKIPS:
+            entry = modules.setdefault(m, {})
             entry[backend] = "SKIP"
             entry.setdefault("reason", KNOWN_SKIPS[m])
-        else:
-            entry[backend] = status
+            overlay_dirty |= overlay_modules.pop(m, None) is not None
+            continue
+        shared = modules.get(m, {}).get(backend)
+        if shared is None:
+            modules.setdefault(m, {})[backend] = status
+            continue
+        if status == shared:
+            host_entry = overlay_modules.get(m)
+            if host_entry is not None and host_entry.pop(backend, None) is not None:
+                overlay_dirty = True
+                if not host_entry:
+                    del overlay_modules[m]
+            continue
+        if overlay_modules.setdefault(m, {}).get(backend) != status:
+            overlay_modules[m][backend] = status
+            overlay_dirty = True
+
+    written = [path]
     path.parent.mkdir(parents=True, exist_ok=True)
     path.write_text(json.dumps(baseline, indent=2, sort_keys=True) + "\n",
                     encoding="utf-8")
+    overlay_path = host_baseline_path(path)
+    if overlay_dirty or overlay_path.exists():
+        if overlay_modules:
+            overlay["host"] = HOST_TAG
+            overlay["stdlib_version"] = stdlib_version()
+            overlay_path.write_text(
+                json.dumps(overlay, indent=2, sort_keys=True) + "\n",
+                encoding="utf-8")
+            written.append(overlay_path)
+        elif overlay_path.exists():
+            overlay_path.unlink()
+            written.append(overlay_path)
+    return written
 
 
 if __name__ == "__main__":

pyre/pyre-interpreter/src/module/_blake2/_blake2_app.py

@@ -26,6 +26,19 @@ def __setattr__(cls, name, value):
 _MISSING = object()
 
 
+def _buffer_size(value):
+    """Byte length of a buffer parameter, the `Py_buffer.len` the clinic
+    converter measures.
+
+    Not `len()`: a memoryview over a wider itemsize counts items, so
+    `array('I', [0] * 5)` is 5 there and 20 bytes here, and only the byte
+    count decides whether the salt fits.
+    """
+    if type(value) is bytes:
+        return len(value)
+    return memoryview(value).nbytes
+
+
 def _make_blake_type(class_name, _salt_size, _person_size, _key_size,
                      _digest_size, max_offset, _block_size):
     class _Blake(metaclass=_Immutable):
@@ -99,6 +112,17 @@ def __new__(cls, *args, **kwargs):
                     "digest_size must be between 1 and %d bytes" %
                     cls.MAX_DIGEST_SIZE
                 )
+            # Salt and person are rejected before the tree parameters and the
+            # key after them, the order lib_pypy/_blake2 sets each field in.
+            # `blake2b(salt=b'x' * 17, fanout=256)` reports the salt.
+            if _buffer_size(salt) > cls.SALT_SIZE:
+                raise ValueError(
+                    "maximum salt length is %d bytes" % cls.SALT_SIZE
+                )
+            if _buffer_size(person) > cls.PERSON_SIZE:
+                raise ValueError(
+                    "maximum person length is %d bytes" % cls.PERSON_SIZE
+                )
             if not 0 <= fanout <= 255:
                 raise ValueError("fanout must be between 0 and 255")
             if not 1 <= depth <= 255:
@@ -118,6 +142,10 @@ def __new__(cls, *args, **kwargs):
                     "inner_size must be between 0 and %d" %
                     cls.MAX_DIGEST_SIZE
                 )
+            if _buffer_size(key) > cls.MAX_KEY_SIZE:
+                raise ValueError(
+                    "maximum key length is %d bytes" % cls.MAX_KEY_SIZE
+                )
 
             # Both clinic bool converters are observable through __bool__.
             bool(usedforsecurity)

pyre/pyre-interpreter/src/typedef.rs

@@ -2833,7 +2833,9 @@ fn module_descr_init(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError
         )));
     }
     let w_doc = w_doc.unwrap_or_else(pyre_object::w_none);
-    let name = unsafe { pyre_object::w_str_get_value(w_name) };
+    // `w_str_get_wtf8` rather than `w_str_get_value`: `module('\udcff')` is
+    // reachable from an import whose filename was surrogateescape-decoded.
+    let name = unsafe { pyre_object::w_str_get_wtf8(w_name) };
     unsafe { pyre_object::w_module_set_name(self_, name) };
     let w_dict = unsafe { pyre_object::w_module_get_w_dict(self_) };
     unsafe {

pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs

@@ -13133,13 +13133,13 @@ pub(crate) fn try_walker_load_global_cell_fold<Sym: WalkSym>(
     // when the name resolves there.  Requires the live frame operand.
     // The builtins fallback needs the module `pick_builtin(w_globals)` picks
     // (`frame.get_builtin()`).  A live frame supplies it directly and also lets
-    // us double-check the name is absent from the frame's AUTHORITATIVE globals
+    // us double-check the operand against the frame's AUTHORITATIVE globals
     // — `bh_load_global_fn` re-resolves the globals it consults from the LIVE
     // frame (`frame.get_w_globals()` when the frame owns `w_code`, else the
     // code's bound globals) and IGNORES the `namespace_ptr` operand.  The
-    // `ns_ptr` hint usually equals that live dict, but a present name there must
-    // resolve from globals (residual), not the builtin, or the fold would be
-    // wrong.  An INLINED callee has no materialised frame (`frame_ptr == 0`, its
+    // `ns_ptr` hint usually equals that live dict; when it does not, nothing
+    // here can prove what the residual would read, so decline.
+    // An INLINED callee has no materialised frame (`frame_ptr == 0`, its
     // `portal_frame_reg` unseeded); derive the builtin module from the concrete
     // globals' `__builtins__` cell instead — the same object `pick_builtin`
     // resolves (baseobjspace.rs:9716) and the one the interpreter fallback would
@@ -13157,10 +13157,15 @@ pub(crate) fn try_walker_load_global_cell_fold<Sym: WalkSym>(
                 pyre_interpreter::w_code_get_w_globals(w_code_ptr as pyre_object::PyObjectRef)
             }
         };
-        if !live_globals.is_null()
-            && live_globals as usize != w_globals as usize
-            && crate::state::module_dict_cell_slot_direct(live_globals, &name).is_some()
-        {
+        // Only the SAME dict makes the absence provable.  `module_dict_cell_slot_direct`
+        // answers `None` both for a name that is absent and for a dict it cannot
+        // read at all — a plain dict, or a module dict that ran
+        // `switch_to_object_strategy` — so on a different dict its `None` says
+        // nothing.  Guard (a) below pins `w_globals`' version, which watches the
+        // wrong dict in that case, and the residual it replaces resolves
+        // `live_globals`; a name present there would read the builtin instead of
+        // the global.
+        if live_globals.is_null() || live_globals as usize != w_globals as usize {
             return Ok(false);
         }
         frame.get_builtin()