Skip to content

Commit c85b83f

Browse files
youknowoneyouknowone
authored
Fix crashes found hunting the last open fuzzing record (RustPython#8524)
* specialize: check the member descriptor's type before caching its slot offset The LOAD_ATTR/STORE_ATTR specializations cached the slot offset of any member descriptor found on the owner's type and then guarded the specialized instruction on the type version alone, while descr_get()/descr_set() check on every access that the instance belongs to the type the descriptor was defined for. A descriptor taken from a wider class and bound to a narrower one read past the instance's slot array once the cache warmed up: class Big: __slots__ = ("a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7") class Narrow: __slots__ = ("z",) Narrow.x = Big.__dict__["a7"] o = Narrow() for _ in range(1000): try: o.x except TypeError: pass # index out of bounds: the len is 1 but the index is 7 (object/core.rs) A class with no slots at all reached the ext_ref().unwrap() on the same line. Assisted-by: Claude * socket: reserve recv()'s buffer fallibly recv() and recvfrom() handed the caller's bufsize straight to Vec::with_capacity, so an unreachable size aborted the process through handle_alloc_error before any syscall was made: socket.socket().recv(2**62) # memory allocation of 4611686018427387904 bytes failed -> SIGABRT try_reserve_exact reports MemoryError instead, which is what CPython raises. Assisted-by: Claude * types: count the __call__ and __get__ slot dispatches as recursion Both wrappers re-enter Python without pushing a frame, so nothing counted the nesting when the special method named the object it was looked up on: class C: pass c = C(); C.__call__ = c c() # native stack overflow, SIGSEGV class D: pass d = D(); D.__get__ = d; D.x = d d.x # the same, through descr_get with_recursion around the two dispatches raises RecursionError instead, the way Py_EnterRecursiveCall bounds a tp_call dispatch. It costs about 5% on a __call__ dispatch and 3% on a __get__ dispatch through these wrappers. Assisted-by: Claude * typevar: show a ParamSpecArgs origin by its repr ParamSpecArgs and ParamSpecKwargs fell back to a Rust `{:?}` of __origin__ when it had no __name__. That walks the object graph natively through Debug for PyInner, where no recursion guard sits, so a single repr() of a deeply nested chain overflowed the native stack: a = object() for _ in range(30000): a = typing.ParamSpecArgs(a) repr(a) # SIGSEGV The origin is formatted with its repr now, which is guarded, and a ParamSpec origin is recognized by its type rather than by carrying a __name__. Assisted-by: Claude * Do not hold a lock across a call back into Python Three places kept a lock while running code that can reach the same object, so a callback that touched it wedged the process: _asyncio.future_add_to_awaited_by(fut, waiter) # waiter.__hash__ adds again select.select(elements, [], [], 0) # fileno() clears `elements` select.poll().poll(1000) # SIGALRM handler registers The future's awaited-by field is read and written under its lock but the set is built outside it, the list extraction re-reads the list on each step the way map_iterable_object() does, and poll() waits on a copy of its descriptors. All three ran forever before and now finish the way they do on CPython. Assisted-by: Claude * Validate memoryview.cast() arguments and export negative strides correctly cast() accepted any struct format and any shape element. A zero-size format ('0s') and a 0 in the shape both reached a division by zero; cast() now takes only a native single character format, optionally '@'-prefixed, and shape elements that are ints greater than zero. A view with a negative stride starts at its last item, so the bytes it exported began there and its own offsets walked off the front of them. Such a view now exports the whole underlying buffer with `start` folded into the descriptor's offsets, and zip_eq() hands over a whole run only when both sides are contiguous in the last dimension. Assisted-by: Claude Assisted-by: Codex:GPT-5 * Charge native recursion to the stack, not to the frame limit with_recursion() checked the limit sys.setrecursionlimit() sets and incremented the same counter that pushing a frame does, so a guard on a native dispatch spent what Python code had left to call with, and did so where sys._getframe() cannot see it: test.support.get_recursion_available() reported frames that were no longer there. Py_EnterRecursiveCall bounds the native stack instead, which is a separate budget, and the C stack check with_recursion already performs is that bound. The snippets pinning the guarded paths nest deep enough to reach the stack rather than the frame limit. Assisted-by: Claude * Report a size that cannot be allocated instead of aborting on it A size taken from Python went straight into an infallible allocation in several places, so the process aborted through handle_alloc_error before any exception could be raised: - str/bytes/bytearray center(), ljust(), rjust() and zfill() reserved the padded result for the caller's width - expandtabs() built its runs of spaces from a tabsize of any width; the argument is a C int, and a wider one does not fit - Buffered{Reader,Writer,Random} allocated buffer_size, and read(), read1() and FileIO.read() their read size - bytes(n) and bytearray(n) allocated n - pbkdf2_hmac() allocated the derived key length, which is a C int Each of these now reports MemoryError, or OverflowError where the argument does not fit the type it is declared with. new_zeroed_bytes() leaves the zeroing to the allocator, so a large request costs the pages that are written to rather than all of them. Assisted-by: Claude * marshal: answer allow_code where a code object is written or read allow_code was answered by walking the whole result a second time, with no depth counter and no record of what it had already seen, so a value that referred back to itself or nested deeply enough ran off the native stack. w_object() and r_object() answer it where the code object is, inside the walk that already bounds its depth and resolves references. A container length is read the way r_long() reads one: it is signed, so a length with the top bit set is out of range rather than four billion items to reserve room for. load() no longer holds a borrow of the buffer read() returned across the seek() it makes afterwards. Assisted-by: Claude * Do not lock an object while running code that can reach it Several places held a lock or a borrow of an object across a call back into Python, so a callback that touched the same object waited on a lock its own caller was holding: - memoryview slice assignment read a source overlapping the destination, and __setitem__ converted the value while holding the write borrow - BytesIO.readinto() read into a buffer viewing the same BytesIO - array.__setitem__ converted the value under the array's write lock, and mmap.write() read a source viewing the same map - bytearray.join() and bytearray.__mod__ drove Python with the bytearray borrowed - array and bytearray answered "is this resizable" after taking the write lock, though an export is exactly a borrow someone else holds A TextIOWrapper cookie now has to name a position inside what was decoded in characters as well as in bytes; only the byte offset was checked, and the character count is what read() and tell() index with. Assisted-by: Claude Assisted-by: Codex:GPT-5 * Stop asserting a pbkdf2 message that depends on the width of a C long The snippet asserted "key length is too great.", which pbkdf2_hmac() only reaches once the length has been converted; where a C long is narrower than the length asked for, the conversion fails first and says so instead. Both are OverflowError, which is what the case is about. test_support.test_get_recursion_depth passes now that a native recursion guard no longer spends frames get_recursion_depth() cannot see. Assisted-by: Claude * Publish and read the same pointer for a thread's top frame set_current_frame() casts the `Py<FrameObject>` it publishes straight to `*mut FrameObject`, so ThreadSlot::top_frame holds the object's base. sys._current_frames() read it back through Py::from_payload_ptr(), which subtracts the payload offset from what it is given. The reference it took therefore incremented, and later decremented, a word 48 bytes ahead of the frame -- inside the object allocated before it, whose OnceLock state word sits exactly there for two frames adjacent in the size class. The neighbour then read an initialized-looking cold pointer that had never been written and locked whatever the uninitialized word addressed, so the thread that owned it crashed rather than the one that read. The slot now holds `*mut Py<FrameObject>`, which is what both sides mean. A thread parked in a call has no FrameObject for its topmost frame, so top_frame is null there and the reader takes the materialize path instead: test_sys.test_current_frames never reaches the branch. The snippet takes _current_frames() against threads that are running. Assisted-by: Claude Assisted-by: Codex:GPT-5 * Decide stop-the-world parking under the thread registry lock do_suspend() published SUSPENDED first and only then re-read `requested`, restoring itself to ATTACHED if the stop had ended in the meantime. That made a thread the second writer able to leave SUSPENDED, so a stop whose completion check had already observed the thread parked could be undone behind the requester's back: worker CAS ATTACHED -> SUSPENDED requester all_non_requester_suspended() -> true, world_stopped = true requester start_the_world(): requested = false, then walks the registry worker reads requested == false, stores ATTACHED With the store landing inside that walk the debug assertion in start_the_world fires; with the walk already past the slot, a following stop force-parks the thread DETACHED -> SUSPENDED, counts it as stopped, and the store then puts it back to ATTACHED with the world declared stopped and the thread running bytecode. `requested` is set in init_thread_countdown() and cleared in start_the_world() with the registry held, and start_the_world() keeps holding it while releasing every SUSPENDED thread. Taking the registry around the check and the transition therefore makes the two orders the only ones possible: park before that release pass and be woken by it, or find the request already withdrawn and stay ATTACHED. The requester is left as the only writer that takes a thread out of SUSPENDED, and the self-restore is gone. suspend_if_needed() takes the VirtualMachine to reach the registry. Assisted-by: Claude Assisted-by: Codex:GPT-5 * Keep an atexit callback alive while it is being compared atexit.unregister() releases the callback list around each __eq__ call and identified the entry it had compared by the address of its Box. __eq__ can call atexit._clear(), which drops that Box, and atexit.register(), whose new Box lands on the freed allocation; the identity search then matched the freshly registered callback and removed it. atexit.register(a); atexit.register(b); atexit.register(c) # __eq__ runs _clear() then register(d), returns True atexit.unregister(probe) left no callbacks registered where CPython leaves d. Entries are Arc-shared now, so unregister() holds the one it is comparing and matches it with Arc::ptr_eq: an address cannot be reused while the comparison that named it is still running. Assisted-by: Claude * Hold atexit entries in PyRc rather than Arc PyObjectRef is Send and Sync only under the threading feature, so an Arc over a callback entry trips clippy::arc_with_non_send_sync in builds without it, such as the wasm package. PyRc is Arc there and Rc otherwise. Assisted-by: Claude * Do not hold a buffer's storage while waiting for a peer FileIO.readinto() and socket.recv_into()/recvfrom_into() took the target buffer's write borrow and kept it for the whole call, including the wait for data a pipe, socket or terminal may never deliver. What CPython holds across that wait is the export, which only forbids resizing; the borrow is a lock every other thread touching the same object waits on, so threading.Thread(target=lambda: sock.recv_into(buf)).start() len(buf) did not answer until the peer sent. A thread parked on that lock is ATTACHED and never reaches a safepoint, so gc.collect() in a third thread waited for the peer as well: one incidental read of the buffer stopped the world from being stopped at all. The wait now runs against storage of its own and the bytes are copied over once they arrive, with the export held throughout so the target still cannot be resized meanwhile. A seekable file answers from itself rather than from a peer, so FileIO.readinto() writes into the target directly there and the buffered read path is unchanged. Assisted-by: Claude * Do not hold a buffer's storage while waiting to hand it over socket.send()/sendall()/sendto()/sendmsg() and FileIO.write() kept the source buffer's read borrow for the whole call, including the wait for a peer that may never make room. That borrow is a lock every other thread writing to the same object waits on, so threading.Thread(target=lambda: sock.sendall(buf)).start() buf[0] = 1 did not return until the peer read; and a thread parked there is ATTACHED and never reaches a safepoint, so gc.collect() in a third thread waited for the peer too -- the same wedge readinto() had on the receiving side. ArgBytesLike::borrow_buf_unlocked() answers with bytes that survive the borrow being dropped. An immutable object hands out a plain reference and locks nothing, so those are sent where they lie and bytes and memoryviews over them cost nothing; only bytes reached through a lock are copied out first. The export is held throughout either way, so the source still cannot be resized while it is being sent. The regression snippet covers both directions now and is renamed for it. Assisted-by: Claude Assisted-by: Codex:GPT-5 * Check select()'s descriptor limit while the sequence is walked seq2set() collected the whole sequence and compared the result's length against FD_SETSIZE afterwards. Selectable::try_from_object() calls fileno(), which runs Python and can append to the list being walked, and the walk re-reads the list on every step, so the collection had no end to reach and the comparison was never made. seq2set in Modules/selectmodule.c checks the count per element instead. stdlib_select.py gains a fileno() that appends to its own list, and releases its sockets with close() rather than by dropping the name. Assisted-by: Claude * Bound native recursion where the C stack cannot be measured check_c_stack_overflow() answers no unconditionally under miri and on musl, where the stack pointer is not read. Since 9c9905a that check is all with_recursion() does, so every guard placed on native recursion -- __call__ and __get__ dispatch among them -- was a no-op on those targets and the nesting ran until the stack ran out. with_recursion() now counts its own depth on those targets and refuses past NATIVE_RECURSION_LIMIT_UNMEASURED. The count is separate from the frame limit sys.setrecursionlimit() sets, and compiles away where the stack pointer can be read. Assisted-by: Claude * Report the failure to allocate pbkdf2's key and Take.readinto's scratch Both buffers are sized from an argument -- pbkdf2_hmac's dklen accepts up to i32::MAX, and readinto's from the length of the destination -- and were built with vec![0u8; n], which aborts the process on allocation failure. new_zeroed_bytes() raises MemoryError instead. Assisted-by: Claude * Read the frozen-code tuple length as a signed length The '(' branches in read_marshal_str_vec() and read_marshal_const_tuple() took the length with read_u32() as usize, so a value with the top bit set read as four billion items rather than as out of range. read_len() is what every other length in this file goes through, and it reinterprets as i32. Both readers serve deserialize_code(), which reads only the frozen modules baked in at build time, so this changes no reachable behavior; marshal.loads() already went through read_len(). Assisted-by: Claude * Make the snippets from the fuzzer sweep assert what they check builtin_str.py expanded a tab to 2**31-1 columns, allocating 2 GiB to observe that the width is accepted; a string with no tab observes the same acceptance without laying anything out. stdlib_array.py caught the refusal of frombytes() on its own exported buffer and passed silently when nothing was raised. stdlib_hashlib.py used a bare `assert False` as its failure branch. Both now say so through the same shapes the other snippets use. stdlib_socket.py also accepts OverflowError from recv() with a size that does not fit the platform's C int. stdlib_threading_current_frames.py indexed the frame chain for "f123" without first asserting it is there. Assisted-by: Claude * Ask for a marshal container's room instead of assuming it A flagged tuple or list is published in the reference table before its children are read, and the placeholder was built with vec![none; len]. The length is the input's to choose and read_len() lets it reach i32::MAX, so marshal.loads(b"\xa8\xff\xff\xff\x7f") -- five bytes -- asks for 17 GB of element slots and aborts the process where the allocator cannot serve it. r_object() allocates the container up front too, but PyTuple_New() reports what it cannot get. The elements are now reserved with try_reserve_exact() and a refusal is raised as MemoryError through the decoder's pending-error channel. PyTuple::new_marshal_placeholder() held nothing but that allocation and is gone; the caller builds the elements and uses new_ref(). Assisted-by: Claude * Compare the blocking-buffer snippet against something measure() asserted len(buf) == len(buf), which holds whatever the length is; it now takes the length the caller expects. The pipe case compared the drained total against the whole source, while an unbuffered write() reports only what it transferred and a signal can cut that short; it now compares against what write() returned. Assisted-by: Claude * Refuse array.frombytes() a source whose items are not bytes frombytes() reads its argument as bytes, but accepted any contiguous buffer, so array("i").frombytes(memoryview(array("d", [1.0]))) appended a double's bytes read as ints instead of raising. array_array_frombytes_impl requires an itemsize of 1; ArgBytesLike now reports the itemsize so the same check can be made here. The BufferError that a resize meets while the array is exported also names the array rather than repeating bytearray's wording. stdlib_array.py's frombytes-of-itself case used typecode "i", where the new check answers before the resize guard is reached; it uses "b" so the guard is what refuses, and asserts the wider case separately. Assisted-by: Claude * Tell apart the ways marshal data can be bad A type byte no reader knows, a back reference that names nothing, and TYPE_NULL all came out as a bare "bad marshal data" ValueError. r_object() answers the first two with "bad marshal data (unknown type code)" and "bad marshal data (invalid reference)", and read_object() answers the third with a TypeError, "NULL object in marshal data for object", since TYPE_NULL stands for no object rather than for a value. MarshalError gains the three cases and deserialize_value() maps them. The container-specific wording r_object() uses for a NULL read inside a tuple or list is not reproduced; the exception type is. Assisted-by: Claude * Hash the collector's tables by address rather than by SipHash A collection keys three sets and two maps by object address, and it visits every tracked object and every edge between them, so the hashing is a per-edge cost. Those tables used the default RandomState, whose SipHash buys resistance against a caller choosing colliding keys -- and nothing chooses these keys: they are addresses this process handed out into tables that live and die inside one collection. A profile of gc.collect() over a 423k-object heap spent 45% of its samples in SipHash. They now hash with a splitmix64 finalizer. The shifts matter: a table picks its bucket from the low bits and an address arrives with those bits zeroed by alignment, so a plain multiply leaves every object in a handful of buckets and is slower than SipHash was. The reachability walk also copied each object's referent vector out of the map it was cached in, a second pass over every edge; it reads them in place, and reference subtraction hands its vector to the map instead of cloning it. Measured over 423k live objects: 0.93s to 0.15s. Over 843k dead ones: 3.00s to 0.79s. extra_tests/snippets/stdlib_threading_gc_import.py, whose collector thread calls gc.collect() in a loop, ran anywhere from 2.7s to 28s and now runs in 3.1-3.5s: a collection that takes longer leaves more garbage for the next one to walk, so the cost fed back on itself. Assisted-by: Claude Assisted-by: Codex:GPT-5 * Keep the collection's candidates and their counts in one table A collection built a set of candidates and, beside it, a map from the same addresses to their reference counts. Both were probed for every edge in the heap -- membership from the set, the count from the map -- so each edge paid to hash the same address twice, and each candidate paid to be inserted twice. The map alone answers both questions. The candidates also keep a walkable order now, which the reference subtraction pass needs since it writes the counts while reading the candidates, and which the unreachable set is built from instead of a set difference. Over the 423k-object heap measured in the previous commit: 0.15s to 0.13s live, and 0.79s to 0.49s dead. Assisted-by: Claude Assisted-by: Codex:GPT-5 * gc: collect referents into one buffer instead of a vector per object Step 3 allocated a `Vec` for every tracked object to hold its referents and kept them all in a map until step 4 read them back. The referents now go into a single growing buffer, with the map holding each object's range into it. Adds `PyObject::gc_extend_referent_ptrs`, which appends to a caller's buffer; `gc_get_referent_ptrs` calls it with a fresh one. Assisted-by: Claude * memoryview and struct: match the checks and errors of the reference memoryview: - `cast()` accepted a source and destination that are both item types, which reinterprets the items rather than re-dividing the bytes; one side now has to be a byte format. - A cast to `shape=()` returned without checking that the buffer holds exactly the one item that shape describes. - `hash()` hashes the bytes, so it now raises ValueError for a view whose items are not bytes, rather than returning a hash that disagrees with the value the view compares equal to. - `tobytes()` takes the `order` argument, with 'F' walking a multidimensional view down its columns; `BufferDescriptor` gained `for_each_segment_fortran` for that walk. struct: - A value the format has no room for reported "argument out of range" instead of naming the format and its range. The format character is now passed to the packing functions to report it. - `Struct.__new__` no longer reads the format; `__init__` does, so `__init__` can be called again and a subclass can pass the format up. Methods raise RuntimeError until it has run, and `Struct` is a base type. Removes the expectedFailure from test_Struct_reinitialization and test_struct_subclass_instantiation. Assisted-by: Claude Assisted-by: Codex:GPT-5 * io: decide the readinto path by file type, not by seekability FileIO.readinto wrote straight into the caller's buffer, holding its write borrow, when the fd was seekable; otherwise it read aside into scratch and copied. Seekability stood in for "this read answers without waiting on a peer", which a pipe on Windows breaks: lseek on one succeeds, so the pipe took the borrow-holding path and every other thread touching that bytearray waited for the peer. host_io::reads_without_waiting answers it directly -- seekability elsewhere, GetFileType() == FILE_TYPE_DISK on Windows. The regression snippet times each operation separately, so a failure names the one that waited; it asserts the transfer is still in flight before checking the export; and the socket case fills the connection until it refuses rather than assuming a size that outruns it, which SO_SNDBUF on an already-connected pair does not settle. Assisted-by: Claude
1 parent f24b257 commit c85b83f

56 files changed

Lines changed: 2144 additions & 499 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Lib/test/test_struct.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,6 @@ def test_trailing_counter(self):
601601
'spam and eggs')
602602
self.assertRaises(struct.error, struct.unpack_from, '14s42', store, 0)
603603

