GNU bug report logs - #75632
31.0.50; igc: Crash report

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 75632 in the body.
You can then email your comments to 75632 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 14:36:36 +0000

Just got the following:


lockix.c:126: Emacs fatal error: assertion failed: res == 0
[Switching to Thread 0x7ffff2b71e00 (LWP 11068)]

Thread 1 "emacs" hit Breakpoint 1, terminate_due_to_signal (sig=sig <at> entry=6, backtrace_limit=backtrace_limit <at> entry=2147483647) at emacs.c:432
432	{
(gdb) bt
#0  terminate_due_to_signal (sig=sig <at> entry=6, backtrace_limit=backtrace_limit <at> entry=2147483647) at emacs.c:432
#1  0x00005555557e24fb in set_state (state=IGC_STATE_DEAD) at igc.c:975
#2  igc_assert_fail (file=<optimized out>, line=<optimized out>, msg=<optimized out>) at igc.c:276
#3  0x000055555585f049 in mps_lib_assert_fail (condition=0x5555558c0763 "res == 0", line=126, file=0x5555558c074d "lockix.c") at /home/yantar92/Dist/mps/code/mpsliban.c:87
#4  LockClaim (lock=0x7fffe8000110) at /home/yantar92/Dist/mps/code/lockix.c:126
#5  0x000055555585f27d in ArenaEnterLock (arena=0x7ffff7fbe000, recursive=0) at /home/yantar92/Dist/mps/code/global.c:576
#6  0x000055555588812e in ArenaEnter (arena=0x7ffff7fbe000) at /home/yantar92/Dist/mps/code/global.c:553
#7  ArenaAccess (addr=0x7fff9d8709e0, mode=mode <at> entry=3, context=context <at> entry=0x7fffffff4650) at /home/yantar92/Dist/mps/code/global.c:655
#8  0x0000555555893432 in sigHandle (sig=<optimized out>, info=0x7fffffff4970, uap=0x7fffffff4840) at /home/yantar92/Dist/mps/code/protsgix.c:97
#9  0x00007ffff2c41100 in <signal handler called> () at /lib64/libc.so.6
#10 0x00005555556d2273 in SDATA (string=<optimized out>) at /home/yantar92/Git/emacs/src/lisp.h:1758
#11 SSDATA (string=<optimized out>) at /home/yantar92/Git/emacs/src/lisp.h:1764
#12 handle_user_signal (sig=sig <at> entry=12) at keyboard.c:8315
#13 0x00005555556f1678 in deliver_process_signal (sig=12, handler=handler <at> entry=0x5555556d2210 <handle_user_signal>) at sysdep.c:1757
#14 0x00005555556d1134 in deliver_user_signal (sig=<optimized out>) at keyboard.c:8352
#15 0x00007ffff2c41100 in <signal handler called> () at /lib64/libc.so.6
#16 0x00007ffff2d189ab in mprotect () at /lib64/libc.so.6
#17 0x000055555588183d in ProtSet (base=0x7ffe4c324000, limit=<optimized out>, mode=0) at /home/yantar92/Dist/mps/code/protix.c:105
#18 0x000055555588b9dc in traceScanSegRes (ts=ts <at> entry=1, rank=rank <at> entry=1, arena=arena <at> entry=0x7ffff7fbe000, seg=seg <at> entry=0x7ffe4803e5f8) at /home/yantar92/Dist/mps/code/trace.c:1204
#19 0x000055555588bbfa in traceScanSeg (ts=1, rank=1, arena=0x7ffff7fbe000, seg=0x7ffe4803e5f8) at /home/yantar92/Dist/mps/code/trace.c:1267
#20 0x000055555588c5d4 in TraceAdvance (trace=trace <at> entry=0x7ffff7fbeaa8) at /home/yantar92/Dist/mps/code/trace.c:1728
#21 0x000055555588ccd4 in TracePoll
    (workReturn=workReturn <at> entry=0x7fffffff58d0, collectWorldReturn=collectWorldReturn <at> entry=0x7fffffff58cc, globals=globals <at> entry=0x7ffff7fbe008, collectWorldAllowed=<optimized out>)
    at /home/yantar92/Dist/mps/code/trace.c:1849
#22 0x000055555588cf1b in ArenaPoll (globals=globals <at> entry=0x7ffff7fbe008) at /home/yantar92/Dist/mps/code/global.c:745
#23 0x000055555588d30a in mps_ap_fill (p_o=p_o <at> entry=0x7fffffff5a40, mps_ap=mps_ap <at> entry=0x7fffe8001980, size=size <at> entry=24) at /home/yantar92/Dist/mps/code/mpsi.c:1097
#24 0x00005555557ddf28 in alloc_impl (size=24, type=IGC_OBJ_CONS, ap=0x7fffe8001980) at igc.c:3956
#25 0x00005555557de04c in alloc (size=size <at> entry=24, type=type <at> entry=IGC_OBJ_CONS) at igc.c:3984
#26 0x00005555557e0964 in igc_make_cons (car=car <at> entry=XIL(0x7ffefc1c18a5), cdr=XIL(0x7ffdf8565feb)) at igc.c:4013
#27 0x0000555555734ee8 in Fcons (car=car <at> entry=XIL(0x7ffefc1c18a5), cdr=<optimized out>) at alloc.c:2958
#28 0x0000555555752a84 in save_restriction_save () at editfns.c:3097
#29 0x00005555557a3267 in helper_save_restriction () at comp.c:5128
#30 0x00007fffdc3efc22 in F6f72672d666f6c642d636f72652d6765742d666f6c64696e672d73706563_org_fold_core_get_folding_spec_0 ()
    at /home/yantar92/.emacs.d/eln-cache/31.0.50-fc0e2b3f/org-fold-core-7b3a75f5-931c108c.eln
#31 0x0000555555758fb3 in funcall_subr (subr=<optimized out>, numargs=numargs <at> entry=2, args=args <at> entry=0x7fffdedff0c0) at eval.c:3178
#32 0x000055555579fdaf in exec_byte_code (fun=<optimized out>, args_template=<optimized out>, args_template <at> entry=257, nargs=<optimized out>, nargs <at> entry=1, args=<optimized out>, args <at> entry=0x7fffffff5e68)
    at /home/yantar92/Git/emacs/src/lisp.h:2332
#33 0x000055555575ab86 in funcall_lambda (fun=XIL(0x7fff9ebde0b5), nargs=nargs <at> entry=1, arg_vector=arg_vector <at> entry=0x7fffffff5e68) at eval.c:3267
#34 0x000055555575af3b in funcall_general (fun=<optimized out>, numargs=numargs <at> entry=1, args=args <at> entry=0x7fffffff5e68) at eval.c:3059
#35 0x0000555555757198 in Ffuncall (nargs=2, args=0x7fffffff5e60) at eval.c:3108
#36 0x00007fffdf8c6a6f in F666f6e742d6c6f636b2d666f6e746966792d6b6579776f7264732d726567696f6e_font_lock_fontify_keywords_region_0 ()
    at /home/yantar92/Git/emacs/src/../native-lisp/31.0.50-fc0e2b3f/preloaded/font-lock-895216f6-4021c0ad.eln
#37 0x0000555555758fca in funcall_subr (subr=<optimized out>, numargs=numargs <at> entry=3, args=args <at> entry=0x7fffffff6188) at eval.c:3180
#38 0x000055555575b08a in funcall_general (fun=<optimized out>, numargs=numargs <at> entry=3, args=args <at> entry=0x7fffffff6188) at /home/yantar92/Git/emacs/src/lisp.h:2332
#39 0x0000555555757198 in Ffuncall (nargs=4, args=0x7fffffff6180) at eval.c:3108
#40 0x00007fffdf8c2740 in F666f6e742d6c6f636b2d64656661756c742d666f6e746966792d726567696f6e_font_lock_default_fontify_region_0 ()
    at /home/yantar92/Git/emacs/src/../native-lisp/31.0.50-fc0e2b3f/preloaded/font-lock-895216f6-4021c0ad.eln
#41 0x0000555555758fca in funcall_subr (subr=<optimized out>, numargs=numargs <at> entry=3, args=args <at> entry=0x7fffffff6308) at eval.c:3180
#42 0x000055555575b08a in funcall_general (fun=<optimized out>, numargs=numargs <at> entry=3, args=args <at> entry=0x7fffffff6308) at /home/yantar92/Git/emacs/src/lisp.h:2332
#43 0x0000555555757198 in Ffuncall (nargs=4, args=0x7fffffff6300) at eval.c:3108
#44 0x00007fffdf8c1735 in F666f6e742d6c6f636b2d666f6e746966792d726567696f6e_font_lock_fontify_region_0 ()
    at /home/yantar92/Git/emacs/src/../native-lisp/31.0.50-fc0e2b3f/preloaded/font-lock-895216f6-4021c0ad.eln
#45 0x0000555555758fca in funcall_subr (subr=<optimized out>, numargs=numargs <at> entry=2, args=args <at> entry=0x7fffdedff040) at eval.c:3180
#46 0x000055555579fdaf in exec_byte_code (fun=<optimized out>, args_template=<optimized out>, args_template <at> entry=257, nargs=<optimized out>, nargs <at> entry=1, args=<optimized out>, args <at> entry=0x7fffffff6608)
    at /home/yantar92/Git/emacs/src/lisp.h:2332
#47 0x000055555575ab86 in funcall_lambda (fun=XIL(0x7ffdf852f91d), nargs=nargs <at> entry=1, arg_vector=arg_vector <at> entry=0x7fffffff6608) at eval.c:3267
#48 0x000055555575af3b in funcall_general (fun=<optimized out>, numargs=numargs <at> entry=1, args=args <at> entry=0x7fffffff6608) at eval.c:3059
--Type <RET> for more, q to quit, c to continue without paging--c
#49 0x0000555555757198 in Ffuncall (nargs=2, args=args <at> entry=0x7fffffff6600) at eval.c:3108
#50 0x000055555575777d in run_hook_wrapped_funcall (nargs=<optimized out>, args=0x7fffffff6600) at eval.c:2887
#51 0x00005555557562b9 in run_hook_with_args (nargs=2, args=0x7fffffff6600, funcall=funcall <at> entry=0x55555575775c <run_hook_wrapped_funcall>) at eval.c:2968
#52 0x000055555575648e in Frun_hook_wrapped (nargs=<optimized out>, args=<optimized out>) at eval.c:2902
#53 0x00007fffdf89fafa in F6a69742d6c6f636b2d2d72756e2d66756e6374696f6e73_jit_lock__run_functions_0 () at /home/yantar92/Git/emacs/src/../native-lisp/31.0.50-fc0e2b3f/preloaded/jit-lock-8a988e43-86e09700.eln
#54 0x0000555555758fb3 in funcall_subr (subr=<optimized out>, numargs=numargs <at> entry=2, args=args <at> entry=0x7fffffff67e8) at eval.c:3178
#55 0x000055555575b08a in funcall_general (fun=<optimized out>, numargs=numargs <at> entry=2, args=args <at> entry=0x7fffffff67e8) at /home/yantar92/Git/emacs/src/lisp.h:2332
#56 0x0000555555757198 in Ffuncall (nargs=3, args=0x7fffffff67e0) at eval.c:3108
#57 0x00007fffdf8a03aa in F6a69742d6c6f636b2d666f6e746966792d6e6f77_jit_lock_fontify_now_0 () at /home/yantar92/Git/emacs/src/../native-lisp/31.0.50-fc0e2b3f/preloaded/jit-lock-8a988e43-86e09700.eln
#58 0x0000555555758fb3 in funcall_subr (subr=<optimized out>, numargs=numargs <at> entry=2, args=args <at> entry=0x7fffffff69a8) at eval.c:3178
#59 0x000055555575b08a in funcall_general (fun=<optimized out>, numargs=numargs <at> entry=2, args=args <at> entry=0x7fffffff69a8) at /home/yantar92/Git/emacs/src/lisp.h:2332
#60 0x0000555555757198 in Ffuncall (nargs=3, args=0x7fffffff69a0) at eval.c:3108
#61 0x00007fffdf89f837 in F6a69742d6c6f636b2d66756e6374696f6e_jit_lock_function_0 () at /home/yantar92/Git/emacs/src/../native-lisp/31.0.50-fc0e2b3f/preloaded/jit-lock-8a988e43-86e09700.eln
#62 0x0000555555758fa0 in funcall_subr (subr=<optimized out>, numargs=numargs <at> entry=1, args=args <at> entry=0x7fffffff6b68) at eval.c:3176
#63 0x000055555575b08a in funcall_general (fun=<optimized out>, numargs=numargs <at> entry=1, args=args <at> entry=0x7fffffff6b68) at /home/yantar92/Git/emacs/src/lisp.h:2332
#64 0x0000555555757198 in Ffuncall (nargs=nargs <at> entry=2, args=args <at> entry=0x7fffffff6b60) at eval.c:3108
#65 0x0000555555755c66 in internal_condition_case_n
    (bfun=bfun <at> entry=0x55555575708c <Ffuncall>, nargs=nargs <at> entry=2, args=args <at> entry=0x7fffffff6b60, handlers=handlers <at> entry=XIL(0x38), hfun=hfun <at> entry=0x5555555d2ae3 <dsafe_eval_handler>) at eval.c:1707
#66 0x00005555555be7d5 in dsafe__call (inhibit_quit=inhibit_quit <at> entry=false, f=0x55555575708c <Ffuncall>, nargs=nargs <at> entry=2, args=args <at> entry=0x7fffffff6b60) at xdisp.c:3095
#67 0x00005555555be84a in dsafe_call1 (f=<optimized out>, arg=arg <at> entry=make_fixnum(12548850)) at xdisp.c:3125
#68 0x00005555555d1120 in handle_fontified_prop (it=<optimized out>) at xdisp.c:4644
#69 0x00005555555d4c6e in handle_stop (it=it <at> entry=0x7fffffff9500) at xdisp.c:4164
#70 0x00005555555e3318 in next_element_from_buffer (it=0x7fffffff9500) at xdisp.c:9752
#71 0x00005555555e1aad in get_next_display_element (it=it <at> entry=0x7fffffff9500) at xdisp.c:8308
#72 0x00005555555e2a70 in forward_to_next_line_start (it=0x7fffffff9500, skipped_p=skipped_p <at> entry=0x7fffffff770f, bidi_it_prev=bidi_it_prev <at> entry=0x0) at xdisp.c:7649
#73 0x00005555555e2f8a in reseat_at_next_visible_line_start (it=it <at> entry=0x7fffffff9500, on_newline_p=on_newline_p <at> entry=false) at xdisp.c:7780
#74 0x00005555555e07c4 in move_it_to (it=it <at> entry=0x7fffffff9500, to_charpos=5143293, to_x=to_x <at> entry=0, to_y=<optimized out>, to_vpos=to_vpos <at> entry=-1, op=op <at> entry=11) at xdisp.c:10964
#75 0x0000555555608676 in redisplay_window (window=XIL(0x7ffe9eccdd4d), just_this_one_p=just_this_one_p <at> entry=false) at xdisp.c:20258
#76 0x000055555560c4f3 in redisplay_window_0 (window=window <at> entry=XIL(0x7ffe9eccdd4d)) at xdisp.c:18111
#77 0x0000555555755b5e in internal_condition_case_1
    (bfun=bfun <at> entry=0x55555560c4c0 <redisplay_window_0>, arg=arg <at> entry=XIL(0x7ffe9eccdd4d), handlers=<optimized out>, hfun=hfun <at> entry=0x5555555c24fd <redisplay_window_error>) at eval.c:1651
#78 0x00005555555bfeee in redisplay_windows (window=XIL(0x7ffe9eccdd4d)) at xdisp.c:18080
#79 0x00005555555bfe9c in redisplay_windows (window=XIL(0x7ffdf43cbaed)) at xdisp.c:18074
#80 0x00005555555f2be4 in redisplay_internal () at xdisp.c:17497
#81 0x00005555555f3f3e in resize_echo_area_exactly () at xdisp.c:13017
#82 0x00005555556e4c46 in command_loop_1 () at keyboard.c:1584
#83 0x0000555555755ae6 in internal_condition_case (bfun=bfun <at> entry=0x5555556e4905 <command_loop_1>, handlers=handlers <at> entry=XIL(0xa8), hfun=hfun <at> entry=0x5555556d5870 <cmd_error>) at eval.c:1627
#84 0x00005555556d0687 in command_loop_2 (handlers=handlers <at> entry=XIL(0xa8)) at keyboard.c:1174
#85 0x0000555555755a21 in internal_catch (tag=tag <at> entry=XIL(0x153f0), func=func <at> entry=0x5555556d0657 <command_loop_2>, arg=arg <at> entry=XIL(0xa8)) at eval.c:1306
#86 0x00005555556d0634 in command_loop () at keyboard.c:1152
#87 0x00005555556d53e3 in recursive_edit_1 () at keyboard.c:760
#88 0x00005555556d578b in Frecursive_edit () at keyboard.c:843
#89 0x00005555556cf8ab in main (argc=1, argv=0x7fffffffd5d8) at emacs.c:2658

Lisp Backtrace:
"org-fold-core-get-folding-spec" (0xdedff0c0)
"org-activate-folds" (0xffff5e68)
"font-lock-fontify-keywords-region" (0xffff6188)
"font-lock-default-fontify-region" (0xffff6308)
"font-lock-fontify-region" (0xdedff040)
0xf852f918 PVEC_CLOSURE
"jit-lock--run-functions" (0xffff67e8)
"jit-lock-fontify-now" (0xffff69a8)
"jit-lock-function" (0xffff6b68)
"redisplay_internal (C function)" (0x0)
(gdb) 
(gdb) q

In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.42, cairo version 1.18.2) of 2025-01-12 built on localhost
Repository revision: b82707d70fd5bb78be5766e247e9629eb6553c30
Repository branch: scratch/igc
Windowing system distributor 'The X.Org Foundation', version 11.0.12101014
System Description: Gentoo Linux

