MPS: profiler

MPS: profiler

[Ihor Radchenko]

Hi,

I am playing around with scratch/igc branch for fun, and there is one
crash that I can reproduce quite consistently.

All it takes is (1) compile Emacs without native-compilation support;
(2) open Emacs; (3) M-x profiler-start; (4) run a complex operation.

Steps to reproduce:

1. emacs -Q doc/misc/org.org
2. M-x profiler-start RET cpu RET
3. M-: (org-element-parse-buffer) RET

Without step 2, no crash.

Hope it helps.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>, Eli Zaretskii
>  <eliz@gnu.org>,
>  eller.helmut@gmail.com
> Date: Thu, 20 Jun 2024 19:25:26 +0000
> 
> I am playing around with scratch/igc branch for fun, and there is one
> crash that I can reproduce quite consistently.
> 
> All it takes is (1) compile Emacs without native-compilation support;
> (2) open Emacs; (3) M-x profiler-start; (4) run a complex operation.
> 
> Steps to reproduce:
> 
> 1. emacs -Q doc/misc/org.org
> 2. M-x profiler-start RET cpu RET
> 3. M-: (org-element-parse-buffer) RET

Doesn't crash here, but then we emulate SIGPROF on Windows in a way
that could explain that.

Re: MPS: profiler

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Steps to reproduce:
>> 
>> 1. emacs -Q doc/misc/org.org
>> 2. M-x profiler-start RET cpu RET
>> 3. M-: (org-element-parse-buffer) RET
>
> Doesn't crash here, but then we emulate SIGPROF on Windows in a way
> that could explain that.

I am on Linux:

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-06-20 built on localhost
Repository revision: 5dfba4a94315f1ece8f05036eaf701cb741816b6
Repository branch: scratch/igc
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: Gentoo Linux

Configured using:
 'configure --with-mps=yes --without-native-compilation
 JAVAC=/etc/java-config-2/current-system-vm/bin/javac
 'CFLAGS=-I/opt/mps/include -L/opt/mps/lib''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LCMS2 LIBXML2 MODULES MPS 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 contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Hi,
>
> I am playing around with scratch/igc branch for fun, and there is one
> crash that I can reproduce quite consistently.
>
> All it takes is (1) compile Emacs without native-compilation support;
> (2) open Emacs; (3) M-x profiler-start; (4) run a complex operation.
>
> Steps to reproduce:
>
> 1. emacs -Q doc/misc/org.org
> 2. M-x profiler-start RET cpu RET
> 3. M-: (org-element-parse-buffer) RET
>
> Without step 2, no crash.
>
> Hope it helps.

Doesn't seem to happen here. Strangely, I get something like this in
*Messages*, but no crash.

...
CPU profiler started
(org-data (:standard-properties [1 1 1 821876 821876 0 nil org-data nil
nil nil 3 ...] :path "/Users/gerd/emacs/github/igc/doc/misc/org.org"
:CATEGORY "org") (section (:standard-properties [1 1 1 142 142 0 nil
first-section nil nil nil 1 ...]) (keyword (:standard-properties [1 1
nil nil 25 0 nil top-comment nil nil nil nil ...] :key "TITLE" :value
"The Org Manual")) (keyword (:standard-properties [25 25 nil nil 60
...

What system are you using? And, maybe, does a git pull help? It's been
moving quite fast the last days.

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Steps to reproduce:
>>> 
>>> 1. emacs -Q doc/misc/org.org
>>> 2. M-x profiler-start RET cpu RET
>>> 3. M-: (org-element-parse-buffer) RET
>>
>> Doesn't crash here, but then we emulate SIGPROF on Windows in a way
>> that could explain that.
>
> I am on Linux:
>
> In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
>  3.24.41, cairo version 1.18.0) of 2024-06-20 built on localhost
> Repository revision: 5dfba4a94315f1ece8f05036eaf701cb741816b6

Thanks, that's the latest and greatest. I'm afraid that means it's time
to get out the debugger :-).

Re: MPS: profiler

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Doesn't seem to happen here. Strangely, I get something like this in
> *Messages*, but no crash.
>
> ...
> CPU profiler started
> (org-data...

That's fine - it is `org-element-parse-buffer' return value.

> What system are you using?

Linux. See my reply to Eli.

> ... And, maybe, does a git pull help? It's been
> moving quite fast the last days.

I did it an hour ago. I guess that it is not moving *that* fast :)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

>>>> Steps to reproduce:
>>>> 
>>>> 1. emacs -Q doc/misc/org.org
>>>> 2. M-x profiler-start RET cpu RET
>>>> 3. M-: (org-element-parse-buffer) RET
> ...
> Thanks, that's the latest and greatest. I'm afraid that means it's time
> to get out the debugger :-).

[yantar92:~/Git/emacs] scratch/igc(+2/-2)+ 1m9s ± 
> gdb --args ./src/emacs -Q
GNU gdb (Gentoo 14.2 vanilla) 14.2
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://bugs.gentoo.org/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./src/emacs...
(gdb) run
Starting program: /home/yantar92/Git/emacs/src/emacs -Q
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7fffe20006c0 (LWP 4692)]
[New Thread 0x7fffe16006c0 (LWP 4693)]
[New Thread 0x7fffe0c006c0 (LWP 4694)]
[New Thread 0x7fffdbe006c0 (LWP 4695)]
[New Thread 0x7fffdac006c0 (LWP 4696)]
[New Thread 0x7fffda2006c0 (LWP 4697)]
[Detaching after vfork from child process 4751]
[Detaching after vfork from child process 4765]
[Detaching after vfork from child process 4766]
[Detaching after vfork from child process 4767]
[Detaching after vfork from child process 4768]
[Detaching after vfork from child process 4769]
[Detaching after vfork from child process 4770]
[Detaching after vfork from child process 4771]
[Detaching after vfork from child process 4782]
[Detaching after vfork from child process 4783]
[Thread 0x7fffdac006c0 (LWP 4696) exited]

Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
copy_intervals (tree=0x7fffe651f500, start=587977, length=11) at intervals.c:2247
2247	 got = (LENGTH (i) - (start - i->position));
(gdb) bt
#0  copy_intervals (tree=0x7fffe651f500, start=587977, length=11) at intervals.c:2247
#1  0x00005555558a1ee6 in copy_intervals_to_string (string=0x7fffeae8dfb4, buffer=0x7fffe4275e90, position=587977, length=11) at intervals.c:2272
#2  0x0000555555800168 in make_buffer_string_both (start=587977, start_byte=588047, end=587988, end_byte=588058, props=true) at editfns.c:1615
#3  0x00005555557ffe53 in make_buffer_string (start=587977, end=587988, props=true) at editfns.c:1554
#4  0x0000555555800302 in Fbuffer_substring (start=0x23e326, end=0x23e352) at editfns.c:1666
#5  0x0000555555870a1d in exec_byte_code (fun=0x7fffe446a465, args_template=771, nargs=3, args=0x7fffe2200928) at bytecode.c:1516
#6  0x00005555558158df in funcall_lambda (fun=0x7fffe5b0842d, nargs=0, arg_vector=0x7fffffffb880) at eval.c:3270
#7  0x0000555555815762 in apply_lambda (fun=0x7fffe5b0842d, args=0x0, count=...) at eval.c:3233
#8  0x0000555555813c6d in eval_sub (form=0x7fffe6dde54b) at eval.c:2663
#9  0x0000555555813170 in Feval (form=0x7fffe6dde54b, lexical=0x30) at eval.c:2475
#10 0x0000555555815303 in funcall_subr (subr=0x555555f2d380 <Seval>, numargs=2, args=0x7fffe22001b0) at eval.c:3181
#11 0x000055555586d392 in exec_byte_code (fun=0x7fffe3170b85, args_template=513, nargs=2, args=0x7fffe2200320) at bytecode.c:827
#12 0x00005555558158df in funcall_lambda (fun=0x7fffe6dde75d, nargs=0, arg_vector=0x7fffffffc110) at eval.c:3270
#13 0x0000555555814d61 in funcall_general (fun=0x7fffe6dde75d, numargs=0, args=0x7fffffffc110) at eval.c:3062
#14 0x0000555555814fc6 in Ffuncall (nargs=1, args=0x7fffffffc108) at eval.c:3111
#15 0x000055555580da20 in call0 (fn=0x7fffe6dde75d) at /home/yantar92/Git/emacs/src/lisp.h:3559
#16 0x0000555555810cc9 in Fhandler_bind_1 (nargs=3, args=0x7fffe2200128) at eval.c:1487
#17 0x000055555581552d in funcall_subr (subr=0x555555f2d240 <Shandler_bind_1>, numargs=3, args=0x7fffe2200128) at eval.c:3202
#18 0x000055555586d392 in exec_byte_code (fun=0x7fffe31696c5, args_template=1025, nargs=4, args=0x7fffffffc990) at bytecode.c:827
#19 0x00005555558158df in funcall_lambda (fun=0x7fffe31696c5, nargs=4, arg_vector=0x7fffffffc970) at eval.c:3270
#20 0x0000555555814d61 in funcall_general (fun=0x7fffe31696c5, numargs=4, args=0x7fffffffc970) at eval.c:3062
#21 0x0000555555814fc6 in Ffuncall (nargs=5, args=0x7fffffffc968) at eval.c:3111
#22 0x000055555580a532 in Ffuncall_interactively (nargs=5, args=0x7fffffffc968) at callint.c:250
#23 0x000055555581552d in funcall_subr (subr=0x555555f2ca80 <Sfuncall_interactively>, numargs=5, args=0x7fffffffc968) at eval.c:3202
#24 0x0000555555814d15 in funcall_general (fun=0x555555f2ca85 <Sfuncall_interactively+5>, numargs=5, args=0x7fffffffc968) at eval.c:3058
#25 0x0000555555814fc6 in Ffuncall (nargs=6, args=0x7fffffffc960) at eval.c:3111
#26 0x000055555581446d in Fapply (nargs=3, args=0x7fffffffcbf0) at eval.c:2783
#27 0x000055555580a92c in Fcall_interactively (function=0x2aaa8d1c2b08, record_flag=0x0, keys=0x7fffe6d46865) at callint.c:342
#28 0x0000555555815339 in funcall_subr (subr=0x555555f2cac0 <Scall_interactively>, numargs=3, args=0x7fffe2200070) at eval.c:3183
#29 0x000055555586d392 in exec_byte_code (fun=0x7fffe36e5c7d, args_template=1025, nargs=1, args=0x7fffffffd350) at bytecode.c:827
#30 0x00005555558158df in funcall_lambda (fun=0x7fffe36e5c7d, nargs=1, arg_vector=0x7fffffffd348) at eval.c:3270
#31 0x0000555555814d61 in funcall_general (fun=0x7fffe36e5c7d, numargs=1, args=0x7fffffffd348) at eval.c:3062
#32 0x0000555555814fc6 in Ffuncall (nargs=2, args=0x7fffffffd340) at eval.c:3111
#33 0x000055555574d687 in command_loop_1 () at keyboard.c:1551
#34 0x00005555558113c9 in internal_condition_case (bfun=0x55555574ce24 <command_loop_1>, handlers=0x90, hfun=0x55555574c355 <cmd_error>) at eval.c:1622
#35 0x000055555574ca71 in command_loop_2 (handlers=0x90) at keyboard.c:1169
#36 0x0000555555810872 in internal_catch (tag=0x12300, func=0x55555574ca47 <command_loop_2>, arg=0x90) at eval.c:1301
#37 0x000055555574ca03 in command_loop () at keyboard.c:1147
#38 0x000055555574bef7 in recursive_edit_1 () at keyboard.c:755
#39 0x000055555574c0a3 in Frecursive_edit () at keyboard.c:838
#40 0x00005555557482fc in main (argc=2, argv=0x7fffffffd838) at emacs.c:2643
(gdb) c
Continuing.

Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
0x00005555558a1dd8 in copy_intervals (tree=0x7fffe651f500, start=587977, length=11) at intervals.c:2247
2247	 got = (LENGTH (i) - (start - i->position));
(gdb) bt
#0  0x00005555558a1dd8 in copy_intervals (tree=0x7fffe651f500, start=587977, length=11) at intervals.c:2247
#1  0x00005555558a1ee6 in copy_intervals_to_string (string=0x7fffeae8dfb4, buffer=0x7fffe4275e90, position=587977, length=11) at intervals.c:2272
#2  0x0000555555800168 in make_buffer_string_both (start=587977, start_byte=588047, end=587988, end_byte=588058, props=true) at editfns.c:1615
#3  0x00005555557ffe53 in make_buffer_string (start=587977, end=587988, props=true) at editfns.c:1554
#4  0x0000555555800302 in Fbuffer_substring (start=0x23e326, end=0x23e352) at editfns.c:1666
#5  0x0000555555870a1d in exec_byte_code (fun=0x7fffe446a465, args_template=771, nargs=3, args=0x7fffe2200928) at bytecode.c:1516
#6  0x00005555558158df in funcall_lambda (fun=0x7fffe5b0842d, nargs=0, arg_vector=0x7fffffffb880) at eval.c:3270
#7  0x0000555555815762 in apply_lambda (fun=0x7fffe5b0842d, args=0x0, count=...) at eval.c:3233
#8  0x0000555555813c6d in eval_sub (form=0x7fffe6dde54b) at eval.c:2663
#9  0x0000555555813170 in Feval (form=0x7fffe6dde54b, lexical=0x30) at eval.c:2475
#10 0x0000555555815303 in funcall_subr (subr=0x555555f2d380 <Seval>, numargs=2, args=0x7fffe22001b0) at eval.c:3181
#11 0x000055555586d392 in exec_byte_code (fun=0x7fffe3170b85, args_template=513, nargs=2, args=0x7fffe2200320) at bytecode.c:827
#12 0x00005555558158df in funcall_lambda (fun=0x7fffe6dde75d, nargs=0, arg_vector=0x7fffffffc110) at eval.c:3270
#13 0x0000555555814d61 in funcall_general (fun=0x7fffe6dde75d, numargs=0, args=0x7fffffffc110) at eval.c:3062
#14 0x0000555555814fc6 in Ffuncall (nargs=1, args=0x7fffffffc108) at eval.c:3111
#15 0x000055555580da20 in call0 (fn=0x7fffe6dde75d) at /home/yantar92/Git/emacs/src/lisp.h:3559
#16 0x0000555555810cc9 in Fhandler_bind_1 (nargs=3, args=0x7fffe2200128) at eval.c:1487
#17 0x000055555581552d in funcall_subr (subr=0x555555f2d240 <Shandler_bind_1>, numargs=3, args=0x7fffe2200128) at eval.c:3202
#18 0x000055555586d392 in exec_byte_code (fun=0x7fffe31696c5, args_template=1025, nargs=4, args=0x7fffffffc990) at bytecode.c:827
#19 0x00005555558158df in funcall_lambda (fun=0x7fffe31696c5, nargs=4, arg_vector=0x7fffffffc970) at eval.c:3270
#20 0x0000555555814d61 in funcall_general (fun=0x7fffe31696c5, numargs=4, args=0x7fffffffc970) at eval.c:3062
#21 0x0000555555814fc6 in Ffuncall (nargs=5, args=0x7fffffffc968) at eval.c:3111
#22 0x000055555580a532 in Ffuncall_interactively (nargs=5, args=0x7fffffffc968) at callint.c:250
#23 0x000055555581552d in funcall_subr (subr=0x555555f2ca80 <Sfuncall_interactively>, numargs=5, args=0x7fffffffc968) at eval.c:3202
#24 0x0000555555814d15 in funcall_general (fun=0x555555f2ca85 <Sfuncall_interactively+5>, numargs=5, args=0x7fffffffc968) at eval.c:3058
#25 0x0000555555814fc6 in Ffuncall (nargs=6, args=0x7fffffffc960) at eval.c:3111
#26 0x000055555581446d in Fapply (nargs=3, args=0x7fffffffcbf0) at eval.c:2783
#27 0x000055555580a92c in Fcall_interactively (function=0x2aaa8d1c2b08, record_flag=0x0, keys=0x7fffe6d46865) at callint.c:342
#28 0x0000555555815339 in funcall_subr (subr=0x555555f2cac0 <Scall_interactively>, numargs=3, args=0x7fffe2200070) at eval.c:3183
#29 0x000055555586d392 in exec_byte_code (fun=0x7fffe36e5c7d, args_template=1025, nargs=1, args=0x7fffffffd350) at bytecode.c:827
#30 0x00005555558158df in funcall_lambda (fun=0x7fffe36e5c7d, nargs=1, arg_vector=0x7fffffffd348) at eval.c:3270
#31 0x0000555555814d61 in funcall_general (fun=0x7fffe36e5c7d, numargs=1, args=0x7fffffffd348) at eval.c:3062
#32 0x0000555555814fc6 in Ffuncall (nargs=2, args=0x7fffffffd340) at eval.c:3111
#33 0x000055555574d687 in command_loop_1 () at keyboard.c:1551
#34 0x00005555558113c9 in internal_condition_case (bfun=0x55555574ce24 <command_loop_1>, handlers=0x90, hfun=0x55555574c355 <cmd_error>) at eval.c:1622
#35 0x000055555574ca71 in command_loop_2 (handlers=0x90) at keyboard.c:1169
#36 0x0000555555810872 in internal_catch (tag=0x12300, func=0x55555574ca47 <command_loop_2>, arg=0x90) at eval.c:1301
#37 0x000055555574ca03 in command_loop () at keyboard.c:1147
#38 0x000055555574bef7 in recursive_edit_1 () at keyboard.c:755
#39 0x000055555574c0a3 in Frecursive_edit () at keyboard.c:838
#40 0x00005555557482fc in main (argc=2, argv=0x7fffffffd838) at emacs.c:2643
(gdb) c
Continuing.

Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
1104		 && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
(gdb) bt
#0  0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
#1  0x00005555558b567f in CLOSUREP (a=0x7fffe6dde715) at /home/yantar92/Git/emacs/src/lisp.h:3368
#2  0x00005555558b5b80 in trace_hash (trace=0x555556329400, depth=16) at profiler.c:189
#3  0x00005555558b5eab in record_backtrace (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:291
#4  0x00005555558b60fd in add_sample (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:353
#5  0x00005555558b6153 in handle_profiler_signal (signal=27) at profiler.c:396
#6  0x0000555555777704 in deliver_process_signal (sig=27, handler=0x5555558b6104 <handle_profiler_signal>) at sysdep.c:1758
#7  0x00005555558b6175 in deliver_profiler_signal (signal=27) at profiler.c:402
#8  0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6
#9  0x00007ffff5922d07 in mprotect () at /lib64/libc.so.6
#10 0x000055555595c498 in ProtSet (base=0x7fffe2d86000, limit=<optimized out>, mode=0) at protix.c:105
#11 0x000055555594f7dd in shieldProtLower (shield=shield@entry=0x7ffff7fc2700, seg=seg@entry=0x7fffe8001988, mode=mode@entry=3) at shield.c:305
#12 0x0000555555950417 in ShieldExpose (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8001988) at shield.c:737
#13 0x000055555595f045 in amcSegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at poolamc.c:1594
#14 0x0000555555948d31 in SegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at seg.c:793
#15 0x00005555559550c0 in _mps_fix2 (mps_ss=0x7fffffffa698, mps_ref_io=0x7fffffffa160) at trace.c:1433
#16 0x00005555558b83af in fix_lisp_obj (ss=0x7fffffffa698, pobj=0x7fffeb08b038) at igc.c:675
#17 0x00005555558b86b5 in fix_array (ss=0x7fffffffa698, array=0x7fffeb08b038, n=4) at igc.c:801
#18 0x00005555558babf6 in fix_vectorlike (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:1554
#19 0x00005555558bc692 in fix_vector (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:2086
#20 0x00005555558ba757 in dflt_scan_obj (ss=0x7fffffffa698, base_start=0x7fffeb08b028, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1481
#21 0x00005555558baac4 in dflt_scanx (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1531
#22 0x00005555558bab63 in dflt_scan (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608) at igc.c:1542
#23 0x000055555595568f in TraceScanFormat (ss=ss@entry=0x7fffffffa690, base=base@entry=0x7fffeb08b000, limit=limit@entry=0x7fffeb08c608) at trace.c:1539
#24 0x000055555595f609 in amcSegScan (totalReturn=0x7fffffffa68c, seg=0x7fffe8289870, ss=0x7fffffffa690) at poolamc.c:1427
#25 0x0000555555948c6f in SegScan (totalReturn=totalReturn@entry=0x7fffffffa68c, seg=seg@entry=0x7fffe8289870, ss=ss@entry=0x7fffffffa690) at seg.c:775
#26 0x0000555555954ae1 in traceScanSegRes (ts=ts@entry=1, rank=rank@entry=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1205
#27 0x0000555555954beb in traceScanSeg (ts=ts@entry=1, rank=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1267
#28 0x0000555555954d63 in TraceSegAccess (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870, mode=mode@entry=1) at trace.c:1320
#29 0x000055555594959b in SegWholeAccess (seg=0x7fffe8289870, arena=0x7ffff7fc2000, addr=0x7fffeb08c590, mode=1, context=0x7fffffffa8d0) at seg.c:1262
#30 0x0000555555948309 in SegAccess
    (seg=0x7fffe8289870, arena=arena@entry=0x7ffff7fc2000, addr=addr@entry=0x7fffeb08c590, mode=1, context=context@entry=0x7fffffffa8d0) at seg.c:723
#31 0x0000555555928f51 in ArenaAccess (addr=0x7fffeb08c590, mode=<optimized out>, mode@entry=3, context=context@entry=0x7fffffffa8d0) at global.c:671
#32 0x000055555595c6e2 in sigHandle (sig=<optimized out>, info=0x7fffffffabf0, uap=0x7fffffffaac0) at protsgix.c:97
#33 0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6
#34 0x00005555558a1dd8 in copy_intervals (tree=0x7fffe651f500, start=587977, length=11) at intervals.c:2247
#35 0x00005555558a1ee6 in copy_intervals_to_string (string=0x7fffeae8dfb4, buffer=0x7fffe4275e90, position=587977, length=11) at intervals.c:2272
#36 0x0000555555800168 in make_buffer_string_both (start=587977, start_byte=588047, end=587988, end_byte=588058, props=true) at editfns.c:1615
#37 0x00005555557ffe53 in make_buffer_string (start=587977, end=587988, props=true) at editfns.c:1554
#38 0x0000555555800302 in Fbuffer_substring (start=0x23e326, end=0x23e352) at editfns.c:1666
#39 0x0000555555870a1d in exec_byte_code (fun=0x7fffe446a465, args_template=771, nargs=3, args=0x7fffe2200928) at bytecode.c:1516
--Type <RET> for more, q to quit, c to continue without paging--c
#40 0x00005555558158df in funcall_lambda (fun=0x7fffe5b0842d, nargs=0, arg_vector=0x7fffffffb880) at eval.c:3270
#41 0x0000555555815762 in apply_lambda (fun=0x7fffe5b0842d, args=0x0, count=...) at eval.c:3233
#42 0x0000555555813c6d in eval_sub (form=0x7fffe6dde54b) at eval.c:2663
#43 0x0000555555813170 in Feval (form=0x7fffe6dde54b, lexical=0x30) at eval.c:2475
#44 0x0000555555815303 in funcall_subr (subr=0x555555f2d380 <Seval>, numargs=2, args=0x7fffe22001b0) at eval.c:3181
#45 0x000055555586d392 in exec_byte_code (fun=0x7fffe3170b85, args_template=513, nargs=2, args=0x7fffe2200320) at bytecode.c:827
#46 0x00005555558158df in funcall_lambda (fun=0x7fffe6dde75d, nargs=0, arg_vector=0x7fffffffc110) at eval.c:3270
#47 0x0000555555814d61 in funcall_general (fun=0x7fffe6dde75d, numargs=0, args=0x7fffffffc110) at eval.c:3062
#48 0x0000555555814fc6 in Ffuncall (nargs=1, args=0x7fffffffc108) at eval.c:3111
#49 0x000055555580da20 in call0 (fn=0x7fffe6dde75d) at /home/yantar92/Git/emacs/src/lisp.h:3559
#50 0x0000555555810cc9 in Fhandler_bind_1 (nargs=3, args=0x7fffe2200128) at eval.c:1487
#51 0x000055555581552d in funcall_subr (subr=0x555555f2d240 <Shandler_bind_1>, numargs=3, args=0x7fffe2200128) at eval.c:3202
#52 0x000055555586d392 in exec_byte_code (fun=0x7fffe31696c5, args_template=1025, nargs=4, args=0x7fffffffc990) at bytecode.c:827
#53 0x00005555558158df in funcall_lambda (fun=0x7fffe31696c5, nargs=4, arg_vector=0x7fffffffc970) at eval.c:3270
#54 0x0000555555814d61 in funcall_general (fun=0x7fffe31696c5, numargs=4, args=0x7fffffffc970) at eval.c:3062
#55 0x0000555555814fc6 in Ffuncall (nargs=5, args=0x7fffffffc968) at eval.c:3111
#56 0x000055555580a532 in Ffuncall_interactively (nargs=5, args=0x7fffffffc968) at callint.c:250
#57 0x000055555581552d in funcall_subr (subr=0x555555f2ca80 <Sfuncall_interactively>, numargs=5, args=0x7fffffffc968) at eval.c:3202
#58 0x0000555555814d15 in funcall_general (fun=0x555555f2ca85 <Sfuncall_interactively+5>, numargs=5, args=0x7fffffffc968) at eval.c:3058
#59 0x0000555555814fc6 in Ffuncall (nargs=6, args=0x7fffffffc960) at eval.c:3111
#60 0x000055555581446d in Fapply (nargs=3, args=0x7fffffffcbf0) at eval.c:2783
#61 0x000055555580a92c in Fcall_interactively (function=0x2aaa8d1c2b08, record_flag=0x0, keys=0x7fffe6d46865) at callint.c:342
#62 0x0000555555815339 in funcall_subr (subr=0x555555f2cac0 <Scall_interactively>, numargs=3, args=0x7fffe2200070) at eval.c:3183
#63 0x000055555586d392 in exec_byte_code (fun=0x7fffe36e5c7d, args_template=1025, nargs=1, args=0x7fffffffd350) at bytecode.c:827
#64 0x00005555558158df in funcall_lambda (fun=0x7fffe36e5c7d, nargs=1, arg_vector=0x7fffffffd348) at eval.c:3270
#65 0x0000555555814d61 in funcall_general (fun=0x7fffe36e5c7d, numargs=1, args=0x7fffffffd348) at eval.c:3062
#66 0x0000555555814fc6 in Ffuncall (nargs=2, args=0x7fffffffd340) at eval.c:3111
#67 0x000055555574d687 in command_loop_1 () at keyboard.c:1551
#68 0x00005555558113c9 in internal_condition_case (bfun=0x55555574ce24 <command_loop_1>, handlers=0x90, hfun=0x55555574c355 <cmd_error>) at eval.c:1622
#69 0x000055555574ca71 in command_loop_2 (handlers=0x90) at keyboard.c:1169
#70 0x0000555555810872 in internal_catch (tag=0x12300, func=0x55555574ca47 <command_loop_2>, arg=0x90) at eval.c:1301
#71 0x000055555574ca03 in command_loop () at keyboard.c:1147
#72 0x000055555574bef7 in recursive_edit_1 () at keyboard.c:755
#73 0x000055555574c0a3 in Frecursive_edit () at keyboard.c:838
#74 0x00005555557482fc in main (argc=2, argv=0x7fffffffd838) at emacs.c:2643
(gdb) c
Continuing.
Couldn't get registers: No such process.
(gdb) [Thread 0x7fffdbe006c0 (LWP 4695) exited]
[Thread 0x7fffe0c006c0 (LWP 4694) exited]
[Thread 0x7fffe16006c0 (LWP 4693) exited]
[Thread 0x7fffe20006c0 (LWP 4692) exited]
[Thread 0x7ffff5249a40 (LWP 4685) exited]
[Thread 0x7fffda2006c0 (LWP 4697) exited]
[New process 4685]

Program terminated with signal SIGSEGV, Segmentation fault.
The program no longer exists.

The program is not being run.
(gdb) 

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: emacs-devel@gnu.org, gerd.moellmann@gmail.com, eller.helmut@gmail.com
> Date: Thu, 20 Jun 2024 19:48:35 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Steps to reproduce:
> >> 
> >> 1. emacs -Q doc/misc/org.org
> >> 2. M-x profiler-start RET cpu RET
> >> 3. M-: (org-element-parse-buffer) RET
> >
> > Doesn't crash here, but then we emulate SIGPROF on Windows in a way
> > that could explain that.
> 
> I am on Linux:

Yes, I know.  Otherwise, why would I write about the above nit?

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>>>>> Steps to reproduce:
>>>>> 
>>>>> 1. emacs -Q doc/misc/org.org
>>>>> 2. M-x profiler-start RET cpu RET
>>>>> 3. M-: (org-element-parse-buffer) RET
>> ...
>> Thanks, that's the latest and greatest. I'm afraid that means it's time
>> to get out the debugger :-).

> Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
> 0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
> 1104		 && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
> (gdb) bt
> #0  0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
> #1  0x00005555558b567f in CLOSUREP (a=0x7fffe6dde715) at /home/yantar92/Git/emacs/src/lisp.h:3368
> #2  0x00005555558b5b80 in trace_hash (trace=0x555556329400, depth=16) at profiler.c:189
> #3  0x00005555558b5eab in record_backtrace (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:291
> #4  0x00005555558b60fd in add_sample (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:353
> #5  0x00005555558b6153 in handle_profiler_signal (signal=27) at profiler.c:396
> #6  0x0000555555777704 in deliver_process_signal (sig=27, handler=0x5555558b6104 <handle_profiler_signal>) at sysdep.c:1758
> #7  0x00005555558b6175 in deliver_profiler_signal (signal=27) at profiler.c:402
> #8  0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6
> #9  0x00007ffff5922d07 in mprotect () at /lib64/libc.so.6

4. MPS tries to un-mprotect something and receives a signal. From the
the profiler.c frames, I'd say it's a SIGPROF. And while handling the
signal, we are doing Lisp stuff. All while MPS is currently trying to
scan.

Hm, I must admit that I don't know how that's supposed to work ATM. Very
strictly speaking, a signal handler is not allowed to do very much...
Or maybe the problem is that the signal gets delivered to Emacs in the
first place?

(AFAIK, MPS doesn't use signals on macOS, but Mach exceptions, so things
probably work pretty different here, like on Windows.)

I guess we need some Linux expert here.

> #10 0x000055555595c498 in ProtSet (base=0x7fffe2d86000, limit=<optimized out>, mode=0) at protix.c:105
> #11 0x000055555594f7dd in shieldProtLower (shield=shield@entry=0x7ffff7fc2700, seg=seg@entry=0x7fffe8001988, mode=mode@entry=3) at shield.c:305
> #12 0x0000555555950417 in ShieldExpose (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8001988) at shield.c:737
> #13 0x000055555595f045 in amcSegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at poolamc.c:1594
> #14 0x0000555555948d31 in SegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at seg.c:793
> #15 0x00005555559550c0 in _mps_fix2 (mps_ss=0x7fffffffa698, mps_ref_io=0x7fffffffa160) at trace.c:1433
> #16 0x00005555558b83af in fix_lisp_obj (ss=0x7fffffffa698, pobj=0x7fffeb08b038) at igc.c:675

3. We are trying to "fix" a single Lisp_Object.

> #17 0x00005555558b86b5 in fix_array (ss=0x7fffffffa698, array=0x7fffeb08b038, n=4) at igc.c:801
> #18 0x00005555558babf6 in fix_vectorlike (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:1554
> #19 0x00005555558bc692 in fix_vector (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:2086
> #20 0x00005555558ba757 in dflt_scan_obj (ss=0x7fffffffa698, base_start=0x7fffeb08b028, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1481
> #21 0x00005555558baac4 in dflt_scanx (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1531
> #22 0x00005555558bab63 in dflt_scan (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608) at igc.c:1542
> #23 0x000055555595568f in TraceScanFormat (ss=ss@entry=0x7fffffffa690, base=base@entry=0x7fffeb08b000, limit=limit@entry=0x7fffeb08c608) at trace.c:1539
> #24 0x000055555595f609 in amcSegScan (totalReturn=0x7fffffffa68c,
> seg=0x7fffe8289870, ss=0x7fffffffa690) at poolamc.c:1427

2. MPS starts scanning the default pool and invokes our dflt_scan
function.

> #25 0x0000555555948c6f in SegScan (totalReturn=totalReturn@entry=0x7fffffffa68c, seg=seg@entry=0x7fffe8289870, ss=ss@entry=0x7fffffffa690) at seg.c:775
> #26 0x0000555555954ae1 in traceScanSegRes (ts=ts@entry=1, rank=rank@entry=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1205
> #27 0x0000555555954beb in traceScanSeg (ts=ts@entry=1, rank=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1267
> #28 0x0000555555954d63 in TraceSegAccess (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870, mode=mode@entry=1) at trace.c:1320
> #29 0x000055555594959b in SegWholeAccess (seg=0x7fffe8289870, arena=0x7ffff7fc2000, addr=0x7fffeb08c590, mode=1, context=0x7fffffffa8d0) at seg.c:1262
> #30 0x0000555555948309 in SegAccess
>     (seg=0x7fffe8289870, arena=arena@entry=0x7ffff7fc2000, addr=addr@entry=0x7fffeb08c590, mode=1, context=context@entry=0x7fffffffa8d0) at seg.c:723
> #31 0x0000555555928f51 in ArenaAccess (addr=0x7fffeb08c590, mode=<optimized out>, mode@entry=3, context=context@entry=0x7fffffffa8d0) at global.c:671
> #32 0x000055555595c6e2 in sigHandle (sig=<optimized out>, info=0x7fffffffabf0, uap=0x7fffffffaac0) at protsgix.c:97
> #33 0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6

1. So here in copy_intervals happens something that invokes MPS. Not 100%
sure but it looks to me that it is because we hit an MPS barrier
(mprotected area).

> #34 0x00005555558a1dd8 in copy_intervals (tree=0x7fffe651f500, start=587977, length=11) at intervals.c:2247
> #35 0x00005555558a1ee6 in copy_intervals_to_string (string=0x7fffeae8dfb4, buffer=0x7fffe4275e90, position=587977, length=11) at intervals.c:2272
> #36 0x0000555555800168 in make_buffer_string_both (start=587977, start_byte=588047, end=587988, end_byte=588058, props=true) at editfns.c:1615
> #37 0x00005555557ffe53 in make_buffer_string (start=587977, end=587988, props=true) at editfns.c:1554
> #38 0x0000555555800302 in Fbuffer_substring (start=0x23e326, end=0x23e352) at editfns.c:1666
> #39 0x0000555555870a1d in exec_byte_code (fun=0x7fffe446a465, args_template=771, nargs=3, args=0x7fffe2200928) at bytecode.c:1516
> --Type <RET> for more, q to quit, c to continue without paging--c
> #40 0x00005555558158df in funcall_lambda (fun=0x7fffe5b0842d, nargs=0, arg_vector=0x7fffffffb880) at eval.c:3270
> #41 0x0000555555815762 in apply_lambda (fun=0x7fffe5b0842d, args=0x0, count=...) at eval.c:3233
> #42 0x0000555555813c6d in eval_sub (form=0x7fffe6dde54b) at eval.c:2663
> #43 0x0000555555813170 in Feval (form=0x7fffe6dde54b, lexical=0x30) at eval.c:2475
> #44 0x0000555555815303 in funcall_subr (subr=0x555555f2d380 <Seval>, numargs=2, args=0x7fffe22001b0) at eval.c:3181
> #45 0x000055555586d392 in exec_byte_code (fun=0x7fffe3170b85, args_template=513, nargs=2, args=0x7fffe2200320) at bytecode.c:827
> #46 0x00005555558158df in funcall_lambda (fun=0x7fffe6dde75d, nargs=0, arg_vector=0x7fffffffc110) at eval.c:3270
> #47 0x0000555555814d61 in funcall_general (fun=0x7fffe6dde75d, numargs=0, args=0x7fffffffc110) at eval.c:3062
> #48 0x0000555555814fc6 in Ffuncall (nargs=1, args=0x7fffffffc108) at eval.c:3111
> #49 0x000055555580da20 in call0 (fn=0x7fffe6dde75d) at /home/yantar92/Git/emacs/src/lisp.h:3559
> #50 0x0000555555810cc9 in Fhandler_bind_1 (nargs=3, args=0x7fffe2200128) at eval.c:1487
> #51 0x000055555581552d in funcall_subr (subr=0x555555f2d240 <Shandler_bind_1>, numargs=3, args=0x7fffe2200128) at eval.c:3202
> #52 0x000055555586d392 in exec_byte_code (fun=0x7fffe31696c5, args_template=1025, nargs=4, args=0x7fffffffc990) at bytecode.c:827
> #53 0x00005555558158df in funcall_lambda (fun=0x7fffe31696c5, nargs=4, arg_vector=0x7fffffffc970) at eval.c:3270
> #54 0x0000555555814d61 in funcall_general (fun=0x7fffe31696c5, numargs=4, args=0x7fffffffc970) at eval.c:3062
> #55 0x0000555555814fc6 in Ffuncall (nargs=5, args=0x7fffffffc968) at eval.c:3111
> #56 0x000055555580a532 in Ffuncall_interactively (nargs=5, args=0x7fffffffc968) at callint.c:250
> #57 0x000055555581552d in funcall_subr (subr=0x555555f2ca80 <Sfuncall_interactively>, numargs=5, args=0x7fffffffc968) at eval.c:3202
> #58 0x0000555555814d15 in funcall_general (fun=0x555555f2ca85 <Sfuncall_interactively+5>, numargs=5, args=0x7fffffffc968) at eval.c:3058
> #59 0x0000555555814fc6 in Ffuncall (nargs=6, args=0x7fffffffc960) at eval.c:3111
> #60 0x000055555581446d in Fapply (nargs=3, args=0x7fffffffcbf0) at eval.c:2783
> #61 0x000055555580a92c in Fcall_interactively (function=0x2aaa8d1c2b08, record_flag=0x0, keys=0x7fffe6d46865) at callint.c:342
> #62 0x0000555555815339 in funcall_subr (subr=0x555555f2cac0 <Scall_interactively>, numargs=3, args=0x7fffe2200070) at eval.c:3183
> #63 0x000055555586d392 in exec_byte_code (fun=0x7fffe36e5c7d, args_template=1025, nargs=1, args=0x7fffffffd350) at bytecode.c:827
> #64 0x00005555558158df in funcall_lambda (fun=0x7fffe36e5c7d, nargs=1, arg_vector=0x7fffffffd348) at eval.c:3270
> #65 0x0000555555814d61 in funcall_general (fun=0x7fffe36e5c7d, numargs=1, args=0x7fffffffd348) at eval.c:3062
> #66 0x0000555555814fc6 in Ffuncall (nargs=2, args=0x7fffffffd340) at eval.c:3111
> #67 0x000055555574d687 in command_loop_1 () at keyboard.c:1551
> #68 0x00005555558113c9 in internal_condition_case (bfun=0x55555574ce24 <command_loop_1>, handlers=0x90, hfun=0x55555574c355 <cmd_error>) at eval.c:1622
> #69 0x000055555574ca71 in command_loop_2 (handlers=0x90) at keyboard.c:1169
> #70 0x0000555555810872 in internal_catch (tag=0x12300, func=0x55555574ca47 <command_loop_2>, arg=0x90) at eval.c:1301
> #71 0x000055555574ca03 in command_loop () at keyboard.c:1147
> #72 0x000055555574bef7 in recursive_edit_1 () at keyboard.c:755
> #73 0x000055555574c0a3 in Frecursive_edit () at keyboard.c:838
> #74 0x00005555557482fc in main (argc=2, argv=0x7fffffffd838) at emacs.c:2643

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: emacs-devel@gnu.org, Eli Zaretskii <eliz@gnu.org>, eller.helmut@gmail.com
> Date: Thu, 20 Jun 2024 20:02:24 +0000
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Doesn't seem to happen here. Strangely, I get something like this in
> > *Messages*, but no crash.
> >
> > ...
> > CPU profiler started
> > (org-data...
> 
> That's fine - it is `org-element-parse-buffer' return value.
> 
> > What system are you using?
> 
> Linux. See my reply to Eli.

Are you using the version of Org that exists on the igc branch, or are
you using a newer version of Org?

Re: MPS: profiler

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> I am on Linux:
>
> Yes, I know.  Otherwise, why would I write about the above nit?

Sure. But you prompted me to provide a bit more info about the
build. Maybe you already saw my settings in other discussions, but not
other people in this thread :)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> > What system are you using?
>> 
>> Linux. See my reply to Eli.
>
> Are you using the version of Org that exists on the igc branch, or are
> you using a newer version of Org?

emacs -Q... I followed my instructions, so it is built-in Org.

The crash also happens with my proper Emacs config, of course.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org, eller.helmut@gmail.com
> Date: Thu, 20 Jun 2024 20:29:42 +0000
> 
> Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
> 0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
> 1104		 && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
> (gdb) bt
> #0  0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
> #1  0x00005555558b567f in CLOSUREP (a=0x7fffe6dde715) at /home/yantar92/Git/emacs/src/lisp.h:3368
> #2  0x00005555558b5b80 in trace_hash (trace=0x555556329400, depth=16) at profiler.c:189
> #3  0x00005555558b5eab in record_backtrace (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:291
> #4  0x00005555558b60fd in add_sample (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:353
> #5  0x00005555558b6153 in handle_profiler_signal (signal=27) at profiler.c:396
> #6  0x0000555555777704 in deliver_process_signal (sig=27, handler=0x5555558b6104 <handle_profiler_signal>) at sysdep.c:1758
> #7  0x00005555558b6175 in deliver_profiler_signal (signal=27) at profiler.c:402
> #8  0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6
> #9  0x00007ffff5922d07 in mprotect () at /lib64/libc.so.6
> #10 0x000055555595c498 in ProtSet (base=0x7fffe2d86000, limit=<optimized out>, mode=0) at protix.c:105
> #11 0x000055555594f7dd in shieldProtLower (shield=shield@entry=0x7ffff7fc2700, seg=seg@entry=0x7fffe8001988, mode=mode@entry=3) at shield.c:305
> #12 0x0000555555950417 in ShieldExpose (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8001988) at shield.c:737
> #13 0x000055555595f045 in amcSegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at poolamc.c:1594
> #14 0x0000555555948d31 in SegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at seg.c:793
> #15 0x00005555559550c0 in _mps_fix2 (mps_ss=0x7fffffffa698, mps_ref_io=0x7fffffffa160) at trace.c:1433
> #16 0x00005555558b83af in fix_lisp_obj (ss=0x7fffffffa698, pobj=0x7fffeb08b038) at igc.c:675
> #17 0x00005555558b86b5 in fix_array (ss=0x7fffffffa698, array=0x7fffeb08b038, n=4) at igc.c:801
> #18 0x00005555558babf6 in fix_vectorlike (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:1554
> #19 0x00005555558bc692 in fix_vector (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:2086
> #20 0x00005555558ba757 in dflt_scan_obj (ss=0x7fffffffa698, base_start=0x7fffeb08b028, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1481
> #21 0x00005555558baac4 in dflt_scanx (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1531
> #22 0x00005555558bab63 in dflt_scan (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608) at igc.c:1542
> #23 0x000055555595568f in TraceScanFormat (ss=ss@entry=0x7fffffffa690, base=base@entry=0x7fffeb08b000, limit=limit@entry=0x7fffeb08c608) at trace.c:1539
> #24 0x000055555595f609 in amcSegScan (totalReturn=0x7fffffffa68c, seg=0x7fffe8289870, ss=0x7fffffffa690) at poolamc.c:1427
> #25 0x0000555555948c6f in SegScan (totalReturn=totalReturn@entry=0x7fffffffa68c, seg=seg@entry=0x7fffe8289870, ss=ss@entry=0x7fffffffa690) at seg.c:775
> #26 0x0000555555954ae1 in traceScanSegRes (ts=ts@entry=1, rank=rank@entry=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1205
> #27 0x0000555555954beb in traceScanSeg (ts=ts@entry=1, rank=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1267
> #28 0x0000555555954d63 in TraceSegAccess (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870, mode=mode@entry=1) at trace.c:1320
> #29 0x000055555594959b in SegWholeAccess (seg=0x7fffe8289870, arena=0x7ffff7fc2000, addr=0x7fffeb08c590, mode=1, context=0x7fffffffa8d0) at seg.c:1262
> #30 0x0000555555948309 in SegAccess
>     (seg=0x7fffe8289870, arena=arena@entry=0x7ffff7fc2000, addr=addr@entry=0x7fffeb08c590, mode=1, context=context@entry=0x7fffffffa8d0) at seg.c:723
> #31 0x0000555555928f51 in ArenaAccess (addr=0x7fffeb08c590, mode=<optimized out>, mode@entry=3, context=context@entry=0x7fffffffa8d0) at global.c:671
> #32 0x000055555595c6e2 in sigHandle (sig=<optimized out>, info=0x7fffffffabf0, uap=0x7fffffffaac0) at protsgix.c:97
> #33 0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6
> #34 0x00005555558a1dd8 in copy_intervals (tree=0x7fffe651f500, start=587977, length=11) at intervals.c:2247

This tells that we got SIGPROF while inside MPS code, in its sigHandle
handler for SIGSEGV caused by protection violation.  Does something
special happens on GNU/Linux when a nested signal is delivered,
i.e. when a signal is delivered when the program is in a signal
handler?  Some special small stack, perhaps?

Also, can 'static struct profiler_log cpu', which is being worked on
by record_backtrace, be affected by MPS in any way?

Ihor, can you show the data involved in this segfault, i.e. trace[i]
which is the argument of CLOSUREP that segfaults?  And in general all
the data inside trace_hash.  You'll need to "source .gdbinit" to be
able to show Lisp data in human-readable format.

Re: MPS: profiler

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> Also, can 'static struct profiler_log cpu', which is being worked on
> by record_backtrace, be affected by MPS in any way?

I'd guess that just "touching" Lisp objects in the SIGPROF handler can
be problematic because these objects themselves can be behind a barrier,
either the same that MPS is currently working on when it got interrupted
or another one.

Re: MPS: profiler

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Ihor Radchenko <yantar92@posteo.net>,  emacs-devel@gnu.org,
>   eller.helmut@gmail.com
> Date: Fri, 21 Jun 2024 08:54:34 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Also, can 'static struct profiler_log cpu', which is being worked on
> > by record_backtrace, be affected by MPS in any way?
> 
> I'd guess that just "touching" Lisp objects in the SIGPROF handler can
> be problematic because these objects themselves can be behind a barrier,
> either the same that MPS is currently working on when it got interrupted
> or another one.

Which Lisp objects do we touch in record_backtrace?  All I see is that
we access spec_pdl and its data.  Is that supposed to be dangerous
while MPS does its thing?

Also note that add_sample (called by the SIGPROF handler) does this:

  if (EQ (backtrace_top_function (), QAutomatic_GC)) /* bug#60237 */
    /* Special case the time-count inside GC because the hash-table
       code is not prepared to be used while the GC is running.
       More specifically it uses ASIZE at many places where it does
       not expect the ARRAY_MARK_FLAG to be set.  We could try and
       harden the hash-table code, but it doesn't seem worth the
       effort.  */
    plog->gc_count = saturated_add (plog->gc_count, count);
  else
    record_backtrace (plog, count);

