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

From: Paul Eggert <[email-masked]>
Date: Fri, 28 Feb 2025 10:34:07 -0800

On 2025-02-27 16:12, Po Lu wrote:
>>      kbd_buffer_store_buffered_event ((union buffered_input_event *) event,
>> 				     hold_quit);
>>    }
>>
>> That cast is invalid and the resulting code need not work as one might
>> expect with a circa 1978 C compiler.
> The cast is valid.  It is accessing a union buffered_input_event from
> the resultant pointer that is not.
> 
> "... 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), or ..."

True, the later accessing is undefined as per C23 §6.5.1 ¶7. But the 
patch you proposed still does that sort of accessing, as the code still 
accesses event->kind when event is a union pointer not a struct pointer. 
(Also, the patched code doesn't copy the struct input_event on platforms 
where ! (HAVE_X11 || HAVE_PGTK); I assume that wasn't intended.)

One littler thing. The pointer cast (even without any further access) is 
undefined as per C23 §6.3.2.3 ¶7 if the original pointer is not 
correctly aligned for the cast's type. Although we have a static_assert 
that the alignment is OK, we don't need that assumption or its 
static_assert if we fix the more-important accessing problem.

So I propose the attached patch instead, to fix the above.

From bb52b06795f33346ff330a6a4871df5b0b34de4e Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Fri, 28 Feb 2025 10:29:18 -0800
Subject: [PATCH] Fix union pointer aliasing when storing events

This fixes Bug#76559.
* src/keyboard.c (kbd_buffer_store_buffered_event): Accept enum
event_kind and void pointer instead of a single union pointer.
All callers changed.
(is_ignored_event_kind): Accept enum event_kind, not a pointer to
the event.  All callers changed.
* src/keyboard.c (kbd_buffer_store_buffered_event):
* src/keyboard.h (kbd_buffer_store_event_hold):
Make the pointer a pointer-to-const while we're at it.
* src/keyboard.h (kbd_buffer_store_event_hold):
We no longer need the static_assert or the cast.
* src/pgtkterm.c (pgtk_selection_event): Fix a closely related bug
where we initialized the wrong union member.
---
 src/androidterm.c |  4 ++--
 src/gtkutil.c     |  3 ++-
 src/keyboard.c    | 54 +++++++++++++++++++++++++++++++----------------
 src/keyboard.h    |  9 +++-----
 src/pgtkterm.c    |  4 ++--
 src/xterm.c       |  2 +-
 6 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/src/androidterm.c b/src/androidterm.c
index 3cb9eab3606..e2bc6db16ac 100644
--- a/src/androidterm.c
+++ b/src/androidterm.c
@@ -1084,7 +1084,7 @@ handle_one_android_event (struct android_display_info *dpyinfo,
 		  inev.ie.code = 66;
 		}
 
-	      kbd_buffer_store_buffered_event (&inev, hold_quit);
+	      kbd_buffer_store_buffered_event (inev.ie.kind, &inev, hold_quit);
 	    }
 
 	  count += nchars;