Configured using:
 'configure --with-mps=yes --with-native-compilation 'CFLAGS=-g3
 -I/opt/mps/include -L/opt/mps/lib'
 JAVAC=/etc/java-config-2/current-system-vm/bin/javac
 PKG_CONFIG_PATH=/usr/share/guile-data/3.0/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LCMS2 LIBXML2 MODULES MPS NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM
XINPUT2 XPM GTK3 ZLIB

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 14:40:54 +0000

"Ihor Radchenko" <yantar92 <at> posteo.net> writes:

> Just got the following:

Yes, that's the signal handling bug.  I'm not entirely sure why we
removed the fix that was in scratch/igc, but we did, so we need another
one.

For a quick fix, you can add

if (igc_busy_p ()) return;

to handle_user_signal, which will silently ignore SIGUSR* received while
MPS may have locked the arena.

Pip

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 14:46:35 +0000

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

> "Ihor Radchenko" <yantar92 <at> posteo.net> writes:
>
>> Just got the following:
>
> Yes, that's the signal handling bug.  I'm not entirely sure why we
> removed the fix that was in scratch/igc, but we did, so we need another
> one.
> ...
> to handle_user_signal, which will silently ignore SIGUSR* received while
> MPS may have locked the arena.

I indeed sent SIGUSR2 just before I saw the crash.

For some context (maybe irrelevant), Emacs hung while performing magit
commit (C-c C-c in magit commit buffer; and the commit was actually
written before the hang). It only ever happened for me on igc branch.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Message #16 received at 75632-done <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 75632-done <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 15:26:10 +0000

"Ihor Radchenko" <yantar92 <at> posteo.net> writes:

> Pip Cet <pipcet <at> protonmail.com> writes:
>
>> "Ihor Radchenko" <yantar92 <at> posteo.net> writes:
>>
>>> Just got the following:
>>
>> Yes, that's the signal handling bug.  I'm not entirely sure why we
>> removed the fix that was in scratch/igc, but we did, so we need another
>> one.
>> ...
>> to handle_user_signal, which will silently ignore SIGUSR* received while
>> MPS may have locked the arena.
>
> I indeed sent SIGUSR2 just before I saw the crash.

It'd be nice if that simply worked.  I think debugging with SIGUSR* is
important, so I've pushed a fix (and I'm closing this bug; if further
discussion is needed, feel free to revert and reopen).

> For some context (maybe irrelevant), Emacs hung while performing magit
> commit (C-c C-c in magit commit buffer; and the commit was actually
> written before the hang). It only ever happened for me on igc branch.

Thanks!  Too bad we lost that backtrace, then.  If it happens again,
please let us know!

(I'm using magit, and I know it waits for subprocesses, so I'll go over
the SIGCHLD handling code to ensure we never drop one of them).

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 18:48:21 +0200

> From: Ihor Radchenko <yantar92 <at> posteo.net>
> Date: Fri, 17 Jan 2025 14:36:36 +0000
> 
> Just got the following:
> 
> 
> lockix.c:126: Emacs fatal error: assertion failed: res == 0
> [Switching to Thread 0x7ffff2b71e00 (LWP 11068)]
> 
> Thread 1 "emacs" hit Breakpoint 1, terminate_due_to_signal (sig=sig <at> entry=6, backtrace_limit=backtrace_limit <at> entry=2147483647) at emacs.c:432
> 432	{
> (gdb) bt
> #0  terminate_due_to_signal (sig=sig <at> entry=6, backtrace_limit=backtrace_limit <at> entry=2147483647) at emacs.c:432
> #1  0x00005555557e24fb in set_state (state=IGC_STATE_DEAD) at igc.c:975
> #2  igc_assert_fail (file=<optimized out>, line=<optimized out>, msg=<optimized out>) at igc.c:276
> #3  0x000055555585f049 in mps_lib_assert_fail (condition=0x5555558c0763 "res == 0", line=126, file=0x5555558c074d "lockix.c") at /home/yantar92/Dist/mps/code/mpsliban.c:87
> #4  LockClaim (lock=0x7fffe8000110) at /home/yantar92/Dist/mps/code/lockix.c:126
> #5  0x000055555585f27d in ArenaEnterLock (arena=0x7ffff7fbe000, recursive=0) at /home/yantar92/Dist/mps/code/global.c:576
> #6  0x000055555588812e in ArenaEnter (arena=0x7ffff7fbe000) at /home/yantar92/Dist/mps/code/global.c:553
> #7  ArenaAccess (addr=0x7fff9d8709e0, mode=mode <at> entry=3, context=context <at> entry=0x7fffffff4650) at /home/yantar92/Dist/mps/code/global.c:655
> #8  0x0000555555893432 in sigHandle (sig=<optimized out>, info=0x7fffffff4970, uap=0x7fffffff4840) at /home/yantar92/Dist/mps/code/protsgix.c:97
> #9  0x00007ffff2c41100 in <signal handler called> () at /lib64/libc.so.6
> #10 0x00005555556d2273 in SDATA (string=<optimized out>) at /home/yantar92/Git/emacs/src/lisp.h:1758
> #11 SSDATA (string=<optimized out>) at /home/yantar92/Git/emacs/src/lisp.h:1764
> #12 handle_user_signal (sig=sig <at> entry=12) at keyboard.c:8315
> #13 0x00005555556f1678 in deliver_process_signal (sig=12, handler=handler <at> entry=0x5555556d2210 <handle_user_signal>) at sysdep.c:1757
> #14 0x00005555556d1134 in deliver_user_signal (sig=<optimized out>) at keyboard.c:8352

Looks like Emacs got SIGUSR1 or SIGUSR2?  How come?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 22:31:48 +0200

> Resent-To: bug-gnu-emacs <at> gnu.org
> Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
>  75632-done <at> debbugs.gnu.org
> Date: Fri, 17 Jan 2025 15:26:10 +0000
> From:  Pip Cet via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> "Ihor Radchenko" <yantar92 <at> posteo.net> writes:
> 
> > Pip Cet <pipcet <at> protonmail.com> writes:
> >
> >> "Ihor Radchenko" <yantar92 <at> posteo.net> writes:
> >>
> >>> Just got the following:
> >>
> >> Yes, that's the signal handling bug.  I'm not entirely sure why we
> >> removed the fix that was in scratch/igc, but we did, so we need another
> >> one.
> >> ...
> >> to handle_user_signal, which will silently ignore SIGUSR* received while
> >> MPS may have locked the arena.
> >
> > I indeed sent SIGUSR2 just before I saw the crash.
> 
> It'd be nice if that simply worked.  I think debugging with SIGUSR* is
> important, so I've pushed a fix (and I'm closing this bug; if further
> discussion is needed, feel free to revert and reopen).