IOW, it avoids recording the backtrace if called during GC.  Perhaps
we should do the same with igc?  Or doe we already set up
backtrace_top_function to return QAutomatic_GC in that case?

And finally, I see in make_log that we already do something about the
log variables that is specific to MPS.  Is that not enough to prevent
this kind of issues?

Re: MPS: profiler

[Helmut Eller]

On Fri, Jun 21 2024, Gerd Möllmann wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Also, can 'static struct profiler_log cpu', which is being worked on
>> by record_backtrace, be affected by MPS in any way?
>
> I'd guess that just "touching" Lisp objects in the SIGPROF handler can
> be problematic because these objects themselves can be behind a barrier,
> either the same that MPS is currently working on when it got interrupted
> or another one.

Perhaps dflt_scan should block SIGPROF while it is running.  Or perhaps
dflt_scan should set some global flag while it running so that the
profiler knows that it's too dangerous to touch anything.  Any better
ideas?

I can use this command to reproduce the problem:

run -Q -batch -eval '(progn
  (defvar *baz* nil)
  (defun bar (len)
    (let ((data (make-list len nil)))
      (setq *baz* (lambda () (bar len) data))))
  (defun foo ()
    (bar 1000)
    (dotimes (_ 10000)
      (funcall *baz*)))
  (profiler-start (quote cpu))
  (foo))'

Re: MPS: profiler

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Ihor Radchenko <yantar92@posteo.net>,  emacs-devel@gnu.org,
>>   eller.helmut@gmail.com
>> Date: Fri, 21 Jun 2024 08:54:34 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Also, can 'static struct profiler_log cpu', which is being worked on
>> > by record_backtrace, be affected by MPS in any way?
>> 
>> I'd guess that just "touching" Lisp objects in the SIGPROF handler can
>> be problematic because these objects themselves can be behind a barrier,
>> either the same that MPS is currently working on when it got interrupted
>> or another one.
>
> Which Lisp objects do we touch in record_backtrace?  All I see is that
> we access spec_pdl and its data.  Is that supposed to be dangerous
> while MPS does its thing?

I don't know. In the backtrace I commented on in a reply to Ihor, we had
this:

  #0  0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
  #1  0x00005555558b567f in CLOSUREP (a=0x7fffe6dde715) at /home/yantar92/Git/emacs/src/lisp.h:3368
  #2  0x00005555558b5b80 in trace_hash (trace=0x555556329400, depth=16) at profiler.c:189


>
> Also note that add_sample (called by the SIGPROF handler) does this:
>
>   if (EQ (backtrace_top_function (), QAutomatic_GC)) /* bug#60237 */
>     /* Special case the time-count inside GC because the hash-table
>        code is not prepared to be used while the GC is running.
>        More specifically it uses ASIZE at many places where it does
>        not expect the ARRAY_MARK_FLAG to be set.  We could try and
>        harden the hash-table code, but it doesn't seem worth the
>        effort.  */
>     plog->gc_count = saturated_add (plog->gc_count, count);
>   else
>     record_backtrace (plog, count);
>
> IOW, it avoids recording the backtrace if called during GC.  Perhaps
> we should do the same with igc?  Or doe we already set up
> backtrace_top_function to return QAutomatic_GC in that case?

I don't think we do. I'm not even sure we could do that reliably since
MPS is running concurrently.

> And finally, I see in make_log that we already do something about the
> log variables that is specific to MPS.  Is that not enough to prevent
> this kind of issues?

No, that's just making sure we trace what's in the malloc'd memory.

Re: MPS: profiler

[Ihor Radchenko]

Helmut Eller <eller.helmut@gmail.com> writes:

>> I'd guess that just "touching" Lisp objects in the SIGPROF handler can
>> be problematic because these objects themselves can be behind a barrier,
>> either the same that MPS is currently working on when it got interrupted
>> or another one.
>
> Perhaps dflt_scan should block SIGPROF while it is running.  Or perhaps
> dflt_scan should set some global flag while it running so that the
> profiler knows that it's too dangerous to touch anything.  Any better
> ideas?

May you infer from the backtrace whether MPS "freezes" everything at the
time when segfault triggers? If it does, such freezes should probably be
recorded by the profiler and accumulated in "GC" entry in the profiler
report, as we do it on master. Otherwise, profiler will be missing some
data.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Fri, Jun 21 2024, Gerd Möllmann wrote:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>> Also, can 'static struct profiler_log cpu', which is being worked on
>>> by record_backtrace, be affected by MPS in any way?
>>
>> I'd guess that just "touching" Lisp objects in the SIGPROF handler can
>> be problematic because these objects themselves can be behind a barrier,
>> either the same that MPS is currently working on when it got interrupted
>> or another one.
>
> Perhaps dflt_scan should block SIGPROF while it is running.  Or perhaps
> dflt_scan should set some global flag while it running so that the
> profiler knows that it's too dangerous to touch anything.  

I kind of doubt that's sufficient because there are time windows in MPS
itself where dflt_scan is not running, and it maybe doing dangerous
things. There is no guarantee I'd say.

> Any better ideas?
>
> I can use this command to reproduce the problem:
>
> run -Q -batch -eval '(progn
>   (defvar *baz* nil)
>   (defun bar (len)
>     (let ((data (make-list len nil)))
>       (setq *baz* (lambda () (bar len) data))))
>   (defun foo ()
>     (bar 1000)
>     (dotimes (_ 10000)
>       (funcall *baz*)))
>   (profiler-start (quote cpu))
>   (foo))'

Could we park or clamp the arena while profiling?

Re: MPS: profiler

[Helmut Eller]

On Fri, Jun 21 2024, Ihor Radchenko wrote:

> May you infer from the backtrace whether MPS "freezes" everything at the
> time when segfault triggers? If it does, such freezes should probably be
> recorded by the profiler and accumulated in "GC" entry in the profiler
> report, as we do it on master. Otherwise, profiler will be missing some
> data.

There is a function mps_arena_busy, that might be useful.  The doc says
that it should only be used for debugging.  But the implementation
actually checks if the arena holds a lock.

Re: MPS: profiler

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Could we park or clamp the arena while profiling?

Profiler might be running for a long time. If I understand correctly,
parking anything for a long time risks that "anything" to be not
garbage-collected at all.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Fri, Jun 21 2024, Ihor Radchenko wrote:
>
>> May you infer from the backtrace whether MPS "freezes" everything at the
>> time when segfault triggers? If it does, such freezes should probably be
>> recorded by the profiler and accumulated in "GC" entry in the profiler
>> report, as we do it on master. Otherwise, profiler will be missing some
>> data.
>
> There is a function mps_arena_busy, that might be useful.  The doc says
> that it should only be used for debugging.  But the implementation
> actually checks if the arena holds a lock.

Yes, the SIBPROF handler could at least return early then.

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Could we park or clamp the arena while profiling?
>
> Profiler might be running for a long time. If I understand correctly,
> parking anything for a long time risks that "anything" to be not
> garbage-collected at all.

Yes, a small downside :-)

Re: MPS: profiler

[Helmut Eller]

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

On Fri, Jun 21 2024, Gerd Möllmann wrote:

>> There is a function mps_arena_busy, that might be useful.  The doc says
>> that it should only be used for debugging.  But the implementation
>> actually checks if the arena holds a lock.
>
> Yes, the SIBPROF handler could at least return early then.

Perhaps something like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Be-careful-while-profiling.patch --]
[-- Type: text/x-diff, Size: 2166 bytes --]

From d86baa8b2af2059b00c9cbec5f404008c73705a8 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Fri, 21 Jun 2024 10:12:28 +0200
Subject: [PATCH] Be careful while profiling

SIGPROF can be delived while the SIGSEGV handler is running.  In this
situation we shouldn't access MPS-managed memory.  E.g. the profiler
should not look inside closures when recording backtraces.

* src/igc.h (igc_busy_p): New.
* src/profiler.c (add_sample): When igc_busy_p, do the same as the old
GC does when it is called during GC, i.e. only increace a counter and
don't record a backtrace.
* src/igc.c (igc_busy_p): Implement it.
---
 src/igc.c      | 6 ++++++
 src/igc.h      | 1 +
 src/profiler.c | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/src/igc.c b/src/igc.c
index ce4058fb25c..7ccde80e329 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -4021,6 +4021,12 @@ igc_alloc_dump (size_t nbytes)
   return base_to_client (block);
 }
 
+bool
+igc_busy_p (void)
+{
+  return mps_arena_busy (global_igc->arena);
+}
+
 /***********************************************************************
 				  Init
  ***********************************************************************/
diff --git a/src/igc.h b/src/igc.h
index 485f23090c2..fc80a92d1a8 100644
--- a/src/igc.h
+++ b/src/igc.h
@@ -154,6 +154,7 @@ #define EMACS_IGC_H
 char *igc_dump_finish_obj (void *client, enum igc_obj_type type,
 			   char *base, char *end);
 void *igc_alloc_dump (size_t nbytes);
+bool igc_busy_p (void);
 
 # define eassert_not_mps() eassert (false)
 #else
