Subject: Re: bug#58334: 29.0.50; ASAN heap use after free in gui_produce_glyphs

From: Eli Zaretskii <[email-masked]>
Date: Fri, 07 Oct 2022 14:44:02 +0300

> From: Gerd Möllmann <[email-masked]>
> Cc: [email-masked],  [email-masked]
> Date: Fri, 07 Oct 2022 13:29:38 +0200
> 
> If we don't change some other shared state, then we're safe if we
> prevent freeing faces?  That's would be good.

Yes, I think so.  But preventing freeing the faces is a losing game,
in the long run, because we cannot prevent that forever without
adversely affecting the Emacs memory footprint.

I think a better way is to re-generate the faces when we discover they
were freed.  This is easy for the basic faces, but fundamentally
impossible for non-basic ones.  That's why I asked you earlier whether
the offending face was a basic one.  However, I think we can rely on
inhibit_free_realized_faces to avoid freeing non-basic faces, if we
use that flag in strategic places.  Basically, non-basic faces are
realized and cached by redisplay itself, so theoretically we should be
able to prevent their freeing (and perhaps we already have that in
place, see redisplay_internal).

So I would recommend to fix FACE_FROM_ID to re-generate the basic
faces if needed, on the assumption that the cases where we have
problems with using face ID are limited to basic faces.  If, after
that, we will find cases with non-basic faces, I'd first look for more
opportunities to use inhibit_free_realized_faces.

One other thing is that inhibit_free_realized_faces is a boolean, so
if nesting is possible, it cannot support such nesting; we'd need a
reference count instead.