This breaks the MS-Windows build: temacs crashes as below:

  Loading loadup.el (source)...
  Dump mode: pbootstrap
  Using load-path (d:/gnu/git/emacs/feature/lisp d:/gnu/git/emacs/feature/lisp/emacs-lisp d:/gnu/git/emacs/feature/lisp/progmodes d:/gnu/git/emacs/feature/lisp/language d:/gnu/git/emacs/feature/lisp/international d:/gnu/git/emacs/feature/lisp/textmodes d:/gnu/git/emacs/feature/lisp/vc)
  Loading emacs-lisp/debug-early...
  Loading emacs-lisp/byte-run...
  Loading emacs-lisp/backquote...
  Loading subr...
  Loading keymap...
  Loading version...
  Loading widget...
  Loading custom...
  Loading emacs-lisp/map-ynp...
  Loading international/mule...
  Loading international/mule-conf...
  Loading env...
  Loading format...
  Loading bindings...
  Loading window...
  Loading files...
  Loading emacs-lisp/macroexp...
  Loading cus-face...
  Loading faces...
  Loading loaddefs...
  Loading d:/gnu/git/emacs/feature/lisp/theme-loaddefs.el (source)...
  Loading button...
  Loading emacs-lisp/cl-preloaded...
  Loading emacs-lisp/oclosure...
  Loading obarray...
  Loading abbrev...
  Loading help...
  Loading jka-cmpr-hook...
  Loading epa-hook...
  Loading international/mule-cmds...
  Loading case-table...
  Loading d:/gnu/git/emacs/feature/lisp/international/charprop.el (source)...
  Loading international/characters...
  Loading international/charscript...
  Loading international/emoji-zwj...
  Loading composite...
  Loading language/chinese...
  Loading language/cyrillic...
  Loading language/indian...
  Loading language/sinhala...
  Loading language/english...
  Loading language/ethiopic...
  Loading language/european...
  Loading language/czech...
  Loading language/slovak...
  Loading language/romanian...
  Loading language/greek...
  Loading language/hebrew...
  Loading international/cp51932...
  Loading international/eucjp-ms...
  Loading language/japanese...
  Loading language/korean...
  Loading language/lao...
  Loading language/tai-viet...
  Loading language/thai...
  Loading language/tibetan...
  Loading language/vietnamese...
  Loading language/misc-lang...
  Loading language/utf-8-lang...
  Loading language/georgian...
  Loading language/khmer...
  Loading language/burmese...
  Loading language/cham...
  Loading language/philippine...
  Loading language/indonesian...
  Loading indent...
  Loading emacs-lisp/cl-generic...
  Loading simple...
  Loading emacs-lisp/seq...
  Loading emacs-lisp/nadvice...
  Loading minibuffer...
  Loading frame...
  Loading startup...
  Loading term/tty-colors...
  Loading font-core...
  Loading emacs-lisp/syntax...
  Loading font-lock...
  Loading jit-lock...
  Loading mouse...
  Loading scroll-bar...
  Loading select...
  Loading emacs-lisp/timer...
  Loading emacs-lisp/easymenu...
  Loading isearch...
  Loading rfn-eshadow...
  Loading menu-bar...
  Loading tab-bar...
  Loading emacs-lisp/lisp...
  Loading textmodes/page...
  Loading register...
  Loading textmodes/paragraphs...
  Loading progmodes/prog-mode...
  Loading emacs-lisp/lisp-mode...
  Loading textmodes/text-mode...
  Loading textmodes/fill...
  Loading newcomment...
  Loading replace...
  Loading emacs-lisp/tabulated-list...
  Loading buff-menu...
  Loading fringe...
  Loading emacs-lisp/regexp-opt...
  Loading image...
  Loading international/fontset...
  Loading dnd...
  Loading tool-bar...
  Loading term/common-win...
  Loading w32-vars...
  Loading term/w32-win...
  Loading disp-table...
  Loading w32-fns...
  Loading ls-lisp...
  Loading dos-w32...
  Loading touch-screen...
  Loading mwheel...
  Loading progmodes/elisp-mode...
  Loading emacs-lisp/float-sup...
  Loading vc/vc-hooks...
  Loading vc/ediff-hook...
  Loading uniquify...
  Loading electric...
  Loading paren...
  Loading emacs-lisp/shorthands...
  Loading emacs-lisp/eldoc...
  Loading emacs-lisp/cconv...
  Loading cus-start...
  Loading tooltip...
  Loading international/iso-transl...
  Loading emacs-lisp/rmc...
  Loading d:/gnu/git/emacs/feature/lisp/leim/leim-list.el (source)...
  Finding pointers to doc strings...
  Finding pointers to doc strings...done
  Dumping under the name bootstrap-emacs.pdmp
  Dumping fingerprint: 18388262a2082a9a0cae7f9f7b2f954b596ec03f060bf8e73cbd63063b9a1d95

  pdumper.c:939: Emacs fatal error: assertion failed: should_end >= end

  Thread 1 hit Breakpoint 1, terminate_due_to_signal (sig=sig <at> entry=22,
      backtrace_limit=backtrace_limit <at> entry=2147483647) at emacs.c:432
  432     {
  (gdb) bt
  #0  terminate_due_to_signal (sig=sig <at> entry=22,
      backtrace_limit=backtrace_limit <at> entry=2147483647) at emacs.c:432
  #1  0x00f836f4 in die (
      msg=msg <at> entry=0x15369e9 <gdb_make_enums_visible+1745> "should_end >= end",
      file=file <at> entry=0x1536431 <gdb_make_enums_visible+281> "pdumper.c",
      line=line <at> entry=939) at alloc.c:8388
  #2  0x00f88e8a in dump_igc_finish_obj (ctx=ctx <at> entry=0xbfebd8) at pdumper.c:939
  #3  0x00f8caca in dump_object_finish (sz=40, out=0xbfe990, ctx=0xbfebd8)
      at pdumper.c:1003
  #4  dump_subr (subr=0x11611e0 <Swatch_debug_on_event.43963>, ctx=0xbfebd8)
      at pdumper.c:3093
  #5  dump_vectorlike (offset=6440296, lv=XIL(0x11611e5), ctx=0xbfebd8)
      at pdumper.c:3187
  #6  dump_object (ctx=ctx <at> entry=0xbfebd8, object=XIL(0x11611e5))
      at pdumper.c:3318
  #7  0x00f8e7f4 in dump_drain_copied_objects (ctx=0xbfebd8) at pdumper.c:3539
  #8  Fdump_emacs_portable (filename=<optimized out>,
      track_referrers=<optimized out>) at pdumper.c:4524
  #9  0x00fb3ecc in eval_sub (form=XIL(0xc35cedb)) at eval.c:2623
  #10 0x00fb3cd9 in eval_sub (form=XIL(0xc35ce1b)) at eval.c:2571
  #11 0x00fb514a in Fprogn (body=XIL(0xc35d0e3)) at eval.c:452
  #12 Flet (args=<optimized out>) at eval.c:1122
  #13 0x00fb3cd9 in eval_sub (form=XIL(0xc35cd33)) at eval.c:2571
  #14 0x00fb58cc in Funwind_protect (args=XIL(0xc35d0f3)) at lisp.h:1563
  #15 0x00fb3cd9 in eval_sub (form=XIL(0xc35cd23)) at eval.c:2571
  #16 0x00fb514a in Fprogn (body=XIL(0)) at eval.c:452
  #17 Flet (args=<optimized out>) at eval.c:1122
  #18 0x00fb3cd9 in eval_sub (form=XIL(0xc35ccf3)) at eval.c:2571
  #19 0x00fb514a in Fprogn (body=XIL(0xc35dbb3)) at eval.c:452
  #20 Flet (args=<optimized out>) at eval.c:1122
  #21 0x00fb3cd9 in eval_sub (form=XIL(0xc35c793)) at eval.c:2571
  #22 0x00fb3cd9 in eval_sub (form=XIL(0xc35c773)) at eval.c:2571
  #23 0x00fb48b4 in Fprogn (body=XIL(0)) at eval.c:452
  #24 Fif (args=XIL(0xc35c32b)) at eval.c:408
  #25 0x00fb3cd9 in eval_sub (form=form <at> entry=XIL(0xc35c25b)) at eval.c:2571
  #26 0x00ff023c in readevalloop (readcharfun=readcharfun <at> entry=XIL(0x6120),
      infile0=infile0 <at> entry=0xbff638,
      sourcename=sourcename <at> entry=XIL(0xb04a364),
      printflag=printflag <at> entry=false, unibyte=unibyte <at> entry=XIL(0),
      readfun=readfun <at> entry=XIL(0), start=start <at> entry=XIL(0),
      end=<optimized out>, end <at> entry=XIL(0)) at lread.c:2540
  #27 0x00ff0cd9 in Fload (file=XIL(0xb04a004), noerror=XIL(0),
      nomessage=XIL(0), nosuffix=XIL(0), must_suffix=<optimized out>)
      at lisp.h:1229
  #28 0x00fb3e69 in eval_sub (form=form <at> entry=XIL(0xb04a02b)) at eval.c:2634
  #29 0x00fb5e32 in Feval (form=XIL(0xb04a02b), lexical=lexical <at> entry=XIL(0x20))
      at eval.c:2479
  #30 0x00f0ef8b in top_level_2 () at lisp.h:1229
  #31 0x00fae15b in internal_condition_case (
      bfun=bfun <at> entry=0xf0ef2c <top_level_2>, handlers=handlers <at> entry=XIL(0x60),
      hfun=hfun <at> entry=0xf18d54 <cmd_error>) at eval.c:1627
  #32 0x00f0f6b8 in top_level_1 (ignore=XIL(0)) at lisp.h:1229
  #33 0x00fae077 in internal_catch (tag=tag <at> entry=XIL(0xc560),
      func=func <at> entry=0xf0f68c <top_level_1>, arg=arg <at> entry=XIL(0))
      at eval.c:1306
  #34 0x00f0ed3f in command_loop () at lisp.h:1229
  #35 0x00f188fa in recursive_edit_1 () at keyboard.c:760
  #36 0x00f18c01 in Frecursive_edit () at keyboard.c:843
  #37 0x0115fc05 in main (argc=<optimized out>, argv=<optimized out>)
      at emacs.c:2658

  Lisp Backtrace:
  "dump-emacs-portable" (0xbfed90)
  "if" (0xbfee1c)
  "let" (0xbfef1c)
  "unwind-protect" (0xbfefcc)
  "let" (0xbff0bc)
  "let" (0xbff1ac)
  "if" (0xbff23c)
  "if" (0xbff2ec)
  "load" (0xbff8e0)
  (gdb) up
  #1  0x00f836f4 in die (
      msg=msg <at> entry=0x15369e9 <gdb_make_enums_visible+1745> "should_end >= end",
      file=file <at> entry=0x1536431 <gdb_make_enums_visible+281> "pdumper.c",
      line=line <at> entry=939) at alloc.c:8388
  8388      terminate_due_to_signal (SIGABRT, INT_MAX);
  (gdb)
  #2  0x00f88e8a in dump_igc_finish_obj (ctx=ctx <at> entry=0xbfebd8) at pdumper.c:939
  939           eassert (should_end >= end);
  (gdb) p should_end
  $1 = <optimized out>
  (gdb) p end
  $2 = 0x1b5c35b0 ""
  (gdb) p igc_dump_finish_obj (ctx->igc_obj_dumped, ctx->igc_type, base, end)
  value has been optimized out
  (gdb) p base
  $3 = <optimized out>
  p igc_dump_finish_obj (ctx->igc_obj_dumped, ctx->igc_type, (char *) ctx->buf + ctx->igc_base_offset, end)
  $4 = 0x1b5c3588 ""

The following patch seems to fix that, but I'm not sure it's the
correct fix.  Can you explain why we need to add-variable-watcher for
this in init_keyboard, in particular in temacs?  since init_keyboard
is called both in temacs and in emacs, it sounds like the code, when
called in emacs, will try to free memory that was malloced in temacs,
is that right?

diff --git a/src/keyboard.c b/src/keyboard.c
index 82505e7..a5d85e9 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -12817,6 +12817,7 @@ init_keyboard (void)
 #endif
 
 #ifdef HAVE_MPS
+  if (initialized)
   {
     Lisp_Object watcher;
 

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 20:52:58 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Resent-To: bug-gnu-emacs <at> gnu.org
>> Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
>>  75632-done <at> debbugs.gnu.org
>> Date: Fri, 17 Jan 2025 15:26:10 +0000
>> From:  Pip Cet via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> "Ihor Radchenko" <yantar92 <at> posteo.net> writes:
>>
>> > Pip Cet <pipcet <at> protonmail.com> writes:
>> >
>> >> "Ihor Radchenko" <yantar92 <at> posteo.net> writes:
>> >>
>> >>> Just got the following:
>> >>
>> >> Yes, that's the signal handling bug.  I'm not entirely sure why we
>> >> removed the fix that was in scratch/igc, but we did, so we need another
>> >> one.
>> >> ...
>> >> to handle_user_signal, which will silently ignore SIGUSR* received while
>> >> MPS may have locked the arena.
>> >
>> > I indeed sent SIGUSR2 just before I saw the crash.
>>
>> It'd be nice if that simply worked.  I think debugging with SIGUSR* is
>> important, so I've pushed a fix (and I'm closing this bug; if further
>> discussion is needed, feel free to revert and reopen).
>
> This breaks the MS-Windows build: temacs crashes as below:

Sorry.  Can you try the fix I just pushed?  While I arrived at that
independently (I was about to push when I got your email), I believe it
explains your crash as well.

>   (gdb) bt
>   #0  terminate_due_to_signal (sig=sig <at> entry=22,
>       backtrace_limit=backtrace_limit <at> entry=2147483647) at emacs.c:432
>   #1  0x00f836f4 in die (
>       msg=msg <at> entry=0x15369e9 <gdb_make_enums_visible+1745> "should_end >= end",
>       file=file <at> entry=0x1536431 <gdb_make_enums_visible+281> "pdumper.c",
>       line=line <at> entry=939) at alloc.c:8388
>   #2  0x00f88e8a in dump_igc_finish_obj (ctx=ctx <at> entry=0xbfebd8) at pdumper.c:939

This function calls igc_dump_finish_obj, which returns 0 because the GC
header for the static subr hasn't been initialized, and then causes the
assertion error.

> The following patch seems to fix that, but I'm not sure it's the
> correct fix.  Can you explain why we need to add-variable-watcher for
> this in init_keyboard, in particular in temacs?

I'm not sure what you are suggesting here.  Is there a good place that
is initialized only in the !pdumper || !temacs case?

> since init_keyboard
> is called both in temacs and in emacs, it sounds like the code, when
> called in emacs, will try to free memory that was malloced in temacs,
> is that right?

I don't think so, but it might call Fadd_variable_watcher twice.  We
should avoid that.

