Skip to content

Raise where the fuzzer found aborts and stack overflows, and collect with the count in the object header - #8551

Draft
youknowone wants to merge 12 commits into
RustPython:mainfrom
youknowone:fuzzer-issues
Draft

Raise where the fuzzer found aborts and stack overflows, and collect with the count in the object header#8551
youknowone wants to merge 12 commits into
RustPython:mainfrom
youknowone:fuzzer-issues

Conversation

@youknowone

@youknowone youknowone commented Aug 18, 2026

Copy link
Copy Markdown
Member

Nine changes from the current fuzzing and static-review sweep. Each one is a
separate commit with its own reasoning; the branch is rebased onto main, and
everything the last sweep already landed has been dropped from it.

Crashes that are now exceptions

  • Allocations sized by Python input. struct.pack('%dx' % 2**60),
    (0).to_bytes(2**60, 'big'), os.read(fd, 2**60) and
    (ctypes.c_char * 2**60)() all reached handle_alloc_error, which under
    panic = "abort" ends the process. They raise MemoryError now, as they do
    in the reference. itertools.product(..., repeat=2**60) and
    _ssl.RAND_bytes(2**40) raise OverflowError; product_new gained the
    negative and too-large checks it has upstream, and RAND_bytes takes the C
    int its signature says it does.
  • The native stack. check_c_stack_overflow() ran on every eighth frame
    entry, and the margin does not cover eight frames of a recursion that
    re-enters through native code. A class Add: __add__ = lambda self, o: self + o
    chain segfaulted on the main thread of a debug build, and a self-sorting
    key= died on SIGILL. The check runs on every entry now; the cost is
    +0.17% instructions retired on a call-heavy benchmark (~4 instructions per
    call).
  • map.__length_hint__. map_methods has no __length_hint__, so
    operator.length_hint() on a map answers 0. Ours walked into the length hint
    of every iterator it held, and a chain of maps 10000 long overflowed the
    native stack answering for the outermost one. It also took the longest of its
    iterators, where a map stops at the shortest.
  • Threads in debug builds. threading.stack_size(256*1024) then starting a
    thread aborted. ExecutingFrame::run reserves 80,848 bytes in a debug build
    against 656 in release — execute_instruction is #[inline(always)] with 200
    instruction arms, and LLVM's StackColoring pass only runs at opt-level >= 1 —
    so one Python call costs 88,672 bytes and the threading bootstrap's six frames
    need 532,032. An explicit size is a floor rather than the size in debug builds
    now; release honours it exactly and threading.stack_size() still reports
    what was asked for.

Length hints

map_py_iter read __length_hint__ only to hand it to PyIterIter, and
returned an empty vector outright when the hint was isize::MAX or more — so
list(x) where x.__length_hint__() is sys.maxsize answered []. Collecting
through PyResult drops the iterator's lower bound, so nothing reserved the room
the hint asked for either.

list(), list.extend() and list.__iadd__() reserve it now and report a hint
they cannot honour as MemoryError; a hint that leaves no room for what the list
already holds is passed over, as list_extend() does. tuple() keeps filling up
without reserving, which is also what the reference does. Errors from
length_hint_opt other than the TypeError it already turns into None reach
the caller.

__iadd__ and inplace_concat went through extract_cloned, which reads
__len__ and not __length_hint__; both call PyList::extend now, the way
list_inplace_concat() calls list_extend(). The tuple, list and dict fast
paths of extract_elements_inner reserve their known length, which the
collect() they used dropped: list.extend() is 9.9% fewer instructions and a
list-construction benchmark 1.9% fewer.

Who is asked for a length hint

map_py_iter asked the iterable it was handed, for every caller. Only some
callers ask it: list_extend() and _PyBytes_FromIterator() ask the iterable,
PySequence_Tuple() asks the iterator, and the bytearray constructor asks
nothing. Which object is asked is the caller's to say now, so tuple(),
min(), max(), collections.deque() and f(*x) answer for an iterable whose
__len__ raises, where they used to report it; sorted() asks the iterable,
being PySequence_List().

bytes_from_object() stood in for PyBytes_FromObject(), for
bytearray_extend() and for the bytearray constructor, which do not agree on
this — the first two ask, the last does not. It is split, and assigning to a
bytearray slice takes the constructor's side with PyByteArray_FromObject().

list.extend() counted what it held before the iterable had been asked, where
list_extend() reads Py_SIZE(self) after. A __length_hint__ that adds to
the list made the guard read a count too small and raise MemoryError where
nothing is wrong; one that empties it made the guard skip a reservation that
cannot be served.

Two more that turned up alongside, neither of them new:

  • product_new() works out npools before it calls PySequence_Tuple() on any
    argument, and fills the pools npools times. Ours filled them by repeating
    the arguments repeat times, which walks that many steps even with no
    arguments to repeat — product(repeat=2**62) counted up to it rather than
    answering [()] — and worked the count out after reading the arguments, so a
    repeat too large to serve ran their code first.
  • pack_single() leaves '?' to PyObject_IsTrue() and returns what that
    raised. Packing classified the error instead, so a ValueError from a
    __bool__ came back as memoryview: invalid value for format '?'.

