GNU bug report logs - #76559
31.0.50; [-O3 + PGTK] Crash when 'copying as kill'/'killing word'

Previous Next

Full log

View this message in rfc822 format

From: Po Lu <luangruo <at> yahoo.com>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: Michael Albinus <michael.albinus <at> gmx.de>, Iurie Marian <marian.iurie <at> gmail.com>, 76559 <at> debbugs.gnu.org
Subject: bug#76559: 31.0.50; [-O3 + PGTK] Crash when 'copying as kill'/'killing word'
Date: Sat, 01 Mar 2025 08:08:29 +0800

Pip Cet <pipcet <at> protonmail.com> writes:

> My understanding is it is safe to access an entire union at once, as
> the old code did, and that GCC concluded based on the undefined
> behavior elsewhere that the only possible union members were those
> with the holes in the right place.

If a union is accessed from a pointer to a struct, the two types are
incompatible and the result may be a trap representation.  AFAIK.  By
contrast, it is perfectly alright for a union pointer to alias one of
its members, according to 6.5 Expressions: "an aggregate or union type
that includes one of the aforementioned types among its members
(including, recursively, a member of a subaggregate or contained
union)".

>> I propose this patch:
>>
>> diff --git a/src/keyboard.c b/src/keyboard.c
>> index b22814d702d..0c83880c491 100644
>> --- a/src/keyboard.c
>> +++ b/src/keyboard.c
>> @@ -3807,7 +3807,13 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
>>    union buffered_input_event *next_slot = next_kbd_event (kbd_store_ptr);
>>    if (kbd_fetch_ptr != next_slot)
>>      {
>> -      *kbd_store_ptr = *event;
>> +#if defined HAVE_X11 || defined HAVE_PGTK
>> +      if (event->kind == SELECTION_REQUEST_EVENT
>> +	  || event->kind == SELECTION_CLEAR_EVENT)
>> +	kbd_store_ptr->sie = event->sie;
>> +      else
>> +	kbd_store_ptr->ie = event->ie;
>> +#endif /* HAVE_X11 || defined HAVE_PGTK */
>>        kbd_store_ptr = next_slot;
>>  #ifdef subprocesses
>>        if (kbd_buffer_nr_stored () > KBD_BUFFER_SIZE / 2
>
> While the patch does happen to invoke more undefined behavior in a way
> which prevents this particular bug, I don't think we should apply it.
> The old code in this specific place was perfectly okay (and the new
> code contains a bug if neither HAVE_X11 nor HAVE_PGTK is defined).
>
> I still think the patch I posted, which avoids the problem by removing
> what appears to be undefined behavior, is a better idea; but I also
> still don't understand how undefined behavior in one code path infects
> other code paths that don't go through the function.

But I don't believe there's any undefined behavior beyond reading a
union from a pointer to a struct object.  It is definitely sanctioned
for a union to alias one of its members.

Previous Next

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