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


Message #14 received at 78812 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 78812 <at> debbugs.gnu.org
Subject: Re: 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 21 days ago.

Previous Next


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