Collector

The collection kept its candidates and their working counts in a side table
keyed by address. Both now live in the object header, in a gc_refs field that
fits in the padding a 64-bit header already had — the header is unchanged at 48
bytes there, and 28 rather than 24 bytes on 32-bit, which the wasm32 build
covers. On a GC benchmark the median live-object collection went from 0.101s to
0.055s and the dead-object one from 0.402s to 0.383s.

Smaller fixes

  • memoryview item assignment raised TypeError for a value of the right kind
    that does not fit; it is a ValueError, and struct's checks and messages
    are matched alongside it.
  • seek_fd on Windows checked only the low half of the file position, so
    INVALID_SET_FILE_POINTER could not be told from a valid position with the
    same low word.
  • A comment claimed the non-unix frame stack was something it is not.

Left alone

Two divergences found along the way want a restructure larger than this branch
should carry, and neither is a crash:

  • memoryview(...).cast('f')[0] = 3.5e38 reports ValueError where
    pack_single() casts and stores inf. _struct packs the same format
    through PyFloat_Pack4(), which does raise, and one Packable implementation
    serves both here.
  • os.read(fd, 2**60) reports MemoryError where CPython returns the data:
    PyBytes_FromStringAndSize(NULL, n) leaves the buffer uninitialised, and a
    page-lazy malloc serves what a calloc will not. Matching it needs a
    fallible uninitialised buffer, which cannot be spelled soundly on stable Rust.
    This branch already moves that call from aborting the process to raising.

Verification

  • Full regrtest sweep: 418 tests OK. The two remaining are the same before and
    after — test.test_future_stmt.test_future (barry_as_FLUFL in the REPL,
    documented unimplemented) and test_set reporting env changed from
    check_free_after_iterating, both reproduced on an unmodified build.
    test_pyrepl is excluded for taking 24 minutes to reach its existing 48
    failures.
  • All 428 snippets pass; new ones cover each case above.
  • clippy clean on both the main and the wasm command lines, rustfmt and
    ruff clean, and the wasm32 build passes.
  • Every case above was diffed against CPython 3.14 output and matches.

The comment said non-unix threading builds have no stop-the-world.
CollectStopTheWorld is gated on `feature = "threading"` alone, and
sys._current_frames stops the world on both paths; the field is the
fallback a reader uses when there is no `top_iframe` to materialize from.

Assisted-by: Claude
`SetFilePointer` answers with the low half of the new position and signals
failure with INVALID_SET_FILE_POINTER, which is also that half of a
position four gigabytes in; telling them apart takes the error code, which
this did not read, so such a seek was reported as an error. Deciding
seekability from it also called such a file unseekable.

`SetFilePointerEx` returns the whole position and a success flag of its
own, which also removes the transmute of the position into halves.

Assisted-by: Claude
Assigning an item reported every packing failure as a TypeError, so
m[0] = 300 on a 'B' view said the value was the wrong type rather than out
of range. Packing now says which of the two it was, and whether the value's
own code raised, in which case that error is the answer as it is:

    m[0] = 300                       ValueError: invalid value for format 'B'
    m[0] = "x"                       TypeError: invalid type for format 'B'
    m[0] = <__index__ that raises>   the raised error

`struct` reports both as `struct.error` and is unchanged; the kind travels
beside the exception for the caller that tells them apart.

Also: None was read as a deletion, so m[0] = None answered "cannot delete
memory" instead of packing it; and deleting through the mapping protocol
never reached the read-only check, which comes first.

Assisted-by: Claude
os.read, _RawIOBase.read, int.to_bytes, struct.pack, ctypes array
creation and the _ssl RAND functions sized a Vec from a Python-supplied
length with vec![], which calls handle_alloc_error and, under
panic = "abort", ends the process. They now allocate through
vm.new_zeroed_bytes and raise MemoryError.

itertools.product built its pools without checking that
len(iterables) * repeat is representable; it now raises OverflowError
"repeat argument too large" and reserves the pool and index vectors
fallibly. repeat is read as isize, so a negative one raises ValueError
"repeat argument cannot be negative" instead of the conversion's message.

_ssl.RAND_bytes and RAND_pseudo_bytes read n as i32, matching the int
they are declared with.

Assisted-by: Claude
A collection kept the count it was working with in a table keyed by the
object's address, and the objects it had proved reachable in a second one.
Between them they were hashed once per candidate and twice per edge in the
heap, which is where most of a collection over a live heap went.

The count now lives in `PyInner::gc_refs`, with `GcBits::COLLECTING` saying
it is meaningful, and reachability is `gc_refs == GC_REACHABLE` rather than
membership in a set. Step 5 splits the candidates and clears the bit in one
pass. gcbench, five interleaved pairs, median: a live heap of 423k objects
goes from 0.101s to 0.055s and a dead one from 0.402s to 0.383s.