diff --git a/src/profiler.c b/src/profiler.c
index 98d83bcf264..c24a92de6d7 100644
--- a/src/profiler.c
+++ b/src/profiler.c
@@ -341,7 +341,11 @@ record_backtrace (struct profiler_log *plog, EMACS_INT count)
 static void
 add_sample (struct profiler_log *plog, EMACS_INT count)
 {
+#ifdef HAVE_MPS
+  if (igc_busy_p ())
+#else
   if (EQ (backtrace_top_function (), QAutomatic_GC)) /* bug#60237 */
+#endif
     /* Special case the time-count inside GC because the hash-table
        code is not prepared to be used while the GC is running.
        More specifically it uses ASIZE at many places where it does
-- 
2.39.2

Re: MPS: profiler

[Pip Cet]

On Thursday, June 20th, 2024 at 19:24, Ihor Radchenko <yantar92@posteo.net> wrote:
> I am playing around with scratch/igc branch for fun, and there is one
> crash that I can reproduce quite consistently.
> 
> All it takes is (1) compile Emacs without native-compilation support;
> (2) open Emacs; (3) M-x profiler-start; (4) run a complex operation.
> 
> Steps to reproduce:
> 
> 1. emacs -Q doc/misc/org.org
> 2. M-x profiler-start RET cpu RET
> 3. M-: (org-element-parse-buffer) RET
> 
> Without step 2, no crash.

Just sharing some information from a private discussion with Ihor:

I can reproduce this locally. I can also "fix" it locally, but the problem seems more complicated since Ihor reports the fix isn't working for him.

1. MPS uses SIGSEGV internally, usually transparently to the client program.

2. The profiler uses SIGPROF, then runs complicated code in the handler. My understanding is it's carefully tuned not to trigger traditional GC, but it can and will cause (handled) SIGSEGV.

3. MPS isn't reentrant enough to handle a SIGSEGV while it's handling a SIGSEGV, and will die with one of a number of errors.

The right thing to do, IMHO, is to let MPS know that it should block SIGPROF (and any other signals that might use managed memory) while handling SIGSEGV. Unfortunately, the MPS code in current git doesn't have facilities to do that, so I've applied this patch to MPS, which works here but doesn't for Ihor:

diff --git a/code/protsgix.c b/code/protsgix.c
index 966569c92..7c60d4fa2 100644
--- a/code/protsgix.c
+++ b/code/protsgix.c
@@ -143,7 +143,7 @@ void ProtSetup(void)
   int result;
 
   sa.sa_sigaction = sigHandle;
-  result = sigemptyset(&sa.sa_mask);
+  result = sigfillset(&sa.sa_mask);
   AVER(result == 0);
   sa.sa_flags = SA_SIGINFO | SA_RESTART;
 
Hope any of this helps,
Pip

Re: MPS: profiler

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Fri, Jun 21 2024, Gerd Möllmann wrote:
>
>>> There is a function mps_arena_busy, that might be useful.  The doc says
>>> that it should only be used for debugging.  But the implementation
>>> actually checks if the arena holds a lock.
>>
>> Yes, the SIBPROF handler could at least return early then.
>
> Perhaps something like this?

Not sure. The result of is_busy is only valid at the point in time when
it is called.

Re: MPS: profiler

[Ihor Radchenko]

Pip Cet <pipcet@protonmail.com> writes:

> ... which works here but doesn't for Ihor:

Jut to make things clear, "does not work" for me is for my main
config. But I have other kinds of crashes there as well, so it may or
may not be directly caused by the proposed change in MPS.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Pip Cet <pipcet@protonmail.com> writes:

> Just sharing some information from a private discussion with Ihor:
>
> I can reproduce this locally. I can also "fix" it locally, but the
> problem seems more complicated since Ihor reports the fix isn't
> working for him.
>
> 1. MPS uses SIGSEGV internally, usually transparently to the client
> program.
>
> 2. The profiler uses SIGPROF, then runs complicated code in the
> handler. My understanding is it's carefully tuned not to trigger
> traditional GC, but it can and will cause (handled) SIGSEGV.
>
> 3. MPS isn't reentrant enough to handle a SIGSEGV while it's handling
> a SIGSEGV, and will die with one of a number of errors.

That's basically it, I agree. A signal handler is not supposed to do
things like that, from the POV of MPS (and others :-)).

>
> The right thing to do, IMHO, is to let MPS know that it should block
> SIGPROF (and any other signals that might use managed memory) while
> handling SIGSEGV. Unfortunately, the MPS code in current git doesn't
> have facilities to do that, so I've applied this patch to MPS, which
> works here but doesn't for Ihor:
>
> diff --git a/code/protsgix.c b/code/protsgix.c
> index 966569c92..7c60d4fa2 100644
> --- a/code/protsgix.c
> +++ b/code/protsgix.c
> @@ -143,7 +143,7 @@ void ProtSetup(void)
>    int result;
>  
>    sa.sa_sigaction = sigHandle;
> -  result = sigemptyset(&sa.sa_mask);
> +  result = sigfillset(&sa.sa_mask);
>    AVER(result == 0);
>    sa.sa_flags = SA_SIGINFO | SA_RESTART;
>

Hm, sigfillset is a bit much maybe. What about only adding
PROF, like in

  https://www.gnu.org/software/libc/manual/html_node/Blocking-for-Handler.html

But I'll better leave that to the experts.

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Helmut Eller <eller.helmut@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>  emacs-devel@gnu.org
> Date: Fri, 21 Jun 2024 07:50:05 +0000
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Could we park or clamp the arena while profiling?
> 
> Profiler might be running for a long time. If I understand correctly,
> parking anything for a long time risks that "anything" to be not
> garbage-collected at all.

Profiler's SIGPROF handler is NOT supposed to run for a long time.  If
you disagree, please show the code called from the handler that can
run for a long time.

Or maybe I misunderstood you suggestion (or that of Gerd): to me,
"while profileing" means just while the profiler's SIGPROF handler
runs, nut the whole time profiler is active.

Re: MPS: profiler

[Eli Zaretskii]

> Date: Fri, 21 Jun 2024 08:23:16 +0000
> From: Pip Cet <pipcet@protonmail.com>
> Cc: emacs-devel@gnu.org, Gerd Möllmann <gerd.moellmann@gmail.com>, Eli Zaretskii <eliz@gnu.org>, eller.helmut@gmail.com
> 
> On Thursday, June 20th, 2024 at 19:24, Ihor Radchenko <yantar92@posteo.net> wrote:
> > I am playing around with scratch/igc branch for fun, and there is one
> > crash that I can reproduce quite consistently.
> > 
> > All it takes is (1) compile Emacs without native-compilation support;
> > (2) open Emacs; (3) M-x profiler-start; (4) run a complex operation.
> > 
> > Steps to reproduce:
> > 
> > 1. emacs -Q doc/misc/org.org
> > 2. M-x profiler-start RET cpu RET
> > 3. M-: (org-element-parse-buffer) RET
> > 
> > Without step 2, no crash.
> 
> Just sharing some information from a private discussion with Ihor:
> 
> I can reproduce this locally. I can also "fix" it locally, but the problem seems more complicated since Ihor reports the fix isn't working for him.
> 
> 1. MPS uses SIGSEGV internally, usually transparently to the client program.
> 
> 2. The profiler uses SIGPROF, then runs complicated code in the handler. My understanding is it's carefully tuned not to trigger traditional GC, but it can and will cause (handled) SIGSEGV.
> 
> 3. MPS isn't reentrant enough to handle a SIGSEGV while it's handling a SIGSEGV, and will die with one of a number of errors.

This is not what happens in this case, so let's stick to what we
actually see, okay?  It's complex enough.

> The right thing to do, IMHO, is to let MPS know that it should block SIGPROF (and any other signals that might use managed memory) while handling SIGSEGV.

I disagree.  MPS must be able to support programs compiled with
"-gprof", so I don't believe SIGPROF should be blocked.  The handler
is our code, so we should modify the handler not to do anything unsafe
if SIGPROF happens while MPS is processing its SIGSEGV.  That's a much
more reliable fix, and one that will allow us to keep our profiling
code.

Re: MPS: profiler

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Ihor Radchenko <yantar92@posteo.net>,  Eli Zaretskii <eliz@gnu.org>,
>   emacs-devel@gnu.org
> Date: Fri, 21 Jun 2024 10:31:55 +0200
> 
> Helmut Eller <eller.helmut@gmail.com> writes:
> 
> > On Fri, Jun 21 2024, Gerd Möllmann wrote:
> >
> >>> There is a function mps_arena_busy, that might be useful.  The doc says
> >>> that it should only be used for debugging.  But the implementation
> >>> actually checks if the arena holds a lock.
> >>
> >> Yes, the SIBPROF handler could at least return early then.
> >
> > Perhaps something like this?
> 
> Not sure. The result of is_busy is only valid at the point in time when
> it is called.

Are you thinking about what happens when GC is run concurrently?
Because this is not what happens here, AFAIU.  Let's focus on fixing
the actual issue we see in the backtrace, and consider its possible
generalizations later.

Are you saying that is_busy could return false in the situation we see
in the backtrace, i.e. during the entire time MPS processes its
protection-induced SIGSEGV?

Re: MPS: profiler

[Pip Cet]

On Thursday, June 20th, 2024 at 20:29, Ihor Radchenko <yantar92@posteo.net> wrote:
> Gerd Möllmann gerd.moellmann@gmail.com writes:
> GNU gdb (Gentoo 14.2 vanilla) 14.2
> Copyright (C) 2023 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
> 
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "x86_64-pc-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> https://bugs.gentoo.org/.
> 
> Find the GDB manual and other documentation resources online at:
> http://www.gnu.org/software/gdb/documentation/.

This is the relevant backtrace:

> Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
> 0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
> 1104 && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
> 
> (gdb) bt
> #0 0x00005555558b53d6 in PSEUDOVECTORP (a=0x7fffe6dde715, code=32) at /home/yantar92/Git/emacs/src/lisp.h:1104
> #1 0x00005555558b567f in CLOSUREP (a=0x7fffe6dde715) at /home/yantar92/Git/emacs/src/lisp.h:3368
> #2 0x00005555558b5b80 in trace_hash (trace=0x555556329400, depth=16) at profiler.c:189
> #3 0x00005555558b5eab in record_backtrace (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:291
> 
> #4 0x00005555558b60fd in add_sample (plog=0x555555fd0960 <cpu>, count=2) at profiler.c:353
> 
> #5 0x00005555558b6153 in handle_profiler_signal (signal=27) at profiler.c:396
> #6 0x0000555555777704 in deliver_process_signal (sig=27, handler=0x5555558b6104 <handle_profiler_signal>) at sysdep.c:1758
> 
> #7 0x00005555558b6175 in deliver_profiler_signal (signal=27) at profiler.c:402
> #8 0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6

This is SIGPROF.

> #9 0x00007ffff5922d07 in mprotect () at /lib64/libc.so.6
> #10 0x000055555595c498 in ProtSet (base=0x7fffe2d86000, limit=<optimized out>, mode=0) at protix.c:105
> 
> #11 0x000055555594f7dd in shieldProtLower (shield=shield@entry=0x7ffff7fc2700, seg=seg@entry=0x7fffe8001988, mode=mode@entry=3) at shield.c:305
> #12 0x0000555555950417 in ShieldExpose (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8001988) at shield.c:737
> #13 0x000055555595f045 in amcSegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at poolamc.c:1594
> #14 0x0000555555948d31 in SegFix (seg=0x7fffe8001988, ss=0x7fffffffa690, refIO=0x7fffffffa0f8) at seg.c:793
> #15 0x00005555559550c0 in _mps_fix2 (mps_ss=0x7fffffffa698, mps_ref_io=0x7fffffffa160) at trace.c:1433
> #16 0x00005555558b83af in fix_lisp_obj (ss=0x7fffffffa698, pobj=0x7fffeb08b038) at igc.c:675
> #17 0x00005555558b86b5 in fix_array (ss=0x7fffffffa698, array=0x7fffeb08b038, n=4) at igc.c:801
> #18 0x00005555558babf6 in fix_vectorlike (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:1554
> #19 0x00005555558bc692 in fix_vector (ss=0x7fffffffa698, v=0x7fffeb08b030) at igc.c:2086
> #20 0x00005555558ba757 in dflt_scan_obj (ss=0x7fffffffa698, base_start=0x7fffeb08b028, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1481
> #21 0x00005555558baac4 in dflt_scanx (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608, closure=0x0) at igc.c:1531
> #22 0x00005555558bab63 in dflt_scan (ss=0x7fffffffa698, base_start=0x7fffeb08b000, base_limit=0x7fffeb08c608) at igc.c:1542
> #23 0x000055555595568f in TraceScanFormat (ss=ss@entry=0x7fffffffa690, base=base@entry=0x7fffeb08b000, limit=limit@entry=0x7fffeb08c608) at trace.c:1539
> #24 0x000055555595f609 in amcSegScan (totalReturn=0x7fffffffa68c, seg=0x7fffe8289870, ss=0x7fffffffa690) at poolamc.c:1427
> #25 0x0000555555948c6f in SegScan (totalReturn=totalReturn@entry=0x7fffffffa68c, seg=seg@entry=0x7fffe8289870, ss=ss@entry=0x7fffffffa690) at seg.c:775
> #26 0x0000555555954ae1 in traceScanSegRes (ts=ts@entry=1, rank=rank@entry=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1205
> #27 0x0000555555954beb in traceScanSeg (ts=ts@entry=1, rank=1, arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870) at trace.c:1267
> #28 0x0000555555954d63 in TraceSegAccess (arena=arena@entry=0x7ffff7fc2000, seg=seg@entry=0x7fffe8289870, mode=mode@entry=1) at trace.c:1320
> #29 0x000055555594959b in SegWholeAccess (seg=0x7fffe8289870, arena=0x7ffff7fc2000, addr=0x7fffeb08c590, mode=1, context=0x7fffffffa8d0) at seg.c:1262
> #30 0x0000555555948309 in SegAccess
> (seg=0x7fffe8289870, arena=arena@entry=0x7ffff7fc2000, addr=addr@entry=0x7fffeb08c590, mode=1, context=context@entry=0x7fffffffa8d0) at seg.c:723
> #31 0x0000555555928f51 in ArenaAccess (addr=0x7fffeb08c590, mode=<optimized out>, mode@entry=3, context=context@entry=0x7fffffffa8d0) at global.c:671
> 
> #32 0x000055555595c6e2 in sigHandle (sig=<optimized out>, info=0x7fffffffabf0, uap=0x7fffffffaac0) at protsgix.c:97
> 
> #33 0x00007ffff5855470 in <signal handler called> () at /lib64/libc.so.6
>

This is SIGSEGV.

So, yes, what I described is precisely what we're seeing in the backtrace.

Pip

Re: MPS: profiler

[Eli Zaretskii]

> Date: Fri, 21 Jun 2024 10:49:41 +0000
> From: Pip Cet <pipcet@protonmail.com>
> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>, Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org, eller.helmut@gmail.com
> 
> So, yes, what I described is precisely what we're seeing in the backtrace.

No, you were describing a nested protection-related SIGSEGV.  But we
haven't yet established that the second SIGSEGV is due to protection.
I asked Ihor to show the data in the function that segfaults, but he
didn't do that yet.  Only after we see the data, we can decide that
this is a nested protection-related SIGSEGV, not a segfault caused by
our code accessing data that is no longer there.

In general, I'd expect MPS to be ready for the case where this kind of
combination of signals happens.  At the very least, this is needed for
supporting -gprof, but also for other legitimate techniques.  It is,
of course, possible that MPS has a bug in that area, or even some
design flaw or limitation (in the latter case, it's supposed to be
documented somewhere, btw), but I'd first assume it's our bug, not
theirs, and explore that possibility first.

Re: MPS: profiler

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Profiler might be running for a long time. If I understand correctly,
>> parking anything for a long time risks that "anything" to be not
>> garbage-collected at all.
>
> ...
> Or maybe I misunderstood you suggestion (or that of Gerd): to me,
> "while profileing" means just while the profiler's SIGPROF handler
> runs, nut the whole time profiler is active.

My understanding that the suggested parking was for the time profiler is
active in Emacs (that is: between M-x profiler-start and M-x
profiler-stop).

Unless I miss something, SIGPROF handler cannot touch the arena to park
it or it will lead to problems exactly like the one I encountered.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: gerd.moellmann@gmail.com, eller.helmut@gmail.com, emacs-devel@gnu.org
> Date: Fri, 21 Jun 2024 10:57:12 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Profiler might be running for a long time. If I understand correctly,
> >> parking anything for a long time risks that "anything" to be not
> >> garbage-collected at all.
> >
> > ...
> > Or maybe I misunderstood you suggestion (or that of Gerd): to me,
> > "while profileing" means just while the profiler's SIGPROF handler
> > runs, nut the whole time profiler is active.
> 
> My understanding that the suggested parking was for the time profiler is
> active in Emacs (that is: between M-x profiler-start and M-x
> profiler-stop).

That's unreasonable and impractical.  Gerd, was this what you
suggested?

> Unless I miss something, SIGPROF handler cannot touch the arena to park
> it or it will lead to problems exactly like the one I encountered.

That depends on what you mean by "park the arena".

Re: MPS: profiler

[Pip Cet]

On Friday, June 21st, 2024 at 10:43, Eli Zaretskii <eliz@gnu.org> wrote:
> This is not what happens in this case, so let's stick to what we
> actually see, okay? It's complex enough.

Yes, it is.

> > The right thing to do, IMHO, is to let MPS know that it should block SIGPROF (and any other signals that might use managed memory) while handling SIGSEGV.
> 
> I disagree. MPS must be able to support programs compiled with
> "-gprof", so I don't believe SIGPROF should be blocked.

Only while handling SIGSEGV, of course.

> The handler
> is our code, so we should modify the handler not to do anything unsafe
> if SIGPROF happens while MPS is processing its SIGSEGV. That's a much
> more reliable fix, and one that will allow us to keep our profiling
> code.

Why are you suggesting that my "fix" (which, yes, is overly broad, we'd need to go through the signals to find all we can safely leave unblocked) requires any changes to the profiling code? As far as I can tell, it just works.

The main difference between the two approaches, both of which require meddling with MPS internals, is that if SIGPROF is blocked, it will be delivered with a delay after the SIGSEGV handler is done running. If the SIGPROF handler runs and doesn't do "anything unsafe", on the other hand, it can record the unusable signal but won't get a new one. I don't see a clear advantage of either, to be honest. Blocking signals, of course, is more reliable than "just knowing" we won't do anything unsafe.

Pip

Re: MPS: profiler

[Eli Zaretskii]

> Date: Fri, 21 Jun 2024 11:00:02 +0000
> From: Pip Cet <pipcet@protonmail.com>
> Cc: yantar92@posteo.net, emacs-devel@gnu.org, gerd.moellmann@gmail.com, eller.helmut@gmail.com
> 
> > > The right thing to do, IMHO, is to let MPS know that it should block SIGPROF (and any other signals that might use managed memory) while handling SIGSEGV.
> > 
> > I disagree. MPS must be able to support programs compiled with
> > "-gprof", so I don't believe SIGPROF should be blocked.
> 
> Only while handling SIGSEGV, of course.
> 
> > The handler
> > is our code, so we should modify the handler not to do anything unsafe
> > if SIGPROF happens while MPS is processing its SIGSEGV. That's a much
> > more reliable fix, and one that will allow us to keep our profiling
> > code.
> 
> Why are you suggesting that my "fix" (which, yes, is overly broad, we'd need to go through the signals to find all we can safely leave unblocked) requires any changes to the profiling code? As far as I can tell, it just works.
> 
> The main difference between the two approaches, both of which require meddling with MPS internals, is that if SIGPROF is blocked, it will be delivered with a delay after the SIGSEGV handler is done running. If the SIGPROF handler runs and doesn't do "anything unsafe", on the other hand, it can record the unusable signal but won't get a new one. I don't see a clear advantage of either, to be honest. Blocking signals, of course, is more reliable than "just knowing" we won't do anything unsafe.

I simply don't believe MPS is so poorly designed as to disallow
signals while its own SIGSEGV handler runs.  SIGPROF is just one
example; there are also SIGALRM, SIGUSR1, SIGUSR2, and maybe others.
It makes little sense to me to expect MPS to block these signals.  I
rather expect them to DTRT with those signals.

My first suspect is our own code.  Until we establish positively that
our code is fine, and what happens here is that we hit the MPS
protection in our SIGPROF handler, I don't think we should consider
changes in the MPS code.

Re: MPS: profiler

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Unless I miss something, SIGPROF handler cannot touch the arena to park
>> it or it will lead to problems exactly like the one I encountered.
>
> That depends on what you mean by "park the arena".

I meant mps_arena_park() as described in
https://memory-pool-system.readthedocs.io/en/latest/guide/lang.html#tidying-up

mps_arena_park(arena);        /* ensure no collection is running */

This is the only meaning of park I can see in the docs.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: gerd.moellmann@gmail.com, eller.helmut@gmail.com, emacs-devel@gnu.org
> Date: Fri, 21 Jun 2024 11:20:53 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Unless I miss something, SIGPROF handler cannot touch the arena to park
> >> it or it will lead to problems exactly like the one I encountered.
> >
> > That depends on what you mean by "park the arena".
> 
> I meant mps_arena_park() as described in
> https://memory-pool-system.readthedocs.io/en/latest/guide/lang.html#tidying-up
> 
> mps_arena_park(arena);        /* ensure no collection is running */
> 
> This is the only meaning of park I can see in the docs.

We cannot disable GC when we profile a program.  It's out of the
question.

Re: MPS: profiler

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ihor Radchenko <yantar92@posteo.net>
>> Cc: gerd.moellmann@gmail.com, eller.helmut@gmail.com, emacs-devel@gnu.org
>> Date: Fri, 21 Jun 2024 10:57:12 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> Profiler might be running for a long time. If I understand correctly,
>> >> parking anything for a long time risks that "anything" to be not
>> >> garbage-collected at all.
>> >
>> > ...
>> > Or maybe I misunderstood you suggestion (or that of Gerd): to me,
>> > "while profileing" means just while the profiler's SIGPROF handler
>> > runs, nut the whole time profiler is active.
>> 
>> My understanding that the suggested parking was for the time profiler is
>> active in Emacs (that is: between M-x profiler-start and M-x
>> profiler-stop).
>
> That's unreasonable and impractical.  Gerd, was this what you
> suggested?

Yes, that's what I meant :-). That woulld be safe, at least.

>> Unless I miss something, SIGPROF handler cannot touch the arena to park
>> it or it will lead to problems exactly like the one I encountered.
>
> That depends on what you mean by "park the arena".

mps_arena_park, mps_arena_clamp.

Re: MPS: profiler

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> My first suspect is our own code.  Until we establish positively that
> our code is fine, and what happens here is that we hit the MPS
> protection in our SIGPROF handler, I don't think we should consider
> changes in the MPS code.

I also think it's Emacs' code. I think MPS cannot reasonably forsee that
a signal handler uses MPS-managed memory, so that this kind of recursion
occurs - MPS handling a signal from a barrier, being interrupted by a
signal which it delivers, and that leading to hitting the same or
another barrier.

Re: MPS: profiler

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> >> Yes, the SIBPROF handler could at least return early then.
>> >
>> > Perhaps something like this?
>> 
>> Not sure. The result of is_busy is only valid at the point in time when
>> it is called.
>
> Are you thinking about what happens when GC is run concurrently?
> Because this is not what happens here, AFAIU.  Let's focus on fixing
> the actual issue we see in the backtrace, and consider its possible
> generalizations later.
>
> Are you saying that is_busy could return false in the situation we see
> in the backtrace, i.e. during the entire time MPS processes its
> protection-induced SIGSEGV?

I'm still not sure. What I'm trying to say is that we need to be sure
that there are no windows left in which things can change. I'd be most
comfortable ATM if the first thing the SIGPROF handler does is check
is_busy and immediately returns, doing nothing.

Re: MPS: profiler

[Helmut Eller]

On Fri, Jun 21 2024, Gerd Möllmann wrote:

> I'm still not sure. What I'm trying to say is that we need to be sure
> that there are no windows left in which things can change. I'd be most
> comfortable ATM if the first thing the SIGPROF handler does is check
> is_busy and immediately returns, doing nothing.

Let's assume for moment that SIGPROF and SIGSEGV are handled in the same
thread.  Then either SIGPROF comes before SIGSEGV or after.

Case 1 (SEGPROF before SIGSEGV): here mps_arena_busy will return false
and will access the MPS-managed memory.  This is fine, because to MPS
this is no different than any other client code.

Case 2 (SEGSEGV before SIGPROF): here mps_arena_busy will return true
and we only increase a counter.  This is also fine.

Convinced this is safe and that we that have covered all cases?

Now for the harder situation where SIGPROF and SIGSEGV aren't handled in
the same thread.  Well, I would say that this doesn't happen in Emacs.
But maybe somebody can come up with example where it does.

Re: MPS: profiler

[Ihor Radchenko]

Helmut Eller <eller.helmut@gmail.com> writes:

> Now for the harder situation where SIGPROF and SIGSEGV aren't handled in
> the same thread.  Well, I would say that this doesn't happen in Emacs.
> But maybe somebody can come up with example where it does.

I happen to have another reproducible crash when I run the code that
uses `make-thread'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Fri, Jun 21 2024, Gerd Möllmann wrote:
>
>> I'm still not sure. What I'm trying to say is that we need to be sure
>> that there are no windows left in which things can change. I'd be most
>> comfortable ATM if the first thing the SIGPROF handler does is check
>> is_busy and immediately returns, doing nothing.
>
> Let's assume for moment that SIGPROF and SIGSEGV are handled in the same
> thread.  Then either SIGPROF comes before SIGSEGV or after.
>
> Case 1 (SEGPROF before SIGSEGV): here mps_arena_busy will return false
> and will access the MPS-managed memory.  This is fine, because to MPS
> this is no different than any other client code.

That would be SIGPROF delivered while client has not hit a barrier
before. SIGPROF handler runs and may hit a barrier or not.

If it doesn't hit a barrier, that's a good case, I think.

If the SIGPROF handler does hit a barrier, MPS handles the SIGSEGV
resulting from it, presumably, and let's say it takes some time.

If no SIGPROF arrives during that time, we are in case 2 (SIGSEGV before
SIFPROF). If a SIGPROF arrives kind of also.

> Case 2 (SEGSEGV before SIGPROF): here mps_arena_busy will return true
> and we only increase a counter.  This is also fine.
>
> Convinced this is safe and that we that have covered all cases?

Hmmmmmmm, I guess it wouldn't hurt to try, so I've pushed your patch :-)

Re: MPS: profiler

[Helmut Eller]

On Fri, Jun 21 2024, Ihor Radchenko wrote:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>> Now for the harder situation where SIGPROF and SIGSEGV aren't handled in
>> the same thread.  Well, I would say that this doesn't happen in Emacs.
>> But maybe somebody can come up with example where it does.
>
> I happen to have another reproducible crash when I run the code that
> uses `make-thread'.

Something involving the profiler and make-thread or just make-thread?

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>> Now for the harder situation where SIGPROF and SIGSEGV aren't handled in
>> the same thread.  Well, I would say that this doesn't happen in Emacs.
>> But maybe somebody can come up with example where it does.
>
> I happen to have another reproducible crash when I run the code that
> uses `make-thread'.

Could you please try with Helmut's patch? (Pushed a minuute ago.)

Re: MPS: profiler

[Ihor Radchenko]

Helmut Eller <eller.helmut@gmail.com> writes:

>> I happen to have another reproducible crash when I run the code that
>> uses `make-thread'.
>
> Something involving the profiler and make-thread or just make-thread?

Just make-thread.
(I am not sure if it is related; if not, I'd better try to prepare a
reproducer later and start a new email thread)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: eller.helmut@gmail.com,  yantar92@posteo.net,  emacs-devel@gnu.org
> Date: Fri, 21 Jun 2024 13:47:12 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> >> Yes, the SIBPROF handler could at least return early then.
> >> >
> >> > Perhaps something like this?
> >> 
> >> Not sure. The result of is_busy is only valid at the point in time when
> >> it is called.
> >
> > Are you thinking about what happens when GC is run concurrently?
> > Because this is not what happens here, AFAIU.  Let's focus on fixing
> > the actual issue we see in the backtrace, and consider its possible
> > generalizations later.
> >
> > Are you saying that is_busy could return false in the situation we see
> > in the backtrace, i.e. during the entire time MPS processes its
> > protection-induced SIGSEGV?
> 
> I'm still not sure. What I'm trying to say is that we need to be sure
> that there are no windows left in which things can change. I'd be most
> comfortable ATM if the first thing the SIGPROF handler does is check
> is_busy and immediately returns, doing nothing.

Does that solve the crashes reported here?

MPS make-thread (was: MPS: profiler)

[Helmut Eller]

> Just make-thread.
> (I am not sure if it is related; if not, I'd better try to prepare a
> reproducer later and start a new email thread)

This crashes for me:

  emacs -Q -batch -eval \
   '(thread-join (make-thread (lambda () (igc--collect))))'

Re: MPS make-thread

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

>> Just make-thread.
>> (I am not sure if it is related; if not, I'd better try to prepare a
>> reproducer later and start a new email thread)
>
> This crashes for me:
>
>   emacs -Q -batch -eval \
>    '(thread-join (make-thread (lambda () (igc--collect))))'

On macOS, and in my fork, it says

emacs(38573,0x17071f000) malloc: *** error for object 0x1040c8008: pointer being freed was not allocated
emacs(38573,0x17071f000) malloc: *** set a breakpoint in malloc_error_break to debug

Re: MPS make-thread

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>>> Just make-thread.
>>> (I am not sure if it is related; if not, I'd better try to prepare a
>>> reproducer later and start a new email thread)
>>
>> This crashes for me:
>>
>>   emacs -Q -batch -eval \
>>    '(thread-join (make-thread (lambda () (igc--collect))))'
>
> On macOS, and in my fork, it says
>
> emacs(38573,0x17071f000) malloc: *** error for object 0x1040c8008: pointer being freed was not allocated
> emacs(38573,0x17071f000) malloc: *** set a breakpoint in malloc_error_break to debug

It's the xfree in run_thread here:

    struct handler *c, *c_next;
    for (c = handlerlist_sentinel; c; c = c_next)
      {
	c_next = c->nextfree;
	xfree (c);
      }

c is an MPS objects. I can fix that later after verifying that all
handlers are now in MPS. Unless someone(tm) is quicker, of course.

Re: MPS make-thread

[Helmut Eller]

On Fri, Jun 21 2024, Gerd Möllmann wrote:

> It's the xfree in run_thread here:
>
>     struct handler *c, *c_next;
>     for (c = handlerlist_sentinel; c; c = c_next)
>       {
> 	c_next = c->nextfree;
> 	xfree (c);
>       }
>
> c is an MPS objects. I can fix that later after verifying that all
> handlers are now in MPS.

I think current_thread and all_threads also need to be roots.  

>  Unless someone(tm) is quicker, of course.

I'll delay looking at threads until native compilation works :-)

Re: MPS: profiler

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> I'm still not sure. What I'm trying to say is that we need to be sure
>> that there are no windows left in which things can change. I'd be most
>> comfortable ATM if the first thing the SIGPROF handler does is check
>> is_busy and immediately returns, doing nothing.
>
> Does that solve the crashes reported here?

I can no longer reproduce on my side, after pulling the latest commit in
the branch - profiler does not lead to crashes in the reproducer I
provided and also using my main config.

Although, what is curious, I am now getting a sizable contribution of the
GC reported by the profiler:

       19193  93% + command-execute
        1131   5%   Automatic GC

If necessary, I can compare the above result with what happens on master.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

>>> Now for the harder situation where SIGPROF and SIGSEGV aren't handled in
>>> the same thread.  Well, I would say that this doesn't happen in Emacs.
>>> But maybe somebody can come up with example where it does.
>>
>> I happen to have another reproducible crash when I run the code that
>> uses `make-thread'.
>
> Could you please try with Helmut's patch? (Pushed a minuute ago.)

The particular crash that happens with my make-thread code (it also
involves calling magit and external git sub-processes) is still there
with Helmut's patch.

(the profiler crash did disappear though; see my other reply to Eli)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> Also, can 'static struct profiler_log cpu', which is being worked on
> by record_backtrace, be affected by MPS in any way?
>
> Ihor, can you show the data involved in this segfault, i.e. trace[i]
> which is the argument of CLOSUREP that segfaults?  And in general all
> the data inside trace_hash.  You'll need to "source .gdbinit" to be
> able to show Lisp data in human-readable format.

I assume that it is no longer necessary to provide this data (I no
longer see the profiler-related crash with the latest scratch/igc
branch)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> I'm still not sure. What I'm trying to say is that we need to be sure
>>> that there are no windows left in which things can change. I'd be most
>>> comfortable ATM if the first thing the SIGPROF handler does is check
>>> is_busy and immediately returns, doing nothing.
>>
>> Does that solve the crashes reported here?
>
> I can no longer reproduce on my side, after pulling the latest commit in
> the branch - profiler does not lead to crashes in the reproducer I
> provided and also using my main config.

👍

> Although, what is curious, I am now getting a sizable contribution of the
> GC reported by the profiler:
>
>        19193  93% + command-execute
>         1131   5%   Automatic GC

No deia, sorry.

Re: MPS make-thread

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Fri, Jun 21 2024, Gerd Möllmann wrote:
>
>> It's the xfree in run_thread here:
>>
>>     struct handler *c, *c_next;
>>     for (c = handlerlist_sentinel; c; c = c_next)
>>       {
>> 	c_next = c->nextfree;
>> 	xfree (c);
>>       }
>>
>> c is an MPS objects. I can fix that later after verifying that all
>> handlers are now in MPS.
>
> I think current_thread and all_threads also need to be roots.

Ok, thanks.

>
>>  Unless someone(tm) is quicker, of course.
>
> I'll delay looking at threads until native compilation works :-)

I thought native comp works for you?

Re: MPS make-thread

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>> On Fri, Jun 21 2024, Gerd Möllmann wrote:
>>
>>> It's the xfree in run_thread here:
>>>
>>>     struct handler *c, *c_next;
>>>     for (c = handlerlist_sentinel; c; c = c_next)
>>>       {
>>> 	c_next = c->nextfree;
>>> 	xfree (c);
>>>       }
>>>
>>> c is an MPS objects. I can fix that later after verifying that all
>>> handlers are now in MPS.
>>
>> I think current_thread and all_threads also need to be roots.
>
> Ok, thanks.
>
>>
>>>  Unless someone(tm) is quicker, of course.
>>
>> I'll delay looking at threads until native compilation works :-)
>
> I thought native comp works for you?

Pushed, please check.

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: gerd.moellmann@gmail.com, emacs-devel@gnu.org, eller.helmut@gmail.com
> Date: Fri, 21 Jun 2024 16:12:35 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Also, can 'static struct profiler_log cpu', which is being worked on
> > by record_backtrace, be affected by MPS in any way?
> >
> > Ihor, can you show the data involved in this segfault, i.e. trace[i]
> > which is the argument of CLOSUREP that segfaults?  And in general all
> > the data inside trace_hash.  You'll need to "source .gdbinit" to be
> > able to show Lisp data in human-readable format.
> 
> I assume that it is no longer necessary to provide this data (I no
> longer see the profiler-related crash with the latest scratch/igc
> branch)

I'd still like to see that, but I guess it's too late now.

Re: MPS: profiler

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> Although, what is curious, I am now getting a sizable contribution of the
> GC reported by the profiler:
>
>        19193  93% + command-execute
>         1131   5%   Automatic GC

And this is without MPS:

       21229  92% + command-execute
        1734   7%   Automatic GC

Does it mean that MPS blocking is comparable to built-in GC? Or maybe
the profiler output is no longer accurate?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> Although, what is curious, I am now getting a sizable contribution of the
>> GC reported by the profiler:
>>
>>        19193  93% + command-execute
>>         1131   5%   Automatic GC
>
> And this is without MPS:
>
>        21229  92% + command-execute
>         1734   7%   Automatic GC
>
> Does it mean that MPS blocking is comparable to built-in GC? Or maybe
> the profiler output is no longer accurate?

I haven't paid much attention to the profiler, because there were, let's
say more important things to get to work first, so anything regarding GC
the profiler spits out is with almost 100%V probablity obscure :-).,

What do you mean by blocking? Igc ignores things like
inhibit_garbage_collection.

Re: MPS: profiler

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

>>>        19193  93% + command-execute
>>>         1131   5%   Automatic GC
>>
>> And this is without MPS:
>>
>>        21229  92% + command-execute
>>         1734   7%   Automatic GC
>>
>> Does it mean that MPS blocking is comparable to built-in GC? Or maybe
>> the profiler output is no longer accurate?
>
> I haven't paid much attention to the profiler, because there were, let's
> say more important things to get to work first, so anything regarding GC
> the profiler spits out is with almost 100%V probablity obscure :-).,
>
> What do you mean by blocking? Igc ignores things like
> inhibit_garbage_collection.

My understanding is that MPS sometimes needs to stop Emacs, just like
the traditional GC does. And I was hoping to see how frequently such
stopping happens in practice compared to old GC. So, I fired the
profiler and saw the above output.

Now, the question is whether the profiler output wrt "Automatic GC" on
scratch/igc branch represent the moments when Emacs is being properly
frozen.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

>> What do you mean by blocking? Igc ignores things like
>> inhibit_garbage_collection.
>
> My understanding is that MPS sometimes needs to stop Emacs, just like
> the traditional GC does. And I was hoping to see how frequently such
> stopping happens in practice compared to old GC. So, I fired the
> profiler and saw the above output.
>
> Now, the question is whether the profiler output wrt "Automatic GC" on
> scratch/igc branch represent the moments when Emacs is being properly
> frozen.

Simply said no.

MPS telemetry could perhaps be used, not sure, and it would of course
need someone to do it :-).

Re: MPS: profiler

[Dmitry Gutov]

On 21/06/2024 14:33, Gerd Möllmann wrote:
>>> My understanding that the suggested parking was for the time profiler is
>>> active in Emacs (that is: between M-x profiler-start and M-x
>>> profiler-stop).
>> That's unreasonable and impractical.  Gerd, was this what you
>> suggested?
> Yes, that's what I meant 🙂. That woulld be safe, at least.

The result of this also might be that the profiler would underestimate 
the time normally spent in garbage collection (something that affects 
performance, and thus is included in what we generally want a profiler 
to tell about).

Re: MPS make-thread

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

>>> I think current_thread and all_threads also need to be roots.
>>
>> Ok, thanks.
>>>
>>>>  Unless someone(tm) is quicker, of course.
>>>
>>> I'll delay looking at threads until native compilation works :-)
>>
>> I thought native comp works for you?
>
> Pushed, please check.

Now, there is no crash when I run my code that creates threads, but
there is a crash soon after :(

thix.c:67: Emacs fatal error: assertion failed: SigCheck Thread: thread
Fatal error 6: Aborted

lockix.c:126: Emacs fatal error: assertion failed: res == 0

Thread 1 "emacs" received signal SIGABRT, Aborted.
0x00007ffff58ae9fb in pthread_kill () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff58ae9fb in pthread_kill () at /lib64/libc.so.6
#1  0x00007ffff58553d2 in raise () at /lib64/libc.so.6
#2  0x00005555556bba66 in terminate_due_to_signal (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=2147483647) at emacs.c:481
#3  0x00005555557b2ec9 in igc_assert_fail (file=<optimized out>, line=<optimized out>, msg=<optimized out>) at igc.c:158
#4  0x000055555582fe29 in mps_lib_assert_fail (condition=0x55555588ff52 "res == 0", line=126, file=0x55555588ff3c "lockix.c")
    at /home/yantar92/Dist/mps/code/mpsliban.c:87
#5  LockClaim (lock=0x7fffe8000110) at /home/yantar92/Dist/mps/code/lockix.c:126
#6  0x000055555583005d in ArenaEnterLock (arena=0x7ffff7fc2000, recursive=0) at /home/yantar92/Dist/mps/code/global.c:576
#7  0x000055555585e09c in ArenaEnter (arena=0x7ffff7fc2000) at /home/yantar92/Dist/mps/code/global.c:553
#8  mps_ap_fill (p_o=p_o@entry=0x7fffffffa490, mps_ap=mps_ap@entry=0x7fffe8001770, size=size@entry=24) at /home/yantar92/Dist/mps/code/mpsi.c:1094
#9  0x00005555557b444c in alloc_impl (size=24, type=IGC_OBJ_CONS, ap=0x7fffe8001770) at igc.c:3159
#10 0x00005555557b454a in alloc (size=size@entry=16, type=type@entry=IGC_OBJ_CONS) at igc.c:3177
#11 0x00005555557b5f24 in igc_make_cons (car=car@entry=0x2, cdr=cdr@entry=0x0) at igc.c:3204
#12 0x000055555571f922 in Fcons (cdr=0x0, car=0x2) at alloc.c:2926
#13 list2 (arg1=arg1@entry=0x7f20, arg2=arg2@entry=0x2) at alloc.c:2978
#14 0x000055555578913c in Fdelete_process (process=0x7fffae000ffd) at process.c:1124
#15 0x00005555557916b0 in kill_buffer_processes (buffer=buffer@entry=0x0) at process.c:8289
#16 0x00005555556bb797 in shut_down_emacs (sig=sig@entry=6, stuff=stuff@entry=0x0) at emacs.c:3140
#17 0x00005555556bba2f in terminate_due_to_signal (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=2147483647) at emacs.c:464
#18 0x00005555557b2ec9 in igc_assert_fail (file=<optimized out>, line=<optimized out>, msg=<optimized out>) at igc.c:158
#19 0x000055555584d2ae in mps_lib_assert_fail (condition=0x55555588c9ee "SigCheck Thread: thread", line=67, file=0x55555588ca06 "thix.c")
    at /home/yantar92/Dist/mps/code/mpsliban.c:87
#20 ThreadCheck (thread=0x7fff98184c40) at /home/yantar92/Dist/mps/code/thix.c:67
#21 ThreadCheck (thread=0x7fff98184c40) at /home/yantar92/Dist/mps/code/thix.c:65
#22 ThreadScan (ss=0x7fffffffb6f0, thread=0x7fff98184c40, stackCold=0x7fffdabffbb0, scan_area=0x5555557b24fc <scan_ambig>, closure=0x0)
    at /home/yantar92/Dist/mps/code/thix.c:259
#23 0x00005555558595a8 in RootScan (ss=ss@entry=0x7fffffffb6f0, root=root@entry=0x7fff98184cb8) at /home/yantar92/Dist/mps/code/root.c:568
#24 0x0000555555859e75 in traceScanRootRes (ts=ts@entry=1, rank=rank@entry=0, arena=arena@entry=0x7ffff7fc2000, root=root@entry=0x7fff98184cb8)
    at /home/yantar92/Dist/mps/code/trace.c:528
#25 0x000055555585a591 in traceScanRoot (root=0x7fff98184cb8, arena=0x7ffff7fc2000, rank=0, ts=<optimized out>) at /home/yantar92/Dist/mps/code/trace.c:545
#26 rootFlip (p=<synthetic pointer>, root=0x7fff98184cb8) at /home/yantar92/Dist/mps/code/trace.c:580
#27 RootsIterate (p=<synthetic pointer>, f=<optimized out>, arena=0x7ffff7fc2008) at /home/yantar92/Dist/mps/code/root.c:665
#28 traceFlip (trace=0x7ffff7fc2aa8) at /home/yantar92/Dist/mps/code/trace.c:652
#29 TraceStart (trace=trace@entry=0x7ffff7fc2aa8, mortality=<optimized out>, finishingTime=<optimized out>) at /home/yantar92/Dist/mps/code/trace.c:1694
#30 0x000055555585b2cf in PolicyStartTrace
    (traceReturn=traceReturn@entry=0x7fffffffb8b0, collectWorldReturn=collectWorldReturn@entry=0x7fffffffb90c, arena=arena@entry=0x7ffff7fc2000, collectWorldAllowed=collectWorldAllowed@entry=1) at /home/yantar92/Dist/mps/code/policy.c:335
#31 0x000055555585db52 in TracePoll
    (workReturn=workReturn@entry=0x7fffffffb910, collectWorldReturn=collectWorldReturn@entry=0x7fffffffb90c, globals=globals@entry=0x7ffff7fc2008, collectWorldAllowed=1) at /home/yantar92/Dist/mps/code/trace.c:1840
#32 0x000055555585dcfb in ArenaPoll (globals=globals@entry=0x7ffff7fc2008) at /home/yantar92/Dist/mps/code/global.c:745
--Type <RET> for more, q to quit, c to continue without paging--c
#33 0x000055555585e0ea in mps_ap_fill (p_o=p_o@entry=0x7fffffffba80, mps_ap=mps_ap@entry=0x7fffe8001770, size=size@entry=48)
    at /home/yantar92/Dist/mps/code/mpsi.c:1097
#34 0x00005555557b444c in alloc_impl (size=48, type=IGC_OBJ_VECTOR, ap=0x7fffe8001770) at igc.c:3159
#35 0x00005555557b454a in alloc (size=size@entry=40, type=type@entry=IGC_OBJ_VECTOR) at igc.c:3177
#36 0x00005555557b6126 in igc_alloc_pseudovector
    (nwords_mem=nwords_mem@entry=4, nwords_lisp=nwords_lisp@entry=0, nwords_zero=nwords_zero@entry=0, tag=tag@entry=PVEC_MARKER) at igc.c:3325
#37 0x000055555572040d in allocate_pseudovector (tag=PVEC_MARKER, zerolen=0, lisplen=0, memlen=4) at alloc.c:3786
#38 build_marker (buf=0x7fff9ca44c88, charpos=12320533, bytepos=13012048) at alloc.c:4179
#39 0x00005555557307f0 in Fpoint_marker () at editfns.c:202
#40 0x000055555573a8bd in save_excursion_save (pdl=0x555556cecc50) at editfns.c:782
#41 0x000055555573ed3d in record_unwind_protect_excursion () at eval.c:3678
#42 0x0000555555781ef4 in exec_byte_code (fun=<optimized out>, fun@entry=0x7fffaea538ed, args_template=<optimized out>, 
    args_template@entry=257, nargs=<optimized out>, nargs@entry=1, args=<optimized out>, args@entry=0x7fffffffbd18) at bytecode.c:942
#43 0x0000555555742c4a in funcall_lambda (fun=0x7fffaea538ed, nargs=nargs@entry=1, arg_vector=arg_vector@entry=0x7fffffffbd18) at eval.c:3270
#44 0x0000555555743175 in funcall_general (fun=<optimized out>, numargs=numargs@entry=1, args=args@entry=0x7fffffffbd18) at eval.c:3062
#45 0x000055555573f3f0 in Ffuncall (nargs=nargs@entry=2, args=args@entry=0x7fffffffbd10) at eval.c:3111
#46 0x000055555574a824 in mapcar1 (leni=leni@entry=13, vals=vals@entry=0x7fffffffbd70, fn=fn@entry=0x7fffaea538ed, seq=seq@entry=0x7fffa6242f8b)
    at fns.c:3349
#47 0x000055555574d463 in Fmapcar (function=0x7fffaea538ed, sequence=0x7fffa6242f8b) at fns.c:3469
#48 0x0000555555741198 in funcall_subr (subr=<optimized out>, numargs=numargs@entry=2, args=args@entry=0x7fffe2200400) at eval.c:3181
#49 0x0000555555781aac in exec_byte_code (fun=<optimized out>, fun@entry=0x7fffae9cdc5d, args_template=<optimized out>, 
    args_template@entry=256, nargs=<optimized out>, nargs@entry=0, args=<optimized out>, args@entry=0x7fffffffbfb0)
    at /home/yantar92/Git/emacs/src/lisp.h:2267
#50 0x0000555555742c4a in funcall_lambda (fun=fun@entry=0x7fffae9cdc5d, nargs=nargs@entry=0, arg_vector=arg_vector@entry=0x7fffffffbfb0) at eval.c:3270
#51 0x00005555557438ef in apply_lambda (fun=fun@entry=0x7fffae9cdc5d, args=<optimized out>, count=count@entry=...) at eval.c:3233
#52 0x00005555557425fe in eval_sub (form=<optimized out>) at eval.c:2663
#53 0x00005555557422a1 in eval_sub (form=<optimized out>) at eval.c:2604
#54 0x0000555555742b00 in Fprogn (body=<optimized out>) at eval.c:448
#55 0x00005555557433c3 in Fif (args=<optimized out>) at eval.c:404
#56 0x0000555555742326 in eval_sub (form=<optimized out>) at eval.c:2567
#57 0x0000555555742b00 in Fprogn (body=<optimized out>) at eval.c:448
#58 0x0000555555742326 in eval_sub (form=<optimized out>) at eval.c:2567
#59 0x00005555557433b0 in Fif (args=0x7fffab5e4c43) at eval.c:403
#60 0x0000555555742326 in eval_sub (form=<optimized out>) at eval.c:2567
#61 0x0000555555742b00 in Fprogn (body=<optimized out>) at eval.c:448
#62 0x0000555555742ee2 in funcall_lambda (fun=<optimized out>, nargs=nargs@entry=0, arg_vector=arg_vector@entry=0x7fffffffc5a0) at eval.c:3368
#63 0x0000555555743175 in funcall_general (fun=<optimized out>, numargs=numargs@entry=0, args=args@entry=0x7fffffffc5a0) at eval.c:3062
#64 0x000055555573f3f0 in Ffuncall (nargs=1, args=0x7fffffffc598) at eval.c:3111
#65 0x000055555573f927 in funcall_nil (nargs=<optimized out>, args=<optimized out>) at eval.c:2794
#66 0x000055555573e4aa in run_hook_with_args (nargs=nargs@entry=1, args=args@entry=0x7fffffffc598, funcall=funcall@entry=0x55555573f91e <funcall_nil>)
    at eval.c:2971
#67 0x000055555573e5fe in Frun_hook_with_args (nargs=nargs@entry=1, args=args@entry=0x7fffffffc598) at eval.c:2836
#68 0x000055555573e61b in run_hook (hook=<optimized out>) at eval.c:2984
#69 0x000055555573e63d in Frun_hooks (nargs=1, args=0x7fffe2200388) at eval.c:2818
#70 0x0000555555741273 in funcall_subr (subr=<optimized out>, numargs=numargs@entry=1, args=args@entry=0x7fffe2200388) at eval.c:3202
#71 0x0000555555781aac in exec_byte_code (fun=<optimized out>, fun@entry=0x7fffa2e46ed5, args_template=<optimized out>, 
    args_template@entry=0, nargs=<optimized out>, nargs@entry=0, args=<optimized out>, args@entry=0x7fffffffc7d8)
    at /home/yantar92/Git/emacs/src/lisp.h:2267
#72 0x0000555555742c4a in funcall_lambda (fun=0x7fffa2e46ed5, nargs=nargs@entry=0, arg_vector=arg_vector@entry=0x7fffffffc7d8) at eval.c:3270
#73 0x0000555555743175 in funcall_general (fun=<optimized out>, numargs=numargs@entry=0, args=args@entry=0x7fffffffc7d8) at eval.c:3062
#74 0x000055555573f3f0 in Ffuncall (nargs=1, args=0x7fffffffc7d0) at eval.c:3111
#75 0x0000555555742407 in eval_sub (form=<optimized out>) at eval.c:2588
#76 0x0000555555742b00 in Fprogn (body=<optimized out>) at eval.c:448
#77 0x0000555555743d3c in Flet (args=0x7fffa2e46ff3) at /home/yantar92/Git/emacs/src/lisp.h:1554
#78 0x0000555555742326 in eval_sub (form=form@entry=0x7fffa2e4900b) at eval.c:2567
#79 0x000055555574466b in Feval (form=0x7fffa2e4900b, lexical=<optimized out>) at eval.c:2475
#80 0x0000555555741198 in funcall_subr (subr=<optimized out>, numargs=numargs@entry=1, args=args@entry=0x7fffe22001a8) at eval.c:3181
#81 0x0000555555781aac in exec_byte_code (fun=<optimized out>, fun@entry=0x7fffeb002f55, args_template=<optimized out>, 
    args_template@entry=768, nargs=<optimized out>, nargs@entry=1, args=<optimized out>, args@entry=0x7fffe2200108)
    at /home/yantar92/Git/emacs/src/lisp.h:2267
#82 0x0000555555742c4a in funcall_lambda (fun=0x7fffeb002f55, nargs=nargs@entry=1, arg_vector=arg_vector@entry=0x7fffe2200108) at eval.c:3270
#83 0x0000555555743175 in funcall_general (fun=<optimized out>, numargs=numargs@entry=1, args=args@entry=0x7fffe2200108) at eval.c:3062
#84 0x000055555573f3f0 in Ffuncall (nargs=nargs@entry=2, args=args@entry=0x7fffe2200100) at eval.c:3111
#85 0x000055555573f72b in Fapply (nargs=2, args=0x7fffe2200100) at eval.c:2740
#86 0x0000555555741273 in funcall_subr (subr=<optimized out>, numargs=numargs@entry=2, args=args@entry=0x7fffe2200100) at eval.c:3202
#87 0x0000555555781aac in exec_byte_code (fun=<optimized out>, fun@entry=0x7fff97c5ab2d, args_template=<optimized out>, 
    args_template@entry=128, nargs=<optimized out>, nargs@entry=1, args=<optimized out>, args@entry=0x7fffffffcf90)
    at /home/yantar92/Git/emacs/src/lisp.h:2267
#88 0x0000555555742c4a in funcall_lambda (fun=0x7fff97c5ab2d, nargs=nargs@entry=1, arg_vector=arg_vector@entry=0x7fffffffcf90) at eval.c:3270
#89 0x0000555555743175 in funcall_general (fun=<optimized out>, numargs=numargs@entry=1, args=args@entry=0x7fffffffcf90) at eval.c:3062
#90 0x000055555573f3f0 in Ffuncall (nargs=nargs@entry=2, args=args@entry=0x7fffffffcf88) at eval.c:3111
#91 0x000055555573b559 in Ffuncall_interactively (nargs=2, args=0x7fffffffcf88) at callint.c:250
#92 0x0000555555741273 in funcall_subr (subr=<optimized out>, numargs=numargs@entry=2, args=args@entry=0x7fffffffcf88) at eval.c:3202
#93 0x0000555555743166 in funcall_general (fun=<optimized out>, numargs=numargs@entry=2, args=args@entry=0x7fffffffcf88)
    at /home/yantar92/Git/emacs/src/lisp.h:2267
#94 0x000055555573f3f0 in Ffuncall (nargs=nargs@entry=3, args=args@entry=0x7fffffffcf80) at eval.c:3111
#95 0x000055555573ce6e in Fcall_interactively (function=<optimized out>, record_flag=<optimized out>, keys=<optimized out>) at callint.c:789
#96 0x00005555557411af in funcall_subr (subr=<optimized out>, numargs=numargs@entry=3, args=args@entry=0x7fffe2200070) at eval.c:3183
#97 0x0000555555781aac in exec_byte_code (fun=<optimized out>, fun@entry=0x7ffff2be9cfd, args_template=<optimized out>, 
    args_template@entry=1025, nargs=<optimized out>, nargs@entry=1, args=<optimized out>, args@entry=0x7fffffffd398)
    at /home/yantar92/Git/emacs/src/lisp.h:2267
#98 0x0000555555742c4a in funcall_lambda (fun=0x7ffff2be9cfd, nargs=nargs@entry=1, arg_vector=arg_vector@entry=0x7fffffffd398) at eval.c:3270
#99 0x0000555555743175 in funcall_general (fun=<optimized out>, numargs=numargs@entry=1, args=args@entry=0x7fffffffd398) at eval.c:3062
#100 0x000055555573f3f0 in Ffuncall (nargs=nargs@entry=2, args=args@entry=0x7fffffffd390) at eval.c:3111
#101 0x00005555556d2d74 in command_loop_1 () at keyboard.c:1551
#102 0x000055555573dddf in internal_condition_case
    (bfun=bfun@entry=0x5555556d1960 <command_loop_1>, handlers=handlers@entry=0x90, hfun=hfun@entry=0x5555556c3232 <cmd_error>) at eval.c:1622
#103 0x00005555556be5fb in command_loop_2 (handlers=handlers@entry=0x90) at keyboard.c:1169
#104 0x000055555573dd1d in internal_catch (tag=tag@entry=0x12300, func=func@entry=0x5555556be5d8 <command_loop_2>, arg=arg@entry=0x90) at eval.c:1301
#105 0x00005555556be5b5 in command_loop () at keyboard.c:1147
#106 0x00005555556c2e0e in recursive_edit_1 () at keyboard.c:755
#107 0x00005555556c3158 in Frecursive_edit () at keyboard.c:838
#108 0x00005555556bd8ff in main (argc=1, argv=<optimized out>) at emacs.c:2643
(gdb) c
Continuing.
Couldn't get registers: No such process.
(gdb) [Thread 0x7fffda2006c0 (LWP 27886) exited]
[Thread 0x7fffdbe006c0 (LWP 27884) exited]
[Thread 0x7fffe16006c0 (LWP 27882) exited]
[Thread 0x7fffe20006c0 (LWP 27881) exited]
[Thread 0x7ffff5249a40 (LWP 27874) exited]
[Thread 0x7fffe0c006c0 (LWP 27883) exited]
[New process 27874]

Program terminated with signal SIGABRT, Aborted.
The program no longer exists.

The program is not being run.
(gdb) 

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS: profiler

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>>> What do you mean by blocking? Igc ignores things like
>>> inhibit_garbage_collection.
>>
>> My understanding is that MPS sometimes needs to stop Emacs, just like
>> the traditional GC does. And I was hoping to see how frequently such
>> stopping happens in practice compared to old GC. So, I fired the
>> profiler and saw the above output.
>>
>> Now, the question is whether the profiler output wrt "Automatic GC" on
>> scratch/igc branch represent the moments when Emacs is being properly
>> frozen.
>
> Simply said no.

Then, we should probably not record the times when MPS is active as
"Automatic GC" in the profiler. Maybe simply skip these samples for the
time being (with a FIXME in the code)? That would be more accurate.

> MPS telemetry could perhaps be used, not sure, and it would of course
> need someone to do it :-).

We will eventually need some kind of telemetry to compare traditional GC
vs. MPS. Of course, getting things work reliably is more important at
this stage.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS make-thread

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>>>> I think current_thread and all_threads also need to be roots.
>>>
>>> Ok, thanks.
>>>>
>>>>>  Unless someone(tm) is quicker, of course.
>>>>
>>>> I'll delay looking at threads until native compilation works :-)
>>>
>>> I thought native comp works for you?
>>
>> Pushed, please check.
>
> Now, there is no crash when I run my code that creates threads, but
> there is a crash soon after :(
>
> thix.c:67: Emacs fatal error: assertion failed: SigCheck Thread: thread
> Fatal error 6: Aborted

Any chance to make this reproducible? Maybe by adding calls to
(igc--collect) in various places? I don't know what you are doing...

Re: MPS make-thread

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Helmut Eller <eller.helmut@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>  emacs-devel@gnu.org
> Date: Fri, 21 Jun 2024 19:58:54 +0000
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Pushed, please check.
> 
> Now, there is no crash when I run my code that creates threads, but
> there is a crash soon after :(

What did you run to get this?  AFAIU, the recipe used batch mode, in
which case there's no "soon after", because Emacs exits.

I see in the backtrace Fdelete_process, so what process was that and
how was it run?

Running xbacktrace after "bt" should produce useful info by showing us
the Lisp backtrace, so always try to do that (unless you know that
"xbacktrace" crashes in the particular situation).

And I think there's another problem here:

> Thread 1 "emacs" received signal SIGABRT, Aborted.
> 0x00007ffff58ae9fb in pthread_kill () from /lib64/libc.so.6
> (gdb) bt
> #0  0x00007ffff58ae9fb in pthread_kill () at /lib64/libc.so.6
> #1  0x00007ffff58553d2 in raise () at /lib64/libc.so.6
> #2  0x00005555556bba66 in terminate_due_to_signal (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=2147483647) at emacs.c:481
> #3  0x00005555557b2ec9 in igc_assert_fail (file=<optimized out>, line=<optimized out>, msg=<optimized out>) at igc.c:158
> #4  0x000055555582fe29 in mps_lib_assert_fail (condition=0x55555588ff52 "res == 0", line=126, file=0x55555588ff3c "lockix.c")
>     at /home/yantar92/Dist/mps/code/mpsliban.c:87
> #5  LockClaim (lock=0x7fffe8000110) at /home/yantar92/Dist/mps/code/lockix.c:126
> #6  0x000055555583005d in ArenaEnterLock (arena=0x7ffff7fc2000, recursive=0) at /home/yantar92/Dist/mps/code/global.c:576
> #7  0x000055555585e09c in ArenaEnter (arena=0x7ffff7fc2000) at /home/yantar92/Dist/mps/code/global.c:553
> #8  mps_ap_fill (p_o=p_o@entry=0x7fffffffa490, mps_ap=mps_ap@entry=0x7fffe8001770, size=size@entry=24) at /home/yantar92/Dist/mps/code/mpsi.c:1094
> #9  0x00005555557b444c in alloc_impl (size=24, type=IGC_OBJ_CONS, ap=0x7fffe8001770) at igc.c:3159
> #10 0x00005555557b454a in alloc (size=size@entry=16, type=type@entry=IGC_OBJ_CONS) at igc.c:3177
> #11 0x00005555557b5f24 in igc_make_cons (car=car@entry=0x2, cdr=cdr@entry=0x0) at igc.c:3204
> #12 0x000055555571f922 in Fcons (cdr=0x0, car=0x2) at alloc.c:2926
> #13 list2 (arg1=arg1@entry=0x7f20, arg2=arg2@entry=0x2) at alloc.c:2978
> #14 0x000055555578913c in Fdelete_process (process=0x7fffae000ffd) at process.c:1124
> #15 0x00005555557916b0 in kill_buffer_processes (buffer=buffer@entry=0x0) at process.c:8289
> #16 0x00005555556bb797 in shut_down_emacs (sig=sig@entry=6, stuff=stuff@entry=0x0) at emacs.c:3140
> #17 0x00005555556bba2f in terminate_due_to_signal (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=2147483647) at emacs.c:464
> #18 0x00005555557b2ec9 in igc_assert_fail (file=<optimized out>, line=<optimized out>, msg=<optimized out>) at igc.c:158
> #19 0x000055555584d2ae in mps_lib_assert_fail (condition=0x55555588c9ee "SigCheck Thread: thread", line=67, file=0x55555588ca06 "thix.c")
>     at /home/yantar92/Dist/mps/code/mpsliban.c:87
> #20 ThreadCheck (thread=0x7fff98184c40) at /home/yantar92/Dist/mps/code/thix.c:67

As you see, MPS hit an assertion violation, which causes us to call
shut_down_emacs.  shut_down_emacs does all kinds of cleanups and
attempts to auto-save unsaved edits, but while doing so it could
re-enter MPS, which happens here.  Should we do something, like park
the arena, early on in shut_down_emacs to allow the auto-saving and
cleanups to run without crashing?  Or maybe ignore (some) assertions
that happen during the cleanup?

Btw, is this the debug build of MPS or a production build?  IOW, is
the second assertion, in mps_lib_assert_fail, expected in a production
build?  AFAIU, that assertion is hit because we re-enter the arena,
and its lock is already taken.

Re: MPS: profiler

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, eller.helmut@gmail.com, emacs-devel@gnu.org
> Date: Fri, 21 Jun 2024 20:02:42 +0000
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Ihor Radchenko <yantar92@posteo.net> writes:
> >
> >> Now, the question is whether the profiler output wrt "Automatic GC" on
> >> scratch/igc branch represent the moments when Emacs is being properly
> >> frozen.
> >
> > Simply said no.
> 
> Then, we should probably not record the times when MPS is active as
> "Automatic GC" in the profiler. Maybe simply skip these samples for the
> time being (with a FIXME in the code)? That would be more accurate.

I don't think I agree.  We might rename "Automatic GC" to something
more accurate, but skipping that will cause the percentage to be
skewed.  By and large, this is just about the interpretation of that
percentage, nothing else.

In any case, you are worrying about these minor issues too early.  We
are nowhere near a point where they matter.  For now, we need to make
sure the basic features of Emacs work reliably without crashing; the
accuracy of their results comes much later.  Let's not put the
proverbial wagon before the horse and waste time discussing issues
which are for now largely theoretical.

> > MPS telemetry could perhaps be used, not sure, and it would of course
> > need someone to do it :-).
> 
> We will eventually need some kind of telemetry to compare traditional GC
> vs. MPS.

Just start Emacs, visit xdisp.c, and scroll through it by leaning on
C-v, and you will immediately see how much faster this GC is.  (You
can also run 'top' or something similar simultaneously to see how the
memory footprint grows and then goes down, while you scroll.)  So
let's please not worry about what the profiler says about GC, because
the answer to your question is abundantly clear without that already.

Re: MPS make-thread

[Helmut Eller]

On Fri, Jun 21 2024, Gerd Möllmann wrote:

>>> I'll delay looking at threads until native compilation works :-)
>>
>> I thought native comp works for you?

I haven't tried with the latest dump loading code.  I think that needs
some work for native-comp-units.

> Pushed, please check.

The expression

  (thread-join (make-thread (lambda () (igc--collect))))

doesn't crash anymore.

Re: MPS make-thread

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Fri, Jun 21 2024, Gerd Möllmann wrote:
>
>>>> I'll delay looking at threads until native compilation works :-)
>>>
>>> I thought native comp works for you?
>
> I haven't tried with the latest dump loading code.  I think that needs
> some work for native-comp-units.

At least we're all in the same boat now :-).

>
>> Pushed, please check.
>
> The expression
>
>   (thread-join (make-thread (lambda () (igc--collect))))
>
> doesn't crash anymore.

👌

Re: MPS make-thread

[Ihor Radchenko]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

>> thix.c:67: Emacs fatal error: assertion failed: SigCheck Thread: thread
>> Fatal error 6: Aborted
>
> Any chance to make this reproducible? Maybe by adding calls to
> (igc--collect) in various places? I don't know what you are doing...

(progn
  (defvar *baz* nil)
  (defun bar (len)
    (let ((data (make-list len nil)))
      (setq *baz* (lambda () (bar len) data))))
  (defun foo ()
    (bar 1000)
    (dotimes (_ 10000)
      (funcall *baz*)))
  (thread-join (make-thread (lambda () (igc--collect))))
  (foo))

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS make-thread

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> Btw, is this the debug build of MPS or a production build?  IOW, is
> the second assertion, in mps_lib_assert_fail, expected in a production
> build?  AFAIU, that assertion is hit because we re-enter the arena,
> and its lock is already taken.

The backtrace I posted was a production build. (--with-mps=yes).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS make-thread

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: gerd.moellmann@gmail.com, eller.helmut@gmail.com, emacs-devel@gnu.org
> Date: Sat, 22 Jun 2024 18:53:43 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Btw, is this the debug build of MPS or a production build?  IOW, is
> > the second assertion, in mps_lib_assert_fail, expected in a production
> > build?  AFAIU, that assertion is hit because we re-enter the arena,
> > and its lock is already taken.
> 
> The backtrace I posted was a production build. (--with-mps=yes).

I was asking how was the MPS library itself built.  IOW, what variant
of the library did you link against?

Re: MPS make-thread

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Helmut Eller <eller.helmut@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>  emacs-devel@gnu.org
> Date: Sat, 22 Jun 2024 18:52:25 +0000
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> >> thix.c:67: Emacs fatal error: assertion failed: SigCheck Thread: thread
> >> Fatal error 6: Aborted
> >
> > Any chance to make this reproducible? Maybe by adding calls to
> > (igc--collect) in various places? I don't know what you are doing...
> 
> (progn
>   (defvar *baz* nil)
>   (defun bar (len)
>     (let ((data (make-list len nil)))
>       (setq *baz* (lambda () (bar len) data))))
>   (defun foo ()
>     (bar 1000)
>     (dotimes (_ 10000)
>       (funcall *baz*)))
>   (thread-join (make-thread (lambda () (igc--collect))))
>   (foo))

I get a crash only after running this several times.  It looks like
this:

  ss.c:66: Emacs fatal error: assertion failed: warmest < stackCold

  lockw3.c:98: Emacs fatal error: assertion failed: lock->claims == 0

  Thread 11 received signal SIGTRAP, Trace/breakpoint trap.
  [Switching to Thread 9512.0x10a08]
  0x774996c3 in KERNELBASE!DebugBreak () from C:\WINDOWS\SysWOW64\KernelBase.dll
  (gdb) thread apply all bt

  Thread 11 (Thread 9512.0x10a08):
  #0  0x774996c3 in KERNELBASE!DebugBreak () from C:\WINDOWS\SysWOW64\KernelBase.dll
  #1  0x002e457c in emacs_abort () at w32fns.c:11279
  #2  0x00194ca0 in terminate_due_to_signal (sig=sig@entry=22, backtrace_limit=backtrace_limit@entry=2147483647) at emacs.c:481
  #3  0x002c2ebc in igc_assert_fail (file=0x7b304d <__mon_yday+4429> "lockw3.c", line=98, msg=0x7b9a5c <__mon_yday+31580> "lock->claims == 0") at igc.c:181
  #4  0x0039c18d in mps_lib_assert_fail (condition=0x7b9a5c <__mon_yday+31580> "lock->claims == 0", line=98, file=0x7b304d <__mon_yday+4429> "lockw3.c") at mpsliban.c:87
  #5  LockClaim (lock=0xa5560c8) at lockw3.c:98
  #6  0x0039c2e5 in ArenaEnterLock (arena=arena@entry=0x7100000, recursive=recursive@entry=0) at global.c:576
  #7  0x003c9cdd in ArenaEnter (arena=0x7100000) at global.c:553
  #8  mps_ap_fill (p_o=p_o@entry=0x1fc5f838, mps_ap=mps_ap@entry=0xa605bc4, size=size@entry=24) at mpsi.c:1094
  #9  0x002c2036 in alloc_impl (size=24, size@entry=16, type=type@entry=IGC_OBJ_STRING, ap=0xa605bc4) at igc.c:3218
  #10 0x002c48f3 in alloc (type=IGC_OBJ_STRING, size=16) at igc.c:3236
  #11 igc_make_string (nchars=nchars@entry=3, nbytes=nbytes@entry=3, unibyte=unibyte@entry=false, clear=clear@entry=false) at igc.c:3344
  #12 0x002c496d in igc_make_multibyte_string (nchars=nchars@entry=3, nbytes=nbytes@entry=3, clear=clear@entry=false) at igc.c:3354
  #13 0x00209f39 in make_clear_multibyte_string (clearit=false, nbytes=3, nchars=3) at alloc.c:2635
  #14 make_clear_string (length=length@entry=3, clearit=clearit@entry=false) at alloc.c:2612
  #15 0x00209f97 in make_clear_string (clearit=<optimized out>, length=<optimized out>) at alloc.c:2621
  #16 make_uninit_string (length=3) at alloc.c:2623
  #17 make_uninit_string (length=3) at alloc.c:2621
  #18 make_unibyte_string (contents=contents@entry=0x82f3a0 <root_dir> "d:/", length=length@entry=3) at alloc.c:2538
  #19 0x0020a058 in make_string (contents=contents@entry=0x82f3a0 <root_dir> "d:/", nbytes=3) at alloc.c:2526
  #20 0x001d9f2b in build_string (str=0x82f3a0 <root_dir> "d:/") at lisp.h:4687
  #21 Fexpand_file_name (name=0x14800eb4, default_directory=default_directory@entry=0x0) at fileio.c:1070
  #22 0x001e1316 in Fdo_auto_save (no_message=<optimized out>, no_message@entry=0x18, current_only=current_only@entry=0x0) at lisp.h:1191
  #23 0x00194ada in shut_down_emacs (sig=sig@entry=22, stuff=stuff@entry=0x0) at lisp.h:1191
  #24 0x00194d06 in terminate_due_to_signal (sig=sig@entry=22, backtrace_limit=backtrace_limit@entry=2147483647) at lisp.h:1191
  #25 0x002c2ebc in igc_assert_fail (file=0x7b8b1f <__mon_yday+27679> "ss.c", line=66, msg=0x7b8b3d <__mon_yday+27709> "warmest < stackCold") at igc.c:181
  #26 0x0039571d in mps_lib_assert_fail (condition=0x7b8b3d <__mon_yday+27709> "warmest < stackCold", line=66, file=0x7b8b1f <__mon_yday+27679> "ss.c") at mpsliban.c:87
  #27 StackScan (ss=0x1fc5fb70, stackCold=0x1edeff28, scan_area=0x2c11f9 <scan_ambig>, closure=0x0) at ss.c:66
  #28 0x003c7494 in RootScan (ss=ss@entry=0x1fc5fb70, root=root@entry=0xa60e83c) at root.c:577
  #29 0x003c7d1d in traceScanRootRes (ts=ts@entry=1, rank=rank@entry=0, arena=arena@entry=0x7100000, root=root@entry=0xa60e83c) at trace.c:528
  #30 0x003c8118 in traceScanRoot (root=0xa60e83c, arena=0x7100000, rank=0, ts=1) at trace.c:545
  #31 rootFlip (p=<synthetic pointer>, root=0xa60e83c) at trace.c:580
  #32 RootsIterate (p=<synthetic pointer>, f=<optimized out>, arena=0x7100008) at root.c:665
  #33 traceFlip (trace=0x7100498) at trace.c:652
  #34 TraceStart (trace=0x7100498, mortality=0.78544231075332438, finishingTime=189006713) at trace.c:1694
  #35 0x003c89ab in TraceStartCollectAll (traceReturn=traceReturn@entry=0x1fc5fca8, arena=arena@entry=0x7100000, why=why@entry=4) at trace.c:1794
  #36 0x003c9788 in ArenaStartCollect (globals=globals@entry=0x7100008, why=why@entry=4) at traceanc.c:634
  #37 0x003c97e4 in ArenaCollect (globals=globals@entry=0x7100008, why=why@entry=4) at traceanc.c:652
  #38 0x003c9886 in mps_arena_collect (arena=0x7100000) at mpsi.c:313
  #39 0x002c4582 in igc_collect () at igc.c:3150
  #40 0x002c4599 in Figc__collect () at igc.c:3159
  #41 0x00239c8a in eval_sub (form=0xe58609b) at eval.c:2613
  #42 0x00239fa3 in Fprogn (body=0x0) at eval.c:448

We must do something about these assertions: when there's an assertion
violation caused by a thread which was started by MPS, we cannot call
shut_down_emacs in that thread's context, for obvious reasons.  We
must instead set some flag which will cause the main thread or one of
the other Lisp threads call shut_down_emacs.  The MPS documentation
says:

          Warning: The installed assertion handler must not call any
          function in MPS, and it must not access memory managed by the
          MPS.

But our handler, igc_assert_fail, does exactly what they say not to
do.

And what does "warmest < stackCold" mean, in human-understandable
terms, anyway?

Re: MPS make-thread

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:
>> > Btw, is this the debug build of MPS or a production build?  IOW, is
>> > the second assertion, in mps_lib_assert_fail, expected in a production
>> > build?  AFAIU, that assertion is hit because we re-enter the arena,
>> > and its lock is already taken.
>> 
>> The backtrace I posted was a production build. (--with-mps=yes).
>
> I was asking how was the MPS library itself built.  IOW, what variant
> of the library did you link against?

No idea. I just did a dumb make install to get MPS.

$ ls /opt/mps/lib/
libmps.a  libmps-debug.a

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS make-thread

[Gerd Möllmann]

Ihor Radchenko <yantar92@posteo.net> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>>> thix.c:67: Emacs fatal error: assertion failed: SigCheck Thread: thread
>>> Fatal error 6: Aborted
>>
>> Any chance to make this reproducible? Maybe by adding calls to
>> (igc--collect) in various places? I don't know what you are doing...
>
> (progn
>   (defvar *baz* nil)
>   (defun bar (len)
>     (let ((data (make-list len nil)))
>       (setq *baz* (lambda () (bar len) data))))
>   (defun foo ()
>     (bar 1000)
>     (dotimes (_ 10000)
>       (funcall *baz*)))
>   (thread-join (make-thread (lambda () (igc--collect))))
>   (foo))

Thanks. I'll take a look tomorrow.

Re: MPS make-thread

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ihor Radchenko <yantar92@posteo.net>
>> Cc: Helmut Eller <eller.helmut@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>>  emacs-devel@gnu.org
>> Date: Sat, 22 Jun 2024 18:52:25 +0000
>> 
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>> 
>> >> thix.c:67: Emacs fatal error: assertion failed: SigCheck Thread: thread
>> >> Fatal error 6: Aborted
>> >
>> > Any chance to make this reproducible? Maybe by adding calls to
>> > (igc--collect) in various places? I don't know what you are doing...
>> 
>> (progn
>>   (defvar *baz* nil)
>>   (defun bar (len)
>>     (let ((data (make-list len nil)))
>>       (setq *baz* (lambda () (bar len) data))))
>>   (defun foo ()
>>     (bar 1000)
>>     (dotimes (_ 10000)
>>       (funcall *baz*)))
>>   (thread-join (make-thread (lambda () (igc--collect))))
>>   (foo))
>
> I get a crash only after running this several times.  It looks like
> this:
>
>   ss.c:66: Emacs fatal error: assertion failed: warmest < stackCold

AFAIR, MPS calls the bottom of a control stack "cold". Warmest could
then be the other end of the stack. And that's x86, so the stack grows
down to lower addresses, so that could make sense. Just guessing of course.

>   lockw3.c:98: Emacs fatal error: assertion failed: lock->claims == 0

And the typical followup crash from using MPS when it already has
crashed.

>   #27 StackScan (ss=0x1fc5fb70, stackCold=0x1edeff28, scan_area=0x2c11f9 <scan_ambig>, closure=0x0) at ss.c:66
>   #28 0x003c7494 in RootScan (ss=ss@entry=0x1fc5fb70, root=root@entry=0xa60e83c) at root.c:577
>   #29 0x003c7d1d in traceScanRootRes (ts=ts@entry=1, rank=rank@entry=0, arena=arena@entry=0x7100000, root=root@entry=0xa60e83c) at trace.c:528
>   #30 0x003c8118 in traceScanRoot (root=0xa60e83c, arena=0x7100000, rank=0, ts=1) at trace.c:545

No further clue from the backtrace.

> We must do something about these assertions: when there's an assertion
> violation caused by a thread which was started by MPS, we cannot call
> shut_down_emacs in that thread's context, for obvious reasons.  We
> must instead set some flag which will cause the main thread or one of
> the other Lisp threads call shut_down_emacs.  The MPS documentation
> says:
>
>           Warning: The installed assertion handler must not call any
>           function in MPS, and it must not access memory managed by the
>           MPS.
>
> But our handler, igc_assert_fail, does exactly what they say not to
> do.

I know. See the comment above that function.

One idea might be to set aside a block of memory for use when we know
that MPS is unusable. Then, make alloc_impl allocate from that block,
and probably we must put MPS in postmortem state. Or maybe we can just
use malloc in alloc_impl. 

I think one should try something like that so that Emacs can do its
thing as usual in such cases. Can of course fail, and will certainly be
fiddly. I currently don't have the energy to do that.

> And what does "warmest < stackCold" mean, in human-understandable
> terms, anyway?

At night it's colder than outside :-). Who knows?

Re: MPS make-thread

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Ihor Radchenko <yantar92@posteo.net>
>>> Cc: Helmut Eller <eller.helmut@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>>>  emacs-devel@gnu.org
>>> Date: Sat, 22 Jun 2024 18:52:25 +0000
>>> 
>>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>> 
>>> >> thix.c:67: Emacs fatal error: assertion failed: SigCheck Thread: thread
>>> >> Fatal error 6: Aborted
>>> >
>>> > Any chance to make this reproducible? Maybe by adding calls to
>>> > (igc--collect) in various places? I don't know what you are doing...
>>> 
>>> (progn
>>>   (defvar *baz* nil)
>>>   (defun bar (len)
>>>     (let ((data (make-list len nil)))
>>>       (setq *baz* (lambda () (bar len) data))))
>>>   (defun foo ()
>>>     (bar 1000)
>>>     (dotimes (_ 10000)
>>>       (funcall *baz*)))
>>>   (thread-join (make-thread (lambda () (igc--collect))))
>>>   (foo))
>>
>> I get a crash only after running this several times.  It looks like
>> this:
>>
>>   ss.c:66: Emacs fatal error: assertion failed: warmest < stackCold

Looks like I get something different, with this config:

  #define EMACS_CONFIG_OPTIONS "--enable-checking=all,igc_debug --with-native-compilation=no
   --with-mps=debug CC=clang 'CFLAGS=-g -O0'"

I would say always use --with-mps=debug when reproducing something, and
maybe also CLAGS="-g -O0". If that's possible, of course.

    frame #0: 0x000000010046b758 emacs`mps_lib_assert_fail(file="/Users/gerd/emacs/github/mps/code/thxc.c", line=53, condition="SigCheck Thread: thread") at mpsliban.c:87:3
    frame #1: 0x00000001004ca014 emacs`ThreadCheck(thread=0x0000000108007fc8) at thxc.c:53:3

I think it checks above if the thread is of type Thread? No idea at the
moment how it can't be at that point. Maybe the mps_thr_t has been
destroyed by us? Have to think a bit how to approach this.

    frame #2: 0x00000001004cb38c emacs`ThreadScan(ss=0x000000016fdf9378, thread=0x0000000108007fc8, stackCold=0x0000000170a66fb0, scan_area=(emacs`scan_ambig at igc.c:1082), closure=0x0000000000000000) at thxc.c:236:3
    frame #3: 0x00000001004cad04 emacs`RootScan(ss=0x000000016fdf9378, root=0x0000000108042eb0) at root.c:568:11
    frame #4: 0x000000010054f6cc emacs`traceScanRootRes(ts=1, rank=0, arena=0x0000000100ea4000, root=0x0000000108042eb0) at trace.c:528:9
    frame #5: 0x000000010054f5c8 emacs`traceScanRoot(ts=1, rank=0, arena=0x0000000100ea4000, root=0x0000000108042eb0) at trace.c:545:9
    frame #6: 0x000000010054f558 emacs`rootFlip(root=0x0000000108042eb0, p=0x000000016fdf9578) at trace.c:580:11
    frame #7: 0x00000001004c5dfc emacs`RootsIterate(arena=0x0000000100ea4008, f=(emacs`rootFlip at trace.c:567), p=0x000000016fdf9578) at root.c:665:11
    frame #8: 0x00000001004c6544 emacs`traceFlip(trace=0x0000000100ea4b50) at trace.c:652:11
    frame #9: 0x00000001004c5750 emacs`TraceStart(trace=0x0000000100ea4b50, mortality=0.78513529903807, finishingTime=39377294) at trace.c:1694:10
    frame #10: 0x00000001004a801c emacs`PolicyStartTrace(traceReturn=0x000000016fdf9760, collectWorldReturn=0x000000016fdf9804, arena=0x0000000100ea4000, collectWorldAllowed=1) at policy.c:335:13
    frame #11: 0x00000001004a7258 emacs`TracePoll(workReturn=0x000000016fdf97f0, collectWorldReturn=0x000000016fdf9804, globals=0x0000000100ea4008, collectWorldAllowed=1) at trace.c:1840:10
    frame #12: 0x0000000100474284 emacs`ArenaPoll(globals=0x0000000100ea4008) at global.c:745:16
    frame #13: 0x0000000100476fb8 emacs`mps_ap_fill(p_o=0x000000016fdf9980, mps_ap=0x00000001080016c8, size=24) at mpsi.c:1097:5
    frame #14: 0x000000010039faac emacs`alloc_impl(size=24, type=IGC_OBJ_CONS, ap=0x00000001080016c8) at igc.c:3221:23
    frame #15: 0x000000010039a744 emacs`alloc(size=16, type=IGC_OBJ_CONS) at igc.c:3239:10
    frame #16: 0x000000010039a6d4 emacs`igc_make_cons(car=(struct Lisp_Symbol *) $8 = 0x0000000100ca51f0, cdr=(struct Lisp_Cons *) $9 = 0x000000015e2affe8) at igc.c:3266:28
    frame #17: 0x000000010026c8c8 emacs`Fcons(car=(struct Lisp_Symbol *) $10 = 0x0000000100ca51f0, cdr=(struct Lisp_Cons *) $11 = 0x000000015e2affe8) at alloc.c:2951:10
    frame #18: 0x000000010026cdc8 emacs`Fmake_list(length=(EMACS_INT) $12 = 1000, init=(struct Lisp_Symbol *) $13 = 0x0000000100ca51f0) at alloc.c:3094:13
    frame #19: 0x00000001002b065c emacs`eval_sub(form=(struct Lisp_Cons *) $14 = 0x000000010b011930) at eval.c:2661:15
  * frame #20: 0x00000001002b3a50 emacs`Flet(args=(struct Lisp_Cons *) $15 = 0x000000010b007b70) at eval.c:1131:18
    frame #21: 0x00000001002b0300 emacs`eval_sub(form=(struct Lisp_Cons *) $16 = 0x000000010b007278) at eval.c:2609:8
    frame #22: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Symbol *) $17 = 0x0000000100ca51f0) at eval.c:448:13
    frame #23: 0x00000001002ba124 emacs`funcall_lambda(fun=(struct Lisp_Vector *) $18 = 0x000000010b004878, nargs=1, arg_vector=(struct Lisp_Symbol *) $19 = 0x0000000270a9f290) at eval.c:3417:15
    frame #24: 0x00000001002b81cc emacs`apply_lambda(fun=(struct Lisp_Vector *) $20 = 0x000000010b004878, args=(struct Lisp_Cons *) $21 = 0x000000010b013a78, count=(bytes = 960)) at eval.c:3282:9
    frame #25: 0x00000001002b07cc emacs`eval_sub(form=(struct Lisp_Cons *) $22 = 0x000000010b013500) at eval.c:2705:12
    frame #26: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Cons *) $23 = 0x000000010b013518) at eval.c:448:13
    frame #27: 0x00000001002ba124 emacs`funcall_lambda(fun=(struct Lisp_Vector *) $24 = 0x000000015e2aef70, nargs=0, arg_vector=(struct Lisp_Symbol *) $25 = 0x0000000270a9f758) at eval.c:3417:15
    frame #28: 0x00000001002b9330 emacs`funcall_general(fun=(struct Lisp_Vector *) $26 = 0x000000015e2aef70, numargs=0, args=(struct Lisp_Symbol *) $27 = 0x0000000270a9f758) at eval.c:3111:12
    frame #29: 0x00000001002b1c68 emacs`Ffuncall(nargs=1, args=(struct Lisp_Symbol *) $28 = 0x0000000270a9f750) at eval.c:3160:21
    frame #30: 0x00000001002b04f0 emacs`eval_sub(form=(struct Lisp_Cons *) $29 = 0x000000010b03b078) at eval.c:2630:10
    frame #31: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Symbol *) $30 = 0x0000000100ca51f0) at eval.c:448:13
    frame #32: 0x00000001002b3c4c emacs`Flet(args=(struct Lisp_Cons *) $31 = 0x000000010b03af40) at eval.c:1160:9
    frame #33: 0x00000001002b0300 emacs`eval_sub(form=(struct Lisp_Cons *) $32 = 0x000000010b03aeb0) at eval.c:2609:8
    frame #34: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Cons *) $33 = 0x000000010b03aec8) at eval.c:448:13
    frame #35: 0x00000001002b0df4 emacs`prog_ignore(body=(struct Lisp_Cons *) $34 = 0x000000010b03ae20) at eval.c:459:3
    frame #36: 0x00000001002b3dd4 emacs`Fwhile(args=(struct Lisp_Cons *) $35 = 0x000000010b03ad78) at eval.c:1181:7
    frame #37: 0x00000001002b0300 emacs`eval_sub(form=(struct Lisp_Cons *) $36 = 0x000000010b03ac80) at eval.c:2609:8
    frame #38: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Symbol *) $37 = 0x0000000100ca51f0) at eval.c:448:13
    frame #39: 0x00000001002b3c4c emacs`Flet(args=(struct Lisp_Cons *) $38 = 0x000000010b03aa88) at eval.c:1160:9
    frame #40: 0x00000001002b0300 emacs`eval_sub(form=(struct Lisp_Cons *) $39 = 0x000000010b03a9b0) at eval.c:2609:8
    frame #41: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Symbol *) $40 = 0x0000000100ca51f0) at eval.c:448:13
    frame #42: 0x00000001002ba124 emacs`funcall_lambda(fun=(struct Lisp_Vector *) $41 = 0x0000000104fb6360, nargs=0, arg_vector=(struct Lisp_Symbol *) $42 = 0x0000000270aa0380) at eval.c:3417:15
    frame #43: 0x00000001002b81cc emacs`apply_lambda(fun=(struct Lisp_Vector *) $43 = 0x0000000104fb6360, args=(struct Lisp_Symbol *) $44 = 0x0000000100ca51f0, count=(bytes = 672)) at eval.c:3282:9
    frame #44: 0x00000001002b07cc emacs`eval_sub(form=(struct Lisp_Cons *) $45 = 0x0000000104fb4bf0) at eval.c:2705:12
    frame #45: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Symbol *) $46 = 0x0000000100ca51f0) at eval.c:448:13
    frame #46: 0x00000001002b0300 emacs`eval_sub(form=(struct Lisp_Cons *) $47 = 0x0000000104fb5668) at eval.c:2609:8
    frame #47: 0x00000001002b0cdc emacs`Fprogn(body=(struct Lisp_Symbol *) $48 = 0x0000000100ca51f0) at eval.c:448:13
    frame #48: 0x00000001002b0300 emacs`eval_sub(form=(struct Lisp_Cons *) $49 = 0x0000000104fb6290) at eval.c:2609:8
    frame #49: 0x00000001002b7a9c emacs`Feval(form=(struct Lisp_Cons *) $50 = 0x0000000104fb6290, lexical=(struct Lisp_Symbol *) $51 = 0x0000000100ca5220) at eval.c:2517:28
    frame #50: 0x00000001002b96ac emacs`funcall_subr(subr=0x0000000100c11d28, numargs=2, args=(struct Lisp_Symbol *) $52 = 0x0000000248d9d3e8) at eval.c:3230:15
    frame #51: 0x000000010032f9c0 emacs`exec_byte_code(fun=(struct Lisp_Vector *) $53 = 0x000000010ef1cdb0, args_template=513, nargs=2, args=(struct Lisp_Symbol *) $54 = 0x0000000248d9d8a0) at bytecode.c:827:14
!gud 827:14:/Users/gerd/emacs/github/tinker/src/bytecode.c
    frame #52: 0x00000001002b9b5c emacs`funcall_lambda(fun=(struct Lisp_Vector *) $55 = 0x0000000104fb3d88, nargs=0, arg_vector=(struct Lisp_Symbol *) $56 = 0x0000000270aa1510) at eval.c:3319:9
    frame #53: 0x00000001002b9330 emacs`funcall_general(fun=(struct Lisp_Vector *) $57 = 0x0000000104fb3d88, numargs=0, args=(struct Lisp_Symbol *) $58 = 0x0000000270aa1510) at eval.c:3111:12
    frame #54: 0x00000001002b1c68 emacs`Ffuncall(nargs=1, args=(struct Lisp_Symbol *) $59 = 0x0000000270aa1508) at eval.c:3160:21
    frame #55: 0x00000001002b55a4 emacs`call0(fn=(struct Lisp_Vector *) $60 = 0x0000000104fb3d88) at lisp.h:3557:10
    frame #56: 0x00000001002b5544 emacs`Fhandler_bind_1(nargs=3, args=(struct Lisp_Symbol *) $61 = 0x0000000248d9d328) at eval.c:1529:21
    frame #57: 0x00000001002b989c emacs`funcall_subr(subr=0x0000000100c11c10, numargs=3, args=(struct Lisp_Symbol *) $62 = 0x0000000248d9d328) at eval.c:3251:9
    frame #58: 0x000000010032f9c0 emacs`exec_byte_code(fun=(struct Lisp_Vector *) $63 = 0x000000010fb20980, args_template=128, nargs=1, args=(struct Lisp_Symbol *) $64 = 0x0000000248d9d368) at bytecode.c:827:14
    frame #59: 0x00000001002b9b5c emacs`funcall_lambda(fun=(struct Lisp_Vector *) $65 = 0x0000000111d41a50, nargs=1, arg_vector=(struct Lisp_Symbol *) $66 = 0x0000000270aa21a0) at eval.c:3319:9
    frame #60: 0x00000001002b9330 emacs`funcall_general(fun=(struct Lisp_Vector *) $67 = 0x0000000111d41a50, numargs=1, args=(struct Lisp_Symbol *) $68 = 0x0000000270aa21a0) at eval.c:3111:12
!gud 3111:12:/Users/gerd/emacs/github/tinker/src/eval.c
    frame #61: 0x00000001002b1c68 emacs`Ffuncall(nargs=2, args=(struct Lisp_Symbol *) $69 = 0x0000000270aa2198) at eval.c:3160:21
    frame #62: 0x00000001002aa654 emacs`Ffuncall_interactively(nargs=2, args=(struct Lisp_Symbol *) $70 = 0x0000000270aa2198) at callint.c:250:32
    frame #63: 0x00000001002b989c emacs`funcall_subr(subr=0x0000000100c11510, numargs=2, args=(struct Lisp_Symbol *) $71 = 0x0000000270aa2198) at eval.c:3251:9
    frame #64: 0x00000001002b92e8 emacs`funcall_general(fun=(struct Lisp_Subr *) $72 = 0x0000000100c11510, numargs=2, args=(struct Lisp_Symbol *) $73 = 0x0000000270aa2198) at eval.c:3107:12
    frame #65: 0x00000001002b1c68 emacs`Ffuncall(nargs=3, args=(struct Lisp_Symbol *) $74 = 0x0000000270aa2190) at eval.c:3160:21
    frame #66: 0x00000001002acc84 emacs`Fcall_interactively(function=(struct Lisp_Symbol *) $75 = 0x0000000110a83a70, record_flag=(struct Lisp_Symbol *) $76 = 0x0000000100ca51f0, keys=(struct Lisp_Vector *) $77 = 0x0000000104dd1af0) at callint.c:789:21
    frame #67: 0x00000001002b96d8 emacs`funcall_subr(subr=0x0000000100c114d8, numargs=3, args=(struct Lisp_Symbol *) $78 = 0x0000000248d9d258) at eval.c:3232:15
    frame #68: 0x000000010032f9c0 emacs`exec_byte_code(fun=(struct Lisp_Vector *) $79 = 0x000000010e814708, args_template=1025, nargs=1, args=(struct Lisp_Symbol *) $80 = 0x0000000270aa36b8) at bytecode.c:827:14
    frame #69: 0x00000001002b9b5c emacs`funcall_lambda(fun=(struct Lisp_Vector *) $81 = 0x000000010e814708, nargs=1, arg_vector=(struct Lisp_Symbol *) $82 = 0x0000000270aa36b0) at eval.c:3319:9
    frame #70: 0x00000001002b9330 emacs`funcall_general(fun=(struct Lisp_Vector *) $83 = 0x000000010e814708, numargs=1, args=(struct Lisp_Symbol *) $84 = 0x0000000270aa36b0) at eval.c:3111:12
    frame #71: 0x00000001002b1c68 emacs`Ffuncall(nargs=2, args=(struct Lisp_Symbol *) $85 = 0x0000000270aa36a8) at eval.c:3160:21
    frame #72: 0x00000001001a7830 emacs`command_loop_1 at keyboard.c:1551:13
    frame #73: 0x00000001002b5644 emacs`internal_condition_case(bfun=(emacs`command_loop_1 at keyboard.c:1325), handlers=(struct Lisp_Symbol *) $86 = 0x0000000100ca5280, hfun=(emacs`cmd_error at keyboard.c:971)) at eval.c:1664:25
    frame #74: 0x00000001001a6dc0 emacs`command_loop_2(handlers=(struct Lisp_Symbol *) $87 = 0x0000000100ca5280) at keyboard.c:1169:11
    frame #75: 0x00000001002b47ac emacs`internal_catch(tag=(struct Lisp_Symbol *) $88 = 0x0000000100cb6d10, func=(emacs`command_loop_2 at keyboard.c:1165), arg=(struct Lisp_Symbol *) $89 = 0x0000000100ca5280) at eval.c:1343:25
    frame #76: 0x00000001001a5e08 emacs`command_loop at keyboard.c:1147:2
    frame #77: 0x00000001001a5bf4 emacs`recursive_edit_1 at keyboard.c:755:9
    frame #78: 0x00000001001a6168 emacs`Frecursive_edit at keyboard.c:838:3
    frame #79: 0x00000001001a29d8 emacs`main(argc=1, argv=0x000000016fdfeda8) at emacs.c:2651:3

Re: MPS make-thread

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Ihor Radchenko <yantar92@posteo.net>,  eller.helmut@gmail.com,
>   emacs-devel@gnu.org
> Date: Sun, 23 Jun 2024 05:18:06 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >   ss.c:66: Emacs fatal error: assertion failed: warmest < stackCold
> 
> AFAIR, MPS calls the bottom of a control stack "cold". Warmest could
> then be the other end of the stack. And that's x86, so the stack grows
> down to lower addresses, so that could make sense. Just guessing of course.
> 
> >   lockw3.c:98: Emacs fatal error: assertion failed: lock->claims == 0
> 
> And the typical followup crash from using MPS when it already has
> crashed.
> 
> >   #27 StackScan (ss=0x1fc5fb70, stackCold=0x1edeff28, scan_area=0x2c11f9 <scan_ambig>, closure=0x0) at ss.c:66
> >   #28 0x003c7494 in RootScan (ss=ss@entry=0x1fc5fb70, root=root@entry=0xa60e83c) at root.c:577
> >   #29 0x003c7d1d in traceScanRootRes (ts=ts@entry=1, rank=rank@entry=0, arena=arena@entry=0x7100000, root=root@entry=0xa60e83c) at trace.c:528
> >   #30 0x003c8118 in traceScanRoot (root=0xa60e83c, arena=0x7100000, rank=0, ts=1) at trace.c:545
> 
> No further clue from the backtrace.

Is it safe to call igc--collect from a non-main Lisp thread?  I don't
understand well the semantics and the intent of what igc_collect does,
so I cannot myself try to reason about this.  Maybe what we do there
is not safe when invoked from another Lisp thread?  Do all our threads
share the same arena, for example?  If so, what happens when a Lisp
thread is waiting for the global lock and another Lisp thread calls
igc--collect?

> > We must do something about these assertions: when there's an assertion
> > violation caused by a thread which was started by MPS, we cannot call
> > shut_down_emacs in that thread's context, for obvious reasons.  We
> > must instead set some flag which will cause the main thread or one of
> > the other Lisp threads call shut_down_emacs.  The MPS documentation
> > says:
> >
> >           Warning: The installed assertion handler must not call any
> >           function in MPS, and it must not access memory managed by the
> >           MPS.
> >
> > But our handler, igc_assert_fail, does exactly what they say not to
> > do.
> 
> I know. See the comment above that function.

So how about not calling terminate_due_to_signal from igc_assert_fail?

> One idea might be to set aside a block of memory for use when we know
> that MPS is unusable. Then, make alloc_impl allocate from that block,
> and probably we must put MPS in postmortem state. Or maybe we can just
> use malloc in alloc_impl. 
> 
> I think one should try something like that so that Emacs can do its
> thing as usual in such cases. Can of course fail, and will certainly be
> fiddly. I currently don't have the energy to do that.

One idea would be to exit the non-main Lisp thread (because I think
shutting down Emacs from there is not safe anyway), then shut down
from the main thread.

Re: MPS make-thread

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Ihor Radchenko <yantar92@posteo.net>,  eller.helmut@gmail.com,
>   emacs-devel@gnu.org
> Date: Sun, 23 Jun 2024 06:54:09 +0200
> 
> >> I get a crash only after running this several times.  It looks like
> >> this:
> >>
> >>   ss.c:66: Emacs fatal error: assertion failed: warmest < stackCold
> 
> Looks like I get something different, with this config:
> 
>   #define EMACS_CONFIG_OPTIONS "--enable-checking=all,igc_debug --with-native-compilation=no
>    --with-mps=debug CC=clang 'CFLAGS=-g -O0'"
> 
> I would say always use --with-mps=debug when reproducing something, and
> maybe also CLAGS="-g -O0". If that's possible, of course.

I know.  I deliberately use the production build because if we all use
the debug build, problems that appear only in production will never
happen to any of us.  In particular, some problems only happen in an
optimized build, so I build with -O2.

Re: MPS make-thread

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> Is it safe to call igc--collect from a non-main Lisp thread?  I don't
> understand well the semantics and the intent of what igc_collect does,
> so I cannot myself try to reason about this.  Maybe what we do there
> is not safe when invoked from another Lisp thread?  Do all our threads
> share the same arena, for example?  If so, what happens when a Lisp
> thread is waiting for the global lock and another Lisp thread calls
> igc--collect?

igc_collect calls mps_arena_collect which acquires the MPS lock, does a
full collection, and leaves the arena in parked state, so that the
caller has to mps_release it itself, which we do in igc_collect. This is
explicitly only intended for debugging purposes. It is thread-safe.
>
>> > We must do something about these assertions: when there's an assertion
>> > violation caused by a thread which was started by MPS, we cannot call
>> > shut_down_emacs in that thread's context, for obvious reasons.  We
>> > must instead set some flag which will cause the main thread or one of
>> > the other Lisp threads call shut_down_emacs.  The MPS documentation
>> > says:
>> >
>> >           Warning: The installed assertion handler must not call any
>> >           function in MPS, and it must not access memory managed by the
>> >           MPS.
>> >
>> > But our handler, igc_assert_fail, does exactly what they say not to
>> > do.
>> 
>> I know. See the comment above that function.
>
> So how about not calling terminate_due_to_signal from igc_assert_fail?

I think igc_assert uses that too, currently, which can fire in various
ciscumstances, among them "harmless" ones like for eassert. (Side note:
I didn't use eassert because I wanted finer control over assertions in
igc.c, for example activating them in igc.c but not the rest of Emacs
because of the general slowdown which made using Emacs painful.)

In the "harmless" case, it would be nice if Emacs could take some
emergency measures, as if it were eassert. Otherwise I was afraid I'd
loose my work :-).

