bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

In GNU Emacs 29.0.50 (build 1, aarch64-apple-darwin21.6.0, NS
 appkit-2113.60 Version 12.6 (Build 21G115)) of 2022-09-21 built on
 Mini.fritz.box
Repository revision: 1231a601ebe1fd9fe454c504dbeb9267440242e7
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2113
System Description:  macOS 12.6

Configured using:
 'configure --cache-file /Users/gerd/tmp/config.cache.master
 --with-native-compilation'
 
Configured features:
ACL DBUS GLIB GNUTLS JSON LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY
KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS XIM ZLIB

I got the following ASAN error today.  Unfortunately, I don't have the
slightest idea how to reproduce this.

==79227==ERROR: AddressSanitizer: heap-use-after-free on address 0x00011f81e7d1 at pc 0x0001005825c4 bp 0x00016fdcf370 sp 0x00016fdcf368
READ of size 1 at 0x00011f81e7d1 thread T0
    #0 0x1005825c0 in re_match_2_internal regex-emacs.c:4352
    #1 0x10057e5cc in rpl_re_search_2 regex-emacs.c:3383
    #2 0x10057d1c4 in rpl_re_search regex-emacs.c:3177
    #3 0x10056115c in fast_string_match_internal search.c:492
    #4 0x1005045c0 in fast_string_match lisp.h:4818
    #5 0x100504018 in Ffind_file_name_handler fileio.c:324
    #6 0x1006dbe5c in openp lread.c:1911
    #7 0x1006d8844 in Fload lread.c:1302
    #8 0x1006e1af0 in save_match_data_load lread.c:1630
    #9 0x10064f8cc in load_with_autoload_queue eval.c:2269
    #10 0x10067d2f8 in Frequire fns.c:3274

previously allocated by thread T0 here:
    #0 0x103332ca8 in wrap_malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3eca8)
    #1 0x1005ae8fc in lmalloc alloc.c:1361
    #2 0x1005b0188 in lisp_malloc alloc.c:994
    #3 0x1005b0a5c in allocate_string_data alloc.c:1889
    #4 0x1005b1bd8 in make_clear_multibyte_string alloc.c:2475
    #5 0x1005b1670 in make_clear_string alloc.c:2443
    #6 0x1005b2714 in make_uninit_string alloc.c:2454
    #7 0x100666c14 in concat_to_string fns.c:821
    #8 0x100666420 in concat2 fns.c:600
    #9 0x1006d7870 in Fget_load_suffixes lread.c:1123
    #10 0x1006d86ac in Fload lread.c:1296
    #11 0x1006e1af0 in save_match_data_load lread.c:1630
    #12 0x10064f8cc in load_with_autoload_queue eval.c:2269

rame #5: 0x00000001005825c4 emacs`re_match_2_internal(bufp=0x000000010111b890, string1=0x0000000000000000, size1=0, string2="/Users/gerd/.config/emacs.d.default/elpa/company-0.9.13/lsp-protocol.el.gz", size2=74, pos=0, regs=0x0000000000000000, stop=74) at regex-emacs.c:4352:18
   4349	
   4350		    PREFETCH ();
   4351		    int len;
