bug#52888: 29.0.50; font_{delete_unmatched,score} do not handle nil FONT_WEIGHT_INDEX

[Sean Whitton <spwhitton@spwhitton.name>]

Hello,

On Thu 30 Dec 2021 at 08:39PM +02, Eli Zaretskii wrote:

> Does it really make sense to accept these fonts in some situations,
> but not in others?  AFAIU, what you suggest would cause Emacs to
> accept these fonts when :weight is not mentioned (and so defaults to
> 'normal'), but to reject them if the 'normal' weight is specified
> explicitly, is that right?  If so, it's confusing, and users will
> complain.  Rejecting such fonts outright is at least consistent, and
> thus better than semi-support.

Ah, yes, that would indeed be confusing.

> I installed on the release branch a temporary fix, similar to what you
> suggested, to avoid undefined behavior with those fonts, but I don't
> think we should install something like that on master.  On master, I
> think ftfont.c and its ilk should be fixed to handle these fonts
> correctly, or reject them if we cannot DTRT with them for some reason.
> I think the fact that we create invalid font entities from such fonts
> is a clear sign that the font backend mishandles them, and if so,
> that's where this problem should be corrected: we should create valid
> font entities to begin with, with ;weight and other similar attributes
> having numerical values, as expected.

I spent some more time in gdb and learned the following.

The ftcrhb backend returns a FcPattern for each of the weights contained
in Inconsolata-VariableFont_wdth,wght.ttf.  So Emacs does not need to
learn anything special about these variable weight files in order to
support them, I think.  However, this code in ftfont_pattern_entity can
sometimes set FONT_WEIGHT_INDEX to nil:

  if (FcPatternGetInteger (p, FC_WEIGHT, 0, &numeric) == FcResultMatch)
    {
      FONT_SET_STYLE (entity, FONT_WEIGHT_INDEX, make_fixnum (numeric));
    }

I haven't yet determined exactly when this can happen, but it suggests
the problem is within FONT_SET_STYLE, as all the backend-specific code
is doing is upplying 'numeric'.

If I might ask a gdb question: to try to determine when this code can
set FONT_WEIGHT_INDEX to nil, I set a breakpoint right after it and then
tried

    condition NN NILP (AREF (entity, FONT_WEIGHT_INDEX))

but this didn't work -- is it possible to do something like that?

Thanks.

-- 
Sean Whitton