GNU bug report logs -
#76559
31.0.50; [-O3 + PGTK] Crash when 'copying as kill'/'killing word'
Previous Next
To reply to this bug, email your comments to 76559 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Tue, 25 Feb 2025 17:34:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Iurie Marian <marian.iurie <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Tue, 25 Feb 2025 17:34:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Dear Maintainers,
Emacs is crashing while copying-as-kill/killing-word, only when compiled
with -O3 optimizations. I am using PGTK (see the configure command
below). I did a git bisect, and it seems that the commit below is
triggering the bug. To reproduce it just compile with -O3 and PGTK, then
simply hit M-<backspace>.
At the bottom you will also see the the stack trace, maybe it's useful.
Kind Regards,
Iurie
======================================================================
d41178368eb73873f34c15b58062a7447802c914 is the first bad commit
commit d41178368eb73873f34c15b58062a7447802c914
Author: Michael Albinus <michael.albinus <at> gmx.de>
Date: Fri Feb 7 12:04:05 2025 +0100
New function insert-special-event and special event sleep-event
* doc/lispref/commands.texi (Misc Events): Add sleep-event.
(Special Events): New function insert-special-event.
* etc/NEWS: New function insert-special-event. New event 'sleep-event'.
Fix typos.
* src/keyboard.c (Finsert_special_event): New defun.
(syms_of_keyboard): Declare Qsleep_event. Define subroutine
Sinsert_special_event. Add sleep-event to Vspecial_event_map.
(Bug#63620)
(kbd_buffer_get_event, make_lispy_event)
(init_while_no_input_ignore_events, is_ignored_event):
* src/termhooks.h (event_kind): Add SLEEP_EVENT.
doc/lispref/commands.texi | 32 ++++++++++++++++++++++
etc/NEWS | 19 +++++++++++---
src/keyboard.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++
src/termhooks.h | 3 +++
4 files changed, 117 insertions(+), 4 deletions(-)
======================================================================
In GNU Emacs 31.0.50 (build 35, x86_64-pc-linux-gnu, GTK+ Version
3.24.38, cairo version 1.16.0) of 2025-02-25 built on
unit_xpchipbarcodereader
Repository revision: d41178368eb73873f34c15b58062a7447802c914
Repository branch: HEAD
System Description: Debian GNU/Linux 12 (bookworm)
Configured using:
'configure 'CFLAGS= -g -Wall -Wextra -pedantic -O3'
--prefix=/home/roche/.local/emacs/build_ --sysconfdir=/etc
--libexecdir=/home/roche/.local/emacs/build_/usr/lib
--localstatedir=/home/roche/.local/emacs/build_/var --with-modules
--without-gconf --without-gsettings --enable-link-time-optimization
--with-x-toolkit=yes --without-xaw3d --without-m17n-flt --with-cairo
--without-compress-install --with-native-compilation=aot
--with-mailutils --with-xft --with-rsvg --with-pgtk'
Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS HARFBUZZ JPEG LIBOTF LIBSELINUX
LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PGTK PNG RSVG SECCOMP
SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM GTK3
ZLIB
Important settings:
value of $LC_ALL: C
locale-coding-system: nil
Major mode: Lisp Interaction
Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
show-paren-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
minibuffer-regexp-mode: t
line-number-mode: t
indent-tabs-mode: t
transient-mark-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr compile comint ansi-osc ansi-color ring comp-run
bytecomp byte-compile comp-common rx emacsbug message mailcap yank-media
puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/pgtk-win pgtk-win term/common-win touch-screen pgtk-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting font-render-setting cairo gtk pgtk multi-tty
move-toolbar make-network-process tty-child-frames native-compile emacs)
======================================================================
(gdb) bt
#0 pgtk_handle_selection_request (event=0x7fffffffd810) at
/home/rce/.local/emacs/sources/src/pgtkselect.c:571
#1 0x000055555569edd5 in pgtk_handle_selection_event
(event=0x7fffffffd810) at
/home/rce/.local/emacs/sources/src/pgtkselect.c:782
#2 process_special_events () at
/home/rce/.local/emacs/sources/src/keyboard.c:4570
#3 0x00005555555b5424 in swallow_events (do_display=true) at
/home/rce/.local/emacs/sources/src/keyboard.c:4610
#4 sit_for (timeout=0x7a, reading=<optimized out>, display_option=1)
at /home/rce/.local/emacs/sources/src/dispnew.c:6867
#5 0x00005555556a7c34 in read_char (commandflag=1,
map=0x7fffef23de23, prev_event=0x0, used_mouse_menu=0x7fffffffdfbb,
end_time=0x0) at /home/rce/.local/emacs/sources/src/lisp.h:704
#6 0x000055555583fa7b in read_key_sequence.constprop.0
(keybuf=<optimized out>, prevent_redisplay=false,
disable_text_conversion_p=false, fix_current_buffer=true,
can_return_switch_frame=true, dont_downcase_last=false, prompt=0x0) at
/home/rce/.local/emacs/sources/src/keyboard.c:10750
#7 0x000055555569ba51 in command_loop_1 () at
/home/rce/.local/emacs/sources/src/keyboard.c:1424
#8 0x000055555573d9f7 in internal_condition_case (bfun=0x55555569b8b0
<command_loop_1>, handlers=<optimized out>, hfun=0x555555699790
<cmd_error>) at /home/rce/.local/emacs/sources/src/eval.c:1602
#9 0x0000555555699a46 in command_loop_2
(handlers=handlers <at> entry=0x90) at
/home/rce/.local/emacs/sources/src/keyboard.c:1163
#10 0x000055555573d94c in internal_catch (tag=<optimized out>,
func=0x555555699a20 <command_loop_2>, arg=0x90) at
/home/rce/.local/emacs/sources/src/eval.c:1282
#11 0x000055555569b451 in command_loop () at
/home/rce/.local/emacs/sources/src/keyboard.c:1141
#12 0x000055555586e628 in recursive_edit_1.isra.0 () at
/home/rce/.local/emacs/sources/src/keyboard.c:749
#13 0x000055555569b88d in Frecursive_edit () at
/home/rce/.local/emacs/sources/src/keyboard.c:832
#14 0x00005555555a8c94 in main (argc=2, argv=0x7fffffffe668) at
/home/rce/.local/emacs/sources/src/emacs.c:2558
(gdb) list
566 pushed = false;
567
568 if (!dpyinfo)
569 goto DONE;
570
571 local_selection_data = LOCAL_SELECTION (selection_symbol, dpyinfo);
572
573 /* Decline if we don't own any selections. */
574 if (NILP (local_selection_data)) goto DONE;
575
(gdb) fr 2
#2 process_special_events () at
/home/rce/.local/emacs/sources/src/keyboard.c:4570
4570 pgtk_handle_selection_event (©);
(gdb) list
4565 input_pending = readable_events (0);
4566
4567 #ifdef HAVE_X11
4568 x_handle_selection_event (©);
4569 #else
4570 pgtk_handle_selection_event (©);
4571 #endif
4572 #elif defined HAVE_HAIKU
4573 if (event->ie.kind != SELECTION_CLEAR_EVENT)
4574 emacs_abort ();
(gdb) info locals
event = 0x555555957a40 <kbd_buffer+384>
copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55cf0b00,
requestor = 0x555555f97a80, selection = 0x45, target = 0x4d, property
= 0x5e, time = 0}
moved_events = <optimized out>
(gdb) x/s 0x55cf0b00
0x55cf0b00: <error: Cannot access memory at address 0x55cf0b00>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 13:11:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> Cc: Michael Albinus <michael.albinus <at> gmx.de>
> From: Iurie Marian <marian.iurie <at> gmail.com>
> Date: Tue, 25 Feb 2025 18:32:39 +0100
>
> Dear Maintainers,
>
> Emacs is crashing while copying-as-kill/killing-word, only when compiled
> with -O3 optimizations. I am using PGTK (see the configure command
> below). I did a git bisect, and it seems that the commit below is
> triggering the bug. To reproduce it just compile with -O3 and PGTK, then
> simply hit M-<backspace>.
>
> At the bottom you will also see the the stack trace, maybe it's useful.
Is this still in GDB, and you can answer some questions about it? Or,
if not, then reproduce the crash and answer the questions then?
> (gdb) info locals
> event = 0x555555957a40 <kbd_buffer+384>
> copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55cf0b00,
> requestor = 0x555555f97a80, selection = 0x45, target = 0x4d, property
> = 0x5e, time = 0}
> moved_events = <optimized out>
>
> (gdb) x/s 0x55cf0b00
> 0x55cf0b00: <error: Cannot access memory at address 0x55cf0b00>
What does this produce:
(gdb) p *event
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 15:22:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> What does this produce:
>
> (gdb) p *event
(gdb) p *event
$1 = {kind = SELECTION_REQUEST_EVENT, ie = {kind =
SELECTION_REQUEST_EVENT, part = scroll_bar_nowhere, code = 0,
modifiers = 1439199584, x = 0x555555f95a80, y = 0x45, timestamp = 77,
frame_or_window = 0x5e, arg = 0x0, device = 0x30}, sie = {kind =
SELECTION_REQUEST_EVENT, dpyinfo = 0x55c87160, requestor =
0x555555f95a80, selection = 0x45, target = 0x4d, property = 0x5e, time
= 0}}
On Wed, 26 Feb 2025 at 14:10, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > Cc: Michael Albinus <michael.albinus <at> gmx.de>
> > From: Iurie Marian <marian.iurie <at> gmail.com>
> > Date: Tue, 25 Feb 2025 18:32:39 +0100
> >
> > Dear Maintainers,
> >
> > Emacs is crashing while copying-as-kill/killing-word, only when compiled
> > with -O3 optimizations. I am using PGTK (see the configure command
> > below). I did a git bisect, and it seems that the commit below is
> > triggering the bug. To reproduce it just compile with -O3 and PGTK, then
> > simply hit M-<backspace>.
> >
> > At the bottom you will also see the the stack trace, maybe it's useful.
>
> Is this still in GDB, and you can answer some questions about it? Or,
> if not, then reproduce the crash and answer the questions then?
>
> > (gdb) info locals
> > event = 0x555555957a40 <kbd_buffer+384>
> > copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55cf0b00,
> > requestor = 0x555555f97a80, selection = 0x45, target = 0x4d, property
> > = 0x5e, time = 0}
> > moved_events = <optimized out>
> >
> > (gdb) x/s 0x55cf0b00
> > 0x55cf0b00: <error: Cannot access memory at address 0x55cf0b00>
>
> What does this produce:
>
> (gdb) p *event
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 15:49:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> From: Iurie Marian <iura.mail <at> gmail.com>
> Date: Wed, 26 Feb 2025 14:43:20 +0100
> Cc: Iurie Marian <marian.iurie <at> gmail.com>, 76559 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
>
> > What does this produce:
> >
> > (gdb) p *event
>
> (gdb) p *event
> $1 = {kind = SELECTION_REQUEST_EVENT, ie = {kind =
> SELECTION_REQUEST_EVENT, part = scroll_bar_nowhere, code = 0,
> modifiers = 1439199584, x = 0x555555f95a80, y = 0x45, timestamp = 77,
> frame_or_window = 0x5e, arg = 0x0, device = 0x30}, sie = {kind =
> SELECTION_REQUEST_EVENT, dpyinfo = 0x55c87160, requestor =
> 0x555555f95a80, selection = 0x45, target = 0x4d, property = 0x5e, time
> = 0}}
And this:
(gdb) p event->dpyinfo->terminal
(gdb) p event->dpyinfo->terminal->Vselection_list
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 16:04:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 76559 <at> debbugs.gnu.org (full text, mbox):
`dpyinfo` is actually invalid...
(gdb) p *event
$3 = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c83670, requestor
= 0x555556035a80, selection = 0x45, target = 0x4d, property = 0x60,
time = 0}
(gdb) p *event->dpyinfo
Cannot access memory at address 0x55c83670
(gdb) x/s 0x55c83670
0x55c83670: <error: Cannot access memory at address 0x55c83670>
(gdb) p event->dpyinfo->terminal
Cannot access memory at address 0x55c83678
On Wed, 26 Feb 2025 at 16:48, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Iurie Marian <iura.mail <at> gmail.com>
> > Date: Wed, 26 Feb 2025 14:43:20 +0100
> > Cc: Iurie Marian <marian.iurie <at> gmail.com>, 76559 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
> >
> > > What does this produce:
> > >
> > > (gdb) p *event
> >
> > (gdb) p *event
> > $1 = {kind = SELECTION_REQUEST_EVENT, ie = {kind =
> > SELECTION_REQUEST_EVENT, part = scroll_bar_nowhere, code = 0,
> > modifiers = 1439199584, x = 0x555555f95a80, y = 0x45, timestamp = 77,
> > frame_or_window = 0x5e, arg = 0x0, device = 0x30}, sie = {kind =
> > SELECTION_REQUEST_EVENT, dpyinfo = 0x55c87160, requestor =
> > 0x555555f95a80, selection = 0x45, target = 0x4d, property = 0x5e, time
> > = 0}}
>
> And this:
>
> (gdb) p event->dpyinfo->terminal
> (gdb) p event->dpyinfo->terminal->Vselection_list
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 16:15:03 GMT)
Full text and
rfc822 format available.
Message #20 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> What does this produce:
>
> (gdb) p *event
(gdb) p *event
$1 = {kind = SELECTION_REQUEST_EVENT, ie = {kind =
SELECTION_REQUEST_EVENT, part = scroll_bar_nowhere, code = 0,
modifiers = 1439199584, x = 0x555555f95a80, y = 0x45, timestamp = 77,
frame_or_window = 0x5e, arg = 0x0, device = 0x30}, sie = {kind =
SELECTION_REQUEST_EVENT, dpyinfo = 0x55c87160, requestor =
0x555555f95a80, selection = 0x45, target = 0x4d, property = 0x5e, time
= 0}}
On Wed, 26 Feb 2025 at 14:10, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > Cc: Michael Albinus <michael.albinus <at> gmx.de>
> > From: Iurie Marian <marian.iurie <at> gmail.com>
> > Date: Tue, 25 Feb 2025 18:32:39 +0100
> >
> > Dear Maintainers,
> >
> > Emacs is crashing while copying-as-kill/killing-word, only when compiled
> > with -O3 optimizations. I am using PGTK (see the configure command
> > below). I did a git bisect, and it seems that the commit below is
> > triggering the bug. To reproduce it just compile with -O3 and PGTK, then
> > simply hit M-<backspace>.
> >
> > At the bottom you will also see the the stack trace, maybe it's useful.
>
> Is this still in GDB, and you can answer some questions about it? Or,
> if not, then reproduce the crash and answer the questions then?
>
> > (gdb) info locals
> > event = 0x555555957a40 <kbd_buffer+384>
> > copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55cf0b00,
> > requestor = 0x555555f97a80, selection = 0x45, target = 0x4d, property
> > = 0x5e, time = 0}
> > moved_events = <optimized out>
> >
> > (gdb) x/s 0x55cf0b00
> > 0x55cf0b00: <error: Cannot access memory at address 0x55cf0b00>
>
> What does this produce:
>
> (gdb) p *event
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 16:36:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> From: Iurie Marian <marian.iurie <at> gmail.com>
> Date: Wed, 26 Feb 2025 17:03:29 +0100
> Cc: Iurie Marian <iura.mail <at> gmail.com>, 76559 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
>
> `dpyinfo` is actually invalid...
>
> (gdb) p *event
> $3 = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c83670, requestor
> = 0x555556035a80, selection = 0x45, target = 0x4d, property = 0x60,
> time = 0}
>
> (gdb) p *event->dpyinfo
> Cannot access memory at address 0x55c83670
>
> (gdb) x/s 0x55c83670
> 0x55c83670: <error: Cannot access memory at address 0x55c83670>
>
> (gdb) p event->dpyinfo->terminal
> Cannot access memory at address 0x55c83678
So dpyinfo got freed somehow?
Po Lu, any suggestions or ideas?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 16:39:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 76559 <at> debbugs.gnu.org (full text, mbox):
btw, just by commenting the line src/keyboard.c:11697, it is not
crashing anymore; maybe this could be a hint.
https://git.savannah.gnu.org/cgit/emacs.git/tree/src/keyboard.c?h=master#n11697
/* Store it into the input event queue. */
/* kbd_buffer_store_event (&ie); */
On Wed, 26 Feb 2025 at 17:03, Iurie Marian <marian.iurie <at> gmail.com> wrote:
>
> `dpyinfo` is actually invalid...
>
> (gdb) p *event
> $3 = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c83670, requestor
> = 0x555556035a80, selection = 0x45, target = 0x4d, property = 0x60,
> time = 0}
>
> (gdb) p *event->dpyinfo
> Cannot access memory at address 0x55c83670
>
> (gdb) x/s 0x55c83670
> 0x55c83670: <error: Cannot access memory at address 0x55c83670>
>
> (gdb) p event->dpyinfo->terminal
> Cannot access memory at address 0x55c83678
>
> On Wed, 26 Feb 2025 at 16:48, Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > > From: Iurie Marian <iura.mail <at> gmail.com>
> > > Date: Wed, 26 Feb 2025 14:43:20 +0100
> > > Cc: Iurie Marian <marian.iurie <at> gmail.com>, 76559 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
> > >
> > > > What does this produce:
> > > >
> > > > (gdb) p *event
> > >
> > > (gdb) p *event
> > > $1 = {kind = SELECTION_REQUEST_EVENT, ie = {kind =
> > > SELECTION_REQUEST_EVENT, part = scroll_bar_nowhere, code = 0,
> > > modifiers = 1439199584, x = 0x555555f95a80, y = 0x45, timestamp = 77,
> > > frame_or_window = 0x5e, arg = 0x0, device = 0x30}, sie = {kind =
> > > SELECTION_REQUEST_EVENT, dpyinfo = 0x55c87160, requestor =
> > > 0x555555f95a80, selection = 0x45, target = 0x4d, property = 0x5e, time
> > > = 0}}
> >
> > And this:
> >
> > (gdb) p event->dpyinfo->terminal
> > (gdb) p event->dpyinfo->terminal->Vselection_list
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 17:10:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Iurie Marian" <marian.iurie <at> gmail.com> writes:
> Dear Maintainers,
>
> Emacs is crashing while copying-as-kill/killing-word, only when compiled
> with -O3 optimizations. I am using PGTK (see the configure command
> below). I did a git bisect, and it seems that the commit below is
> triggering the bug. To reproduce it just compile with -O3 and PGTK, then
> simply hit M-<backspace>.
Just for completeness, you're using lto as well as -O3, which seems more
likely to trigger this bug than Michael's change below.
And I can't reproduce it here.
Which precise compiler are you using? gcc --version would be very
helpful here.
> (gdb) list
> 566 pushed = false;
> 567
> 568 if (!dpyinfo)
> 569 goto DONE;
> 570
> 571 local_selection_data = LOCAL_SELECTION (selection_symbol, dpyinfo);
> 572
> 573 /* Decline if we don't own any selections. */
> 574 if (NILP (local_selection_data)) goto DONE;
> 575
>
> (gdb) fr 2
> #2 process_special_events () at
> /home/rce/.local/emacs/sources/src/keyboard.c:4570
> 4570 pgtk_handle_selection_event (©);
> (gdb) list
> 4565 input_pending = readable_events (0);
> 4566
> 4567 #ifdef HAVE_X11
> 4568 x_handle_selection_event (©);
> 4569 #else
> 4570 pgtk_handle_selection_event (©);
> 4571 #endif
> 4572 #elif defined HAVE_HAIKU
> 4573 if (event->ie.kind != SELECTION_CLEAR_EVENT)
> 4574 emacs_abort ();
> (gdb) info locals
> event = 0x555555957a40 <kbd_buffer+384>
> copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55cf0b00,
dypinfo looks like it was truncated to 32 bits. Can you check that
0x555555cf0b00 is a valid dpyinfo structure?
The definition in question is:
struct selection_input_event
{
ENUM_BF (event_kind) kind : EVENT_KIND_WIDTH;
struct pgtk_display_info *dpyinfo;
/* We spell it with an "o" here because X does. */
GdkWindow *requestor;
GdkAtom selection, target, property;
guint32 time;
};
Can you run "ptype/o struct selection_input_event" and "ptype/o struct
input_event" to see what might be going on? "ptype/o union
buffered_input_event" would also be interesting, to see whether our
union trick might no longer be working.
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 17:26:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Yes, it looks like Michael's changes have nothing to do with this bug,
but these seem just to reveal some undefined behavior... idk. Btw,
just by commenting the line src/keyboard.c:11697, it is not crashing
anymore; maybe this could be a hint.
> gcc --version
gcc (Debian 12.2.0-14) 12.2.0
> Can you check that 0x555555cf0b00 is a valid dpyinfo structure?
(gdb) info locals
event = 0x555555953aa0 <kbd_buffer+384>
copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c82260, requestor =
0x555555f93a80, selection = 0x45, target = 0x4d, property = 0x5e, time = 0}
moved_events = <optimized out>
(gdb) x 0x555555c82260
0x555555c82260: 0x00
> Can you run "ptype/o struct selection_input_event" [...]
(gdb) ptype/o struct selection_input_event
/* offset | size */ type = struct selection_input_event {
/* 0: 0 | 4 */ enum event_kind kind : 16;
/* XXX 6-byte hole */
/* 8 | 8 */ struct pgtk_display_info *dpyinfo;
/* 16 | 8 */ GdkWindow *requestor;
/* 24 | 8 */ GdkAtom selection;
/* 32 | 8 */ GdkAtom target;
/* 40 | 8 */ GdkAtom property;
/* 48 | 4 */ guint32 time;
/* XXX 4-byte padding */
/* total size (bytes): 56 */
}
(gdb) ptype/o struct input_event
/* offset | size */ type = struct input_event {
/* 0: 0 | 4 */ enum event_kind kind : 16;
/* 2: 0 | 4 */ enum scroll_bar_part part : 16;
/* 4 | 4 */ unsigned int code;
/* 8 | 4 */ unsigned int modifiers;
/* XXX 4-byte hole */
/* 16 | 8 */ Lisp_Object x;
/* 24 | 8 */ Lisp_Object y;
/* 32 | 8 */ Time timestamp;
/* 40 | 8 */ Lisp_Object frame_or_window;
/* 48 | 8 */ Lisp_Object arg;
/* 56 | 8 */ Lisp_Object device;
/* total size (bytes): 64 */
}
(gdb) ptype/o union buffered_input_event
/* offset | size */ type = union buffered_input_event {
/* 4 */ enum event_kind kind : 16;
/* 64 */ struct input_event {
/* 0: 0 | 4 */ enum event_kind kind : 16;
/* 2: 0 | 4 */ enum scroll_bar_part part : 16;
/* 4 | 4 */ unsigned int code;
/* 8 | 4 */ unsigned int modifiers;
/* XXX 4-byte hole */
/* 16 | 8 */ Lisp_Object x;
/* 24 | 8 */ Lisp_Object y;
/* 32 | 8 */ Time timestamp;
/* 40 | 8 */ Lisp_Object frame_or_window;
/* 48 | 8 */ Lisp_Object arg;
/* 56 | 8 */ Lisp_Object device;
/* total size (bytes): 64 */
} ie;
/* 56 */ struct selection_input_event {
/* 0: 0 | 4 */ enum event_kind kind : 16;
/* XXX 6-byte hole */
/* 8 | 8 */ struct pgtk_display_info *dpyinfo;
/* 16 | 8 */ GdkWindow *requestor;
/* 24 | 8 */ GdkAtom selection;
/* 32 | 8 */ GdkAtom target;
--Type <RET> for more, q to quit, c to continue without paging--
/* 40 | 8 */ GdkAtom property;
/* 48 | 4 */ guint32 time;
/* XXX 4-byte padding */
/* total size (bytes): 56 */
} sie;
/* total size (bytes): 64 */
}
On Wed, 26 Feb 2025 at 18:08, Pip Cet <pipcet <at> protonmail.com> wrote:
> "Iurie Marian" <marian.iurie <at> gmail.com> writes:
>
> > Dear Maintainers,
> >
> > Emacs is crashing while copying-as-kill/killing-word, only when compiled
> > with -O3 optimizations. I am using PGTK (see the configure command
> > below). I did a git bisect, and it seems that the commit below is
> > triggering the bug. To reproduce it just compile with -O3 and PGTK, then
> > simply hit M-<backspace>.
>
> Just for completeness, you're using lto as well as -O3, which seems more
> likely to trigger this bug than Michael's change below.
>
> And I can't reproduce it here.
>
> Which precise compiler are you using? gcc --version would be very
> helpful here.
>
> > (gdb) list
> > 566 pushed = false;
> > 567
> > 568 if (!dpyinfo)
> > 569 goto DONE;
> > 570
> > 571 local_selection_data = LOCAL_SELECTION (selection_symbol,
> dpyinfo);
> > 572
> > 573 /* Decline if we don't own any selections. */
> > 574 if (NILP (local_selection_data)) goto DONE;
> > 575
> >
> > (gdb) fr 2
> > #2 process_special_events () at
> > /home/rce/.local/emacs/sources/src/keyboard.c:4570
> > 4570 pgtk_handle_selection_event (©);
> > (gdb) list
> > 4565 input_pending = readable_events (0);
> > 4566
> > 4567 #ifdef HAVE_X11
> > 4568 x_handle_selection_event (©);
> > 4569 #else
> > 4570 pgtk_handle_selection_event (©);
> > 4571 #endif
> > 4572 #elif defined HAVE_HAIKU
> > 4573 if (event->ie.kind != SELECTION_CLEAR_EVENT)
> > 4574 emacs_abort ();
> > (gdb) info locals
> > event = 0x555555957a40 <kbd_buffer+384>
> > copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55cf0b00,
>
> dypinfo looks like it was truncated to 32 bits. Can you check that
> 0x555555cf0b00 is a valid dpyinfo structure?
>
> The definition in question is:
>
> struct selection_input_event
> {
> ENUM_BF (event_kind) kind : EVENT_KIND_WIDTH;
> struct pgtk_display_info *dpyinfo;
> /* We spell it with an "o" here because X does. */
> GdkWindow *requestor;
> GdkAtom selection, target, property;
> guint32 time;
> };
>
> Can you run "ptype/o struct selection_input_event" and "ptype/o struct
> input_event" to see what might be going on? "ptype/o union
> buffered_input_event" would also be interesting, to see whether our
> union trick might no longer be working.
>
> Pip
>
>
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 18:49:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Iurie Marian" <marian.iurie <at> gmail.com> writes:
> Yes, it looks like Michael's changes have nothing to do with this bug,
> but these seem just to reveal some undefined behavior... idk. Btw,
> just by commenting the line src/keyboard.c:11697, it is not crashing
> anymore; maybe this could be a hint.
>
>> gcc --version
> gcc (Debian 12.2.0-14) 12.2.0
>
>> Can you check that 0x555555cf0b00 is a valid dpyinfo structure?
> (gdb) info locals
> event = 0x555555953aa0 <kbd_buffer+384>
> copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c82260, requestor = 0x555555f93a80, selection = 0x45, target = 0x4d, property =
> 0x5e, time = 0}
> moved_events = <optimized out>
>
> (gdb) x 0x555555c82260
> 0x555555c82260: 0x00
Well, that only tells us the first byte is 0, which is probably correct.
Can you retry with x/64gx 0x555555c82260 (or the new address) so we see
some more data?
>> Can you run "ptype/o struct selection_input_event" [...]
>
> (gdb) ptype/o struct selection_input_event
> /* offset | size */ type = struct selection_input_event {
> /* 0: 0 | 4 */ enum event_kind kind : 16;
> /* XXX 6-byte hole */
This is strange, but it looks like this may be a C undefined behavior
bug (or, less likely, an actual GCC bug). If the event_kind bitfield is
listed with size 4, shouldn't the hole after it be listed with size 4,
not size 6?
Here's the code obtained by disass/s evq_flush which copies the relevant
part of the header:
3810 *kbd_store_ptr = *event;
0x00000000002f9f4c <+108>: movd %xmm0,(%rdx)
0x00000000002f9f50 <+112>: movdqa 0x20(%rsp),%xmm4
0x00000000002f9f56 <+118>: movdqa 0x10(%rsp),%xmm5
0x00000000002f9f5c <+124>: movq %xmm1,0x4(%rdx)
The first movd (not movq or movdq!) copies four bytes containing the
event_kind. The unaligned movq at +124 copies 8 bytes to bytes 4-11 of
the struct, which copies the low-order 4 bytes of the dpyinfo.
the code continues with:
0x00000000002f9f61 <+129>: mov %rax,0x1094e0(%rip) # 0x403448 <kbd_store_ptr>
0x00000000002f9f68 <+136>: sub %rcx,%rax
0x00000000002f9f6b <+139>: sar $0x6,%rax
0x00000000002f9f6f <+143>: mov %r12,0x20(%rdx)
0x00000000002f9f73 <+147>: mov %rbp,0x38(%rdx)
0x00000000002f9f77 <+151>: movups %xmm4,0x10(%rdx)
0x00000000002f9f7b <+155>: movups %xmm5,0x28(%rdx)
but, as far as I can tell, bytes 12-15 are never touched by this code.
Here's the corresponding code which copies the event structure:
327 union buffered_input_event ev = evq->q[0];
0x00000000002f9fee <+270>: lea 0x19a64b(%rip),%rcx # 0x494640 <event_q.lto_priv.0>
0x00000000002f9ff5 <+277>: mov (%rcx),%rdi
0x00000000002f9ff8 <+280>: movd (%rdi),%xmm0
0x00000000002f9ffc <+284>: movdqu 0x10(%rdi),%xmm2
0x00000000002fa001 <+289>: movdqu 0x28(%rdi),%xmm3
0x00000000002fa006 <+294>: movq 0x4(%rdi),%xmm1
0x00000000002fa00b <+299>: mov 0x20(%rdi),%r12
0x00000000002fa00f <+303>: mov 0x38(%rdi),%rbp
0x00000000002fa013 <+307>: pextrw $0x0,%xmm0,%r13d
0x00000000002fa019 <+313>: movaps %xmm2,0x20(%rsp)
0x00000000002fa01e <+318>: movaps %xmm3,0x10(%rsp)
Again, I see no code here which touches 0xc(%rdi) or the three bytes
after it.
But union buffered_input_event has no hole at bytes 12-15, only two of
its union members do.
So it seems this may be a bug with bitfield enums; it's not quite clear
to me why we're using one here, but it doesn't seem to be working as
intended.
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 20:14:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> Can you retry with x/64gx [...]
(gdb) info locals
event = 0x555555953aa0 <kbd_buffer+384>
copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c84660, requestor =
0x555556038a80, selection = 0x45, target = 0x4d, property = 0x5e, time = 0}
moved_events = <optimized out>
(gdb) x/64gx 0x555555c84660
0x555555c84660: 0x0000000000000000 0x0000555555b05f28
0x555555c84670: 0x0000555555b7a000 0x0000000000000000
0x555555c84680: 0x00007fffef274083 0x0000000100000001
0x555555c84690: 0x0000555555c6d750 0x0000000300000006
0x555555c846a0: 0x0000000000000011 0x0000000000000000
0x555555c846b0: 0x0000000000000000 0x0000000000000000
0x555555c846c0: 0x4058000000000000 0x4058000000000000
0x555555c846d0: 0x0000002000000000 0x0000000000000001
0x555555c846e0: 0xffffffffffffffff 0x000000000000002a
0x555555c846f0: 0x0000000000000000 0x0000555555da3120
0x555555c84700: 0x0000555555da0d20 0xffffffffffffffff
0x555555c84710: 0xffffffff00000000 0x00000000ffffffff
0x555555c84720: 0x0000000000000000 0x0000000000000000
0x555555c84730: 0x0000000000000000 0x0000000000000000
0x555555c84740: 0x0000000000000000 0x0000000000000000
0x555555c84750: 0x0000555555b06178 0x0000555555b06178
0x555555c84760: 0x0000555555b06178 0x0000000000000000
0x555555c84770: 0x0000000000000000 0x0000000000000000
0x555555c84780: 0x0000000000000000 0x0000000000000000
0x555555c84790: 0x0000000000000000 0x000000000221256a
0x555555c847a0: 0x0000000000000000 0x0000555555d6ce40
0x555555c847b0: 0x0000555555c12aa0 0x0000555555c6dce0
0x555555c847c0: 0x0000000000000000 0x0000000000000000
0x555555c847d0: 0x0000555555e75b90 0x0000555555b06178
0x555555c847e0: 0x0000000000000000 0x0000000000000000
0x555555c847f0: 0x3ff0000000000000 0x3ff0000000000000
0x555555c84800: 0x0000000000000004 0x0000000000000021
0x555555c84810: 0x0000555555c0c360 0x00007ffff7fade20
0x555555c84820: 0x0000000000000000 0x0000000000000041
0x555555c84830: 0x0000555555c0fb00 0x0000555555c7bb40
0x555555c84840: 0x0000000000000000 0x0000555555c0faa0
0x555555c84850: 0x0000555555c80220 0x0000000000000000
I have just tried with gcc 14.2 and it works well - NO crash, although it
shows the same "6-byte hole" for `ptype/o struct selection_input_event`.
Kind Regards
Iurie
On Wed, 26 Feb 2025 at 19:47, Pip Cet <pipcet <at> protonmail.com> wrote:
> "Iurie Marian" <marian.iurie <at> gmail.com> writes:
>
> > Yes, it looks like Michael's changes have nothing to do with this bug,
> > but these seem just to reveal some undefined behavior... idk. Btw,
> > just by commenting the line src/keyboard.c:11697, it is not crashing
> > anymore; maybe this could be a hint.
> >
> >> gcc --version
> > gcc (Debian 12.2.0-14) 12.2.0
> >
> >> Can you check that 0x555555cf0b00 is a valid dpyinfo structure?
> > (gdb) info locals
> > event = 0x555555953aa0 <kbd_buffer+384>
> > copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c82260, requestor
> = 0x555555f93a80, selection = 0x45, target = 0x4d, property =
> > 0x5e, time = 0}
> > moved_events = <optimized out>
> >
> > (gdb) x 0x555555c82260
> > 0x555555c82260: 0x00
>
> Well, that only tells us the first byte is 0, which is probably correct.
> Can you retry with x/64gx 0x555555c82260 (or the new address) so we see
> some more data?
>
> >> Can you run "ptype/o struct selection_input_event" [...]
> >
> > (gdb) ptype/o struct selection_input_event
> > /* offset | size */ type = struct selection_input_event {
> > /* 0: 0 | 4 */ enum event_kind kind : 16;
> > /* XXX 6-byte hole */
>
> This is strange, but it looks like this may be a C undefined behavior
> bug (or, less likely, an actual GCC bug). If the event_kind bitfield is
> listed with size 4, shouldn't the hole after it be listed with size 4,
> not size 6?
>
> Here's the code obtained by disass/s evq_flush which copies the relevant
> part of the header:
>
> 3810 *kbd_store_ptr = *event;
> 0x00000000002f9f4c <+108>: movd %xmm0,(%rdx)
> 0x00000000002f9f50 <+112>: movdqa 0x20(%rsp),%xmm4
> 0x00000000002f9f56 <+118>: movdqa 0x10(%rsp),%xmm5
> 0x00000000002f9f5c <+124>: movq %xmm1,0x4(%rdx)
>
> The first movd (not movq or movdq!) copies four bytes containing the
> event_kind. The unaligned movq at +124 copies 8 bytes to bytes 4-11 of
> the struct, which copies the low-order 4 bytes of the dpyinfo.
>
> the code continues with:
>
> 0x00000000002f9f61 <+129>: mov %rax,0x1094e0(%rip) #
> 0x403448 <kbd_store_ptr>
> 0x00000000002f9f68 <+136>: sub %rcx,%rax
> 0x00000000002f9f6b <+139>: sar $0x6,%rax
> 0x00000000002f9f6f <+143>: mov %r12,0x20(%rdx)
> 0x00000000002f9f73 <+147>: mov %rbp,0x38(%rdx)
> 0x00000000002f9f77 <+151>: movups %xmm4,0x10(%rdx)
> 0x00000000002f9f7b <+155>: movups %xmm5,0x28(%rdx)
>
> but, as far as I can tell, bytes 12-15 are never touched by this code.
>
> Here's the corresponding code which copies the event structure:
>
> 327 union buffered_input_event ev = evq->q[0];
> 0x00000000002f9fee <+270>: lea 0x19a64b(%rip),%rcx #
> 0x494640 <event_q.lto_priv.0>
> 0x00000000002f9ff5 <+277>: mov (%rcx),%rdi
> 0x00000000002f9ff8 <+280>: movd (%rdi),%xmm0
> 0x00000000002f9ffc <+284>: movdqu 0x10(%rdi),%xmm2
> 0x00000000002fa001 <+289>: movdqu 0x28(%rdi),%xmm3
> 0x00000000002fa006 <+294>: movq 0x4(%rdi),%xmm1
> 0x00000000002fa00b <+299>: mov 0x20(%rdi),%r12
> 0x00000000002fa00f <+303>: mov 0x38(%rdi),%rbp
> 0x00000000002fa013 <+307>: pextrw $0x0,%xmm0,%r13d
> 0x00000000002fa019 <+313>: movaps %xmm2,0x20(%rsp)
> 0x00000000002fa01e <+318>: movaps %xmm3,0x10(%rsp)
>
>
> Again, I see no code here which touches 0xc(%rdi) or the three bytes
> after it.
>
> But union buffered_input_event has no hole at bytes 12-15, only two of
> its union members do.
>
> So it seems this may be a bug with bitfield enums; it's not quite clear
> to me why we're using one here, but it doesn't seem to be working as
> intended.
>
> Pip
>
>
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 20:25:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Pip Cet <pipcet <at> protonmail.com> writes:
> "Iurie Marian" <marian.iurie <at> gmail.com> writes:
>
>> Yes, it looks like Michael's changes have nothing to do with this bug,
>> but these seem just to reveal some undefined behavior... idk. Btw,
>> just by commenting the line src/keyboard.c:11697, it is not crashing
>> anymore; maybe this could be a hint.
>>
>>> gcc --version
>> gcc (Debian 12.2.0-14) 12.2.0
>>
>>> Can you check that 0x555555cf0b00 is a valid dpyinfo structure?
>> (gdb) info locals
>> event = 0x555555953aa0 <kbd_buffer+384>
>> copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c82260, requestor = 0x555555f93a80, selection = 0x45, target = 0x4d, property =
>> 0x5e, time = 0}
>> moved_events = <optimized out>
>>
>> (gdb) x 0x555555c82260
>> 0x555555c82260: 0x00
>
> Well, that only tells us the first byte is 0, which is probably correct.
> Can you retry with x/64gx 0x555555c82260 (or the new address) so we see
> some more data?
>
>>> Can you run "ptype/o struct selection_input_event" [...]
>>
>> (gdb) ptype/o struct selection_input_event
>> /* offset | size */ type = struct selection_input_event {
>> /* 0: 0 | 4 */ enum event_kind kind : 16;
>> /* XXX 6-byte hole */
>
> This is strange, but it looks like this may be a C undefined behavior
> bug (or, less likely, an actual GCC bug). If the event_kind bitfield is
> listed with size 4, shouldn't the hole after it be listed with size 4,
> not size 6?
Investigating the undefined behavior bug theory further, I find that
applying this patch changes the code emitted for evq_flush (I think this
is strange, since evq_flush doesn't call kbd_buffer_store_event, it
calls kbd_buffer_store_buffered_event!).
diff --git a/src/keyboard.h b/src/keyboard.h
index 5e04b54eb74..c1c75cc7ea5 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -505,10 +505,9 @@ #define EVENT_HEAD_KIND(event_head) \
kbd_buffer_store_event_hold (struct input_event *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);
+ union buffered_input_event bie;
+ bie.ie = *event;
+ kbd_buffer_store_buffered_event (&bie, hold_quit);
}
extern void poll_for_input_1 (void);
extern void show_help_echo (Lisp_Object, Lisp_Object, Lisp_Object,
Can you confirm whether it changes anything for you?
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Wed, 26 Feb 2025 20:38:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Alright! This change fixes the issue!
It works with gcc 12.2 as well!
Thank you very much, Pip!
KR,
Iurie
On Wed, 26 Feb 2025 at 21:24, Pip Cet <pipcet <at> protonmail.com> wrote:
> Pip Cet <pipcet <at> protonmail.com> writes:
>
> > "Iurie Marian" <marian.iurie <at> gmail.com> writes:
> >
> >> Yes, it looks like Michael's changes have nothing to do with this bug,
> >> but these seem just to reveal some undefined behavior... idk. Btw,
> >> just by commenting the line src/keyboard.c:11697, it is not crashing
> >> anymore; maybe this could be a hint.
> >>
> >>> gcc --version
> >> gcc (Debian 12.2.0-14) 12.2.0
> >>
> >>> Can you check that 0x555555cf0b00 is a valid dpyinfo structure?
> >> (gdb) info locals
> >> event = 0x555555953aa0 <kbd_buffer+384>
> >> copy = {kind = SELECTION_REQUEST_EVENT, dpyinfo = 0x55c82260, requestor
> = 0x555555f93a80, selection = 0x45, target = 0x4d, property =
> >> 0x5e, time = 0}
> >> moved_events = <optimized out>
> >>
> >> (gdb) x 0x555555c82260
> >> 0x555555c82260: 0x00
> >
> > Well, that only tells us the first byte is 0, which is probably correct.
> > Can you retry with x/64gx 0x555555c82260 (or the new address) so we see
> > some more data?
> >
> >>> Can you run "ptype/o struct selection_input_event" [...]
> >>
> >> (gdb) ptype/o struct selection_input_event
> >> /* offset | size */ type = struct selection_input_event {
> >> /* 0: 0 | 4 */ enum event_kind kind : 16;
> >> /* XXX 6-byte hole */
> >
> > This is strange, but it looks like this may be a C undefined behavior
> > bug (or, less likely, an actual GCC bug). If the event_kind bitfield is
> > listed with size 4, shouldn't the hole after it be listed with size 4,
> > not size 6?
>
> Investigating the undefined behavior bug theory further, I find that
> applying this patch changes the code emitted for evq_flush (I think this
> is strange, since evq_flush doesn't call kbd_buffer_store_event, it
> calls kbd_buffer_store_buffered_event!).
>
> diff --git a/src/keyboard.h b/src/keyboard.h
> index 5e04b54eb74..c1c75cc7ea5 100644
> --- a/src/keyboard.h
> +++ b/src/keyboard.h
> @@ -505,10 +505,9 @@ #define EVENT_HEAD_KIND(event_head) \
> kbd_buffer_store_event_hold (struct input_event *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);
> + union buffered_input_event bie;
> + bie.ie = *event;
> + kbd_buffer_store_buffered_event (&bie, hold_quit);
> }
> extern void poll_for_input_1 (void);
> extern void show_help_echo (Lisp_Object, Lisp_Object, Lisp_Object,
>
> Can you confirm whether it changes anything for you?
>
> Pip
>
>
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 05:36:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Pip Cet <pipcet <at> protonmail.com> writes:
> This is strange, but it looks like this may be a C undefined behavior
> bug (or, less likely, an actual GCC bug). If the event_kind bitfield is
> listed with size 4, shouldn't the hole after it be listed with size 4,
> not size 6?
I'm afraid that must be a Gdb or GCC debuginfo generation bug, since the
total size of the structure is 56. Whereas the aggregate of the values
printed by GDB is 58.
> Here's the code obtained by disass/s evq_flush which copies the relevant
> part of the header:
>
> 3810 *kbd_store_ptr = *event;
> 0x00000000002f9f4c <+108>: movd %xmm0,(%rdx)
> 0x00000000002f9f50 <+112>: movdqa 0x20(%rsp),%xmm4
> 0x00000000002f9f56 <+118>: movdqa 0x10(%rsp),%xmm5
> 0x00000000002f9f5c <+124>: movq %xmm1,0x4(%rdx)
If kbd_buffer_store_buffered_event is forcibly inlined, my GCC 12.2.0
generates virtually identical machine code to yours:
0x000000000067fd8c <+188>: mov 0x1f653d(%rip),%rdi # 0x8762d0 <event_q.lto_priv.0>
0x000000000067fd93 <+195>: movd (%rdi),%xmm0
0x000000000067fd97 <+199>: movdqu 0x10(%rdi),%xmm2
0x000000000067fd9c <+204>: movdqu 0x28(%rdi),%xmm3
0x000000000067fda1 <+209>: movq 0x4(%rdi),%xmm1
0x000000000067fda6 <+214>: mov 0x20(%rdi),%r14
0x000000000067fdaa <+218>: mov 0x38(%rdi),%r13
[...]
0x000000000067fdb4 <+228>: movaps %xmm2,0x20(%rsp)
0x000000000067fdb9 <+233>: movaps %xmm3,0x10(%rsp)
[...]
0x000000000067fd27 <+87>: movd %xmm0,(%rdx) ;; bytes 0-3
0x000000000067fd2b <+91>: movdqa 0x20(%rsp),%xmm4
0x000000000067fd31 <+97>: movdqa 0x10(%rsp),%xmm5
0x000000000067fd37 <+103>: movq %xmm1,0x4(%rdx) ;; bytes 4-11
0x000000000067fd4a <+122>: mov %r14,0x20(%rdx) ;; bytes 32-39
0x000000000067fd4e <+126>: mov %r13,0x38(%rdx) ;; bytes 56-63
0x000000000067fd52 <+130>: movups %xmm4,0x10(%rdx) ;; bytes 16-31
0x000000000067fd56 <+134>: movups %xmm5,0x28(%rdx) ;; bytes 40-55
You'll observe that the gap coincides exactly with the padding in struct
input_event. This is the RTL GCC produces just as it is lowered from
optimized expression trees:
(insn 582 581 583 62 (set (mem:V2HI (reg/f:DI 106 [ _68 ]) [66 MEM <vector(2) short unsigned int> [(union buffered_input_event *)_68]+0 S4 A32])
(reg:V2HI 170 [ vect_ev_kind_24.71 ])) "keyboard.c":3810:22 -1
(nil)) ;; bytes 0-3
(insn 583 582 584 62 (set (mem:V2SI (plus:DI (reg/f:DI 106 [ _68 ])
(const_int 4 [0x4])) [66 MEM <vector(2) unsigned int> [(union buffered_input_event *)_68 + 4B]+0 S8 A32])
(reg:V2SI 167 [ vect_ev_ie_code_157.76 ])) "keyboard.c":3810:22 -1
(nil)) ;; bytes 4-11
(insn 584 583 585 62 (set (mem:V2DI (plus:DI (reg/f:DI 106 [ _68 ])
(const_int 16 [0x10])) [66 MEM <vector(2) long unsigned int> [(union buffered_input_event *)_68 + 16B]+0 S16 A64])
(reg:V2DI 164 [ vect_ev_ie_x_134.83 ])) "keyboard.c":3810:22 -1
(nil)) ;; bytes 16-31
(insn 585 584 586 62 (set (mem:DI (plus:DI (reg/f:DI 106 [ _68 ])
(const_int 32 [0x20])) [66 MEM <Time> [(union buffered_input_event *)_68 + 32B]+0 S8 A64])
(reg:DI 84 [ ev$ie$timestamp ])) "keyboard.c":3810:22 -1
(nil)) ;; bytes 32-39
(insn 586 585 587 62 (set (mem:V2DI (plus:DI (reg/f:DI 106 [ _68 ])
(const_int 40 [0x28])) [66 MEM <vector(2) long unsigned int> [(union buffered_input_event *)_68 + 40B]+0 S16 A64]) ;; bytes 40-55
(reg:V2DI 163 [ vect_ev_ie_frame_or_window_11.90 ])) "keyboard.c":3810:22 -1
(nil))
(insn 587 586 588 62 (set (mem/f:DI (plus:DI (reg/f:DI 106 [ _68 ])
(const_int 56 [0x38])) [66 MEM <struct Lisp_X *> [(union buffered_input_event *)_68 + 56B]+0 S8 A64])
(reg/f:DI 110 [ ev$ie$device ])) "keyboard.c":3810:22 -1
(nil)) ;; bytes 56-63
Judging by the expression names, it's clearly copying only the `struct
input_event' union member. In fact, this erroneous code is generated as
soon as SRA processes aggregate references.
MEM <short unsigned int> [(union buffered_input_event *)_68] = ev$kind_24;
MEM <short unsigned int> [(union buffered_input_event *)_68 + 2B] = ev$ie$part_10;
MEM <unsigned int> [(union buffered_input_event *)_68 + 4B] = ev$ie$code_157;
MEM <unsigned int> [(union buffered_input_event *)_68 + 8B] = ev$ie$modifiers_58;
MEM <struct Lisp_X *> [(union buffered_input_event *)_68 + 16B] = ev$ie$x_134;
MEM <struct Lisp_X *> [(union buffered_input_event *)_68 + 24B] = ev$ie$y_90;
MEM <Time> [(union buffered_input_event *)_68 + 32B] = ev$ie$timestamp_23;
MEM <struct Lisp_X *> [(union buffered_input_event *)_68 + 40B] = ev$ie$frame_or_window_11;
MEM <struct Lisp_X *> [(union buffered_input_event *)_68 + 48B] = ev$ie$arg_78;
MEM <struct Lisp_X *> [(union buffered_input_event *)_68 + 56B] = ev$ie$device_77;
What I suspect is that there is some CU in which `struct
selection_input_event' is incomplete or fails to appear in `union
buffered_input_event' and that that misleads the compiler during
link-time recompilation. But I could be far off the mark, since I am
very much out of touch with contemporary GCC.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 05:38:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> Po Lu, any suggestions or ideas?
I replied to Pip Cet. It's likely a GCC bug, and one which I cannot
reproduce in 14.2.1 or the recentest GCC 15 snapshot, rather than
undefined behavior in any sense...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 08:42:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> From: Po Lu <luangruo <at> yahoo.com>
> Cc: Iurie Marian <marian.iurie <at> gmail.com>, michael.albinus <at> gmx.de,
> iura.mail <at> gmail.com, 76559 <at> debbugs.gnu.org
> Date: Thu, 27 Feb 2025 13:36:59 +0800
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > Po Lu, any suggestions or ideas?
>
> I replied to Pip Cet. It's likely a GCC bug, and one which I cannot
> reproduce in 14.2.1 or the recentest GCC 15 snapshot, rather than
> undefined behavior in any sense...
Thanks.
I believe we should recommend against using -O3. Too many bad bugs
happened recently, especially in the latest and hottest versions of
GCC to justify such a recommendation. Especially since using -O3 is
extremely unlikely to produce tangible performance advantages (if
someone has data to the contrary, I'd be interested in seeing it), due
to very large inner loops in our code. Moreover, the excessive
inlining affected by -O3 could well cause CPU caches to overflow, so
it could well be a net loss in performance.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 10:14:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Po Lu <luangruo <at> yahoo.com> writes:
> What I suspect is that there is some CU in which `struct
> selection_input_event' is incomplete or fails to appear in `union
> buffered_input_event' and that that misleads the compiler during
> link-time recompilation. But I could be far off the mark, since I am
> very much out of touch with contemporary GCC.
After devoting another 30 minutes to this problem, I've reduced it to a
trivial test case that reveals the same miscompilation under GCC 14.2.1:
[reduction.h (text/plain, attachment)]
[reduction1.c (text/plain, attachment)]
[reduction.c (text/plain, attachment)]
[Message part 5 (text/plain, inline)]
Simply execute:
$ gcc -fdump-tree-all -fdump-rtl-all -save-temps -O3 -flto -g3 \
reduction1.c reduction.c -o reduction && ./reduction
ffffffff
and observe that the high 32 bits of the dpyinfo pointer in the saved
event are truncated, as would be expected from these expressions:
ev_12 = MEM <unsigned int> [(union buffered_input_event *)&main_inev];
[...]
MEM <unsigned int> [(union buffered_input_event *)&kbd_buffer] = ev_12;
MEM <unsigned int> [(union buffered_input_event *)&kbd_buffer + 4B] = ev$ie$code_16;
MEM <unsigned int> [(union buffered_input_event *)&kbd_buffer + 8B] = ev$ie$modifiers_1;
MEM <long int> [(union buffered_input_event *)&kbd_buffer + 16B] = ev$ie$device_6;
With aggregate access expansion disabled, all is normal:
$ gcc -fdump-tree-all -fdump-rtl-all -fno-tree-sra -save-temps \
-O3 -flto -g3 reduction1.c reduction.c -o reduction && ./reduction
ffffffffffffffff
In a word, tampering with optimizer options is tantamount to playing
with fire. Don't do it, if your object is a functioning Emacs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 10:16:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Po Lu <luangruo <at> yahoo.com> writes:
> Po Lu <luangruo <at> yahoo.com> writes:
>
>> What I suspect is that there is some CU in which `struct
>> selection_input_event' is incomplete or fails to appear in `union
>> buffered_input_event' and that that misleads the compiler during
>> link-time recompilation. But I could be far off the mark, since I am
>> very much out of touch with contemporary GCC.
>
> After devoting another 30 minutes to this problem, I've reduced it to a
> trivial test case that reveals the same miscompilation under GCC 14.2.1:
>
>
>
>
>
> Simply execute:
>
> $ gcc -fdump-tree-all -fdump-rtl-all -save-temps -O3 -flto -g3 \
> reduction1.c reduction.c -o reduction && ./reduction
> ffffffff
>
> and observe that the high 32 bits of the dpyinfo pointer in the saved
> event are truncated, as would be expected from these expressions:
>
> ev_12 = MEM <unsigned int> [(union buffered_input_event *)&main_inev];
>
> [...]
>
> MEM <unsigned int> [(union buffered_input_event *)&kbd_buffer] = ev_12;
> MEM <unsigned int> [(union buffered_input_event *)&kbd_buffer + 4B] = ev$ie$code_16;
> MEM <unsigned int> [(union buffered_input_event *)&kbd_buffer + 8B] = ev$ie$modifiers_1;
> MEM <long int> [(union buffered_input_event *)&kbd_buffer + 16B] = ev$ie$device_6;
>
> With aggregate access expansion disabled, all is normal:
>
> $ gcc -fdump-tree-all -fdump-rtl-all -fno-tree-sra -save-temps \
> -O3 -flto -g3 reduction1.c reduction.c -o reduction && ./reduction
> ffffffffffffffff
>
> In a word, tampering with optimizer options is tantamount to playing
> with fire. Don't do it, if your object is a functioning Emacs.
Which, if it wasn't apparent, belies my earlier theory respecting what
would be reported in C++ mode as ODR violations. It's a genuine GCC
bug.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 11:34:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Po Lu" <luangruo <at> yahoo.com> writes:
> Pip Cet <pipcet <at> protonmail.com> writes:
>
>> This is strange, but it looks like this may be a C undefined behavior
>> bug (or, less likely, an actual GCC bug). If the event_kind bitfield is
>> listed with size 4, shouldn't the hole after it be listed with size 4,
>> not size 6?
>
> I'm afraid that must be a Gdb or GCC debuginfo generation bug, since the
> total size of the structure is 56. Whereas the aggregate of the values
> printed by GDB is 58.
Indeed. It's probably unrelated.
> Judging by the expression names, it's clearly copying only the `struct
> input_event' union member. In fact, this erroneous code is generated as
> soon as SRA processes aggregate references.
Yes, SRA seems to be the pass to blame.
> What I suspect is that there is some CU in which `struct
> selection_input_event' is incomplete or fails to appear in `union
> buffered_input_event' and that that misleads the compiler during
> link-time recompilation. But I could be far off the mark, since I am
> very much out of touch with contemporary GCC.
See my other message. I'm still puzzled as to why an unsafe cast in one
call path modifies the assumptions made for another call path, but I
guess it all falls under undefined behavior and we should simply apply
that patch and make sure to follow the C standard more closely.
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 12:05:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Po Lu" <luangruo <at> yahoo.com> writes:
> Po Lu <luangruo <at> yahoo.com> writes:
>
>> In a word, tampering with optimizer options is tantamount to playing
>> with fire. Don't do it, if your object is a functioning Emacs.
>
> Which, if it wasn't apparent, belies my earlier theory respecting what
> would be reported in C++ mode as ODR violations. It's a genuine GCC
> bug.
I don't think your case proves that. In your "reduction" code, you have
this:
void
kbd_buffer_store_buffered_event (union buffered_input_event *event,
struct input_event *hold_quit)
{
*hold_quit = event->ie;
*kbd_store_ptr = *event;
}
When event->ie is accessed, GCC can conclude that the 'ie' union member,
not the 'sie' union member, is active, and legitimately perform the
truncation which results in the incorrect code.
It is, of course, possible that there's still a GCC bug here.
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 12:23:01 GMT)
Full text and
rfc822 format available.
Message #68 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Eli Zaretskii" <eliz <at> gnu.org> writes:
>> From: Po Lu <luangruo <at> yahoo.com>
>> Cc: Iurie Marian <marian.iurie <at> gmail.com>, michael.albinus <at> gmx.de,
>> iura.mail <at> gmail.com, 76559 <at> debbugs.gnu.org
>> Date: Thu, 27 Feb 2025 13:36:59 +0800
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > Po Lu, any suggestions or ideas?
>>
>> I replied to Pip Cet. It's likely a GCC bug, and one which I cannot
>> reproduce in 14.2.1 or the recentest GCC 15 snapshot, rather than
>> undefined behavior in any sense...
(I'm not convinced there's a GCC bug there, but it doesn't matter:
aggressive GCC optimizations currently result in broken Emacs builds).
> Thanks.
>
> I believe we should recommend against using -O3. Too many bad bugs
If we do that, can we include link time optimization in that? I find
LTO'd code extremely difficult to read, since things like the calling
convention no longer apply (even when a function call isn't inlined, LTO
can still conclude that some registers survive the call and reuse them
even though the calling convention disagrees).
However, let's be careful not to blame GCC until we have a smoking gun.
So far, it's usually Emacs which is at fault.
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 12:39:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Pip Cet <pipcet <at> protonmail.com> writes:
> See my other message. I'm still puzzled as to why an unsafe cast in one
> call path modifies the assumptions made for another call path, but I
> guess it all falls under undefined behavior and we should simply apply
> that patch and make sure to follow the C standard more closely.
I revisited the material provisions in n1256, and the problem is that a
`union buffered_input_event' is read from the EVENT pointer, whether or
not the object behind the pointer is of a type compatible with the
union. It is permissible for an object whose type is a member of a
union or aggregate to be accessed from a pointer whose type is the same
union or aggregate, but apparently not more generally to access a union
from a pointer of its type from which it is only defined to access one
of its members, even if the latter is identical in size.
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
Please test.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 13:17:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 27 Feb 2025 12:22:36 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: Po Lu <luangruo <at> yahoo.com>, marian.iurie <at> gmail.com, michael.albinus <at> gmx.de, iura.mail <at> gmail.com, 76559 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>, Paul Eggert <eggert <at> cs.ucla.edu>
>
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>
> > I believe we should recommend against using -O3. Too many bad bugs
>
> If we do that, can we include link time optimization in that? I find
> LTO'd code extremely difficult to read, since things like the calling
> convention no longer apply (even when a function call isn't inlined, LTO
> can still conclude that some registers survive the call and reuse them
> even though the calling convention disagrees).
Maybe we should also recommend against LTO. Paul, WDYT about this?
> However, let's be careful not to blame GCC until we have a smoking gun.
> So far, it's usually Emacs which is at fault.
Sure. I only said what I said based on enough examples we saw in the
recent years.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 13:31:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Date: Thu, 27 Feb 2025 12:22:36 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: Po Lu <luangruo <at> yahoo.com>, marian.iurie <at> gmail.com,
>> michael.albinus <at> gmx.de, iura.mail <at> gmail.com, 76559 <at> debbugs.gnu.org,
>> Stefan Kangas <stefankangas <at> gmail.com>, Paul Eggert
>> <eggert <at> cs.ucla.edu>
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> > I believe we should recommend against using -O3. Too many bad bugs
>>
>> If we do that, can we include link time optimization in that? I find
>> LTO'd code extremely difficult to read, since things like the calling
>> convention no longer apply (even when a function call isn't inlined, LTO
>> can still conclude that some registers survive the call and reuse them
>> even though the calling convention disagrees).
It's marginally easier to read the RTL (which ultimately becomes no more
than a Lisp-like representation of the assembly but with some tree
expressions and analogous details preserved for reference,
e.g. non-inlined procedure calls).
> Maybe we should also recommend against LTO. Paul, WDYT about this?
I think there is no occasion to compile Emacs with any optimization
options besides -O2.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 18:51:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 76559 <at> debbugs.gnu.org (full text, mbox):
On 2025-02-27 04:22, Pip Cet wrote:
> (I'm not convinced there's a GCC bug there, but it doesn't matter:
> aggressive GCC optimizations currently result in broken Emacs builds).
I don't see a GCC bug there. If a program casts a pointer to a different
pointer type and dereferences the result, behavior is undefined (with
exceptions for void * and char * that don't apply here). And even if we
blacklist gcc -O3 -flto, who's to say some other compiler won't do the
optimization?
The violation of C rules occurs in keyboard.h, here:
INLINE void
kbd_buffer_store_event_hold (struct input_event *event,
struct input_event *hold_quit)
{
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.
One possible fix to the bug would be to change the API of
kbd_buffer_store_event_hold to accept a pointer to the union rather than
a pointer to the struct, and change its callers accordingly. There are
other possible fixes. I'm dubious whether Po Lu's latest proposal
<https://bugs.gnu.org/76559#71> suffices, though, as it doesn't fix the
invalid pointer cast.
>> I believe we should recommend against using -O3. Too many bad bugs
>
> If we do that, can we include link time optimization in that?
I've found real bugs with -flto, an option that is not supposed to
introduce bugs though it may trigger latent bugs. Although I'm not a fan
of -flto for typical use (for one thing, it's too expensive) I don't
think we should discourage it at least when debugging.
Similarly for -O3.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 18:51:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
@Po Lu: I have just tested, your proposal and of course it works well (I've
used that build version for a while).
Btw, I noticed that this crash occurs ONLY for builds where -O3 and LTO are
used together.
On Thu, 27 Feb 2025 at 14:30, Po Lu <luangruo <at> yahoo.com> wrote:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> Date: Thu, 27 Feb 2025 12:22:36 +0000
> >> From: Pip Cet <pipcet <at> protonmail.com>
> >> Cc: Po Lu <luangruo <at> yahoo.com>, marian.iurie <at> gmail.com,
> >> michael.albinus <at> gmx.de, iura.mail <at> gmail.com, 76559 <at> debbugs.gnu.org,
> >> Stefan Kangas <stefankangas <at> gmail.com>, Paul Eggert
> >> <eggert <at> cs.ucla.edu>
> >>
> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> >>
> >> > I believe we should recommend against using -O3. Too many bad bugs
> >>
> >> If we do that, can we include link time optimization in that? I find
> >> LTO'd code extremely difficult to read, since things like the calling
> >> convention no longer apply (even when a function call isn't inlined, LTO
> >> can still conclude that some registers survive the call and reuse them
> >> even though the calling convention disagrees).
>
> It's marginally easier to read the RTL (which ultimately becomes no more
> than a Lisp-like representation of the assembly but with some tree
> expressions and analogous details preserved for reference,
> e.g. non-inlined procedure calls).
>
> > Maybe we should also recommend against LTO. Paul, WDYT about this?
>
> I think there is no occasion to compile Emacs with any optimization
> options besides -O2.
>
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 19:59:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 27 Feb 2025 10:50:18 -0800
> Cc: Po Lu <luangruo <at> yahoo.com>, marian.iurie <at> gmail.com,
> michael.albinus <at> gmx.de, iura.mail <at> gmail.com, 76559 <at> debbugs.gnu.org,
> Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
> From: Paul Eggert <eggert <at> cs.ucla.edu>
>
> >> I believe we should recommend against using -O3. Too many bad bugs
> >
> > If we do that, can we include link time optimization in that?
>
> I've found real bugs with -flto, an option that is not supposed to
> introduce bugs though it may trigger latent bugs. Although I'm not a fan
> of -flto for typical use (for one thing, it's too expensive) I don't
> think we should discourage it at least when debugging.
>
> Similarly for -O3.
The problem with -O3 that too many recent GCC versions had bugs in it,
especially in the first version of a major release. By "recommend
against" I meant to advise users and distros (as opposed to Emacs
developers) not to use it. I didn't mean to reject it, so if you or
someone else wants to use it as a means for finding bugs, you can.
People nowadays are likely to upgrade to the latest GCC version as
soon as it's out, and immediately start using it for production, and
are also too eager to use -O3 without checking whether it gives them
any gains. The recommendation I have in mind aims at making the lives
of those users safer and less complicated.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Thu, 27 Feb 2025 21:14:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 76559 <at> debbugs.gnu.org (full text, mbox):
On 2/27/25 11:58, Eli Zaretskii wrote:
> People nowadays are likely to upgrade to the latest GCC version as
> soon as it's out, and immediately start using it for production, and
> are also too eager to use -O3 without checking whether it gives them
> any gains. The recommendation I have in mind aims at making the lives
> of those users safer and less complicated.
Yes, that all sounds good in the context of ordinary builds. Similarly,
we could recommend against options like -Os and -fsanitize=undefined in
ordinary builds.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Fri, 28 Feb 2025 00:13:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Paul Eggert <eggert <at> cs.ucla.edu> writes:
> On 2025-02-27 04:22, Pip Cet wrote:
>> (I'm not convinced there's a GCC bug there, but it doesn't matter:
>> aggressive GCC optimizations currently result in broken Emacs
>> builds).
>
> I don't see a GCC bug there. If a program casts a pointer to a
> different pointer type and dereferences the result, behavior is
> undefined (with exceptions for void * and char * that don't apply
> here). And even if we blacklist gcc -O3 -flto, who's to say some other
> compiler won't do the optimization?
>
> The violation of C rules occurs in keyboard.h, here:
>
> INLINE void
> kbd_buffer_store_event_hold (struct input_event *event,
> struct input_event *hold_quit)
> {
> 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 ..."
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Fri, 28 Feb 2025 07:37:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 27 Feb 2025 13:13:31 -0800
> Cc: pipcet <at> protonmail.com, luangruo <at> yahoo.com, marian.iurie <at> gmail.com,
> michael.albinus <at> gmx.de, iura.mail <at> gmail.com, 76559 <at> debbugs.gnu.org,
> stefankangas <at> gmail.com
> From: Paul Eggert <eggert <at> cs.ucla.edu>
>
> On 2/27/25 11:58, Eli Zaretskii wrote:
> > People nowadays are likely to upgrade to the latest GCC version as
> > soon as it's out, and immediately start using it for production, and
> > are also too eager to use -O3 without checking whether it gives them
> > any gains. The recommendation I have in mind aims at making the lives
> > of those users safer and less complicated.
>
> Yes, that all sounds good in the context of ordinary builds. Similarly,
> we could recommend against options like -Os and -fsanitize=undefined in
> ordinary builds.
Thanks, I added something to that effect to INSTALL.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Fri, 28 Feb 2025 17:07:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Po Lu" <luangruo <at> yahoo.com> writes:
> Pip Cet <pipcet <at> protonmail.com> writes:
>
>> See my other message. I'm still puzzled as to why an unsafe cast in one
>> call path modifies the assumptions made for another call path, but I
>> guess it all falls under undefined behavior and we should simply apply
>> that patch and make sure to follow the C standard more closely.
>
> I revisited the material provisions in n1256, and the problem is that a
> `union buffered_input_event' is read from the EVENT pointer, whether or
> not the object behind the pointer is of a type compatible with the
> union. It is permissible for an object whose type is a member of a
> union or aggregate to be accessed from a pointer whose type is the same
> union or aggregate, but apparently not more generally to access a union
> from a pointer of its type from which it is only defined to access one
> of its members, even if the latter is identical in size.
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.
> 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.
It seems to me that this is a situation that GCC should warn about,
rather than silently generating unintended code. Maybe we should submit
your "reduction" case as a wishlist item for a new GCC warning.
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Fri, 28 Feb 2025 18:35:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[0001-Fix-union-pointer-aliasing-when-storing-events.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 00:09:02 GMT)
Full text and
rfc822 format available.
Message #104 received at 76559 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 03:21:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
BTW, if so, how do you explain this reproducer? At no point in the
code's execution is hold_quit ever read or written to.
[reduction.h (text/plain, attachment)]
[reduction1.c (text/plain, attachment)]
[reduction.c (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 10:22:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Po Lu" <luangruo <at> yahoo.com> writes:
> 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.
>
> BTW, if so, how do you explain this reproducer? At no point in the
> code's execution is hold_quit ever read or written to.
That does seem to be an actual bug, where SRA does the wrong thing if a
function is marked with __attribute__ ((optimize (0))). At first glance
(-fdump-tree-sra-details) it seems like the whole-structure access in
that function is ignored, which makes sense if the SRA pass isn't run
for a non-optimized function but still assumes it has looked at all
accesses.
So we should definitely report this (after reducing to a single file and
fixing the prototypes), and any of our suspicions about wrong behavior
for other code.
Please let us know the GCC bug number(s).
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 11:26:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
I have reached the bottom of this unpleasant business. The following
reproducer does not require enabling LTO or type punning (or casts at
all, come to that), just "gcc-15 -std=c99 -O3".
[reduction-single.c (text/plain, attachment)]
[Message part 3 (text/plain, inline)]
GCC's intraprocedural SRA pass operates by collecting all references to
decls of aggregate type in a procedure, and creating a tree in which
every node represents all references to each decl which share an
identical offset and size such that the most general reference appears
nearest to the top. In creating such a tree, it assumes that a sorting
function will arrange a vector of references in decreasing order of
generality: larger references appear before smaller ones, aggregate
references before complex or vector references, integral reference
before reals, references to types of greater precision before those of
lesser precision, and, all else being equal, the sorting is stabilized
by reference to the order in which the operand types are defined.
Accordingly, the first element of the vector is assumed to provide the
root of a tree. The compiler proceeds by "totally scalarizing" a decl's
references from its root, if it is of record type and every field is
itself scalar or scalarizable, and scalarizing all statements that
reference the same decl with the same offset and size in the manner
previously defined.
Many properties of every reference to a declaration are fixed at this
time, such as whether the declaration is completely encompassed by
scalarized variables, which fails when a union or a struct is excluded
from a decl's access tree for being identical in offset and size, but
has a different data layout from the type of the access at its root.
In Emacs's case it should be possible to circumvent this problem by
creating an incomplete declaration of `union buffered_input_event'
before `struct input_event', so that the union may be defined first and
inhibit total scalarization, but this will not address the oversight in
GCC. This eliminates the miscompilation, but also a minor class of
optimizations. I believe this optimization must be re-engineered if it
is optimally to scalarize combinations of union and member references.
diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index c26559edc66..edd9b477b39 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -2483,6 +2483,19 @@ sort_and_splice_var_accesses (tree var)
}
unscalarizable_region = true;
}
+ else if (AGGREGATE_TYPE_P (access->type)
+ && AGGREGATE_TYPE_P (ac2->type)
+ && !types_compatible_p (access->type, ac2->type))
+ {
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "Cannot scalarize the following region "
+ "because data may be scalarized in an incompatible "
+ "format.\n");
+ dump_access (dump_file, access, false);
+ }
+ unscalarizable_region = true;
+ }
else if (types_risk_mangled_binary_repr_p (access->type, ac2->type))
{
if (dump_file && (dump_flags & TDF_DETAILS))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 11:30:02 GMT)
Full text and
rfc822 format available.
Message #116 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Pip Cet <pipcet <at> protonmail.com> writes:
> That does seem to be an actual bug, where SRA does the wrong thing if a
> function is marked with __attribute__ ((optimize (0))). At first glance
> (-fdump-tree-sra-details) it seems like the whole-structure access in
> that function is ignored, which makes sense if the SRA pass isn't run
> for a non-optimized function but still assumes it has looked at all
> accesses.
Intraprocedural SRA is not so sophisticated as you assume, alas. It
does not account for data flow at all, only decls and the operands of
GIMPLE statements throughout the totality of a procedure, and it is
undermined by quite a substantial oversight.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 11:33:01 GMT)
Full text and
rfc822 format available.
Message #119 received at 76559 <at> debbugs.gnu.org (full text, mbox):
"Paul Eggert" <eggert <at> cs.ucla.edu> writes:
> 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.)
If patches like this one are necessary:
@@ -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
essentially all code dealing with X events needs to be rewritten, since
they use the same approach. My vague memory is that C always made an
exception for unions like XEvent or our union buffered_input_event,
where the first element of all union members is identical and used
to decide which of the union members is "fully" active.
Looking at the -fdump-tree-sra-details output, I still think it's weird
that no whole-structure access for "ev" is being recorded for evq_flush
(and its inlined functions):
access { base = (8089)'ev', offset = 0, size = 512, expr = ev.ie, type = struct input_event, reverse = 0, grp_read = 1, grp_write = 1, grp_assignment_read = 1, grp_assignment_write = 1, grp_scalar_read = 0, grp_scalar_write = 0, grp_total_scalarization = 1, grp_hint = 0, grp_covered = 1, grp_unscalarizable_region = 0, grp_unscalarized_data = 0, grp_same_access_path = 0, grp_partial_lhs = 0, grp_to_be_replaced = 0, grp_to_be_debug_replaced = 0}
is the only recorded such access unless I comment out
//*hold_quit = event->ie;
access { base = (8089)'ev', offset = 0, size = 512, expr = ev, type = union buffered_input_event, reverse = 0, grp_read = 1, grp_write = 1, grp_assignment_read = 1, grp_assignment_write = 1, grp_scalar_read = 0, grp_scalar_write = 0, grp_total_scalarization = 0, grp_hint = 0, grp_covered = 0, grp_unscalarizable_region = 0, grp_unscalarized_data = 1, grp_same_access_path = 1, grp_partial_lhs = 0, grp_to_be_replaced = 0, grp_to_be_debug_replaced = 0}
So maybe it is time to ask the GCC folks for help here, even though we
do not have a reduced reproducer for the problem (which may well be on
our end) yet.)
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 13:44:02 GMT)
Full text and
rfc822 format available.
Message #122 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Paul Eggert <eggert <at> cs.ucla.edu> writes:
> 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.)
Clearly not...
> 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.
I surmise that it doesn't fix the underlying compiler issue if you add
__attribute__ ((force_inline)) to kbd_buffer_store_buffered_event, as it
is a product of an assumption by GCC's SRA pass that no two accesses of
a single local aggregate declaration with the same offset and size but
of different types alias. (Mine does, as `struct input_event' and
`struct selection_input_event' are of different sizes, so that the
accesses are not taken to alias.) Besides, I did not succeed in
convincing the OP's version of GCC's to inline
kbd_buffer_store_buffered_event without expressly directing so despite
having enabled LTO.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sat, 01 Mar 2025 13:53:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Pip Cet <pipcet <at> protonmail.com> writes:
> If patches like this one are necessary:
>
> @@ -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
>
> essentially all code dealing with X events needs to be rewritten, since
> they use the same approach. My vague memory is that C always made an
> exception for unions like XEvent or our union buffered_input_event,
> where the first element of all union members is identical and used
> to decide which of the union members is "fully" active.
Correct. But I think Paul's point is that the pointed-to object is not
a union, and in consequence that the exception does not apply.
> Looking at the -fdump-tree-sra-details output, I still think it's weird
> that no whole-structure access for "ev" is being recorded for evq_flush
> (and its inlined functions):
>
> access { base = (8089)'ev', offset = 0, size = 512, expr = ev.ie, type = struct input_event, reverse = 0, grp_read = 1, grp_write = 1, grp_assignment_read = 1, grp_assignment_write = 1, grp_scalar_read = 0, grp_scalar_write = 0, grp_total_scalarization = 1, grp_hint = 0, grp_covered = 1, grp_unscalarizable_region = 0, grp_unscalarized_data = 0, grp_same_access_path = 0, grp_partial_lhs = 0, grp_to_be_replaced = 0, grp_to_be_debug_replaced = 0}
>
> is the only recorded such access unless I comment out
>
> //*hold_quit = event->ie;
>
> access { base = (8089)'ev', offset = 0, size = 512, expr = ev, type = union buffered_input_event, reverse = 0, grp_read = 1, grp_write = 1, grp_assignment_read = 1, grp_assignment_write = 1, grp_scalar_read = 0, grp_scalar_write = 0, grp_total_scalarization = 0, grp_hint = 0, grp_covered = 0, grp_unscalarizable_region = 0, grp_unscalarized_data = 1, grp_same_access_path = 1, grp_partial_lhs = 0, grp_to_be_replaced = 0, grp_to_be_debug_replaced = 0}
GCC assumes that, being of the same size and having the same offset, the
access to `struct input_event' never aliases another access in the same
procedure with the same properties but a different type. Which type
prevails is contingent on the order in which they are defined. If an
incomplete declaration of `union buffered_input_event' is inserted
before `struct input_event', you will observe the access to `ev.ie'
disappearing in place of that to `ev' as a whole.
> So maybe it is time to ask the GCC folks for help here, even though we
> do not have a reduced reproducer for the problem (which may well be on
> our end) yet.)
We do now.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sun, 02 Mar 2025 02:15:02 GMT)
Full text and
rfc822 format available.
Message #128 received at 76559 <at> debbugs.gnu.org (full text, mbox):
On 2025-03-01 05:43, Po Lu wrote:
> I surmise that it doesn't fix the underlying compiler issue if you add
> __attribute__ ((force_inline)) to kbd_buffer_store_buffered_event, as it
> is a product of an assumption by GCC's SRA pass that no two accesses of
> a single local aggregate declaration with the same offset and size but
> of different types alias.
I don't see why that assumption would cause GCC to generate the wrong
code here, as no single thread of execution would have two such accesses.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sun, 02 Mar 2025 05:11:01 GMT)
Full text and
rfc822 format available.
Message #131 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Paul Eggert <eggert <at> cs.ucla.edu> writes:
> On 2025-03-01 05:43, Po Lu wrote:
>> I surmise that it doesn't fix the underlying compiler issue if you add
>> __attribute__ ((force_inline)) to kbd_buffer_store_buffered_event, as it
>> is a product of an assumption by GCC's SRA pass that no two accesses of
>> a single local aggregate declaration with the same offset and size but
>> of different types alias.
>
> I don't see why that assumption would cause GCC to generate the wrong
> code here, as no single thread of execution would have two such
> accesses.
It doesn't take control flow into account, but rather _every statement
operand_ in the current procedure that addresses an aggregate declared
in the same procedure. Just read perform_intra_sra in gcc/tree-sra.cc.
find_var_candidates, scan_function, analyze_all_variable_accesses, and
sra_modify_function_body, don't walk any manner of control dependence
graph, but all declarations and basic blocks defined in a function.
sort_and_splice_var_accesses excludes all but one access to a single
aggregate with the same offset and size. Since the existence of a
record reference thereby excludes the union, totally_scalarizable_type_p
concludes that the access admits of "total secularization", and
analyze_all_variable_accesses proceeds to scalarize this record
reference. Subsequently, sra_modify_assign observes that the access of
union access on the RHS of this statement (where the `ev' declaration in
evq_flush is substituted for `*event' by inlining):
*kbd_store_ptr = *event;
has the same size and offset as the access to `event->ie' (`ev.ie'),
deletes the statement, and substitutes a series of loads produced by the
scalarization of the access to `ev.ie', without any regard to the type
of the RHS. If it is arranged that `union buffered_input_event' be
declared before `struct input_event', the former will receive a lower
UID and a greater precedence with analyze_all_variable_accesses, and
exclude the loads of `struct input_event' from consideration, which
eliminates the problem in this specific instance because unions are not
totally scalarizable.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sun, 02 Mar 2025 06:55:02 GMT)
Full text and
rfc822 format available.
Message #134 received at 76559 <at> debbugs.gnu.org (full text, mbox):
On 2025-03-01 21:10, Po Lu wrote:
>> I don't see why that assumption would cause GCC to generate the wrong
>> code here, as no single thread of execution would have two such
>> accesses.
> It doesn't take control flow into account, but rather _every statement
> operand_ in the current procedure that addresses an aggregate declared
> in the same procedure.
In that case it's a GCC bug, right? Since GCC promises that one can do
union punning.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sun, 02 Mar 2025 08:09:01 GMT)
Full text and
rfc822 format available.
Message #137 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Paul Eggert <eggert <at> cs.ucla.edu> writes:
> On 2025-03-01 21:10, Po Lu wrote:
>>> I don't see why that assumption would cause GCC to generate the wrong
>>> code here, as no single thread of execution would have two such
>>> accesses.
>> It doesn't take control flow into account, but rather _every statement
>> operand_ in the current procedure that addresses an aggregate declared
>> in the same procedure.
>
> In that case it's a GCC bug, right? Since GCC promises that one can do
> union punning.
Yes, that's what I was getting at. I even proposed a fix. But the GCC
list has lately been infused with a series of threads proposing new
obstacles to engaging with its developers (search "Sourceware
infrastructure plans"), and I'm not interested...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sun, 02 Mar 2025 09:37:03 GMT)
Full text and
rfc822 format available.
Message #140 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Thank you for investigating.
GCC bugs are reported at https://gcc.gnu.org/bugzilla/, not via a
mailing list, and GCC patches go to the gcc-patches ML.
The gcc ML is for general discussion of development and so on (I also
don't think what you describe is a fair characterisation of the
discussions, but it's irrelevant is my point -- you needn't engage with
that mailing list at all).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sun, 02 Mar 2025 10:34:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 76559 <at> debbugs.gnu.org (full text, mbox):
R"Po Lu" <luangruo <at> yahoo.com> writes:
> Paul Eggert <eggert <at> cs.ucla.edu> writes:
>
>> On 2025-03-01 21:10, Po Lu wrote:
>>>> I don't see why that assumption would cause GCC to generate the wrong
>>>> code here, as no single thread of execution would have two such
>>>> accesses.
>>> It doesn't take control flow into account, but rather _every statement
>>> operand_ in the current procedure that addresses an aggregate declared
>>> in the same procedure.
>>
>> In that case it's a GCC bug, right? Since GCC promises that one can do
>> union punning.
>
> Yes, that's what I was getting at. I even proposed a fix. But the GCC
> list has lately been infused with a series of threads proposing new
> obstacles to engaging with its developers (search "Sourceware
> infrastructure plans"), and I'm not interested...
Are you going to report the GCC bug? If not, I'd like to do so, with
your permission.
As far as the problem itself is concerned, it's a minor oversight in the
SRA code which fails to account for holes when choosing a representative
union access out of a list of all places which access a given structure,
so the fix seems simple.
Working around the problem may be more difficult, and it might be
easiest to disable the SRA pass for bad GCC versions, but maybe
something will occur to the GCC folks.
Pip
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Sun, 02 Mar 2025 11:30:04 GMT)
Full text and
rfc822 format available.
Message #146 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Pip Cet <pipcet <at> protonmail.com> writes:
> Are you going to report the GCC bug? If not, I'd like to do so, with
> your permission.
By all means (and please don't even consider requesting my permission to
do so--Free Software means it is every person's birthright to take the
initiative into his own hands).
> Working around the problem may be more difficult, and it might be
> easiest to disable the SRA pass for bad GCC versions, but maybe
> something will occur to the GCC folks.
I had a very conservative workaround that disqualifies all accesses
where more than two incompatible aggregate types (all aggregates are
impacted, not just unions & records) are concerned are rejected, but
maybe it's too timid for their tastes or I omitted to post it in this
thread. In the meantime, let's apply `-fno-tree-sra' on GCC.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Mon, 03 Mar 2025 08:03:02 GMT)
Full text and
rfc822 format available.
Message #149 received at 76559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 2025-03-01 21:10, Po Lu wrote:
> It doesn't take control flow into account, but rather _every statement
> operand_ in the current procedure that addresses an aggregate declared
> in the same procedure.
Although, as you mentioned, we could work around this wrong-code GCC bug
by using -fno-tree-sra (something we already do for GCC pre-SSE2 x86),
we should avoid the tricky union locals that trigger the bug on x86-64
with gcc -flto. After all, these union locals make the code harder for
humans to read, so removing them is a win regardless of the GCC bug. And
if removing the union locals happens to work around the GCC bug then so
much the better.
Proposed patch attached.
[0001-Avoid-some-union-buffered_input_event-uses.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Mon, 03 Mar 2025 09:49:01 GMT)
Full text and
rfc822 format available.
Message #152 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Paul Eggert <eggert <at> cs.ucla.edu> writes:
> On 2025-03-01 21:10, Po Lu wrote:
>> It doesn't take control flow into account, but rather _every statement
>> operand_ in the current procedure that addresses an aggregate declared
>> in the same procedure.
>
> Although, as you mentioned, we could work around this wrong-code GCC
> bug by using -fno-tree-sra (something we already do for GCC pre-SSE2
> x86), we should avoid the tricky union locals that trigger the bug on
> x86-64 with gcc -flto. After all, these union locals make the code
> harder for humans to read, so removing them is a win regardless of the
> GCC bug. And if removing the union locals happens to work around the
> GCC bug then so much the better.
[...]
> +/* Store EVENT obtained at interrupt level into kbd_buffer, fifo.
> + This is like kbd_buffer_store_event_hold, but for struct
> + selection_input_event instead of struct input_event.
> +
> + If HOLD_QUIT && HOLD_QUIT->kind != NO_EVENT, discard EVENT.
> +
> + This is used to postpone the processing of the quit event until all
> + subsequent input events have been parsed (and discarded). */
> +
> +void
> +kbd_buffer_store_selection_event_hold
> + (struct selection_input_event const *event,
> + struct input_event const *hold_quit)
The patch is fine by me, but the line wrapping hereabouts is gruesome.
Please delete the const qualifiers and wrap the parameter list after
`event', since they probably will not assist code generation.
Also, come to think of it, I doubt it's appropriate for selection event
delivery (which is meant purely to be a transparent process to users) to
be influenced by hold_quit at all.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Mon, 03 Mar 2025 14:48:02 GMT)
Full text and
rfc822 format available.
Message #155 received at 76559 <at> debbugs.gnu.org (full text, mbox):
Po Lu <luangruo <at> yahoo.com> writes:
> Paul Eggert <eggert <at> cs.ucla.edu> writes:
>
>> On 2025-03-01 21:10, Po Lu wrote:
>>> It doesn't take control flow into account, but rather _every statement
>>> operand_ in the current procedure that addresses an aggregate declared
>>> in the same procedure.
>>
>> Although, as you mentioned, we could work around this wrong-code GCC
>> bug by using -fno-tree-sra (something we already do for GCC pre-SSE2
>> x86), we should avoid the tricky union locals that trigger the bug on
>> x86-64 with gcc -flto. After all, these union locals make the code
>> harder for humans to read, so removing them is a win regardless of the
>> GCC bug. And if removing the union locals happens to work around the
>> GCC bug then so much the better.
>
> [...]
>
>> +/* Store EVENT obtained at interrupt level into kbd_buffer, fifo.
>> + This is like kbd_buffer_store_event_hold, but for struct
>> + selection_input_event instead of struct input_event.
>> +
>> + If HOLD_QUIT && HOLD_QUIT->kind != NO_EVENT, discard EVENT.
>> +
>> + This is used to postpone the processing of the quit event until all
>> + subsequent input events have been parsed (and discarded). */
>> +
>> +void
>> +kbd_buffer_store_selection_event_hold
>> + (struct selection_input_event const *event,
>> + struct input_event const *hold_quit)
>
> The patch is fine by me, but the line wrapping hereabouts is gruesome.
> Please delete the const qualifiers and wrap the parameter list after
> `event', since they probably will not assist code generation.
>
> Also, come to think of it, I doubt it's appropriate for selection event
> delivery (which is meant purely to be a transparent process to users) to
> be influenced by hold_quit at all.
Which reminds me: what is to be done on the release branch, if any?
Beyond directing users not to enable "-flto", which goes without saying.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Mon, 03 Mar 2025 15:36:01 GMT)
Full text and
rfc822 format available.
Message #158 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> From: Po Lu <luangruo <at> yahoo.com>
> Cc: Pip Cet <pipcet <at> protonmail.com>, iura.mail <at> gmail.com, Stefan Kangas
> <stefankangas <at> gmail.com>, michael.albinus <at> gmx.de, Eli Zaretskii
> <eliz <at> gnu.org>, 76559 <at> debbugs.gnu.org, marian.iurie <at> gmail.com
> Date: Mon, 03 Mar 2025 22:46:42 +0800
>
> Which reminds me: what is to be done on the release branch, if any?
> Beyond directing users not to enable "-flto", which goes without saying.
Something simple and localized, if possible? I believe one of the
previously-suggested patches was like that, even if it was not 110%
correct or safe.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Mon, 03 Mar 2025 17:58:01 GMT)
Full text and
rfc822 format available.
Message #161 received at 76559 <at> debbugs.gnu.org (full text, mbox):
On 2025-03-03 01:47, Po Lu wrote:
> The patch is fine by me, but the line wrapping hereabouts is gruesome.
> Please delete the const qualifiers and wrap the parameter list after
> `event', since they probably will not assist code generation.
Thanks for the review. I installed it with those changes.
> Also, come to think of it, I doubt it's appropriate for selection event
> delivery (which is meant purely to be a transparent process to users) to
> be influenced by hold_quit at all.
Yeah, I noticed that too. Someone with more time and expertise than I
should probably take a look at it. Maybe the intent is to make quits
work even when a large selection is arriving, but I doubt whether the
code is doing that in all cases, and it could well be I'm
misunderstanding the intent.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Mon, 03 Mar 2025 18:10:02 GMT)
Full text and
rfc822 format available.
Message #164 received at 76559 <at> debbugs.gnu.org (full text, mbox):
On 2025-03-03 07:34, Eli Zaretskii wrote:
>> From: Po Lu <luangruo <at> yahoo.com>
>> Date: Mon, 03 Mar 2025 22:46:42 +0800
>> Which reminds me: what is to be done on the release branch, if any?
>> Beyond directing users not to enable "-flto", which goes without saying.
>
> Something simple and localized, if possible? I believe one of the
> previously-suggested patches was like that, even if it was not 110%
> correct or safe.
The patch in <https://bugs.gnu.org/76559#71> won't work if neither
HAVE_X11 nor HAVE_PGTK is defined. There's an obvious fix for that if
somebody wants to take up the banner of writing a patch not to be merged
to master. However, I hope the "don't use -O3" advice is enough.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#76559; Package
emacs.
(Mon, 03 Mar 2025 19:41:02 GMT)
Full text and
rfc822 format available.
Message #167 received at 76559 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 3 Mar 2025 10:09:45 -0800
> Cc: pipcet <at> protonmail.com, iura.mail <at> gmail.com, stefankangas <at> gmail.com,
> michael.albinus <at> gmx.de, Po Lu <luangruo <at> yahoo.com>, 76559 <at> debbugs.gnu.org,
> marian.iurie <at> gmail.com
> From: Paul Eggert <eggert <at> cs.ucla.edu>
>
> On 2025-03-03 07:34, Eli Zaretskii wrote:
> >> From: Po Lu <luangruo <at> yahoo.com>
> >> Date: Mon, 03 Mar 2025 22:46:42 +0800
> >> Which reminds me: what is to be done on the release branch, if any?
> >> Beyond directing users not to enable "-flto", which goes without saying.
> >
> > Something simple and localized, if possible? I believe one of the
> > previously-suggested patches was like that, even if it was not 110%
> > correct or safe.
>
> The patch in <https://bugs.gnu.org/76559#71> won't work if neither
> HAVE_X11 nor HAVE_PGTK is defined.
That didn't work on master, either, and I just fixed it, because it
broke the MinGW build.
> There's an obvious fix for that if somebody wants to take up the
> banner of writing a patch not to be merged to master. However, I
> hope the "don't use -O3" advice is enough.
OK.
Merged 76559 76729.
Request was from
Paul Eggert <eggert <at> cs.ucla.edu>
to
control <at> debbugs.gnu.org.
(Tue, 04 Mar 2025 06:37:02 GMT)
Full text and
rfc822 format available.
This bug report was last modified 31 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.