Fix GDBserver regression due to change to avoid reading shell registers

Simon reported that the recent change to make GDB and GDBserver avoid
reading shell registers caused a GDBserver regression, caught with
ASan while running gdb.server/non-existing-program.exp:

 $ /home/smarchi/build/binutils-gdb/gdb/testsuite/../../gdb/../gdbserver/gdbserver stdio non-existing-program
 =================================================================
 ==127719==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f0000000e9 at pc 0x55bcbfa301f4 bp 0x7ffd238a7320 sp 0x7ffd238a7310
 WRITE of size 1 at 0x60f0000000e9 thread T0
      0x55bcbfa301f3 in scoped_restore_tmpl<bool>::~scoped_restore_tmpl() /home/smarchi/src/binutils-gdb/gdbserver/../gdbsupport/scoped_restore.h:86
      0x55bcbfa2ffe9 in post_fork_inferior(int, char const*) /home/smarchi/src/binutils-gdb/gdbserver/fork-child.cc:120
      0x55bcbf9c9199 in linux_process_target::create_inferior(char const*, std::__debug::vector<char*, std::allocator<char*> > const&) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:991
      0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3941
      0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4084
      0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2)
      0x55bcbf8ef2bd in _start (/home/smarchi/build/binutils-gdb/gdbserver/gdbserver+0x1352bd)

 0x60f0000000e9 is located 169 bytes inside of 176-byte region [0x60f000000040,0x60f0000000f0)
 freed by thread T0 here:
      0x7ff9d6c6f0c7 in operator delete(void*) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:160
      0x55bcbf910d00 in remove_process(process_info*) /home/smarchi/src/binutils-gdb/gdbserver/inferiors.cc:164
      0x55bcbf9c4ac7 in linux_process_target::remove_linux_process(process_info*) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:454
      0x55bcbf9cdaa6 in linux_process_target::mourn(process_info*) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:1599
      0x55bcbf988dc4 in target_mourn_inferior(ptid_t) /home/smarchi/src/binutils-gdb/gdbserver/target.cc:205
      0x55bcbfa32020 in startup_inferior(process_stratum_target*, int, int, target_waitstatus*, ptid_t*) /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/fork-inferior.c:515
      0x55bcbfa2fdeb in post_fork_inferior(int, char const*) /home/smarchi/src/binutils-gdb/gdbserver/fork-child.cc:111
      0x55bcbf9c9199 in linux_process_target::create_inferior(char const*, std::__debug::vector<char*, std::allocator<char*> > const&) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:991
      0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3941
      0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4084
      0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2)

 previously allocated by thread T0 here:
      0x7ff9d6c6e5a7 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99
      0x55bcbf910ad0 in add_process(int, int) /home/smarchi/src/binutils-gdb/gdbserver/inferiors.cc:144
      0x55bcbf9c477d in linux_process_target::add_linux_process_no_mem_file(int, int) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:425
      0x55bcbf9c8f4c in linux_process_target::create_inferior(char const*, std::__debug::vector<char*, std::allocator<char*> > const&) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:985
      0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3941
      0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4084
      0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2)

Above we see that in the non-existing-program case, the process gets
deleted before the starting_up flag gets restored to false.

This happens because startup_inferior calls target_mourn_inferior
before throwing an error, and in GDBserver, unlike in GDB, mourning
deletes the process.

Fix this by not using a scoped_restore to manage the starting_up flag,
since we should only clear it when startup_inferior doesn't throw.

Change-Id: I67325d6f81c64de4e89e20e4ec4556f57eac7f6c
This commit is contained in:
Pedro Alves
2022-06-29 16:38:43 +01:00
parent b955c336f9
commit 7e8621cf6d

@ -105,12 +105,21 @@ post_fork_inferior (int pid, const char *program)
#endif #endif
process_info *proc = find_process_pid (pid); process_info *proc = find_process_pid (pid);
scoped_restore save_starting_up
= make_scoped_restore (&proc->starting_up, true); /* If the inferior fails to start, startup_inferior mourns the
process (which deletes it), and then throws an error. This means
that on exception return, we don't need or want to clear this
flag back, as PROC won't exist anymore. Thus, we don't use a
scoped_restore. */
proc->starting_up = true;
startup_inferior (the_target, pid, startup_inferior (the_target, pid,
START_INFERIOR_TRAPS_EXPECTED, START_INFERIOR_TRAPS_EXPECTED,
&cs.last_status, &cs.last_ptid); &cs.last_status, &cs.last_ptid);
/* If we get here, the process was successfully started. */
proc->starting_up = false;
current_thread->last_resume_kind = resume_stop; current_thread->last_resume_kind = resume_stop;
current_thread->last_status = cs.last_status; current_thread->last_status = cs.last_status;
signal_pid = pid; signal_pid = pid;