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

[Pip Cet]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> Date: Mon, 13 Jan 2025 18:21:05 +0000
>> From: Pip Cet <pipcet@protonmail.com>
>> Cc: 75521@debbugs.gnu.org, stefankangas@gmail.com
>>
>> "Eli Zaretskii" <eliz@gnu.org> writes:
>>
>> >> 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).
>
> I think there's a misunderstanding here.  Can you please walk me
> through the code below:
>

>   Lisp_Object table = AREF (font_style_table, prop - FONT_WEIGHT_INDEX);

At this point, table, AREF (font_style_table, 0), and Vfont_weight_table
all refer to the same Lisp object T.

>       elt = make_vector (2, make_fixnum (100));
>       ASET (elt, 1, val);

elt is a new Lisp object.

>       ASET (font_style_table, prop - FONT_WEIGHT_INDEX,
>           CALLN (Fvconcat, table, make_vector (1, elt)));

1. make_vector creates a temporary one-element vector
2. vconcat creates a new Lisp object, T2
3. AREF (font_style_table, 0) is changed to refer to T2

Vfont_weight_table still refers to T.

> and tell what is the "old vector" here that you are talking about?

The old vector is T.  It is no longer visible to GC once the function
terminates and table goes out of scope.  It is still the value of
Vfont_weight_table.

> And if that old vector is reachable from font_style_table, why do you
> say it is not marked, if mark_object_root_visitor marks its argument
> recursively?

mark_object_root_visitor is never called for Vfont_weight_table because
it is not staticpro'd.  It is called for font_style_table, but
font_style_table no longer refers to T; it refers to T2 only.

>> >> 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.
>
> So you are saying that the problem that we don't assign the new value
> (received from vconcat) to the appropriate variable
> (Vfont_weight_table etc.)?

My first patch assumes that behavior was intentional, and retains it; my
second patch assumes it wasn't, and changes things so font_style_table
is no longer needed, because we just use Vfont_weight_table directly in
its place.

We should not attempt to keep two different memory locations
synchronized by trying to catch all modifications.  That never works,
and there's no reason for this complication.

>> > 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.
>
> Sorry, you lost me here.

As I've tried to explain a number of times, the problem is that AREF
(font_style_table, 0) isn't necessarily equal to Vfont_weight_table.
The comment claims it is, but that's incorrect.

Pip