I also don't find the current behaviour too bad if one knows that
further assertion might be triggered if something happens in MPS. If one
knows that will happen, of course, hence the comment.

And it's all temporary. Hopefully a better solution will be implemented
at some point. Maybe one as I described.
>
>> One idea might be to set aside a block of memory for use when we know
>> that MPS is unusable. Then, make alloc_impl allocate from that block,
>> and probably we must put MPS in postmortem state. Or maybe we can just
>> use malloc in alloc_impl. 
>> 
>> I think one should try something like that so that Emacs can do its
>> thing as usual in such cases. Can of course fail, and will certainly be
>> fiddly. I currently don't have the energy to do that.
>
> One idea would be to exit the non-main Lisp thread (because I think
> shutting down Emacs from there is not safe anyway), then shut down
> from the main thread.

Yes, maybe. I guess one has to try and see what works best.

Re: MPS make-thread

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: yantar92@posteo.net,  eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Sun, 23 Jun 2024 08:45:44 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Is it safe to call igc--collect from a non-main Lisp thread?  I don't
> > understand well the semantics and the intent of what igc_collect does,
> > so I cannot myself try to reason about this.  Maybe what we do there
> > is not safe when invoked from another Lisp thread?  Do all our threads
> > share the same arena, for example?  If so, what happens when a Lisp
> > thread is waiting for the global lock and another Lisp thread calls
> > igc--collect?
> 
> igc_collect calls mps_arena_collect which acquires the MPS lock, does a
> full collection, and leaves the arena in parked state, so that the
> caller has to mps_release it itself, which we do in igc_collect. This is
> explicitly only intended for debugging purposes. It is thread-safe.