604-
@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: '>h' != '>hh'
605604
def test_Struct_reinitialization(self):
606605
# Issue 9422: there was a memory leak when reinitializing a
607606
# Struct instance. This test can be used to detect the leak
@@ -826,7 +825,6 @@ def test_error_propagation(fmt_str):
826825
test_error_propagation('N')
827826
test_error_propagation('n')
828827

829-
@unittest.expectedFailure # TODO: RUSTPYTHON
830828
def test_struct_subclass_instantiation(self):
831829
# Regression test for https://github.com/python/cpython/issues/112358
832830
class MyStruct(struct.Struct):

‎Lib/test/test_support.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,6 @@ def test_has_strftime_extensions(self):
631631
else:
632632
self.assertTrue(support.has_strftime_extensions)
633633

634-
@unittest.expectedFailure # TODO: RUSTPYTHON; - _testinternalcapi module not available
635634
def test_get_recursion_depth(self):
636635
# test support.get_recursion_depth()
637636
code = textwrap.dedent("""

‎crates/common/src/borrow.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ impl_from!('a, T, BorrowedValue<'a, T>,
3434
);
3535

3636
impl<'a, T: ?Sized> BorrowedValue<'a, T> {
37+
/// Whether reaching the value holds a lock that other threads wait on.
38+
///
39+
/// An immutable object hands out a plain reference and answers `false`;
40+
/// one whose storage can change hands out a guard. A caller about to wait
41+
/// for something unrelated -- a peer, a file, a signal -- can use this to
42+
/// decide whether it may keep the borrow for the duration.
43+
#[must_use]
44+
pub const fn is_locked(&self) -> bool {
45+
!matches!(self, Self::Ref(_))
46+
}
47+
3748
pub fn map<U: ?Sized, F>(s: Self, f: F) -> BorrowedValue<'a, U>
3849
where
3950
F: FnOnce(&T) -> &U,

‎crates/common/src/str.rs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -592,20 +592,21 @@ pub fn codepoint_range_end(s: &Wtf8, n_chars: usize) -> Option<usize> {
592592
}
593593

594594
#[must_use]
595-
pub fn zfill(bytes: &[u8], width: usize) -> Vec<u8> {
595+
/// Returns `None` for a width whose result cannot be allocated.
596+
pub fn zfill(bytes: &[u8], width: usize) -> Option<Vec<u8>> {
596597
if width <= bytes.len() {
597-
bytes.to_vec()
598-
} else {
599-
let (sign, s) = match bytes.first() {
600-
Some(_sign @ (b'+' | b'-')) => (unsafe { bytes.get_unchecked(..1) }, &bytes[1..]),
601-
_ => (&b""[..], bytes),
602-
};
603-
let mut filled = Vec::new();
604-
filled.extend_from_slice(sign);
605-
filled.extend(core::iter::repeat_n(b'0', width - bytes.len()));
606-
filled.extend_from_slice(s);
607-
filled
598+
return Some(bytes.to_vec());
608599
}
600+
let (sign, s) = match bytes.first() {
601+
Some(_sign @ (b'+' | b'-')) => (unsafe { bytes.get_unchecked(..1) }, &bytes[1..]),
602+
_ => (&b""[..], bytes),
603+
};
604+
let mut filled = Vec::new();
605+
filled.try_reserve_exact(width).ok()?;
606+
filled.extend_from_slice(sign);
607+
filled.extend(core::iter::repeat_n(b'0', width - bytes.len()));
608+
filled.extend_from_slice(s);
609+
Some(filled)
609610
}
610611

611612
/// Convert a string to ascii compatible, escaping unicode-s into escape

‎crates/compiler-core/src/marshal.rs‎

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ pub enum MarshalError {
1919
InvalidLocation,
2020
/// Bad type marker
2121
BadType,
22+
/// A type marker no reader knows
23+
UnknownType,
24+
/// A back reference that names nothing
25+
InvalidRef,
26+
/// A marker that stands for no object at all
27+
NullObject,
28+
/// A container length that is negative or does not fit, named by what it counts
29+
BadSize(&'static str),
2230
}
2331

2432
impl core::fmt::Display for MarshalError {
@@ -29,6 +37,10 @@ impl core::fmt::Display for MarshalError {
2937
Self::InvalidUtf8 => f.write_str("invalid utf8"),
3038
Self::InvalidLocation => f.write_str("invalid source location"),
3139
Self::BadType => f.write_str("bad type marker"),
40+
Self::UnknownType => f.write_str("unknown type code"),
41+
Self::InvalidRef => f.write_str("invalid reference"),
42+
Self::NullObject => f.write_str("NULL object in marshal data for object"),
43+
Self::BadSize(what) => write!(f, "{what} size out of range"),
3244
}
3345
}
3446
}
@@ -111,7 +123,7 @@ impl TryFrom<u8> for Type {
111123
b'A' => Self::AsciiInterned,
112124
b'z' => Self::ShortAscii,
113125
b'Z' => Self::ShortAsciiInterned,
114-
_ => return Err(MarshalError::BadType),
126+
_ => return Err(MarshalError::UnknownType),
115127
})
116128
}
117129
}
@@ -146,6 +158,13 @@ pub trait Read {
146158
fn read_u64(&mut self) -> Result<u64> {
147159
Ok(u64::from_le_bytes(*self.read_array()?))
148160
}
161+
162+
/// A length, read the way `r_long` reads one: it is signed, so a value
163+
/// with the top bit set is out of range rather than four billion items.
164+
fn read_len(&mut self, what: &'static str) -> Result<usize> {
165+
let len = self.read_u32()? as i32;
166+
usize::try_from(len).map_err(|_| MarshalError::BadSize(what))
167+
}
149168
}
150169

151170
pub(crate) trait ReadBorrowed<'a>: Read {
@@ -305,7 +324,7 @@ fn reserve_ref_slot<T>(has_flag: bool, refs: &mut Vec<Option<T>>) -> Option<usiz
305324
fn resolve_ref<T: Clone>(idx: usize, refs: &[Option<T>]) -> Result<T> {
306325
refs.get(idx)
307326
.and_then(|v| v.clone())
308-
.ok_or(MarshalError::InvalidBytecode)
327+
.ok_or(MarshalError::InvalidRef)
309328
}
310329

311330
/// Read a marshal bytes object (TYPE_STRING = b's'), resolving TYPE_REF
@@ -408,7 +427,7 @@ fn read_marshal_str_vec<R: Read, Bag: ConstantBag>(
408427
}
409428

410429
let n = match type_byte {
411-
b'(' => rdr.read_u32()? as usize,
430+
b'(' => rdr.read_len("tuple")?,
412431
b')' => rdr.read_u8()? as usize,
413432
_ => return Err(MarshalError::BadType),
414433
};
@@ -471,7 +490,7 @@ fn read_marshal_const_tuple<R: Read, Bag: ConstantBag>(
471490
}
472491

473492
let n = match type_byte {
474-
b'(' => rdr.read_u32()? as usize,
493+
b'(' => rdr.read_len("tuple")?,
475494
b')' => rdr.read_u8()? as usize,
476495
_ => return Err(MarshalError::BadType),
477496
};
@@ -553,7 +572,7 @@ pub trait MarshalBag: Copy {
553572
fn make_code(
554573
&self,
555574
code: CodeObject<<Self::ConstantBag as ConstantBag>::Constant>,
556-
) -> Self::Value;
575+
) -> Result<Self::Value>;
557576

558577
/// Construct a runtime code object while retaining the exact values read
559578
/// from ``co_consts``. Compiler bags ignore this second channel; runtime
@@ -563,7 +582,7 @@ pub trait MarshalBag: Copy {
563582
&self,
564583
code: CodeObject<<Self::ConstantBag as ConstantBag>::Constant>,
565584
_constants: Vec<Self::Value>,
566-
) -> Self::Value {
585+
) -> Result<Self::Value> {
567586
self.make_code(code)
568587
}
569588

@@ -583,8 +602,12 @@ pub trait MarshalBag: Copy {
583602
/// Install partially-built containers in the marshal reference table
584603
/// before reading their children, as CPython's `r_object()` does.
585604
/// Runtime bags can opt in; constant bags retain collect-then-construct.
586-
fn make_tuple_placeholder(&self, _len: usize) -> Option<Self::Value> {
587-
None
605+
///
606+
/// `len` comes straight from the input and is only bounded by what a
607+
/// length can hold, so a bag that opts in reports the room it cannot get
608+
/// rather than taking it for granted.
609+
fn make_tuple_placeholder(&self, _len: usize) -> Result<Option<Self::Value>> {
610+
Ok(None)
588611
}
589612

590613
fn set_tuple_item(
@@ -596,8 +619,8 @@ pub trait MarshalBag: Copy {
596619
Err(MarshalError::BadType)
597620
}
598621

599-
fn make_list_placeholder(&self, _len: usize) -> Option<Self::Value> {
600-
None
622+
fn make_list_placeholder(&self, _len: usize) -> Result<Option<Self::Value>> {
623+
Ok(None)
601624
}
602625

603626
fn set_list_item(&self, _list: &Self::Value, _index: usize, _value: Self::Value) -> Result<()> {
@@ -725,8 +748,8 @@ impl<Bag: ConstantBag> MarshalBag for Bag {
725748
fn make_code(
726749
&self,
727750
code: CodeObject<<Self::ConstantBag as ConstantBag>::Constant>,
728-
) -> Self::Value {
729-
self.make_code(code)
751+
) -> Result<Self::Value> {
752+
Ok(self.make_code(code))
730753
}
731754

732755
fn make_stop_iter(&self) -> Result<Self::Value> {
@@ -830,10 +853,7 @@ fn deserialize_value_after_header<R: Read, Bag: MarshalBag>(
830853
// TYPE_REF: return previously stored object
831854
if type_code == Type::Ref as u8 {
832855
let idx = rdr.read_u32()? as usize;
833-
return refs
834-
.get(idx)
835-
.and_then(|v| v.clone())
836-
.ok_or(MarshalError::InvalidBytecode);
856+
return resolve_ref(idx, refs);
837857
}
838858

839859
// Reserve ref slot before reading (matches write order)
@@ -986,7 +1006,7 @@ fn deserialize_code_value_inner<R: Read, Bag: MarshalBag>(
9861006
linetable,
9871007
exceptiontable,
9881008
};
989-
Ok(bag.make_code_with_constants(code, constant_values))
1009+
bag.make_code_with_constants(code, constant_values)
9901010
}
9911011

9921012
fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
@@ -1033,13 +1053,13 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
10331053
bag.make_complex(value)
10341054
}
10351055
Type::Ascii | Type::Unicode => {
1036-
let len = rdr.read_u32()?;
1037-
let value = rdr.read_wtf8(len)?;
1056+
let len = rdr.read_len("string")?;
1057+
let value = rdr.read_wtf8(len as u32)?;
10381058
bag.make_str(value)
10391059
}
10401060
Type::AsciiInterned | Type::Interned => {
1041-
let len = rdr.read_u32()?;
1042-
let value = rdr.read_wtf8(len)?;
1061+
let len = rdr.read_len("string")?;
1062+
let value = rdr.read_wtf8(len as u32)?;
10431063
bag.make_interned_str(value)
10441064
}
10451065
Type::ShortAscii => {
@@ -1056,7 +1076,7 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
10561076
let len = rdr.read_u8()? as usize;
10571077
let d = depth - 1;
10581078
if let Some(index) = slot
1059-
&& let Some(tuple) = bag.make_tuple_placeholder(len)
1079+
&& let Some(tuple) = bag.make_tuple_placeholder(len)?
10601080
{
10611081
refs[index] = Some(tuple.clone());
10621082
for item_index in 0..len {
@@ -1070,17 +1090,17 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
10701090
}
10711091
}
10721092
Type::Null => {
1073-
return Err(MarshalError::BadType);
1093+
return Err(MarshalError::NullObject);
10741094
}
10751095
Type::Ref => {
10761096
// Handled in deserialize_value_depth before calling this function
10771097
return Err(MarshalError::BadType);
10781098
}
10791099
Type::Tuple => {
1080-
let len = rdr.read_u32()? as usize;
1100+
let len = rdr.read_len("tuple")?;
10811101
let d = depth - 1;
10821102
if let Some(index) = slot
1083-
&& let Some(tuple) = bag.make_tuple_placeholder(len)
1103+
&& let Some(tuple) = bag.make_tuple_placeholder(len)?
10841104
{
10851105
refs[index] = Some(tuple.clone());
10861106
for item_index in 0..len {
@@ -1094,10 +1114,10 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
10941114
}
10951115
}
10961116
Type::List => {
1097-
let len = rdr.read_u32()? as usize;
1117+
let len = rdr.read_len("list")?;
10981118
let d = depth - 1;
10991119
if let Some(index) = slot
1100-
&& let Some(list) = bag.make_list_placeholder(len)
1120+
&& let Some(list) = bag.make_list_placeholder(len)?
11011121
{
11021122
refs[index] = Some(list.clone());
11031123
for item_index in 0..len {
@@ -1111,7 +1131,7 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
11111131
}
11121132
}
11131133
Type::Set => {
1114-
let len = rdr.read_u32()? as usize;
1134+
let len = rdr.read_len("set")?;
11151135
let d = depth - 1;
11161136
if let Some(index) = slot
11171137
&& let Some(set) = bag.make_set_placeholder()
@@ -1128,7 +1148,7 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
11281148
}
11291149
}
11301150
Type::FrozenSet => {
1131-
let len = rdr.read_u32()?;
1151+
let len = rdr.read_len("set")?;
11321152
let d = depth - 1;
11331153
let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs));
11341154
itertools::process_results(it, |it| bag.make_frozenset(it))??
@@ -1165,8 +1185,8 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>(
11651185
}
11661186
Type::Bytes => {
11671187
// After marshaling, byte arrays are converted into bytes.
1168-
let len = rdr.read_u32()?;
1169-
let value = rdr.read_slice(len)?;
1188+
let len = rdr.read_len("bytes object")?;
1189+
let value = rdr.read_slice(len as u32)?;
11701190
bag.make_bytes(value)
11711191
}
11721192
Type::Code => return Err(MarshalError::BadType),

‎crates/host_env/src/io.rs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,29 @@ pub fn is_seekable(fd: crt_fd::Borrowed<'_>) -> bool {
199199
os::seek_fd(fd, 0, libc::SEEK_CUR).is_ok()
200200
}
201201

202+
/// Whether a read from `fd` answers from data the file already holds, rather
203+
/// than waiting for whoever writes the other end.
204+
///
205+
/// Seeking answers this everywhere but Windows, where a pipe seeks too --
206+
/// `lseek` on one succeeds and reports a position, so a reader that took
207+
/// seekability for an answer would wait on a peer while holding whatever it
208+
/// holds for the length of the call.
209+
#[cfg(not(windows))]
210+
pub fn reads_without_waiting(fd: crt_fd::Borrowed<'_>) -> bool {
211+
is_seekable(fd)
212+
}
213+
214+
#[cfg(windows)]
215+
pub fn reads_without_waiting(fd: crt_fd::Borrowed<'_>) -> bool {
216+
use std::os::windows::io::AsRawHandle;
217+
use windows_sys::Win32::Storage::FileSystem::{FILE_TYPE_DISK, GetFileType};
218+
219+
let Ok(handle) = crt_fd::as_handle(fd) else {
220+
return false;
221+
};
222+
unsafe { GetFileType(handle.as_raw_handle() as _) == FILE_TYPE_DISK }
223+
}
224+
202225
pub fn validate_whence(whence: i32) -> bool {
203226
let standard = (0..=2).contains(&whence);
204227
#[cfg(any(target_os = "dragonfly", target_os = "freebsd", target_os = "linux"))]

‎crates/host_env/src/io_unsupported.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ pub fn is_seekable(_fd: crt_fd::Borrowed<'_>) -> bool {
176176
false
177177
}
178178

179+
pub fn reads_without_waiting(_fd: crt_fd::Borrowed<'_>) -> bool {
180+
false
181+
}
182+
179183
pub fn validate_whence(whence: i32) -> bool {
180184
(0..=2).contains(&whence)
181185
}

0 commit comments

Comments
 (0)