From patchwork Mon Nov 28 18:40:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 61185 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 07B9B38493F1 for ; Mon, 28 Nov 2022 18:42:00 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 07B9B38493F1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1669660920; bh=FclHDsjzu7N/XvR1Jz8ctBb1KOX45KaooHvErXqun5w=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=vQtN2u2riZgk6mzLnfnZdySjCyTkyw2yGdQTWpcEpGh6jFqd5wqR5pZ35HpfmgtZ4 sbGlEZXWuUpPKUozIGIlkiN699sFB44gzqzowSeo6JTE6SBRq0R+J3IJyD0OApJJxk qoxa65Wm5s5MzNULyfqe3hBzWqntF955kr0WN23g= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 49670385516D for ; Mon, 28 Nov 2022 18:41:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 49670385516D X-ASG-Debug-ID: 1669660847-0c856e02a143f5a0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id UESUp8bRaZj6kKds (version=TLSv1 cipher=AES128-SHA bits=128 verify=NO); Mon, 28 Nov 2022 13:40:47 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from epycamd.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id 04006441D68; Mon, 28 Nov 2022 13:40:47 -0500 (EST) X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Cc: Simon Marchi , Andrew Burgess Subject: [PATCH gdb-12-branch 5/6] gdbserver: switch to right process in find_one_thread Date: Mon, 28 Nov 2022 13:40:41 -0500 X-ASG-Orig-Subj: [PATCH gdb-12-branch 5/6] gdbserver: switch to right process in find_one_thread Message-Id: <20221128184042.3781569-6-simon.marchi@efficios.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221128184042.3781569-1-simon.marchi@efficios.com> References: <20221128184042.3781569-1-simon.marchi@efficios.com> MIME-Version: 1.0 X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1669660847 X-Barracuda-Encrypted: AES128-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 11958 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=5.0 tests=WEIRD_PORT X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.102463 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 WEIRD_PORT URI: Uses non-standard port number for HTTP X-Spam-Status: No, score=-3498.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP, WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" New in this version: add a dedicated test. When I do this: $ ./gdb -nx --data-directory=data-directory -q \ /bin/sleep \ -ex "maint set target-non-stop on" \ -ex "tar ext :1234" \ -ex "set remote exec-file /bin/sleep" \ -ex "run 1231 &" \ -ex add-inferior \ -ex "inferior 2" Reading symbols from /bin/sleep... (No debugging symbols found in /bin/sleep) Remote debugging using :1234 Starting program: /bin/sleep 1231 Reading /lib64/ld-linux-x86-64.so.2 from remote target... warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead. Reading /lib64/ld-linux-x86-64.so.2 from remote target... Reading /usr/lib/debug/.build-id/a6/7a1408f18db3576757eea210d07ba3fc560dff.debug from remote target... [New inferior 2] Added inferior 2 on connection 1 (extended-remote :1234) [Switching to inferior 2 [] ()] (gdb) Reading /lib/x86_64-linux-gnu/libc.so.6 from remote target... attach 3659848 Attaching to process 3659848 /home/smarchi/src/binutils-gdb/gdb/thread.c:85: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed. Note the "attach" command just above. When doing it on the command-line with a -ex switch, the bug doesn't trigger. The internal error of GDB is actually caused by GDBserver crashing, and the error recovery of GDB is not on point. This patch aims to fix just the GDBserver crash, not the GDB problem. GDBserver crashes with a segfault here: (gdb) bt #0 0x00005555557fb3f4 in find_one_thread (ptid=...) at /home/smarchi/src/binutils-gdb/gdbserver/thread-db.cc:177 #1 0x00005555557fd5cf in thread_db_thread_handle (ptid=, handle=0x7fffffffc400, handle_len=0x7fffffffc3f0) at /home/smarchi/src/binutils-gdb/gdbserver/thread-db.cc:461 #2 0x000055555578a0b6 in linux_process_target::thread_handle (this=0x5555558a64c0 , ptid=, handle=0x7fffffffc400, handle_len=0x7fffffffc3f0) at /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:6905 #3 0x00005555556dfcc6 in handle_qxfer_threads_worker (thread=0x60b000000510, buffer=0x7fffffffc8a0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1645 #4 0x00005555556e00e6 in operator() (__closure=0x7fffffffc5e0, thread=0x60b000000510) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1696 #5 0x00005555556f54be in for_each_thread >(struct {...}) (func=...) at /home/smarchi/src/binutils-gdb/gdbserver/gdbthread.h:159 #6 0x00005555556e0242 in handle_qxfer_threads_proper (buffer=0x7fffffffc8a0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1694 #7 0x00005555556e04ba in handle_qxfer_threads (annex=0x629000000213 "", readbuf=0x621000019100 '\276' ..., writebuf=0x0, offset=0, len=4097) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1732 #8 0x00005555556e1989 in handle_qxfer (own_buf=0x629000000200 "qXfer:threads", packet_len=26, new_packet_len_p=0x7fffffffd630) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:2045 #9 0x00005555556e720a in handle_query (own_buf=0x629000000200 "qXfer:threads", packet_len=26, new_packet_len_p=0x7fffffffd630) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:2685 #10 0x00005555556f1a01 in process_serial_event () at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4176 #11 0x00005555556f4457 in handle_serial_event (err=0, client_data=0x0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4514 #12 0x0000555555820f56 in handle_file_event (file_ptr=0x607000000250, ready_mask=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573 #13 0x0000555555821895 in gdb_wait_for_event (block=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694 #14 0x000055555581f533 in gdb_do_one_event (mstimeout=-1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264 #15 0x00005555556ec9fb in start_event_loop () at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3512 #16 0x00005555556f0769 in captured_main (argc=4, argv=0x7fffffffe0d8) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3992 #17 0x00005555556f0e3f in main (argc=4, argv=0x7fffffffe0d8) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4078 The reason is a wrong current process when find_one_thread is called. The current process is the 2nd one, which was just attached. It does not yet have thread_db data (proc->priv->thread_db is nullptr). As we iterate on all threads of all process to fulfull the qxfer:threads:read request, we get to a thread of process 1 for which we haven't read thread_db information yet (lwp_info::thread_known is false), so we get into find_one_thread. find_one_thread uses `current_process ()->priv->thread_db`, assuming the current process matches the ptid passed as a parameter, which is wrong. A segfault happens when trying to dereference that thread_db pointer. Fix this by making find_one_thread not assume what the current process / current thread is. If it needs to call into libthread_db, which we know will try to read memory from the current process, then temporarily set the current process. In the case where the thread is already know and we return early, we don't need to switch process. Add a test to reproduce this specific situation. Change-Id: I09b00883e8b73b7e5f89d0f47cb4e9c0f3d6caaa Approved-By: Andrew Burgess --- .../gdb.multi/attach-while-running.c | 26 +++++++ .../gdb.multi/attach-while-running.exp | 73 +++++++++++++++++++ gdbserver/thread-db.cc | 29 +++++--- 3 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 gdb/testsuite/gdb.multi/attach-while-running.c create mode 100644 gdb/testsuite/gdb.multi/attach-while-running.exp diff --git a/gdb/testsuite/gdb.multi/attach-while-running.c b/gdb/testsuite/gdb.multi/attach-while-running.c new file mode 100644 index 00000000000..dd321dfe007 --- /dev/null +++ b/gdb/testsuite/gdb.multi/attach-while-running.c @@ -0,0 +1,26 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +int global_var = 123; + +int +main (void) +{ + sleep (30); +} diff --git a/gdb/testsuite/gdb.multi/attach-while-running.exp b/gdb/testsuite/gdb.multi/attach-while-running.exp new file mode 100644 index 00000000000..125273d0524 --- /dev/null +++ b/gdb/testsuite/gdb.multi/attach-while-running.exp @@ -0,0 +1,73 @@ +# Copyright 2022 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This test was introduced to reproduce a specific bug in GDBserver, where +# attaching an inferior while another one was running would trigger a segfault +# in GDBserver. Reproducing the bug required specific circumstances: +# +# - The first process must be far enough to have loaded its libc or +# libpthread (whatever triggers the loading of libthread_db), such that +# its proc->priv->thread_db is not nullptr +# +# - However, its lwp must still be in the `!lwp->thread_known` state, +# meaning GDBserver hasn't asked libthread_db to compute the thread +# handle yet. That means, GDB must not have refreshed the thread list +# yet, since that would cause the thread handles to be computed. That +# means, no stopping on a breakpoint, since that causes a thread list +# update. That's why the first inferior needs to be started with "run +# &". +# +# - Attaching the second process would segfault GDBserver. +# +# All of this to say, if modifying this test, please keep in mind the original +# intent. + +standard_testfile + +if [use_gdb_stub] { + unsupported "test requires running" + return +} + +if { [build_executable "failed to prepare" ${testfile} ${srcfile}] } { + return +} + +proc do_test {} { + save_vars { $::GDBFLAGS } { + append ::GDBFLAGS " -ex \"maint set target-non-stop on\"" + clean_restart $::binfile + } + + gdb_test_multiple "run &" "" { + -re ".*$::gdb_prompt " { + pass $gdb_test_name + } + } + gdb_test "add-inferior" "Added inferior 2 on connection 1 .*" + gdb_test "inferior 2" "Switching to inferior 2 .*" + + set spawn_id [spawn_wait_for_attach $::binfile] + set pid [spawn_id_get_pid $spawn_id] + + # This call would crash GDBserver. + gdb_attach $pid + + # Read a variable from the inferior, just to make sure the attach worked + # fine. + gdb_test "print global_var" " = 123" +} + +do_test diff --git a/gdbserver/thread-db.cc b/gdbserver/thread-db.cc index 6e0e2228a5f..bf98ca9557a 100644 --- a/gdbserver/thread-db.cc +++ b/gdbserver/thread-db.cc @@ -155,30 +155,35 @@ thread_db_state_str (td_thr_state_e state) } #endif -/* Get thread info about PTID, accessing memory via the current - thread. */ +/* Get thread info about PTID. */ static int find_one_thread (ptid_t ptid) { - td_thrhandle_t th; - td_thrinfo_t ti; - td_err_e err; - struct lwp_info *lwp; - struct thread_db *thread_db = current_process ()->priv->thread_db; - int lwpid = ptid.lwp (); - thread_info *thread = find_thread_ptid (ptid); - lwp = get_thread_lwp (thread); + lwp_info *lwp = get_thread_lwp (thread); if (lwp->thread_known) return 1; - /* Get information about this thread. */ - err = thread_db->td_ta_map_lwp2thr_p (thread_db->thread_agent, lwpid, &th); + /* Get information about this thread. libthread_db will need to read some + memory, which will be done on the current process, so make PTID's process + the current one. */ + process_info *proc = find_process_pid (ptid.pid ()); + gdb_assert (proc != nullptr); + + scoped_restore_current_thread restore_thread; + switch_to_process (proc); + + thread_db *thread_db = proc->priv->thread_db; + td_thrhandle_t th; + int lwpid = ptid.lwp (); + td_err_e err = thread_db->td_ta_map_lwp2thr_p (thread_db->thread_agent, lwpid, + &th); if (err != TD_OK) error ("Cannot get thread handle for LWP %d: %s", lwpid, thread_db_err_str (err)); + td_thrinfo_t ti; err = thread_db->td_thr_get_info_p (&th, &ti); if (err != TD_OK) error ("Cannot get thread info for LWP %d: %s",