The bits, generation, owner and count take eight bytes between them. A
64-bit header had those eight as the padding its alignment forces, so it is
unchanged at 48 bytes; a 32-bit header grows from 24 to 28.

Assisted-by: Claude
A debug build already started Python threads on 8 MiB rather than Rust's
2 MiB default, but an explicit threading.stack_size(N) went through
verbatim. test_threading asks for 256 KiB, and starting a thread on that
walked off the end of the stack: the guard page fault landed in the
prologue of ExecutingFrame::run.

Unoptimized, that prologue reserves 80,848 bytes where the optimized one
reserves 656 -- execute_instruction is #[inline(always)] and LLVM only
colors stack slots from opt-level 1, so the frame is the sum of all 200
instruction arms' temporaries rather than the largest. A Python call costs
88,672 bytes of native stack there, and threading's bootstrap is six
frames deep, so 256 KiB holds less than half of what starting a thread
takes.

The floor reaches thread::Builder only; threading.stack_size() still
answers with what was asked for, and release builds are unchanged.

Assisted-by: Claude
The C-stack guard ran on one frame entry in eight. That asks the margin to
cover eight frames rather than one, and it does not: an unoptimized frame
entered through native code takes 88,672 bytes against a debug margin of
262,144. A recursion whose steps re-enter that way -- `__add__` calling
itself, a sort key that sorts -- ran off the end of the stack instead of
raising RecursionError. On a debug build `class Add: __add__ = lambda s, o:
s + o; Add() + 1` segfaulted on the main thread; it now raises, as it does
under CPython and in release builds.

enter_iframe checked and then called enter_iframe_unchecked, which checked
again; it now leaves the check to the one call.

Measured on a call-dominated benchmark, five interleaved pairs: instructions
retired go up 0.17%, about four per call, which is the stack pointer read
and the compare.

Assisted-by: Claude
`map_py_iter` read `__length_hint__` only to pass it to `PyIterIter`, and
returned an empty vector when the hint was `isize::MAX` or more. Collecting
through `PyResult` dropped the iterator's lower bound, so nothing reserved
the room the hint asked for.

`list()`, `list.extend()` and `list.__iadd__()` now reserve it and report a
hint they cannot honour as `MemoryError`; a hint that leaves no room for the
elements the list already holds is passed over, as `list_extend()` does.
`tuple()` and the other callers keep filling up without reserving.

`length_hint_opt` errors other than the `TypeError` it already turns into
`None` now reach the caller instead of being dropped.

`__iadd__` and `inplace_concat` went through `extract_cloned`, which reads
`__len__` and not `__length_hint__`; both call `PyList::extend` now, the way
`list_inplace_concat()` calls `list_extend()`.

The tuple, list and dict fast paths of `extract_elements_inner` reserve their
known length, which the `collect()` they used dropped.

Assisted-by: Claude
`map_methods` has no `__length_hint__`, so `operator.length_hint()` on a map
answers 0, not the length of what it draws from.

The method walked into the length hint of every iterator it holds, and a
chain of maps 10000 long overflowed the native stack answering for the
outermost one. It also took the longest of its iterators, where a map stops
at the shortest.

Assisted-by: Claude
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: c4e0dac6-a658-48f9-8b59-991022cf2740

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

`map_py_iter` asked the iterable it was handed, for every caller, and reported
what asking raised. Only some callers ask it: `list_extend()` and
`_PyBytes_FromIterator()` ask the iterable, `PySequence_Tuple()` asks the
iterator, and the bytearray constructor asks nothing. `tuple()`, `min()`,
`max()`, `collections.deque()` and `f(*x)` raised for an iterable whose
`__len__` or `__length_hint__` does, where they answer.

Which object is asked is now the caller's to say. `sorted()` asks the iterable,
being `PySequence_List()`.

`bytes_from_object()` stood in for `PyBytes_FromObject()`, for
`bytearray_extend()` and for the bytearray constructor, which do not agree on
this: the first two ask, the last does not. It is split, and assigning to a
bytearray slice takes the constructor's side with `PyByteArray_FromObject()`.

`list.extend()` counted what it held before the iterable had been asked, where
`list_extend()` reads `Py_SIZE(self)` after. A `__length_hint__` that adds to
the list made the overflow guard read a count too small and raise `MemoryError`
where nothing is wrong; one that empties it made the guard skip a reservation
that cannot be served.

Assisted-by: Claude
`product_new()` checks `repeat` and works out `npools` before it calls
`PySequence_Tuple()` on any argument, and fills the pools `npools` times.

The pools were filled by repeating the arguments `repeat` times instead, which
walks that many steps even with no arguments to repeat: `product(repeat=2**62)`
counted up to it rather than answering `[()]`. The count was also worked out
after the arguments had been read, so a repeat too large to serve ran their
code first.

Assisted-by: Claude
`pack_single()` leaves `'?'` to `PyObject_IsTrue()` and returns what that
raised. Packing classified the error instead, so a `ValueError` from a
`__bool__` came back as "memoryview: invalid value for format '?'".

Assisted-by: Claude
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant