From cced7cacecad104fff03e94814a3fca1d304b16f Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Wed, 12 Jan 2022 17:13:38 +0000 Subject: [PATCH] gdb: preserve `|` in connection details string Consider this GDB session: $ gdb -q (gdb) target remote | gdbserver - ~/tmp/hello.x Remote debugging using | gdbserver - ~/tmp/hello.x ... snip ... (gdb) info connections Num What Description * 1 remote gdbserver - ~/tmp/hello.x Remote target using gdb-specific protocol (gdb) python conn = gdb.selected_inferior().connection (gdb) python print(conn.details) gdbserver - ~/tmp/hello.x (gdb) I think there are two things wrong here, first in the "What" column of the 'info connections' output, I think the text should be: remote | gdbserver - ~/tmp/hello.x to correctly show the user how the connection was established. And in a similar fashion, I think that the `details` string of the gdb.TargetConnection object should be: | gdbserver - ~/tmp/hello.x This commit makes this change. Currently the '|' is detected and removed in gdb/serial.c. The string passed to the pipe_ops structure (from gdb/ser-pipe.c), doesn't then, contain the `|`, this is instead implied by the fact that it is a pipes based implementation of the serial_ops interface. After this commit we still detect the `|` in gdb/serial.c, but we now store the full string (including the `|`) in the serial::name member variable. For pipe based serial connections, this name is only used for displaying the two fields I mention above, and in pipe_open (from gdb/ser-pipe.c), and in pipe_open, we now know to skip over the `|`. The benefit I see from this change is that GDB's output now more accurately reflects the commands used to start a target, thus making it easier for a user to understand what is going on. --- gdb/ser-pipe.c | 6 ++ gdb/serial.c | 7 +-- gdb/testsuite/gdb.server/server-pipe.exp | 80 ++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 gdb/testsuite/gdb.server/server-pipe.exp diff --git a/gdb/ser-pipe.c b/gdb/ser-pipe.c index 06c3d3a174e..80bb73e0418 100644 --- a/gdb/ser-pipe.c +++ b/gdb/ser-pipe.c @@ -62,6 +62,12 @@ pipe_open (struct serial *scb, const char *name) int err_pdes[2]; int pid; + if (*name == '|') + { + name++; + name = skip_spaces (name); + } + if (gdb_socketpair_cloexec (AF_UNIX, SOCK_STREAM, 0, pdes) < 0) return -1; if (gdb_socketpair_cloexec (AF_UNIX, SOCK_STREAM, 0, err_pdes) < 0) diff --git a/gdb/serial.c b/gdb/serial.c index c6e3cb9bbbb..41ba753a4b8 100644 --- a/gdb/serial.c +++ b/gdb/serial.c @@ -201,12 +201,7 @@ serial_open (const char *name) const char *open_name = name; if (startswith (name, "|")) - { - ops = serial_interface_lookup ("pipe"); - /* Discard ``|'' and any space before the command itself. */ - ++open_name; - open_name = skip_spaces (open_name); - } + ops = serial_interface_lookup ("pipe"); /* Check for a colon, suggesting an IP address/port pair. Do this *after* checking for all the interesting prefixes. We don't want to constrain the syntax of what can follow them. */ diff --git a/gdb/testsuite/gdb.server/server-pipe.exp b/gdb/testsuite/gdb.server/server-pipe.exp new file mode 100644 index 00000000000..ca2164b03e5 --- /dev/null +++ b/gdb/testsuite/gdb.server/server-pipe.exp @@ -0,0 +1,80 @@ +# 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 . + +# Test starting gdbserver using the 'target remove | ....' syntax. +# The extended-remote target type is tested too. The test checks that +# the `|` character can be seen in the 'info connections' output, and +# (when Python is available) in the gdb.TargetConnection.details +# string. + +# This test relies on starting gdbserver using the pipe syntax. Not +# sure how well this will run if part of this test is being run +# elsewhere. +if { [is_remote target] || [is_remote host] } { + return 0 +} + +load_lib gdbserver-support.exp + +standard_testfile + +if { [skip_gdbserver_tests] } { + return 0 +} + +set gdbserver [find_gdbserver] +if { $gdbserver == "" } { + unsupported "could not find gdbserver" + return +} + +standard_testfile normal.c +set executable ${testfile} + +if {[build_executable "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +# Start GDB using remote type TARGET, which should be either "remote" +# or "extended-remote". Check the output of 'info connections', and +# the contents of the gdb.TargetConnection.details string. +proc do_test { target } { + clean_restart ${::binfile} + + # Make sure we're disconnected, in case we're testing with an + # extended-remote board, therefore already connected. + gdb_test "disconnect" ".*" + + gdb_test "target ${target} | ${::gdbserver} - ${::binfile}" ".*" \ + "start gdbserver using pipe syntax" + + gdb_breakpoint main + gdb_continue_to_breakpoint main + gdb_test "info frame" ".* in main .*" + + gdb_test "info connections" "${target} \| ${::gdbserver} - ${::binfile} \[^\r\n\]+" + + if { ![skip_python_tests] } { + gdb_test_no_output "python conn = gdb.selected_inferior().connection" + gdb_test "python print(conn.details)" "\| ${::gdbserver} - ${::binfile}" + } +} + +# Test with the two remote protocol types. +foreach_with_prefix target { remote extended-remote } { + do_test ${target} +} -- 2.30.2