Then why did we get the MPS assertion about warm and cold in this case?

Re: MPS make-thread

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> igc_collect calls mps_arena_collect which acquires the MPS lock, does a
>> full collection, and leaves the arena in parked state, so that the
>> caller has to mps_release it itself, which we do in igc_collect. This is
>> explicitly only intended for debugging purposes. It is thread-safe.
>
> Then why did we get the MPS assertion about warm and cold in this case?

I don't know, but I think the real error happens before that. The
backtrace from my debug version shows that something is wrong with the
mps_thr_t that it is using.

Re: MPS make-thread

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> igc_collect calls mps_arena_collect which acquires the MPS lock, does a
>>> full collection, and leaves the arena in parked state, so that the
>>> caller has to mps_release it itself, which we do in igc_collect. This is
>>> explicitly only intended for debugging purposes. It is thread-safe.
>>
>> Then why did we get the MPS assertion about warm and cold in this case?
>
> I don't know, but I think the real error happens before that. The
> backtrace from my debug version shows that something is wrong with the
> mps_thr_t that it is using.

FYI, I think I have it. Pushing later.

Re: MPS make-thread

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> igc_collect calls mps_arena_collect which acquires the MPS lock, does a
>>>> full collection, and leaves the arena in parked state, so that the
>>>> caller has to mps_release it itself, which we do in igc_collect. This is
>>>> explicitly only intended for debugging purposes. It is thread-safe.
>>>
>>> Then why did we get the MPS assertion about warm and cold in this case?
>>
>> I don't know, but I think the real error happens before that. The
>> backtrace from my debug version shows that something is wrong with the
>> mps_thr_t that it is using.
>
> FYI, I think I have it. Pushing later.

