Subject: bug#78812: feature/igc: [PATCH] Make lface_id_to_name an exact root

From: Pip Cet <[email-masked]>
Date: Wed, 18 Jun 2025 12:27:01 +0000

"Helmut Eller" <[email-masked]> writes:

> On Tue, Jun 17 2025, Pip Cet wrote:
>
>> "Helmut Eller" <[email-masked]> writes:
>>
>>> This is a proposal to make the variable lface_id_to_name an exact root.
>>
>> Thank you!
>>
>> I'll apply this as it is, but I'd suggest one change, which I'd like to
>> quickly run by you:
>>
>> Some of the code in igc_xpalloc_lisp_objs_exact handles item_size !=
>> word_size, and some doesn't.  Would you prefer to remove the parameter
>> or change the code to handle multi-word items, which might be useful for
>> arrays of arrays of Lisp_Object?
>
> Let's remove it.  The item_size is redundant here.  See the amended
> patch.  I also added the missing xfree :-).

Thanks.  I missed that one, and I also missed that memcpy (ptr, NULL, 0)
is undefined behavior, see bug#78824.

However, the new patch looks good on both counts.

>> I think we should add label arguments to all igc*alloc* functions.  With
>> your patch, we don't see the lface-id-to-name root until the first time
>> it's been resized, and it would make more sense to see it right away.
>
> I added that as a separate patch.

Thanks!  LGTM (not that that means much, see above).

>> There's the general problem of memcpy atomicity, but I don't have a
>> solution for that; the complicated code in xpalloc_exact is still
>> insufficient, so we might as well remove it.
>>
>> While the ordinary memcpy function in glibc will always copy large
>> chunks, some debug builds use byte-based replacements, and then we have
>> a potential problem.  This is particularly annoying because
>> stress-testing such builds by triggering collections from a background
>> thread might see false positive segfaults.  I haven't seen one in
>> practice, but I use such tools very rarely.
>
> Fixing all problematic uses of memcpy would be quite tricky.  Two
> obvious cases are larger_vector and copy_hash_table.  I suppose that
> memcpy is also a reason why software write barriers would be a lot of
> work.

So let's use memcpy for now, where there's no obvious way to do it using
a loop (and performance isn't important enough to use memcpy, of course).

I'll apply the new patch once we've confirmed that bug#78824 was the
zero-byte memcpy issue, if that's okay?

Pip