@@ -1817,7 +1817,7 @@ handle_one_android_event (struct android_display_info *dpyinfo,
  OTHER:
   if (inev.ie.kind != NO_EVENT)
     {
-      kbd_buffer_store_buffered_event (&inev, hold_quit);
+      kbd_buffer_store_buffered_event (inev.ie.kind, &inev, hold_quit);
       count++;
     }
 
diff --git a/src/gtkutil.c b/src/gtkutil.c
index 0770874eb40..f6a215e0e90 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -6547,7 +6547,8 @@ xg_widget_key_press_event_cb (GtkWidget *widget, GdkEvent *event,
   if (inev.ie.kind != NO_EVENT)
     {
       xg_pending_quit_event.kind = NO_EVENT;
-      kbd_buffer_store_buffered_event (&inev, &xg_pending_quit_event);
+      kbd_buffer_store_buffered_event (inev.ie.kind, &inev,
+				       &xg_pending_quit_event);
     }
 
   XNoOp (FRAME_X_DISPLAY (f));
diff --git a/src/keyboard.c b/src/keyboard.c
index b22814d702d..77e557e852f 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -408,7 +408,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES	(1 << 2)
 static void deliver_user_signal (int);
 static char *find_user_signal_name (int);
 static void store_user_signal_events (void);
-static bool is_ignored_event (union buffered_input_event *);
+static bool is_ignored_event_kind (enum event_kind);
 
 /* Advance or retreat a buffered input event pointer.  */
 
@@ -3629,7 +3629,7 @@ readable_events (int flags)
 		      && (event->kind == FOCUS_IN_EVENT
 			  || event->kind == FOCUS_OUT_EVENT))
 		     || (input_pending_p_filter_events
-			 && is_ignored_event (event))))
+			 && is_ignored_event_kind (event->kind))))
 #ifdef USE_TOOLKIT_SCROLL_BARS
 		  && !((flags & READABLE_EVENTS_IGNORE_SQUEEZABLES)
 		       && (event->kind == SCROLL_BAR_CLICK_EVENT
@@ -3714,7 +3714,11 @@ kbd_buffer_store_event (register struct input_event *event)
   kbd_buffer_store_event_hold (event, 0);
 }
 
-/* Store EVENT obtained at interrupt level into kbd_buffer, fifo.
+/* Store a kind-KIND EVENT obtained at interrupt level into kbd_buffer, fifo.
+   EVENT is either a struct input event *, or a struct
+   selection_input_event *, or a union buffered_input_event *; it is not
+   a pointer to the union as dereferencing such a pointer, even to get
+   just its 'kind' member, would violate C23 section 6.5.1 para 7.
 
    If HOLD_QUIT is 0, just stuff EVENT into the fifo.
    Else, if HOLD_QUIT.kind != NO_EVENT, discard EVENT.
@@ -3725,34 +3729,35 @@ kbd_buffer_store_event (register struct input_event *event)
    subsequent input events have been parsed (and discarded).  */
 
 void
-kbd_buffer_store_buffered_event (union buffered_input_event *event,
+kbd_buffer_store_buffered_event (enum event_kind kind, void const *event,
 				 struct input_event *hold_quit)
 {
-  if (event->kind == NO_EVENT)
+  if (kind == NO_EVENT)
     emacs_abort ();
 
   if (hold_quit && hold_quit->kind != NO_EVENT)
     return;
 
-  if (event->kind == ASCII_KEYSTROKE_EVENT)
+  if (kind == ASCII_KEYSTROKE_EVENT)
     {
-      int c = event->ie.code & 0377;
+      struct input_event const *inev = event;
+      int c = inev->code & 0377;
 
-      if (event->ie.modifiers & ctrl_modifier)
+      if (inev->modifiers & ctrl_modifier)
 	c = make_ctrl_char (c);
 
-      c |= (event->ie.modifiers
+      c |= (inev->modifiers
 	    & (meta_modifier | alt_modifier
 	       | hyper_modifier | super_modifier));
 
       if (c == quit_char)
 	{
-	  KBOARD *kb = FRAME_KBOARD (XFRAME (event->ie.frame_or_window));
+	  KBOARD *kb = FRAME_KBOARD (XFRAME (inev->frame_or_window));
 
 	  if (single_kboard && kb != current_kboard)
 	    {
 	      kset_kbd_queue
-		(kb, list2 (make_lispy_switch_frame (event->ie.frame_or_window),
+		(kb, list2 (make_lispy_switch_frame (inev->frame_or_window),
 			    make_fixnum (c)));
 	      kb->kbd_queue_has_data = true;
 
@@ -3771,7 +3776,7 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 
 	  if (hold_quit)
 	    {
-	      *hold_quit = event->ie;
+	      *hold_quit = *inev;
 	      return;
 	    }
 
@@ -3782,9 +3787,9 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 	  {
 	    Lisp_Object focus;
 
-	    focus = FRAME_FOCUS_FRAME (XFRAME (event->ie.frame_or_window));
+	    focus = FRAME_FOCUS_FRAME (XFRAME (inev->frame_or_window));
 	    if (NILP (focus))
-	      focus = event->ie.frame_or_window;
+	      focus = inev->frame_or_window;
 	    internal_last_event_frame = focus;
 	    Vlast_event_frame = focus;
 	  }
@@ -3807,7 +3812,20 @@ 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;
+      switch (kind)
+	{
+#if defined HAVE_X11 || defined HAVE_PGTK
+	case SELECTION_REQUEST_EVENT: case SELECTION_CLEAR_EVENT:
+	  struct selection_input_event const *sie = event;
+	  kbd_store_ptr->sie = *sie;
+	  break;
+#endif
+	default:
+	  struct input_event const *ie = event;
+	  kbd_store_ptr->ie = *ie;
+	  break;
+	}
+
       kbd_store_ptr = next_slot;
 #ifdef subprocesses
       if (kbd_buffer_nr_stored () > KBD_BUFFER_SIZE / 2
@@ -3825,7 +3843,7 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
   /* If we're inside while-no-input, and this event qualifies
      as input, set quit-flag to cause an interrupt.  */
   if (!NILP (Vthrow_on_input)
-      && !is_ignored_event (event))
+      && !is_ignored_event_kind (kind))
     Vquit_flag = Vthrow_on_input;
 }
 
@@ -12877,11 +12895,11 @@ init_while_no_input_ignore_events (void)
 }
 
 static bool
-is_ignored_event (union buffered_input_event *event)
+is_ignored_event_kind (enum event_kind kind)
 {
   Lisp_Object ignore_event;
 
-  switch (event->kind)
+  switch (kind)
     {
     case FOCUS_IN_EVENT: ignore_event = Qfocus_in; break;
     case FOCUS_OUT_EVENT: ignore_event = Qfocus_out; break;
diff --git a/src/keyboard.h b/src/keyboard.h
index 5e04b54eb74..563c1b084bd 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -499,16 +499,13 @@ #define EVENT_HEAD_KIND(event_head) \
 extern void swallow_events (bool);
 extern bool lucid_event_type_list_p (Lisp_Object);
 extern void kbd_buffer_store_event (struct input_event *);
-extern void kbd_buffer_store_buffered_event (union buffered_input_event *,
+extern void kbd_buffer_store_buffered_event (enum event_kind, void const *,
 					     struct input_event *);
 INLINE void
-kbd_buffer_store_event_hold (struct input_event *event,
+kbd_buffer_store_event_hold (struct input_event const *event,
 			     struct input_event *hold_quit)
 {
-  static_assert (alignof (struct input_event) == alignof (union buffered_input_event)
-		 && sizeof (struct input_event) == sizeof (union buffered_input_event));
-  kbd_buffer_store_buffered_event ((union buffered_input_event *) event,
-				   hold_quit);
+  kbd_buffer_store_buffered_event (event->kind, event, hold_quit);
 }
 extern void poll_for_input_1 (void);
 extern void show_help_echo (Lisp_Object, Lisp_Object, Lisp_Object,
diff --git a/src/pgtkterm.c b/src/pgtkterm.c
index 00377ff73a0..f89bd7e68a0 100644
--- a/src/pgtkterm.c
+++ b/src/pgtkterm.c
@@ -347,7 +347,7 @@ evq_flush (struct input_event *hold_quit)
 	evq->q[i - 1] = evq->q[i];
       evq->nr--;
 
-      kbd_buffer_store_buffered_event (&ev, hold_quit);
+      kbd_buffer_store_buffered_event (ev.kind, &ev, hold_quit);
       n++;
     }
 
@@ -6939,7 +6939,7 @@ pgtk_selection_event (GtkWidget *widget, GdkEvent *event,
 
       if (f)
 	{
-	  EVENT_INIT (inev.ie);
+	  memset (&inev.sie, 0, sizeof inev.sie);
 
 	  inev.sie.kind = (event->type == GDK_SELECTION_CLEAR
 			   ? SELECTION_CLEAR_EVENT
diff --git a/src/xterm.c b/src/xterm.c
index 2ccf267bbd3..b4ee037c8be 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -25593,7 +25593,7 @@ handle_one_xevent (struct x_display_info *dpyinfo,
  done:
   if (inev.ie.kind != NO_EVENT)
     {
-      kbd_buffer_store_buffered_event (&inev, hold_quit);
+      kbd_buffer_store_buffered_event (inev.ie.kind, &inev, hold_quit);
       count++;
     }
 
-- 
2.45.2