bug#7587: 23.2; `format-mode-line' makes Emacs crash

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: michael_heerdegen@web.de,  7587@debbugs.gnu.org
> Date: Fri, 10 Dec 2010 15:50:31 -0500
> 
> > This happens because, for some reason I don't understand, when
> > face-remapping-alist is non-nil, lookup_basic_face insists on getting
> > only one of the basic faces, and otherwise aborts:
> 
> I don't know anything about this code

Who does?  Miles, do you?

> but the name `lookup_basic_face' suggests it only works for basic
> faces

Actually, it works for non-basic faces as well--but only if
face-remapping-alist is nil.  See this part of it, which I cited:

  if (NILP (Vface_remapping_alist))
    return face_id;             /* Nothing to do.  */

The patch I suggest simply make lookup_basic_face behave consistently
regardless of the value of face-remapping-alist.

> so maybe the problem is that the function gets incorrectly called
> for a non-basic face.

It _does_ get called with a basic face (`mode-line'), but that face
has been remapped.  Remapping actually creates a new face ID in the
face cache, but lookup_basic_face gets an ID (an integer), not a
symbol Qmode_line etc.  So if the face ID that lookup_basic_face gets
is not one of the IDs reserved for the 12 basic faces, _and_
face-remapping-alist is non-nil, lookup_basic_face always pukes.

Here's more context about the chain of calls that leads to the crash:

The call to lookup_basic_face comes from init_iterator:

  /* Perhaps remap BASE_FACE_ID to a user-specified alternative.  */
  if (! NILP (Vface_remapping_alist))
    remapped_base_face_id = lookup_basic_face (XFRAME (w->frame), base_face_id);

init_iterator does this because it expects a truly basic face in
base_face_id, and it wants to use its face-remapped variety, as the
user would expect.  But in this case, we pass to it an already
remapped basic face, see this part of format-mode-line:

  if (!NILP (face))
    {
      if (EQ (face, Qt))
        face = (EQ (window, selected_window) ? Qmode_line : 
Qmode_line_inactive);
      face_id = lookup_named_face (XFRAME (WINDOW_FRAME (w)), face, 0);
    }
  ...
  init_iterator (&it, w, -1, -1, NULL, face_id);

lookup_named_face returns the face ID of the remapped mode-line face.
So in this case, face_id is already an ID of the remapped mode-line
face, and returning it unaltered from lookup_basic_face is TRT.

As an alternative, we could refrain from calling lookup_basic_face in
init_iterator if the argument base_face_id is not one of the 12 basic
face IDs.