From: "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>
To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [PATCH] gdb/remote: use current_inferior in read_ptid if multi-process not supported
Date: Mon, 7 Mar 2022 07:59:42 +0000 [thread overview]
Message-ID: <MWHPR1101MB2271AE1D9C4AE4A4704AB20DC4089@MWHPR1101MB2271.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220209075824.3070-1-tankut.baris.aktemur@intel.com>
Kindly pinging.
Thanks
-Baris
On Wednesday, February 9, 2022 8:58 AM, Aktemur, Tankut Baris wrote:
> When parsing the ptid out of a reply package, if the multi-process
> extensions are not supported, use current_inferior's pid as the pid of
> the reported thread, instead of inferior_ptid. This is needed because
> the inferior_ptid may be null_ptid although a legit context exists,
> due to a prior context switch via switch_to_inferior_no_thread.
>
> Below is a scenario that illustrates what could go wrong. First,
> setup a multi-target scenario. This is needed, because in a
> multi-target setting, the inferior_ptid is cleared out before waiting
> on targets. The second inferior below sits on top of a remote target.
> Multi-process packets are disabled.
>
> $ # First, spawn a process with PID 26253 to attach to later.
> $ gdb-up a.out
> Reading symbols from a.out...
> (gdb) maint set target-non-stop on
> (gdb) set remote multiprocess-feature-packet off
> (gdb) start
> ...
> (gdb) add-inferior -no-connection
> [New inferior 2]
> Added inferior 2
> (gdb) inferior 2
> [Switching to inferior 2 [<null>] (<noexec>)]
> (gdb) target extended-remote | gdbserver --multi -
> Remote debugging using | gdbserver --multi -
> Remote debugging using stdio
> (gdb) attach 26253
> Attaching to Remote target
> Attached; pid = 26253
> [New Thread 26253]
> [New inferior 3]
> Reading /tmp/a.out from remote target...
> ...
> [New Thread 26253]
> ...
> Reading /usr/local/lib/debug/....debug from remote target...
> >>> GDB seems to hang here.
>
> After attaching to a process and reading some library files, GDB
> seems to hang. One interesting thing to note is that
>
> [New Thread 26253]
>
> appears twice. We also see
>
> [New inferior 3]
>
> Running the same scenario with "debug infrun on" reveals more details.
>
> ...
> (gdb) attach 26253
> [infrun] scoped_disable_commit_resumed: reason=attaching
> Attaching to Remote target
> Attached; pid = 26253
> [New Thread 26253]
> [infrun] infrun_async: enable=1
> [infrun] attach_command: immediately after attach:
> [infrun] attach_command: thread 26253.26253.0, executing = 1, resumed = 0, state =
> RUNNING
> [infrun] clear_proceed_status_thread: 26253.26253.0
> [infrun] reset: reason=attaching
> [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for target
> native, no resumed threads
> [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target
> extended-remote
> [infrun] fetch_inferior_event: enter
> [infrun] scoped_disable_commit_resumed: reason=handling event
> [infrun] do_target_wait: Found 2 inferiors, starting at #1
> [infrun] random_pending_event_thread: None found.
> [infrun] print_target_wait_results: target_wait (-1.0.0 [Thread 0], status) =
> [infrun] print_target_wait_results: 26253.26253.0 [Thread 26253],
> [infrun] print_target_wait_results: status->kind = STOPPED, sig = GDB_SIGNAL_0
> [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_0
> [infrun] start_step_over: enter
> [infrun] start_step_over: stealing global queue of threads to step, length = 0
> [infrun] operator(): step-over queue now empty
> [infrun] start_step_over: exit
> [infrun] context_switch: Switching context from 0.0.0 to 26253.26253.0
> [infrun] handle_signal_stop: stop_pc=0x7f849d8cf151
> [infrun] stop_waiting: stop_waiting
> [infrun] stop_all_threads: starting
> [infrun] stop_all_threads: pass=0, iterations=0
> [New inferior 3]
> Reading /tmp/a.out from remote target...
> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files
> locally instead.
> Reading /tmp/a.out from remote target...
> Reading symbols from target:/tmp/a.out...
> [New Thread 26253]
> [infrun] stop_all_threads: 4723.4723.0 not executing
> [infrun] stop_all_threads: 26253.26253.0 not executing
> [infrun] stop_all_threads: 42000.26253.0 executing, need stop
> [infrun] print_target_wait_results: target_wait (-1.0.0 [Thread 0], status) =
> [infrun] print_target_wait_results: -1.0.0 [Thread 0],
> [infrun] print_target_wait_results: status->kind = IGNORE
> [infrun] print_target_wait_results: target_wait (-1.0.0 [Thread 0], status) =
> [infrun] print_target_wait_results: -1.0.0 [Thread 0],
> [infrun] print_target_wait_results: status->kind = IGNORE
>
> GDB tried to stop Thread 42000.26253.0, which does not exist, and we
> are waiting for a stop event that will never happen. The PID in
> '42000.26253.0', namely 42000, is the PID of magic_null_ptid.
> It comes from gdb/remote.c:read_ptid:
>
> /* Since the stub is not sending a process id, then default to
> what's in inferior_ptid, unless it's null at this point. If so,
> then since there's no way to know the pid of the reported
> threads, use the magic number. */
> if (inferior_ptid == null_ptid)
> pid = magic_null_ptid.pid ();
> else
> pid = inferior_ptid.pid ();
>
> if (obuf)
> *obuf = pp;
> return ptid_t (pid, tid);
>
> Because multi-process was turned off, GDB did not parse an explicitly
> specified PID. Furthermore, inferior_ptid == null_ptid, and
> eventually GDB picked the PID from magic_null_ptid.
>
> If target-non-stop is not turned on at the beginning, the same bug
> reveals itself as a duplicated thread as shown below.
>
> # Same setup as above, without 'maint set target-non-stop on'.
> ...
> (gdb) attach 26253
> Attaching to Remote target
> Attached; pid = 26253
> [New inferior 3]
> ...
> [New Thread 26253]
> ...
> (gdb) info threads
> Id Target Id Frame
> 1.1 process 13517 "a.out" main () at test.c:3
> * 2.1 Thread 26253 "a.out" 0x00007f12750c5151 in read () from target:/lib/x86_64-linux-
> gnu/libc.so.6
> 3.1 Thread 26253 "a.out" Remote 'g' packet reply is too long (expected 560 bytes, got
> 2496 bytes):
> 00feffffffffffff000a3a75127f000051510c75127f0000000400000000000060d24ef6af550000000000000000
> 0000680d000000000000b85b31e3fc7f0000c0283a75127f000000e55b75127f000010d04ef6af55000046020000
> 0000000060c73975127f0000a0d23975127f0000000a3a75127f0000000000000000000051510c75127f00004602
> 0000330000002b000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000007f03000000000000ffff00000000000000000000000000000000000000000000
> 80143a75127f000080143a75127f000040143a75127f000040143a75127f00007d0000007e0000007f0000008000
> 0000300c3a75127f0000300c3a75127f00000e000000000000000e00000000000000000000000000000000000000
> 00000000ffffffffffffffffffffffffffffffff0400000004000000040000000400000020143a75127f00002014
> 3a75127f000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 0000000000000000000000000000000000000000000000000000801f0000000000000000000000e55b75127f0000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> 000000000000000000000000
> (gdb)
>
> Fix the problem by preferring current_inferior()'s pid instead of
> magic_null_ptid.
>
> Regression-tested on X86-64 Linux.
>
> Co-authored-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
> ---
> gdb/remote.c | 9 +-
> .../gdb.multi/attach-no-multi-process.c | 34 +++++++
> .../gdb.multi/attach-no-multi-process.exp | 90 +++++++++++++++++++
> 3 files changed, 129 insertions(+), 4 deletions(-)
> create mode 100644 gdb/testsuite/gdb.multi/attach-no-multi-process.c
> create mode 100644 gdb/testsuite/gdb.multi/attach-no-multi-process.exp
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 49eeb63445d..281260af971 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -3145,14 +3145,15 @@ read_ptid (const char *buf, const char **obuf)
> return null_ptid;
> }
>
> - /* Since the stub is not sending a process id, then default to
> - what's in inferior_ptid, unless it's null at this point. If so,
> + /* Since the stub is not sending a process id, default to
> + what's current_inferior, unless it's null at this point. If so,
> then since there's no way to know the pid of the reported
> threads, use the magic number. */
> - if (inferior_ptid == null_ptid)
> + inferior *inf = current_inferior ();
> + if (inf == nullptr || inf->pid == 0)
> pid = magic_null_ptid.pid ();
> else
> - pid = inferior_ptid.pid ();
> + pid = inf->pid;
>
> if (obuf)
> *obuf = pp;
> diff --git a/gdb/testsuite/gdb.multi/attach-no-multi-process.c
> b/gdb/testsuite/gdb.multi/attach-no-multi-process.c
> new file mode 100644
> index 00000000000..df09dd69ccc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/attach-no-multi-process.c
> @@ -0,0 +1,34 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2020-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 <http://www.gnu.org/licenses/>. */
> +
> +/* This program is intended to be started outside of gdb, and then
> + attached to by GDB. */
> +
> +#include <unistd.h>
> +
> +int
> +main ()
> +{
> + /* Don't run forever in case GDB crashes and DejaGNU fails to kill
> + this program. */
> + alarm (10);
> +
> + while (1)
> + ;
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.multi/attach-no-multi-process.exp
> b/gdb/testsuite/gdb.multi/attach-no-multi-process.exp
> new file mode 100644
> index 00000000000..0ba99351ec5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/attach-no-multi-process.exp
> @@ -0,0 +1,90 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2020-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 <http://www.gnu.org/licenses/>.
> +
> +# Test attaching to a process, as a second inferior, through a
> +# gdbserver that does not support multi-process extensions.
> +
> +load_lib gdbserver-support.exp
> +
> +standard_testfile
> +
> +# The plain remote target can't do multiple inferiors.
> +if {[target_info gdb_protocol] != ""} {
> + return
> +}
> +
> +if {![can_spawn_for_attach]} {
> + return
> +}
> +
> +if {[build_executable "build" $testfile $srcfile {debug}] == -1} {
> + return -1
> +}
> +
> +proc test {target_non_stop} {
> + global binfile
> +
> + save_vars { ::GDBFLAGS } {
> + # Make GDB read files from the local file system, not through the
> + # remote targets, to speed things up.
> + set ::GDBFLAGS "${::GDBFLAGS} -ex \"set sysroot\""
> + clean_restart ${binfile}
> + }
> +
> + gdb_test_no_output "set remote multiprocess-feature-packet off"
> + gdb_test_no_output "maint set target-non-stop ${target_non_stop}"
> +
> + # Start the first inferior.
> + if {![runto_main]} {
> + return
> + }
> +
> + # The second inferior is an extended remote.
> + gdb_test "add-inferior -no-connection" "Added inferior 2.*" \
> + "add the second inferior"
> + gdb_test "inferior 2" ".*Switching to inferior 2.*" \
> + "switch to inferior 2"
> + set res [gdbserver_start "--multi" ""]
> + set gdbserver_gdbport [lindex $res 1]
> + gdb_target_cmd "extended-remote" $gdbserver_gdbport
> +
> + # Start a program, then attach to it.
> + set spawn_id_list [spawn_wait_for_attach [list $binfile]]
> + set test_spawn_id [lindex $spawn_id_list 0]
> + set testpid [spawn_id_get_pid $test_spawn_id]
> + gdb_test "attach $testpid" \
> + "Attaching to Remote target.*" \
> + "attach to the program via remote"
> +
> + # Check that we have two threads. Bad GDB duplicated the
> + # thread coming from the remote when target-non-stop is off;
> + # or hanged during attach when target-non-stop is on.
> + gdb_test "info threads" \
> + [multi_line \
> + " Id\[^\r\n\]+" \
> + " 1\.1\[^\r\n\]+" \
> + ". 2\.1\[^\r\n\]+"
> + ]
> +
> + # Clean the spawned process and gdbserver.
> + gdbserver_exit 0
> + kill_wait_spawned_process $test_spawn_id
> +}
> +
> +foreach_with_prefix target_non_stop {off on} {
> + test $target_non_stop
> +}
> --
> 2.33.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
next prev parent reply other threads:[~2022-03-07 7:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-09 7:58 Tankut Baris Aktemur
2022-02-22 7:33 ` Aktemur, Tankut Baris
2022-03-07 7:59 ` Aktemur, Tankut Baris [this message]
2022-03-07 16:03 ` Pedro Alves
2022-03-29 8:25 ` Aktemur, Tankut Baris
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR1101MB2271AE1D9C4AE4A4704AB20DC4089@MWHPR1101MB2271.namprd11.prod.outlook.com \
--to=tankut.baris.aktemur@intel.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).