That went better than I expected... Pushed, please let me know if it
works for you.

Re: MPS make-thread

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: yantar92@posteo.net,  eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Sun, 23 Jun 2024 12:27:42 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> >
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >>
> >>>> igc_collect calls mps_arena_collect which acquires the MPS lock, does a
> >>>> full collection, and leaves the arena in parked state, so that the
> >>>> caller has to mps_release it itself, which we do in igc_collect. This is
> >>>> explicitly only intended for debugging purposes. It is thread-safe.
> >>>
> >>> Then why did we get the MPS assertion about warm and cold in this case?
> >>
> >> I don't know, but I think the real error happens before that. The
> >> backtrace from my debug version shows that something is wrong with the
> >> mps_thr_t that it is using.
> >
> > FYI, I think I have it. Pushing later.
> 
> That went better than I expected... Pushed, please let me know if it
> works for you.

Thanks, seems to work fine now.

Re: MPS make-thread

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> That went better than I expected... Pushed, please let me know if it
>> works for you.
>
> Thanks, seems to work fine now.

👍 Thanks

MPS native subrs (was: MPS make-thread)

[Helmut Eller via Emacs development discussions.]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

On Sat, Jun 22 2024, Helmut Eller wrote:
>>> I thought native comp works for you?
>
> I haven't tried with the latest dump loading code.  I think that needs
> some work for native-comp-units.

Here are some patches for this.  I also have some questions:

1) In fix_subr, the &s->intspec.native field should only traced if the
   subr is a non-primitive.  Because, for primitives, it's a (possibly
   non-aligned) char*.  Right?

2) In dump_subr, the command_modes field is dumped with dump_field_lv
   for non-primitives but for primitives with dump_field_emacs_ptr.
   Is this intentional or a bug?

3) In fix_subr, why is the command_modes field only traced if
   HAVE_NATIVE_COMP?  (Today was the first time I heard of the
   command-modes function; still have no clue what it's used for :-)



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-some-checks-when-dumping-and-when-loading-the-du.patch --]
[-- Type: text/x-diff, Size: 4615 bytes --]

From 64f552d11cb9660d153647ba713276dd5e07ec77 Mon Sep 17 00:00:00 2001
From: Helmut Eller <helmut@249.130.205.92.host.secureserver.net>
Date: Tue, 25 Jun 2024 16:44:35 +0200
Subject: [PATCH 1/3] Add some checks when dumping and when loading the dump

* src/igc.h (igc_dump_check_object_starts): New.
* src/igc.c (igc_dump_check_object_starts): Implementation.
(check_dump): New.
(igc_on_pdump_loaded): Use it.
* src/pdumper.c (dump_igc_check_object_starts): New.
(Fdump_emacs_portable): Use it.
---
 src/igc.c     | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/igc.h     |  3 +++
 src/pdumper.c | 16 +++++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/src/igc.c b/src/igc.c
index 4c9860c35d9..49abc136a0c 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -4093,6 +4093,62 @@ igc_dump_finish_obj (void *client, enum igc_obj_type type,
   return base + nbytes;
 }
 
