On 7/7/20 1:25 AM, Pedro Alves wrote:

> However, with the fix, the testcase now runs into another Asan-reported issue:
> 
> =================================================================
> ==9211==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000b1080 at pc 0x000000dd0344 bp 0x7ffe4bebca90 sp 0x7ffe4bebca80
> READ of size 4 at 0x6160000b1080 thread T0
>     #0 0xdd0343 in refcounted_object::incref() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/../gdbsupport/refcounted-object.h:34
>     #1 0x150066f in scoped_restore_current_thread::scoped_restore_current_thread() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:1471
>     #2 0xf83564 in fetch_inferior_event() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/infrun.c:3952
>     #3 0xf40736 in inferior_event_handler(inferior_event_type) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inf-loop.c:42
>     #4 0x1321e75 in remote_async_serial_handler /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/remote.c:14160
>     #5 0x1371849 in run_async_handler_and_reschedule /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/ser-base.c:137
>     #6 0x1371ae2 in fd_event /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/ser-base.c:188
>     #7 0x19fc1f7 in handle_file_event /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdbsupport/event-loop.cc:548
>     #8 0x19fca14 in gdb_wait_for_event /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdbsupport/event-loop.cc:673
>     #9 0x19fab94 in gdb_do_one_event() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdbsupport/event-loop.cc:215
>     #10 0x1087704 in start_event_loop /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:356
>     #11 0x10879f5 in captured_command_loop /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:416
>     #12 0x108aef1 in captured_main /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:1253
>     #13 0x108af81 in gdb_main(captured_main_args*) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:1268
>     #14 0x8af9fa in main /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/gdb.c:32
>     #15 0x7f07d5efbfe9 in __libc_start_main (/lib64/libc.so.6+0x20fe9)
>     #16 0x8af809 in _start (/home/pedro/brno/pedro/gdb/binutils-gdb-2/build-asan/gdb/gdb+0x8af809)
> 
> 0x6160000b1080 is located 0 bytes inside of 592-byte region [0x6160000b1080,0x6160000b12d0)
> freed by thread T0 here:
>     #0 0x7f07d97966d8 in operator delete(void*, unsigned long) (/lib64/libasan.so.4+0xe16d8)
>     #1 0x14f7147 in delete_thread_1 /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:452
>     #2 0x14f7171 in delete_thread(thread_info*) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:460
>     #3 0xf6278d in exit_inferior_1 /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inferior.c:204
>     #4 0xf62b5b in exit_inferior(inferior*) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inferior.c:236
>     #5 0x14c32a6 in generic_mourn_inferior() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/target.c:3119
>     #6 0x12f388a in remote_unpush_target /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/remote.c:5523
>     #7 0x1309835 in remote_target::readchar(int) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/remote.c:9137
> 
> 
> Odd, kind of looks like we're mishandling the thread_info refcounts.
> 

This fixes it.  multi-target.exp now passes Asan-clean with this one on top
of "part 1" patch.  I still haven't run the full testsuite.

The issue is that the remote target is unpushed while within scoped_restore_current_thread' dtor's
get_frame_id call, which results in threads being deleted.  However, back in
scoped_restore_current_thread's ctor, we only increment the refcount after get_frame_id
returns.  Incrementing the refcounts earlier fixes it.

However, we should probably also propagate the TARGET_CLOSE_ERROR in this case.
That alone would fix it, though it seems cleaner to do both tweaks.