mmap: bound find/rfind by the needle rather than clamping the subtraction

majit/majit-translate/src/inline.rs

@@ -1127,33 +1127,12 @@ pub fn op_variable_refs(kind: &OpKind) -> Vec<crate::flowspace::model::Variable>
     }
 }
 
-/// `true` iff `kind` is side-effect-free and may be removed from the
-/// graph when its result has no readers.  Direct port of RPython
-/// `simplify.py:405-417 CanRemove` set + the lltype-level
-/// `lloperation.enum_ops_without_sideeffects()` extension that
-/// `simplify.py:414-416` unions in.
+/// `true` iff `kind` is side-effect-free for the existing folding,
+/// CSE, and control-flow-shape consumers of this predicate.
 ///
-/// Pure ops correspond to RPython:
-///   - flowspace pure: `add sub mul div mod ... lt le eq ... bool len
-///     hash getattr getitem`-family (`simplify.py:407-412`).
-///   - lltype pure: `int_add int_lt ... getfield(_pure)
-///     getarrayitem(_pure) getinteriorfield ... cast_*` from
-///     `enum_ops_without_sideeffects`.
-///
-/// Side-effecting ops correspond to RPython `setfield setarrayitem
-/// setinteriorfield` (writes), `direct_call indirect_call` (calls
-/// without elidable EI), every `*guard*` opname (control-flow guards),
-/// and the JIT marker family (`jit_marker`, `debug_merge_point`,
-/// `loop_header`, `live`, `record_known_result`,
-/// `record_quasiimmut_field`, `jit_debug`).
-///
-/// Used by `model::prune_dead_phis` to mirror PyPy
-/// `simplify.py:441-445`'s split: pure ops route their args via
-/// `dependencies[op.result] += op.args` (args become live only if
-/// the result becomes live), while non-pure ops add their args
-/// directly to `read_vars` (args always live).  Without the split,
-/// a phi feeding only a dead pure op would be kept alive via the
-/// pure op's args even though both should die together.
+/// Dead-operation removal uses [`can_remove_op`] instead.  RPython keeps
+/// `LLOp.is_pure()` (`lloperation.py:82-93`) and `CanRemove`
+/// (`simplify.py:411-423`) as distinct predicates.
 pub fn is_pure_op(kind: &OpKind) -> bool {
     match kind {
         // `new` / `new_with_vtable` / `new_array_clear` heap-allocate fresh
@@ -1215,19 +1194,18 @@ pub fn is_pure_op(kind: &OpKind) -> bool {
         | OpKind::VtableMethodPtr { .. }
         // `newtuple` is `PureOperation` (`operation.py:542-548`).
         | OpKind::NewTuple { .. }
-        // `newlist` is `PureOperation` — a fresh list allocation has no
-        // observable effect on existing state.
+        // `newlist` subclasses `HLOperation` (`operation.py:551-557`), not
+        // `PureOperation`; its DCE authorization comes from the high-level
+        // `simplify.py:411-418 CanRemove` list alone.
         | OpKind::NewList { .. }
-        // `getslice` is a `PureOperation` (`operation.py:461`,
-        // `pure=True`) — the slice copy reads the source and allocates a
-        // fresh list, with no observable effect on existing state.
+        // `getslice` is registered `pure=True` for flowspace folding/CSE
+        // (`operation.py:461`), but its possible exception excludes it from
+        // dead-op removal; see `can_remove_op`.
         | OpKind::GetSlice { .. }
-        // `isinstance` lowers to `int_between` over `obj.typeptr`'s
-        // subclass-range fields plus an optional null branch — all
-        // pure reads, classified `canfold=True` upstream
-        // (`lloperation.py instance_isinstance`).  Keeping dead
-        // `IsInstance` results alive would block prune_dead_phis Step
-        // 5 even though the predicate is side-effect-free.
+        // The high-level `isinstance` entry in `simplify.py:411-418`
+        // `CanRemove` authorizes DCE.  Its lowering reads subclass-range
+        // fields and emits `int_between`; there is no
+        // `instance_isinstance` lloperation row.
         | OpKind::IsInstance { .. }
         // `LoadStatic` reads a `static` declaration's compile-time
         // address — equivalent to `LOAD_GLOBAL` → Constant lookup,
@@ -1309,6 +1287,22 @@ pub fn is_pure_op(kind: &OpKind) -> bool {
     }
 }
 
+/// `true` iff `kind` may be removed when its result has no readers.
+/// Models RPython `simplify.py:411-423 CanRemove`, including the default
+/// `raising_is_ok=False` used by `enum_ops_without_sideeffects()`.
+///
+/// This is deliberately a restriction of [`is_pure_op`], so switching the
+/// dead-op pass to this predicate cannot authorize any new removal.
+pub fn can_remove_op(kind: &OpKind) -> bool {
+    match kind {
+        // `getslice` is absent from `simplify.py:411-418 CanRemove` and is
+        // raising at `lloperation.py:578`, so
+        // `enum_ops_without_sideeffects()` does not add it either.
+        OpKind::GetSlice { .. } => false,
+        _ => is_pure_op(kind),
+    }
+}
+
 /// Whitelist of `OpKind::UnaryOp` opnames that are side-effect-free
 /// upstream — direct port of the unary entries in
 /// `simplify.CanRemove` (`rpython/translator/simplify.py:405-417`)
@@ -1418,11 +1412,10 @@ fn is_pure_binop_opname(opname: &str) -> bool {
     // Pyre's BinOp arrives here pre-rtyper (frontend names like
     // `add`); the post-rtyper shape is also accepted so a future
     // post-rtyper DCE call site requires no further widening.
-    let prefix_match = opname
+    let integer_prefix_match = opname
         .strip_prefix("int_")
-        .or_else(|| opname.strip_prefix("uint_"))
-        .or_else(|| opname.strip_prefix("float_"));
-    if let Some(suffix) = prefix_match {
+        .or_else(|| opname.strip_prefix("uint_"));
+    if let Some(suffix) = integer_prefix_match {
         return matches!(
             suffix,
             "add"
@@ -1443,6 +1436,15 @@ fn is_pure_binop_opname(opname: &str) -> bool {
                 | "ge"
         );
     }
+    if let Some(suffix) = opname.strip_prefix("float_") {
+        // `lloperation.py:246-261` defines only arithmetic through true
+        // division and the six comparisons, and explicitly leaves
+        // `float_mod` to `math.fmod`.
+        return matches!(
+            suffix,
+            "add" | "sub" | "mul" | "truediv" | "lt" | "le" | "eq" | "ne" | "gt" | "ge"
+        );
+    }
     false
 }
 
@@ -1559,6 +1561,42 @@ mod tests {
     use crate::model::{CallTarget, FunctionGraph, OpKind, ValueType};
     use crate::parse::CallPath;
 
+    #[test]
+    fn dead_op_removal_and_float_purity_use_their_rpython_tables() {
+        let getslice = OpKind::GetSlice { args: vec![] };
+        assert!(is_pure_op(&getslice));
+        assert!(!can_remove_op(&getslice));
+
+        for name in [
+            "float_add",
+            "float_sub",
+            "float_mul",
+            "float_truediv",
+            "float_lt",
+            "float_le",
+            "float_eq",
+            "float_ne",
+            "float_gt",
+            "float_ge",
+        ] {
+            assert!(is_pure_binop_opname(name), "{name} is an lltype float op");
+        }
+        for name in [
+            "float_floordiv",
+            "float_mod",
+            "float_lshift",
+            "float_rshift",
+            "float_and",
+            "float_or",
+            "float_xor",
+        ] {
+            assert!(
+                !is_pure_binop_opname(name),
+                "{name} is absent from the lltype float table"
+            );
+        }
+    }
+
     #[test]
     fn lowered_blackhole_op_purity_is_opname_aware() {
         let pure = |name: &str| {

majit/majit-translate/src/model.rs

@@ -3894,7 +3894,7 @@ pub(crate) fn prune_dead_boxing_remnants(graph: &mut FunctionGraph) -> usize {
 ///      `Block::canraise()` mirrors the upstream
 ///      `block.exitswitch is c_last_exception` check; the raising
 ///      op is the last entry in `block.operations`.
-///        - If `is_pure_op(kind)` AND the op is not the raising_op
+///        - If `can_remove_op(kind)` AND the op is not the raising_op
 ///          AND `op.result.is_some()`: route operands via
 ///          `dependencies[op.result] += operands`
 ///          (`simplify.py:444-445 dependencies[op.result].
@@ -3979,7 +3979,7 @@ pub(crate) fn prune_dead_boxing_remnants(graph: &mut FunctionGraph) -> usize {
     reason = "Eq and Hash use immutable identity/value data; interior mutation is excluded, matching RPython identity-keyed dict semantics"
 )]
 pub fn prune_dead_phis(graph: &mut FunctionGraph) {
-    use crate::inline::is_pure_op;
+    use crate::inline::can_remove_op;
     use std::collections::HashMap;
     let start = graph.startblock;
     let return_block = graph.returnblock;
@@ -4110,7 +4110,7 @@ pub fn prune_dead_phis(graph: &mut FunctionGraph) {
             // `simplify.py:441-445`:
             //   if not canremove(op, block):    read_vars.update(args)
             //   else:                           dependencies[result] += args
-            let removable = is_pure_op(&op.kind) && Some(i) != raising_op_idx;
+            let removable = can_remove_op(&op.kind) && Some(i) != raising_op_idx;
             if let Some(result_var) = op.result.clone()
                 && removable
             {
@@ -4231,7 +4231,7 @@ pub fn prune_dead_phis(graph: &mut FunctionGraph) {
                 Some(r) => !read_vars.contains(r),
                 None => false,
             };
-            if dead && is_pure_op(&op.kind) && Some(i) != raising_op_idx {
+            if dead && can_remove_op(&op.kind) && Some(i) != raising_op_idx {
                 dead_op_positions.push((block.id, i));
             }
         }
@@ -6714,6 +6714,38 @@ mod tests {
         );
     }
 
+    #[test]
+    fn prune_dead_phis_keeps_dead_raising_getslice() {
+        let mut graph = FunctionGraph::new("test");
+        let entry = graph.startblock;
+        let list = graph
+            .push_op_var(entry, OpKind::ConstRefNull, true)
+            .unwrap();
+        let start = graph.push_op_var(entry, OpKind::ConstInt(0), true).unwrap();
+        let stop = graph.push_op_var(entry, OpKind::ConstInt(1), true).unwrap();
+        let slice = graph
+            .push_op_var(
+                entry,
+                OpKind::GetSlice {
+                    args: vec![list, start, stop],
+                },
+                true,
+            )
+            .unwrap();
+        graph.set_return(entry, None);
+
+        prune_dead_phis(&mut graph);
+
+        assert!(
+            graph
+                .block(entry)
+                .operations
+                .iter()
+                .any(|op| op.result.as_ref() == Some(&slice)),
+            "dead getslice must remain because `simplify.CanRemove` excludes it"
+        );
+    }
+
     #[test]
     fn remove_dead_aggregates_drops_discarded_ctor_and_its_field_stores() {
         // entry: tmp = SyntheticTransparentCtor("Tuple");

pyre/extra_tests/parity_tests/mmap_find_span_shorter_than_needle.py

@@ -0,0 +1,77 @@
+# CPython-suite gap: test_mmap's exhaustive find/rfind sweep (test_find_end,
+# test_rfind) never uses an empty pattern — its list is
+# [b"o", b"on", b"two", b"ones", b"s"] — so the near-end answer an empty needle
+# owes is untested there. The oversized-needle cases below are covered by that
+# sweep, and are kept only to hold the two halves of one bound together.
+# parity-tests reason: test_mmap is not in the suite gate (still 3 failures and
+# 21 errors), so nothing in the vendored suite protects either half today.
+
+"""`mmap.find`/`rfind` over a span that cannot hold the needle report -1.
+
+The scan's upper bound is `span - len(needle)`. Clamping that subtraction at
+zero still leaves one index to try, and reading a needle-sized window there
+runs off the end of a shorter span, so the interpreter aborts instead of
+answering.
+
+The empty needle is the boundary in the other direction, and it is the half
+with no oracle in the vendored suite: it matches at the near end of the span —
+`start` for `find`, `end` for `rfind` — where a shared "empty or inverted"
+guard would fold it into -1 along with the spans that really have no room.
+
+Every case pins the value rather than the absence of a panic, so the fixture
+keeps its meaning once the abort is gone.
+"""
+
+import mmap
+
+m = mmap.mmap(-1, 4)
+m[:] = b"abca"
+
+# The empty needle matches at the near end of the span.
+assert m.find(b"") == 0, m.find(b"")
+assert m.rfind(b"") == 4, m.rfind(b"")
+assert m.find(b"", 2) == 2, m.find(b"", 2)
+assert m.rfind(b"", 0, 2) == 2, m.rfind(b"", 0, 2)
+assert m.find(b"", 4) == 4, m.find(b"", 4)
+assert m.rfind(b"", 4) == 4, m.rfind(b"", 4)
+assert m.find(b"", -2) == 2, m.find(b"", -2)
+assert m.rfind(b"", -2) == 4, m.rfind(b"", -2)
+
+# An inverted span holds nothing at all, not even the empty needle.
+assert m.find(b"", 3, 1) == -1, m.find(b"", 3, 1)
+assert m.rfind(b"", 3, 1) == -1, m.rfind(b"", 3, 1)
+
+# The needle is longer than the whole map.
+assert m.find(b"abcab") == -1, m.find(b"abcab")
+assert m.rfind(b"abcab") == -1, m.rfind(b"abcab")
+
+# The needle fits the map but not the requested span.
+assert m.find(b"abc", 2) == -1, m.find(b"abc", 2)
+assert m.rfind(b"abc", 2) == -1, m.rfind(b"abc", 2)
+assert m.find(b"abc", 0, 2) == -1, m.find(b"abc", 0, 2)
+assert m.rfind(b"abc", 0, 2) == -1, m.rfind(b"abc", 0, 2)
+
+# A span exactly the needle's length still has one candidate.
+assert m.find(b"bc", 1, 3) == 1, m.find(b"bc", 1, 3)
+assert m.rfind(b"bc", 1, 3) == 1, m.rfind(b"bc", 1, 3)
+
+# The ordinary answers, so a bound that returns -1 too eagerly is caught too.
+assert m.find(b"a") == 0, m.find(b"a")
+assert m.rfind(b"a") == 3, m.rfind(b"a")
+assert m.find(b"ca") == 2, m.find(b"ca")
+assert m.find(b"a", -1) == 3, m.find(b"a", -1)
+assert m.find(b"abca", -10) == 0, m.find(b"abca", -10)
+
+m.close()
+
+# A one-byte map is the smallest span an oversized needle can overrun.
+one = mmap.mmap(-1, 1)
+one[:] = b"a"
+assert one.find(b"ab") == -1, one.find(b"ab")
+assert one.rfind(b"ab") == -1, one.rfind(b"ab")
+assert one.find(b"") == 0, one.find(b"")
+assert one.rfind(b"") == 1, one.rfind(b"")
+assert one.find(b"a") == 0, one.find(b"a")
+one.close()
+
+print("OK")

pyre/pyre-interpreter/src/builtins.rs

@@ -78,7 +78,7 @@ pub(crate) unsafe fn backing_exports_incref(buffer: &pyre_object::buffer::Buffer
                 // releasing the view it came from would let `close`/`resize`
                 // unmap while this one still reads the mapping.
                 let _ = w_obj;
-                #[cfg(all(unix, not(feature = "sandbox")))]
+                #[cfg(all(any(unix, windows), not(feature = "sandbox")))]
                 if crate::module::mmap::interp_mmap::is_mmap(*w_obj) {
                     crate::module::mmap::interp_mmap::mmap_exports_incref(*w_obj);
                 }
@@ -114,7 +114,7 @@ pub(crate) unsafe fn buffer_export_incref(obj: PyObjectRef) -> bool {
             pyre_object::memoryview::w_memoryview_exports_incref(obj);
             return true;
         }
-        #[cfg(all(unix, not(feature = "sandbox")))]
+        #[cfg(all(any(unix, windows), not(feature = "sandbox")))]
         if crate::module::mmap::interp_mmap::is_mmap(obj) {
             crate::module::mmap::interp_mmap::mmap_exports_incref(obj);
             return true;
@@ -137,7 +137,7 @@ pub(crate) unsafe fn buffer_export_decref(obj: PyObjectRef) {
         } else if pyre_object::memoryview::is_w_memoryview(obj) {
             pyre_object::memoryview::w_memoryview_exports_decref(obj);
         } else {
-            #[cfg(all(unix, not(feature = "sandbox")))]
+            #[cfg(all(any(unix, windows), not(feature = "sandbox")))]
             crate::module::mmap::interp_mmap::mmap_exports_decref(obj);
         }
     }
@@ -394,7 +394,7 @@ pub(crate) fn w_memoryview_new_simple_with_owner(
 
 /// Build the `W_MMap.readbuf_w`/`writebuf_w` view: one contiguous external
 /// byte window whose owner remains the mmap object.
-#[cfg(all(unix, not(feature = "sandbox")))]
+#[cfg(all(any(unix, windows), not(feature = "sandbox")))]
 unsafe fn w_memoryview_new_mmap(
     w_obj: PyObjectRef,
     address: usize,
@@ -657,7 +657,7 @@ fn w_memoryview_new_with_flags_impl(
             // keeps its zero-copy window and derived geometry.
             return Ok(w_memoryview_new_derived(w_obj, |v| v.clone()));
         }
-        #[cfg(all(unix, not(feature = "sandbox")))]
+        #[cfg(all(any(unix, windows), not(feature = "sandbox")))]
         if let Some(view) = crate::module::mmap::interp_mmap::mmap_buffer_view(w_obj) {
             let (address, length, readonly) = view?;
             return Ok(w_memoryview_new_mmap(w_obj, address, length, readonly));
@@ -1781,7 +1781,7 @@ fn memoryview_repr(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError>
 
 /// Drop an mmap-backed view's export directly, bypassing any Python-callable
 /// release.  Returns `true` when it handled an mmap backing.
-#[cfg(all(unix, not(feature = "sandbox")))]
+#[cfg(all(any(unix, windows), not(feature = "sandbox")))]
 unsafe fn release_external_backing(backing: PyObjectRef) -> bool {
     if crate::module::mmap::interp_mmap::is_mmap(backing) {
         unsafe { crate::module::mmap::interp_mmap::mmap_exports_decref(backing) };
@@ -1790,7 +1790,7 @@ unsafe fn release_external_backing(backing: PyObjectRef) -> bool {
     false
 }
 
-#[cfg(not(all(unix, not(feature = "sandbox"))))]
+#[cfg(not(all(any(unix, windows), not(feature = "sandbox"))))]
 unsafe fn release_external_backing(_backing: PyObjectRef) -> bool {
     false
 }
@@ -15547,7 +15547,7 @@ unsafe fn fileio_writebuf(
                 obj,
             ));
         }
-        #[cfg(all(unix, not(feature = "sandbox")))]
+        #[cfg(all(any(unix, windows), not(feature = "sandbox")))]
         if let Some(view) = crate::module::mmap::interp_mmap::mmap_buffer_view(obj) {
             let (address, length, readonly) = view?;
             if !readonly {