* [PATCH v2 0/6] Add vDefaultInferiorFd feature
@ 2024-01-19 11:56 Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 1/6] gdb.server/non-existing-program.exp: Use gdbserver_start Alexandra Hájková
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
Currently, when GDBserver is run locally using stdio, the inferior
is unable to read from STDIN so we can't give it any input.
The main motivation to address this issue is to use GDB together
with Valgrind, using vgdb --multi feature which allows to run
Valgrind from inside GDB. Valgrind then acts as a locally run
GDBserver that uses stdio.
Add a new DefaultInferiorFd feature and the corresponding packet.
This feature allows GDB to send, to GDBserver, the file descriptor
numbers of the terminal to which GDB is connected. The inferior is
then started connected to the same terminal as GDB. This allows the
inferior run by local GDBserver to read from GDB's STDIN and write
its output to GDB's STOUT/ERR the same way as native target.
v2: - [2/6]:- add an error handling in a case dup would fail
- hoist the calls to mark_fd_no_cloexec near the dup()s
[3/6] - improve documentation formatting
- rephrase and extend the documentation
- cosmetics
- add a new target method so that gdbserver doesn't advertise
this new feauture on platforms it does not work at
[4/3] - cosmetics
Alexandra Hájková (6):
gdb.server/non-existing-program.exp: Use gdbserver_start.
gdb/ser-pipe.c: Duplicate the file descriptors
Add new vDefaultInferiorFd packet
gdbserver/linux-low.cc: Connect the inferior to the terminal
remote.c: Add terminal handling functions
Add defaultinf.exp test to the testsuite
gdb/doc/gdb.texinfo | 42 ++++++
gdb/remote.c | 83 +++++++++++
gdb/ser-pipe.c | 40 ++++++
gdb/serial.c | 4 +
gdb/serial.h | 4 +
gdb/testsuite/gdb.server/defaultinf.c | 39 +++++
gdb/testsuite/gdb.server/defaultinf.exp | 59 ++++++++
.../gdb.server/non-existing-program.exp | 54 ++-----
gdb/testsuite/lib/gdbserver-support.exp | 62 +++++---
gdbserver/linux-low.cc | 38 ++++-
gdbserver/linux-low.h | 2 +
gdbserver/server.cc | 135 +++++++++++++++++-
gdbserver/server.h | 12 ++
gdbserver/target.cc | 6 +
gdbserver/target.h | 6 +
15 files changed, 524 insertions(+), 62 deletions(-)
create mode 100644 gdb/testsuite/gdb.server/defaultinf.c
create mode 100644 gdb/testsuite/gdb.server/defaultinf.exp
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/6] gdb.server/non-existing-program.exp: Use gdbserver_start.
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
@ 2024-01-19 11:56 ` Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2] remote.c: Make packet_check_result return a structure Alexandra Hájková
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
Also modify GDBserver_start to match more messages from the server
and return them to the caller.
This test tests if GDBserver exits cleanly when GDBserver is called using
stdio with some program that does not exist. My series modifies the GDBserver
in such a way, it defers starting the inferior till certain packets arrive
when stdio is used. The result was, when GDBserver was called using stdio with
some nonexistent file, it was waiting for those packets to come but since GDB
was never attached, they would never do so and the test timed out. Running
GDBserver using stdio without planning to attach there with GDB is not a
realistic situation and what the user would normally do. The test was
modifyied to start GDBserver with a port number instead of stdio by calling
GDBserver_start proc which was modifyied to return a message from
GDBserver.
---
.../gdb.server/non-existing-program.exp | 54 +++++-----------
gdb/testsuite/lib/gdbserver-support.exp | 62 +++++++++++++------
2 files changed, 58 insertions(+), 58 deletions(-)
diff --git a/gdb/testsuite/gdb.server/non-existing-program.exp b/gdb/testsuite/gdb.server/non-existing-program.exp
index 2d08e500def..751a55822b6 100644
--- a/gdb/testsuite/gdb.server/non-existing-program.exp
+++ b/gdb/testsuite/gdb.server/non-existing-program.exp
@@ -30,44 +30,20 @@ if { $gdbserver == "" } {
return
}
-# Fire off gdbserver. The port doesn't really matter, gdbserver tries
-# to spawn the program before opening the connection.
-set spawn_id [remote_spawn target "$gdbserver stdio non-existing-program"]
-
-set msg "gdbserver exits cleanly"
-set saw_exiting 0
-expect {
- # This is what we get on ptrace-based targets with
- # startup-with-shell disabled (e.g., when the SHELL variable is
- # unset).
- -re "stdin/stdout redirected.*gdbserver: Cannot exec non-existing-program\r\ngdbserver: Error: No such file or directory\r\n\r\nDuring startup program exited with code 127\.\r\nExiting\r\n" {
- set saw_exiting 1
- exp_continue
- }
- # Likewise, but with startup-with-shell enabled, which is the
- # default behaviour.
- -re "stdin/stdout redirected.*exec: non-existing-program: not found\r\nDuring startup program exited with code 127\.\r\nExiting\r\n" {
- set saw_exiting 1
- exp_continue
- }
- # This is what we get on Windows.
- -re "Error creating process\r\n\r\nExiting\r\n" {
- set saw_exiting 1
- exp_continue
- }
- -re "A problem internal to GDBserver has been detected" {
- fail "$msg (GDBserver internal error)"
- wait
- }
- eof {
- gdb_assert $saw_exiting $msg
- wait
+foreach_with_prefix start_with_shell { on off } {
+ if { $start_with_shell } {
+ set arg "--startup-with-shell"
+ } else {
+ set arg "--no-startup-with-shell"
}
- timeout {
- fail "$msg (timeout)"
- }
-}
-# expect defaults to spawn_id in many places. Avoid confusing any
-# following code.
-unset spawn_id
+ set msg "GDBserver exits cleanly"
+ set res [gdbserver_start $arg non-existent-file]
+ set status [lindex $res 2]
+
+ # We expect GDBserver to fail with the message indicating executable
+ # does not exist.
+ gdb_assert { $status == "During startup program exited with code 127\.\r\nExiting\r\n"
+ || $status == "Error creating process\r\n\r\nExiting\r\n"} $msg
+
+}
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 8bcf4fbbb01..92afd6a4642 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -244,7 +244,8 @@ proc gdbserver_default_get_comm_port { port } {
# Start a gdbserver process with initial OPTIONS and trailing ARGUMENTS.
# The port will be filled in between them automatically.
#
-# Returns the target protocol and socket to connect to.
+# Returns the target protocol, socket to connect to and the status
+# message from the gdbserver.
proc gdbserver_start { options arguments } {
global portnum
@@ -363,33 +364,56 @@ proc gdbserver_start { options arguments } {
# talk to the program using GDBserver's tty instead.
global inferior_spawn_id
set inferior_spawn_id $server_spawn_id
+ set msg 0
# Wait for the server to open its TCP socket, so that GDB can connect.
expect {
- -i $server_spawn_id
- -timeout 120
- -notransfer
- -re "Listening on" { }
- -re "Can't (bind address|listen on socket): Address already in use\\.\r\n" {
- verbose -log "Port $portnum is already in use."
- if ![target_info exists gdb,socketport] {
- # Bump the port number to avoid the conflict.
- wait -i $expect_out(spawn_id)
- incr portnum
- continue
- }
- }
- -re ".*: cannot resolve name: .*\r\n" {
- error "gdbserver cannot resolve name."
- }
- timeout {
+ -i $server_spawn_id
+ -timeout 120
+ -notransfer
+ -re "Listening on" {
+ set msg $expect_out(0,string)
+ }
+ -re "Can't (bind address|listen on socket): Address already in use\\.\r\n" {
+ verbose -log "Port $portnum is already in use."
+ set msg "Port is already in use"
+ if ![target_info exists gdb,socketport] {
+ # Bump the port number to avoid the conflict.
+ wait -i $expect_out(spawn_id)
+ incr portnum
+ continue
+ }
+ }
+ -re ".*: cannot resolve name: .*\r\n" {
+ error "gdbserver cannot resolve name."
+ }
+ # Likewise, but with startup-with-shell enabled, which is the
+ # default behaviour.
+ -re "During startup program exited with code 127\.\r\nExiting\r\n" {
+ set msg $expect_out(0,string)
+ exp_continue
+ }
+ -re "Can't bind address: Address already in use\.\r\nExiting\r\n" {
+ set msg $expect_out(0,string)
+ exp_continue
+ }
+ # This is what we get on Windows.
+ -re "Error creating process\r\n\r\nExiting\r\n" {
+ set msg $expect_out(0,string)
+ exp_continue
+ }
+ -re "A problem internal to GDBserver has been detected" {
+ set msg $expect_out(0,string)
+ wait
+ }
+ timeout {
error "Timeout waiting for gdbserver response."
}
}
break
}
- return [list $protocol [$get_remote_address $debughost $portnum]]
+ return [list $protocol [$get_remote_address $debughost $portnum] $msg]
}
# Start a gdbserver process running SERVER_EXEC, and connect GDB
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] remote.c: Make packet_check_result return a structure
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 1/6] gdb.server/non-existing-program.exp: Use gdbserver_start Alexandra Hájková
@ 2024-01-19 11:56 ` Alexandra Hájková
2024-02-22 14:37 ` Alexandra Petlanova Hajkova
2024-01-19 11:56 ` [PATCH v2 2/6] gdb/ser-pipe.c: Duplicate the file descriptors Alexandra Hájková
` (5 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
packet_check_result currently returns an packet_result enum.
If GDB will recieve an error in a format E.errtext, which
is possible for some q packets, such errtext is lost if
treated by packet_check_result.
Introduce a new structure which bundles enum packet_result
with possible error message or error code returned by
the GDBserver.
I plan to make more GDBserver response checking functions to use
packet_check_result to make remote.c code more consistent.
This will also allow to use E.errtext more in the future.
There's no infrastructure to test this with a test case so
I tested this by modifying store_registers_using_G function
to get an error message:
[remote] Sending packet: $GG00000000 ...
gdbserver: Wrong sized register packet (expected 1792 bytes, got 1793)
gdbserver: Invalid hex digit 71
Killing process(es): 1104002
[remote] Packet received: E01
Could not write registers; remote failure reply '01'
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
---
v2:
- Added a unit test for packet_check_result.
- Improved formatting and reorganised packet_result structure.
- Renamed pkt_status to pkt_result and result to status varariablese
where suitable.
gdb/remote.c | 147 +++++++++++++++++++++++++++++++++++++++------------
1 file changed, 113 insertions(+), 34 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index dcc1a0d0639..8e51e588bce 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -149,13 +149,46 @@ get_target_type_name (bool target_connected)
/* Analyze a packet's return value and update the packet config
accordingly. */
-enum packet_result
+enum packet_status
{
PACKET_ERROR,
PACKET_OK,
PACKET_UNKNOWN
};
+/* Keeps packet's return value. If packet's return value is PACKET_ERROR,
+ err_msg contains an error message string from E.string or the number
+ stored as a string from E.num. */
+struct packet_result
+{
+ packet_result (enum packet_status status, std::string err_msg):
+ m_status (status), m_err_msg (std::move (err_msg))
+ {
+ gdb_assert (status == PACKET_ERROR);
+ }
+
+ explicit packet_result (enum packet_status status):
+ m_status (status)
+ {
+ gdb_assert (status != PACKET_ERROR);
+ }
+
+ enum packet_status status () const
+ {
+ return this->m_status;
+ }
+
+ const char *err_msg () const
+ {
+ gdb_assert (this->m_status == PACKET_ERROR);
+ return this->m_err_msg.c_str ();
+ }
+
+private:
+ enum packet_status m_status;
+ std::string m_err_msg;
+};
+
/* Enumeration of packets for a remote target. */
enum {
@@ -732,8 +765,8 @@ struct remote_features
/* Check result value in BUF for packet WHICH_PACKET and update the packet's
support configuration accordingly. */
- packet_result packet_ok (const char *buf, const int which_packet);
- packet_result packet_ok (const gdb::char_vector &buf, const int which_packet);
+ packet_status packet_ok (const char *buf, const int which_packet);
+ packet_status packet_ok (const gdb::char_vector &buf, const int which_packet);
/* Configuration of a remote target's memory read packet. */
memory_packet_config m_memory_read_packet_config;
@@ -1254,7 +1287,7 @@ class remote_target : public process_stratum_target
int unit_size,
ULONGEST *xfered_len);
- packet_result remote_send_printf (const char *format, ...)
+ packet_status remote_send_printf (const char *format, ...)
ATTRIBUTE_PRINTF (2, 3);
target_xfer_status remote_flash_write (ULONGEST address,
@@ -2418,7 +2451,10 @@ add_packet_config_cmd (const unsigned int which_packet, const char *name,
}
}
-static enum packet_result
+/* Check GDBserver's reply packet. Return packet_result
+ structure which contains the packet_status enum
+ and an error message for the PACKET_ERROR case. */
+static packet_result
packet_check_result (const char *buf)
{
if (buf[0] != '\0')
@@ -2428,42 +2464,46 @@ packet_check_result (const char *buf)
if (buf[0] == 'E'
&& isxdigit (buf[1]) && isxdigit (buf[2])
&& buf[3] == '\0')
+ {
/* "Enn" - definitely an error. */
- return PACKET_ERROR;
+ return { PACKET_ERROR, buf + 1 };
+ }
/* Always treat "E." as an error. This will be used for
more verbose error messages, such as E.memtypes. */
if (buf[0] == 'E' && buf[1] == '.')
- return PACKET_ERROR;
+ {
+ return { PACKET_ERROR, buf + 2 };
+ }
/* The packet may or may not be OK. Just assume it is. */
- return PACKET_OK;
+ return packet_result ( PACKET_OK );
}
else
+ {
/* The stub does not support the packet. */
- return PACKET_UNKNOWN;
+ return packet_result ( PACKET_UNKNOWN );
+ }
}
-static enum packet_result
+static packet_result
packet_check_result (const gdb::char_vector &buf)
{
return packet_check_result (buf.data ());
}
-packet_result
+packet_status
remote_features::packet_ok (const char *buf, const int which_packet)
{
packet_config *config = &m_protocol_packets[which_packet];
packet_description *descr = &packets_descriptions[which_packet];
- enum packet_result result;
-
if (config->detect != AUTO_BOOLEAN_TRUE
&& config->support == PACKET_DISABLE)
internal_error (_("packet_ok: attempt to use a disabled packet"));
- result = packet_check_result (buf);
- switch (result)
+ packet_result result = packet_check_result (buf);
+ switch (result.status ())
{
case PACKET_OK:
case PACKET_ERROR:
@@ -2498,10 +2538,10 @@ remote_features::packet_ok (const char *buf, const int which_packet)
break;
}
- return result;
+ return result.status ();
}
-packet_result
+packet_status
remote_features::packet_ok (const gdb::char_vector &buf, const int which_packet)
{
return packet_ok (buf.data (), which_packet);
@@ -3000,7 +3040,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
gdb::array_view<const int> syscall_counts)
{
const char *catch_packet;
- enum packet_result result;
+ enum packet_status result;
int n_sysno = 0;
if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
@@ -8791,9 +8831,10 @@ remote_target::send_g_packet ()
xsnprintf (rs->buf.data (), get_remote_packet_size (), "g");
putpkt (rs->buf);
getpkt (&rs->buf);
- if (packet_check_result (rs->buf) == PACKET_ERROR)
+ packet_result result = packet_check_result (rs->buf);
+ if (result.status () == PACKET_ERROR)
error (_("Could not read registers; remote failure reply '%s'"),
- rs->buf.data ());
+ result.err_msg ());
/* We can get out of synch in various cases. If the first character
in the buffer is not a hex character, assume that has happened
@@ -9099,9 +9140,10 @@ remote_target::store_registers_using_G (const struct regcache *regcache)
bin2hex (regs, p, rsa->sizeof_g_packet);
putpkt (rs->buf);
getpkt (&rs->buf);
- if (packet_check_result (rs->buf) == PACKET_ERROR)
+ packet_result pkt_status = packet_check_result (rs->buf);
+ if (pkt_status.status () == PACKET_ERROR)
error (_("Could not write registers; remote failure reply '%s'"),
- rs->buf.data ());
+ pkt_status.err_msg ());
}
/* Store register REGNUM, or all registers if REGNUM == -1, from the contents
@@ -9683,7 +9725,7 @@ remote_target::remote_read_bytes (CORE_ADDR memaddr,
FORMAT and the remaining arguments, then gets the reply. Returns
whether the packet was a success, a failure, or unknown. */
-packet_result
+packet_status
remote_target::remote_send_printf (const char *format, ...)
{
struct remote_state *rs = get_remote_state ();
@@ -9705,8 +9747,9 @@ remote_target::remote_send_printf (const char *format, ...)
rs->buf[0] = '\0';
getpkt (&rs->buf);
+ packet_result pkt_status = packet_check_result (rs->buf);
- return packet_check_result (rs->buf);
+ return pkt_status.status ();
}
/* Flash writing can take quite some time. We'll set
@@ -9718,7 +9761,7 @@ void
remote_target::flash_erase (ULONGEST address, LONGEST length)
{
int addr_size = gdbarch_addr_bit (current_inferior ()->arch ()) / 8;
- enum packet_result ret;
+ enum packet_status ret;
scoped_restore restore_timeout
= make_scoped_restore (&remote_timeout, remote_flash_timeout);
@@ -11308,7 +11351,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
if (target_has_execution ()
&& m_features.packet_support (PACKET_qCRC) != PACKET_DISABLE)
{
- enum packet_result result;
+ enum packet_status status;
/* Make sure the remote is pointing at the right process. */
set_general_process ();
@@ -11324,10 +11367,10 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
getpkt (&rs->buf);
- result = m_features.packet_ok (rs->buf, PACKET_qCRC);
- if (result == PACKET_ERROR)
+ status = m_features.packet_ok (rs->buf, PACKET_qCRC);
+ if (status == PACKET_ERROR)
return -1;
- else if (result == PACKET_OK)
+ else if (status == PACKET_OK)
{
for (target_crc = 0, tmp = &rs->buf[1]; *tmp; tmp++)
target_crc = target_crc * 16 + fromhex (*tmp);
@@ -12210,7 +12253,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
struct remote_state *rs = get_remote_state ();
char *p = rs->buf.data ();
char *endp = p + get_remote_packet_size ();
- enum packet_result result;
+ enum packet_status result;
strcpy (p, "qGetTLSAddr:");
p += strlen (p);
@@ -12256,7 +12299,7 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
struct remote_state *rs = get_remote_state ();
char *p = rs->buf.data ();
char *endp = p + get_remote_packet_size ();
- enum packet_result result;
+ enum packet_status result;
strcpy (p, "qGetTIBAddr:");
p += strlen (p);
@@ -13821,7 +13864,7 @@ remote_target::get_trace_status (struct trace_status *ts)
{
/* Initialize it just to avoid a GCC false warning. */
char *p = NULL;
- enum packet_result result;
+ enum packet_status result;
struct remote_state *rs = get_remote_state ();
if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE)
@@ -14201,7 +14244,7 @@ remote_target::set_trace_buffer_size (LONGEST val)
struct remote_state *rs = get_remote_state ();
char *buf = rs->buf.data ();
char *endbuf = buf + get_remote_packet_size ();
- enum packet_result result;
+ enum packet_status result;
gdb_assert (val >= 0 || val == -1);
buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
@@ -15527,9 +15570,10 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
putpkt (rs->buf);
getpkt (&rs->buf);
+ packet_result pkt_status = packet_check_result (rs->buf);
/* Verify if the request was successful. */
- return packet_check_result (rs->buf.data ()) == PACKET_OK;
+ return pkt_status.status () == PACKET_OK;
}
/* Return true if remote target T is non-stop. */
@@ -15626,6 +15670,39 @@ test_memory_tagging_functions ()
expected.length ()) == 0);
}
+static void
+test_packet_check_result ()
+{
+ std::string buf = "E01";
+
+ packet_result result = packet_check_result (buf.data ());
+ SELF_CHECK (result.status () == PACKET_ERROR);
+
+ buf.assign("E1");
+ result = packet_check_result (buf.data ());
+ SELF_CHECK (result.status () == PACKET_OK);
+
+ buf.assign("E000");
+ result = packet_check_result (buf.data ());
+ SELF_CHECK (result.status () == PACKET_OK);
+
+ buf.assign("E.msg");
+ result = packet_check_result (buf.data ());
+ SELF_CHECK (result.status () == PACKET_ERROR);
+
+ buf.assign("E.");
+ result = packet_check_result (buf.data ());
+ SELF_CHECK (result.status () == PACKET_ERROR);
+
+ buf.assign("some response");
+ result = packet_check_result (buf.data ());
+ SELF_CHECK (result.status () == PACKET_OK);
+
+ buf.assign("");
+ result = packet_check_result (buf.data ());
+ SELF_CHECK (result.status () == PACKET_UNKNOWN);
+}
+
} // namespace selftests
#endif /* GDB_SELF_TEST */
@@ -16125,5 +16202,7 @@ from the target."),
#if GDB_SELF_TEST
selftests::register_test ("remote_memory_tagging",
selftests::test_memory_tagging_functions);
+ selftests::register_test ("packet_check_result",
+ selftests::test_packet_check_result);
#endif
}
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/6] gdb/ser-pipe.c: Duplicate the file descriptors
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 1/6] gdb.server/non-existing-program.exp: Use gdbserver_start Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2] remote.c: Make packet_check_result return a structure Alexandra Hájková
@ 2024-01-19 11:56 ` Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 3/6] Add new vDefaultInferiorFd packet Alexandra Hájková
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
Duplicate the numbers of STDOUT/STDIN/STDERR file descriptors
GDB is connected to. Preserved numbers of the file descriptors
could be then sent to the gdbserver. If gdbserver is run locally
and will accept the numbers of the file descriptors, it can start
the inferior connected to the same STDIN/OUT/ERR, GDB is connected to.
---
gdb/ser-pipe.c | 40 ++++++++++++++++++++++++++++++++++++++++
gdb/serial.c | 4 ++++
gdb/serial.h | 4 ++++
3 files changed, 48 insertions(+)
diff --git a/gdb/ser-pipe.c b/gdb/ser-pipe.c
index 842b656eb5a..849926c179b 100644
--- a/gdb/ser-pipe.c
+++ b/gdb/ser-pipe.c
@@ -77,6 +77,32 @@ pipe_open (struct serial *scb, const char *name)
perror_with_name (_("could not open socket pair"), save);
}
+ /* Preserve STDIN/STDOUT/STDERR so they won't be closed on
+ exec later, after we fork. */
+ int saved_stdin = dup (STDIN_FILENO);
+ int saved_stdout = dup (STDOUT_FILENO);
+ int saved_stderr = dup (STDERR_FILENO);
+ if (saved_stdin == -1 || saved_stdout == -1 || saved_stderr == -1)
+ {
+ /* If any FD failed to dup() then we can't used the default-fd mechanism,
+ so close any files that succeeded. */
+ if (saved_stdin == -1)
+ close (saved_stdin);
+ if (saved_stdout == -1)
+ close (saved_stdout);
+ if (saved_stderr == -1)
+ close (saved_stderr);
+ saved_stdin = saved_stdout = saved_stderr = -1;
+ }
+
+ mark_fd_no_cloexec (saved_stdout);
+ mark_fd_no_cloexec (saved_stdin);
+ mark_fd_no_cloexec (saved_stderr);
+
+ scb->fds[0] = saved_stdin;
+ scb->fds[1] = saved_stdout;
+ scb->fds[2] = saved_stderr;
+
/* Create the child process to run the command in. Note that the
apparent call to vfork() below *might* actually be a call to
fork() due to the fact that autoconf will ``#define vfork fork''
@@ -91,6 +117,12 @@ pipe_open (struct serial *scb, const char *name)
close (pdes[1]);
close (err_pdes[0]);
close (err_pdes[1]);
+ close (saved_stdout);
+ close (saved_stdin);
+ close (saved_stderr);
+ scb->fds[0] = -1;
+ scb->fds[1] = -1;
+ scb->fds[2] = -1;
perror_with_name (_("could not vfork"), save);
}
@@ -140,6 +172,14 @@ pipe_open (struct serial *scb, const char *name)
close (pdes[1]);
if (err_pdes[1] != -1)
close (err_pdes[1]);
+
+ unmark_fd_no_cloexec(saved_stdout);
+ unmark_fd_no_cloexec(saved_stdin);
+ unmark_fd_no_cloexec(saved_stderr);
+ close (saved_stdout);
+ close (saved_stdin);
+ close (saved_stderr);
+
/* :end chunk */
state = XNEW (struct pipe_state);
state->pid = pid;
diff --git a/gdb/serial.c b/gdb/serial.c
index 734a580ed02..8fe7bcfe4b4 100644
--- a/gdb/serial.c
+++ b/gdb/serial.c
@@ -180,6 +180,10 @@ new_serial (const struct serial_ops *ops)
scb->ops = ops;
+ scb->fds[0] = -1;
+ scb->fds[1] = -1;
+ scb->fds[2] = -1;
+
scb->bufp = scb->buf;
scb->error_fd = -1;
scb->refcnt = 1;
diff --git a/gdb/serial.h b/gdb/serial.h
index 69507e69295..bfec99ad7c6 100644
--- a/gdb/serial.h
+++ b/gdb/serial.h
@@ -254,6 +254,10 @@ struct serial
int async_state; /* Async internal state. */
void *async_context; /* Async event thread's context */
serial_event_ftype *async_handler;/* Async event handler */
+
+ /* File descriptors for preserved STDIN/STDOUT/STDERR
+ to be sent to gdbserver when run locally. */
+ int fds[3];
};
struct serial_ops
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/6] Add new vDefaultInferiorFd packet
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
` (2 preceding siblings ...)
2024-01-19 11:56 ` [PATCH v2 2/6] gdb/ser-pipe.c: Duplicate the file descriptors Alexandra Hájková
@ 2024-01-19 11:56 ` Alexandra Hájková
2024-01-19 12:04 ` Eli Zaretskii
2024-01-19 11:56 ` [PATCH v2 4/6] gdbserver/linux-low.cc: Connect the inferior to the terminal Alexandra Hájková
` (3 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
Add a new DefaultInferiorFd feature and the corresponding packet.
This feature allows GDB to send, to GDBserver, the file descriptor
numbers of the terminal to which GDB is connected. The inferior is
then started connected to the same terminal as GDB. This allows the
inferior run by local GDBserver to read from GDB's STDIN and write
its output to GDB's STOUT/ERR the same way as native target.
---
gdb/doc/gdb.texinfo | 42 +++++++++++++
gdb/remote.c | 28 +++++++++
gdbserver/linux-low.cc | 6 ++
gdbserver/linux-low.h | 2 +
gdbserver/server.cc | 135 ++++++++++++++++++++++++++++++++++++++++-
gdbserver/server.h | 12 ++++
gdbserver/target.cc | 6 ++
gdbserver/target.h | 6 ++
8 files changed, 234 insertions(+), 3 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e98c15242bc..8ea8d96f73c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23251,6 +23251,18 @@ If you specify the program to debug on the command line, then the
resume using commands like @kbd{step} and @kbd{continue} as with
@code{target remote} mode.
+When GDBserver is run locally using stdio, for example, with
+@w{@kbd{target remote | gdbserver - @var{inferior}}}, the inferior's
+@code{stdin} is closed, which means the inferior can't read any
+input.
+
+If GDBserver supports the default inferior file descriptors feature
+(@pxref{default inferior file descriptors}), it will start the
+inferior connected to the same terminal as @value{GDBN}, which
+enables the inferior to read from @code{stdin} and write to
+@code{stdout} and @code{stderr}, in the same way native targets do.
+
+
@anchor{Attaching in Types of Remote Connections}
@item Attaching
@strong{With target remote mode:} The @value{GDBN} command @code{attach} is
@@ -43114,6 +43126,36 @@ for success (@pxref{Stop Reply Packets})
@cindex @samp{vStopped} packet
@xref{Notification Packets}.
+@anchor{default inferior file descriptors}
+@cindex @samp{vDefaultInferiorFd} packet
+@item vDefaultInferiorFd;@var{fd0};@var{fd1};@var{fd2}
+@itemx vDefaultInferiorFd
+vDefaultInferiorFd packet contains the file descriptors of the same @code{stdin}/
+@code{stdout}/@code{stderr} @value{GDBN} is connected to. If GDBserver is run
+locally, it will accept the packet and will use the file descriptors sent by @value{GDBN}
+to start the inferior connected to the same @code{stdin}/@code{stdout}/@code{stderr}
+@value{GDBN} is connected to. This allows the inferior run under the local GDBserver
+to read from the @code{stdin} and write to @code{stdout}/@code{stderr}.
+This makes a remote target (which is run locally) to behave similarly to the native target.
+
+When this feature it not used, inferior is started with its @code{stdin} closed and its
+@code{stdout} is redirected to @code{stderr} @value{GDBN} is connected to.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
+This packet is also available in extended mode (@pxref{extended mode}).
+This feature does not support @code{set inferior-tty} command.
+At the time of writing this feature only works on Linux.
+
+Reply:
+@table @samp
+@item OK
+for success
+@item E.errtext
+for an error
+@end table
+
@item X @var{addr},@var{length}:@var{XX@dots{}}
@anchor{X packet}
@cindex @samp{X} packet
diff --git a/gdb/remote.c b/gdb/remote.c
index 72f14e28f54..d90e071f4fb 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -305,6 +305,10 @@ enum {
packets and the tag violation stop replies. */
PACKET_memory_tagging_feature,
+ /* Support for connecting inferior to the same terminal as
+ GDB when running GDBserver locally. */
+ PACKET_vDefaultInferiorFd,
+
PACKET_MAX
};
@@ -726,6 +730,11 @@ struct remote_features
bool remote_memory_tagging_p () const
{ return packet_support (PACKET_memory_tagging_feature) == PACKET_ENABLE; }
+ /* Returns true if connecting inferior to the same terminal as GDB is
+ supported, false otherwise. */
+ bool remote_fd_switch_p () const
+ { return packet_support (PACKET_vDefaultInferiorFd) == PACKET_ENABLE; }
+
/* Reset all packets back to "unknown support". Called when opening a
new connection to a remote target. */
void reset_all_packet_configs_support ();
@@ -5016,6 +5025,18 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
which later probes to skip. */
remote_query_supported ();
+ if (m_features.packet_support (PACKET_vDefaultInferiorFd) != PACKET_DISABLE
+ && rs->remote_desc->fds[0] != -1 && rs->remote_desc->fds[1] != -1
+ && rs->remote_desc->fds[2] != -1)
+ {
+ xsnprintf (rs->buf.data(), rs->buf.size (), "vDefaultInferiorFd;%d;%d;%d",
+ rs->remote_desc->fds[0], rs->remote_desc->fds[1],
+ rs->remote_desc->fds[2]);
+
+ putpkt (rs->buf);
+ getpkt (&rs->buf, 0);
+ }
+
/* Check vCont support and set the remote state's vCont_action_support
attribute. */
remote_vcont_probe ();
@@ -5723,6 +5744,7 @@ static const struct protocol_feature remote_protocol_features[] = {
{ "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed },
{ "memory-tagging", PACKET_DISABLE, remote_supported_packet,
PACKET_memory_tagging_feature },
+ { "vDefaultInferiorFd", PACKET_DISABLE, remote_supported_packet, PACKET_vDefaultInferiorFd },
};
static char *remote_support_xml;
@@ -5834,6 +5856,10 @@ remote_target::remote_query_supported ()
!= AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "memory-tagging+");
+ if (m_features.packet_set_cmd_state (PACKET_vDefaultInferiorFd)
+ != AUTO_BOOLEAN_FALSE)
+ remote_query_supported_append (&q, "vDefaultInferiorFd+");
+
/* Keep this one last to work around a gdbserver <= 7.10 bug in
the qSupported:xmlRegisters=i386 handling. */
if (remote_support_xml != NULL
@@ -15990,6 +16016,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (PACKET_memory_tagging_feature,
"memory-tagging-feature", "memory-tagging-feature", 0);
+ add_packet_config_cmd (PACKET_vDefaultInferiorFd, "vDefaultInferiorFd", "vDefaultInferiorFd", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 444eebc6bbe..dc3d2629fdb 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -6199,6 +6199,12 @@ linux_process_target::low_supports_catch_syscall ()
return false;
}
+bool
+linux_process_target::supports_default_fds ()
+{
+ return true;
+}
+
CORE_ADDR
linux_process_target::read_pc (regcache *regcache)
{
diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
index d34d2738238..dfbd079e223 100644
--- a/gdbserver/linux-low.h
+++ b/gdbserver/linux-low.h
@@ -322,6 +322,8 @@ class linux_process_target : public process_stratum_target
bool supports_catch_syscall () override;
+ bool supports_default_fds () override;
+
/* Return the information to access registers. This has public
visibility because proc-service uses it. */
virtual const regs_info *get_regs_info () = 0;
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 74c7763d777..f9ce02a8594 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -81,6 +81,12 @@ static bool extended_protocol;
static bool response_needed;
static bool exit_requested;
+/* To be able to connect the inferior to GDB's stdin/out/err,
+ when GDBserver is run locally, we need to wait with starting
+ the inferior to have time to recieve the vDefaultInferiorFd
+ packet. Set to true if need to postpone starting the inferior. */
+static bool deferred_inferior_startup = false;
+
/* --once: Exit after the first connection has closed. */
bool run_once;
@@ -171,6 +177,34 @@ get_client_state ()
return cs;
}
+int multi_mode = 0;
+
+/* To be able to connect the inferior to GDB's stdin/out/err,
+ when GDBserver is run locally, we need to wait with starting
+ the inferior to have time to recieve the vDefaultInferiorFd
+ packet.
+
+ Can only be called when deferred_inferior_startup is true.
+ Starts the inferior and throws an error if startup fails.
+ If startup is successful then deferred_inferior_startup is
+ reset to false. Throw an error on failure. */
+static void
+do_deferred_startup ()
+{
+ gdb_assert (deferred_inferior_startup);
+ client_state &cs = get_client_state ();
+ target_create_inferior (program_path.get (), program_args);
+
+ if (current_thread != nullptr)
+ current_process ()->dlls_changed = false;
+
+ if ((cs.last_status.kind () == TARGET_WAITKIND_EXITED
+ || cs.last_status.kind () == TARGET_WAITKIND_SIGNALLED)
+ && !multi_mode)
+ error ("No program to debug");
+
+ deferred_inferior_startup = false;
+}
/* Put a stop reply to the stop reply queue. */
@@ -2501,6 +2535,45 @@ supported_btrace_packets (char *buf)
strcat (buf, ";qXfer:btrace-conf:read+");
}
+/* Parse a vDefaultInferiorFd packet from pkt
+ and save the received file descriptors into
+ fds. Return true on success, false otherwise.
+ Set the file descriptors to -1 on failure. */
+static bool
+parse_vdefaultinf (const char *pkt, int *fds)
+{
+ char *end;
+ int ret = true;
+ errno = 0;
+
+ if (*pkt != '\0')
+ {
+ fds[0] = (int) strtoul (pkt, &end, 10);
+ if ((pkt == end) || (*end != ';'))
+ ret = false;
+ pkt = end + 1;
+ fds[1] = (int) strtoul (pkt, &end, 10);
+ if ((pkt == end) || (*end != ';'))
+ ret = false;
+ pkt = end + 1;
+ fds[2] = (int) strtoul (pkt, &end, 10);
+ if ((pkt == end) || (*end != '\0'))
+ ret = false;
+ if (errno != 0)
+ ret = false;
+ }
+ else
+ ret = false;
+ if (!ret)
+ {
+ fds[0] = -1;
+ fds[1] = -1;
+ fds[2] = -1;
+ }
+
+ return ret;
+}
+
/* Handle all of the extended 'q' packets. */
static void
@@ -2711,6 +2784,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_memory_tagging ())
cs.memory_tagging_feature = true;
}
+ else if (feature == "vDefaultInferiorFd+")
+ cs.vDefaultInferiorFd_supported = 1;
else
{
/* Move the unknown features all together. */
@@ -2840,6 +2915,18 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_memory_tagging ())
strcat (own_buf, ";memory-tagging+");
+ if (cs.vDefaultInferiorFd_supported)
+ {
+ /* If GDB didn't advertise vDefaultInferiorFd then we don't mention
+ vDefaultInferiorFd in the reply. If GDB did mention vDefaultInferiorFd
+ then we only claim support if we are started as an stdio target. */
+ if (remote_connection_is_stdio ()
+ && target_supports_default_fds ())
+ strcat (own_buf, ";vDefaultInferiorFd+");
+ else
+ strcat (own_buf, ";vDefaultInferiorFd-");
+ }
+
/* Reinitialize components as needed for the new connection. */
hostio_handle_new_gdb_connection ();
target_handle_new_gdb_connection ();
@@ -3543,6 +3630,19 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
return;
}
+ if (startswith (own_buf, "vDefaultInferiorFd;"))
+ {
+ if (!parse_vdefaultinf (own_buf + 19, cs.fds))
+ {
+ strcpy (own_buf, "E.invalid vDefaultInferiorFd packet");
+ return;
+ }
+
+ cs.vDefaultInferiorFd_accepted = true;
+ write_ok (own_buf);
+ return;
+ }
+
if (startswith (own_buf, "vKill;"))
{
if (!target_running ())
@@ -4038,7 +4138,6 @@ captured_main (int argc, char *argv[])
char *arg_end;
const char *port = NULL;
char **next_arg = &argv[1];
- volatile int multi_mode = 0;
volatile int attach = 0;
int was_running;
bool selftest = false;
@@ -4307,8 +4406,27 @@ captured_main (int argc, char *argv[])
for (i = 1; i < n; i++)
program_args.push_back (xstrdup (next_arg[i]));
- /* Wait till we are at first instruction in program. */
- target_create_inferior (program_path.get (), program_args);
+ if (remote_connection_is_stdio ())
+ {
+ /* Debugger might want to set the default inferior
+ in/out/err file descriptors, in which case we need to
+ defer starting the inferior until this information
+ arrives. */
+ deferred_inferior_startup = true;
+ multi_mode = 1;
+
+ /* Only used when reporting the first stop to GDB. This
+ should be replaced when we start the inferior, but lets
+ be on the safe side and set this now just so we carry
+ around a sane state. */
+ cs.last_status.set_exited (0);
+ cs.last_ptid = minus_one_ptid;
+ }
+ else
+ {
+ /* Wait till we are at first instruction in program. */
+ target_create_inferior (program_path.get (), program_args);
+ }
/* We are now (hopefully) stopped at the first instruction of
the target process. This assumes that the target process was
@@ -4358,6 +4476,7 @@ captured_main (int argc, char *argv[])
cs.hwbreak_feature = 0;
cs.vCont_supported = 0;
cs.memory_tagging_feature = false;
+ cs.vDefaultInferiorFd_supported = false;
remote_open (port);
@@ -4547,6 +4666,16 @@ process_serial_event (void)
response_needed = true;
char ch = cs.own_buf[0];
+ if (deferred_inferior_startup)
+ {
+ /* Actually start the inferior if not a qSupported or
+ vDefaultInferiorFd packet. vDefaultInferiorFd is
+ only accepted after qSupported and before any other
+ request. */
+ if (!startswith(cs.own_buf, "qSupported")
+ && !startswith(cs.own_buf, "vDefaultInferiorFd"))
+ do_deferred_startup ();
+ }
switch (ch)
{
case 'q':
diff --git a/gdbserver/server.h b/gdbserver/server.h
index 0074818c6df..dacc5cb5d6f 100644
--- a/gdbserver/server.h
+++ b/gdbserver/server.h
@@ -192,6 +192,18 @@ struct client_state
/* If true, memory tagging features are supported. */
bool memory_tagging_feature = false;
+ /* If true, connecting the inferior to the terminal's
+ stdin/out/err when running GDBserver locally is supported. */
+ bool vDefaultInferiorFd_supported = false;
+
+ /* If true, GDBserver accepted file descriptors sent by GDB
+ and connecting the inferior to the same terminal as GDB
+ when running GDBserver locally is then supported. */
+ bool vDefaultInferiorFd_accepted = false;
+
+ /* File descriptors pointing to terminal's stdin/out/err
+ sent by GDB to GDBserver. */
+ int fds[3];
};
client_state &get_client_state ();
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index 23b699d33bb..f19edb67dc5 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -840,6 +840,12 @@ process_stratum_target::supports_catch_syscall ()
return false;
}
+bool
+process_stratum_target::supports_default_fds ()
+{
+ return false;
+}
+
int
process_stratum_target::get_ipa_tdesc_idx ()
{
diff --git a/gdbserver/target.h b/gdbserver/target.h
index 3643b9110da..01d2882a7e1 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -497,6 +497,9 @@ class process_stratum_target
/* Return true if the target supports catch syscall. */
virtual bool supports_catch_syscall ();
+ /* Return true if the target supports vDefaultInferiorFd. */
+ virtual bool supports_default_fds ();
+
/* Return tdesc index for IPA. */
virtual int get_ipa_tdesc_idx ();
@@ -578,6 +581,9 @@ int kill_inferior (process_info *proc);
#define target_supports_catch_syscall() \
the_target->supports_catch_syscall ()
+#define target_supports_default_fds() \
+ the_target->supports_default_fds ()
+
#define target_get_ipa_tdesc_idx() \
the_target->get_ipa_tdesc_idx ()
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/6] gdbserver/linux-low.cc: Connect the inferior to the terminal
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
` (3 preceding siblings ...)
2024-01-19 11:56 ` [PATCH v2 3/6] Add new vDefaultInferiorFd packet Alexandra Hájková
@ 2024-01-19 11:56 ` Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 5/6] remote.c: Add terminal handling functions Alexandra Hájková
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
If GDBserver is connected to GDB via stdio and it recieved file
descriptors of the STDIN/OUT/ERR of the terminal GDB is connected to,
GDBserver will start the inferior connected to them.
---
gdbserver/linux-low.cc | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index dc3d2629fdb..2e91f00da24 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -954,6 +954,8 @@ linux_process_target::low_new_thread (lwp_info *info)
static void
linux_ptrace_fun ()
{
+ client_state &cs = get_client_state ();
+
if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
(PTRACE_TYPE_ARG4) 0) < 0)
trace_start_error_with_name ("ptrace");
@@ -961,10 +963,38 @@ linux_ptrace_fun ()
if (setpgid (0, 0) < 0)
trace_start_error_with_name ("setpgid");
+ /* If GDBserver is connected to GDB via stdio and it recieved the file
+ descriptors for the terminal stdin/out/err from GDB, start the inferior
+ connected to them. */
+ if (remote_connection_is_stdio () && cs.vDefaultInferiorFd_accepted)
+ {
+ struct stat buf{};
+
+ if ((fstat(cs.fds[0], &buf) == -1)
+ || (fstat(cs.fds[1], &buf) == -1)
+ || (fstat(cs.fds[2], &buf) == -1))
+ {
+ write (2, "Fstat failed. Can't connect inferior to the terminal.\n",
+ sizeof ("Fstat failed. Can't connect inferior to the terminal.\n") - 1);
+ cs.vDefaultInferiorFd_accepted = false;
+ }
+
+ /* Dupped file descriptors will be inherited by the inferior. */
+ if (!cs.vDefaultInferiorFd_accepted
+ || (dup2 (cs.fds[0], 0) == -1)
+ || (dup2 (cs.fds[1], 1) == -1)
+ || (dup2 (cs.fds[2], 2) == -1))
+ {
+ write (2, "Dup2 failed. Can't connect inferior to the terminal.\n",
+ sizeof ("Dup2 failed. Can't connect inferior to the terminal.\n") - 1);
+ cs.vDefaultInferiorFd_accepted = false;
+ }
+ }
+
/* If GDBserver is connected to gdb via stdio, redirect the inferior's
stdout to stderr so that inferior i/o doesn't corrupt the connection.
Also, redirect stdin to /dev/null. */
- if (remote_connection_is_stdio ())
+ if (remote_connection_is_stdio () && !cs.vDefaultInferiorFd_accepted)
{
if (close (0) < 0)
trace_start_error_with_name ("close");
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 5/6] remote.c: Add terminal handling functions
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
` (4 preceding siblings ...)
2024-01-19 11:56 ` [PATCH v2 4/6] gdbserver/linux-low.cc: Connect the inferior to the terminal Alexandra Hájková
@ 2024-01-19 11:56 ` Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 6/6] Add defaultinf.exp test to the testsuite Alexandra Hájková
2024-02-22 14:38 ` [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Petlanova Hajkova
7 siblings, 0 replies; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
Re-use terminal handling functions from inf-child.c so GDBserver can
call them. This helps local remote target to behave more like native
target.
---
gdb/remote.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/gdb/remote.c b/gdb/remote.c
index d90e071f4fb..3406eaf43e7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -607,6 +607,10 @@ class remote_state
qSupported. */
gdb_thread_options supported_thread_options = 0;
+ /* gdbserver accepted the file descriptor numbers of the terminal
+ GDB is connected to. */
+ bool fds_accepted = false;
+
private:
/* Asynchronous signal handle registered as event loop source for
when we have pending events ready to be passed to the core. */
@@ -908,10 +912,18 @@ class remote_target : public process_stratum_target
int can_do_single_step () override;
+ bool supports_terminal_ours () override;
+
+ void terminal_init () override;
+
void terminal_inferior () override;
+ void terminal_save_inferior() override;
+
void terminal_ours () override;
+ void terminal_ours_for_output() override;
+
bool supports_non_stop () override;
bool supports_multi_process () override;
@@ -1731,6 +1743,36 @@ remote_target::get_remote_state ()
return &m_remote_state;
}
+bool
+remote_target::supports_terminal_ours ()
+{
+ return true;
+}
+
+void
+remote_target::terminal_init ()
+{
+ struct remote_state *rs = get_remote_state ();
+ if (rs->fds_accepted)
+ child_terminal_init (this);
+}
+
+void
+remote_target::terminal_save_inferior ()
+{
+ struct remote_state *rs = get_remote_state ();
+ if (rs->fds_accepted)
+ child_terminal_save_inferior (this);
+}
+
+void
+remote_target::terminal_ours_for_output ()
+{
+ struct remote_state *rs = get_remote_state ();
+ if (rs->fds_accepted)
+ child_terminal_ours_for_output (this);
+}
+
/* Fetch the remote exec-file from the current program space. */
static const char *
@@ -5035,6 +5077,12 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
putpkt (rs->buf);
getpkt (&rs->buf, 0);
+
+ if (m_features.packet_ok (rs->buf, PACKET_vDefaultInferiorFd) == PACKET_OK)
+ rs->fds_accepted = true;
+ else
+ error (_("Remote replied unexpectedly to 'vDefaultInferiorFd': %s"),
+ rs->buf.data ());
}
/* Check vCont support and set the remote state's vCont_action_support
@@ -7565,11 +7613,18 @@ remote_target::terminal_inferior ()
/* NOTE: At this point we could also register our selves as the
recipient of all input. Any characters typed could then be
passed on down to the target. */
+
+ struct remote_state *rs = get_remote_state ();
+ if (rs->fds_accepted)
+ child_terminal_inferior (this);
}
void
remote_target::terminal_ours ()
{
+ struct remote_state *rs = get_remote_state ();
+ if (rs->fds_accepted)
+ child_terminal_ours (this);
}
static void
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 6/6] Add defaultinf.exp test to the testsuite
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
` (5 preceding siblings ...)
2024-01-19 11:56 ` [PATCH v2 5/6] remote.c: Add terminal handling functions Alexandra Hájková
@ 2024-01-19 11:56 ` Alexandra Hájková
2024-02-22 14:38 ` [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Petlanova Hajkova
7 siblings, 0 replies; 12+ messages in thread
From: Alexandra Hájková @ 2024-01-19 11:56 UTC (permalink / raw)
To: gdb-patches
Test the behaviour of the inferior when GDBserver is run locally
and using stdio. The inferior should be able to write to STOUT
and STDERR and read from STDIN.
---
gdb/testsuite/gdb.server/defaultinf.c | 39 ++++++++++++++++
gdb/testsuite/gdb.server/defaultinf.exp | 59 +++++++++++++++++++++++++
2 files changed, 98 insertions(+)
create mode 100644 gdb/testsuite/gdb.server/defaultinf.c
create mode 100644 gdb/testsuite/gdb.server/defaultinf.exp
diff --git a/gdb/testsuite/gdb.server/defaultinf.c b/gdb/testsuite/gdb.server/defaultinf.c
new file mode 100644
index 00000000000..8963c7dbabf
--- /dev/null
+++ b/gdb/testsuite/gdb.server/defaultinf.c
@@ -0,0 +1,39 @@
+/* Copyright 2023 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/>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+int
+main ()
+{
+ printf ("Hello stdout!\n");
+ fprintf (stderr, "Hello stderr!\n");
+
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t nread;
+
+ while ((nread = getline (&line, &len, stdin)) != -1)
+ {
+ if (line[nread - 1] == '\n')
+ line[nread - 1] = '\0';
+ printf ("Got line len %zd: '%s'\n", nread, line);
+ }
+
+ printf ("Got EOF\n");
+ free (line);
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.server/defaultinf.exp b/gdb/testsuite/gdb.server/defaultinf.exp
new file mode 100644
index 00000000000..03163e150e0
--- /dev/null
+++ b/gdb/testsuite/gdb.server/defaultinf.exp
@@ -0,0 +1,59 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2023 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 the behaviour of the inferior when GDBserver is run locally
+# and using stdio. The inferior should be able to write to STOUT
+# and STDERR and read from STDIN.
+
+require {!is_remote target} {!is_remote host}
+
+load_lib gdbserver-support.exp
+
+require allow_gdbserver_tests
+
+set gdbserver [find_gdbserver]
+if { $gdbserver == "" } {
+ unsupported "could not find GDBserver"
+ return
+}
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
+ return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test "target remote | ${::gdbserver} - [standard_output_file $testfile]" ".*" \
+ "start gdbserver using pipe syntax"
+
+ set input_msg "hello world"
+ set input_len [expr [string length ${input_msg}] + 1]
+ gdb_test_multiple "c" "Test STDOUT/ERR, STDIN" {
+ -re "Hello stdout!\r\nHello stderr!" {
+ send_gdb "${input_msg}\n"
+ exp_continue
+ }
+ -re "${input_msg}\r\nGot line len ${input_len}: '${input_msg}'" {
+ send_gdb "\004"
+ exp_continue
+ }
+ -re "Got EOF" {
+ pass "$gdb_test_name"
+ }
+ }
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/6] Add new vDefaultInferiorFd packet
2024-01-19 11:56 ` [PATCH v2 3/6] Add new vDefaultInferiorFd packet Alexandra Hájková
@ 2024-01-19 12:04 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2024-01-19 12:04 UTC (permalink / raw)
To: Alexandra Hájková; +Cc: gdb-patches
> From: Alexandra Hájková <ahajkova@redhat.com>
> Date: Fri, 19 Jan 2024 12:56:27 +0100
>
> Add a new DefaultInferiorFd feature and the corresponding packet.
> This feature allows GDB to send, to GDBserver, the file descriptor
> numbers of the terminal to which GDB is connected. The inferior is
> then started connected to the same terminal as GDB. This allows the
> inferior run by local GDBserver to read from GDB's STDIN and write
> its output to GDB's STOUT/ERR the same way as native target.
> ---
> gdb/doc/gdb.texinfo | 42 +++++++++++++
> gdb/remote.c | 28 +++++++++
> gdbserver/linux-low.cc | 6 ++
> gdbserver/linux-low.h | 2 +
> gdbserver/server.cc | 135 ++++++++++++++++++++++++++++++++++++++++-
> gdbserver/server.h | 12 ++++
> gdbserver/target.cc | 6 ++
> gdbserver/target.h | 6 ++
> 8 files changed, 234 insertions(+), 3 deletions(-)
The documentation part of this is okay, thanks.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] remote.c: Make packet_check_result return a structure
2024-01-19 11:56 ` [PATCH v2] remote.c: Make packet_check_result return a structure Alexandra Hájková
@ 2024-02-22 14:37 ` Alexandra Petlanova Hajkova
2024-02-23 16:19 ` Tom Tromey
0 siblings, 1 reply; 12+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-02-22 14:37 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 13868 bytes --]
ping
(this still applies cleanly)
On Fri, Jan 19, 2024 at 12:57 PM Alexandra Hájková <ahajkova@redhat.com>
wrote:
> packet_check_result currently returns an packet_result enum.
> If GDB will recieve an error in a format E.errtext, which
> is possible for some q packets, such errtext is lost if
> treated by packet_check_result.
> Introduce a new structure which bundles enum packet_result
> with possible error message or error code returned by
> the GDBserver.
> I plan to make more GDBserver response checking functions to use
> packet_check_result to make remote.c code more consistent.
> This will also allow to use E.errtext more in the future.
>
> There's no infrastructure to test this with a test case so
> I tested this by modifying store_registers_using_G function
> to get an error message:
>
> [remote] Sending packet: $GG00000000 ...
>
> gdbserver: Wrong sized register packet (expected 1792 bytes, got 1793)
> gdbserver: Invalid hex digit 71
> Killing process(es): 1104002
> [remote] Packet received: E01
> Could not write registers; remote failure reply '01'
>
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> ---
> v2:
> - Added a unit test for packet_check_result.
> - Improved formatting and reorganised packet_result structure.
> - Renamed pkt_status to pkt_result and result to status varariablese
> where suitable.
>
> gdb/remote.c | 147 +++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 113 insertions(+), 34 deletions(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index dcc1a0d0639..8e51e588bce 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -149,13 +149,46 @@ get_target_type_name (bool target_connected)
> /* Analyze a packet's return value and update the packet config
> accordingly. */
>
> -enum packet_result
> +enum packet_status
> {
> PACKET_ERROR,
> PACKET_OK,
> PACKET_UNKNOWN
> };
>
> +/* Keeps packet's return value. If packet's return value is PACKET_ERROR,
> + err_msg contains an error message string from E.string or the number
> + stored as a string from E.num. */
> +struct packet_result
> +{
> + packet_result (enum packet_status status, std::string err_msg):
> + m_status (status), m_err_msg (std::move (err_msg))
> + {
> + gdb_assert (status == PACKET_ERROR);
> + }
> +
> + explicit packet_result (enum packet_status status):
> + m_status (status)
> + {
> + gdb_assert (status != PACKET_ERROR);
> + }
> +
> + enum packet_status status () const
> + {
> + return this->m_status;
> + }
> +
> + const char *err_msg () const
> + {
> + gdb_assert (this->m_status == PACKET_ERROR);
> + return this->m_err_msg.c_str ();
> + }
> +
> +private:
> + enum packet_status m_status;
> + std::string m_err_msg;
> +};
> +
> /* Enumeration of packets for a remote target. */
>
> enum {
> @@ -732,8 +765,8 @@ struct remote_features
>
> /* Check result value in BUF for packet WHICH_PACKET and update the
> packet's
> support configuration accordingly. */
> - packet_result packet_ok (const char *buf, const int which_packet);
> - packet_result packet_ok (const gdb::char_vector &buf, const int
> which_packet);
> + packet_status packet_ok (const char *buf, const int which_packet);
> + packet_status packet_ok (const gdb::char_vector &buf, const int
> which_packet);
>
> /* Configuration of a remote target's memory read packet. */
> memory_packet_config m_memory_read_packet_config;
> @@ -1254,7 +1287,7 @@ class remote_target : public process_stratum_target
> int unit_size,
> ULONGEST *xfered_len);
>
> - packet_result remote_send_printf (const char *format, ...)
> + packet_status remote_send_printf (const char *format, ...)
> ATTRIBUTE_PRINTF (2, 3);
>
> target_xfer_status remote_flash_write (ULONGEST address,
> @@ -2418,7 +2451,10 @@ add_packet_config_cmd (const unsigned int
> which_packet, const char *name,
> }
> }
>
> -static enum packet_result
> +/* Check GDBserver's reply packet. Return packet_result
> + structure which contains the packet_status enum
> + and an error message for the PACKET_ERROR case. */
> +static packet_result
> packet_check_result (const char *buf)
> {
> if (buf[0] != '\0')
> @@ -2428,42 +2464,46 @@ packet_check_result (const char *buf)
> if (buf[0] == 'E'
> && isxdigit (buf[1]) && isxdigit (buf[2])
> && buf[3] == '\0')
> + {
> /* "Enn" - definitely an error. */
> - return PACKET_ERROR;
> + return { PACKET_ERROR, buf + 1 };
> + }
>
> /* Always treat "E." as an error. This will be used for
> more verbose error messages, such as E.memtypes. */
> if (buf[0] == 'E' && buf[1] == '.')
> - return PACKET_ERROR;
> + {
> + return { PACKET_ERROR, buf + 2 };
> + }
>
> /* The packet may or may not be OK. Just assume it is. */
> - return PACKET_OK;
> + return packet_result ( PACKET_OK );
> }
> else
> + {
> /* The stub does not support the packet. */
> - return PACKET_UNKNOWN;
> + return packet_result ( PACKET_UNKNOWN );
> + }
> }
>
> -static enum packet_result
> +static packet_result
> packet_check_result (const gdb::char_vector &buf)
> {
> return packet_check_result (buf.data ());
> }
>
> -packet_result
> +packet_status
> remote_features::packet_ok (const char *buf, const int which_packet)
> {
> packet_config *config = &m_protocol_packets[which_packet];
> packet_description *descr = &packets_descriptions[which_packet];
>
> - enum packet_result result;
> -
> if (config->detect != AUTO_BOOLEAN_TRUE
> && config->support == PACKET_DISABLE)
> internal_error (_("packet_ok: attempt to use a disabled packet"));
>
> - result = packet_check_result (buf);
> - switch (result)
> + packet_result result = packet_check_result (buf);
> + switch (result.status ())
> {
> case PACKET_OK:
> case PACKET_ERROR:
> @@ -2498,10 +2538,10 @@ remote_features::packet_ok (const char *buf, const
> int which_packet)
> break;
> }
>
> - return result;
> + return result.status ();
> }
>
> -packet_result
> +packet_status
> remote_features::packet_ok (const gdb::char_vector &buf, const int
> which_packet)
> {
> return packet_ok (buf.data (), which_packet);
> @@ -3000,7 +3040,7 @@ remote_target::set_syscall_catchpoint (int pid, bool
> needed, int any_count,
> gdb::array_view<const int>
> syscall_counts)
> {
> const char *catch_packet;
> - enum packet_result result;
> + enum packet_status result;
> int n_sysno = 0;
>
> if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
> @@ -8791,9 +8831,10 @@ remote_target::send_g_packet ()
> xsnprintf (rs->buf.data (), get_remote_packet_size (), "g");
> putpkt (rs->buf);
> getpkt (&rs->buf);
> - if (packet_check_result (rs->buf) == PACKET_ERROR)
> + packet_result result = packet_check_result (rs->buf);
> + if (result.status () == PACKET_ERROR)
> error (_("Could not read registers; remote failure reply '%s'"),
> - rs->buf.data ());
> + result.err_msg ());
>
> /* We can get out of synch in various cases. If the first character
> in the buffer is not a hex character, assume that has happened
> @@ -9099,9 +9140,10 @@ remote_target::store_registers_using_G (const
> struct regcache *regcache)
> bin2hex (regs, p, rsa->sizeof_g_packet);
> putpkt (rs->buf);
> getpkt (&rs->buf);
> - if (packet_check_result (rs->buf) == PACKET_ERROR)
> + packet_result pkt_status = packet_check_result (rs->buf);
> + if (pkt_status.status () == PACKET_ERROR)
> error (_("Could not write registers; remote failure reply '%s'"),
> - rs->buf.data ());
> + pkt_status.err_msg ());
> }
>
> /* Store register REGNUM, or all registers if REGNUM == -1, from the
> contents
> @@ -9683,7 +9725,7 @@ remote_target::remote_read_bytes (CORE_ADDR memaddr,
> FORMAT and the remaining arguments, then gets the reply. Returns
> whether the packet was a success, a failure, or unknown. */
>
> -packet_result
> +packet_status
> remote_target::remote_send_printf (const char *format, ...)
> {
> struct remote_state *rs = get_remote_state ();
> @@ -9705,8 +9747,9 @@ remote_target::remote_send_printf (const char
> *format, ...)
>
> rs->buf[0] = '\0';
> getpkt (&rs->buf);
> + packet_result pkt_status = packet_check_result (rs->buf);
>
> - return packet_check_result (rs->buf);
> + return pkt_status.status ();
> }
>
> /* Flash writing can take quite some time. We'll set
> @@ -9718,7 +9761,7 @@ void
> remote_target::flash_erase (ULONGEST address, LONGEST length)
> {
> int addr_size = gdbarch_addr_bit (current_inferior ()->arch ()) / 8;
> - enum packet_result ret;
> + enum packet_status ret;
> scoped_restore restore_timeout
> = make_scoped_restore (&remote_timeout, remote_flash_timeout);
>
> @@ -11308,7 +11351,7 @@ remote_target::verify_memory (const gdb_byte
> *data, CORE_ADDR lma, ULONGEST size
> if (target_has_execution ()
> && m_features.packet_support (PACKET_qCRC) != PACKET_DISABLE)
> {
> - enum packet_result result;
> + enum packet_status status;
>
> /* Make sure the remote is pointing at the right process. */
> set_general_process ();
> @@ -11324,10 +11367,10 @@ remote_target::verify_memory (const gdb_byte
> *data, CORE_ADDR lma, ULONGEST size
>
> getpkt (&rs->buf);
>
> - result = m_features.packet_ok (rs->buf, PACKET_qCRC);
> - if (result == PACKET_ERROR)
> + status = m_features.packet_ok (rs->buf, PACKET_qCRC);
> + if (status == PACKET_ERROR)
> return -1;
> - else if (result == PACKET_OK)
> + else if (status == PACKET_OK)
> {
> for (target_crc = 0, tmp = &rs->buf[1]; *tmp; tmp++)
> target_crc = target_crc * 16 + fromhex (*tmp);
> @@ -12210,7 +12253,7 @@ remote_target::get_thread_local_address (ptid_t
> ptid, CORE_ADDR lm,
> struct remote_state *rs = get_remote_state ();
> char *p = rs->buf.data ();
> char *endp = p + get_remote_packet_size ();
> - enum packet_result result;
> + enum packet_status result;
>
> strcpy (p, "qGetTLSAddr:");
> p += strlen (p);
> @@ -12256,7 +12299,7 @@ remote_target::get_tib_address (ptid_t ptid,
> CORE_ADDR *addr)
> struct remote_state *rs = get_remote_state ();
> char *p = rs->buf.data ();
> char *endp = p + get_remote_packet_size ();
> - enum packet_result result;
> + enum packet_status result;
>
> strcpy (p, "qGetTIBAddr:");
> p += strlen (p);
> @@ -13821,7 +13864,7 @@ remote_target::get_trace_status (struct
> trace_status *ts)
> {
> /* Initialize it just to avoid a GCC false warning. */
> char *p = NULL;
> - enum packet_result result;
> + enum packet_status result;
> struct remote_state *rs = get_remote_state ();
>
> if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE)
> @@ -14201,7 +14244,7 @@ remote_target::set_trace_buffer_size (LONGEST val)
> struct remote_state *rs = get_remote_state ();
> char *buf = rs->buf.data ();
> char *endbuf = buf + get_remote_packet_size ();
> - enum packet_result result;
> + enum packet_status result;
>
> gdb_assert (val >= 0 || val == -1);
> buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
> @@ -15527,9 +15570,10 @@ remote_target::store_memtags (CORE_ADDR address,
> size_t len,
>
> putpkt (rs->buf);
> getpkt (&rs->buf);
> + packet_result pkt_status = packet_check_result (rs->buf);
>
> /* Verify if the request was successful. */
> - return packet_check_result (rs->buf.data ()) == PACKET_OK;
> + return pkt_status.status () == PACKET_OK;
> }
>
> /* Return true if remote target T is non-stop. */
> @@ -15626,6 +15670,39 @@ test_memory_tagging_functions ()
> expected.length ()) == 0);
> }
>
> +static void
> +test_packet_check_result ()
> +{
> + std::string buf = "E01";
> +
> + packet_result result = packet_check_result (buf.data ());
> + SELF_CHECK (result.status () == PACKET_ERROR);
> +
> + buf.assign("E1");
> + result = packet_check_result (buf.data ());
> + SELF_CHECK (result.status () == PACKET_OK);
> +
> + buf.assign("E000");
> + result = packet_check_result (buf.data ());
> + SELF_CHECK (result.status () == PACKET_OK);
> +
> + buf.assign("E.msg");
> + result = packet_check_result (buf.data ());
> + SELF_CHECK (result.status () == PACKET_ERROR);
> +
> + buf.assign("E.");
> + result = packet_check_result (buf.data ());
> + SELF_CHECK (result.status () == PACKET_ERROR);
> +
> + buf.assign("some response");
> + result = packet_check_result (buf.data ());
> + SELF_CHECK (result.status () == PACKET_OK);
> +
> + buf.assign("");
> + result = packet_check_result (buf.data ());
> + SELF_CHECK (result.status () == PACKET_UNKNOWN);
> +}
> +
> } // namespace selftests
> #endif /* GDB_SELF_TEST */
>
> @@ -16125,5 +16202,7 @@ from the target."),
> #if GDB_SELF_TEST
> selftests::register_test ("remote_memory_tagging",
> selftests::test_memory_tagging_functions);
> + selftests::register_test ("packet_check_result",
> + selftests::test_packet_check_result);
> #endif
> }
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/6] Add vDefaultInferiorFd feature
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
` (6 preceding siblings ...)
2024-01-19 11:56 ` [PATCH v2 6/6] Add defaultinf.exp test to the testsuite Alexandra Hájková
@ 2024-02-22 14:38 ` Alexandra Petlanova Hajkova
7 siblings, 0 replies; 12+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-02-22 14:38 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2805 bytes --]
ping
On Fri, Jan 19, 2024 at 12:57 PM Alexandra Hájková <ahajkova@redhat.com>
wrote:
> Currently, when GDBserver is run locally using stdio, the inferior
> is unable to read from STDIN so we can't give it any input.
> The main motivation to address this issue is to use GDB together
> with Valgrind, using vgdb --multi feature which allows to run
> Valgrind from inside GDB. Valgrind then acts as a locally run
> GDBserver that uses stdio.
>
> Add a new DefaultInferiorFd feature and the corresponding packet.
> This feature allows GDB to send, to GDBserver, the file descriptor
> numbers of the terminal to which GDB is connected. The inferior is
> then started connected to the same terminal as GDB. This allows the
> inferior run by local GDBserver to read from GDB's STDIN and write
> its output to GDB's STOUT/ERR the same way as native target.
>
> v2: - [2/6]:- add an error handling in a case dup would fail
> - hoist the calls to mark_fd_no_cloexec near the dup()s
> [3/6] - improve documentation formatting
> - rephrase and extend the documentation
> - cosmetics
> - add a new target method so that gdbserver doesn't advertise
> this new feauture on platforms it does not work at
> [4/3] - cosmetics
>
> Alexandra Hájková (6):
> gdb.server/non-existing-program.exp: Use gdbserver_start.
> gdb/ser-pipe.c: Duplicate the file descriptors
> Add new vDefaultInferiorFd packet
> gdbserver/linux-low.cc: Connect the inferior to the terminal
> remote.c: Add terminal handling functions
> Add defaultinf.exp test to the testsuite
>
> gdb/doc/gdb.texinfo | 42 ++++++
> gdb/remote.c | 83 +++++++++++
> gdb/ser-pipe.c | 40 ++++++
> gdb/serial.c | 4 +
> gdb/serial.h | 4 +
> gdb/testsuite/gdb.server/defaultinf.c | 39 +++++
> gdb/testsuite/gdb.server/defaultinf.exp | 59 ++++++++
> .../gdb.server/non-existing-program.exp | 54 ++-----
> gdb/testsuite/lib/gdbserver-support.exp | 62 +++++---
> gdbserver/linux-low.cc | 38 ++++-
> gdbserver/linux-low.h | 2 +
> gdbserver/server.cc | 135 +++++++++++++++++-
> gdbserver/server.h | 12 ++
> gdbserver/target.cc | 6 +
> gdbserver/target.h | 6 +
> 15 files changed, 524 insertions(+), 62 deletions(-)
> create mode 100644 gdb/testsuite/gdb.server/defaultinf.c
> create mode 100644 gdb/testsuite/gdb.server/defaultinf.exp
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] remote.c: Make packet_check_result return a structure
2024-02-22 14:37 ` Alexandra Petlanova Hajkova
@ 2024-02-23 16:19 ` Tom Tromey
0 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2024-02-23 16:19 UTC (permalink / raw)
To: Alexandra Petlanova Hajkova; +Cc: gdb-patches
>>>>> Alexandra Petlanova Hajkova <ahajkova@redhat.com> writes:
> ping
> (this still applies cleanly)
Andrew approved v6 here:
https://sourceware.org/pipermail/gdb-patches/2024-February/206488.html
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-02-23 16:19 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-19 11:56 [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 1/6] gdb.server/non-existing-program.exp: Use gdbserver_start Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2] remote.c: Make packet_check_result return a structure Alexandra Hájková
2024-02-22 14:37 ` Alexandra Petlanova Hajkova
2024-02-23 16:19 ` Tom Tromey
2024-01-19 11:56 ` [PATCH v2 2/6] gdb/ser-pipe.c: Duplicate the file descriptors Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 3/6] Add new vDefaultInferiorFd packet Alexandra Hájková
2024-01-19 12:04 ` Eli Zaretskii
2024-01-19 11:56 ` [PATCH v2 4/6] gdbserver/linux-low.cc: Connect the inferior to the terminal Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 5/6] remote.c: Add terminal handling functions Alexandra Hájková
2024-01-19 11:56 ` [PATCH v2 6/6] Add defaultinf.exp test to the testsuite Alexandra Hájková
2024-02-22 14:38 ` [PATCH v2 0/6] Add vDefaultInferiorFd feature Alexandra Petlanova Hajkova
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).