bug#40661: Crash in regex search during redisplay

[Richard Copley <rcopley@gmail.com>]

On Mon, 20 Apr 2020 at 20:07, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Richard Copley <rcopley@gmail.com>
> > Date: Mon, 20 Apr 2020 19:30:29 +0100
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Daniel Colascione <dancol@dancol.org>, 40661@debbugs.gnu.org
> >
> > I have a little worry about the situation where the procedure of doing
> > a "decoding" entails a regexp buffer search. (I can't make out whether
> > or not "decoding" means executing arbitrary Lisp code.)
>
> This can happen if the coding-system has a post-read-conversion
> attribute, yes.
>
> > Is it possible, with your patch, that we might re-enable buffer
> > shrinking sooner than desirable?
>
> I don't think I understand the scenario you have in mind.  If decoding
> calls regexp search, then we inhibit shrinking inside the regex
> routines, not outside of them.  Which code will re-enable shrinking
> before the regex routines return?
> > (This is reiterating a worry you yourself mentioned earlier, "I'm
> > not sure we want to conflate these two purposes".)
>

What I had in mind was that we do inhibit shrinking outside the regex
search routines, for example (coding.c:8005),

  current_buffer->text->inhibit_shrinking = 1;
  decode_coding (coding);
  current_buffer->text->inhibit_shrinking = 0;

I was worried about the case where a regex buffer search happens 'half
way through' decode_coding. Then something bad might happen during the
second half, when inhibit_shrinking is zero.

But ...

> coding.c uses this flag only while it runs purely C code.  Lisp cannot
> run at that point.  If a coding-system has a post-read-conversion, it
> will run only after the C decoder finishes.

... so that's OK then.

> > In the comment, you mention relocation as well as shrinking.
>
> It's the relocation that causes the crash, because following the
> relocation we unmap a portion of the process memory, where previously
> we had buffer text, from the process's address space.  Shrinking of
> the gap is what triggers the relocation (when the original gap was
> very large).
>
> > Does it make sense to combine this new guard with the existing
> > freeze_buffer_relocation in some way?
>
> I thought about that, but decided against it, for two reasons.  First,
> freeze_buffer_relocation should at some point go away: it is not a
> no-op only in an Emacs that uses ralloc.c, which currently only the
> MS-DOS build does, AFAIK.

OK.

> And second, it freezes relocation at a
> higher level than needed, above the re_match_2_internal function which
> is the only one that can call Lisp.

Right, but freeze_buffer_relocation was surely done like that on
purpose, to hoist it outside the loop in search_buffer_re. Anyway, I'm
sure it's no big deal.