Calling Lisp from undo.c's record_* functions

[Eli Zaretskii]

Bootstrapping Emacs crashed on me today while building python-wy.el.
Emacs segfaulted while trying to access memory outside its address
space.

Debugging uncovered the following sequence of calls:

  . some Lisp calls 'insert' whose argument is a 12K string
  . this eventually calls insert_from_string_1, which enlarges the
    buffer gap to accommodate for the inserted text
  . in the midst of manipulating the gap, insert_from_string_1 calls
    record_insert
  . record_insert calls record_point, which calls run_undoable_change,
    which calls Lisp
  . the Lisp interpreter decides it's a good time to GC and calls
    garbage_collect
  . garbage_collect calls compact_buffer, which decides the buffer in
    which the insertion happened can be compacted (since the gap
    manipulation is not yet done, and it looks like the buffer has a
    lot of slack space), so it shrinks the gap
  . bottom line: the gap was shrunk behind the back of
    insert_from_string_1, which totally doesn't expect that, and
    proceeds doing silly things, like setting the gap size to a large
    negative value, and from there we are on a certain and very short
    path to a crash

This was caused by a recent change that added a call to
run_undoable_change to various functions in undo.c that record
changes; run_undoable_change calls a Lisp function.

My dilemma is: how to fix this cleanly and correctly?

The record_* functions that are affected by this are called from quite
a few places, most of them in insdel.c, but some in other places.  I
didn't audit all of them, but those I did generally manipulate the gap
and have C pointers to buffer text lying around, because they don't
expect any Lisp to be run or GC to happen.  All of those places are
now living dangerously.

Question #1: do we really need to call Lisp from so deep inside the
bowels of buffer manipulation routines?  Is that safe?  Perhaps we
should reimplement undo-auto--undoable-change inC?

Question #2: one solution is inhibit GC in run_undoable_change.  But
since that could run arbitrary Lisp, is that a good idea? what if we
run out of memory?

Question #3: another possible solution is to set the current buffer's
inhibit_shrinking flag around the call to Lisp in run_undoable_change
-- is this better?  Note that this won't prevent GC in general, so the
follow-up question is can insdel.c functions afford a GC while they
run?

Comments?  Suggestions?

TIA