-> 4352		    int corig = RE_STRING_CHAR_AND_LENGTH (d, len, target_multibyte);
   4353		    int c = corig;
   4354		    if (target_multibyte)
   4355		      {

And to make things worse, I can't get an xbacktrace because the "new"
lldb, which I got with Xcode 14, says it has a bug.  Tadah :-/.

(lldb) xbacktrace
PLEASE submit a bug report to https://developer.apple.com/bug-reporting/ and include the crash backtrace.
Stack dump:

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

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

> In GNU Emacs 29.0.50 (build 1, aarch64-apple-darwin21.6.0, NS
>  appkit-2113.60 Version 12.6 (Build 21G115)) of 2022-09-21 built on
>  Mini.fritz.box
> Repository revision: 1231a601ebe1fd9fe454c504dbeb9267440242e7
> Repository branch: master
> Windowing system distributor 'Apple', version 10.3.2113
> System Description:  macOS 12.6
>
> Configured using:
>  'configure --cache-file /Users/gerd/tmp/config.cache.master
>  --with-native-compilation'
>  
> Configured features:
> ACL DBUS GLIB GNUTLS JSON LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY
> KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS XIM ZLIB
>
> I got the following ASAN error today.  Unfortunately, I don't have the
> slightest idea how to reproduce this.
>
> ==79227==ERROR: AddressSanitizer: heap-use-after-free on address 0x00011f81e7d1 at pc 0x0001005825c4 bp 0x00016fdcf370 sp 0x00016fdcf368
> READ of size 1 at 0x00011f81e7d1 thread T0
>     #0 0x1005825c0 in re_match_2_internal regex-emacs.c:4352
>     #1 0x10057e5cc in rpl_re_search_2 regex-emacs.c:3383
>     #2 0x10057d1c4 in rpl_re_search regex-emacs.c:3177
>     #3 0x10056115c in fast_string_match_internal search.c:492
>     #4 0x1005045c0 in fast_string_match lisp.h:4818
>     #5 0x100504018 in Ffind_file_name_handler fileio.c:324
>     #6 0x1006dbe5c in openp lread.c:1911
>     #7 0x1006d8844 in Fload lread.c:1302
>     #8 0x1006e1af0 in save_match_data_load lread.c:1630
>     #9 0x10064f8cc in load_with_autoload_queue eval.c:2269
>     #10 0x10067d2f8 in Frequire fns.c:3274

Forget to copy the part where it is freed:

freed by thread T0 here:
    #0 0x103332de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
    #1 0x100985e38 in rpl_free free.c:48
    #2 0x1005b71a4 in lisp_free alloc.c:1038
    #3 0x1005cbda4 in compact_small_strings alloc.c:2191
    #4 0x1005c9f24 in sweep_strings alloc.c:2072
    #5 0x1005bd028 in gc_sweep alloc.c:7397
    #6 0x1005bb178 in garbage_collect alloc.c:6245
    #7 0x1005ba694 in maybe_garbage_collect alloc.c:6090
    #8 0x1006505ac in maybe_gc lisp.h:5624
    #9 0x100648ffc in Ffuncall eval.c:2972
    #10 0x10064bcd0 in internal_condition_case_n eval.c:1555
    #11 0x1000cdc8c in safe__call xdisp.c:3026
    #12 0x1000cdfc4 in safe__call1 xdisp.c:3062
    #13 0x1001d6404 in prepare_menu_bars xdisp.c:13572
    #14 0x1000f2340 in redisplay_internal xdisp.c:16523
    #15 0x100108f34 in redisplay xdisp.c:16105
    #16 0x10088fa84 in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8662
    #17 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
    #18 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
    #19 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
    #20 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
    #21 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
    #22 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
    #23 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
    #24 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
    #25 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
    #26 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
    #27 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
    #28 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
    #29 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)

>
> previously allocated by thread T0 here:
>     #0 0x103332ca8 in wrap_malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3eca8)
>     #1 0x1005ae8fc in lmalloc alloc.c:1361
>     #2 0x1005b0188 in lisp_malloc alloc.c:994
>     #3 0x1005b0a5c in allocate_string_data alloc.c:1889
>     #4 0x1005b1bd8 in make_clear_multibyte_string alloc.c:2475
>     #5 0x1005b1670 in make_clear_string alloc.c:2443
>     #6 0x1005b2714 in make_uninit_string alloc.c:2454
>     #7 0x100666c14 in concat_to_string fns.c:821
>     #8 0x100666420 in concat2 fns.c:600
>     #9 0x1006d7870 in Fget_load_suffixes lread.c:1123
>     #10 0x1006d86ac in Fload lread.c:1296
>     #11 0x1006e1af0 in save_match_data_load lread.c:1630
>     #12 0x10064f8cc in load_with_autoload_queue eval.c:2269
>
> rame #5: 0x00000001005825c4 emacs`re_match_2_internal(bufp=0x000000010111b890, string1=0x0000000000000000, size1=0, string2="/Users/gerd/.config/emacs.d.default/elpa/company-0.9.13/lsp-protocol.el.gz", size2=74, pos=0, regs=0x0000000000000000, stop=74) at regex-emacs.c:4352:18
>    4349	
>    4350		    PREFETCH ();
>    4351		    int len;
> -> 4352		    int corig = RE_STRING_CHAR_AND_LENGTH (d, len, target_multibyte);
>    4353		    int c = corig;
>    4354		    if (target_multibyte)
>    4355		      {
>
> And to make things worse, I can't get an xbacktrace because the "new"
> lldb, which I got with Xcode 14, says it has a bug.  Tadah :-/.
>
> (lldb) xbacktrace
> PLEASE submit a bug report to https://developer.apple.com/bug-reporting/ and include the crash backtrace.
> Stack dump:

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

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

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>> ==79227==ERROR: AddressSanitizer: heap-use-after-free on address 0x00011f81e7d1 at pc 0x0001005825c4 bp 0x00016fdcf370 sp 0x00016fdcf368
>> READ of size 1 at 0x00011f81e7d1 thread T0
>>     #0 0x1005825c0 in re_match_2_internal regex-emacs.c:4352
>>     #1 0x10057e5cc in rpl_re_search_2 regex-emacs.c:3383
>>     #2 0x10057d1c4 in rpl_re_search regex-emacs.c:3177
>>     #3 0x10056115c in fast_string_match_internal search.c:492
>>     #4 0x1005045c0 in fast_string_match lisp.h:4818
>>     #5 0x100504018 in Ffind_file_name_handler fileio.c:324
>>     #6 0x1006dbe5c in openp lread.c:1911
>>     #7 0x1006d8844 in Fload lread.c:1302
>>     #8 0x1006e1af0 in save_match_data_load lread.c:1630
>>     #9 0x10064f8cc in load_with_autoload_queue eval.c:2269
>>     #10 0x10067d2f8 in Frequire fns.c:3274

Here's a guess:

Suppose that strings a compacted in a GC happening between
fast_string_match and re_match_2_internal.  That GC compacts strings,
moves the data of the string being matched from one block to another,
and the block where the string data used to be is freed.

Then the char* used in the regexp machine point into no-man's-land.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Sat, 24 Sep 2022 16:17:20 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > ==79227==ERROR: AddressSanitizer: heap-use-after-free on address 0x00011f81e7d1 at pc 0x0001005825c4 bp 0x00016fdcf370 sp 0x00016fdcf368
> > READ of size 1 at 0x00011f81e7d1 thread T0
> >     #0 0x1005825c0 in re_match_2_internal regex-emacs.c:4352
> >     #1 0x10057e5cc in rpl_re_search_2 regex-emacs.c:3383
> >     #2 0x10057d1c4 in rpl_re_search regex-emacs.c:3177
> >     #3 0x10056115c in fast_string_match_internal search.c:492
> >     #4 0x1005045c0 in fast_string_match lisp.h:4818
> >     #5 0x100504018 in Ffind_file_name_handler fileio.c:324
> >     #6 0x1006dbe5c in openp lread.c:1911
> >     #7 0x1006d8844 in Fload lread.c:1302
> >     #8 0x1006e1af0 in save_match_data_load lread.c:1630
> >     #9 0x10064f8cc in load_with_autoload_queue eval.c:2269
> >     #10 0x10067d2f8 in Frequire fns.c:3274
> 
> Forget to copy the part where it is freed:
> 
> freed by thread T0 here:
>     #0 0x103332de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
>     #1 0x100985e38 in rpl_free free.c:48
>     #2 0x1005b71a4 in lisp_free alloc.c:1038
>     #3 0x1005cbda4 in compact_small_strings alloc.c:2191
>     #4 0x1005c9f24 in sweep_strings alloc.c:2072
>     #5 0x1005bd028 in gc_sweep alloc.c:7397
>     #6 0x1005bb178 in garbage_collect alloc.c:6245
>     #7 0x1005ba694 in maybe_garbage_collect alloc.c:6090
>     #8 0x1006505ac in maybe_gc lisp.h:5624
>     #9 0x100648ffc in Ffuncall eval.c:2972
>     #10 0x10064bcd0 in internal_condition_case_n eval.c:1555
>     #11 0x1000cdc8c in safe__call xdisp.c:3026
>     #12 0x1000cdfc4 in safe__call1 xdisp.c:3062
>     #13 0x1001d6404 in prepare_menu_bars xdisp.c:13572
>     #14 0x1000f2340 in redisplay_internal xdisp.c:16523
>     #15 0x100108f34 in redisplay xdisp.c:16105
>     #16 0x10088fa84 in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8662
>     #17 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>     #18 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
>     #19 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
>     #20 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
>     #21 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
>     #22 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
>     #23 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
>     #24 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
>     #25 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
>     #26 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
>     #27 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
>     #28 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
>     #29 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)

So it was freed by GC, which probably relocated string data or
something.  But I don't understand the relation between these two
backtraces: was the one that accessed a freed string called by the one
which triggered GC?  IOW, what is the relation between the call to
'require', which ended up calling re_match_2_internal, and the call to
prepare_menu_bars, which triggered GC?

re_search gets Lisp strings as its arguments, so unless GC was called
while the regexp search was in progress, I cannot understand how this
could happen.

Is there any way to know which argument of re_match_2_internal was
used to access the free'd heap?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Sat, 24 Sep 2022 16:17:20 +0200
>> 
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>> 
>> > ==79227==ERROR: AddressSanitizer: heap-use-after-free on address
>> > 0x00011f81e7d1 at pc 0x0001005825c4 bp 0x00016fdcf370 sp
>> > 0x00016fdcf368
>> > READ of size 1 at 0x00011f81e7d1 thread T0
>> >     #0 0x1005825c0 in re_match_2_internal regex-emacs.c:4352
>> >     #1 0x10057e5cc in rpl_re_search_2 regex-emacs.c:3383
>> >     #2 0x10057d1c4 in rpl_re_search regex-emacs.c:3177
>> >     #3 0x10056115c in fast_string_match_internal search.c:492
>> >     #4 0x1005045c0 in fast_string_match lisp.h:4818
>> >     #5 0x100504018 in Ffind_file_name_handler fileio.c:324
>> >     #6 0x1006dbe5c in openp lread.c:1911
>> >     #7 0x1006d8844 in Fload lread.c:1302
>> >     #8 0x1006e1af0 in save_match_data_load lread.c:1630
>> >     #9 0x10064f8cc in load_with_autoload_queue eval.c:2269
>> >     #10 0x10067d2f8 in Frequire fns.c:3274
>> 
>> Forget to copy the part where it is freed:
>> 
>> freed by thread T0 here:
>>     #0 0x103332de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
>>     #1 0x100985e38 in rpl_free free.c:48
>>     #2 0x1005b71a4 in lisp_free alloc.c:1038
>>     #3 0x1005cbda4 in compact_small_strings alloc.c:2191
>>     #4 0x1005c9f24 in sweep_strings alloc.c:2072
>>     #5 0x1005bd028 in gc_sweep alloc.c:7397
>>     #6 0x1005bb178 in garbage_collect alloc.c:6245
>>     #7 0x1005ba694 in maybe_garbage_collect alloc.c:6090
>>     #8 0x1006505ac in maybe_gc lisp.h:5624
>>     #9 0x100648ffc in Ffuncall eval.c:2972
>>     #10 0x10064bcd0 in internal_condition_case_n eval.c:1555
>>     #11 0x1000cdc8c in safe__call xdisp.c:3026
>>     #12 0x1000cdfc4 in safe__call1 xdisp.c:3062
>>     #13 0x1001d6404 in prepare_menu_bars xdisp.c:13572
>>     #14 0x1000f2340 in redisplay_internal xdisp.c:16523
>>     #15 0x100108f34 in redisplay xdisp.c:16105
>>     #16 0x10088fa84 in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8662
>>     #17 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>>     #18 0x1901f661c in
>> CA::Context::commit_transaction(CA::Transaction*, double,
>> double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
>>     #19 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
>>     #20 0x18bee1698 in __62+[CATransaction(NSCATransaction)
>> NS_setFlushesWithDisplayLink]_block_invoke+0x12c
>> (AppKit:arm64e+0x1ac698)
>>     #21 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
>>     #22 0x1892101a0 in
>> __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20
>> (CoreFoundation:arm64e+0x841a0)
>>     #23 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
>>     #24 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
>>     #25 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
>>     #26 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
>>     #27 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
>>     #28 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
>>     #29 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)
>
> So it was freed by GC, which probably relocated string data or
> something.  But I don't understand the relation between these two
> backtraces: was the one that accessed a freed string called by the one
> which triggered GC?  IOW, what is the relation between the call to
> 'require', which ended up calling re_match_2_internal, and the call to
> prepare_menu_bars, which triggered GC?

I don't understand that part either.

> re_search gets Lisp strings as its arguments, so unless GC was called
> while the regexp search was in progress, I cannot understand how this
> could happen.

Right, that's what I also think.  See also my other mail.

> Is there any way to know which argument of re_match_2_internal was
> used to access the free'd heap?

I can't figure it out from the code, and LLDB got the segmentation
fault, so I can't look.  Maybe Stefan can figure that out.

But in general, I think the small string compaction could be a serious
problem here, as soon as a GC happens while the regexp machine holds
pointers.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org
> Date: Sat, 24 Sep 2022 17:08:12 +0200
> 
> But in general, I think the small string compaction could be a serious
> problem here, as soon as a GC happens while the regexp machine holds
> pointers.

What is the path from regexp match to GC?  The GC was triggered by
redisplay, but how did redisplay start while regexp match was in
progress?  Do you see any code in regexp that could trigger redisplay?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: 58042@debbugs.gnu.org
>> Date: Sat, 24 Sep 2022 17:08:12 +0200
>> 
>> But in general, I think the small string compaction could be a serious
>> problem here, as soon as a GC happens while the regexp machine holds
>> pointers.
>
> What is the path from regexp match to GC?

I think since bug#56108 it's safe to say that a GC can happen while
matching.  In that bug, a regexp_cache entry was "freed" by GC.

> The GC was triggered by
> redisplay, but how did redisplay start while regexp match was in
> progress?  Do you see any code in regexp that could trigger redisplay?

I'm afraid, I don't follow.  Why do you think redisplay comes into play
here?

Anyways, my working hypotheses currently goes like this:

We match using some Lisp string S and get its data pointer, say D.
Since D is not null, S must be a live string.

(Actually I didn't check that this is still the case, but I think I've
been setting s.data to null for free strings right from the start, and I
can't imagine why anyone would change that.)

Between the point we get D, and the point of the crash, a GC happens.
We know in principle that a GC can happen while matching since
bug#56108.  I'm taking that as a given.  The GC compacts strings and
changes S's data pointer.

After GC, S.data != D.

Now, ASAN knows that a struct sdata was allocated and freed in the past
that contains S.data.  Or perhaps better said S.data points into the
part of the the freed struct sdata that ASAN checks.

How can that hapoen?

I have no idea, but that's the scenario I give the most credibility so
far.

WDYT?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org
> Date: Sun, 25 Sep 2022 07:50:17 +0200
> 
> > The GC was triggered by
> > redisplay, but how did redisplay start while regexp match was in
> > progress?  Do you see any code in regexp that could trigger redisplay?
> 
> I'm afraid, I don't follow.  Why do you think redisplay comes into play
> here?

Because of this part of your message in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58042#8:

  freed by thread T0 here:
      #0 0x103332de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
      #1 0x100985e38 in rpl_free free.c:48
      #2 0x1005b71a4 in lisp_free alloc.c:1038
      #3 0x1005cbda4 in compact_small_strings alloc.c:2191
      #4 0x1005c9f24 in sweep_strings alloc.c:2072
      #5 0x1005bd028 in gc_sweep alloc.c:7397
      #6 0x1005bb178 in garbage_collect alloc.c:6245
      #7 0x1005ba694 in maybe_garbage_collect alloc.c:6090
      #8 0x1006505ac in maybe_gc lisp.h:5624
      #9 0x100648ffc in Ffuncall eval.c:2972
      #10 0x10064bcd0 in internal_condition_case_n eval.c:1555
      #11 0x1000cdc8c in safe__call xdisp.c:3026
      #12 0x1000cdfc4 in safe__call1 xdisp.c:3062
      #13 0x1001d6404 in prepare_menu_bars xdisp.c:13572
      #14 0x1000f2340 in redisplay_internal xdisp.c:16523
      #15 0x100108f34 in redisplay xdisp.c:16105

AFAIU, this says that the GC which freed the string data was caused by
safe__call1 inside prepare_menu_bars, which was called from
redisplay_internal.

> Anyways, my working hypotheses currently goes like this:
> 
> We match using some Lisp string S and get its data pointer, say D.
> Since D is not null, S must be a live string.
> 
> (Actually I didn't check that this is still the case, but I think I've
> been setting s.data to null for free strings right from the start, and I
> can't imagine why anyone would change that.)
> 
> Between the point we get D, and the point of the crash, a GC happens.
> We know in principle that a GC can happen while matching since
> bug#56108.  I'm taking that as a given.  The GC compacts strings and
> changes S's data pointer.
> 
> After GC, S.data != D.

Yes, but I have difficulty with the fact that GC was caused by
redisplay, and redisplay cannot be invoked while we are in
re_match_2_internal, AFAIK.  So something else is missing here (or
maybe I'm misinterpreting the ASAN report you posted).

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>       #14 0x1000f2340 in redisplay_internal xdisp.c:16523
>       #15 0x100108f34 in redisplay xdisp.c:16105
>
> AFAIU, this says that the GC which freed the string data was caused by
> safe__call1 inside prepare_menu_bars, which was called from
> redisplay_internal.

Ah, okay!  Sorry, I didn't remember that redisplay on the stack.  Please
see below.

> Yes, but I have difficulty with the fact that GC was caused by
> redisplay, and redisplay cannot be invoked while we are in
> re_match_2_internal, AFAIK.  So something else is missing here (or
> maybe I'm misinterpreting the ASAN report you posted).

The second and third backtrace that ASAN displays (freed by, and
previously allocated) are not backtraces directly involved in the crash.
They display some history related to the pointer that causes the crash.

When something is allocated or freed, ASAN records callstacks that show
from where that happens.  Also, in the case pf free, it somehow arranges
that accessing that freed memory leads to a signal.  I think it uses VM
page protection for that.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org
> Date: Sun, 25 Sep 2022 09:06:59 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >       #14 0x1000f2340 in redisplay_internal xdisp.c:16523
> >       #15 0x100108f34 in redisplay xdisp.c:16105
> >
> > AFAIU, this says that the GC which freed the string data was caused by
> > safe__call1 inside prepare_menu_bars, which was called from
> > redisplay_internal.
> 
> Ah, okay!  Sorry, I didn't remember that redisplay on the stack.  Please
> see below.
> 
> > Yes, but I have difficulty with the fact that GC was caused by
> > redisplay, and redisplay cannot be invoked while we are in
> > re_match_2_internal, AFAIK.  So something else is missing here (or
> > maybe I'm misinterpreting the ASAN report you posted).
> 
> The second and third backtrace that ASAN displays (freed by, and
> previously allocated) are not backtraces directly involved in the crash.
> They display some history related to the pointer that causes the crash.

So you are saying that the backtrace I quoted, which shows that GC
that freed the string was triggered by redisplay, is NOT the GC which
actually freed the particular string involved in the
read-from-freed-heap?  If so, where's the backtrace showing the GC
that did free/relocate this particular string?

IOW, I think I'm now confused wrt what exactly the ASAN data tells us.
Can you perhaps help me understand that, quoting the relevant
backtraces as you go?

Thanks.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: 58042@debbugs.gnu.org
>> Date: Sun, 25 Sep 2022 09:06:59 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >       #14 0x1000f2340 in redisplay_internal xdisp.c:16523
>> >       #15 0x100108f34 in redisplay xdisp.c:16105
>> >
>> > AFAIU, this says that the GC which freed the string data was caused by
>> > safe__call1 inside prepare_menu_bars, which was called from
>> > redisplay_internal.
>> 
>> Ah, okay!  Sorry, I didn't remember that redisplay on the stack.  Please
>> see below.
>> 
>> > Yes, but I have difficulty with the fact that GC was caused by
>> > redisplay, and redisplay cannot be invoked while we are in
>> > re_match_2_internal, AFAIK.  So something else is missing here (or
>> > maybe I'm misinterpreting the ASAN report you posted).
>> 
>> The second and third backtrace that ASAN displays (freed by, and
>> previously allocated) are not backtraces directly involved in the crash.
>> They display some history related to the pointer that causes the crash.
>
> So you are saying that the backtrace I quoted, which shows that GC
> that freed the string was triggered by redisplay, is NOT the GC which
> actually freed the particular string involved in the
> read-from-freed-heap?

That's my working assumption, yes.

> If so, where's the backtrace showing the GC
> that did free/relocate this particular string?

It's not there.

> IOW, I think I'm now confused wrt what exactly the ASAN data tells us.
> Can you perhaps help me understand that, quoting the relevant
> backtraces as you go?

That confueses me, too.

Everything in the hypothesis seems to work, except that I can't explain
how the pointer S.data, to use that term again, can end up pointing into
memory that ASAN has page-protected.

- S must be live at the beginning of the match, otherwise S.data ==
  NULL.

- The freeing of the struct sblock during rediplay happens in the same
  thread as the match where we crash.  So it must have happened before
  the match.

So, the question seems to be what scenario would create a live string
that points into a freed sdata struct.

I'm out of ideas, and close to giving up.  Any alternative theories are
of course more than welcome.  I'm just seeking something that maybe can
be falsified.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org
> Date: Sun, 25 Sep 2022 10:28:48 +0200
> 
> So, the question seems to be what scenario would create a live string
> that points into a freed sdata struct.

That sounds highly improbable to me.  But stranger things have
happened...

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: 58042@debbugs.gnu.org
>> Date: Sun, 25 Sep 2022 10:28:48 +0200
>> 
>> So, the question seems to be what scenario would create a live string
>> that points into a freed sdata struct.
>
> That sounds highly improbable to me.  But stranger things have
> happened...

Yeah :-/.

In the meantime, and in an attempt to get some more information, I've
made me a script that starts Emacs in LLDB, with my init file, and exits
Emacs after a delay, and then does things in LLDB depending on what
happened.

I left that script running over night, and the result wasn't very
helpful.  After almost 2 hours of running, I got an ASAN error in
copyRect:(NSRect)srcRect to:(NSPoint)dest, nsterm.m.  And LLDB crashed
again.

This is with HEAD 568920a5b703e80c43e1b6f31778ea5776218a1e.

I meanwhile wonder what that all means.  An "invalid display" that isn't
reproducible, a crash in regexp, a crash in copyRect, and then the
crashes in LLDB itself.

I think I'll let that sit for a bit.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

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

Happened again today when starting Emacs with my init filem and I can't
make sense of it.  And, of course,LLDB finally crashed :-(.

(lldb) PLEASE submit a bug report to https://developer.apple.com/bug-reporting/ and include the crash backtrace.
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  lldb                                       0x00000001041e55dc llvm::sys::PrintStackTrace(llvm::raw_ostream&,

This is c3eb6c0563cc95b2134af9fe0ee6f304ddbb0480, which is from the
noverlay branch.

==15586==ERROR: AddressSanitizer: heap-use-after-free on address 0x00011f90d0a1 at pc 0x000100582044 bp 0x00016fdc8290 sp 0x00016fdc8288
READ of size 1 at 0x00011f90d0a1 thread T0
    #0 0x100582040 in re_match_2_internal regex-emacs.c:4328
    #1 0x10057e2a4 in rpl_re_search_2 regex-emacs.c:3383
    #2 0x10057ce9c in rpl_re_search regex-emacs.c:3177
    #3 0x100560e34 in fast_string_match_internal search.c:492
    #4 0x100504298 in fast_string_match lisp.h:4816
    #5 0x100503cf0 in Ffind_file_name_handler fileio.c:324
    #6 0x1006dbb34 in openp lread.c:1911
    #7 0x1006d851c in Fload lread.c:1302
    #8 0x1006e17c8 in save_match_data_load lread.c:1630
    #9 0x10064f5a4 in load_with_autoload_queue eval.c:2269
    #10 0x10067cfd0 in Frequire fns.c:3274
    #11 0x100654630 in funcall_subr eval.c:3019
    #12 0x10072e674 in exec_byte_code bytecode.c:809
    #13 0x10072c238 in Fbyte_code bytecode.c:329
    #14 0x100641c48 in eval_sub eval.c:2486
    #15 0x1006e118c in readevalloop lread.c:2339
    #16 0x1006d9d80 in Fload lread.c:1581
    #17 0x1006e17c8 in save_match_data_load lread.c:1630
    #18 0x10064f5a4 in load_with_autoload_queue eval.c:2269
    #19 0x10067cfd0 in Frequire fns.c:3274
    #20 0x100641c48 in eval_sub eval.c:2486
    #21 0x1006f5a04 in readevalloop_eager_expand_eval lread.c:2154
    #22 0x1006e117c in readevalloop lread.c:2337
    #23 0x1006e29dc in Feval_buffer lread.c:2410
    #24 0x100654900 in funcall_subr eval.c:3023
    #25 0x10072e674 in exec_byte_code bytecode.c:809
    #26 0x10065cd48 in fetch_and_exec_byte_code eval.c:3064
    #27 0x100655570 in funcall_lambda eval.c:3136
    #28 0x100653d48 in funcall_general eval.c:2927
    #29 0x100648db4 in Ffuncall eval.c:2977
    #30 0x1006de658 in call4 lisp.h:3317
    #31 0x1006d96d0 in Fload lread.c:1477
    #32 0x1006e17c8 in save_match_data_load lread.c:1630
    #33 0x10064f5a4 in load_with_autoload_queue eval.c:2269
    #34 0x10067cfd0 in Frequire fns.c:3274
    #35 0x100641c48 in eval_sub eval.c:2486
    #36 0x1006f5a04 in readevalloop_eager_expand_eval lread.c:2154
    #37 0x1006e117c in readevalloop lread.c:2337
    #38 0x1006e29dc in Feval_buffer lread.c:2410
    #39 0x100654900 in funcall_subr eval.c:3023
    #40 0x10072e674 in exec_byte_code bytecode.c:809
    #41 0x10065cd48 in fetch_and_exec_byte_code eval.c:3064
    #42 0x100655570 in funcall_lambda eval.c:3136
    #43 0x100653d48 in funcall_general eval.c:2927
    #44 0x100648db4 in Ffuncall eval.c:2977
    #45 0x1006de658 in call4 lisp.h:3317
    #46 0x1006d96d0 in Fload lread.c:1477
    #47 0x1006e17c8 in save_match_data_load lread.c:1630
    #48 0x10064f5a4 in load_with_autoload_queue eval.c:2269
    #49 0x10067cfd0 in Frequire fns.c:3274
    #50 0x100641c48 in eval_sub eval.c:2486
    #51 0x1006f5a04 in readevalloop_eager_expand_eval lread.c:2154
    #52 0x1006e117c in readevalloop lread.c:2337
    #53 0x1006e29dc in Feval_buffer lread.c:2410
    #54 0x100654900 in funcall_subr eval.c:3023
    #55 0x10072e674 in exec_byte_code bytecode.c:809
    #56 0x10065cd48 in fetch_and_exec_byte_code eval.c:3064
    #57 0x100655570 in funcall_lambda eval.c:3136
    #58 0x100653d48 in funcall_general eval.c:2927
    #59 0x100648db4 in Ffuncall eval.c:2977
    #60 0x1006de658 in call4 lisp.h:3317
    #61 0x1006d96d0 in Fload lread.c:1477
    #62 0x1006e17c8 in save_match_data_load lread.c:1630
    #63 0x10064f5a4 in load_with_autoload_queue eval.c:2269
    #64 0x10067cfd0 in Frequire fns.c:3274
    #65 0x100641c48 in eval_sub eval.c:2486
    #66 0x1006f5a04 in readevalloop_eager_expand_eval lread.c:2154
    #67 0x1006e117c in readevalloop lread.c:2337
    #68 0x1006e29dc in Feval_buffer lread.c:2410
    #69 0x100654900 in funcall_subr eval.c:3023
    #70 0x10072e674 in exec_byte_code bytecode.c:809
    #71 0x10065cd48 in fetch_and_exec_byte_code eval.c:3064
    #72 0x100655570 in funcall_lambda eval.c:3136
    #73 0x100653d48 in funcall_general eval.c:2927
    #74 0x100648db4 in Ffuncall eval.c:2977
    #75 0x1006de658 in call4 lisp.h:3317
    #76 0x1006d96d0 in Fload lread.c:1477
    #77 0x100641ed0 in eval_sub eval.c:2494
    #78 0x100643134 in Fprogn eval.c:436
    #79 0x100647a78 in Flet eval.c:1023
    #80 0x1006411c8 in eval_sub eval.c:2433
    #81 0x100643134 in Fprogn eval.c:436
    #82 0x100655a94 in funcall_lambda eval.c:3216
    #83 0x100651410 in apply_lambda eval.c:3086
    #84 0x100642a50 in eval_sub eval.c:2570
    #85 0x1006f5a04 in readevalloop_eager_expand_eval lread.c:2154
    #86 0x1006e117c in readevalloop lread.c:2337
    #87 0x1006e29dc in Feval_buffer lread.c:2410
    #88 0x100654900 in funcall_subr eval.c:3023
    #89 0x10072e674 in exec_byte_code bytecode.c:809
    #90 0x10065cd48 in fetch_and_exec_byte_code eval.c:3064
    #91 0x100655570 in funcall_lambda eval.c:3136
    #92 0x100653d48 in funcall_general eval.c:2927
    #93 0x100648db4 in Ffuncall eval.c:2977
    #94 0x1006de658 in call4 lisp.h:3317
    #95 0x1006d96d0 in Fload lread.c:1477
    #96 0x100654900 in funcall_subr eval.c:3023
    #97 0x10072e674 in exec_byte_code bytecode.c:809
    #98 0x10065cd48 in fetch_and_exec_byte_code eval.c:3064
    #99 0x100655570 in funcall_lambda eval.c:3136
    #100 0x100651410 in apply_lambda eval.c:3086
    #101 0x10064251c in eval_sub eval.c:2527
    #102 0x10064fb8c in Feval eval.c:2343
    #103 0x1004524b0 in top_level_2 keyboard.c:1141
    #104 0x10064b100 in internal_condition_case eval.c:1471
    #105 0x1004523c4 in top_level_1 keyboard.c:1149
    #106 0x10064988c in internal_catch eval.c:1194
    #107 0x100417d64 in command_loop keyboard.c:1109
    #108 0x1004177f4 in recursive_edit_1 keyboard.c:719
    #109 0x1004187b0 in Frecursive_edit keyboard.c:802
    #110 0x100410988 in main emacs.c:2521
    #111 0x101545088 in start+0x204 (dyld:arm64e+0x5088)

0x00011f90d0a1 is located 1953 bytes inside of 8184-byte region [0x00011f90c900,0x00011f90e8f8)
freed by thread T0 here:
    #0 0x103332de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
    #1 0x100985df8 in rpl_free free.c:48
    #2 0x1005b6e7c in lisp_free alloc.c:1038
    #3 0x1005cba7c in compact_small_strings alloc.c:2191
    #4 0x1005c9bfc in sweep_strings alloc.c:2072
    #5 0x1005bcd00 in gc_sweep alloc.c:7397
    #6 0x1005bae50 in garbage_collect alloc.c:6245
    #7 0x1005ba36c in maybe_garbage_collect alloc.c:6090
    #8 0x100650284 in maybe_gc lisp.h:5622
    #9 0x100648cd4 in Ffuncall eval.c:2972
    #10 0x10064b9a8 in internal_condition_case_n eval.c:1555
    #11 0x1000cd964 in safe__call xdisp.c:3026
    #12 0x1000cdc9c in safe__call1 xdisp.c:3062
    #13 0x1001d60dc in prepare_menu_bars xdisp.c:13572
    #14 0x1000f2018 in redisplay_internal xdisp.c:16523
    #15 0x100108c0c in redisplay xdisp.c:16105
    #16 0x10088fa44 in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8662
    #17 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
    #18 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
    #19 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
    #20 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
    #21 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
    #22 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
    #23 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
    #24 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
    #25 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
    #26 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
    #27 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
    #28 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
    #29 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)

previously allocated by thread T0 here:
    #0 0x103332ca8 in wrap_malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3eca8)
    #1 0x1005ae5d4 in lmalloc alloc.c:1361
    #2 0x1005afe60 in lisp_malloc alloc.c:994
    #3 0x1005b0734 in allocate_string_data alloc.c:1889
    #4 0x1005b18b0 in make_clear_multibyte_string alloc.c:2475
    #5 0x1005b1348 in make_clear_string alloc.c:2443
    #6 0x1005b23ec in make_uninit_string alloc.c:2454
    #7 0x1005b2358 in make_unibyte_string alloc.c:2369
    #8 0x1006dba68 in openp lread.c:1908
    #9 0x1006d851c in Fload lread.c:1302
    #10 0x1006e17c8 in save_match_data_load lread.c:1630
    #11 0x10064f5a4 in load_with_autoload_queue eval.c:2269
    #12 0x10067cfd0 in Frequire fns.c:3274
    #13 0x100654630 in funcall_subr eval.c:3019
    #14 0x10072e674 in exec_byte_code bytecode.c:809
    #15 0x10072c238 in Fbyte_code bytecode.c:329
    #16 0x100641c48 in eval_sub eval.c:2486
    #17 0x1006e118c in readevalloop lread.c:2339
    #18 0x1006d9d80 in Fload lread.c:1581
    #19 0x1006e17c8 in save_match_data_load lread.c:1630
    #20 0x10064f5a4 in load_with_autoload_queue eval.c:2269
    #21 0x10067cfd0 in Frequire fns.c:3274
    #22 0x100641c48 in eval_sub eval.c:2486
    #23 0x1006f5a04 in readevalloop_eager_expand_eval lread.c:2154
    #24 0x1006e117c in readevalloop lread.c:2337
    #25 0x1006e29dc in Feval_buffer lread.c:2410
    #26 0x100654900 in funcall_subr eval.c:3023
    #27 0x10072e674 in exec_byte_code bytecode.c:809
    #28 0x10065cd48 in fetch_and_exec_byte_code eval.c:3064
    #29 0x100655570 in funcall_lambda eval.c:3136

SUMMARY: AddressSanitizer: heap-use-after-free regex-emacs.c:4328 in re_match_2_internal
Shadow bytes around the buggy address:
  0x007023f419c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f419d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f419e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f419f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f41a00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x007023f41a10: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd
  0x007023f41a20: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f41a30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f41a40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f41a50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007023f41a60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==15586==ABORTING
(lldb) AddressSanitizer report breakpoint hit. Use 'thread info -s' to get extended information about the report.

(lldb) xbacktrace
(unsigned char *) data = 0x0000000100a205e0 "require"
(unsigned char *) data = 0x0000000100a25940 "byte-code"
(unsigned char *) data = 0x0000000100a205e0 "require"
(unsigned char *) data = 0x0000000100a24000 "eval-buffer"
(unsigned char *) data = 0x0000000107e7d013 "load-with-code-conversion"
(unsigned char *) data = 0x0000000100a205e0 "require"
(unsigned char *) data = 0x0000000100a24000 "eval-buffer"
(unsigned char *) data = 0x0000000107e7d013 "load-with-code-conversion"
(unsigned char *) data = 0x0000000100a205e0 "require"
(unsigned char *) data = 0x0000000100a24000 "eval-buffer"
(unsigned char *) data = 0x0000000107e7d013 "load-with-code-conversion"
(unsigned char *) data = 0x0000000100a205e0 "require"
(unsigned char *) data = 0x0000000100a24000 "eval-buffer"
(unsigned char *) data = 0x0000000107e7d013 "load-with-code-conversion"
(unsigned char *) data = 0x0000000100a1dac0 "load"
(unsigned char *) data = 0x0000000100a1d760 "let"
(unsigned char *) data = 0x0000000105c184b0 "chemacs-load-user-init"
(unsigned char *) data = 0x0000000100a24000 "eval-buffer"
(unsigned char *) data = 0x0000000107e7d013 "load-with-code-conversion"
(unsigned char *) data = 0x0000000100a1dac0 "load"
(unsigned char *) data = 0x0000000107e7ee82 "startup--load-user-init-file"
(unsigned char *) data = 0x0000000107e7f852 "command-line"
(unsigned char *) data = 0x0000000107e80b37 "normal-top-level"

frame #5: 0x0000000100582044 emacs`re_match_2_internal(bufp=0x000000010111ace8, string1=0x0000000000000000, size1=0, string2="/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib", size2=78, pos=0, regs=0x0000000000000000, stop=78) at regex-emacs.c:4328:15
   4325		    DEBUG_PRINT ("EXECUTING anychar.\n");
   4326	
   4327		    PREFETCH ();
-> 4328		    buf_ch = RE_STRING_CHAR_AND_LENGTH (d, buf_charlen,
   4329							target_multibyte);
   4330		    buf_ch = TRANSLATE (buf_ch);
   4331		    if (buf_ch == '\n')
(lldb) 
frame #6: 0x000000010057e2a8 emacs`rpl_re_search_2(bufp=0x000000010111ace8, str1=0x0000000000000000, size1=0, str2="/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib", size2=78, startpos=0, range=0, regs=0x0000000000000000, stop=78) at regex-emacs.c:3383:13
   3380		  && !bufp->can_be_null)
   3381		return -1;
   3382	
-> 3383	      val = re_match_2_internal (bufp, string1, size1, string2, size2,
   3384					 startpos, regs, stop);
   3385	
   3386	      if (val >= 0)
(lldb) down
frame #5: 0x0000000100582044 emacs`re_match_2_internal(bufp=0x000000010111ace8, string1=0x0000000000000000, size1=0, string2="/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib", size2=78, pos=0, regs=0x0000000000000000, stop=78) at regex-emacs.c:4328:15
   4325		    DEBUG_PRINT ("EXECUTING anychar.\n");
   4326	
   4327		    PREFETCH ();
-> 4328		    buf_ch = RE_STRING_CHAR_AND_LENGTH (d, buf_charlen,
   4329							target_multibyte);
   4330		    buf_ch = TRANSLATE (buf_ch);
   4331		    if (buf_ch == '\n')
(lldb) p d
(re_char *) $285 = 0x000000011f90d0a1 "magit-section-20220901.331/puny.dylib"

frame #10: 0x0000000100503cf4 emacs`Ffind_file_name_handler(filename=(struct Lisp_String *) $318 = 0x000000011f6ec4c0, operation=(struct Lisp_Symbol *) $321 = 0x00000001010ec310) at fileio.c:324:24
   321 		    operations = Fget (handler, Qoperations);
   322 	
   323 		  if (STRINGP (string)
-> 324 		      && (match_pos = fast_string_match (string, filename)) > pos
   325 		      && (NILP (operations) || ! NILP (Fmemq (operation, operations))))
   326 		    {
   327 		      Lisp_Object tem;
(lldb) p filename
(Lisp_Object) $322 = 0x000000011f6ec4c4 (struct Lisp_String *) $324 = 0x000000011f6ec4c0
(lldb) p *$324
(struct Lisp_String) $325 = {
  u = {
    s = {
      size = 78
      size_byte = -1
      intervals = NULL
      data = 0x000000011f5d2f38 "/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib"
    }
    next = 0x000000000000004e
    gcaligned = 'N'
  }
}
(lldb) p string
(Lisp_Object) $326 = 0x000000011ce990c4 (struct Lisp_String *) $328 = 0x000000011ce990c0
(lldb) p *$328
(struct Lisp_String) $329 = {
  u = {
    s = {
      size = 313
      size_byte = -1
      intervals = NULL
      data = 0x000000011cdce9f0 "\\`\\(.+\\.\\(?:7z\\|CAB\\|LZH\\|MSU\\|ZIP\\|a\\(?:pk\\|r\\)\\|c\\(?:ab\\|pio\\|rate\\)\\|de\\(?:b\\|pot\\)\\|e\\(?:pub\\|xe\\)\\|iso\\|jar\\|lzh\\|m\\(?:su\\|tree\\)\\|od[bfgpst]\\|pax\\|r\\(?:ar\\|pm\\)\\|shar\\|t\\(?:ar\\|bz\\|gz\\|lz\\|xz\\|zst\\)\\|warc\\|x\\(?:ar\\|p[is]\\)\\|zip\\)\\(?:\\.\\(?:Z\\|bz2\\|gz\\|l\\(?:rz\\|z\\(?:ma\\|[4o]\\)?\\)\\|uu\\|xz\\|zst\\)\\)?\\)\\(/.*\\)\\'"
    }
    next = 0x0000000000000139
    gcaligned = '9'
  }
}

frame #8: 0x0000000100560e38 emacs`fast_string_match_internal(regexp=(struct Lisp_String *) $342 = 0x000000011ce990c0, string=(struct Lisp_String *) $344 = 0x000000011f6ec4c0, table=(struct Lisp_Symbol *) $347 = 0x00000001010e5860) at search.c:492:19
   489 	  struct regexp_cache *cache_entry
   490 	    = compile_pattern (regexp, 0, table, 0, STRING_MULTIBYTE (string));
   491 	  freeze_pattern (cache_entry);
-> 492 	  ptrdiff_t val = re_search (&cache_entry->buf, SSDATA (string),
   493 				     SBYTES (string), 0,
   494 				     SBYTES (string), 0);
   495 	  unbind_to (count, Qnil);
(lldb) p cache_entry
(regexp_cache *) $348 = 0x000000010111acc8
(lldb) p *cache_entry
(regexp_cache) $349 = {
  next = NULL
  regexp = 0x000000011f6dbbc4 (struct Lisp_String *) $351 = 0x000000011f6dbbc0
  f_whitespace_regexp = NULL
  syntax_table = 0x0000000000000030 (struct Lisp_Symbol *) $354 = 0x00000001010e5890
  buf = {
    buffer = 0x0000000108991b80 "\v\U00000006\U00000001\U00000003\U0000000e\U00000004"
    allocated = 648
    used = 555
    charset_unibyte = 1
    fastmap = 0x000000010111ad28 ""
    translate = NULL
    re_nsub = 2
    can_be_null = true
    regs_allocated = 0
    fastmap_accurate = true
    used_syntax = false
    multibyte = false
    target_multibyte = false
  }
  fastmap = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  posix = false
  busy = true
}
(lldb) p *$351 
(struct Lisp_String) $355 = {
  u = {
    s = {
      size = 313
      size_byte = -1
      intervals = NULL
      data = 0x000000011f5cfd90 "\\`\\(.+\\.\\(?:7z\\|CAB\\|LZH\\|MSU\\|ZIP\\|a\\(?:pk\\|r\\)\\|c\\(?:ab\\|pio\\|rate\\)\\|de\\(?:b\\|pot\\)\\|e\\(?:pub\\|xe\\)\\|iso\\|jar\\|lzh\\|m\\(?:su\\|tree\\)\\|od[bfgpst]\\|pax\\|r\\(?:ar\\|pm\\)\\|shar\\|t\\(?:ar\\|bz\\|gz\\|lz\\|xz\\|zst\\)\\|warc\\|x\\(?:ar\\|p[is]\\)\\|zip\\)\\(?:\\.\\(?:Z\\|bz2\\|gz\\|l\\(?:rz\\|z\\(?:ma\\|[4o]\\)?\\)\\|uu\\|xz\\|zst\\)\\)?\\)\\(/.*\\)\\'"
    }
    next = 0x0000000000000139
    gcaligned = '9'
  }
}

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Tue, 04 Oct 2022 16:33:45 +0200
> 
> 0x00011f90d0a1 is located 1953 bytes inside of 8184-byte region [0x00011f90c900,0x00011f90e8f8)
> freed by thread T0 here:
>     #0 0x103332de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
>     #1 0x100985df8 in rpl_free free.c:48
>     #2 0x1005b6e7c in lisp_free alloc.c:1038
>     #3 0x1005cba7c in compact_small_strings alloc.c:2191
>     #4 0x1005c9bfc in sweep_strings alloc.c:2072
>     #5 0x1005bcd00 in gc_sweep alloc.c:7397
>     #6 0x1005bae50 in garbage_collect alloc.c:6245
>     #7 0x1005ba36c in maybe_garbage_collect alloc.c:6090
>     #8 0x100650284 in maybe_gc lisp.h:5622
>     #9 0x100648cd4 in Ffuncall eval.c:2972
>     #10 0x10064b9a8 in internal_condition_case_n eval.c:1555
>     #11 0x1000cd964 in safe__call xdisp.c:3026
>     #12 0x1000cdc9c in safe__call1 xdisp.c:3062
>     #13 0x1001d60dc in prepare_menu_bars xdisp.c:13572
>     #14 0x1000f2018 in redisplay_internal xdisp.c:16523
>     #15 0x100108c0c in redisplay xdisp.c:16105
>     #16 0x10088fa44 in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8662
>     #17 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>     #18 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
>     #19 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
>     #20 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
>     #21 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
>     #22 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
>     #23 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
>     #24 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
>     #25 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
>     #26 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
>     #27 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
>     #28 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
>     #29 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)

Any idea how is the above related to the other two backtraces?  Why
don't I see 'main' at the top of the backtrace here?  Can the
sanitizer be asked to produce more than 30 backtrace frames?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Tue, 04 Oct 2022 16:33:45 +0200
>> 
>> 0x00011f90d0a1 is located 1953 bytes inside of 8184-byte region [0x00011f90c900,0x00011f90e8f8)
>> freed by thread T0 here:
>>     #0 0x103332de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
>>     #1 0x100985df8 in rpl_free free.c:48
>>     #2 0x1005b6e7c in lisp_free alloc.c:1038
>
> Any idea how is the above related to the other two backtraces?  Why
> don't I see 'main' at the top of the backtrace here?  Can the
> sanitizer be asked to produce more than 30 backtrace frames?

The three backtraces are printed by the ASAN lib, with or without LLDB.

From top to bottom we're going into the past

1. Present = Where the problem was found with the pointer
2. Past = where the memory block was freed that the pointer is in.
3. Pre-Past = where block was allocated that is freed in (2)

I don't know why the ASAN output in (1) stops after 30 frames.  And I
don't know if the 30 can be changed.  But 30 for (2) and (3) seems
reasonable to me.  After all, this means 2 * 30 pointers most be
recorded per allocated memory block, and that's a quite noticeable
overhead, performance-wise.  30 looks like a heuristic.  More make
programs slower, less is less helpful.

When running under LLDB, we stop at (1), and can see the full callstack,
if we want, starting in the ASAN lib where it signals SIGABRT, and going
up to main etc.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org
> Date: Wed, 05 Oct 2022 06:37:58 +0200
> 
> >From top to bottom we're going into the past
> 
> 1. Present = Where the problem was found with the pointer
> 2. Past = where the memory block was freed that the pointer is in.
> 3. Pre-Past = where block was allocated that is freed in (2)
> 
> I don't know why the ASAN output in (1) stops after 30 frames.  And I
> don't know if the 30 can be changed.  But 30 for (2) and (3) seems
> reasonable to me.  After all, this means 2 * 30 pointers most be
> recorded per allocated memory block, and that's a quite noticeable
> overhead, performance-wise.  30 looks like a heuristic.  More make
> programs slower, less is less helpful.
> 
> When running under LLDB, we stop at (1), and can see the full callstack,
> if we want, starting in the ASAN lib where it signals SIGABRT, and going
> up to main etc.

Then I guess we will have to wait until LLDB folks get their act
together and fix LLDB to not crash before it provides the information
to us?  Or is it possible for you to downgrade to the previous,
working version of LLDB?

The question that we should try answering is this: what variable holds
the C pointer to the data of a Lisp string that is being relocated
and/or compacted by GC between the time the C pointer is assigned and
the time its value is dereferenced?  And I don't see how to answer
that question without understanding how redisplay was called in the
middle of what seems to be loading of a Lisp package, because none of
the items 1 and 3 show anything that could call redisplay.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> Then I guess we will have to wait until LLDB folks get their act
> together and fix LLDB to not crash before it provides the information
> to us?  Or is it possible for you to downgrade to the previous,
> working version of LLDB?

I'd rather not.  If it's possible at all, I don't know, it's certainly a
lot of work.

BTW, I've submitted a bug report, as LLDB requested, because of the
uppercase PLEASE :).  Let's see if that lands anywhere.  I don't have
high hopes.

> The question that we should try answering is this: what variable holds
> the C pointer to the data of a Lisp string that is being relocated
> and/or compacted by GC between the time the C pointer is assigned and
> the time its value is dereferenced?

I think we can answer that question, at least with a good probability.
If you look what the offending (I think) pointer points to:

frame #5: 0x0000000100582044 emacs`re_match_2_internal(bufp=0x000000010111ace8, string1=0x0000000000000000, size1=0, string2="/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib", size2=78, pos=0, regs=0x0000000000000000, stop=78) at regex-emacs.c:4328:15
   4325		    DEBUG_PRINT ("EXECUTING anychar.\n");
   4326	
   4327		    PREFETCH ();
-> 4328		    buf_ch = RE_STRING_CHAR_AND_LENGTH (d, buf_charlen,
   4329							target_multibyte);
   4330		    buf_ch = TRANSLATE (buf_ch);
   4331		    if (buf_ch == '\n')
(lldb) p d
(re_char *) $285 = 0x000000011f90d0a1 "magit-section-20220901.331/puny.dylib"

That looks like part of the filename here:

frame #10: 0x0000000100503cf4 emacs`Ffind_file_name_handler(filename=(struct Lisp_String *) $318 = 0x000000011f6ec4c0, operation=(struct Lisp_Symbol *) $321 = 0x00000001010ec310) at fileio.c:324:24
   321 		    operations = Fget (handler, Qoperations);
   322 	
   323 		  if (STRINGP (string)
-> 324 		      && (match_pos = fast_string_match (string, filename)) > pos
   325 		      && (NILP (operations) || ! NILP (Fmemq (operation, operations))))
   326 		    {
   327 		      Lisp_Object tem;
(lldb) p filename
(Lisp_Object) $322 = 0x000000011f6ec4c4 (struct Lisp_String *) $324 = 0x000000011f6ec4c0
(lldb) p *$324
(struct Lisp_String) $325 = {
  u = {
    s = {
      size = 78
      size_byte = -1
      intervals = NULL
      data = 0x000000011f5d2f38 "/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib"
    }
    next = 0x000000000000004e
    gcaligned = 'N'
  }
}

So, I'd say that the filename string data has been moved somewhere else
during compaction.  Which would mean GC somehow ran between the point
where "d" in frame#5 was initially set up from the filename, and line
4328 where the problem is detected.

> I don't see how to answer
> that question without understanding how redisplay was called in the
> middle of what seems to be loading of a Lisp package, because none of
> the items 1 and 3 show anything that could call redisplay.

What I can see is that, apparently, redisplay got called because Emacs
received a MacOS event, and did a prepare_menu_bars etc etc.

How that's possible, if it is, while Emacs is in between frame#10 and
frame#5 I have not the slightest idea.  And please note that this is all
happening in the same thread T0, according to ASAN.

Maybe someone knowing the Mac port has an idea if this can happen?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org
> Date: Wed, 05 Oct 2022 08:58:51 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The question that we should try answering is this: what variable holds
> > the C pointer to the data of a Lisp string that is being relocated
> > and/or compacted by GC between the time the C pointer is assigned and
> > the time its value is dereferenced?
> 
> I think we can answer that question, at least with a good probability.
> If you look what the offending (I think) pointer points to:
> 
> frame #5: 0x0000000100582044 emacs`re_match_2_internal(bufp=0x000000010111ace8, string1=0x0000000000000000, size1=0, string2="/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib", size2=78, pos=0, regs=0x0000000000000000, stop=78) at regex-emacs.c:4328:15
>    4325		    DEBUG_PRINT ("EXECUTING anychar.\n");
>    4326	
>    4327		    PREFETCH ();
> -> 4328		    buf_ch = RE_STRING_CHAR_AND_LENGTH (d, buf_charlen,
>    4329							target_multibyte);
>    4330		    buf_ch = TRANSLATE (buf_ch);
>    4331		    if (buf_ch == '\n')
> (lldb) p d
> (re_char *) $285 = 0x000000011f90d0a1 "magit-section-20220901.331/puny.dylib"
> 
> That looks like part of the filename here:
> 
> frame #10: 0x0000000100503cf4 emacs`Ffind_file_name_handler(filename=(struct Lisp_String *) $318 = 0x000000011f6ec4c0, operation=(struct Lisp_Symbol *) $321 = 0x00000001010ec310) at fileio.c:324:24
>    321 		    operations = Fget (handler, Qoperations);
>    322 	
>    323 		  if (STRINGP (string)
> -> 324 		      && (match_pos = fast_string_match (string, filename)) > pos
>    325 		      && (NILP (operations) || ! NILP (Fmemq (operation, operations))))
>    326 		    {
>    327 		      Lisp_Object tem;
> (lldb) p filename
> (Lisp_Object) $322 = 0x000000011f6ec4c4 (struct Lisp_String *) $324 = 0x000000011f6ec4c0
> (lldb) p *$324
> (struct Lisp_String) $325 = {
>   u = {
>     s = {
>       size = 78
>       size_byte = -1
>       intervals = NULL
>       data = 0x000000011f5d2f38 "/Users/gerd/.config/emacs.d.default/elpa/magit-section-20220901.331/puny.dylib"
>     }
>     next = 0x000000000000004e
>     gcaligned = 'N'
>   }
> }
> 
> So, I'd say that the filename string data has been moved somewhere else
> during compaction.  Which would mean GC somehow ran between the point
> where "d" in frame#5 was initially set up from the filename, and line
> 4328 where the problem is detected.

That part is clear, but the "GC somehow ran" part is not, and that is
the part which we must understand to fix the problem.  The filename's
SSDATA is passed to re_search as a C string, under the assumption that
GC cannot happen while re_search runs.  If that assumption is false,
we need to understand exactly how and in what cases, because without
that there's nothing we can do -- regex-emacs.c code deals explicitly
only with C strings.

IOW, this isn't the case like

  char *ptr = SSDATA (lisp_string);
  ...
  dereference (ptr);

where GC can happen as part of "...".  Those cases are easy to fix.
But this is not that case.

> > I don't see how to answer
> > that question without understanding how redisplay was called in the
> > middle of what seems to be loading of a Lisp package, because none of
> > the items 1 and 3 show anything that could call redisplay.
> 
> What I can see is that, apparently, redisplay got called because Emacs
> received a MacOS event, and did a prepare_menu_bars etc etc.

You mean, a macOS event can be received asynchronously, and will
interrupt some processing in C, like inside regex-emacs.c?  If that
can happen, no code in Emacs is safe, ever.  I don't believe this is
possible: we no longer process window-system events asynchronously,
AFAIK, and for this very reason.  But maybe macOS is different?  In
that case, either we should change the macOS code to avoid doing that,
or we should have some means of blocking such "interrupts" around
specific code fragments, akin to block_input.

> How that's possible, if it is, while Emacs is in between frame#10 and
> frame#5 I have not the slightest idea.  And please note that this is all
> happening in the same thread T0, according to ASAN.

Yes, I've seen that it's the same thread.  Having redisplay run from
another thread would be a larger disaster.

> Maybe someone knowing the Mac port has an idea if this can happen?

I hope so.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> What I can see is that, apparently, redisplay got called because Emacs
>> received a MacOS event, and did a prepare_menu_bars etc etc.
>
> You mean, a macOS event can be received asynchronously, and will
> interrupt some processing in C, like inside regex-emacs.c?

If it can, I don't know.  But is the GC during redisplay is the one
moving the string, that would be the consequence, I think.

> If that can happen, no code in Emacs is safe, ever.  I don't believe
> this is possible: we no longer process window-system events
> asynchronously, AFAIK, and for this very reason.  But maybe macOS is
> different?  In that case, either we should change the macOS code to
> avoid doing that, or we should have some means of blocking such
> "interrupts" around specific code fragments, akin to block_input.

Yeah.  It would be good if that wouldn't happen ever, if it can.

If it can't happen, then the GC in redisplay that we see is not directly
related to all of this. and your question how redisplay can run while
matching is also off the table, I think.  I don't know a way how that
could happen.

But some GC must run and move strings around.  I don't know how else to
explain the invalid pointer.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

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

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> What I can see is that, apparently, redisplay got called because Emacs
>>> received a MacOS event, and did a prepare_menu_bars etc etc.
>>
>> You mean, a macOS event can be received asynchronously, and will
>> interrupt some processing in C, like inside regex-emacs.c?
>
> If it can, I don't know.  But is the GC during redisplay is the one
> moving the string, that would be the consequence, I think.
>
>> If that can happen, no code in Emacs is safe, ever.  I don't believe
>> this is possible: we no longer process window-system events
>> asynchronously, AFAIK, and for this very reason.  But maybe macOS is
>> different?  In that case, either we should change the macOS code to
>> avoid doing that, or we should have some means of blocking such
>> "interrupts" around specific code fragments, akin to block_input.
>
> Yeah.  It would be good if that wouldn't happen ever, if it can.

I just got another ASAN error in a branch based on master.  It looks
completely different, but I find it eye-opening for our case.  Look at
this:

==45724==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a4b04 bp 0x00016fd155e0 sp 0x00016fd155d8
READ of size 8 at 0x000107130d00 thread T0
    #0 0x1002a4b00 in PSEUDOVECTORP lisp.h:1110
    #1 0x1002a4b70 in SYMBOL_WITH_POS_P lisp.h:1122
    #2 0x10025a620 in EQ lisp.h:1342
    #3 0x100281198 in run_window_change_functions window.c:3964
    #4 0x1000f1bac in redisplay_internal xdisp.c:16600
    #5 0x100107ee0 in redisplay xdisp.c:16111
    #6 0x10089366c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
    #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
    #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
    #9 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
    #10 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
    #11 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
    #12 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
    #13 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
    #14 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
    #15 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
    #16 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
    #17 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
    #18 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
    #19 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)
    #20 0x18bd74e10 in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]+0x52c (AppKit:arm64e+0x3fe10)
    #21 0x18bd66fdc in -[NSApplication run]+0x250 (AppKit:arm64e+0x31fdc)
    #22 0x100870bd0 in -[EmacsApp run] nsterm.m:5799
    #23 0x1008c7b2c in ns_read_socket_1 nsterm.m:4679
    #24 0x1008ae550 in ns_read_socket nsterm.m:4697
    #25 0x100437394 in gobble_input keyboard.c:7379
    #26 0x100438bfc in handle_async_input keyboard.c:7610
    #27 0x100438bdc in process_pending_signals keyboard.c:7624
    #28 0x10064bd90 in probably_quit eval.c:1657
    #29 0x10065fe6c in maybe_quit lisp.h:3737
    #30 0x10066cb7c in Fmemq fns.c:1837
    #31 0x100645de8 in FletX eval.c:936

There is a path from maybe_quit to redisplay, and didn't we have
maybe_quit alreasy in the matcher code?  Mind-boggling!

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org
> Date: Wed, 05 Oct 2022 11:00:00 +0200
> 
> ==45724==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a4b04 bp 0x00016fd155e0 sp 0x00016fd155d8
> READ of size 8 at 0x000107130d00 thread T0
>     #0 0x1002a4b00 in PSEUDOVECTORP lisp.h:1110
>     #1 0x1002a4b70 in SYMBOL_WITH_POS_P lisp.h:1122
>     #2 0x10025a620 in EQ lisp.h:1342
>     #3 0x100281198 in run_window_change_functions window.c:3964
>     #4 0x1000f1bac in redisplay_internal xdisp.c:16600
>     #5 0x100107ee0 in redisplay xdisp.c:16111
>     #6 0x10089366c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
>     #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>     #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
>     #9 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
>     #10 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
>     #11 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
>     #12 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
>     #13 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
>     #14 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
>     #15 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
>     #16 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
>     #17 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
>     #18 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
>     #19 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)
>     #20 0x18bd74e10 in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]+0x52c (AppKit:arm64e+0x3fe10)
>     #21 0x18bd66fdc in -[NSApplication run]+0x250 (AppKit:arm64e+0x31fdc)
>     #22 0x100870bd0 in -[EmacsApp run] nsterm.m:5799
>     #23 0x1008c7b2c in ns_read_socket_1 nsterm.m:4679
>     #24 0x1008ae550 in ns_read_socket nsterm.m:4697
>     #25 0x100437394 in gobble_input keyboard.c:7379
>     #26 0x100438bfc in handle_async_input keyboard.c:7610
>     #27 0x100438bdc in process_pending_signals keyboard.c:7624
>     #28 0x10064bd90 in probably_quit eval.c:1657
>     #29 0x10065fe6c in maybe_quit lisp.h:3737
>     #30 0x10066cb7c in Fmemq fns.c:1837
>     #31 0x100645de8 in FletX eval.c:936
> 
> There is a path from maybe_quit to redisplay, and didn't we have
> maybe_quit alreasy in the matcher code?  Mind-boggling!

Ouch!  This seems to be macOS-specific, though.

So I guess we should do this dance around calls to maybe_quit in
regex-emacs.c:

  specpdl_ref gc_count = inhibit_garbage_collection ();
  maybe_quit ();
  unbind_to (gc_count, Qnil);

Or maybe even better, do this inside probably_quit (because who knows
how many other callers of maybe_quit could be hit by this unexpected
GC)?

Can you try this?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> So I guess we should do this dance around calls to maybe_quit in
> regex-emacs.c:
>
>   specpdl_ref gc_count = inhibit_garbage_collection ();
>   maybe_quit ();
>   unbind_to (gc_count, Qnil);
>
> Or maybe even better, do this inside probably_quit (because who knows
> how many other callers of maybe_quit could be hit by this unexpected
> GC)?
>
> Can you try this?

Isn't the -[EmacsView layoutSublayersOfLayer:] the problem?  AFAICT from
a web search, this is an event handler method that is also called from
by the framework?

In the olden days, it was a serious error to call into Lisp from an
event handler.  All bets were off when that happened, not only related
to GC.  I believe that hasn't changed much.

That code was introduced by Alan around this time.

1ba02d85a964e1b2c6a9735cd3decdc524e06dc1
Author:     Alan Third <alan@idiocy.org>
AuthorDate: Sat Jun 12 10:25:47 2021 +0100
Commit:     Alan Third <alan@idiocy.org>
CommitDate: Sat Jul 31 11:13:05 2021 +0100

Maybe Allen can say something, I've CC'd him.

Or maybe we should add your fix, too?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

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

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> So I guess we should do this dance around calls to maybe_quit in
>> regex-emacs.c:
>>
>>   specpdl_ref gc_count = inhibit_garbage_collection ();
>>   maybe_quit ();
>>   unbind_to (gc_count, Qnil);
>>
>> Or maybe even better, do this inside probably_quit (because who knows
>> how many other callers of maybe_quit could be hit by this unexpected
>> GC)?
>>
>> Can you try this?
>
> Isn't the -[EmacsView layoutSublayersOfLayer:] the problem?  AFAICT from
> a web search, this is an event handler method that is also called from
> by the framework?
>
> In the olden days, it was a serious error to call into Lisp from an
> event handler.  All bets were off when that happened, not only related
> to GC.  I believe that hasn't changed much.
>
> That code was introduced by Alan around this time.
>
> 1ba02d85a964e1b2c6a9735cd3decdc524e06dc1
> Author:     Alan Third <alan@idiocy.org>
> AuthorDate: Sat Jun 12 10:25:47 2021 +0100
> Commit:     Alan Third <alan@idiocy.org>
> CommitDate: Sat Jul 31 11:13:05 2021 +0100
>
> Maybe Allen can say something, I've CC'd him.
>
> Or maybe we should add your fix, too?

And a similar question to Po Lu because of

f81065a91be5a54b78e202df6918aff443588ae1
Author:     Po Lu <luangruo@yahoo.com>
AuthorDate: Mon May 30 16:03:11 2022 +0800
Commit:     Po Lu <luangruo@yahoo.com>
CommitDate: Mon May 30 16:03:11 2022 +0800

which added a call to redisplay to - (NSDragOperation) draggingUpdated:
(id <NSDraggingInfo>) sender.  Is that safe here?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

>> Isn't the -[EmacsView layoutSublayersOfLayer:] the problem?  AFAICT from
>> a web search, this is an event handler method that is also called from
>> by the framework?
>>
>> In the olden days, it was a serious error to call into Lisp from an
>> event handler.  All bets were off when that happened, not only related
>> to GC.  I believe that hasn't changed much.

Today, event handling code calls Lisp all the time (through safe_call
etc.)  That happens in handle_one_xevent, ns_select, et cetera.

It shouldn't affect GC at all because input is blocked for the entire
duration of each GC, except for when finalizers are run after unmarked
objects are sweeped.

So AFAIU it has been safe ever since read_socket_hook stopped being
called from a signal handler.

>> That code was introduced by Alan around this time.
>>
>> 1ba02d85a964e1b2c6a9735cd3decdc524e06dc1
>> Author:     Alan Third <alan@idiocy.org>
>> AuthorDate: Sat Jun 12 10:25:47 2021 +0100
>> Commit:     Alan Third <alan@idiocy.org>
>> CommitDate: Sat Jul 31 11:13:05 2021 +0100
>>
>> Maybe Allen can say something, I've CC'd him.
>>
>> Or maybe we should add your fix, too?
>
> And a similar question to Po Lu because of
>
> f81065a91be5a54b78e202df6918aff443588ae1
> Author:     Po Lu <luangruo@yahoo.com>
> AuthorDate: Mon May 30 16:03:11 2022 +0800
> Commit:     Po Lu <luangruo@yahoo.com>
> CommitDate: Mon May 30 16:03:11 2022 +0800
>
> which added a call to redisplay to - (NSDragOperation) draggingUpdated:
> (id <NSDraggingInfo>) sender.  Is that safe here?

It should be safe there since we use safe_call, as the only problem
these days is that it isn't safe to longjmp out of an NS event handler.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[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:
>>
>>> So I guess we should do this dance around calls to maybe_quit in
>>> regex-emacs.c:
>>>
>>>   specpdl_ref gc_count = inhibit_garbage_collection ();
>>>   maybe_quit ();
>>>   unbind_to (gc_count, Qnil);
>>>
>>> Or maybe even better, do this inside probably_quit (because who knows
>>> how many other callers of maybe_quit could be hit by this unexpected
>>> GC)?
>>>
>>> Can you try this?
>>
>> Isn't the -[EmacsView layoutSublayersOfLayer:] the problem?  AFAICT from
>> a web search, this is an event handler method that is also called from
>> by the framework?
>>
>> In the olden days, it was a serious error to call into Lisp from an
>> event handler.  All bets were off when that happened, not only related
>> to GC.  I believe that hasn't changed much.
>>
>> That code was introduced by Alan around this time.
>>
>> 1ba02d85a964e1b2c6a9735cd3decdc524e06dc1
>> Author:     Alan Third <alan@idiocy.org>
>> AuthorDate: Sat Jun 12 10:25:47 2021 +0100
>> Commit:     Alan Third <alan@idiocy.org>
>> CommitDate: Sat Jul 31 11:13:05 2021 +0100
>>
>> Maybe Allen can say something, I've CC'd him.
>>
>> Or maybe we should add your fix, too?
>
> And a similar question to Po Lu because of
>
> f81065a91be5a54b78e202df6918aff443588ae1
> Author:     Po Lu <luangruo@yahoo.com>
> AuthorDate: Mon May 30 16:03:11 2022 +0800
> Commit:     Po Lu <luangruo@yahoo.com>
> CommitDate: Mon May 30 16:03:11 2022 +0800
>
> which added a call to redisplay to - (NSDragOperation) draggingUpdated:
> (id <NSDraggingInfo>) sender.  Is that safe here?

And an update to the second ASAN error that I could actually reproduce
by starting Emacs on my branch:

==64010==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a48d8 bp 0x00016fdcaa80 sp 0x00016fdcaa78
READ of size 8 at 0x000107130d00 thread T0
    #0 0x1002a48d4 in PSEUDOVECTORP lisp.h:1110
    #1 0x1002a4944 in SYMBOL_WITH_POS_P lisp.h:1122
    #2 0x10025a3f4 in EQ lisp.h:1342
    #3 0x100280f6c in run_window_change_functions window.c:3964
    #4 0x1000f1980 in redisplay_internal xdisp.c:16600
    #5 0x100107cb4 in redisplay xdisp.c:16111
    #6 0x10089364c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
    #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
    #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
    #9 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
    #10 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
    #11 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
    #12 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
    #13 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
    #14 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
    #15 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
    #16 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
    #17 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
    #18 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
    #19 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)
    #20 0x18bd74e10 in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]+0x52c (AppKit:arm64e+0x3fe10)
    #21 0x18bd66fdc in -[NSApplication run]+0x250 (AppKit:arm64e+0x31fdc)
    #22 0x100870bb0 in -[EmacsApp run] nsterm.m:5799
    #23 0x1008c7b0c in ns_read_socket_1 nsterm.m:4679
    #24 0x1008ae530 in ns_read_socket nsterm.m:4697
    #25 0x100437168 in gobble_input keyboard.c:7379
    #26 0x1004389d0 in handle_async_input keyboard.c:7610
    #27 0x1004389b0 in process_pending_signals keyboard.c:7624
    #28 0x100438acc in unblock_input_to keyboard.c:7639
    #29 0x100432cac in unblock_input keyboard.c:7658
    #30 0x1005ba024 in garbage_collect alloc.c:6256
    #31 0x1005b950c in maybe_garbage_collect alloc.c:6090
    #32 0x10064f6a8 in maybe_gc lisp.h:5622
    #33 0x10063fcfc in eval_sub eval.c:2388
    #34 0x100640838 in eval_sub eval.c:2449
    #35 0x10064234c in Fprogn eval.c:436
    #36 0x100654eb8 in funcall_lambda eval.c:3218
    #37 0x1006532c4 in funcall_general eval.c:2941
    #38 0x100647fcc in Ffuncall eval.c:2979
    #39 0x100651ca8 in Fapply eval.c:2650
    #40 0x10063ead8 in apply1 eval.c:2866
    #41 0x1006484bc in Fmacroexpand eval.c:1149
    #42 0x10065394c in funcall_subr eval.c:3019
    #43 0x10072e004 in exec_byte_code bytecode.c:809
    #44 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
    #45 0x100654994 in funcall_lambda eval.c:3138
    #46 0x10065316c in funcall_general eval.c:2929
    #47 0x100647fcc in Ffuncall eval.c:2979
    #48 0x1006fcd80 in call2 lisp.h:3302
    #49 0x1006f4ecc in readevalloop_eager_expand_eval lread.c:2151
    #50 0x1006e0b0c in readevalloop lread.c:2343
    #51 0x1006e236c in Feval_buffer lread.c:2416
    #52 0x100653d24 in funcall_subr eval.c:3025
    #53 0x10072e004 in exec_byte_code bytecode.c:809
    #54 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
    #55 0x100654994 in funcall_lambda eval.c:3138
    #56 0x10065316c in funcall_general eval.c:2929
    #57 0x100647fcc in Ffuncall eval.c:2979
    #58 0x1006ddfe8 in call4 lisp.h:3317
    #59 0x1006d9058 in Fload lread.c:1483
    #60 0x1006e1158 in save_match_data_load lread.c:1636
    #61 0x10064e9c8 in load_with_autoload_queue eval.c:2271
    #62 0x10067cb40 in Frequire fns.c:3308
    #63 0x100653a54 in funcall_subr eval.c:3021
    #64 0x10072e004 in exec_byte_code bytecode.c:809
    #65 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
    #66 0x100654994 in funcall_lambda eval.c:3138
    #67 0x10065316c in funcall_general eval.c:2929
    #68 0x100647fcc in Ffuncall eval.c:2979
    #69 0x100651ca8 in Fapply eval.c:2650
    #70 0x100654414 in funcall_subr eval.c:3044
    #71 0x10072e004 in exec_byte_code bytecode.c:809
    #72 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
    #73 0x100654994 in funcall_lambda eval.c:3138
    #74 0x100650834 in apply_lambda eval.c:3088
    #75 0x100641734 in eval_sub eval.c:2529
    #76 0x1006f5394 in readevalloop_eager_expand_eval lread.c:2160
    #77 0x1006e0b0c in readevalloop lread.c:2343
    #78 0x1006e236c in Feval_buffer lread.c:2416
    #79 0x100653d24 in funcall_subr eval.c:3025
    #80 0x10072e004 in exec_byte_code bytecode.c:809
    #81 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
    #82 0x100654994 in funcall_lambda eval.c:3138
    #83 0x10065316c in funcall_general eval.c:2929
    #84 0x100647fcc in Ffuncall eval.c:2979
    #85 0x1006ddfe8 in call4 lisp.h:3317
    #86 0x1006d9058 in Fload lread.c:1483
    #87 0x1006410e8 in eval_sub eval.c:2496
    #88 0x10064234c in Fprogn eval.c:436
    #89 0x100646c90 in Flet eval.c:1023
    #90 0x1006403e0 in eval_sub eval.c:2435
    #91 0x10064234c in Fprogn eval.c:436
    #92 0x100654eb8 in funcall_lambda eval.c:3218
    #93 0x100650834 in apply_lambda eval.c:3088
    #94 0x100641c68 in eval_sub eval.c:2572
    #95 0x1006f5394 in readevalloop_eager_expand_eval lread.c:2160
    #96 0x1006e0b0c in readevalloop lread.c:2343
    #97 0x1006e236c in Feval_buffer lread.c:2416
    #98 0x100653d24 in funcall_subr eval.c:3025
    #99 0x10072e004 in exec_byte_code bytecode.c:809
    #100 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
    #101 0x100654994 in funcall_lambda eval.c:3138
    #102 0x10065316c in funcall_general eval.c:2929
    #103 0x100647fcc in Ffuncall eval.c:2979
    #104 0x1006ddfe8 in call4 lisp.h:3317
    #105 0x1006d9058 in Fload lread.c:1483
    #106 0x100653d24 in funcall_subr eval.c:3025
    #107 0x10072e004 in exec_byte_code bytecode.c:809
    #108 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
    #109 0x100654994 in funcall_lambda eval.c:3138
    #110 0x100650834 in apply_lambda eval.c:3088
    #111 0x100641734 in eval_sub eval.c:2529
    #112 0x10064efb0 in Feval eval.c:2345
    #113 0x100451650 in top_level_2 keyboard.c:1141
    #114 0x10064a318 in internal_condition_case eval.c:1471
    #115 0x100451564 in top_level_1 keyboard.c:1149
    #116 0x100648aa4 in internal_catch eval.c:1194
    #117 0x100416f04 in command_loop keyboard.c:1109
    #118 0x100416994 in recursive_edit_1 keyboard.c:719
    #119 0x100417950 in Frecursive_edit keyboard.c:802
    #120 0x10040fb00 in main emacs.c:2515
    #121 0x101549088 in start+0x204 (dyld:arm64e+0x5088)

That is redisplay during garbage_collect!

The change to probably_quit didn't help.  Commenting out the call to
redisplay in the layout stuff did.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Po Lu <luangruo@yahoo.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>>> Isn't the -[EmacsView layoutSublayersOfLayer:] the problem?  AFAICT from
>>> a web search, this is an event handler method that is also called from
>>> by the framework?
>>>
>>> In the olden days, it was a serious error to call into Lisp from an
>>> event handler.  All bets were off when that happened, not only related
>>> to GC.  I believe that hasn't changed much.
>
> Today, event handling code calls Lisp all the time (through safe_call
> etc.)  That happens in handle_one_xevent, ns_select, et cetera.
>
> It shouldn't affect GC at all because input is blocked for the entire
> duration of each GC, except for when finalizers are run after unmarked
> objects are sweeped.
>
> So AFAIU it has been safe ever since read_socket_hook stopped being
> called from a signal handler.
>
>>> That code was introduced by Alan around this time.
>>>
>>> 1ba02d85a964e1b2c6a9735cd3decdc524e06dc1
>>> Author:     Alan Third <alan@idiocy.org>
>>> AuthorDate: Sat Jun 12 10:25:47 2021 +0100
>>> Commit:     Alan Third <alan@idiocy.org>
>>> CommitDate: Sat Jul 31 11:13:05 2021 +0100
>>>
>>> Maybe Allen can say something, I've CC'd him.
>>>
>>> Or maybe we should add your fix, too?
>>
>> And a similar question to Po Lu because of
>>
>> f81065a91be5a54b78e202df6918aff443588ae1
>> Author:     Po Lu <luangruo@yahoo.com>
>> AuthorDate: Mon May 30 16:03:11 2022 +0800
>> Commit:     Po Lu <luangruo@yahoo.com>
>> CommitDate: Mon May 30 16:03:11 2022 +0800
>>
>> which added a call to redisplay to - (NSDragOperation) draggingUpdated:
>> (id <NSDraggingInfo>) sender.  Is that safe here?
>
> It should be safe there since we use safe_call, as the only problem
> these days is that it isn't safe to longjmp out of an NS event handler.

Ok, I can't say much to this.  But please look at the my latest post.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> And an update to the second ASAN error that I could actually reproduce
> by starting Emacs on my branch:
>
> ==64010==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a48d8 bp 0x00016fdcaa80 sp 0x00016fdcaa78
> READ of size 8 at 0x000107130d00 thread T0
>     #0 0x1002a48d4 in PSEUDOVECTORP lisp.h:1110
>     #1 0x1002a4944 in SYMBOL_WITH_POS_P lisp.h:1122
>     #2 0x10025a3f4 in EQ lisp.h:1342
>     #3 0x100280f6c in run_window_change_functions window.c:3964
>     #4 0x1000f1980 in redisplay_internal xdisp.c:16600
>     #5 0x100107cb4 in redisplay xdisp.c:16111
>     #6 0x10089364c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
>     #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>     #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
>     #9 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
>     #10 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
>     #11 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
>     #12 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
>     #13 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
>     #14 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
>     #15 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
>     #16 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
>     #17 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
>     #18 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
>     #19 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)
>     #20 0x18bd74e10 in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]+0x52c (AppKit:arm64e+0x3fe10)
>     #21 0x18bd66fdc in -[NSApplication run]+0x250 (AppKit:arm64e+0x31fdc)
>     #22 0x100870bb0 in -[EmacsApp run] nsterm.m:5799
>     #23 0x1008c7b0c in ns_read_socket_1 nsterm.m:4679
>     #24 0x1008ae530 in ns_read_socket nsterm.m:4697
>     #25 0x100437168 in gobble_input keyboard.c:7379
>     #26 0x1004389d0 in handle_async_input keyboard.c:7610
>     #27 0x1004389b0 in process_pending_signals keyboard.c:7624
>     #28 0x100438acc in unblock_input_to keyboard.c:7639
>     #29 0x100432cac in unblock_input keyboard.c:7658
>     #30 0x1005ba024 in garbage_collect alloc.c:6256
>     #31 0x1005b950c in maybe_garbage_collect alloc.c:6090
>     #32 0x10064f6a8 in maybe_gc lisp.h:5622
>     #33 0x10063fcfc in eval_sub eval.c:2388
>     #34 0x100640838 in eval_sub eval.c:2449
>     #35 0x10064234c in Fprogn eval.c:436
>     #36 0x100654eb8 in funcall_lambda eval.c:3218
>     #37 0x1006532c4 in funcall_general eval.c:2941
>     #38 0x100647fcc in Ffuncall eval.c:2979
>     #39 0x100651ca8 in Fapply eval.c:2650
>     #40 0x10063ead8 in apply1 eval.c:2866
>     #41 0x1006484bc in Fmacroexpand eval.c:1149
>     #42 0x10065394c in funcall_subr eval.c:3019
>     #43 0x10072e004 in exec_byte_code bytecode.c:809
>     #44 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #45 0x100654994 in funcall_lambda eval.c:3138
>     #46 0x10065316c in funcall_general eval.c:2929
>     #47 0x100647fcc in Ffuncall eval.c:2979
>     #48 0x1006fcd80 in call2 lisp.h:3302
>     #49 0x1006f4ecc in readevalloop_eager_expand_eval lread.c:2151
>     #50 0x1006e0b0c in readevalloop lread.c:2343
>     #51 0x1006e236c in Feval_buffer lread.c:2416
>     #52 0x100653d24 in funcall_subr eval.c:3025
>     #53 0x10072e004 in exec_byte_code bytecode.c:809
>     #54 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #55 0x100654994 in funcall_lambda eval.c:3138
>     #56 0x10065316c in funcall_general eval.c:2929
>     #57 0x100647fcc in Ffuncall eval.c:2979
>     #58 0x1006ddfe8 in call4 lisp.h:3317
>     #59 0x1006d9058 in Fload lread.c:1483
>     #60 0x1006e1158 in save_match_data_load lread.c:1636
>     #61 0x10064e9c8 in load_with_autoload_queue eval.c:2271
>     #62 0x10067cb40 in Frequire fns.c:3308
>     #63 0x100653a54 in funcall_subr eval.c:3021
>     #64 0x10072e004 in exec_byte_code bytecode.c:809
>     #65 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #66 0x100654994 in funcall_lambda eval.c:3138
>     #67 0x10065316c in funcall_general eval.c:2929
>     #68 0x100647fcc in Ffuncall eval.c:2979
>     #69 0x100651ca8 in Fapply eval.c:2650
>     #70 0x100654414 in funcall_subr eval.c:3044
>     #71 0x10072e004 in exec_byte_code bytecode.c:809
>     #72 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #73 0x100654994 in funcall_lambda eval.c:3138
>     #74 0x100650834 in apply_lambda eval.c:3088
>     #75 0x100641734 in eval_sub eval.c:2529
>     #76 0x1006f5394 in readevalloop_eager_expand_eval lread.c:2160
>     #77 0x1006e0b0c in readevalloop lread.c:2343
>     #78 0x1006e236c in Feval_buffer lread.c:2416
>     #79 0x100653d24 in funcall_subr eval.c:3025
>     #80 0x10072e004 in exec_byte_code bytecode.c:809
>     #81 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #82 0x100654994 in funcall_lambda eval.c:3138
>     #83 0x10065316c in funcall_general eval.c:2929
>     #84 0x100647fcc in Ffuncall eval.c:2979
>     #85 0x1006ddfe8 in call4 lisp.h:3317
>     #86 0x1006d9058 in Fload lread.c:1483
>     #87 0x1006410e8 in eval_sub eval.c:2496
>     #88 0x10064234c in Fprogn eval.c:436
>     #89 0x100646c90 in Flet eval.c:1023
>     #90 0x1006403e0 in eval_sub eval.c:2435
>     #91 0x10064234c in Fprogn eval.c:436
>     #92 0x100654eb8 in funcall_lambda eval.c:3218
>     #93 0x100650834 in apply_lambda eval.c:3088
>     #94 0x100641c68 in eval_sub eval.c:2572
>     #95 0x1006f5394 in readevalloop_eager_expand_eval lread.c:2160
>     #96 0x1006e0b0c in readevalloop lread.c:2343
>     #97 0x1006e236c in Feval_buffer lread.c:2416
>     #98 0x100653d24 in funcall_subr eval.c:3025
>     #99 0x10072e004 in exec_byte_code bytecode.c:809
>     #100 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #101 0x100654994 in funcall_lambda eval.c:3138
>     #102 0x10065316c in funcall_general eval.c:2929
>     #103 0x100647fcc in Ffuncall eval.c:2979
>     #104 0x1006ddfe8 in call4 lisp.h:3317
>     #105 0x1006d9058 in Fload lread.c:1483
>     #106 0x100653d24 in funcall_subr eval.c:3025
>     #107 0x10072e004 in exec_byte_code bytecode.c:809
>     #108 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #109 0x100654994 in funcall_lambda eval.c:3138
>     #110 0x100650834 in apply_lambda eval.c:3088
>     #111 0x100641734 in eval_sub eval.c:2529
>     #112 0x10064efb0 in Feval eval.c:2345
>     #113 0x100451650 in top_level_2 keyboard.c:1141
>     #114 0x10064a318 in internal_condition_case eval.c:1471
>     #115 0x100451564 in top_level_1 keyboard.c:1149
>     #116 0x100648aa4 in internal_catch eval.c:1194
>     #117 0x100416f04 in command_loop keyboard.c:1109
>     #118 0x100416994 in recursive_edit_1 keyboard.c:719
>     #119 0x100417950 in Frecursive_edit keyboard.c:802
>     #120 0x10040fb00 in main emacs.c:2515
>     #121 0x101549088 in start+0x204 (dyld:arm64e+0x5088)
>
> That is redisplay during garbage_collect!

Yes, but that is redisplay after the main part of garbage_collect is
over: after that unblock_input, we even run finalizers (Lisp code)
straight from garbage_collect.

I'm going to guess that window_sub_list is returning a window that was
not marked during GC.  It's a problem that also exists with my
incremental garbage collector.  Does this help?

diff --git a/src/alloc.c b/src/alloc.c
index 419c5e558b..522925d248 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6634,6 +6634,9 @@ mark_window (struct Lisp_Vector *ptr)
       mark_glyph_matrix (w->desired_matrix);
     }
 
+  if (w->next)
+    mark_window (w->next);
+
   /* Filter out killed buffers from both buffer lists
      in attempt to help GC to reclaim killed buffers faster.
      We can do it elsewhere for live windows, but this is the

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Can somone please help me understand how this works?

Let's say we are in memq called for list L.  Fmemq uses FOR_EACH_TAIL,
which can call maybe_quit, which executes arbitrary Lisp, which can
modify L.  And probably similarly in another 100 places.

I don't get it.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> Can somone please help me understand how this works?
>
> Let's say we are in memq called for list L.  Fmemq uses FOR_EACH_TAIL,
> which can call maybe_quit, which executes arbitrary Lisp, which can
> modify L.  And probably similarly in another 100 places.
>
> I don't get it.

AFAIU if it is particularly dangerous to modify L there, then input
should be blocked around Fmemq.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Po Lu <luangruo@yahoo.com> writes:

> I'm going to guess that window_sub_list is returning a window that was
> not marked during GC.  It's a problem that also exists with my
> incremental garbage collector.  Does this help?
>
> diff --git a/src/alloc.c b/src/alloc.c
> index 419c5e558b..522925d248 100644
> --- a/src/alloc.c
> +++ b/src/alloc.c
> @@ -6634,6 +6634,9 @@ mark_window (struct Lisp_Vector *ptr)
>        mark_glyph_matrix (w->desired_matrix);
>      }
>  
> +  if (w->next)
> +    mark_window (w->next);
> +
>    /* Filter out killed buffers from both buffer lists
>       in attempt to help GC to reclaim killed buffers faster.
>       We can do it elsewhere for live windows, but this is the

Indeed, that seems to work!

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> Po Lu <luangruo@yahoo.com> writes:
>
>> I'm going to guess that window_sub_list is returning a window that was
>> not marked during GC.  It's a problem that also exists with my
>> incremental garbage collector.  Does this help?
>>
>> diff --git a/src/alloc.c b/src/alloc.c
>> index 419c5e558b..522925d248 100644
>> --- a/src/alloc.c
>> +++ b/src/alloc.c
>> @@ -6634,6 +6634,9 @@ mark_window (struct Lisp_Vector *ptr)
>>        mark_glyph_matrix (w->desired_matrix);
>>      }
>>  
>> +  if (w->next)
>> +    mark_window (w->next);
>> +
>>    /* Filter out killed buffers from both buffer lists
>>       in attempt to help GC to reclaim killed buffers faster.
>>       We can do it elsewhere for live windows, but this is the
>
> Indeed, that seems to work!

Right.  I've not had the time to investigate why unmarked windows remain
in the window tree, so I have the equivalent of that in my incremental
garbage collector.

I think there is an implicit assumption being made (for example, about a
list where all live windows are put) somewhere that is being broken, but
I haven't found where.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Po Lu <luangruo@yahoo.com> writes:

>> Indeed, that seems to work!
>
> Right.  I've not had the time to investigate why unmarked windows remain
> in the window tree, so I have the equivalent of that in my incremental
> garbage collector.

Thanks!  At least one GC-related bug less, if you put that in.

And with Eli's proposed patch 2.  I've added that to my local branch,
but it took long for the re_match case to re-appear, so maybe it should
be put into master as well, so that more people test if it has adverse
effects.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Po Lu <luangruo@yahoo.com> writes:

> AFAIU if it is particularly dangerous to modify L there, then input
> should be blocked around Fmemq.

Thanks!

(I'll pretend I don't know about this in the future :-).

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Po Lu <luangruo@yahoo.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Po Lu <luangruo@yahoo.com> writes:
>>
>>> I'm going to guess that window_sub_list is returning a window that was
>>> not marked during GC.  It's a problem that also exists with my
>>> incremental garbage collector.  Does this help?
>>>
>>> diff --git a/src/alloc.c b/src/alloc.c
>>> index 419c5e558b..522925d248 100644
>>> --- a/src/alloc.c
>>> +++ b/src/alloc.c
>>> @@ -6634,6 +6634,9 @@ mark_window (struct Lisp_Vector *ptr)
>>>        mark_glyph_matrix (w->desired_matrix);
>>>      }
>>>  
>>> +  if (w->next)
>>> +    mark_window (w->next);
>>> +
>>>    /* Filter out killed buffers from both buffer lists
>>>       in attempt to help GC to reclaim killed buffers faster.
>>>       We can do it elsewhere for live windows, but this is the
>>
>> Indeed, that seems to work!

In case it matters--I didn't mention that I actually used the change
below, because w->next is a Lisp_Object in master.

diff --git a/src/alloc.c b/src/alloc.c
index 419c5e558b..826ff1dba5 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6625,6 +6625,9 @@ mark_window (struct Lisp_Vector *ptr)
 
   mark_vectorlike (&ptr->header);
 
+  if (!NILP (w->next))
+    mark_object (w->next);
+
   /* Mark glyph matrices, if any.  Marking window
      matrices is sufficient because frame matrices
      use the same glyph memory.  */

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> Po Lu <luangruo@yahoo.com> writes:
>
>> I'm going to guess that window_sub_list is returning a window that was
>> not marked during GC.  It's a problem that also exists with my
>> incremental garbage collector.  Does this help?
>>
>> diff --git a/src/alloc.c b/src/alloc.c
>> index 419c5e558b..522925d248 100644
>> --- a/src/alloc.c
>> +++ b/src/alloc.c
>> @@ -6634,6 +6634,9 @@ mark_window (struct Lisp_Vector *ptr)
>>        mark_glyph_matrix (w->desired_matrix);
>>      }
>>  
>> +  if (w->next)
>> +    mark_window (w->next);
>> +
>>    /* Filter out killed buffers from both buffer lists
>>       in attempt to help GC to reclaim killed buffers faster.
>>       We can do it elsewhere for live windows, but this is the
>
> Indeed, that seems to work!

Could you please replace that code with:

  if (!NILP (w->next)
      && !vectorlike_marked_p (&XWINDOW (w->next)->header))
    emacs_abort ();

And see if Emacs ever aborts?

I just remembered that the old garbage collector does not work the same
way as the one in my branch, so that bug shouldn't be possible.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> In case it matters--I didn't mention that I actually used the change
> below, because w->next is a Lisp_Object in master.

I sent another reply to that message you should read.

w->next is struct window * in my branch, but in the ordinary garbage
collector mark_vectorlike should itself mark all fields between frame
and mode_line_help_echo.

So if that mechanism isn't working correctly, then we have a bigger
problem on hand.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Po Lu <luangruo@yahoo.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Po Lu <luangruo@yahoo.com> writes:
>>
>>> I'm going to guess that window_sub_list is returning a window that was
>>> not marked during GC.  It's a problem that also exists with my
>>> incremental garbage collector.  Does this help?
>>>
>>> diff --git a/src/alloc.c b/src/alloc.c
>>> index 419c5e558b..522925d248 100644
>>> --- a/src/alloc.c
>>> +++ b/src/alloc.c
>>> @@ -6634,6 +6634,9 @@ mark_window (struct Lisp_Vector *ptr)
>>>        mark_glyph_matrix (w->desired_matrix);
>>>      }
>>>  
>>> +  if (w->next)
>>> +    mark_window (w->next);
>>> +
>>>    /* Filter out killed buffers from both buffer lists
>>>       in attempt to help GC to reclaim killed buffers faster.
>>>       We can do it elsewhere for live windows, but this is the
>>
>> Indeed, that seems to work!
>
> Could you please replace that code with:
>
>   if (!NILP (w->next)
>       && !vectorlike_marked_p (&XWINDOW (w->next)->header))
>     emacs_abort ();
>
> And see if Emacs ever aborts?
>
> I just remembered that the old garbage collector does not work the same
> way as the one in my branch, so that bug shouldn't be possible.

With the change

diff --git a/src/alloc.c b/src/alloc.c
index 419c5e558b..4e0dd12729 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6625,6 +6625,15 @@ mark_window (struct Lisp_Vector *ptr)
 
   mark_vectorlike (&ptr->header);
 
+#if 1
+  if (!NILP (w->next)
+      && !vectorlike_marked_p (&XWINDOW (w->next)->header))
+    emacs_abort ();
+#else
+  if (!NILP (w->next))
+    mark_object (w->next);
+#endif
+
   /* Mark glyph matrices, if any.  Marking window
      matrices is sufficient because frame matrices
      use the same glyph memory.  */

I don't get an abort, but the ASAN error again

==67682==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a481c bp 0x00016fdcc3c0 sp 0x00016fdcc3b8
READ of size 8 at 0x000107130d00 thread T0
    #0 0x1002a4818 in PSEUDOVECTORP lisp.h:1110
    #1 0x1002a4888 in SYMBOL_WITH_POS_P lisp.h:1122
    #2 0x10025a338 in EQ lisp.h:1342
    #3 0x100280eb0 in run_window_change_functions window.c:3964
    #4 0x1000f18c4 in redisplay_internal xdisp.c:16600
    #5 0x100107bf8 in redisplay xdisp.c:16111
    #6 0x10089364c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
    #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
    #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*,
    double, double*)+0x1c0 (QuartzCore:arm6

frame #8: 0x0000000100280eb4 emacs`run_window_change_functions at window.c:3964:7
   3961		     (de-)selected as its frame's or the globally selected
   3962		     window.  */
   3963		  if (((frame_selected_change
-> 3964			&& (EQ (window, old_selected_window)
   3965			    || EQ (window, selected_window)))
   3966		       || (frame_selected_window_change
   3967			   && (EQ (window, FRAME_OLD_SELECTED_WINDOW (f))

(lldb) p window
(Lisp_Object) $18 = 0x00000001071c2935 (struct window *) $23 = 0x00000001071c2930
(lldb) p old_selected_window
(Lisp_Object) $24 = 0x0000000107130d05 (struct Lisp_Vector *) $28 = 0x0000000107130d00

old_selected_window looks strange.  It's a global that is not
staticpro'd

\o/

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

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

> old_selected_window looks strange.  It's a global that is not
> staticpro'd

And with this it works again:

diff --git a/src/window.c b/src/window.c
index 12a212a85a..da80fabe33 100644
--- a/src/window.c
+++ b/src/window.c
@@ -8213,6 +8213,8 @@ init_window_once (void)
 
   minibuf_selected_window = Qnil;
   staticpro (&minibuf_selected_window);
+  old_selected_window = Qnil;
+  staticpro (&old_selected_window);
 
   pdumper_do_now_and_after_late_load (init_window_once_for_pdumper);
 }

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> I don't get an abort, but the ASAN error again

Interesting.

> ==67682==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a481c bp 0x00016fdcc3c0 sp 0x00016fdcc3b8
> READ of size 8 at 0x000107130d00 thread T0
>     #0 0x1002a4818 in PSEUDOVECTORP lisp.h:1110
>     #1 0x1002a4888 in SYMBOL_WITH_POS_P lisp.h:1122
>     #2 0x10025a338 in EQ lisp.h:1342
>     #3 0x100280eb0 in run_window_change_functions window.c:3964
>     #4 0x1000f18c4 in redisplay_internal xdisp.c:16600
>     #5 0x100107bf8 in redisplay xdisp.c:16111
>     #6 0x10089364c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
>     #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>     #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*,
>     double, double*)+0x1c0 (QuartzCore:arm6
>
> frame #8: 0x0000000100280eb4 emacs`run_window_change_functions at window.c:3964:7
>    3961		     (de-)selected as its frame's or the globally selected
>    3962		     window.  */
>    3963		  if (((frame_selected_change
> -> 3964			&& (EQ (window, old_selected_window)
>    3965			    || EQ (window, selected_window)))
>    3966		       || (frame_selected_window_change
>    3967			   && (EQ (window, FRAME_OLD_SELECTED_WINDOW (f))
>
> (lldb) p window
> (Lisp_Object) $18 = 0x00000001071c2935 (struct window *) $23 = 0x00000001071c2930
> (lldb) p old_selected_window
> (Lisp_Object) $24 = 0x0000000107130d05 (struct Lisp_Vector *) $28 = 0x0000000107130d00
>
> old_selected_window looks strange.  It's a global that is not
> staticpro'd

Isn't old_selected_window supposed to be kept in sync with
FRAME_OLD_SELECTED_WINDOW in old_selected_frame, with the latter being
removed once it is deleted?

Would someone who knows the window code well please take a look at this?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> And with this it works again:
>
> diff --git a/src/window.c b/src/window.c
> index 12a212a85a..da80fabe33 100644
> --- a/src/window.c
> +++ b/src/window.c
> @@ -8213,6 +8213,8 @@ init_window_once (void)
>  
>    minibuf_selected_window = Qnil;
>    staticpro (&minibuf_selected_window);
> +  old_selected_window = Qnil;
> +  staticpro (&old_selected_window);
>  
>    pdumper_do_now_and_after_late_load (init_window_once_for_pdumper);
>  }

Right, but please see what I said about old_selected_frame; I think it
is intentionally not staticpro'd.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: 58042@debbugs.gnu.org, Alan Third <alan@idiocy.org>
> Date: Wed, 05 Oct 2022 12:14:04 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So I guess we should do this dance around calls to maybe_quit in
> > regex-emacs.c:
> >
> >   specpdl_ref gc_count = inhibit_garbage_collection ();
> >   maybe_quit ();
> >   unbind_to (gc_count, Qnil);
> >
> > Or maybe even better, do this inside probably_quit (because who knows
> > how many other callers of maybe_quit could be hit by this unexpected
> > GC)?
> >
> > Can you try this?
> 
> Isn't the -[EmacsView layoutSublayersOfLayer:] the problem?  AFAICT from
> a web search, this is an event handler method that is also called from
> by the framework?
> 
> In the olden days, it was a serious error to call into Lisp from an
> event handler.  All bets were off when that happened, not only related
> to GC.  I believe that hasn't changed much.
> 
> That code was introduced by Alan around this time.
> 
> 1ba02d85a964e1b2c6a9735cd3decdc524e06dc1
> Author:     Alan Third <alan@idiocy.org>
> AuthorDate: Sat Jun 12 10:25:47 2021 +0100
> Commit:     Alan Third <alan@idiocy.org>
> CommitDate: Sat Jul 31 11:13:05 2021 +0100
> 
> Maybe Allen can say something, I've CC'd him.

AFAIR, this was the best way Alan could fix display problems on
macOS.  He tried several other approaches, and all of them were worse.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Po Lu <luangruo@yahoo.com>,  Alan Third <alan@idiocy.org>,
>   58042@debbugs.gnu.org
> Date: Wed, 05 Oct 2022 12:45:04 +0200
> 
> And an update to the second ASAN error that I could actually reproduce
> by starting Emacs on my branch:
> 
> ==64010==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a48d8 bp 0x00016fdcaa80 sp 0x00016fdcaa78
> READ of size 8 at 0x000107130d00 thread T0
>     #0 0x1002a48d4 in PSEUDOVECTORP lisp.h:1110
>     #1 0x1002a4944 in SYMBOL_WITH_POS_P lisp.h:1122
>     #2 0x10025a3f4 in EQ lisp.h:1342
>     #3 0x100280f6c in run_window_change_functions window.c:3964
>     #4 0x1000f1980 in redisplay_internal xdisp.c:16600
>     #5 0x100107cb4 in redisplay xdisp.c:16111
>     #6 0x10089364c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
>     #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>     #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*, double, double*)+0x1c0 (QuartzCore:arm64e+0x16d61c)
>     #9 0x19008b4c8 in CA::Transaction::commit()+0x2bc (QuartzCore:arm64e+0x24c8)
>     #10 0x18bee1698 in __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke+0x12c (AppKit:arm64e+0x1ac698)
>     #11 0x18c646754 in ___NSRunLoopObserverCreateWithHandler_block_invoke+0x3c (AppKit:arm64e+0x911754)
>     #12 0x1892101a0 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__+0x20 (CoreFoundation:arm64e+0x841a0)
>     #13 0x18920fff0 in __CFRunLoopDoObservers+0x24c (CoreFoundation:arm64e+0x83ff0)
>     #14 0x18920f524 in __CFRunLoopRun+0x300 (CoreFoundation:arm64e+0x83524)
>     #15 0x18920ea80 in CFRunLoopRunSpecific+0x254 (CoreFoundation:arm64e+0x82a80)
>     #16 0x191e4e334 in RunCurrentEventLoopInMode+0x120 (HIToolbox:arm64e+0x32334)
>     #17 0x191e4dfc0 in ReceiveNextEventCommon+0x140 (HIToolbox:arm64e+0x31fc0)
>     #18 0x191e4de64 in _BlockUntilNextEventMatchingListInModeWithFilter+0x44 (HIToolbox:arm64e+0x31e64)
>     #19 0x18bd76518 in _DPSNextEvent+0x358 (AppKit:arm64e+0x41518)
>     #20 0x18bd74e10 in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]+0x52c (AppKit:arm64e+0x3fe10)
>     #21 0x18bd66fdc in -[NSApplication run]+0x250 (AppKit:arm64e+0x31fdc)
>     #22 0x100870bb0 in -[EmacsApp run] nsterm.m:5799
>     #23 0x1008c7b0c in ns_read_socket_1 nsterm.m:4679
>     #24 0x1008ae530 in ns_read_socket nsterm.m:4697
>     #25 0x100437168 in gobble_input keyboard.c:7379
>     #26 0x1004389d0 in handle_async_input keyboard.c:7610
>     #27 0x1004389b0 in process_pending_signals keyboard.c:7624
>     #28 0x100438acc in unblock_input_to keyboard.c:7639
>     #29 0x100432cac in unblock_input keyboard.c:7658
>     #30 0x1005ba024 in garbage_collect alloc.c:6256
>     #31 0x1005b950c in maybe_garbage_collect alloc.c:6090
>     #32 0x10064f6a8 in maybe_gc lisp.h:5622
>     #33 0x10063fcfc in eval_sub eval.c:2388
>     #34 0x100640838 in eval_sub eval.c:2449
>     #35 0x10064234c in Fprogn eval.c:436
>     #36 0x100654eb8 in funcall_lambda eval.c:3218
>     #37 0x1006532c4 in funcall_general eval.c:2941
>     #38 0x100647fcc in Ffuncall eval.c:2979
>     #39 0x100651ca8 in Fapply eval.c:2650
>     #40 0x10063ead8 in apply1 eval.c:2866
>     #41 0x1006484bc in Fmacroexpand eval.c:1149
>     #42 0x10065394c in funcall_subr eval.c:3019
>     #43 0x10072e004 in exec_byte_code bytecode.c:809
>     #44 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #45 0x100654994 in funcall_lambda eval.c:3138
>     #46 0x10065316c in funcall_general eval.c:2929
>     #47 0x100647fcc in Ffuncall eval.c:2979
>     #48 0x1006fcd80 in call2 lisp.h:3302
>     #49 0x1006f4ecc in readevalloop_eager_expand_eval lread.c:2151
>     #50 0x1006e0b0c in readevalloop lread.c:2343
>     #51 0x1006e236c in Feval_buffer lread.c:2416
>     #52 0x100653d24 in funcall_subr eval.c:3025
>     #53 0x10072e004 in exec_byte_code bytecode.c:809
>     #54 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #55 0x100654994 in funcall_lambda eval.c:3138
>     #56 0x10065316c in funcall_general eval.c:2929
>     #57 0x100647fcc in Ffuncall eval.c:2979
>     #58 0x1006ddfe8 in call4 lisp.h:3317
>     #59 0x1006d9058 in Fload lread.c:1483
>     #60 0x1006e1158 in save_match_data_load lread.c:1636
>     #61 0x10064e9c8 in load_with_autoload_queue eval.c:2271
>     #62 0x10067cb40 in Frequire fns.c:3308
>     #63 0x100653a54 in funcall_subr eval.c:3021
>     #64 0x10072e004 in exec_byte_code bytecode.c:809
>     #65 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #66 0x100654994 in funcall_lambda eval.c:3138
>     #67 0x10065316c in funcall_general eval.c:2929
>     #68 0x100647fcc in Ffuncall eval.c:2979
>     #69 0x100651ca8 in Fapply eval.c:2650
>     #70 0x100654414 in funcall_subr eval.c:3044
>     #71 0x10072e004 in exec_byte_code bytecode.c:809
>     #72 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #73 0x100654994 in funcall_lambda eval.c:3138
>     #74 0x100650834 in apply_lambda eval.c:3088
>     #75 0x100641734 in eval_sub eval.c:2529
>     #76 0x1006f5394 in readevalloop_eager_expand_eval lread.c:2160
>     #77 0x1006e0b0c in readevalloop lread.c:2343
>     #78 0x1006e236c in Feval_buffer lread.c:2416
>     #79 0x100653d24 in funcall_subr eval.c:3025
>     #80 0x10072e004 in exec_byte_code bytecode.c:809
>     #81 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #82 0x100654994 in funcall_lambda eval.c:3138
>     #83 0x10065316c in funcall_general eval.c:2929
>     #84 0x100647fcc in Ffuncall eval.c:2979
>     #85 0x1006ddfe8 in call4 lisp.h:3317
>     #86 0x1006d9058 in Fload lread.c:1483
>     #87 0x1006410e8 in eval_sub eval.c:2496
>     #88 0x10064234c in Fprogn eval.c:436
>     #89 0x100646c90 in Flet eval.c:1023
>     #90 0x1006403e0 in eval_sub eval.c:2435
>     #91 0x10064234c in Fprogn eval.c:436
>     #92 0x100654eb8 in funcall_lambda eval.c:3218
>     #93 0x100650834 in apply_lambda eval.c:3088
>     #94 0x100641c68 in eval_sub eval.c:2572
>     #95 0x1006f5394 in readevalloop_eager_expand_eval lread.c:2160
>     #96 0x1006e0b0c in readevalloop lread.c:2343
>     #97 0x1006e236c in Feval_buffer lread.c:2416
>     #98 0x100653d24 in funcall_subr eval.c:3025
>     #99 0x10072e004 in exec_byte_code bytecode.c:809
>     #100 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #101 0x100654994 in funcall_lambda eval.c:3138
>     #102 0x10065316c in funcall_general eval.c:2929
>     #103 0x100647fcc in Ffuncall eval.c:2979
>     #104 0x1006ddfe8 in call4 lisp.h:3317
>     #105 0x1006d9058 in Fload lread.c:1483
>     #106 0x100653d24 in funcall_subr eval.c:3025
>     #107 0x10072e004 in exec_byte_code bytecode.c:809
>     #108 0x10065c16c in fetch_and_exec_byte_code eval.c:3066
>     #109 0x100654994 in funcall_lambda eval.c:3138
>     #110 0x100650834 in apply_lambda eval.c:3088
>     #111 0x100641734 in eval_sub eval.c:2529
>     #112 0x10064efb0 in Feval eval.c:2345
>     #113 0x100451650 in top_level_2 keyboard.c:1141
>     #114 0x10064a318 in internal_condition_case eval.c:1471
>     #115 0x100451564 in top_level_1 keyboard.c:1149
>     #116 0x100648aa4 in internal_catch eval.c:1194
>     #117 0x100416f04 in command_loop keyboard.c:1109
>     #118 0x100416994 in recursive_edit_1 keyboard.c:719
>     #119 0x100417950 in Frecursive_edit keyboard.c:802
>     #120 0x10040fb00 in main emacs.c:2515
>     #121 0x101549088 in start+0x204 (dyld:arm64e+0x5088)
> 
> That is redisplay during garbage_collect!
> 
> The change to probably_quit didn't help.  Commenting out the call to
> redisplay in the layout stuff did.

I don't think I understand what this diagnostics says.  The backtrace
tells us that Emacs performed GC, then called unblock_input, which
called gobble_input, which on NS triggers redisplay.  So far I see no
problem; do you?

Then redisplay called run_window_change_functions, which attempted to
compare some window with old_selected_window, and one of these two
(which one?) was found to be in freed heap memory?  Why? because one
of these two windows is not a live window anymore?  So we should
sprinkle more WINDOW_LIVE_P tests in that loop in
run_window_change_functions?

This has nothing per se to do with GC and with redisplay being run
from the unblock_input, this can happen regardless of these.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

On 22-10-05 15:13 , Eli Zaretskii wrote:
>>>
>> That is redisplay during garbage_collect!
>>
>> The change to probably_quit didn't help.  Commenting out the call to
>> redisplay in the layout stuff did.
> 
> This has nothing per se to do with GC and with redisplay being run
> from the unblock_input, this can happen regardless of these.

Right, this is a different problem.  See later in the thread(s).

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  58042@debbugs.gnu.org,  Alan Third
>  <alan@idiocy.org>
> Date: Wed, 05 Oct 2022 13:10:03 +0200
> 
> Can somone please help me understand how this works?
> 
> Let's say we are in memq called for list L.  Fmemq uses FOR_EACH_TAIL,
> which can call maybe_quit, which executes arbitrary Lisp, which can
> modify L.

"Arbitrary Lisp" being redisplay that calls various hooks, like
window-configuration-change-hook etc.?  IOW, this is a macOS only
thing?

Perhaps on macOS probably_quit should bind inhibit_redisplay non-zero?
I see no particular reason to trigger redisplay from maybe_quit.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

On 22-10-05 15:27 , Eli Zaretskii wrote:
>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  58042@debbugs.gnu.org,  Alan Third
>>   <alan@idiocy.org>
>> Date: Wed, 05 Oct 2022 13:10:03 +0200
>>
>> Can somone please help me understand how this works?
>>
>> Let's say we are in memq called for list L.  Fmemq uses FOR_EACH_TAIL,
>> which can call maybe_quit, which executes arbitrary Lisp, which can
>> modify L.
> 
> "Arbitrary Lisp" being redisplay that calls various hooks, like
> window-configuration-change-hook etc.?  IOW, this is a macOS only
> thing?

I don't know.  What Po Lu said sounded to me like it isn't specific to 
macOS (safe_call in event handlers, IIRC).

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  58042@debbugs.gnu.org,  Alan Third
>  <alan@idiocy.org>
> Date: Wed, 05 Oct 2022 19:15:22 +0800
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Can somone please help me understand how this works?
> >
> > Let's say we are in memq called for list L.  Fmemq uses FOR_EACH_TAIL,
> > which can call maybe_quit, which executes arbitrary Lisp, which can
> > modify L.  And probably similarly in another 100 places.
> >
> > I don't get it.
> 
> AFAIU if it is particularly dangerous to modify L there, then input
> should be blocked around Fmemq.

How do you know whether it's "particularly dangerous"?

We call maybe_quit in many places, basically anywhere where we have
potentially long loops.  It isn't just Fmemq.  So if we want to
prevent maybe_quit from indirectly calling arbitrary Lisp, we'd need
to block_input inside probably_quit.  Which means
process_pending_signals will not call the read-socket hook and will
not gobble input.  That's bad, I think.

And note that this is only problematic on macOS (AFAIU), because there
the read-socket hook can trigger redisplay.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Alan Third <alan@idiocy.org>,
>   58042@debbugs.gnu.org
> Date: Wed, 05 Oct 2022 19:10:02 +0800
> 
> I'm going to guess that window_sub_list is returning a window that was
> not marked during GC.

Why wasn't it marked?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Alan Third <alan@idiocy.org>,
>   58042@debbugs.gnu.org
> Date: Wed, 05 Oct 2022 19:23:53 +0800
> 
> >> +  if (w->next)
> >> +    mark_window (w->next);
> >> +
> >>    /* Filter out killed buffers from both buffer lists
> >>       in attempt to help GC to reclaim killed buffers faster.
> >>       We can do it elsewhere for live windows, but this is the
> >
> > Indeed, that seems to work!
> 
> Right.  I've not had the time to investigate why unmarked windows remain
> in the window tree

Maybe those windows were deleted?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> We call maybe_quit in many places, basically anywhere where we have
> potentially long loops.  It isn't just Fmemq.  So if we want to
> prevent maybe_quit from indirectly calling arbitrary Lisp, we'd need
> to block_input inside probably_quit.  Which means
> process_pending_signals will not call the read-socket hook and will
> not gobble input.  That's bad, I think.
>
> And note that this is only problematic on macOS (AFAIU), because there
> the read-socket hook can trigger redisplay.

There are many different ways to trigger redisplay from the read-socket
hook in the Haiku port as well, and I haven't seen any problems there.

Besides, any call to automatic GC today can run arbitrary Lisp through
finalizer functions, and that includes redisplay.  So unless the
read_socket_hook does not cons at all, there is no way to prevent
probably_quit from running Lisp code.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Maybe those windows were deleted?

No, it turns out to be a different problem, as Gerd found out
down-thread.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> I don't know.  What Po Lu said sounded to me like it isn't specific to
> macOS (safe_call in event handlers, IIRC).

Yes, and there are also finalizer functions called by automatic GC.
So unless read_socket_hook does not cons at all there is no way to stop
it from running random Lisp.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: gerd.moellmann@gmail.com,  58042@debbugs.gnu.org,  alan@idiocy.org
> Date: Wed, 05 Oct 2022 21:52:52 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > We call maybe_quit in many places, basically anywhere where we have
> > potentially long loops.  It isn't just Fmemq.  So if we want to
> > prevent maybe_quit from indirectly calling arbitrary Lisp, we'd need
> > to block_input inside probably_quit.  Which means
> > process_pending_signals will not call the read-socket hook and will
> > not gobble input.  That's bad, I think.
> >
> > And note that this is only problematic on macOS (AFAIU), because there
> > the read-socket hook can trigger redisplay.
> 
> There are many different ways to trigger redisplay from the read-socket
> hook in the Haiku port as well, and I haven't seen any problems there.
> 
> Besides, any call to automatic GC today can run arbitrary Lisp through
> finalizer functions, and that includes redisplay.  So unless the
> read_socket_hook does not cons at all, there is no way to prevent
> probably_quit from running Lisp code.

That we have other loopholes doesn't mean we shouldn't be concerned
with this one.  IMO, we should plug all those loopholes one by one.

Finalizers are very rarely used (not at all in core, I believe), so
it's a small wonder we didn't see bug reports.  As for Haiku, how man
y active users of it exist, and how "crazy" are the hooks they define
for redisplay to call?  If those hooks remain nil, nothing bad will
ever happen.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: gerd.moellmann@gmail.com,  alan@idiocy.org,  58042@debbugs.gnu.org
> Date: Wed, 05 Oct 2022 21:53:26 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Maybe those windows were deleted?
> 
> No, it turns out to be a different problem, as Gerd found out
> down-thread.

Are you sure?  GC can only remove dead windows, no?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On October 5, 2022 10:09:09 PM GMT+08:00, Eli Zaretskii <eliz@gnu.org> wrote:

>That we have other loopholes doesn't mean we shouldn't be concerned
>with this one.  IMO, we should plug all those loopholes one by one.

Judging by how long the NS relayout code has been installed for, and how it has not actually caused problems in Fmemq, I'm inclined to wait for someone to complain about memq not working before we remove it.  I tried several months ago, and removing that call to redisplay resulted in the system refusing to resize the NS window.

The call to redisplay in the drag and drop code should not cause any problems, as that hook cannot be called from ns_read_socket.

Thanks.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Po Lu <luangruo@yahoo.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> I don't get an abort, but the ASAN error again
>
> Interesting.
>
>> ==67682==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a481c bp 0x00016fdcc3c0 sp 0x00016fdcc3b8
>> READ of size 8 at 0x000107130d00 thread T0
>>     #0 0x1002a4818 in PSEUDOVECTORP lisp.h:1110
>>     #1 0x1002a4888 in SYMBOL_WITH_POS_P lisp.h:1122
>>     #2 0x10025a338 in EQ lisp.h:1342
>>     #3 0x100280eb0 in run_window_change_functions window.c:3964
>>     #4 0x1000f18c4 in redisplay_internal xdisp.c:16600
>>     #5 0x100107bf8 in redisplay xdisp.c:16111
>>     #6 0x10089364c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
>>     #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>>     #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*,
>>     double, double*)+0x1c0 (QuartzCore:arm6
>>
>> frame #8: 0x0000000100280eb4 emacs`run_window_change_functions at window.c:3964:7
>>    3961		     (de-)selected as its frame's or the globally selected
>>    3962		     window.  */
>>    3963		  if (((frame_selected_change
>> -> 3964			&& (EQ (window, old_selected_window)
>>    3965			    || EQ (window, selected_window)))
>>    3966		       || (frame_selected_window_change
>>    3967			   && (EQ (window, FRAME_OLD_SELECTED_WINDOW (f))
>>
>> (lldb) p window
>> (Lisp_Object) $18 = 0x00000001071c2935 (struct window *) $23 = 0x00000001071c2930
>> (lldb) p old_selected_window
>> (Lisp_Object) $24 = 0x0000000107130d05 (struct Lisp_Vector *) $28 = 0x0000000107130d00
>>
>> old_selected_window looks strange.  It's a global that is not
>> staticpro'd
>
> Isn't old_selected_window supposed to be kept in sync with
> FRAME_OLD_SELECTED_WINDOW in old_selected_frame, with the latter being
> removed once it is deleted?
>
> Would someone who knows the window code well please take a look at this?

I've submitted bug#58327 for this problem, so that it won't be
forgotten.  (I'm sure I will forget it at some point, because I have
added the staticpro in my local branch.)

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Can we come to a decision about what to do with probably_quit, based
what we know now?  The threads under this bug are a bit deep and
complicated, so I'd like to make this a bit more visible.

I think the problem has been analyized to be:

1. The re_matcher uses char* pointer P into data of string S.
2. The re_matcher uses maybe_quit
3. maybe_quit can call garbage_collect
4. garbage_collect can call Lisp (finalizers, redisplay)
(4a. That Lisp can again garbage_collect)
5. One of the GCs can relocate the string data of S in step 1.
6. P is then invalid.

Possible solution:

Inhibit GC in probably_quit, so that P remains valid.

Q: Should we do that?  And if so, when?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Thu, 06 Oct 2022 07:35:26 +0200
> 
> Can we come to a decision about what to do with probably_quit, based
> what we know now?  The threads under this bug are a bit deep and
> complicated, so I'd like to make this a bit more visible.
> 
> I think the problem has been analyized to be:
> 
> 1. The re_matcher uses char* pointer P into data of string S.
> 2. The re_matcher uses maybe_quit
> 3. maybe_quit can call garbage_collect
> 4. garbage_collect can call Lisp (finalizers, redisplay)
> (4a. That Lisp can again garbage_collect)
> 5. One of the GCs can relocate the string data of S in step 1.
> 6. P is then invalid.
> 
> Possible solution:
> 
> Inhibit GC in probably_quit, so that P remains valid.
> 
> Q: Should we do that?

IMO, yes.

> And if so, when?

"Now"?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

On 22-10-06 8:59 , Eli Zaretskii wrote:
>>>> Q: Should we do that?
> 
> IMO, yes. >
>> And if so, when?
> 
> "Now"?

Done in master.  Thanks.

Not sure what to do with redisplay, or Lisp in general being called from 
event handlers.

I tend to close this bug, and submit a new one, ATM.  Maybe we should at 
least think a bit more about what the consequences of that are.  Or 
maybe not, because it seems to "work well enough", and I wouldn't know 
what to do about this anyway.

Any preferences?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> Date: Thu, 6 Oct 2022 09:21:49 +0200
> Cc: 58042@debbugs.gnu.org
> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> 
> Not sure what to do with redisplay, or Lisp in general being called from 
> event handlers.

I think the conclusion was that block_input is the solution, and
should be used where necessary.

> I tend to close this bug, and submit a new one, ATM.  Maybe we should at 
> least think a bit more about what the consequences of that are.  Or 
> maybe not, because it seems to "work well enough", and I wouldn't know 
> what to do about this anyway.
> 
> Any preferences?

A new bug or maybe even nothing.  Because we don't have any ideas for
how to solve such a bug in general.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> A new bug or maybe even nothing.  Because we don't have any ideas for
> how to solve such a bug in general.

Right.  I'll close this bug then.

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> Isn't the -[EmacsView layoutSublayersOfLayer:] the problem?  AFAICT from
>>> a web search, this is an event handler method that is also called from
>>> by the framework?
>>>
>>> In the olden days, it was a serious error to call into Lisp from an
>>> event handler.  All bets were off when that happened, not only related
>>> to GC.  I believe that hasn't changed much.
>
> Today, event handling code calls Lisp all the time (through safe_call
> etc.)  That happens in handle_one_xevent, ns_select, et cetera.

Really?

> It shouldn't affect GC at all because input is blocked for the entire
> duration of each GC, except for when finalizers are run after unmarked
> objects are sweeped.

The problem was not if it's run from within the GC, the problem was what
this code does when *it* runs the GC (or other state-changing functions).
[ And indeed, the fix Gerd installed was to prevent GC while running
  pending_signals.  But I suspect this is not sufficient because there
  are other forms of global state that can get messed up.  ]

In bug#62732 we have a related problem when code run from `maybe_quit`
(an atimer in that case) from the regexp engine, and that atimer
itself performs a regexp-operation, which messes up the outer regexp
engine invocation because the regexp engine is still not re-entrant (in
that bug, the problem is the `gl_state` global variable).


        Stefan

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Really?

Yes.

> The problem was not if it's run from within the GC, the problem was what
> this code does when *it* runs the GC (or other state-changing functions).
> [ And indeed, the fix Gerd installed was to prevent GC while running
>   pending_signals.  But I suspect this is not sufficient because there
>   are other forms of global state that can get messed up.  ]
>
> In bug#62732 we have a related problem when code run from `maybe_quit`
> (an atimer in that case) from the regexp engine, and that atimer
> itself performs a regexp-operation, which messes up the outer regexp
> engine invocation because the regexp engine is still not re-entrant (in
> that bug, the problem is the `gl_state` global variable).

bug#62732?  That's:

  29.0.60; uniquify-trailing-separator-p affects any buffer whose name
  matches a dir in CWD

I don't see how it's related to reentrant use of the regexp engine.
BTW, which atimer is it?

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Really?
> Yes.

Damn!  I thought at least `handle_one_xevent` was "ELisp-clean".

>> In bug#62732 we have a related problem when code run from `maybe_quit`
>> (an atimer in that case) from the regexp engine, and that atimer
>> itself performs a regexp-operation, which messes up the outer regexp
>> engine invocation because the regexp engine is still not re-entrant (in
>> that bug, the problem is the `gl_state` global variable).
>
> bug#62732?  That's:

Hmm... not sure how I ended up writing this.  I meant bug#63253
Sorry 'bout that.

> I don't see how it's related to reentrant use of the regexp engine.
> BTW, which atimer is it?

The atimer for `with-delayed-message`.


        Stefan

bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>,  Eli
>  Zaretskii <eliz@gnu.org>,
>   58042@debbugs.gnu.org,  Alan Third <alan@idiocy.org>
> Date: Tue, 09 May 2023 09:04:03 +0800
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > Really?
> 
> Yes.
> 
> > The problem was not if it's run from within the GC, the problem was what
> > this code does when *it* runs the GC (or other state-changing functions).
> > [ And indeed, the fix Gerd installed was to prevent GC while running
> >   pending_signals.  But I suspect this is not sufficient because there
> >   are other forms of global state that can get messed up.  ]
> >
> > In bug#62732 we have a related problem when code run from `maybe_quit`
> > (an atimer in that case) from the regexp engine, and that atimer
> > itself performs a regexp-operation, which messes up the outer regexp
> > engine invocation because the regexp engine is still not re-entrant (in
> > that bug, the problem is the `gl_state` global variable).
> 
> bug#62732?

He meant bug#63253, I think.