Subject: Re: bug#76091: 31.0.50; festure/igc: buffer.h:829: Emacs fatal error: assertion failed: BUFFERP (a)

From: Pip Cet <[email-masked]>
Date: Sat, 08 Feb 2025 10:24:01 +0000

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

>> Date: Fri, 07 Feb 2025 19:43:18 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: Helmut Eller <[email-masked]>, Gregor Zattler <[email-masked]>, Eli Zaretskii <[email-masked]>, [email-masked]
>>
>> Gerd Möllmann <[email-masked]> writes:
>>
>> >>> Well, yes.  I'll split it up into individual patches, so we can review
>> >>> them one by one?
>> >>
>> >> Not necessary, but thanks.
>> >
>> > I'm through. Except for the shrinking case that you already mentioned I haven't found other problems.
>> >
>> > So LGTM.
>>
>> Thanks!
>>
>> Hope I didn't break anything, but I just pushed the series.  Please feel
>> free to revert or fix as appropriate!
>
> Here I get compilation warnings:
>
>   igc.c: In function 'igc_realloc_ambig':
>   igc.c:3324:55: warning: potential null pointer dereference [-Wnull-dereference]
>    3324 |   ptrdiff_t old_size = (char *)r->d.end - (char *)r->d.start;
> 	|                                                   ~~~~^~~~~~
>   igc.c:3324:36: warning: potential null pointer dereference [-Wnull-dereference]
>    3324 |   ptrdiff_t old_size = (char *)r->d.end - (char *)r->d.start;
> 	|                                ~~~~^~~~
>   igc.c:3324:55: warning: potential null pointer dereference [-Wnull-dereference]
>    3324 |   ptrdiff_t old_size = (char *)r->d.end - (char *)r->d.start;
> 	|                                                   ~~~~^~~~~~
>   igc.c:3324:36: warning: potential null pointer dereference [-Wnull-dereference]
>    3324 |   ptrdiff_t old_size = (char *)r->d.end - (char *)r->d.start;
> 	|                                ~~~~^~~~
>   igc.c: In function 'igc_xpalloc_exact':
>   igc.c:3397:27: warning: argument to variable-length array may be too large [-Wvla-larger-than=]
>    3397 |       volatile mps_word_t area[(item_size + (sizeof (mps_word_t) - 1))
> 	|                           ^~~~
>   igc.c:3397:27: note: limit is 4031 bytes, but argument may be as large as 4294967292
>   igc.c: In function 'igc_xnrealloc_ambig':
>   igc.c:3415:57: warning: potential null pointer dereference [-Wnull-dereference]
>    3415 |   ptrdiff_t old_nbytes = (char *)r->d.end - (char *)r->d.start;
> 	|                                                     ~~~~^~~~~~
>   igc.c:3415:38: warning: potential null pointer dereference [-Wnull-dereference]
>    3415 |   ptrdiff_t old_nbytes = (char *)r->d.end - (char *)r->d.start;
> 	|                                  ~~~~^~~~
>   igc.c:3415:57: warning: potential null pointer dereference [-Wnull-dereference]
>    3415 |   ptrdiff_t old_nbytes = (char *)r->d.end - (char *)r->d.start;
> 	|                                                     ~~~~^~~~~~
>   igc.c:3415:38: warning: potential null pointer dereference [-Wnull-dereference]
>    3415 |   ptrdiff_t old_nbytes = (char *)r->d.end - (char *)r->d.start;
> 	|                                  ~~~~^~~~
>
> Most of them seem to be because root_find can return NULL.  The nit

Those are harmless; I'll add an eassert (r), that should prevent the
warnings (together with Gerd's fix for the igc_realloc (NULL) case).

> about arena[] is probably because one cannot use sizeof and variables
> in an array declaration?  We should probably either use the maximum as

I'll add another eassert to ensure no one is calling this with
unreasonably large item_size values.  If anyone does, it's probably best
to crash early.

> a constant, or use alloca instead.

The vla limit applies to alloca as well.

> Also, does this:
>
>> (Of course, with WIDE_EMACS_INT, mps_word_t is not good enough; unless
>> we carefully write the scan function not to assume that the two
>> half-words comprising a 64-bit Lisp_Object are in sync.  As Eli was very
>> opposed to the idea of removing WIDE_EMACS_INT again, we might have to
>> find a workaround there.)
>
> mean that the wide-int 32-bit build is now in trouble?

Yes, it does.  All scanning functions need to be written to deal with
half-written Lisp_Object values in that case.  (Easier than it sounds
because we USE_LSB_TAG).

> Or did you install some workaround for the above?

Not yet :-)

Pip