GNU bug report logs - #78812
feature/igc: [PATCH] Make lface_id_to_name an exact root

Previous Next

Package: emacs;

Reported by: Helmut Eller <eller.helmut <at> gmail.com>

Date: Tue, 17 Jun 2025 07:57:01 UTC

Severity: normal

Tags: patch

Full log


View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 78812 <at> debbugs.gnu.org
Subject: bug#78812: feature/igc: [PATCH] Make lface_id_to_name an exact root
Date: Wed, 18 Jun 2025 12:27:01 +0000
"Helmut Eller" <eller.helmut <at> gmail.com> writes:

> On Tue, Jun 17 2025, Pip Cet wrote:
>
>> "Helmut Eller" <eller.helmut <at> gmail.com> 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





This bug report was last modified 2 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.