bug#21394: 25.0.50; Segfault when displaying unprintable character in ec

This code in xdisp.c looks very suspicious to me:

------
static int last_escape_glyph_face_id = (1 << FACE_ID_BITS);
static int last_escape_glyph_merged_face_id = 0;

static int
merge_escape_glyph_face (struct it *it)
{
int face_id;

if (it->f == last_escape_glyph_frame
      && it->face_id == last_escape_glyph_face_id)
    face_id = last_escape_glyph_merged_face_id;
else
    {
      /* Merge the `escape-glyph' face into the current face. */
      face_id = merge_faces (it->f, Qescape_glyph, 0, it->face_id);
      last_escape_glyph_frame = it->f;
      last_escape_glyph_face_id = it->face_id;
      last_escape_glyph_merged_face_id = face_id;
    }
return face_id;
}
------

That caches a face id in last_escape_glyph_merged_face_id, which is cleared only in redisplay_internal(). But message() doesn't call redisplay_internal(), it calls try_window() directly (xdisp.c:10687) (and resize_window before that, which blows up).

This patch appears, so far, to run without a segfault (I neglected to record timings so can't give you a p-value, but back of the envelope it's >99% that it fixes the segfault), and it fixes what I think is a possible explanation for the segfault, so my current plan is to leave it running overnight and ask for inclusion (of this or an equivalent, cleaner patch, of course) if it doesn't blow up.

However, I have yet to look at the other bugs and doubt this is the whole story.

On Tue, Sep 1, 2015 at 7:37 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> Date: Tue, 1 Sep 2015 18:52:44 +0000
> From: Pip Cet <pipcet@gmail.com>
> Cc: 21394@debbugs.gnu.org
>
> I see the problematic face always has face ID of 18, and the 'used'
> field is always 15 when the segfault strikes. So I guess the next
> step is to make the breakpoint in cache_face conditional on i being
> 18,
>
> i is used in two different ways in that function, as a face hash and as an
> index into faces_by_id. I assume you mean the latter?

Yes.

> and then see whether c->used is set to 19 during that call to
> cache_face. If it does, then a watchpoint (by location) on c->used
> should show which code makes the value smaller.
>
> So I wrote a perl script to set a watchpoint on c->used whenever we allocate a
> new face cache c in make_face_cache, and clear the watchpoint when we hit
> free_face_cache. Output attached, but do let me know what else you would like
> watched. I think that has all the information your approach would have given
> us.

Sounds like the face ID of 18 is "remembered" somewhere, and not
"forgotten" when the face cache is free'd?

From:	Pip Cet
Subject:	bug#21394: 25.0.50; Segfault when displaying unprintable character in echo area while frames are being created
Date:	Tue, 1 Sep 2015 20:19:58 +0000