+void
+igc_dump_check_object_starts (Lisp_Object relocs, void *dump_base,
+			      void *hot_start, void *hot_end,
+			      void *cold_start, void *heap_end)
+{
+  eassert (is_aligned (dump_base));
+  eassert (is_aligned (hot_start));
+  eassert (is_aligned (hot_end));
+  eassert (is_aligned (cold_start));
+  eassert (is_aligned (hot_end));
+  struct region
+  {
+    mps_addr_t start, end;
+  } regions[] = {
+    {hot_start, hot_end},
+    {cold_start, heap_end},
+  };
+  for (size_t i = 0; i < ARRAYELTS (regions); i++)
+    {
+      struct region region = regions[i];
+      mps_addr_t p = region.start;
+      while (p != region.end)
+	{
+	  eassert (p < region.end);
+	  Lisp_Object r = XCAR (relocs);
+	  relocs = XCDR (relocs);
+	  EMACS_INT start_off = XFIXNUM (XCAR (r));
+	  EMACS_INT end_off = XFIXNUM (XCAR (XCDR (r)));
+	  mps_addr_t start = (uint8_t *) dump_base + start_off;
+	  mps_addr_t end = (uint8_t *) dump_base + end_off;
+	  eassert (start == p);
+	  p = dflt_skip (p);
+	  eassert (end == p);
+	}
+    }
+  eassert (NILP (relocs));
+}
+
+static bool
+check_dump (mps_addr_t start, mps_addr_t end)
+{
+  struct pdumper_object_it it = { 0 };
+  for (mps_addr_t p = start; p != end; p = dflt_skip (p))
+    {
+      eassert (p < end);
+      struct igc_header *h = p;
+      if (h->obj_type != IGC_OBJ_PAD)
+	{
+	  mps_addr_t obj = pdumper_next_object (&it);
+	  eassert (p == obj);
+	}
+    }
+  eassert (pdumper_next_object (&it) == NULL);
+  return true;
+}
+
 static mps_addr_t pinned_objects_in_dump[3];
 
 /* Called from pdumper_load. [START, END) is the hot section of the
@@ -4129,6 +4185,7 @@ igc_on_pdump_loaded (void *dump_base, void *hot_start, void *hot_end,
   /* Ignore relocs */
   set_header (heap_end, IGC_OBJ_PAD, relocs_size, 0);
 
+  eassert (check_dump (h, cold_end));
   /* Pin some stuff in the dump  */
   mps_addr_t pinned_roots[] = {
     charset_table,
diff --git a/src/igc.h b/src/igc.h
index 8beb4fd2dce..5e59a706af7 100644
--- a/src/igc.h
+++ b/src/igc.h
@@ -153,6 +153,9 @@ #define EMACS_IGC_H
 size_t igc_header_size (void);
 char *igc_dump_finish_obj (void *client, enum igc_obj_type type,
 			   char *base, char *end);
+void igc_dump_check_object_starts (Lisp_Object relocs, void *dump_base,
+				   void *hot_start, void *hot_end,
+				   void *cold_start, void *heap_end);
 void *igc_alloc_dump (size_t nbytes);
 bool igc_busy_p (void);
 Lisp_Object igc_discard_killed_buffers (Lisp_Object list);
diff --git a/src/pdumper.c b/src/pdumper.c
index e0ff08c9d3d..a9d67fe203a 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -882,6 +882,21 @@ dump_align_output (struct dump_context *ctx, int alignment)
 }
 
 # ifdef HAVE_MPS
+static void
+dump_igc_check_object_starts (struct dump_context *ctx)
+{
+  Lisp_Object relocs = CALLN (Fsort, Freverse (ctx->igc_object_starts),
+			      Qdump_emacs_portable__sort_predicate);
+  void *dump_base = (uint8_t *) ctx->buf;
+  size_t dump_header_size = ROUNDUP (sizeof ctx->header, sizeof (uintptr_t));
+  void *hot_start = (uint8_t *) dump_base + dump_header_size;
+  void *hot_end = (uint8_t *) dump_base + ctx->header.discardable_start;
+  void *cold_start = (uint8_t *) dump_base + ctx->header.cold_start;
+  void *heap_end = (uint8_t *) dump_base + ctx->header.heap_end;
+  igc_dump_check_object_starts (relocs, dump_base,
+				hot_start, hot_end, cold_start, heap_end);
+}
+
 static void
 dump_igc_start_obj (struct dump_context *ctx, enum igc_obj_type type,
 		    const void *in)
@@ -4471,6 +4486,7 @@ DEFUN ("dump-emacs-portable",
 
 # ifdef HAVE_MPS
   ctx->header.heap_end = ctx->offset;
+  dump_igc_check_object_starts (ctx);
   dump_igc_start_obj (ctx, IGC_OBJ_DUMPED_BYTES, &discardable_end);
 # endif
 
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Support-dumping-native-subrs.patch --]
[-- Type: text/x-diff, Size: 2209 bytes --]

From dd05fd62893d87c744c4b2b58d75812110bba19b Mon Sep 17 00:00:00 2001
From: Helmut Eller <helmut@249.130.205.92.host.secureserver.net>
Date: Tue, 25 Jun 2024 19:47:46 +0200
Subject: [PATCH 2/3] Support dumping native subrs

* src/pdumber.c (dump_cold_native_subr): Emit headers.
(dump_do_dump_relocation): Copy the C strings out of the dump.
(dump_igc_start_obj): Check alignment.
---
 src/pdumper.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/pdumper.c b/src/pdumper.c
index a9d67fe203a..667f75ef6bf 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -903,6 +903,7 @@ dump_igc_start_obj (struct dump_context *ctx, enum igc_obj_type type,
 {
   eassert (ctx->igc_type == IGC_OBJ_INVALID);
   eassert (ctx->igc_obj_dumped == NULL);
+  eassert (ctx->offset % igc_header_size () == 0);
   ctx->igc_obj_dumped = (void *) in;
   ctx->igc_type = type;
   ctx->igc_base_offset = ctx->offset;
@@ -3620,6 +3621,10 @@ dump_cold_native_subr (struct dump_context *ctx, Lisp_Object subr)
   /* Dump subr contents.  */
   dump_off subr_offset = dump_recall_object (ctx, subr);
   eassert (subr_offset > 0);
+# ifdef HAVE_MPS
+  /* FIXME: more descriptive name? but igc_obj_type has no more free bits */
+  dump_igc_start_obj (ctx, IGC_OBJ_DUMPED_BYTES, (void *)~0);
+# endif
   dump_remember_fixup_ptr_raw
     (ctx,
      subr_offset + dump_offsetof (struct Lisp_Subr, symbol_name),
@@ -3633,6 +3638,9 @@ dump_cold_native_subr (struct dump_context *ctx, Lisp_Object subr)
      ctx->offset);
   const char *c_name = XSUBR (subr)->native_c_name;
   dump_write (ctx, c_name, 1 + strlen (c_name));
+# ifdef HAVE_MPS
+  dump_igc_finish_obj (ctx);
+# endif
 }
 #endif
 
@@ -5738,7 +5746,13 @@ dump_do_dump_relocation (const uintptr_t dump_base,
 	  XNATIVE_COMP_UNIT (subr->native_comp_u);
 	if (!comp_u->handle)
 	  error ("NULL handle in compilation unit %s", SSDATA (comp_u->file));
+#ifdef HAVE_MPS
+	/* FIXME: needs finalization? */
+	subr->symbol_name = xstrdup (subr->symbol_name);
+	const char *c_name = xstrdup (subr->native_c_name);
+#else
 	const char *c_name = subr->native_c_name;
+#endif
 	eassert (c_name);
 	void *func = dynlib_sym (comp_u->handle, c_name);
 	if (!func)
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Only-GUI-versions-have-images-caches.patch --]
[-- Type: text/x-diff, Size: 965 bytes --]

From 8fd45d8daa5bd0587f2b6518d6ba556a33ec4212 Mon Sep 17 00:00:00 2001
From: Helmut Eller <helmut@249.130.205.92.host.secureserver.net>
Date: Tue, 25 Jun 2024 20:20:38 +0200
Subject: [PATCH 3/3] Only GUI versions have images caches

* src/igc.c (fix_frame) [HAVE_WINDOW_SYSTEM]: Avoid compilation errors
in non-GUI configurations.
---
 src/igc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/igc.c b/src/igc.c
index 49abc136a0c..f2f8a02927f 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -1714,10 +1714,10 @@ fix_frame (mps_ss_t ss, struct frame *f)
     IGC_FIX12_RAW (ss, &f->face_cache);
     if (f->terminal)
       IGC_FIX12_RAW (ss, &f->terminal);
-    if (f->image_cache)
-      IGC_FIX12_RAW (ss, &f->image_cache);
 
 #ifdef HAVE_WINDOW_SYSTEM
+    if (f->image_cache)
+      IGC_FIX12_RAW (ss, &f->image_cache);
     if (FRAME_WINDOW_P (f) && FRAME_OUTPUT_DATA (f))
       {
 	struct font **font_ptr = &FRAME_FONT (f);
-- 
2.39.2

Re: MPS native subrs

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Sat, Jun 22 2024, Helmut Eller wrote:
>>>> I thought native comp works for you?
>>
>> I haven't tried with the latest dump loading code.  I think that needs
>> some work for native-comp-units.
>
> Here are some patches for this.  

Thanks. Looks like gnu.org is down, for some time now. I'll push your
patches when it works again.

> I also have some questions:

Oh oh :-)

> 1) In fix_subr, the &s->intspec.native field should only traced if the
>    subr is a non-primitive.  Because, for primitives, it's a (possibly
>    non-aligned) char*.  Right?

Right. I thought I had copied that literally from process_mark_stack,
but apparently not. In process_mark_stack it reads

		if (NATIVE_COMP_FUNCTIONP (obj))
		  {
		    set_vector_marked (ptr);
		    struct Lisp_Subr *subr = XSUBR (obj);
		    mark_stack_push_value (subr->intspec.native);
		    mark_stack_push_value (subr->command_modes);
		    mark_stack_push_value (subr->native_comp_u);
		    mark_stack_push_value (subr->lambda_list);
		    mark_stack_push_value (subr->type);
		  }

And command_modes is even fixed twice in fix_subr. There went
something seriously wrong :-/.

> 2) In dump_subr, the command_modes field is dumped with dump_field_lv
>    for non-primitives but for primitives with dump_field_emacs_ptr.
>    Is this intentional or a bug?

Hm, strange. I also can't make sense of that. For meLisp_Object is
Lisp_Object and not a pointer.

>
> 3) In fix_subr, why is the command_modes field only traced if
>    HAVE_NATIVE_COMP?  (Today was the first time I heard of the
>    command-modes function; still have no clue what it's used for :-)

Ja, see 1). Looks like a blackout to me. No idea. Maybe that's from the
phase after I had first tried to mix the two GCs, so that I could make
incremental progress, object type by object, Then I gave up on that
because it didn't work well, and finally had to get "everything" done
with a big bang. That was so incredibly mechanical work that possibly my
brain wandered off intermittendly :-/.

How command-modes, a defun, is exactly used I don't know either, sorry.
I understand the doc string as indicating that certain commands are only
applicable in certain modes, in the sense of minor- and major-mode. I
don't remember that from the good old days, and it doesn't seem to be
explained anywhere.

Re: MPS native subrs

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> How command-modes, a defun, is exactly used I don't know either, sorry.
> I understand the doc string as indicating that certain commands are only
> applicable in certain modes, in the sense of minor- and major-mode. I
> don't remember that from the good old days, and it doesn't seem to be
> explained anywhere.

Seems to have appeared in

58e0c8ee86e2c36245f1c5a1483f1c73600b4914
Author:     Lars Ingebrigtsen <larsi@gnus.org>
AuthorDate: Sun Feb 14 13:21:24 2021 +0100

Extend the syntax of `interactive' to list applicable modes

Re: MPS native subrs

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Thanks. Looks like gnu.org is down, for some time now. I'll push your
> patches when it works again.

Done.

Re: MPS native subrs

[Helmut Eller]

On Tue, Jun 25 2024, Gerd Möllmann wrote:

> Seems to have appeared in
>
> 58e0c8ee86e2c36245f1c5a1483f1c73600b4914
> Author:     Lars Ingebrigtsen <larsi@gnus.org>
> AuthorDate: Sun Feb 14 13:21:24 2021 +0100
>
> Extend the syntax of `interactive' to list applicable modes

But it's always nil for primitive subrs.  To me, this feature seems so
rarely used that it would be more than good enough if only named
commands can have command-modes.  For named commands, the symbols plist
seems like the better place to put it than the function itself.  If we
could get rid of the command_modes field, then we wouldn't need to trace
primitives at all.

More generally, it seems that the DEFUN macro works much like the DEFVAR
macro, in the sense that it creates a struct and puts it in a static
variable.  So the Lisp_Subrs structs for primitives are, just like the
Lisp_Fwd structs, already in the data section.  We could re-use them
instead of re-creating them in the dump.  Of course, only if we can get
rid of the command_modes field.

Re: MPS native subrs

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Tue, Jun 25 2024, Gerd Möllmann wrote:
>
>> Seems to have appeared in
>>
>> 58e0c8ee86e2c36245f1c5a1483f1c73600b4914
>> Author:     Lars Ingebrigtsen <larsi@gnus.org>
>> AuthorDate: Sun Feb 14 13:21:24 2021 +0100
>>
>> Extend the syntax of `interactive' to list applicable modes
>
> But it's always nil for primitive subrs.  

Right. I've added Lars in CC, maybe he can say something. I personally
don't remember anything about this being discussed or something.

> To me, this feature seems so rarely used that it would be more than
> good enough if only named commands can have command-modes. For named
> commands, the symbols plist seems like the better place to put it than
> the function itself. If we could get rid of the command_modes field,
> then we wouldn't need to trace primitives at all

Agree 100%.

> More generally, it seems that the DEFUN macro works much like the DEFVAR
> macro, in the sense that it creates a struct and puts it in a static
> variable.  So the Lisp_Subrs structs for primitives are, just like the
> Lisp_Fwd structs, already in the data section.  We could re-use them
> instead of re-creating them in the dump.  Of course, only if we can get
> rid of the command_modes field.

And it would again greatly simplify things. Maybe we should just do it :-)

Re: MPS make-thread

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> > FYI, I think I have it. Pushing later.
>> 
>> That went better than I expected... Pushed, please let me know if it
>> works for you.
>
> Thanks, seems to work fine now.

Also no crashes with my full config running the code that uses
make-thread and that crashed earlier.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: MPS make-thread

[Gerd Möllmann]

Thanks!

Sent from my iPhone

> On 26. Jun 2024, at 13:18, Ihor Radchenko <yantar92@posteo.net> wrote:
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>>>> FYI, I think I have it. Pushing later.
>>> 
>>> That went better than I expected... Pushed, please let me know if it
>>> works for you.
>> 
>> Thanks, seems to work fine now.
> 
> Also no crashes with my full config running the code that uses
> make-thread and that crashed earlier.
> 
> --
> Ihor Radchenko // yantar92,
> Org mode contributor,
> 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>

Re: MPS native subrs (was: MPS make-thread)

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Tue, 25 Jun 2024 20:42:44 +0200
> 
> Here are some patches for this.  I also have some questions:
> 
> 1) In fix_subr, the &s->intspec.native field should only traced if the
>    subr is a non-primitive.  Because, for primitives, it's a (possibly
>    non-aligned) char*.  Right?
> 
> 2) In dump_subr, the command_modes field is dumped with dump_field_lv
>    for non-primitives but for primitives with dump_field_emacs_ptr.
>    Is this intentional or a bug?
> 
> 3) In fix_subr, why is the command_modes field only traced if
>    HAVE_NATIVE_COMP?  (Today was the first time I heard of the
>    command-modes function; still have no clue what it's used for :-)

Andrea, can you help with these questions?

Re: MPS native subrs

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Tue, 25 Jun 2024 22:48:01 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > How command-modes, a defun, is exactly used I don't know either, sorry.
> > I understand the doc string as indicating that certain commands are only
> > applicable in certain modes, in the sense of minor- and major-mode. I
> > don't remember that from the good old days, and it doesn't seem to be
> > explained anywhere.
> 
> Seems to have appeared in
> 
> 58e0c8ee86e2c36245f1c5a1483f1c73600b4914
> Author:     Lars Ingebrigtsen <larsi@gnus.org>
> AuthorDate: Sun Feb 14 13:21:24 2021 +0100
> 
> Extend the syntax of `interactive' to list applicable modes

It's documented in the ELisp manual:


 -- Special Form: interactive &optional arg-descriptor &rest modes
 [...]
     The MODES list allows specifying which modes the command is meant
     to be used in.  See *note Command Modes:: for more details about
     the effect of specifying MODES, and when to use it.

Feature which uses it is also documented in NEWS.29:

  ** New key binding after 'M-x' or 'M-X': 'M-X'.
  Emacs allows different completion predicates to be used with 'M-x'
  (i.e., 'execute-extended-command') via the
  'read-extended-command-predicate' user option.  Emacs also has the
  'M-X' (note upper case X) command, which only displays commands
  especially relevant to the current buffer.  Emacs now allows toggling
  between these modes while the user is inputting a command by hitting
  'M-X' while in the minibuffer.

Re: MPS native subrs

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Tue, 25 Jun 2024 22:48:01 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > How command-modes, a defun, is exactly used I don't know either, sorry.
> > I understand the doc string as indicating that certain commands are only
> > applicable in certain modes, in the sense of minor- and major-mode. I
> > don't remember that from the good old days, and it doesn't seem to be
> > explained anywhere.
> 
> Seems to have appeared in
> 
> 58e0c8ee86e2c36245f1c5a1483f1c73600b4914
> Author:     Lars Ingebrigtsen <larsi@gnus.org>
> AuthorDate: Sun Feb 14 13:21:24 2021 +0100
> 
> Extend the syntax of `interactive' to list applicable modes

It's documented in the ELisp manual:

 -- Special Form: interactive &optional arg-descriptor &rest modes
 [...]
     The MODES list allows specifying which modes the command is meant
     to be used in.  See *note Command Modes:: for more details about
     the effect of specifying MODES, and when to use it.

 -- Macro: declare specs...
 [...]
     ‘(modes MODES)’
          Specify that this command is meant to be applicable only to
          specified MODES.  *Note Command Modes::.

See also "Command Modes" in the ELisp manual, which is dedicated to
these features.

Re: MPS native subrs

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Tue, 25 Jun 2024 22:10:37 +0200
> 
> How command-modes, a defun, is exactly used I don't know either, sorry.
> I understand the doc string as indicating that certain commands are only
> applicable in certain modes, in the sense of minor- and major-mode. I
> don't remember that from the good old days, and it doesn't seem to be
> explained anywhere.

It's explained in its doc string, and you can grep the sources for its
uses.

Re: MPS native subrs

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org, larsi@gnus.org
> Date: Wed, 26 Jun 2024 09:00:14 +0200
> 
> Helmut Eller <eller.helmut@gmail.com> writes:
> 
> > On Tue, Jun 25 2024, Gerd Möllmann wrote:
> >
> >> Seems to have appeared in
> >>
> >> 58e0c8ee86e2c36245f1c5a1483f1c73600b4914
> >> Author:     Lars Ingebrigtsen <larsi@gnus.org>
> >> AuthorDate: Sun Feb 14 13:21:24 2021 +0100
> >>
> >> Extend the syntax of `interactive' to list applicable modes
> >
> > But it's always nil for primitive subrs.  
> 
> Right. I've added Lars in CC, maybe he can say something. I personally
> don't remember anything about this being discussed or something.

This was discussed at length in this thread:

  https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00665.html

and several other related long discussions in Feb 2021.

Re: MPS native subrs

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> This was discussed at length in this thread:
>
>   https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg00665.html
>
> and several other related long discussions in Feb 2021.

Thanks.

Kind of tldr, but I understand that putting something on a symbol's
plist would not suffice because of things like

  (lambda () (interactive ...) ...)

for example.

Re: MPS native subrs

[Andrea Corallo]

Helmut Eller via "Emacs development discussions." <emacs-devel@gnu.org>
writes:

> On Sat, Jun 22 2024, Helmut Eller wrote:
>>>> I thought native comp works for you?
>>
>> I haven't tried with the latest dump loading code.  I think that needs
>> some work for native-comp-units.
>
> Here are some patches for this.  I also have some questions:
>
> 1) In fix_subr, the &s->intspec.native field should only traced if the
>    subr is a non-primitive.  Because, for primitives, it's a (possibly
>    non-aligned) char*.  Right?

I believe that's correct.

> 2) In dump_subr, the command_modes field is dumped with dump_field_lv
>    for non-primitives but for primitives with dump_field_emacs_ptr.
>    Is this intentional or a bug?

Right, I'm not the author of the change but I suspect as well it should
be 'dump_field_lv'.

> 3) In fix_subr, why is the command_modes field only traced if
>    HAVE_NATIVE_COMP?  (Today was the first time I heard of the
>    command-modes function; still have no clue what it's used for :-)

I think Gerd already replied.

  Andrea

Re: MPS native subrs

[Helmut Eller]

On Wed, Jun 26 2024, Gerd Möllmann wrote:

>> More generally, it seems that the DEFUN macro works much like the DEFVAR
>> macro, in the sense that it creates a struct and puts it in a static
>> variable.  So the Lisp_Subrs structs for primitives are, just like the
>> Lisp_Fwd structs, already in the data section.  We could re-use them
>> instead of re-creating them in the dump.  Of course, only if we can get
>> rid of the command_modes field.
>
> And it would again greatly simplify things. Maybe we should just do it :-)

Curiously, igc-info doesn't show any PVEC_SUBRS.  I think this is going on:

The pdumper does something special for primitives and builtin symbols
(and threads).  Those are dumped to the discardable section.  On load,
it first performs relocations and then copies everything from the
discardable section to the data section.  All builtin symbols can be
copied with a single memcpy to lispsym because they are sorted and
dumped in the correct order.  The same happens for primitives: those are
adjacent because the DEFUN macro sets the SUBR_SECTION_ATTRIBUTE.

This explains why igc-info doesn't show any PVEC_SUBRS (unless
configured with native compilation).  So we already have what we want.
It's probably no problem that we don't trace the subr section, because
the command_modes field is nil anyway.

I'm not sure why it is done this way; maybe it makes walking the object
graph more uniform.

Re: MPS native subrs

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Wed, Jun 26 2024, Gerd Möllmann wrote:
>
>>> More generally, it seems that the DEFUN macro works much like the DEFVAR
>>> macro, in the sense that it creates a struct and puts it in a static
>>> variable.  So the Lisp_Subrs structs for primitives are, just like the
>>> Lisp_Fwd structs, already in the data section.  We could re-use them
>>> instead of re-creating them in the dump.  Of course, only if we can get
>>> rid of the command_modes field.
>>
>> And it would again greatly simplify things. Maybe we should just do it :-)
>
> Curiously, igc-info doesn't show any PVEC_SUBRS.  I think this is going on:
>
> The pdumper does something special for primitives and builtin symbols
> (and threads).  Those are dumped to the discardable section.  On load,
> it first performs relocations and then copies everything from the
> discardable section to the data section.  All builtin symbols can be
> copied with a single memcpy to lispsym because they are sorted and
> dumped in the correct order.  The same happens for primitives: those are
> adjacent because the DEFUN macro sets the SUBR_SECTION_ATTRIBUTE.
>
> This explains why igc-info doesn't show any PVEC_SUBRS (unless
> configured with native compilation).  So we already have what we want.
> It's probably no problem that we don't trace the subr section, because
> the command_modes field is nil anyway.

Interesting, thanks!

> I'm not sure why it is done this way; maybe it makes walking the object
> graph more uniform.

Hm.