Subject: Re: bug#77325: Crash in Fjson_parse_buffer: ZV changes underneath it?

From: Pip Cet <[email-masked]>
Date: Fri, 28 Mar 2025 15:05:22 +0000

"Daniel Colascione" <[email-masked]> writes:

> Eli Zaretskii <[email-masked]> writes:
>
>>> From: Daniel Colascione <[email-masked]>
>>> Date: Thu, 27 Mar 2025 21:07:02 -0400
>>>
>>>
>>> Somehow, the buffer changes underneath json_parse.  We pass an
>>> out-of-bounds position to SET_PT_BOTH (position, byte), which either
>>> asserts or crashes.  Not sure how the buffer could have changed ---
>>> maybe a handler-bind?  The JSON parser doesn't seem to do anything
>>> except allocate and signal.
>>
>> Can you post a recipe for reproducing this?
>
> Didn't have a good repro.  Pip's fix works though.  I was barking up
> the wrong tree: I'm parsing JSON out of a process buffer in a loop and
> dispatching commands as they come in. One of these commands switched the
> buffer, so in the next iteration of the loop, I started parsing JSON out
> of some other random buffer.  It just so happened that other buffer was
> narrowed, so we crashed.  I'll let Pip do the honors of checking in the
> fix if he wants.

Eli, is that okay?  I'll simplify the else branch, which has an
unnecessary "else if" in the original patch.

> I initially thought a GC finalizer might have been switching the buffer,
> but turns out GC doesn't actually run for me while parsing.

json.c assumes no GC on the master branch, because it doesn't protect
its object workspace (and possibly for other reasons).

> IGC does GC all the time --- but it's not observable because we pump
> messages from the GC only at dedicated points and run GC hooks only in
> response to these messages. however, notice that on the IGC branch that
> we pump GC messages, including finalizer callbacks, on the allocation
> path for, e.g. various pseudovectors.  That'll cause Lisp to run where
> it wouldn't have before. Is that going to be a problem?  ISTM we can
> either pump messages in maybe_quit() or just rely on igc_on_idle().

Oh, I forgot about that one!  I'll open a new bug so this gets a
number: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=77338

Pip