> diff --git a/src/keyboard.c b/src/keyboard.c
> index 82505e7..a5d85e9 100644
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -12817,6 +12817,7 @@ init_keyboard (void)
>  #endif
>
>  #ifdef HAVE_MPS
> +  if (initialized)
>    {
>      Lisp_Object watcher;

I can add that (with a FIXME comment because it breaks the !pdumper
case, which is currently an #error but appears to work).

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 23:04:57 +0200

> Date: Fri, 17 Jan 2025 20:52:58 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> > This breaks the MS-Windows build: temacs crashes as below:
> 
> Sorry.  Can you try the fix I just pushed?  While I arrived at that
> independently (I was about to push when I got your email), I believe it
> explains your crash as well.

It fixes the crash, thanks.

> > The following patch seems to fix that, but I'm not sure it's the
> > correct fix.  Can you explain why we need to add-variable-watcher for
> > this in init_keyboard, in particular in temacs?
> 
> I'm not sure what you are suggesting here.  Is there a good place that
> is initialized only in the !pdumper || !temacs case?

The "if (initialized)" test is exactly for that.  pdumper_load sets
initialized = true before it returns.  So in temacs initialized will
be false when init_keyboard is called.

> I don't think so, but it might call Fadd_variable_watcher twice.  We
> should avoid that.

So maybe the patch I proposed should be installed regardless.

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 17 Jan 2025 21:30:41 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Fri, 17 Jan 2025 20:52:58 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> > This breaks the MS-Windows build: temacs crashes as below:
>>
>> Sorry.  Can you try the fix I just pushed?  While I arrived at that
>> independently (I was about to push when I got your email), I believe it
>> explains your crash as well.
>
> It fixes the crash, thanks.

Thanks!

I still think we should initialize the vector header of subrs and the
thread structure to contain the appropriate size fields.  Subrs
currently claim size 0 (and no auto-marked Lisp fields), which is
confusing.  Threads report totally incorrect values, because of what
seems like a bug to me: VECSIZE is used where VECSIZE - PSEUDOVECSIZE is
intended.

But this problem was caused by the GC header, not the vectorlike header.
Initializing the GC header is harder because of the hash value.  (No
great harm in leaving it set to 0 for those very few objects, though).

>> > The following patch seems to fix that, but I'm not sure it's the
>> > correct fix.  Can you explain why we need to add-variable-watcher for
>> > this in init_keyboard, in particular in temacs?
>>
>> I'm not sure what you are suggesting here.  Is there a good place that
>> is initialized only in the !pdumper || !temacs case?
>
> The "if (initialized)" test is exactly for that.  pdumper_load sets
> initialized = true before it returns.  So in temacs initialized will
> be false when init_keyboard is called.

But if we build without pdumper (after removing the relevant #error and
a few minor fixes, it works just fine), initialized is false, so SIGUSR2
wouldn't work at all in those builds.

>> I don't think so, but it might call Fadd_variable_watcher twice.  We
>> should avoid that.
>
> So maybe the patch I proposed should be installed regardless.

I had a look: Fadd_variable_watcher handles the case of several
identical watchers by ignoring the second call.

If we want to avoid calling Fadd_variable_watcher twice, we can use "if
(!initialized)", which will run the code just once (however, the
explicit call to watch_debug_on_event needs to be performed twice,
before and after the dump, because the xstrdup()ed string isn't dumped).

In the pdumper case, it'd run in temacs, put the right subr in the
symbol's plist, dump it, and then it'd be restored from the dump and
we'd only need the initial call.

Would that be okay?

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 09:28:55 +0200

> Date: Fri, 17 Jan 2025 21:30:41 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> I'm not sure what you are suggesting here.  Is there a good place that
> >> is initialized only in the !pdumper || !temacs case?
> >
> > The "if (initialized)" test is exactly for that.  pdumper_load sets
> > initialized = true before it returns.  So in temacs initialized will
> > be false when init_keyboard is called.
> 
> But if we build without pdumper (after removing the relevant #error and
> a few minor fixes, it works just fine), initialized is false, so SIGUSR2
> wouldn't work at all in those builds.

Do we want to support MPS in unexec builds?  I thought we didn't.
(Does it even work in such builds?)

> In the pdumper case, it'd run in temacs, put the right subr in the
> symbol's plist, dump it, and then it'd be restored from the dump and
> we'd only need the initial call.
> 
> Would that be okay?

I have no idea, because I don't understand why is that the solution to
SIGUSR handling.  (How about adding some comments which explain the
purpose of watching that variable?)  Also see above regarding unexec
build with MPS.

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 11:42:44 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Fri, 17 Jan 2025 21:30:41 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> >> I'm not sure what you are suggesting here.  Is there a good place that
>> >> is initialized only in the !pdumper || !temacs case?
>> >
>> > The "if (initialized)" test is exactly for that.  pdumper_load sets
>> > initialized = true before it returns.  So in temacs initialized will
>> > be false when init_keyboard is called.
>>
>> But if we build without pdumper (after removing the relevant #error and
>> a few minor fixes, it works just fine), initialized is false, so SIGUSR2
>> wouldn't work at all in those builds.
>
> Do we want to support MPS in unexec builds?  I thought we didn't.
> (Does it even work in such builds?)

I was talking about --with-dumping=none, not unexec builds.  I"m not
fixing unexec bugs!

>> In the pdumper case, it'd run in temacs, put the right subr in the
>> symbol's plist, dump it, and then it'd be restored from the dump and
>> we'd only need the initial call.
>>
>> Would that be okay?
>
> I have no idea, because I don't understand why is that the solution to
> SIGUSR handling.  (How about adding some comments which explain the
> purpose of watching that variable?)  Also see above regarding unexec
> build with MPS.

Vdebug_on_event points to data behind a memory barrier.  It's replaced
by a C string which is xstrdup'd, so that's not behind a memory barrier.
keeping the two in sync is done with a variable watcher, as is the case
for gc-cons-threshold.

Variable watchers survive dump/reload cycles, so we set it up just once,
when !initialized; when entering the code in the post-dump state, we
perform the initial dummy "watch" event to initialize the C string.

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 14:25:03 +0200

> Date: Sat, 18 Jan 2025 11:42:44 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> >> But if we build without pdumper (after removing the relevant #error and
> >> a few minor fixes, it works just fine), initialized is false, so SIGUSR2
> >> wouldn't work at all in those builds.
> >
> > Do we want to support MPS in unexec builds?  I thought we didn't.
> > (Does it even work in such builds?)
> 
> I was talking about --with-dumping=none, not unexec builds.  I"m not
> fixing unexec bugs!

OK, so the only interesting use case is running temacs when not
dumping it.

> >> In the pdumper case, it'd run in temacs, put the right subr in the
> >> symbol's plist, dump it, and then it'd be restored from the dump and
> >> we'd only need the initial call.
> >>
> >> Would that be okay?
> >
> > I have no idea, because I don't understand why is that the solution to
> > SIGUSR handling.  (How about adding some comments which explain the
> > purpose of watching that variable?)  Also see above regarding unexec
> > build with MPS.
> 
> Vdebug_on_event points to data behind a memory barrier.  It's replaced
> by a C string which is xstrdup'd, so that's not behind a memory barrier.
> keeping the two in sync is done with a variable watcher, as is the case
> for gc-cons-threshold.
> 
> Variable watchers survive dump/reload cycles, so we set it up just once,
> when !initialized; when entering the code in the post-dump state, we
> perform the initial dummy "watch" event to initialize the C string.

OK, but what does this have to do with SIGUSR handling?

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 14:27:03 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sat, 18 Jan 2025 11:42:44 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> >> But if we build without pdumper (after removing the relevant #error and
>> >> a few minor fixes, it works just fine), initialized is false, so SIGUSR2
>> >> wouldn't work at all in those builds.
>> >
>> > Do we want to support MPS in unexec builds?  I thought we didn't.
>> > (Does it even work in such builds?)
>>
>> I was talking about --with-dumping=none, not unexec builds.  I"m not
>> fixing unexec bugs!
>
> OK, so the only interesting use case is running temacs when not
> dumping it.

I think it's important to keep temacs/emacs without dumping runnable.
It's certainly still interesting when temacs is called emacs.

>> >> In the pdumper case, it'd run in temacs, put the right subr in the
>> >> symbol's plist, dump it, and then it'd be restored from the dump and
>> >> we'd only need the initial call.
>> >>
>> >> Would that be okay?
>> >
>> > I have no idea, because I don't understand why is that the solution to
>> > SIGUSR handling.  (How about adding some comments which explain the
>> > purpose of watching that variable?)  Also see above regarding unexec
>> > build with MPS.
>>
>> Vdebug_on_event points to data behind a memory barrier.  It's replaced
>> by a C string which is xstrdup'd, so that's not behind a memory barrier.
>> keeping the two in sync is done with a variable watcher, as is the case
>> for gc-cons-threshold.
>>
>> Variable watchers survive dump/reload cycles, so we set it up just once,
>> when !initialized; when entering the code in the post-dump state, we
>> perform the initial dummy "watch" event to initialize the C string.
>
> OK, but what does this have to do with SIGUSR handling?

The SIGUSR* handler needs to access that string.  Signal handlers cannot
access MPS-managed memory, so we need to put the string in regular
xmalloc memory.

I think that's clear from the commit message, by the way.  If it isn't,
I'd appreciate advice on how to improve it.  (Maybe it's a good idea to
use the extended ChangeLog format with an explanatory paragraph between
the header line (which some git utilities insist on keeping shorter than
it should be) and the individual files' changes.)

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 17:50:16 +0200

> Date: Sat, 18 Jan 2025 14:27:03 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> I was talking about --with-dumping=none, not unexec builds.  I"m not
> >> fixing unexec bugs!
> >
> > OK, so the only interesting use case is running temacs when not
> > dumping it.
> 
> I think it's important to keep temacs/emacs without dumping runnable.
> It's certainly still interesting when temacs is called emacs.

Yes, of course, no argument here.

> >> Vdebug_on_event points to data behind a memory barrier.  It's replaced
> >> by a C string which is xstrdup'd, so that's not behind a memory barrier.
> >> keeping the two in sync is done with a variable watcher, as is the case
> >> for gc-cons-threshold.
> >>
> >> Variable watchers survive dump/reload cycles, so we set it up just once,
> >> when !initialized; when entering the code in the post-dump state, we
> >> perform the initial dummy "watch" event to initialize the C string.
> >
> > OK, but what does this have to do with SIGUSR handling?
> 
> The SIGUSR* handler needs to access that string.  Signal handlers cannot
> access MPS-managed memory, so we need to put the string in regular
> xmalloc memory.

So this is just to allow the signal handler access to the event name?

I'd be much happier if we instead treated this as we treat SIGIO that
is delivered when the user presses a keyboard key: set a flag saying
that input is available, and then process it as we do with keyboard.
The SIGUSR event is eventually entered into the keyboard buffer
anyway, so why should we treat it so differently from SIGIO?

> I think that's clear from the commit message, by the way.  If it isn't,
> I'd appreciate advice on how to improve it.

These details are better said in comments to the code than in commit
log messages.  Looking for explanations for what the code does in Git
history is less convenient, and I'm quite sure some people will not
even think about it, at least not at first.

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 17:31:57 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sat, 18 Jan 2025 14:27:03 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> >> I was talking about --with-dumping=none, not unexec builds.  I"m not
>> >> fixing unexec bugs!
>> >
>> > OK, so the only interesting use case is running temacs when not
>> > dumping it.
>>
>> I think it's important to keep temacs/emacs without dumping runnable.
>> It's certainly still interesting when temacs is called emacs.
>
> Yes, of course, no argument here.

So (!initialized) it is?  That works for temacs and emacs (I originally
thought it would be wasteful for non-interactive temacs, but I'm sure
someone's using that :-) ).

>> >> Vdebug_on_event points to data behind a memory barrier.  It's replaced
>> >> by a C string which is xstrdup'd, so that's not behind a memory barrier.
>> >> keeping the two in sync is done with a variable watcher, as is the case
>> >> for gc-cons-threshold.
>> >>
>> >> Variable watchers survive dump/reload cycles, so we set it up just once,
>> >> when !initialized; when entering the code in the post-dump state, we
>> >> perform the initial dummy "watch" event to initialize the C string.
>> >
>> > OK, but what does this have to do with SIGUSR handling?
>>
>> The SIGUSR* handler needs to access that string.  Signal handlers cannot
>> access MPS-managed memory, so we need to put the string in regular
>> xmalloc memory.
>
> So this is just to allow the signal handler access to the event name?

So it can determine whether to set some flags, or mark an input signal
pending and set some other flags, yes.

> I'd be much happier if we instead treated this as we treat SIGIO that
> is delivered when the user presses a keyboard key: set a flag saying
> that input is available, and then process it as we do with keyboard.

IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
directly (or use QUITP); we wouldn't be able to break out of one of
those if we only set Vinhibit_quit from what's currently
store_user_signal_events, would we?

If we want to change behavior in this way, we should do it on master,
then merge it into feature/igc.

> The SIGUSR event is eventually entered into the keyboard buffer
> anyway, so why should we treat it so differently from SIGIO?

Vdebug_on_event isn't, AFAICS.  Not a major change to make it so,
though.  I don't know why we eat this event, but we do.

>> I think that's clear from the commit message, by the way.  If it isn't,
>> I'd appreciate advice on how to improve it.
>
> These details are better said in comments to the code than in commit
> log messages.  Looking for explanations for what the code does in Git
> history is less convenient, and I'm quite sure some people will not
> even think about it, at least not at first.

Good point, agreed.

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 20:57:01 +0200

> Date: Sat, 18 Jan 2025 17:31:57 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> I think it's important to keep temacs/emacs without dumping runnable.
> >> It's certainly still interesting when temacs is called emacs.
> >
> > Yes, of course, no argument here.
> 
> So (!initialized) it is?  That works for temacs and emacs (I originally
> thought it would be wasteful for non-interactive temacs, but I'm sure
> someone's using that :-) ).

As long as we understand well what it means to free in emacs a string
that was xstrdup'ed in temacs, yes.  Suppose debug-on-event is changed
in emacs to something else, how would xfree work in that case?

> > I'd be much happier if we instead treated this as we treat SIGIO that
> > is delivered when the user presses a keyboard key: set a flag saying
> > that input is available, and then process it as we do with keyboard.
> 
> IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
> directly (or use QUITP); we wouldn't be able to break out of one of
> those if we only set Vinhibit_quit from what's currently
> store_user_signal_events, would we?

Sorry, I don't follow.  Why does it matter whether we set those
variables from a signal handler or in gobble_input?

> > The SIGUSR event is eventually entered into the keyboard buffer
> > anyway, so why should we treat it so differently from SIGIO?
> 
> Vdebug_on_event isn't, AFAICS.  Not a major change to make it so,
> though.  I don't know why we eat this event, but we do.

I guess because we want the debugger to pop up before any queued
events are processed.  But I don't see why doing the same in
gobble_input should produce different behavior.

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 19:53:50 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sat, 18 Jan 2025 17:31:57 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> >> I think it's important to keep temacs/emacs without dumping runnable.
>> >> It's certainly still interesting when temacs is called emacs.
>> >
>> > Yes, of course, no argument here.
>>
>> So (!initialized) it is?  That works for temacs and emacs (I originally
>> thought it would be wasteful for non-interactive temacs, but I'm sure
>> someone's using that :-) ).
>
> As long as we understand well what it means to free in emacs a string
> that was xstrdup'ed in temacs, yes.  Suppose debug-on-event is changed
> in emacs to something else, how would xfree work in that case?

pdumper doesn't work that way (unexec did, I think): a static variable
that isn't "remembered" by pdumper isn't saved, and it starts out as
NULL in the dumped Emacs, and is replaced by the new xstrdup.  The old
xstrdup (and the memory it allocated) are gone after the dump.

>> > I'd be much happier if we instead treated this as we treat SIGIO that
>> > is delivered when the user presses a keyboard key: set a flag saying
>> > that input is available, and then process it as we do with keyboard.
>>
>> IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
>> directly (or use QUITP); we wouldn't be able to break out of one of
>> those if we only set Vinhibit_quit from what's currently
>> store_user_signal_events, would we?
>
> Sorry, I don't follow.  Why does it matter whether we set those
> variables from a signal handler or in gobble_input?

We might never reach gobble_input if the variable isn't set in the
signal handler: some loops check QUITP (etc.), and if there's a bug in
them and Vinhibit_quit is set, the loop might never call maybe_quit.

For example, SIGUSR2 should be able to interrupt this loop in gap_right:

  while (1)
    {
      /* I gets number of characters left to copy.  */
      i = bytepos - new_s1;
      if (i == 0)
	break;
      /* If a quit is requested, stop copying now.
	 Change BYTEPOS to be where we have actually moved the gap to.
	 Note that this cannot happen when we are called to make the
	 gap larger or smaller, since make_gap_larger and
	 make_gap_smaller set inhibit-quit.  */
      if (QUITP)
	{
          /* FIXME: This can point in the middle of a multibyte character.  */
	  bytepos = new_s1;
	  charpos = BYTE_TO_CHAR (bytepos);
	  break;
	}
      /* Move at most 32000 chars before checking again for a quit.  */
      if (i > 32000)
	i = 32000;
      new_s1 += i;
      memmove (to, from, i);
      from += i, to += i;
    }

Since QUITP is defined as

#define QUITP (!NILP (Vquit_flag) && NILP (Vinhibit_quit))

that means the signal handler (not gobble_input) needs to touch these
flags.

It's possible to add something like

if (pending_signals)
  {
    ptrdiff_t old_bytepos = bytepos;
    ptrdiff_t old_charpos = charpos;
    bytepos = new_s1;
    charpos = BYTE_TO_CHAR (bytepos);
    maybe_quit ();
  }

to this loop, and then it would be interruptible by SIGUSR2, but it
would also be interruptible by other input events, so I'm not sure that
wouldn't cause crashes.

>> > The SIGUSR event is eventually entered into the keyboard buffer
>> > anyway, so why should we treat it so differently from SIGIO?
>>
>> Vdebug_on_event isn't, AFAICS.  Not a major change to make it so,
>> though.  I don't know why we eat this event, but we do.
>
> I guess because we want the debugger to pop up before any queued
> events are processed.

"instead of", I think.  That's a detail I don't care much for: I think
it'd be helpful for sigusr2 to show up in M-x view-lossage even if it
triggered the debugger.

> But I don't see why doing the same in
> gobble_input should produce different behavior.

while (true)
  {
    while (!QUITP);
    maybe_quit();
  )

should be interruptible by SIGUSR2.

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 22:18:16 +0200

> Date: Sat, 18 Jan 2025 19:53:50 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> So (!initialized) it is?  That works for temacs and emacs (I originally
> >> thought it would be wasteful for non-interactive temacs, but I'm sure
> >> someone's using that :-) ).
> >
> > As long as we understand well what it means to free in emacs a string
> > that was xstrdup'ed in temacs, yes.  Suppose debug-on-event is changed
> > in emacs to something else, how would xfree work in that case?
> 
> pdumper doesn't work that way (unexec did, I think): a static variable
> that isn't "remembered" by pdumper isn't saved, and it starts out as
> NULL in the dumped Emacs, and is replaced by the new xstrdup.  The old
> xstrdup (and the memory it allocated) are gone after the dump.

But if we use !initialized, the code which installs the variable
watcher will not run in the dumped Emacs, right?

> >> IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
> >> directly (or use QUITP); we wouldn't be able to break out of one of
> >> those if we only set Vinhibit_quit from what's currently
> >> store_user_signal_events, would we?
> >
> > Sorry, I don't follow.  Why does it matter whether we set those
> > variables from a signal handler or in gobble_input?
> 
> We might never reach gobble_input if the variable isn't set in the
> signal handler: some loops check QUITP (etc.), and if there's a bug in
> them and Vinhibit_quit is set, the loop might never call maybe_quit.

Then how about setting Vquit_flag and resetting Vinhibit_quit in the
handler, but leaving the check whether we should call the debugger to
gobble_input?

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 18 Jan 2025 20:40:47 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sat, 18 Jan 2025 19:53:50 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> >> So (!initialized) it is?  That works for temacs and emacs (I originally
>> >> thought it would be wasteful for non-interactive temacs, but I'm sure
>> >> someone's using that :-) ).
>> >
>> > As long as we understand well what it means to free in emacs a string
>> > that was xstrdup'ed in temacs, yes.  Suppose debug-on-event is changed
>> > in emacs to something else, how would xfree work in that case?
>>
>> pdumper doesn't work that way (unexec did, I think): a static variable
>> that isn't "remembered" by pdumper isn't saved, and it starts out as
>> NULL in the dumped Emacs, and is replaced by the new xstrdup.  The old
>> xstrdup (and the memory it allocated) are gone after the dump.
>
> But if we use !initialized, the code which installs the variable
> watcher will not run in the dumped Emacs, right?

Correct.  Variable watchers are symbol properties, and symbol properties
are restored from the dump, so there is no need to add them again.

>> >> IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
>> >> directly (or use QUITP); we wouldn't be able to break out of one of
>> >> those if we only set Vinhibit_quit from what's currently
>> >> store_user_signal_events, would we?
>> >
>> > Sorry, I don't follow.  Why does it matter whether we set those
>> > variables from a signal handler or in gobble_input?
>>
>> We might never reach gobble_input if the variable isn't set in the
>> signal handler: some loops check QUITP (etc.), and if there's a bug in
>> them and Vinhibit_quit is set, the loop might never call maybe_quit.
>
> Then how about setting Vquit_flag and resetting Vinhibit_quit in the
> handler, but leaving the check whether we should call the debugger to
> gobble_input?

1. We can't do that unconditionally.  An ordinary SIGUSR2 should not
cause a quit, only a special event should.  So we need to check whether
the event we received is special or not.

2. probably_quit handles the quit flag first, then handles pending
signals only when it is called again, by the next maybe_quit.  That
means by the time we reach gobble_input, it's too late to call the
debugger.  So we need to set debug_* at the same time as setting
Vquit_flag (or rewrite probably_quit, which probably has very good
reasons for its peculiar behavior).

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sun, 19 Jan 2025 07:39:34 +0200

> Date: Sat, 18 Jan 2025 20:40:47 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> > But if we use !initialized, the code which installs the variable
> > watcher will not run in the dumped Emacs, right?
> 
> Correct.  Variable watchers are symbol properties, and symbol properties
> are restored from the dump, so there is no need to add them again.

Then why do we need to do this in C and not in Lisp?  Can't we set up
the watcher as part of some preloaded Lisp file?

> >> >> IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
> >> >> directly (or use QUITP); we wouldn't be able to break out of one of
> >> >> those if we only set Vinhibit_quit from what's currently
> >> >> store_user_signal_events, would we?
> >> >
> >> > Sorry, I don't follow.  Why does it matter whether we set those
> >> > variables from a signal handler or in gobble_input?
> >>
> >> We might never reach gobble_input if the variable isn't set in the
> >> signal handler: some loops check QUITP (etc.), and if there's a bug in
> >> them and Vinhibit_quit is set, the loop might never call maybe_quit.
> >
> > Then how about setting Vquit_flag and resetting Vinhibit_quit in the
> > handler, but leaving the check whether we should call the debugger to
> > gobble_input?
> 
> 1. We can't do that unconditionally.  An ordinary SIGUSR2 should not
> cause a quit, only a special event should.  So we need to check whether
> the event we received is special or not.
> 
> 2. probably_quit handles the quit flag first, then handles pending
> signals only when it is called again, by the next maybe_quit.  That
> means by the time we reach gobble_input, it's too late to call the
> debugger.  So we need to set debug_* at the same time as setting
> Vquit_flag (or rewrite probably_quit, which probably has very good
> reasons for its peculiar behavior).

OK, but could we at least make this somewhat cleaner?  Instead of
copying the name of the event aside, can we produce a C integer that
identifies the event, and store that number instead?  Accessing
integers from a signal handler should not be a problem, right?  (In
retrospect, we should have probably made debug-on-event a function,
then we could produce that number inside the function, thus avoiding
the need for watching the variable.  Maybe we should introduce the
function now and deprecate the variable?)

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sun, 19 Jan 2025 09:44:26 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sat, 18 Jan 2025 20:40:47 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> > But if we use !initialized, the code which installs the variable
>> > watcher will not run in the dumped Emacs, right?
>>
>> Correct.  Variable watchers are symbol properties, and symbol properties
>> are restored from the dump, so there is no need to add them again.
>
> Then why do we need to do this in C and not in Lisp?  Can't we set up
> the watcher as part of some preloaded Lisp file?
>
>> >> >> IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
>> >> >> directly (or use QUITP); we wouldn't be able to break out of one of
>> >> >> those if we only set Vinhibit_quit from what's currently
>> >> >> store_user_signal_events, would we?
>> >> >
>> >> > Sorry, I don't follow.  Why does it matter whether we set those
>> >> > variables from a signal handler or in gobble_input?
>> >>
>> >> We might never reach gobble_input if the variable isn't set in the
>> >> signal handler: some loops check QUITP (etc.), and if there's a bug in
>> >> them and Vinhibit_quit is set, the loop might never call maybe_quit.
>> >
>> > Then how about setting Vquit_flag and resetting Vinhibit_quit in the
>> > handler, but leaving the check whether we should call the debugger to
>> > gobble_input?
>>
>> 1. We can't do that unconditionally.  An ordinary SIGUSR2 should not
>> cause a quit, only a special event should.  So we need to check whether
>> the event we received is special or not.
>>
>> 2. probably_quit handles the quit flag first, then handles pending
>> signals only when it is called again, by the next maybe_quit.  That
>> means by the time we reach gobble_input, it's too late to call the
>> debugger.  So we need to set debug_* at the same time as setting
>> Vquit_flag (or rewrite probably_quit, which probably has very good
>> reasons for its peculiar behavior).
>
> OK, but could we at least make this somewhat cleaner?  Instead of

I see no reason for why the old code behaved as it did, so I'm proposing
this:

1. doc change: don't use "special event" to mean two totally different
things in process_special_events and special_event_name; remove
special_event_name.

2. when debug-on-event is modified/called, set a per-signal flag in the
user_signal_info struct, saving us a strcmp.

3. Add a watcher as on feature/igc to reset the flags of all user
signals.

4. Make that watcher callable as Fdebug_on_event directly.

I stopped there.  I'm confused about input_available_clear_time: it's
only cleared if we don't enter the debugger.  Adding the code to clear
it to the debug_on_event case might help us break out of some broken
pselects, and it shouldn't ever hurt us (famous last words).

But maybe I totally misunderstood what you suggested, too.

diff --git a/src/keyboard.c b/src/keyboard.c
index fa19c9f8ad3..217ec6582fc 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -8254,6 +8254,10 @@ deliver_input_available_signal (int sig)
   /* Signal number.  */
   int sig;
 
+  /* True if this event is supposed to make us enter the debugger
+     instead of its normal behavior.  */
+  bool debug_on_event;
+
   /* Name of the signal.  */
   char *name;
 
@@ -8279,6 +8283,7 @@ add_user_signal (int sig, const char *name)
 
   p = xmalloc (sizeof *p);
   p->sig = sig;
+  p->debug_on_event = false;
   p->name = xstrdup (name);
   p->npending = 0;
   p->next = user_signals;
@@ -8288,42 +8293,68 @@ add_user_signal (int sig, const char *name)
   sigaction (sig, &action, 0);
 }
 
+DEFUN ("debug_on_event", Fdebug_on_event, Sdebug_on_event, 1, 4, 0,
+       doc: /* Make EVENT enter the debugger.
+
+EVENT should be a symbol, either sigusr1 or sigusr2.
+
+This function supports an additional calling convention to be usable as
+a variable watcher.  */)
+  (Lisp_Object arg_or_symbol, Lisp_Object newval, Lisp_Object operation,
+   Lisp_Object where)
+{
+  Lisp_Object ret = Qnil;
+  Lisp_Object symbol = arg_or_symbol;
+  if (EQ (symbol, Qdebug_on_event))
+    symbol = newval;
+
+  CHECK_SYMBOL (symbol);
+
+  struct user_signal_info *p;
+  for (p = user_signals; p; p = p->next)
+    if (SYMBOLP (symbol) && strcmp (SSDATA (SYMBOL_NAME (symbol)), p->name) == 0)
+      {
+	p->debug_on_event = true;
+	ret = Qt;
+      }
+    else
+      p->debug_on_event = false;
+
+  Vdebug_on_event = symbol;
+
+  return ret;
+}
+
 static void
 handle_user_signal (int sig)
 {
   struct user_signal_info *p;
-  const char *special_event_name = NULL;
-
-  if (SYMBOLP (Vdebug_on_event))
-    special_event_name = SSDATA (SYMBOL_NAME (Vdebug_on_event));
 
   for (p = user_signals; p; p = p->next)
     if (p->sig == sig)
       {
-        if (special_event_name
-	    && strcmp (special_event_name, p->name) == 0)
+	if (p->debug_on_event)
           {
             /* Enter the debugger in many ways.  */
             debug_on_next_call = true;
             debug_on_quit = true;
             Vquit_flag = Qt;
             Vinhibit_quit = Qnil;
-
-            /* Eat the event.  */
-            break;
           }
-
-	p->npending++;
-#if defined (USABLE_SIGIO) || defined (USABLE_SIGPOLL)
-	if (interrupt_input)
-	  handle_input_available_signal (sig);
 	else
-#endif
 	  {
-	    /* Tell wait_reading_process_output that it needs to wake
-	       up and look around.  */
-	    if (input_available_clear_time)
-	      *input_available_clear_time = make_timespec (0, 0);
+	    p->npending++;
+#if defined (USABLE_SIGIO) || defined (USABLE_SIGPOLL)
+	    if (interrupt_input)
+	      handle_input_available_signal (sig);
+	    else
+#endif
+	      {
+		/* Tell wait_reading_process_output that it needs to wake
+		   up and look around.  */
+		if (input_available_clear_time)
+		  *input_available_clear_time = make_timespec (0, 0);
+	      }
 	  }
 	break;
       }
@@ -12755,6 +12786,16 @@ init_keyboard (void)
   poll_suppress_count = 1;
   start_polling ();
 #endif
+
+  DEFSYM (Qdebug_on_event, "debug-on-event");
+
+  /* Variable watchers persist across the dump, no need to add them
+     twice.  */
+  if (!initialized)
+    {
+      Fadd_variable_watcher (Qdebug_on_event, Qdebug_on_event);
+    }
+  Fdebug_on_event (Qdebug_on_event, Vdebug_on_event, Qnil, Qnil);
 }
 
 /* This type's only use is in syms_of_keyboard, to put properties on the
@@ -13139,6 +13180,7 @@ syms_of_keyboard (void)
   defsubr (&Sevent_symbol_parse_modifiers);
   defsubr (&Sevent_convert_list);
   defsubr (&Sinternal_handle_focus_in);
+  defsubr (&Sdebug_on_event);
   defsubr (&Sread_key_sequence);
   defsubr (&Sread_key_sequence_vector);
   defsubr (&Srecursive_edit);


> copying the name of the event aside, can we produce a C integer that
> identifies the event, and store that number instead?  Accessing

Sorry, I ignored this when writing the patch.  I don't think we should
hardcode any assumptions about signal numbers, and I don't see another
readily available integer we can use; in any case, it's now trivial to
modify debug-on-event so several events enter the debugger.  This might
be handy for the Android port, which, IIUC, currently binds
triple-volume-down to quit, not to enter the debugger.  (I think all
other ports have stable signal handling interfaces, but I may be wrong).

> integers from a signal handler should not be a problem, right?  (In
> retrospect, we should have probably made debug-on-event a function,
> then we could produce that number inside the function, thus avoiding
> the need for watching the variable.  Maybe we should introduce the
> function now and deprecate the variable?)

I've introduced a function, so you can call debug-on-event or set it,
but I haven't deprecated the variable yet, because there's no way to
read the value from Lisp otherwise.

With this change, we still have six (!) debug-on-* variables, and two
additional debug-on-* functions.  They all behave slightly differently.

Is this at all what you meant?  I realize it's incomplete, but it's
probably worth it to do things properly.

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sun, 19 Jan 2025 12:38:49 +0200

> Date: Sun, 19 Jan 2025 09:44:26 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> > OK, but could we at least make this somewhat cleaner?  Instead of
> 
> I see no reason for why the old code behaved as it did, so I'm proposing
> this:
> 
> 1. doc change: don't use "special event" to mean two totally different
> things in process_special_events and special_event_name; remove
> special_event_name.
> 
> 2. when debug-on-event is modified/called, set a per-signal flag in the
> user_signal_info struct, saving us a strcmp.
> 
> 3. Add a watcher as on feature/igc to reset the flags of all user
> signals.

Can we add the watcher in Lisp instead?  Are there any reasons to have
it in C and at temacs time?

Also, perhaps make the function debug-on-event call the watcher, not
itself be the watcher.  I think that might make the API cleaner.  The
watcher function doesn't have to be exposed to Lisp, btw.

> 4. Make that watcher callable as Fdebug_on_event directly.
> 
> I stopped there.  I'm confused about input_available_clear_time: it's
> only cleared if we don't enter the debugger.  Adding the code to clear
> it to the debug_on_event case might help us break out of some broken
> pselects, and it shouldn't ever hurt us (famous last words).
> 
> But maybe I totally misunderstood what you suggested, too.

No, I think you understood me.

> +DEFUN ("debug_on_event", Fdebug_on_event, Sdebug_on_event, 1, 4, 0,
> +       doc: /* Make EVENT enter the debugger.
> +
> +EVENT should be a symbol, either sigusr1 or sigusr2.
> +
> +This function supports an additional calling convention to be usable as
> +a variable watcher.  */)
> +  (Lisp_Object arg_or_symbol, Lisp_Object newval, Lisp_Object operation,
> +   Lisp_Object where)

There's no EVENT in the function's arguments.  And what are the other
arguments?  The last two seem to be unused?

> +	if (p->debug_on_event)
>            {
>              /* Enter the debugger in many ways.  */
>              debug_on_next_call = true;
>              debug_on_quit = true;
>              Vquit_flag = Qt;
>              Vinhibit_quit = Qnil;
> -
> -            /* Eat the event.  */
> -            break;

Why did you remove this last part?  With the change to a flag, we no
longer have a problem that requires delaying the event processing,
right?  So why risk the change in behavior, something that you warned
against when I suggested the same?

> Sorry, I ignored this when writing the patch.  I don't think we should
> hardcode any assumptions about signal numbers, and I don't see another
> readily available integer we can use

I thought about just inventing one.  But that's a moot point if we go
with the flag approach.

> > integers from a signal handler should not be a problem, right?  (In
> > retrospect, we should have probably made debug-on-event a function,
> > then we could produce that number inside the function, thus avoiding
> > the need for watching the variable.  Maybe we should introduce the
> > function now and deprecate the variable?)
> 
> I've introduced a function, so you can call debug-on-event or set it,
> but I haven't deprecated the variable yet, because there's no way to
> read the value from Lisp otherwise.

Yes, good.  We should also deprecate the usage of the variable, in the
doc string and in the manual.

> With this change, we still have six (!) debug-on-* variables, and two
> additional debug-on-* functions.  They all behave slightly differently.
> 
> Is this at all what you meant?  I realize it's incomplete, but it's
> probably worth it to do things properly.

Yes, that's going in the right direction, I think.  Thanks.

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sun, 19 Jan 2025 13:37:18 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sun, 19 Jan 2025 09:44:26 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> > OK, but could we at least make this somewhat cleaner?  Instead of
>>
>> I see no reason for why the old code behaved as it did, so I'm proposing
>> this:
>>
>> 1. doc change: don't use "special event" to mean two totally different
>> things in process_special_events and special_event_name; remove
>> special_event_name.
>>
>> 2. when debug-on-event is modified/called, set a per-signal flag in the
>> user_signal_info struct, saving us a strcmp.
>>
>> 3. Add a watcher as on feature/igc to reset the flags of all user
>> signals.
>
> Can we add the watcher in Lisp instead?

Yes, please.  That way, we could have a proper interactive form, and the
current watcher could use an "internal" name (--).

> Are there any reasons to have it in C

Not that I can think of, but there are plenty of reasons to add it from
Lisp instead.

> and at temacs time?

I don't think it's a realistic scenario that someone needs to debug
non-interactive temacs, cannot use the default signal, and wants to
change it.

> Also, perhaps make the function debug-on-event call the watcher, not
> itself be the watcher.  I think that might make the API cleaner.  The

You've lost me there.  I was thinking debug-on-event, a Lisp function,
calls debug--on-event, a C subr, and we also install, from Lisp, a
variable watcher on the deprecated debug-on-event variable.  No static
subrs, which would have prevented the bug.

> watcher function doesn't have to be exposed to Lisp, btw.

I'd just use an anonymous lambda constructed in Lisp?  Causes a problem,
see below.

>> +DEFUN ("debug_on_event", Fdebug_on_event, Sdebug_on_event, 1, 4, 0,
                 ^  ^

fixing that, of course.


>> +       doc: /* Make EVENT enter the debugger.
>> +
>> +EVENT should be a symbol, either sigusr1 or sigusr2.
>> +
>> +This function supports an additional calling convention to be usable as
>> +a variable watcher.  */)
>> +  (Lisp_Object arg_or_symbol, Lisp_Object newval, Lisp_Object operation,
>> +   Lisp_Object where)
>
> There's no EVENT in the function's arguments.  And what are the other
> arguments?  The last two seem to be unused?

Yes, that was a failed attempt to make the DEFUN satisfy both the
variable watcher API and be callable from Lisp.  Bad idea.

>> +	if (p->debug_on_event)
>>            {
>>              /* Enter the debugger in many ways.  */
>>              debug_on_next_call = true;
>>              debug_on_quit = true;
>>              Vquit_flag = Qt;
>>              Vinhibit_quit = Qnil;
>> -
>> -            /* Eat the event.  */
>> -            break;
>
> Why did you remove this last part?  With the change to a flag, we no
> longer have a problem that requires delaying the event processing,
> right?

> So why risk the change in behavior, something that you warned
> against when I suggested the same?

No behavior change intended!  The new code is:

	if (p->debug_on_event)
          {
            /* Enter the debugger in many ways.  */
            debug_on_next_call = true;
            debug_on_quit = true;
            Vquit_flag = Qt;
            Vinhibit_quit = Qnil;
          }
	else
	  {
             ...
	  }
	break;

So we hit the "break" statement just as we did in the original code,
unless I'm very confused.  I misread the code originally (missed the
"break", and thought we always cleared the select timeout; I still think
that's what we *should* do).

>> Is this at all what you meant?  I realize it's incomplete, but it's
>> probably worth it to do things properly.
>
> Yes, that's going in the right direction, I think.  Thanks.

Before you read the patch: bindings.el doesn't seem like the best place
for this code; it's related to signal handlers, and I added a comment
while I was there, but I couldn't find one right away.  Any suggestions?

As little C code as possible, and we'll just run with the default signal
handler until Lisp fixes it.

Potentially separate bug:

In emacs -Q, the variable documentation contains the bytecode
representation of the lambda watcher:

  Calls these functions when changed: (#[1028 \300%3!\207 [debug--on-event] 6 

(fn SYMBOL NEWVAL OPERATION WHERE)])

I don't think ordinary Emacs users can read byte-code objects.  I
certainly cannot.

Should we fix that in the documentation code, work around it by naming
the watcher, or both?

diff --git a/lisp/bindings.el b/lisp/bindings.el
index 9987f28c027..628a71d2b4c 100644
--- a/lisp/bindings.el
+++ b/lisp/bindings.el
@@ -1642,7 +1642,37 @@ ctl-x-4-map
 (define-key ctl-x-4-map "c" 'clone-indirect-buffer-other-window)
 
 ;; Signal handlers
+(defun debug-on-event (event)
+  "Make EVENT enter the debugger.
+
+When Emacs receives the event specified as the argument to this
+function, it will try to break into the debugger as soon as possible
+instead of processing the event normally through `special-event-map'.
+
+EVENT should be a symbol, either `sigusr1' or `sigusr2', or nil."
+  (debug--on-event event))
+
+(defvar debug-on-event 'sigusr2
+  "Enter debugger on this event.
+When Emacs receives the event specified by this variable,
+it will try to break into the debugger as soon as possible instead
+of processing the event normally through `special-event-map'.
+
+Currently, the only supported values for this
+variable are `sigusr1' and `sigusr2'.")
+
+(make-obsolete-variable
+ 'debug-on-event
+ "use the function `debug-on-event' instead."
+ "31.0")
+
+(add-variable-watcher 'debug-on-event
+                      (lambda (_symbol newval _operation _where)
+                        (debug--on-event newval)))
+
 (define-key special-event-map [sigusr1] 'ignore)
+;; SIGUSR2, by default, invokes the debugger directly, so this binding
+;; is ignored.
 (define-key special-event-map [sigusr2] 'ignore)
 
 ;; Text conversion
diff --git a/src/keyboard.c b/src/keyboard.c
index fa19c9f8ad3..36a18976029 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -8254,6 +8254,10 @@ deliver_input_available_signal (int sig)
   /* Signal number.  */
   int sig;
 
+  /* True if this event is supposed to make us enter the debugger
+     instead of being treated as an input event.  */
+  bool debug_on_event;
+
   /* Name of the signal.  */
   char *name;
 
@@ -8280,6 +8284,7 @@ add_user_signal (int sig, const char *name)
   p = xmalloc (sizeof *p);
   p->sig = sig;
   p->name = xstrdup (name);
+  p->debug_on_event = !strcmp (p->name, "sigusr2");
   p->npending = 0;
   p->next = user_signals;
   user_signals = p;
@@ -8288,42 +8293,56 @@ add_user_signal (int sig, const char *name)
   sigaction (sig, &action, 0);
 }
 
+DEFUN ("debug--on-event", Fdebug__on_event, Sdebug__on_event, 1, 1, 0,
+       doc: /* Internal function: use debug-on-event instead.  */)
+  (Lisp_Object event)
+{
+  Lisp_Object ret = Qnil;
+  CHECK_SYMBOL (event);
+
+  struct user_signal_info *p;
+  for (p = user_signals; p; p = p->next)
+    if (strcmp (SSDATA (SYMBOL_NAME (event)), p->name) == 0)
+      {
+	p->debug_on_event = true;
+	ret = Qt;
+      }
+    else
+      p->debug_on_event = false;
+
+  return ret;
+}
+
 static void
 handle_user_signal (int sig)
 {
   struct user_signal_info *p;
-  const char *special_event_name = NULL;
-
-  if (SYMBOLP (Vdebug_on_event))
-    special_event_name = SSDATA (SYMBOL_NAME (Vdebug_on_event));
 
   for (p = user_signals; p; p = p->next)
     if (p->sig == sig)
       {
-        if (special_event_name
-	    && strcmp (special_event_name, p->name) == 0)
+	if (p->debug_on_event)
           {
             /* Enter the debugger in many ways.  */
             debug_on_next_call = true;
             debug_on_quit = true;
             Vquit_flag = Qt;
             Vinhibit_quit = Qnil;
-
-            /* Eat the event.  */
-            break;
           }
-
-	p->npending++;
-#if defined (USABLE_SIGIO) || defined (USABLE_SIGPOLL)
-	if (interrupt_input)
-	  handle_input_available_signal (sig);
 	else
-#endif
 	  {
-	    /* Tell wait_reading_process_output that it needs to wake
-	       up and look around.  */
-	    if (input_available_clear_time)
-	      *input_available_clear_time = make_timespec (0, 0);
+	    p->npending++;
+#if defined (USABLE_SIGIO) || defined (USABLE_SIGPOLL)
+	    if (interrupt_input)
+	      handle_input_available_signal (sig);
+	    else
+#endif
+	      {
+		/* Tell wait_reading_process_output that it needs to wake
+		   up and look around.  */
+		if (input_available_clear_time)
+		  *input_available_clear_time = make_timespec (0, 0);
+	      }
 	  }
 	break;
       }
@@ -13139,6 +13158,7 @@ syms_of_keyboard (void)
   defsubr (&Sevent_symbol_parse_modifiers);
   defsubr (&Sevent_convert_list);
   defsubr (&Sinternal_handle_focus_in);
+  defsubr (&Sdebug__on_event);
   defsubr (&Sread_key_sequence);
   defsubr (&Sread_key_sequence_vector);
   defsubr (&Srecursive_edit);
@@ -13787,17 +13807,6 @@ syms_of_keyboard (void)
   Vselection_inhibit_update_commands
     = list2 (Qhandle_switch_frame, Qhandle_select_window);
 
-  DEFVAR_LISP ("debug-on-event",
-               Vdebug_on_event,
-               doc: /* Enter debugger on this event.
-When Emacs receives the special event specified by this variable,
-it will try to break into the debugger as soon as possible instead
-of processing the event normally through `special-event-map'.
-
-Currently, the only supported values for this
-variable are `sigusr1' and `sigusr2'.  */);
-  Vdebug_on_event = Qsigusr2;
-
   DEFVAR_BOOL ("attempt-stack-overflow-recovery",
                attempt_stack_overflow_recovery,
                doc: /* If non-nil, attempt to recover from C stack overflows.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sun, 19 Jan 2025 15:45:55 +0200

> Date: Sun, 19 Jan 2025 13:37:18 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> > and at temacs time?
> 
> I don't think it's a realistic scenario that someone needs to debug
> non-interactive temacs, cannot use the default signal, and wants to
> change it.

I meant the interactive temacs, which begins by loading all the
preloaded packages.  So one of those could add the watcher.

> > Also, perhaps make the function debug-on-event call the watcher, not
> > itself be the watcher.  I think that might make the API cleaner.  The
> 
> You've lost me there.  I was thinking debug-on-event, a Lisp function,
> calls debug--on-event, a C subr, and we also install, from Lisp, a
> variable watcher on the deprecated debug-on-event variable.

That's what I meant, yes.

> Before you read the patch: bindings.el doesn't seem like the best place
> for this code; it's related to signal handlers, and I added a comment
> while I was there, but I couldn't find one right away.  Any suggestions?

One of the first files we load in loadup, I think?  Like subr.el,
perhaps?

> Potentially separate bug:
> 
> In emacs -Q, the variable documentation contains the bytecode
> representation of the lambda watcher:
> 
>   Calls these functions when changed: (#[1028 \300%3!\207 [debug--on-event] 6 
> 
> (fn SYMBOL NEWVAL OPERATION WHERE)])
> 
> I don't think ordinary Emacs users can read byte-code objects.  I
> certainly cannot.

It's not optimal, indeed.  See a somewhat better arrangement we have
with the variables which should trigger redisplay of the current
buffer, at the end of frame.el.

> Should we fix that in the documentation code, work around it by naming
> the watcher, or both?

See above: changing the way we call the watcher is probably easier,
and yields satisfactory results.

Thanks.

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Wed, 22 Jan 2025 12:30:45 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sun, 19 Jan 2025 13:37:18 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> > and at temacs time?
>>
>> I don't think it's a realistic scenario that someone needs to debug
>> non-interactive temacs, cannot use the default signal, and wants to
>> change it.
>
> I meant the interactive temacs, which begins by loading all the
> preloaded packages.  So one of those could add the watcher.

Do we really need this to be added early?  If you need another signal
before loadup finishes, you're modifying early Lisp anyway, and then you
can just call the builtin function rather than use the variable.

>> > Also, perhaps make the function debug-on-event call the watcher, not
>> > itself be the watcher.  I think that might make the API cleaner.  The
>>
>> You've lost me there.  I was thinking debug-on-event, a Lisp function,
>> calls debug--on-event, a C subr, and we also install, from Lisp, a
>> variable watcher on the deprecated debug-on-event variable.
>
> That's what I meant, yes.

I also tried to move debug-on-event (both the variable and the function)
to debug.el, which seems the most natural place.

The watcher is added in bindings.el, because that's the second place
people will look in when trying to figure out how to bind sigusr2 to a
different event.

>> Before you read the patch: bindings.el doesn't seem like the best place
>> for this code; it's related to signal handlers, and I added a comment
>> while I was there, but I couldn't find one right away.  Any suggestions?
>
> One of the first files we load in loadup, I think?  Like subr.el,
> perhaps?

See above.  I think if you need to put an --eval '(debug--on-event
(quote sigusr1))' in your temacs invocation, you're likely knowledgeable
enough to do so.

The only case that's really different is temacs -nl: it would probably
behave differently if it worked (to the extend bare temacs even does).

temacs -nl fixable, but I'll wait for a response before looking into
doing that properly.  Might not be worth it.

>> Potentially separate bug:
>>
>> In emacs -Q, the variable documentation contains the bytecode
>> representation of the lambda watcher:
>>
>>   Calls these functions when changed: (#[1028 \300%3!\207 [debug--on-event] 6
>>
>> (fn SYMBOL NEWVAL OPERATION WHERE)])
>>
>> I don't think ordinary Emacs users can read byte-code objects.  I
>> certainly cannot.
>
> It's not optimal, indeed.  See a somewhat better arrangement we have
> with the variables which should trigger redisplay of the current
> buffer, at the end of frame.el.

I've tried this:

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 9324cf85454..f4f595aedbe 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1712,7 +1712,9 @@ help-fns--var-watchpoints
     (when watchpoints
       (princ "  Calls these functions when changed: ")
       ;; FIXME: Turn function names into hyperlinks.
-      (princ watchpoints)
+      (dolist (watchpoint watchpoints)
+        (princ (help-fns-function-name watchpoint))
+        (princ "\n  "))
       (terpri))))
 
it generates a three-digit hex hash of the function, which is a lot
nicer than bytecode, but still not ideal.

Adding a docstring and a name to the function produces:

  Calls these functions when changed: (#[1028 \300!\207 [debug--on-event] 6 (bindings.elc . 52251)])

If there's a better way to fix this minor problem, we probably should.

>> Should we fix that in the documentation code, work around it by naming
>> the watcher, or both?
>
> See above: changing the way we call the watcher is probably easier,
> and yields satisfactory results.

Here's the current patch.  It doesn't feel quite right yet, but I can't
tell why right now:

diff --git a/lisp/bindings.el b/lisp/bindings.el
index 9987f28c027..bd02aefa348 100644
--- a/lisp/bindings.el
+++ b/lisp/bindings.el
@@ -1642,7 +1642,13 @@ ctl-x-4-map
 (define-key ctl-x-4-map "c" 'clone-indirect-buffer-other-window)
 
 ;; Signal handlers
+(add-variable-watcher 'debug-on-event
+                      (lambda (_symbol newval _operation _where)
+                        (debug--on-event newval)))
+
 (define-key special-event-map [sigusr1] 'ignore)
+;; SIGUSR2, by default, invokes the debugger directly, so this binding
+;; is ignored.
 (define-key special-event-map [sigusr2] 'ignore)
 
 ;; Text conversion
diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 0f7d7c3c020..cdf777f6c9a 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -388,11 +388,6 @@ minibuffer-prompt-properties--setter
 					    (const :tag "only shift-selection or mouse-drag" only)
 					    (const :tag "off" nil))
 				    "24.1")
-             (debug-on-event debug
-                             (choice (const :tag "None" nil)
-                                     (const :tag "When sent SIGUSR1" sigusr1)
-                                     (const :tag "When sent SIGUSR2" sigusr2))
-                             "24.1")
              (translate-upper-case-key-bindings keyboard boolean "29.1")
              ;; This is not good news because it will use the wrong
              ;; version-specific directories when you upgrade.  We need
diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index 0ca3a0f931c..ea2d89ef2a2 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -860,6 +860,39 @@ 'cancel-debug-watch
 
 (make-obsolete-variable 'debugger-previous-backtrace
                         "no longer used." "29.1")
+
+;;;###autoload
+(defun debug-on-event (event)
+  "Make EVENT enter the debugger.
+
+When Emacs receives the special event specified as the argument to this
+function, it will try to break into the debugger as soon as possible
+instead of processing the event normally through `special-event-map'.
+
+EVENT should be a symbol, either `sigusr1' or `sigusr2', or nil."
+  (setq debug-on-event event)
+  (debug--on-event event))
+
+;;;###autoload
+(defcustom debug-on-event 'sigusr2
+  "Enter debugger on this event.
+When Emacs receives the special event specified by this variable,
+it will try to break into the debugger as soon as possible instead
+of processing the event normally through `special-event-map'.
+
+Currently, the only supported values for this
+variable are `sigusr1' and `sigusr2'."
+  :group 'debug
+  :type '(choice (const :tag "None" nil)
+                 (const :tag "When sent SIGUSR1" sigusr1)
+                 (const :tag "When sent SIGUSR2" sigusr2))
+  :version "24.1")
+
+(make-obsolete-variable
+ 'debug-on-event
+ "use the function `debug-on-event' instead."
+ "31.0")
+
 (defvar debugger-previous-backtrace nil)
 
 (provide 'debug)
diff --git a/src/keyboard.c b/src/keyboard.c
index 6208b333e9c..d2043d669cf 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -8254,6 +8254,10 @@ deliver_input_available_signal (int sig)
   /* Signal number.  */
   int sig;
 
+  /* True if this event is supposed to make us enter the debugger
+     instead of being treated as an input event.  */
+  bool debug_on_event;
+
   /* Name of the signal.  */
   char *name;
 
@@ -8280,6 +8284,7 @@ add_user_signal (int sig, const char *name)
   p = xmalloc (sizeof *p);
   p->sig = sig;
   p->name = xstrdup (name);
+  p->debug_on_event = !strcmp (p->name, "sigusr2");
   p->npending = 0;
   p->next = user_signals;
   user_signals = p;
@@ -8288,42 +8293,56 @@ add_user_signal (int sig, const char *name)
   sigaction (sig, &action, 0);
 }
 
+DEFUN ("debug--on-event", Fdebug__on_event, Sdebug__on_event, 1, 1, 0,
+       doc: /* Internal function: use debug-on-event instead.  */)
+  (Lisp_Object event)
+{
+  Lisp_Object ret = Qnil;
+  CHECK_SYMBOL (event);
+
+  struct user_signal_info *p;
+  for (p = user_signals; p; p = p->next)
+    if (strcmp (SSDATA (SYMBOL_NAME (event)), p->name) == 0)
+      {
+	p->debug_on_event = true;
+	ret = Qt;
+      }
+    else
+      p->debug_on_event = false;
+
+  return ret;
+}
+
 static void
 handle_user_signal (int sig)
 {
   struct user_signal_info *p;
-  const char *special_event_name = NULL;
-
-  if (SYMBOLP (Vdebug_on_event))
-    special_event_name = SSDATA (SYMBOL_NAME (Vdebug_on_event));
 
   for (p = user_signals; p; p = p->next)
     if (p->sig == sig)
       {
-        if (special_event_name
-	    && strcmp (special_event_name, p->name) == 0)
+	if (p->debug_on_event)
           {
             /* Enter the debugger in many ways.  */
             debug_on_next_call = true;
             debug_on_quit = true;
             Vquit_flag = Qt;
             Vinhibit_quit = Qnil;
-
-            /* Eat the event.  */
-            break;
           }
-
-	p->npending++;
-#if defined (USABLE_SIGIO) || defined (USABLE_SIGPOLL)
-	if (interrupt_input)
-	  handle_input_available_signal (sig);
 	else
-#endif
 	  {
-	    /* Tell wait_reading_process_output that it needs to wake
-	       up and look around.  */
-	    if (input_available_clear_time)
-	      *input_available_clear_time = make_timespec (0, 0);
+	    p->npending++;
+#if defined (USABLE_SIGIO) || defined (USABLE_SIGPOLL)
+	    if (interrupt_input)
+	      handle_input_available_signal (sig);
+	    else
+#endif
+	      {
+		/* Tell wait_reading_process_output that it needs to wake
+		   up and look around.  */
+		if (input_available_clear_time)
+		  *input_available_clear_time = make_timespec (0, 0);
+	      }
 	  }
 	break;
       }
@@ -13139,6 +13158,7 @@ syms_of_keyboard (void)
   defsubr (&Sevent_symbol_parse_modifiers);
   defsubr (&Sevent_convert_list);
   defsubr (&Sinternal_handle_focus_in);
+  defsubr (&Sdebug__on_event);
   defsubr (&Sread_key_sequence);
   defsubr (&Sread_key_sequence_vector);
   defsubr (&Srecursive_edit);
@@ -13787,17 +13807,6 @@ syms_of_keyboard (void)
   Vselection_inhibit_update_commands
     = list2 (Qhandle_switch_frame, Qhandle_select_window);
 
-  DEFVAR_LISP ("debug-on-event",
-               Vdebug_on_event,
-               doc: /* Enter debugger on this event.
-When Emacs receives the special event specified by this variable,
-it will try to break into the debugger as soon as possible instead
-of processing the event normally through `special-event-map'.
-
-Currently, the only supported values for this
-variable are `sigusr1' and `sigusr2'.  */);
-  Vdebug_on_event = Qsigusr2;
-
   DEFVAR_BOOL ("attempt-stack-overflow-recovery",
                attempt_stack_overflow_recovery,
                doc: /* If non-nil, attempt to recover from C stack overflows.


Thanks for the comments!  I hope I didn't run off and do something
totally different from what you intended.

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Wed, 22 Jan 2025 17:04:17 +0200

> Date: Wed, 22 Jan 2025 12:30:45 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> Date: Sun, 19 Jan 2025 13:37:18 +0000
> >> From: Pip Cet <pipcet <at> protonmail.com>
> >> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> >>
> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> >>
> >> > and at temacs time?
> >>
> >> I don't think it's a realistic scenario that someone needs to debug
> >> non-interactive temacs, cannot use the default signal, and wants to
> >> change it.
> >
> > I meant the interactive temacs, which begins by loading all the
> > preloaded packages.  So one of those could add the watcher.
> 
> Do we really need this to be added early?

Only if we care about sending SIGUSR2 before bindings.el is loaded.

I guess we can leave it in bindings.el, and wait for someone to
complain that it doesn't work earlier.

> > One of the first files we load in loadup, I think?  Like subr.el,
> > perhaps?
> 
> See above.  I think if you need to put an --eval '(debug--on-event
> (quote sigusr1))' in your temacs invocation, you're likely knowledgeable
> enough to do so.

Does it really help?  I think temacs loads the preloaded files before
it processes --eval on the command line, isn't that so?

> The only case that's really different is temacs -nl: it would probably
> behave differently if it worked (to the extend bare temacs even does).

Not sure "temacs -nl" is important in this context.  I've never used
that except for debugging very rare and special problems.

> temacs -nl fixable, but I'll wait for a response before looking into
> doing that properly.  Might not be worth it.

I wouldn't complicate the code because of "temacs -nl".

> > It's not optimal, indeed.  See a somewhat better arrangement we have
> > with the variables which should trigger redisplay of the current
> > buffer, at the end of frame.el.
> 
> I've tried this:
> 
> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
> index 9324cf85454..f4f595aedbe 100644
> --- a/lisp/help-fns.el
> +++ b/lisp/help-fns.el
> @@ -1712,7 +1712,9 @@ help-fns--var-watchpoints
>      (when watchpoints
>        (princ "  Calls these functions when changed: ")
>        ;; FIXME: Turn function names into hyperlinks.
> -      (princ watchpoints)
> +      (dolist (watchpoint watchpoints)
> +        (princ (help-fns-function-name watchpoint))
> +        (princ "\n  "))
>        (terpri))))
>  
> it generates a three-digit hex hash of the function, which is a lot
> nicer than bytecode, but still not ideal.
> 
> Adding a docstring and a name to the function produces:
> 
>   Calls these functions when changed: (#[1028 \300!\207 [debug--on-event] 6 (bindings.elc . 52251)])
> 
> If there's a better way to fix this minor problem, we probably should.

This might be a misunderstanding.  I meant to use a subr, as we do in
frame.el.  "C-h v" then says:

    Calls these functions when changed: (#<subr set-buffer-redisplay>)

Isn't that slightly nicer than

  (#[1028 \300!\207 [debug--on-event] 6 (bindings.elc . 52251)]) 

?  That would mean make debug--on-event a subr, not a DEFUN.

> @@ -8280,6 +8284,7 @@ add_user_signal (int sig, const char *name)
>    p = xmalloc (sizeof *p);
>    p->sig = sig;
>    p->name = xstrdup (name);
> +  p->debug_on_event = !strcmp (p->name, "sigusr2");

Hmm... why only sigusr2? what about sigusr1?

> I hope I didn't run off and do something totally different from what
> you intended.

Doesn't look like that, no.

Thanks.

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Wed, 22 Jan 2025 15:55:05 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Wed, 22 Jan 2025 12:30:45 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> >> Date: Sun, 19 Jan 2025 13:37:18 +0000
>> >> From: Pip Cet <pipcet <at> protonmail.com>
>> >> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
>> >>
>> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>> >>
>> >> > and at temacs time?
>> >>
>> >> I don't think it's a realistic scenario that someone needs to debug
>> >> non-interactive temacs, cannot use the default signal, and wants to
>> >> change it.
>> >
>> > I meant the interactive temacs, which begins by loading all the
>> > preloaded packages.  So one of those could add the watcher.
>>
>> Do we really need this to be added early?
>
> Only if we care about sending SIGUSR2 before bindings.el is loaded.
>
> I guess we can leave it in bindings.el, and wait for someone to
> complain that it doesn't work earlier.

I'm not sure I understand: you're thinking someone might modify one of
the dozen or so Lisp files that are loaded before, in the Emacs tree, to
attempt to set an obsolete variable to allow entering the debugger,
which would also require changing things around so the debugger could
even be loaded at that point?  I'm confident that we'd have to wait for
some time.

>> > One of the first files we load in loadup, I think?  Like subr.el,
>> > perhaps?
>>
>> See above.  I think if you need to put an --eval '(debug--on-event
>> (quote sigusr1))' in your temacs invocation, you're likely knowledgeable
>> enough to do so.
>
> Does it really help?  I think temacs loads the preloaded files before
> it processes --eval on the command line, isn't that so?

You're right, but it turns out SIGUSR2 doesn't do anything noticeable
with or without the patch, so SIGUSR1 can't, either.

>> > It's not optimal, indeed.  See a somewhat better arrangement we have
>> > with the variables which should trigger redisplay of the current
>> > buffer, at the end of frame.el.
>>
>> I've tried this:
>>
>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>> index 9324cf85454..f4f595aedbe 100644
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -1712,7 +1712,9 @@ help-fns--var-watchpoints
>>      (when watchpoints
>>        (princ "  Calls these functions when changed: ")
>>        ;; FIXME: Turn function names into hyperlinks.
>> -      (princ watchpoints)
>> +      (dolist (watchpoint watchpoints)
>> +        (princ (help-fns-function-name watchpoint))
>> +        (princ "\n  "))
>>        (terpri))))
>>
>> it generates a three-digit hex hash of the function, which is a lot
>> nicer than bytecode, but still not ideal.
>>
>> Adding a docstring and a name to the function produces:
>>
>>   Calls these functions when changed: (#[1028 \300!\207 [debug--on-event] 6 (bindings.elc . 52251)])
>>
>> If there's a better way to fix this minor problem, we probably should.
>
> This might be a misunderstanding.  I meant to use a subr, as we do in
> frame.el.  "C-h v" then says:

You said:
> Can we add the watcher in Lisp instead?  Are there any reasons to have
> it in C and at temacs time?

I realize now that you can read the first sentence two ways, but the
second one seems clear to me: there are no reasons to have the watcher
in C.  But as that patch had the defun in C, we can switch back to it.

>     Calls these functions when changed: (#<subr set-buffer-redisplay>)
>
> Isn't that slightly nicer than
>
>   (#[1028 \300!\207 [debug--on-event] 6 (bindings.elc . 52251)])
>
> ?  That would mean make debug--on-event a subr, not a DEFUN.

Let's add the symbol, not the function cell, and everything will be
fine.

We still need to fix the output of C-h v when watchers are in effect,
but that's a separate issue.

However, I also don't understand how a subr, which is defined by DEFUN,
is different from "a DEFUN".  Did you mean to write "defun"?

>> @@ -8280,6 +8284,7 @@ add_user_signal (int sig, const char *name)
>>    p = xmalloc (sizeof *p);
>>    p->sig = sig;
>>    p->name = xstrdup (name);
>> +  p->debug_on_event = !strcmp (p->name, "sigusr2");
>
> Hmm... why only sigusr2? what about sigusr1?

The default behavior is for sigusr2 to enter the debugger and for
sigusr1 to execute its binding, which is 'ignore.  Changing SIGUSR1 to
also enter the debugger by default would be a breaking change and
require significant documentation adjustments.  (Also, why?)

Pip

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Wed, 22 Jan 2025 18:12:37 +0200

> Date: Wed, 22 Jan 2025 15:55:05 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> However, I also don't understand how a subr, which is defined by DEFUN,
> is different from "a DEFUN".  Did you mean to write "defun"?

I guess so.

> >> @@ -8280,6 +8284,7 @@ add_user_signal (int sig, const char *name)
> >>    p = xmalloc (sizeof *p);
> >>    p->sig = sig;
> >>    p->name = xstrdup (name);
> >> +  p->debug_on_event = !strcmp (p->name, "sigusr2");
> >
> > Hmm... why only sigusr2? what about sigusr1?
> 
> The default behavior is for sigusr2 to enter the debugger and for
> sigusr1 to execute its binding, which is 'ignore.  Changing SIGUSR1 to
> also enter the debugger by default would be a breaking change and
> require significant documentation adjustments.  (Also, why?)

I guess I was confused and thought that both SIGUSR1 and SIGUSR2 were
supposed to start the debugger.

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

From: Richard Stallman <rms <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: pipcet <at> protonmail.com, yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Fri, 24 Jan 2025 22:05:31 -0500

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I guess I was confused and thought that both SIGUSR1 and SIGUSR2 were
  > supposed to start the debugger.

Could people make sure that the documentation is up to date about this
point in all the places it is described?

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Richard Stallman <rms <at> gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sat, 25 Jan 2025 09:31:05 +0000

"Richard Stallman" <rms <at> gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
>   > I guess I was confused and thought that both SIGUSR1 and SIGUSR2 were
>   > supposed to start the debugger.
>
> Could people make sure that the documentation is up to date about this
> point in all the places it is described?

It currently isn't; the section on sigusr1/sigusr2 in commands.texi
doesn't mention debug-on-event, and the section on debug-on-event in
debugging.texi doesn't mention its default value.

That means anyone attempting to bind sigusr2 as a key will be in for a
big surprise, because the curent behavior is to enter the debugger by
default.

This diff may explain better than a long list what I'd like to change
about the documentation, even if the wording is bad.

It is very rare to need to alter or call (after the patch) the
debug-on-error variable/function.  Setting it to nil may be useful in
exceptional situations, but I don't actually see the need for using
SIGUSR1 at this time.  My impression was that this was originally meant
to be expanded to more signals (that would be useful, as some other
programs generate specific events and it's hard to modify them), but
that never happened.

Even if the sentence about signals not carrying additional information
were accurate (on GNU/Linux, it simply isn't), I don't see the relevance
to this section at all, so I removed it.

Again, the wording can and should be improved, but the old docs are
inaccurate and the new ones, I hope, less so.

Pip

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index 39514145a1e..9ccc07a3291 100644
--- a/doc/lispref/commands.texi
+++ b/doc/lispref/commands.texi
@@ -2731,16 +2731,20 @@ Misc Events
 @cindex user signals
 @item sigusr1
 @itemx sigusr2
-These events are generated when the Emacs process receives
-the signals @code{SIGUSR1} and @code{SIGUSR2}.  They contain no
-additional data because signals do not carry additional information.
-They can be useful for debugging (@pxref{Error Debugging}).
-
-To catch a user signal, bind the corresponding event to an interactive
-command in the @code{special-event-map} (@pxref{Controlling Active Maps}).
-The command is called with no arguments, and the specific signal event is
-available in @code{last-input-event} (@pxref{Event Input Misc}.  For
-example:
+When the Emacs process receives @code{SIGUSR1}, the @code{sigusr1} event
+is generated and treated as an input event.  This event contains no
+additional data.  This can be useful for debugging (@pxref{Error
+Debugging}).
+
+It is possible to modify @code{debug-on-event} to do the same when
+@code{SIGUSR2} is received instead, or for both events.  By default,
+binding @code{sigusr2} has no effect.
+
+To catch a user signal which is not used by @code{debug-on-event}, bind
+the corresponding event to an interactive command in the
+@code{special-event-map} (@pxref{Controlling Active Maps}).  The command
+is called with no arguments, and the specific signal event is available
+in @code{last-input-event} (@pxref{Event Input Misc}.  For example:
 
 @smallexample
 (defun sigusr-handler ()
diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
index 9b36f9ecdb2..d9b12137b4b 100644
--- a/doc/lispref/debugging.texi
+++ b/doc/lispref/debugging.texi
@@ -112,6 +112,14 @@ Error Debugging
 defined in this subsection to debug errors in Lisp that the redisplay
 code has invoked.  @xref{Debugging Redisplay}, for help with these.
 
+One way to invoke the debugger even in extreme situations is to send
+Emacs a @code{SIGUSR2} signal.  This will make Emacs try to enter the
+debugger as soon as possible after the signal.  There is a small risk
+this results in unexpected or erroneous behavior.
+
+In this case, @code{special-event-map} or other bindings are not
+consulted.
+
 @defopt debug-on-error
 This variable determines whether the debugger is called when an error
 is signaled and not handled.  If @code{debug-on-error} is @code{t},
@@ -187,12 +195,13 @@ Error Debugging
 @end defopt
 
 @defopt debug-on-event
-If you set @code{debug-on-event} to a special event (@pxref{Special
-Events}), Emacs will try to enter the debugger as soon as it receives
-this event, bypassing @code{special-event-map}.  At present, the only
-supported values correspond to the signals @code{SIGUSR1} and
-@code{SIGUSR2} (this is the default).  This can be helpful when
-@code{inhibit-quit} is set and Emacs is not otherwise responding.
+You can change which signals enter the debugger by setting
+@code{debug-on-event} to an event name; the default is @code{sigusr2}.
+This can be helpful when @code{inhibit-quit} is set and
+Emacs is not otherwise responding.
+
+At present, the only other supported values are @code{sigusr1}, to use
+@code{SIGUSR1} instead, and @code{nil}, disabling the feature entirely.
 @end defopt
 
 @cindex message, finding what causes a particular message

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

From: Richard Stallman <rms <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: eliz <at> gnu.org, yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sun, 26 Jan 2025 13:17:39 -0500

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > This diff may explain better than a long list what I'd like to change
  > about the documentation, even if the wording is bad.

Thanks for getting this started.  I expect that the other people here
will help fix any little details and get this installed.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Previous Next

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