public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/7] [gdb/testsuite] Fix gdb.server/server-kill.exp for remote target
Date: Thu,  9 Mar 2023 10:45:40 +0100	[thread overview]
Message-ID: <20230309094545.4537-2-tdevries@suse.de> (raw)
In-Reply-To: <20230309094545.4537-1-tdevries@suse.de>

In commit 80dc83fd0e7 ("gdb/remote: handle target dying just before a stepi")
an observation is made that test-case gdb.server/server-kill.exp claims to
kill gdbserver, but actually kills the inferior.  Consequently, the commit
adds testing of killing gdbserver alongside.

The problem is that:
- the original observation is incorrect (possibly caused by misreading getppid
  as getpid)
- consequently, the test-case doesn't test killing the inferior, instead it
  tests killing gdbserver twice
- the method to get the gdbserver PID added in the commit doesn't work
  for target board remote-gdbserver-on-localhost, it returns the
  PID of the ssh client session instead.

Fixing the method for getting the inferior PID gives us fails, and there's no
evidence that killing the inferior ever worked.

So, fix this by reverting the commit and just killing gdbserver, using the
original method of getting the gdbserver PID which does work for target board
remote-gdbserver-on-localhost.

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.server/server-kill.exp | 47 +++++-------------------
 1 file changed, 10 insertions(+), 37 deletions(-)

diff --git a/gdb/testsuite/gdb.server/server-kill.exp b/gdb/testsuite/gdb.server/server-kill.exp
index 5622b27f6bf..0e9df6e1a5f 100644
--- a/gdb/testsuite/gdb.server/server-kill.exp
+++ b/gdb/testsuite/gdb.server/server-kill.exp
@@ -28,21 +28,6 @@ if { [build_executable "failed to prepare" ${testfile}] } {
     return -1
 }
 
-# Global control variable used by the proc prepare.  Should be set to
-# either 'inferior' or 'server'.
-#
-# In the proc prepare we start gdbserver and extract a pid, which will
-# later be killed by calling the proc kill_server.
-#
-# When KILL_PID_OF is set to 'inferior' then the pid we kill is that
-# of the inferior running under gdbserver, when this process dies
-# gdbserver itself will exit.
-#
-# When KILL_PID_OF is set to 'server' then the pid we kill is that of
-# the gdbserver itself, this is a much more aggressive strategy and
-# exposes different bugs within GDB.
-set kill_pid_of "inferior"
-
 # Spawn GDBserver, run to main, extract GDBserver's PID and save it in
 # the SERVER_PID global.
 
@@ -67,24 +52,16 @@ proc prepare {} {
 
     gdbserver_run ""
 
-    # Continue past server_pid assignment.  We do this for both scenarios,
-    # to avoid doing a backtrace from _start, which may not trigger remote
-    # communication.
     gdb_breakpoint ${srcfile}:[gdb_get_line_number "i = 0;"]
     gdb_continue_to_breakpoint "after server_pid assignment"
 
-    if { $::kill_pid_of == "inferior" } {
-	# Get the pid of GDBServer.
-	set test "p server_pid"
-	set server_pid 0
-	gdb_test_multiple $test $test {
-	    -re " = ($decimal)\r\n$gdb_prompt $" {
-		set server_pid $expect_out(1,string)
-		pass $test
-	    }
+    # Get the pid of GDBServer.
+    set server_pid 0
+    gdb_test_multiple "p server_pid" "" {
+	-re -wrap " = ($decimal)" {
+	    set server_pid $expect_out(1,string)
+	    pass $gdb_test_name
 	}
-    } else {
-	set server_pid [exp_pid -i $::server_spawn_id]
     }
 
     if {$server_pid == 0} {
@@ -165,12 +142,8 @@ proc_with_prefix test_stepi {} {
     gdb_test "stepi" "(Target disconnected|Remote connection closed|Remote communication error).*"
 }
 
-# Run each test twice, see the description of KILL_PID_OF earlier in
-# this file for more details.
+test_tstatus
+test_unwind_nosyms
+test_unwind_syms
+test_stepi
 
-foreach_with_prefix kill_pid_of { "inferior" "server" } {
-    test_tstatus
-    test_unwind_nosyms
-    test_unwind_syms
-    test_stepi
-}
-- 
2.35.3


  reply	other threads:[~2023-03-09  9:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-09  9:45 [PATCH 1/7] [gdb/testsuite] Fix gdb.server/connect-with-no-symbol-file.exp " Tom de Vries
2023-03-09  9:45 ` Tom de Vries [this message]
2023-03-09  9:45 ` [PATCH 3/7] [gdb/testsuite] Fix gdb.server/multi-ui-errors.exp " Tom de Vries
2023-03-09  9:45 ` [PATCH 4/7] [gdb/testsuite] Fix gdb.server/sysroot.exp " Tom de Vries
2023-03-09 13:42   ` Tom de Vries
2023-03-09  9:45 ` [PATCH 5/7] [gdb/testsuite] Fix gdbserver path in remote-stdio-gdbserver.exp Tom de Vries
2023-03-09  9:45 ` [PATCH 6/7] [gdb/testsuite] Fix gdb.server/unittest.exp for remote target Tom de Vries
2023-03-09  9:45 ` [PATCH 7/7] [gdb/testsuite] Fix gdb.server/*.exp " Tom de Vries
2023-03-09 11:30 ` [PATCH 1/7] [gdb/testsuite] Fix gdb.server/connect-with-no-symbol-file.exp " Tom de Vries

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=20230309094545.4537-2-tdevries@suse.de \
    --to=tdevries@suse.de \
    --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).