FrameLocalsProxy subscript reads one locals-plus slot; executioncontext reverse_debugging fold

majit/majit-metainterp/src/jitprof.rs

@@ -185,23 +185,25 @@ struct TimingState {
 /// "a bridge aborted" sends the next reader looking for bridge activity that is
 /// not there.
 ///
-/// `ABORT_FORCE_QUASIIMMUT` is the one reason nothing on this side can raise,
-/// because the quasi-immutable *write* path is unimplemented. `hook_setfield`
-/// (`rclass.rs`) genops `jit_force_quasi_immutable`, but no `OpKind` carries it,
-/// `jtransform` has no arm for it, it has no jitcode opcode, and there is
-/// neither a metainterp opimpl nor a `do_force_quasi_immutable` — so unlike
-/// `opimpl_jit_force_quasi_immutable` (`pyjitpl.py:1094-1118`), which raises
-/// `SwitchToBlackhole(ABORT_FORCE_QUASIIMMUT)` once the mutate field is
-/// non-null, no path here reaches a `SwitchToBlackhole`. The *read* half
-/// (`record_quasiimmut_field` → [`majit_ir::OpCode::QuasiimmutField`]) is
-/// complete, so the asymmetry is in the port, not in this table.
+/// `ABORT_FORCE_QUASIIMMUT` has two producers, both sitting on the residual-call
+/// boundary, because that is where a quasi-immutable write lands here:
+/// `try_walker_force_quasi_immut_namespace_write` and
+/// `try_walker_force_quasi_immut_mapdict_write`
+/// (`pyre-jit-trace/src/jitcode_dispatch/residual_call.rs`). Each calls
+/// `note_force_quasi_immut_abort`, which stages the reason so the single
+/// `aborted_tracing` that follows tallies it here instead of in the `Generic`
+/// catch-all — the stand-in for the reason
+/// `opimpl_jit_force_quasi_immutable` (`pyjitpl.py:1094-1118`) carries on the
+/// `SwitchToBlackhole` instance it raises.
 ///
-/// That tally is therefore a true zero rather than a dead slot, and it stays
-/// zero for as long as the write path is missing. Nothing currently declares a
-/// `?`-suffixed field, so the gap is unreachable rather than latent — but
-/// `AbortReason` gains no variant for it either, so the first `?` field anyone
-/// declares gets a setfield hook with nothing downstream and no abort. Keep the
-/// slot; do not synthesise a producer for it.
+/// The mapdict producer runs only when `PYRE_QMUT_MAPDICT_FORCE` is present in
+/// the environment: `mapdict_qmut_force_enabled` asks `var_os(..).is_some()`,
+/// so an empty value arms it too, and a `PYRE_*` name set by a caller reaches
+/// the child.  Nothing in the tree sets it, and with it unset a run reaches
+/// this tally only through `try_walker_force_quasi_immut_namespace_write`;
+/// that path is live, and — again with the variable unset —
+/// `MAJIT_STATS=1 PYRE_MC_DIAG=1` on
+/// `bench/synth/trace_too_long_effect_replay.py` prints `abrt_force_qmut=1`.
 pub const ABORT_COUNTER_KINDS: &[(i32, &str)] = &[
     (counters::ABORT_TOO_LONG, "too_long"),
     (counters::ABORT_BRIDGE, "bridge_or_generic"),

pyre/bench/synth/getframe_inline_subwalk_multiframe.py

@@ -10,8 +10,7 @@
 # callee during tracing and prevents this loop from compiling.  The specialized
 # path therefore records the callee frame and creates its `FrameLocalsProxy`
 # without forcing the outer standard virtualizable; positive-depth lookup stays
-# on the established virtual-reference walk until its per-level lowering is
-# available.
+# on the established virtual-reference walk.
 #
 # A real PyPy run compiles one loop with no bridges, forcings, virtualizable
 # forcings, or aborts.  Pyre must print the same value, compile one loop, and
@@ -26,14 +25,27 @@
 # `bridges_compiled=0`, so the compiled loop leaves for the blackhole before it
 # ever crosses its back edge.  The attribution is a one-run check -- a `leaf`
 # holding only the depth-zero read records `guard_failures=1`, one holding only
-# the `_getframe(2)` read reproduces the full count.  The recorded
-# `guard_failures` is therefore a known shortfall of the positive-depth walk,
-# not a number to preserve; it is expected to fall once that walk gets its
-# per-level lowering.  `loops_compiled` counts two traces here: `main`'s loop,
-# and a linear `Finish` trace for the inlined `mid` -> `leaf` chain, whose own
-# four `GuardNotForced`s never fail.  The wasm baseline records
-# `guard_failures=1` instead because that backend always materializes the
-# virtualizable, which makes `GuardNotForced` a no-op there.
+# the `_getframe(2)` read reproduces the full count.
+#
+# The count is a known shortfall, not a number to preserve -- but it will not
+# come down from the `sys._getframe` arm.  Letting the inline level take that
+# arm's own per-level lowering, by deleting the decline in front of it, was
+# measured: the fixture still answers correctly, `guard_failures` stays at
+# exactly the recorded value, and the `Finish` trace gains three
+# `GuardNotForced`s, because the lowering forces once per hop rather than not
+# at all.  Reaching `main`'s frame object from `leaf` means materializing it
+# however the walk is spelled.
+#
+# What removes the cost is inlining `mid` -> `leaf` into `main`'s loop, where
+# the depth-2 lookup lands on the trace's own virtualizable and needs no force:
+# `PYPYLOG=jit-summary:<file> pypy3 -P` on this file reports one loop, no
+# bridges, `forcings: 0` and no aborts, against the two traces pyre records.
+#
+# `loops_compiled` counts those two traces: `main`'s loop, and a linear
+# `Finish` trace for the inlined `mid` -> `leaf` chain, whose own four
+# `GuardNotForced`s never fail.  The wasm baseline records `guard_failures=1`
+# instead because that backend always materializes the virtualizable, which
+# makes `GuardNotForced` a no-op there.
 import sys
 
 

pyre/extra_tests/parity_tests/framelocalsproxy_delete_slot.py

@@ -0,0 +1,50 @@
+# CPython-suite gap: `test_frame` deletes through the proxy only to check that
+# a fast local refuses, so nothing in the suite pins what `del` and `pop`
+# report for a key that names no slot, or for a key that cannot be hashed.
+#
+# `framelocalsproxy_setitem` with no value, and `framelocalsproxy_pop`, both
+# resolve the key through the same scan the subscript uses, and that scan
+# hashes the key before it looks at any name.  Probing a materialized snapshot
+# first instead gets the refusal right and everything else wrong: the miss
+# arrives as the snapshot's `KeyError`, an unhashable key arrives in the dict's
+# terms, and for `pop` the discarded probe turns the hash's `TypeError` into a
+# `KeyError` naming the unhashable key.
+#
+# parity-tests reason: every line below is exception text, which is only worth
+# anything next to the runtime it has to agree with.
+import sys
+
+
+def report(label, call):
+    try:
+        print(label, "->", call())
+    except Exception as exc:
+        print(label, "->", type(exc).__name__, exc.args)
+
+
+def delete_and_pop():
+    bound = 1  # noqa: F841 - named through the proxy below
+    proxy = sys._getframe(0).f_locals
+    # A key the frame has a slot for is refused whichever way it is asked.
+    report("del local", lambda: proxy.__delitem__("bound"))
+    report("pop local", lambda: proxy.pop("bound"))
+    # The hash runs before the scan, so neither of these reaches the extras
+    # dict to be described in its terms.
+    report("del unhashable", lambda: proxy.__delitem__(["unhashable"]))
+    report("pop unhashable", lambda: proxy.pop(["unhashable"]))
+    # A frame with no extras dict yet reports the key itself, or the default.
+    report("del absent", lambda: proxy.__delitem__("absent"))
+    report("pop absent", lambda: proxy.pop("absent"))
+    report("pop absent default", lambda: proxy.pop("absent", "fallback"))
+    # Once the extras dict exists it answers both, and a second delete of the
+    # same name is a miss again.
+    proxy["extra"] = 7
+    report("pop extra", lambda: proxy.pop("extra"))
+    proxy["extra"] = 8
+    report("del extra", lambda: proxy.__delitem__("extra"))
+    report("del extra again", lambda: proxy.__delitem__("extra"))
+    print("bound untouched", bound)
+
+
+delete_and_pop()
+print("OK")

pyre/extra_tests/parity_tests/framelocalsproxy_getitem_slot.py

@@ -0,0 +1,145 @@
+# CPython-suite gap: `test_frame` reads `frame.f_locals[name]` only for names
+# that are bound, so nothing in the suite pins what the proxy reports for an
+# unbound slot, for a name it does not carry at all, or for a key that cannot
+# be hashed.
+#
+# `framelocalsproxy_getitem` resolves the key to a locals-plus index and reads
+# that one slot.  Answering the same lookup by materializing the whole mapping
+# and subscripting it gets the value right and everything else wrong: the miss
+# arrives as the mapping's own `KeyError(key)`, and an unhashable key arrives
+# as the mapping's key-flavoured `TypeError` rather than the one the hash
+# raises before any slot is examined.
+#
+# The same scan answers a write, and it hashes the key before it looks at any
+# name: a key that hashes like nothing the frame carries names no slot, however
+# it compares.
+#
+# parity-tests reason: the value a hit returns is identical either way, so a
+# snippet that reads a bound local cannot see the difference.  What separates
+# the two shapes is exception text, which is only worth anything next to the
+# runtime it has to agree with.
+import sys
+
+
+def scalar_slots():
+    bound = 1
+    if bound == 0:
+        unbound = 2  # noqa: F841 - compiled into a slot that is never bound
+    proxy = sys._getframe(0).f_locals
+    print("bound", proxy["bound"])
+    for name in ("unbound", "absent"):
+        try:
+            proxy[name]
+        except KeyError as exc:
+            print(name, "KeyError", exc.args)
+    try:
+        proxy[["unhashable"]]
+    except TypeError as exc:
+        print("unhashable", "TypeError", exc)
+    # A name the frame has no slot for is stored in, and read back from, the
+    # frame's separate extras mapping.
+    proxy["extra"] = 7
+    print("extra", proxy["extra"])
+
+
+class SameHash:
+    """A non-`str` key that both hashes and compares like the name it holds."""
+
+    def __init__(self, name):
+        self.name = name
+
+    def __hash__(self):
+        return hash(self.name)
+
+    def __eq__(self, other):
+        return other == self.name
+
+    def __repr__(self):
+        # The miss below reports the key with `%R`, so the default repr would
+        # put this object's address in the message.
+        return f"<key {self.name}>"
+
+
+class OtherHash(SameHash):
+    """The same key, hashing like nothing the frame carries."""
+
+    def __hash__(self):
+        return hash(self.name) ^ 1
+
+
+def non_str_keys():
+    bound = 1  # noqa: F841 - read through the proxy below
+    proxy = sys._getframe(0).f_locals
+    print("same-hash", proxy[SameHash("bound")])
+    # The scan compares a name only when its hash matches the key's, so a key
+    # that claims equality with a name it does not hash like never reaches the
+    # comparison and reads as absent.
+    key = OtherHash("bound")
+    print("hashes differ", hash(key) != hash(key.name))
+    try:
+        proxy[key]
+    except KeyError as exc:
+        print("other-hash", "KeyError", exc.args)
+
+
+def non_str_key_writes():
+    bound = 1
+    proxy = sys._getframe(0).f_locals
+    # The scan hashes the key before looking at any name, so the key never
+    # reaches the extras dict to be reported in its terms.
+    try:
+        proxy[["unhashable"]] = 1
+    except TypeError as exc:
+        print("write unhashable", "TypeError", exc)
+    proxy[SameHash("bound")] = 2
+    print("same-hash write", bound, proxy["bound"])
+    # This one is filtered out by the hash before the comparison, so it names
+    # no slot and is stored in the extras dict under the key object itself.
+    proxy[OtherHash("bound")] = 3
+    print("other-hash write", bound, proxy["bound"])
+    print("extras keys", sorted(repr(key) for key in proxy if not isinstance(key, str)))
+
+
+def hidden_slot():
+    # PEP 709 inlines a comprehension into its enclosing scope, and in a class
+    # body the iteration variable becomes a hidden slot.  Hidden is a property
+    # of the write direction only: the scan skips such a slot when it is
+    # looking for somewhere to store, so the assignment below goes to the
+    # extras dict, but the read that follows still reports the live slot.
+    def probe():
+        proxy = sys._getframe(1).f_locals
+        before = proxy["i"]
+        proxy["i"] = 99
+        return before, proxy["i"]
+
+    class Body:
+        seen = [probe() for i in range(2)]
+
+    print("hidden", Body.seen, "leaked" if "i" in Body.__dict__ else "not leaked")
+
+
+def cell_and_free_slots():
+    captured = "cell"
+
+    def inner():
+        own = "own"
+        proxy = sys._getframe(0).f_locals
+        # `captured` has to be named in this body for the compiler to make it a
+        # freevar; reading it only through the proxy would leave it out of the
+        # locals-plus table entirely.
+        print("free", proxy["captured"], captured, "local", proxy["own"])
+        return own
+
+    # `captured` is a varname of this frame AND a cellvar, so the slot holds
+    # the cell and a reader has to dereference it; in `inner` the same name is
+    # a freevar, which lands past every varname in the locals-plus order.
+    print("cell", sys._getframe(0).f_locals["captured"])
+    inner()
+
+
+scalar_slots()
+non_str_keys()
+non_str_key_writes()
+hidden_slot()
+cell_and_free_slots()
+print("OK")

pyre/extra_tests/snippets/compile_universal_newline.py

@@ -0,0 +1,104 @@
+# pyre-check: gate=1
+"""A source's line terminators are all `\n` by the time the tokenizer sees it.
+
+`pytokenizer.py:654-662` universal_newline rewrites a line ending in `\r\n` or
+in a lone `\r` to one ending in `\n`, and `generate_tokens` calls it on every
+line it takes from `splitlines(True)` (`pyparse.py:202`).  So the rewrite is not
+string syntax and not a property of one entry point: it reaches a file, a
+`compile()` argument, `ast.parse` and `-c` alike, and the text of a
+triple-quoted literal along with the code around it.
+
+Neither rewrite moves a line boundary, so the line a statement reports is the
+one it reported before.
+"""
+
+import ast
+import os
+import subprocess
+import sys
+import tempfile
+
+CRLF = "x = \"\"\"a\r\nb\"\"\""
+LONE_CR = "x = \"\"\"a\rb\"\"\""
+
+
+def value_of(source, name="x"):
+    namespace = {}
+    exec(compile(source, "<test>", "exec"), namespace)
+    return namespace[name]
+
+
+# The literal spans the terminator, so its own text carries the rewrite.
+assert value_of(CRLF) == "a\nb", repr(value_of(CRLF))
+assert value_of(LONE_CR) == "a\nb", repr(value_of(LONE_CR))
+
+# A terminator is not an escape, so being raw or bytes changes nothing.
+assert value_of("x = r\"\"\"a\rb\"\"\"") == "a\nb", repr(value_of("x = r\"\"\"a\rb\"\"\""))
+assert value_of("x = b\"\"\"a\r\nb\"\"\"") == b"a\nb", repr(value_of("x = b\"\"\"a\r\nb\"\"\""))
+
+# `ast.parse` reads the same rewritten source, so the constant it carries and
+# the segment `end_col_offset` describes agree with it.
+tree = ast.parse(CRLF)
+assert tree.body[0].value.value == "a\nb", repr(tree.body[0].value.value)
+assert ast.parse(LONE_CR).body[0].value.value == "a\nb"
+
+# Statements still land on the lines they were written on.
+namespace = {}
+exec(compile("x = 1\r\ny = 2\r\nz = 3\r", "<test>", "exec"), namespace)
+assert (namespace["x"], namespace["y"], namespace["z"]) == (1, 2, 3), namespace
+try:
+    exec(compile("x = 1\rraise ValueError('boom')", "<test>", "exec"), {})
+except ValueError as exc:
+    assert exc.__traceback__.tb_next.tb_lineno == 2, exc.__traceback__.tb_next.tb_lineno
+else:
+    raise AssertionError("the raise did not run")
+
+# A failed compile reports the offending line, and that line came from the
+# rewritten source too.  How the reference spells the terminator is its own
+# business -- CPython drops it and PyPy keeps `\n` -- so what is pinned here is
+# only that no carriage return survives into it.
+try:
+    compile("x = 1\r\ny = (\r\nz = 3\r\n", "<test>", "exec")
+except SyntaxError as exc:
+    assert exc.lineno == 2, exc.lineno
+    assert exc.text is not None and "\r" not in exc.text, repr(exc.text)
+    assert exc.text.rstrip("\r\n") == "y = (", repr(exc.text)
+else:
+    raise AssertionError("the unclosed paren did not raise")
+
+# A lone `\r` reaches the same slicing.  Before the rewrite ran on the string
+# the report is sliced from, this source held no `\n` at all, so the offending
+# line came back as `None` rather than as the second line.
+try:
+    compile("a = 1\rb = (\r", "<test>", "exec")
+except SyntaxError as exc:
+    assert exc.lineno == 2, exc.lineno
+    assert exc.text is not None and "\r" not in exc.text, repr(exc.text)
+    assert exc.text.rstrip("\r\n") == "b = (", repr(exc.text)
+else:
+    raise AssertionError("the unclosed paren did not raise")
+
+# The same source reaching the compiler as a `-c` argument, and as a file.
+PROGRAM = CRLF + "\nprint(repr(x))\n"
+
+completed = subprocess.run(
+    [sys.executable, "-c", PROGRAM],
+    capture_output=True,
+    text=True,
+)
+assert completed.returncode == 0, completed.stderr
+assert completed.stdout.strip() == "'a\\nb'", completed.stdout
+
+# Written as bytes so the carriage returns reach the file itself rather than
+# whatever the platform's text mode would spell them as.
+handle, path = tempfile.mkstemp(suffix=".py")
+os.write(handle, PROGRAM.encode())
+os.close(handle)
+try:
+    completed = subprocess.run([sys.executable, path], capture_output=True, text=True)
+    assert completed.returncode == 0, completed.stderr
+    assert completed.stdout.strip() == "'a\\nb'", completed.stdout
+finally:
+    os.unlink(path)
+
+print("OK")