From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1551) id 6CEB538460B3; Wed, 29 Jun 2022 18:33:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6CEB538460B3 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Pedro Alves To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Fix GDBserver regression due to change to avoid reading shell registers X-Act-Checkin: binutils-gdb X-Git-Author: Pedro Alves X-Git-Refname: refs/heads/master X-Git-Oldrev: b955c336f93a1b8ab433b713739c145c56f5c027 X-Git-Newrev: 7e8621cf6db8d69c325d86dba59037367cd0f853 Message-Id: <20220629183307.6CEB538460B3@sourceware.org> Date: Wed, 29 Jun 2022 18:33:07 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2022 18:33:07 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D7e8621cf6db8= d69c325d86dba59037367cd0f853 commit 7e8621cf6db8d69c325d86dba59037367cd0f853 Author: Pedro Alves Date: Wed Jun 29 16:38:43 2022 +0100 Fix GDBserver regression due to change to avoid reading shell registers =20 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: =20 $ /home/smarchi/build/binutils-gdb/gdb/testsuite/../../gdb/../gdbserve= r/gdbserver stdio non-existing-program =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D127719=3D=3DERROR: AddressSanitizer: heap-use-after-free on addr= ess 0x60f0000000e9 at pc 0x55bcbfa301f4 bp 0x7ffd238a7320 sp 0x7ffd238a7310 WRITE of size 1 at 0x60f0000000e9 thread T0 #0 0x55bcbfa301f3 in scoped_restore_tmpl::~scoped_restore_tm= pl() /home/smarchi/src/binutils-gdb/gdbserver/../gdbsupport/scoped_restore.= h:86 #1 0x55bcbfa2ffe9 in post_fork_inferior(int, char const*) /home/sm= archi/src/binutils-gdb/gdbserver/fork-child.cc:120 #2 0x55bcbf9c9199 in linux_process_target::create_inferior(char co= nst*, std::__debug::vector > const&) /home/sma= rchi/src/binutils-gdb/gdbserver/linux-low.cc:991 #3 0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/= gdbserver/server.cc:3941 #4 0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver= /server.cc:4084 #5 0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc= .so.6+0x240b2) #6 0x55bcbf8ef2bd in _start (/home/smarchi/build/binutils-gdb/gdbs= erver/gdbserver+0x1352bd) =20 0x60f0000000e9 is located 169 bytes inside of 176-byte region [0x60f00= 0000040,0x60f0000000f0) freed by thread T0 here: #0 0x7ff9d6c6f0c7 in operator delete(void*) ../../../../src/libsan= itizer/asan/asan_new_delete.cpp:160 #1 0x55bcbf910d00 in remove_process(process_info*) /home/smarchi/s= rc/binutils-gdb/gdbserver/inferiors.cc:164 #2 0x55bcbf9c4ac7 in linux_process_target::remove_linux_process(pr= ocess_info*) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:454 #3 0x55bcbf9cdaa6 in linux_process_target::mourn(process_info*) /h= ome/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:1599 #4 0x55bcbf988dc4 in target_mourn_inferior(ptid_t) /home/smarchi/s= rc/binutils-gdb/gdbserver/target.cc:205 #5 0x55bcbfa32020 in startup_inferior(process_stratum_target*, int= , int, target_waitstatus*, ptid_t*) /home/smarchi/src/binutils-gdb/gdbserve= r/../gdb/nat/fork-inferior.c:515 #6 0x55bcbfa2fdeb in post_fork_inferior(int, char const*) /home/sm= archi/src/binutils-gdb/gdbserver/fork-child.cc:111 #7 0x55bcbf9c9199 in linux_process_target::create_inferior(char co= nst*, std::__debug::vector > const&) /home/sma= rchi/src/binutils-gdb/gdbserver/linux-low.cc:991 #8 0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/= gdbserver/server.cc:3941 #9 0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver= /server.cc:4084 #10 0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/lib= c.so.6+0x240b2) =20 previously allocated by thread T0 here: #0 0x7ff9d6c6e5a7 in operator new(unsigned long) ../../../../src/l= ibsanitizer/asan/asan_new_delete.cpp:99 #1 0x55bcbf910ad0 in add_process(int, int) /home/smarchi/src/binut= ils-gdb/gdbserver/inferiors.cc:144 #2 0x55bcbf9c477d in linux_process_target::add_linux_process_no_me= m_file(int, int) /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:425 #3 0x55bcbf9c8f4c in linux_process_target::create_inferior(char co= nst*, std::__debug::vector > const&) /home/sma= rchi/src/binutils-gdb/gdbserver/linux-low.cc:985 #4 0x55bcbf954549 in captured_main /home/smarchi/src/binutils-gdb/= gdbserver/server.cc:3941 #5 0x55bcbf9552f0 in main /home/smarchi/src/binutils-gdb/gdbserver= /server.cc:4084 #6 0x7ff9d663b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc= .so.6+0x240b2) =20 Above we see that in the non-existing-program case, the process gets deleted before the starting_up flag gets restored to false. =20 This happens because startup_inferior calls target_mourn_inferior before throwing an error, and in GDBserver, unlike in GDB, mourning deletes the process. =20 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. =20 Change-Id: I67325d6f81c64de4e89e20e4ec4556f57eac7f6c Diff: --- gdbserver/fork-child.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/gdbserver/fork-child.cc b/gdbserver/fork-child.cc index 7ea66f2a435..078612f7091 100644 --- a/gdbserver/fork-child.cc +++ b/gdbserver/fork-child.cc @@ -105,12 +105,21 @@ post_fork_inferior (int pid, const char *program) #endif =20 process_info *proc =3D find_process_pid (pid); - scoped_restore save_starting_up - =3D 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 =3D true; =20 startup_inferior (the_target, pid, START_INFERIOR_TRAPS_EXPECTED, &cs.last_status, &cs.last_ptid); + + /* If we get here, the process was successfully started. */ + proc->starting_up =3D false; + current_thread->last_resume_kind =3D resume_stop; current_thread->last_status =3D cs.last_status; signal_pid =3D pid;