Subject: Re: bug#75521: scratch/igc: Delete unused macro DEFVAR_LISP_NOPROX

From: Pip Cet <[email-masked]>
Date: Mon, 13 Jan 2025 18:21:05 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Mon, 13 Jan 2025 17:18:58 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> >> Date: Mon, 13 Jan 2025 16:46:39 +0000
>> >> From: Pip Cet <[email-masked]>
>> >> Cc: [email-masked], [email-masked]
>> >>
>> >> "Eli Zaretskii" <[email-masked]> writes:
>> >>
>> >> >> Date: Mon, 13 Jan 2025 14:16:04 +0000
>> >> >> From: Pip Cet <[email-masked]>
>> >> >> Cc: [email-masked], [email-masked]
>> >> >>
>> >> >> "Eli Zaretskii" <[email-masked]> writes:
>> >> >>
>> >> >> >> But then font_style_table is sometimes modified so it points to
>> >> >> >> (i.e. contains as its element) a new vector, and I don't see how
>> >> >> >> Vfont_weight_table is protected then.
>> >> >> >
>> >> >> > Modified where?
>> >> >>
>> >> >> font_style_to_value:
>> >> >>
>> >> >>       if (! noerror)
>> >> >> 	return -1;
>> >> >>       eassert (len < 255);
>> >> >>       elt = make_vector (2, make_fixnum (100));
>> >> >>       ASET (elt, 1, val);
>> >> >>       ASET (font_style_table, prop - FONT_WEIGHT_INDEX,
>> >> >> 	    CALLN (Fvconcat, table, make_vector (1, elt)));
>> >> >>       return (100 << 8) | (i << 4);
>> >> >
>> >> > OK, but the values gets plugged int font_style_table, right?  And my
>> >> > reading of mark_object_root_visitor is that it marks its argument
>> >> > recursively, so any object reachable from font_style_table should also
>> >> > be protected?  Am I wrong?
>> >>
>> >> No, you're not.  Initially, Vfont_weight_table is protected because it's
>> >> reached recursively from font_style_table.  If font_style_table is
>> >> modified no longer to contain Vfont_weight_table (by the code above),
>> >> Vfont_weight_table loses its protection.  Accessing Vfont_weight_table
>> >> from Lisp when it's no longer protected will eventually cause a crash.
>> >
>> > But the above code just uses ASET to put in one of font_style_table's
>> > slots a Lisp vector.  That vector is therefore reachable from
>> > font_style_table (which is also a Lisp vector).  No?
>>
>> But the old vector, which was in the slot before, is no longer reachable
>> after the ASET.
>
> AFAIU, we copy the old vector to the new one when we call vconcat.

Fvconcat creates a new vector, with some of its contents copied from the
old vector.  It does not destroy the old vector or make it reachable
from the new vector.  The old vector remains a valid Lisp object, and
it's still reachable from Lisp by evaluating font-weight-table, but it's
not marked during GC.  (When using pdumper, it's not usually freed
because it's in the dump, and we never free dumped objects, but pdumper
also eassert()s that a dump object which was unreachable during a GC
cycle never becomes reachable again.  If you build with checking
enabled, it's easy enough to get a crash if font_style_to_value ever
modifies font_style_table, but I don't know for which font backends it
ever does that by itself: doing it in GDB triggers the bug just as I
described, though).

>> That old vector can still be referenced from Lisp as
>> font-weight-table (or one of the other two).  That means an object is
>> reachable from Lisp but not protected from GC.
>
> I don't understand: if it's reachable from Lisp, it must be protected
> by definition, no?

No, that's not correct.  GC only protects global Lisp_Object variables
if they're staticpro'd.  A non-staticpro'd Lisp_Object variable must be
protected in another way, or we have an unprotected reachable object.

> GC can only free objects that are not reachable from any other object.

No, that's not correct: the value of a forwarded symbol is not marked by
alloc.c GC.

	      case SYMBOL_FORWARDED:
		/* If the value is forwarded to a buffer or keyboard field,
		   these are marked when we see the corresponding object.
		   And if it's forwarded to a C variable, either it's not
		   a Lisp_Object var, or it's staticpro'd already, or it's
		   reachable from font_style_table which is also
		   staticpro'd.  */
		break;

That comment is incorrect, because font_style_table may contain a
newly-consed vector, not the one that's still referred to as
font-weight-table.

Pip