public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb/remote: use current_inferior in read_ptid if multi-process not supported
@ 2022-03-29 15:05 Tankut Baris Aktemur
  0 siblings, 0 replies; only message in thread
From: Tankut Baris Aktemur @ 2022-03-29 15:05 UTC (permalink / raw)
  To: gdb-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2f761de2bab729dcd010057f153d73f2c01396b4

commit 2f761de2bab729dcd010057f153d73f2c01396b4
Author: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Date:   Tue Mar 29 16:19:10 2022 +0200

    gdb/remote: use current_inferior in read_ptid if multi-process not supported
    
    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): 00feffffffffffff000a3a75127f000051510c75127f0000000400000000000060d24ef6af5500000000000000000000680d000000000000b85b31e3fc7f0000c0283a75127f000000e55b75127f000010d04ef6af550000460200000000000060c73975127f0000a0d23975127f0000000a3a75127f0000000000000000000051510c75127f000046020000330000002b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000ffff0000000000000000000000000000000000000000000080143a75127f000080143a75127f000040143a75127f000040143a75127f00007d0000007e0000007f00000080000000300c3a75127f0000300c3a75127f00000e000000000000000e0000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffff0400000004000000040000000400000020143a75127f000020143a75127f000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000000000000000000000e55b75127f0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
      (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>

Diff:
---
 gdb/remote.c                                       |  9 ++-
 gdb/testsuite/gdb.multi/attach-no-multi-process.c  | 34 +++++++++
 .../gdb.multi/attach-no-multi-process.exp          | 88 ++++++++++++++++++++++
 3 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index aa6a67a96e0..219e7203a94 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 doesn't have a PID yet.  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->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..cdcc2afc7b0
--- /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)
+    usleep (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..081579fb964
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/attach-no-multi-process.exp
@@ -0,0 +1,88 @@
+# 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
+
+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 } {
+	# If GDB and GDBserver are both running locally, set the sysroot to avoid
+	# reading files via the remote protocol.
+	if { ![is_remote host] && ![is_remote target] } {
+	    set ::GDBFLAGS "${::GDBFLAGS} -ex \"set sysroot\""
+	}
+	set ::GDBFLAGS \
+	    "${::GDBFLAGS} -ex \"set remote multiprocess-feature-packet off\""
+	set ::GDBFLAGS \
+	    "${::GDBFLAGS} -ex \"maint set target-non-stop ${target_non_stop}\""
+	clean_restart ${binfile}
+    }
+
+    # 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
+}


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-03-29 15:05 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-29 15:05 [binutils-gdb] gdb/remote: use current_inferior in read_ptid if multi-process not supported Tankut Baris Aktemur

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).