Skip to content

Commit 527c86b

Browse files
committed
mmap: bound find/rfind by the needle rather than clamping the subtraction
The scan's upper bound is `span - len(needle)`, written as a saturating subtraction. When the needle is longer than the span that clamps to 0, which still leaves index 0 to try, and reading a needle-sized window there indexes past the end of the span: `mmap.find(b"ab")` on a one-byte map aborts the interpreter with "range end index 2 out of range for slice of length 1". Reject the case before the scan instead, in both find and rfind, and give the empty needle its own answer: it matches at the near end of the span, `start` for find and `end` for rfind, where the shared `start >= end || is_empty` guard used to fold it into -1 along with the inverted-span case. test_mmap stops aborting and reports its remaining failures normally (51 tests, 3 failures + 21 errors). test_mmap's own sweep already covers the oversized needle — `test_find_end` walks every start/end pair against `bytes.find` as the oracle, which is where the abort came from — but its pattern list has no empty needle, and that module is not in the suite gate. The parity fixture therefore carries the empty-needle half, keeps the oversized cases beside it so the two halves of one bound stay together, and pins values rather than the absence of the abort. It fails on the unpatched binary. Verified on dynasm; parity_tests green. The cranelift binary in this tree predates the change. Assisted-by: Claude
1 parent 424da05 commit 527c86b

2 files changed

Lines changed: 93 additions & 4 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# CPython-suite gap: test_mmap's exhaustive find/rfind sweep (test_find_end,
2+
# test_rfind) never uses an empty pattern — its list is
3+
# [b"o", b"on", b"two", b"ones", b"s"] — so the near-end answer an empty needle
4+
# owes is untested there. The oversized-needle cases below are covered by that
5+
# sweep, and are kept only to hold the two halves of one bound together.
6+
# parity-tests reason: test_mmap is not in the suite gate (still 3 failures and
7+
# 21 errors), so nothing in the vendored suite protects either half today.
8+
9+
"""`mmap.find`/`rfind` over a span that cannot hold the needle report -1.
10+
11+
The scan's upper bound is `span - len(needle)`. Clamping that subtraction at
12+
zero still leaves one index to try, and reading a needle-sized window there
13+
runs off the end of a shorter span, so the interpreter aborts instead of
14+
answering.
15+
16+
The empty needle is the boundary in the other direction, and it is the half
17+
with no oracle in the vendored suite: it matches at the near end of the span —
18+
`start` for `find`, `end` for `rfind` — where a shared "empty or inverted"
19+
guard would fold it into -1 along with the spans that really have no room.
20+
21+
Every case pins the value rather than the absence of a panic, so the fixture
22+
keeps its meaning once the abort is gone.
23+
"""
24+
25+
import mmap
26+
27+
m = mmap.mmap(-1, 4)
28+
m[:] = b"abca"
29+
30+
# The empty needle matches at the near end of the span.
31+
assert m.find(b"") == 0, m.find(b"")
32+
assert m.rfind(b"") == 4, m.rfind(b"")
33+
assert m.find(b"", 2) == 2, m.find(b"", 2)
34+
assert m.rfind(b"", 0, 2) == 2, m.rfind(b"", 0, 2)
35+
assert m.find(b"", 4) == 4, m.find(b"", 4)
36+
assert m.rfind(b"", 4) == 4, m.rfind(b"", 4)
37+
assert m.find(b"", -2) == 2, m.find(b"", -2)
38+
assert m.rfind(b"", -2) == 4, m.rfind(b"", -2)
39+
40+
# An inverted span holds nothing at all, not even the empty needle.
41+
assert m.find(b"", 3, 1) == -1, m.find(b"", 3, 1)
42+
assert m.rfind(b"", 3, 1) == -1, m.rfind(b"", 3, 1)
43+
44+
# The needle is longer than the whole map.
45+
assert m.find(b"abcab") == -1, m.find(b"abcab")
46+
assert m.rfind(b"abcab") == -1, m.rfind(b"abcab")
47+
48+
# The needle fits the map but not the requested span.
49+
assert m.find(b"abc", 2) == -1, m.find(b"abc", 2)
50+
assert m.rfind(b"abc", 2) == -1, m.rfind(b"abc", 2)
51+
assert m.find(b"abc", 0, 2) == -1, m.find(b"abc", 0, 2)
52+
assert m.rfind(b"abc", 0, 2) == -1, m.rfind(b"abc", 0, 2)
53+
54+
# A span exactly the needle's length still has one candidate.
55+
assert m.find(b"bc", 1, 3) == 1, m.find(b"bc", 1, 3)
56+
assert m.rfind(b"bc", 1, 3) == 1, m.rfind(b"bc", 1, 3)
57+
58+
# The ordinary answers, so a bound that returns -1 too eagerly is caught too.
59+
assert m.find(b"a") == 0, m.find(b"a")
60+
assert m.rfind(b"a") == 3, m.rfind(b"a")
61+
assert m.find(b"ca") == 2, m.find(b"ca")
62+
assert m.find(b"a", -1) == 3, m.find(b"a", -1)
63+
assert m.find(b"abca", -10) == 0, m.find(b"abca", -10)
64+
65+
m.close()
66+
67+
# A one-byte map is the smallest span an oversized needle can overrun.
68+
one = mmap.mmap(-1, 1)
69+
one[:] = b"a"
70+
assert one.find(b"ab") == -1, one.find(b"ab")
71+
assert one.rfind(b"ab") == -1, one.rfind(b"ab")
72+
assert one.find(b"") == 0, one.find(b"")
73+
assert one.rfind(b"") == 1, one.rfind(b"")
74+
assert one.find(b"a") == 0, one.find(b"a")
75+
one.close()
76+
77+
print("OK")

‎pyre/pyre-interpreter/src/module/mmap/interp_mmap.rs‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -673,11 +673,17 @@ fn init_mmap_type(ns: pyre_object::PyObjectRef) {
673673
} else {
674674
len
675675
};
676-
if start >= end || needle.is_empty() {
676+
if start > end {
677+
return Ok(pyre_object::w_int_new(-1));
678+
}
679+
if needle.is_empty() {
680+
return Ok(pyre_object::w_int_new(start as i64));
681+
}
682+
if needle.len() > end - start {
677683
return Ok(pyre_object::w_int_new(-1));
678684
}
679685
let hay = unsafe { std::slice::from_raw_parts(p.add(start), end - start) };
680-
let pos = (0..=hay.len().saturating_sub(needle.len()))
686+
let pos = (0..=hay.len() - needle.len())
681687
.find(|&i| &hay[i..i + needle.len()] == needle)
682688
.map(|i| (start + i) as i64)
683689
.unwrap_or(-1);
@@ -727,11 +733,17 @@ fn init_mmap_type(ns: pyre_object::PyObjectRef) {
727733
} else {
728734
len
729735
};
730-
if start >= end || needle.is_empty() {
736+
if start > end {
737+
return Ok(pyre_object::w_int_new(-1));
738+
}
739+
if needle.is_empty() {
740+
return Ok(pyre_object::w_int_new(end as i64));
741+
}
742+
if needle.len() > end - start {
731743
return Ok(pyre_object::w_int_new(-1));
732744
}
733745
let hay = unsafe { std::slice::from_raw_parts(p.add(start), end - start) };
734-
let pos = (0..=hay.len().saturating_sub(needle.len()))
746+
let pos = (0..=hay.len() - needle.len())
735747
.rev()
736748
.find(|&i| &hay[i..i + needle.len()] == needle)
737749
.map(|i| (start + i) as i64)

0 commit comments

Comments
 (0)