* [PATCH 00/10] remote: More multi-target preparation
@ 2018-05-16 14:18 Pedro Alves
2018-05-16 14:18 ` [PATCH 01/10] remote: struct remote_state, use op new Pedro Alves
` (9 more replies)
0 siblings, 10 replies; 35+ messages in thread
From: Pedro Alves @ 2018-05-16 14:18 UTC (permalink / raw)
To: gdb-patches
Here's another chunk split from my multi-target branch.
This series makes us end up with a remote_state instance per
remote_target instance.
Pedro Alves (10):
remote: struct remote_state, use op new
remote: Eliminate remote_hostio_close_cleanup
remote: Make readahead_cache a C++ class
remote: multiple remote_arch_state instances per arch
remote: remote_arch_state pointers -> remote_arch_state objects
remote: Small cleanup in compare_section_command
remote: Move discard_pending_stop_replies call
Handle "show remote memory-write-packet-size" when not connected
remote: Make vcont_builder a class
remote: one struct remote_state per struct remote_target
gdb/remote-fileio.c | 214 ++--
gdb/remote-fileio.h | 3 +-
gdb/remote-notif.c | 22 +-
gdb/remote-notif.h | 21 +-
gdb/remote.c | 2029 ++++++++++++++++++++++---------------
gdb/remote.h | 9 +-
gdb/testsuite/gdb.base/remote.exp | 6 +-
7 files changed, 1344 insertions(+), 960 deletions(-)
--
2.14.3
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 01/10] remote: struct remote_state, use op new 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves @ 2018-05-16 14:18 ` Pedro Alves 2018-05-18 20:57 ` Simon Marchi 2018-05-16 14:18 ` [PATCH 09/10] remote: Make vcont_builder a class Pedro Alves ` (8 subsequent siblings) 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:18 UTC (permalink / raw) To: gdb-patches A bit of C++ification. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> (struct vCont_action_support): Use bool and initialize all fields. (struct readahead_cache): Initialize all fields. (remote_state): Use bool and initialize all fields. (remote_state::remote_state, ~remote_state::remote_state): New. (new_remote_state): Delete. (_initialize_remote): Use new to allocate remote_state. --- gdb/remote.c | 115 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 56 insertions(+), 59 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 58ed9e4f4d..ca72c1a1c2 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -541,16 +541,16 @@ static struct cmd_list_element *remote_show_cmdlist; struct vCont_action_support { /* vCont;t */ - int t; + bool t = false; /* vCont;r */ - int r; + bool r = false; /* vCont;s */ - int s; + bool s = false; /* vCont;S */ - int S; + bool S = false; }; /* Controls whether GDB is willing to use range stepping. */ @@ -577,21 +577,21 @@ struct readahead_cache { /* The file descriptor for the file that is being cached. -1 if the cache is invalid. */ - int fd; + int fd = -1; /* The offset into the file that the cache buffer corresponds to. */ - ULONGEST offset; + ULONGEST offset = 0; /* The buffer holding the cache contents. */ - gdb_byte *buf; + gdb_byte *buf = nullptr; /* The buffer's size. We try to read as much as fits into a packet at a time. */ - size_t bufsize; + size_t bufsize = 0; /* Cache hit and miss counters. */ - ULONGEST hit_count; - ULONGEST miss_count; + ULONGEST hit_count = 0; + ULONGEST miss_count = 0; }; /* Description of the remote protocol state for the currently @@ -600,6 +600,9 @@ struct readahead_cache struct remote_state { + remote_state (); + ~remote_state (); + /* A buffer to use for incoming packets, and its current size. The buffer is grown dynamically for larger incoming packets. Outgoing packets may also be constructed in this buffer. @@ -611,13 +614,13 @@ struct remote_state /* True if we're going through initial connection setup (finding out about the remote side's threads, relocating symbols, etc.). */ - int starting_up; + bool starting_up = false; /* If we negotiated packet size explicitly (and thus can bypass heuristics for the largest packet size that will not overflow a buffer in the stub), this will be set to that packet size. Otherwise zero, meaning to use the guessed size. */ - long explicit_packet_size; + long explicit_packet_size = 0; /* remote_wait is normally called when the target is running and waits for a stop reply packet. But sometimes we need to call it @@ -626,15 +629,15 @@ struct remote_state the response, we can stash it in BUF and tell remote_wait to skip calling getpkt. This flag is set when BUF contains a stop reply packet and the target is not waiting. */ - int cached_wait_status; + int cached_wait_status = 0; /* True, if in no ack mode. That is, neither GDB nor the stub will expect acks from each other. The connection is assumed to be reliable. */ - int noack_mode; + bool noack_mode = false; /* True if we're connected in extended remote mode. */ - int extended; + bool extended = false; /* True if we resumed the target and we're waiting for the target to stop. In the mean time, we can't start another command/query. @@ -642,14 +645,14 @@ struct remote_state timeout waiting for a reply that would never come and eventually we'd close the connection. This can happen in asynchronous mode because we allow GDB commands while the target is running. */ - int waiting_for_stop_reply; + bool waiting_for_stop_reply = false; /* The status of the stub support for the various vCont actions. */ - struct vCont_action_support supports_vCont; + vCont_action_support supports_vCont; - /* Nonzero if the user has pressed Ctrl-C, but the target hasn't + /* True if the user has pressed Ctrl-C, but the target hasn't responded to that. */ - int ctrlc_pending_p; + bool ctrlc_pending_p = false; /* True if we saw a Ctrl-C while reading or writing from/to the remote descriptor. At that point it is not safe to send a remote @@ -657,12 +660,12 @@ struct remote_state process it once we're done with sending/receiving the current packet, which should be shortly. If however that takes too long, and the user presses Ctrl-C again, we offer to disconnect. */ - int got_ctrlc_during_io; + bool got_ctrlc_during_io = false; /* Descriptor for I/O to remote machine. Initialize it to NULL so that remote_open knows that we don't have a file open when the program starts. */ - struct serial *remote_desc; + struct serial *remote_desc = nullptr; /* These are the threads which we last sent to the remote system. The TID member will be -1 for all or -2 for not sent yet. */ @@ -671,26 +674,26 @@ struct remote_state /* This is the traceframe which we last selected on the remote system. It will be -1 if no traceframe is selected. */ - int remote_traceframe_number; + int remote_traceframe_number = -1; - char *last_pass_packet; + char *last_pass_packet = nullptr; /* The last QProgramSignals packet sent to the target. We bypass sending a new program signals list down to the target if the new packet is exactly the same as the last we sent. IOW, we only let the target know about program signals list changes. */ - char *last_program_signals_packet; + char *last_program_signals_packet = nullptr; - enum gdb_signal last_sent_signal; + gdb_signal last_sent_signal = GDB_SIGNAL_0; - int last_sent_step; + bool last_sent_step = false; /* The execution direction of the last resume we got. */ - enum exec_direction_kind last_resume_exec_dir; + exec_direction_kind last_resume_exec_dir = EXEC_FORWARD; - char *finished_object; - char *finished_annex; - ULONGEST finished_offset; + char *finished_object = nullptr; + char *finished_annex = nullptr; + ULONGEST finished_offset = 0; /* Should we try the 'ThreadInfo' query packet? @@ -699,24 +702,24 @@ struct remote_state query or the older, more complex syntax for thread queries. This is an auto-detect variable (set to true at each connect, and set to false when the target fails to recognize it). */ - int use_threadinfo_query; - int use_threadextra_query; + bool use_threadinfo_query = false; + bool use_threadextra_query = false; - threadref echo_nextthread; - threadref nextthread; - threadref resultthreadlist[MAXTHREADLISTRESULTS]; + threadref echo_nextthread {}; + threadref nextthread {}; + threadref resultthreadlist[MAXTHREADLISTRESULTS] {}; /* The state of remote notification. */ - struct remote_notif_state *notif_state; + struct remote_notif_state *notif_state = nullptr; /* The branch trace configuration. */ - struct btrace_config btrace_config; + struct btrace_config btrace_config {}; /* The argument to the last "vFile:setfs:" packet we sent, used to avoid sending repeated unnecessary "vFile:setfs:" packets. Initialized to -1 to indicate that no "vFile:setfs:" packet has yet been sent. */ - int fs_pid; + int fs_pid = -1; /* A readahead cache for vFile:pread. Often, reading a binary involves a sequence of small reads. E.g., when parsing an ELF @@ -764,6 +767,20 @@ struct remote_thread_info : public private_thread_info int vcont_resumed = 0; }; +remote_state::remote_state () +{ + /* The default buffer size is unimportant; it will be expanded + whenever a larger buffer is needed. */ + this->buf_size = 400; + this->buf = (char *) xmalloc (this->buf_size); +} + +remote_state::~remote_state () +{ + xfree (this->last_pass_packet); + xfree (this->last_program_signals_packet); +} + /* This data could be associated with a target, but we do not always have access to the current target when we need it, so for now it is static. This will be fine for as long as only one target is in use @@ -776,26 +793,6 @@ get_remote_state_raw (void) return remote_state; } -/* Allocate a new struct remote_state with xmalloc, initialize it, and - return it. */ - -static struct remote_state * -new_remote_state (void) -{ - struct remote_state *result = XCNEW (struct remote_state); - - /* The default buffer size is unimportant; it will be expanded - whenever a larger buffer is needed. */ - result->buf_size = 400; - result->buf = (char *) xmalloc (result->buf_size); - result->remote_traceframe_number = -1; - result->last_sent_signal = GDB_SIGNAL_0; - result->last_resume_exec_dir = EXEC_FORWARD; - result->fs_pid = -1; - - return result; -} - /* Description of the remote protocol for a given architecture. */ struct packet_reg @@ -14010,7 +14007,7 @@ _initialize_remote (void) /* Initialize the per-target state. At the moment there is only one of these, not one per target. Only one target is active at a time. */ - remote_state = new_remote_state (); + remote_state = new struct remote_state (); add_target (remote_target_info, remote_target::open); add_target (extended_remote_target_info, extended_remote_target::open); -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 01/10] remote: struct remote_state, use op new 2018-05-16 14:18 ` [PATCH 01/10] remote: struct remote_state, use op new Pedro Alves @ 2018-05-18 20:57 ` Simon Marchi 2018-05-21 15:36 ` [PATCH 1.2 01/10] remote: struct remote_state, use op new, fix leaks Pedro Alves 0 siblings, 1 reply; 35+ messages in thread From: Simon Marchi @ 2018-05-18 20:57 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > A bit of C++ification. LGTM, I just noted some questions. > @@ -657,12 +660,12 @@ struct remote_state > process it once we're done with sending/receiving the current > packet, which should be shortly. If however that takes too long, > and the user presses Ctrl-C again, we offer to disconnect. */ > - int got_ctrlc_during_io; > + bool got_ctrlc_during_io = false; > > /* Descriptor for I/O to remote machine. Initialize it to NULL so that > remote_open knows that we don't have a file open when the program > starts. */ > - struct serial *remote_desc; > + struct serial *remote_desc = nullptr; > > /* These are the threads which we last sent to the remote system. The > TID member will be -1 for all or -2 for not sent yet. */ Should general_thread and continue_thread (below, not shown here) be initialized too? > @@ -764,6 +767,20 @@ struct remote_thread_info : public private_thread_info > int vcont_resumed = 0; > }; > > +remote_state::remote_state () > +{ > + /* The default buffer size is unimportant; it will be expanded > + whenever a larger buffer is needed. */ > + this->buf_size = 400; > + this->buf = (char *) xmalloc (this->buf_size); > +} > + > +remote_state::~remote_state () > +{ > + xfree (this->last_pass_packet); > + xfree (this->last_program_signals_packet); Should other fields be freed here? - buf - finished_object - finished_annex Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1.2 01/10] remote: struct remote_state, use op new, fix leaks 2018-05-18 20:57 ` Simon Marchi @ 2018-05-21 15:36 ` Pedro Alves 0 siblings, 0 replies; 35+ messages in thread From: Pedro Alves @ 2018-05-21 15:36 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 05/18/2018 09:45 PM, Simon Marchi wrote: > On 2018-05-16 10:18 AM, Pedro Alves wrote: >> A bit of C++ification. > > LGTM, I just noted some questions. Thanks. >> @@ -657,12 +660,12 @@ struct remote_state >> process it once we're done with sending/receiving the current >> packet, which should be shortly. If however that takes too long, >> and the user presses Ctrl-C again, we offer to disconnect. */ >> - int got_ctrlc_during_io; >> + bool got_ctrlc_during_io = false; >> >> /* Descriptor for I/O to remote machine. Initialize it to NULL so that >> remote_open knows that we don't have a file open when the program >> starts. */ >> - struct serial *remote_desc; >> + struct serial *remote_desc = nullptr; >> >> /* These are the threads which we last sent to the remote system. The >> TID member will be -1 for all or -2 for not sent yet. */ > > Should general_thread and continue_thread (below, not shown here) be initialized too? Indeed, I forgot that ptid_t's default ctor is trivial. Note that currently, remote_target::open_1 resets/clears the global remote_state instance: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /* Reset the target state; these things will be queried either by remote_query_supported or as they are needed. */ reset_all_packet_configs_support (); rs->cached_wait_status = 0; rs->explicit_packet_size = 0; rs->noack_mode = 0; rs->extended = extended_p; rs->waiting_for_stop_reply = 0; rs->ctrlc_pending_p = 0; rs->got_ctrlc_during_io = 0; rs->general_thread = not_sent_ptid; rs->continue_thread = not_sent_ptid; rs->remote_traceframe_number = -1; rs->last_resume_exec_dir = EXEC_FORWARD; /* Probe for ability to use "ThreadInfo" query, as required. */ rs->use_threadinfo_query = 1; rs->use_threadextra_query = 1; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ but after patch #10, we can remove that code, since we'll be always starting with a new remote_state object. > >> @@ -764,6 +767,20 @@ struct remote_thread_info : public private_thread_info >> int vcont_resumed = 0; >> }; >> >> +remote_state::remote_state () >> +{ >> + /* The default buffer size is unimportant; it will be expanded >> + whenever a larger buffer is needed. */ >> + this->buf_size = 400; >> + this->buf = (char *) xmalloc (this->buf_size); >> +} >> + >> +remote_state::~remote_state () >> +{ >> + xfree (this->last_pass_packet); >> + xfree (this->last_program_signals_packet); > > Should other fields be freed here? > > - buf > - finished_object > - finished_annex Good point. I've double checked and it looks like those are the ones missing. Arguably the addition of the dtor could/should go as a separate patch, since we currently "leak" these fields on gdb exit, but it doesn't seem worth the bother. So here's an updated patch with the improved dtor. ~~~ Subject: [PATCH] remote: struct remote_state, use op new, fix leaks A bit of C++ification. Also plugs a few leaks, since currently nothing is freeing a few fields of remote_state. They're now freed in the added destructor. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> (struct vCont_action_support): Use bool and initialize all fields. (struct readahead_cache): Initialize all fields. (remote_state): Use bool and initialize all fields. (remote_state::remote_state, remote_state::~remote_state): New. (new_remote_state): Delete. (_initialize_remote): Use new to allocate remote_state. --- gdb/remote.c | 122 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 58ed9e4f4d..d63e43ac0a 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -541,16 +541,16 @@ static struct cmd_list_element *remote_show_cmdlist; struct vCont_action_support { /* vCont;t */ - int t; + bool t = false; /* vCont;r */ - int r; + bool r = false; /* vCont;s */ - int s; + bool s = false; /* vCont;S */ - int S; + bool S = false; }; /* Controls whether GDB is willing to use range stepping. */ @@ -577,21 +577,21 @@ struct readahead_cache { /* The file descriptor for the file that is being cached. -1 if the cache is invalid. */ - int fd; + int fd = -1; /* The offset into the file that the cache buffer corresponds to. */ - ULONGEST offset; + ULONGEST offset = 0; /* The buffer holding the cache contents. */ - gdb_byte *buf; + gdb_byte *buf = nullptr; /* The buffer's size. We try to read as much as fits into a packet at a time. */ - size_t bufsize; + size_t bufsize = 0; /* Cache hit and miss counters. */ - ULONGEST hit_count; - ULONGEST miss_count; + ULONGEST hit_count = 0; + ULONGEST miss_count = 0; }; /* Description of the remote protocol state for the currently @@ -600,6 +600,9 @@ struct readahead_cache struct remote_state { + remote_state (); + ~remote_state (); + /* A buffer to use for incoming packets, and its current size. The buffer is grown dynamically for larger incoming packets. Outgoing packets may also be constructed in this buffer. @@ -611,13 +614,13 @@ struct remote_state /* True if we're going through initial connection setup (finding out about the remote side's threads, relocating symbols, etc.). */ - int starting_up; + bool starting_up = false; /* If we negotiated packet size explicitly (and thus can bypass heuristics for the largest packet size that will not overflow a buffer in the stub), this will be set to that packet size. Otherwise zero, meaning to use the guessed size. */ - long explicit_packet_size; + long explicit_packet_size = 0; /* remote_wait is normally called when the target is running and waits for a stop reply packet. But sometimes we need to call it @@ -626,15 +629,15 @@ struct remote_state the response, we can stash it in BUF and tell remote_wait to skip calling getpkt. This flag is set when BUF contains a stop reply packet and the target is not waiting. */ - int cached_wait_status; + int cached_wait_status = 0; /* True, if in no ack mode. That is, neither GDB nor the stub will expect acks from each other. The connection is assumed to be reliable. */ - int noack_mode; + bool noack_mode = false; /* True if we're connected in extended remote mode. */ - int extended; + bool extended = false; /* True if we resumed the target and we're waiting for the target to stop. In the mean time, we can't start another command/query. @@ -642,14 +645,14 @@ struct remote_state timeout waiting for a reply that would never come and eventually we'd close the connection. This can happen in asynchronous mode because we allow GDB commands while the target is running. */ - int waiting_for_stop_reply; + bool waiting_for_stop_reply = false; /* The status of the stub support for the various vCont actions. */ - struct vCont_action_support supports_vCont; + vCont_action_support supports_vCont; - /* Nonzero if the user has pressed Ctrl-C, but the target hasn't + /* True if the user has pressed Ctrl-C, but the target hasn't responded to that. */ - int ctrlc_pending_p; + bool ctrlc_pending_p = false; /* True if we saw a Ctrl-C while reading or writing from/to the remote descriptor. At that point it is not safe to send a remote @@ -657,40 +660,40 @@ struct remote_state process it once we're done with sending/receiving the current packet, which should be shortly. If however that takes too long, and the user presses Ctrl-C again, we offer to disconnect. */ - int got_ctrlc_during_io; + bool got_ctrlc_during_io = false; /* Descriptor for I/O to remote machine. Initialize it to NULL so that remote_open knows that we don't have a file open when the program starts. */ - struct serial *remote_desc; + struct serial *remote_desc = nullptr; /* These are the threads which we last sent to the remote system. The TID member will be -1 for all or -2 for not sent yet. */ - ptid_t general_thread; - ptid_t continue_thread; + ptid_t general_thread = null_ptid; + ptid_t continue_thread = null_ptid; /* This is the traceframe which we last selected on the remote system. It will be -1 if no traceframe is selected. */ - int remote_traceframe_number; + int remote_traceframe_number = -1; - char *last_pass_packet; + char *last_pass_packet = nullptr; /* The last QProgramSignals packet sent to the target. We bypass sending a new program signals list down to the target if the new packet is exactly the same as the last we sent. IOW, we only let the target know about program signals list changes. */ - char *last_program_signals_packet; + char *last_program_signals_packet = nullptr; - enum gdb_signal last_sent_signal; + gdb_signal last_sent_signal = GDB_SIGNAL_0; - int last_sent_step; + bool last_sent_step = false; /* The execution direction of the last resume we got. */ - enum exec_direction_kind last_resume_exec_dir; + exec_direction_kind last_resume_exec_dir = EXEC_FORWARD; - char *finished_object; - char *finished_annex; - ULONGEST finished_offset; + char *finished_object = nullptr; + char *finished_annex = nullptr; + ULONGEST finished_offset = 0; /* Should we try the 'ThreadInfo' query packet? @@ -699,24 +702,24 @@ struct remote_state query or the older, more complex syntax for thread queries. This is an auto-detect variable (set to true at each connect, and set to false when the target fails to recognize it). */ - int use_threadinfo_query; - int use_threadextra_query; + bool use_threadinfo_query = false; + bool use_threadextra_query = false; - threadref echo_nextthread; - threadref nextthread; - threadref resultthreadlist[MAXTHREADLISTRESULTS]; + threadref echo_nextthread {}; + threadref nextthread {}; + threadref resultthreadlist[MAXTHREADLISTRESULTS] {}; /* The state of remote notification. */ - struct remote_notif_state *notif_state; + struct remote_notif_state *notif_state = nullptr; /* The branch trace configuration. */ - struct btrace_config btrace_config; + struct btrace_config btrace_config {}; /* The argument to the last "vFile:setfs:" packet we sent, used to avoid sending repeated unnecessary "vFile:setfs:" packets. Initialized to -1 to indicate that no "vFile:setfs:" packet has yet been sent. */ - int fs_pid; + int fs_pid = -1; /* A readahead cache for vFile:pread. Often, reading a binary involves a sequence of small reads. E.g., when parsing an ELF @@ -764,6 +767,23 @@ struct remote_thread_info : public private_thread_info int vcont_resumed = 0; }; +remote_state::remote_state () +{ + /* The default buffer size is unimportant; it will be expanded + whenever a larger buffer is needed. */ + this->buf_size = 400; + this->buf = (char *) xmalloc (this->buf_size); +} + +remote_state::~remote_state () +{ + xfree (this->last_pass_packet); + xfree (this->last_program_signals_packet); + xfree (this->buf); + xfree (this->finished_object); + xfree (this->finished_annex); +} + /* This data could be associated with a target, but we do not always have access to the current target when we need it, so for now it is static. This will be fine for as long as only one target is in use @@ -776,26 +796,6 @@ get_remote_state_raw (void) return remote_state; } -/* Allocate a new struct remote_state with xmalloc, initialize it, and - return it. */ - -static struct remote_state * -new_remote_state (void) -{ - struct remote_state *result = XCNEW (struct remote_state); - - /* The default buffer size is unimportant; it will be expanded - whenever a larger buffer is needed. */ - result->buf_size = 400; - result->buf = (char *) xmalloc (result->buf_size); - result->remote_traceframe_number = -1; - result->last_sent_signal = GDB_SIGNAL_0; - result->last_resume_exec_dir = EXEC_FORWARD; - result->fs_pid = -1; - - return result; -} - /* Description of the remote protocol for a given architecture. */ struct packet_reg @@ -14010,7 +14010,7 @@ _initialize_remote (void) /* Initialize the per-target state. At the moment there is only one of these, not one per target. Only one target is active at a time. */ - remote_state = new_remote_state (); + remote_state = new struct remote_state (); add_target (remote_target_info, remote_target::open); add_target (extended_remote_target_info, extended_remote_target::open); -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 09/10] remote: Make vcont_builder a class 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves 2018-05-16 14:18 ` [PATCH 01/10] remote: struct remote_state, use op new Pedro Alves @ 2018-05-16 14:18 ` Pedro Alves 2018-05-22 5:07 ` Simon Marchi 2018-05-16 14:18 ` [PATCH 10/10] remote: one struct remote_state per struct remote_target Pedro Alves ` (7 subsequent siblings) 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:18 UTC (permalink / raw) To: gdb-patches gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (vcont_builder): Now a class. Make all data members private. (vcont_builder) <vcont_builder, restart, flush, push_action>: Declare methods. (vcont_builder_restart): Rename to ... (vcont_builder::restart): ... this. (vcont_builder_flush): Rename to ... (vcont_builder::flush): ... this. (vcont_builder_push_action): Rename to ... (vcont_builder::push_action): ... this. (remote_target::commit_resume): Adjust. --- gdb/remote.c | 79 ++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 7ec8d24a3b..4166598750 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -6091,41 +6091,50 @@ get_remote_inferior (inferior *inf) outgoing packet buffer. This is used to send multiple vCont packets if we have more actions than would fit a single packet. */ -struct vcont_builder +class vcont_builder { +public: + vcont_builder () + {} + + void restart (); + void flush (); + void push_action (ptid_t ptid, bool step, gdb_signal siggnal); + +private: /* Pointer to the first action. P points here if no action has been appended yet. */ - char *first_action; + char *m_first_action; /* Where the next action will be appended. */ - char *p; + char *m_p; /* The end of the buffer. Must never write past this. */ - char *endp; + char *m_endp; }; /* Prepare the outgoing buffer for a new vCont packet. */ -static void -vcont_builder_restart (struct vcont_builder *builder) +void +vcont_builder::restart () { struct remote_state *rs = get_remote_state (); - builder->p = rs->buf; - builder->endp = rs->buf + get_remote_packet_size (); - builder->p += xsnprintf (builder->p, builder->endp - builder->p, "vCont"); - builder->first_action = builder->p; + m_p = rs->buf; + m_endp = rs->buf + get_remote_packet_size (); + m_p += xsnprintf (m_p, m_endp - m_p, "vCont"); + m_first_action = m_p; } /* If the vCont packet being built has any action, send it to the remote end. */ -static void -vcont_builder_flush (struct vcont_builder *builder) +void +vcont_builder::flush () { struct remote_state *rs; - if (builder->p == builder->first_action) + if (m_p == m_first_action) return; rs = get_remote_state (); @@ -6146,33 +6155,30 @@ vcont_builder_flush (struct vcont_builder *builder) what we've got so far to the remote end and start over a new vCont packet (with the new action). */ -static void -vcont_builder_push_action (struct vcont_builder *builder, - ptid_t ptid, int step, enum gdb_signal siggnal) +void +vcont_builder::push_action (ptid_t ptid, bool step, gdb_signal siggnal) { char buf[MAX_ACTION_SIZE + 1]; - char *endp; - size_t rsize; - endp = append_resumption (buf, buf + sizeof (buf), - ptid, step, siggnal); + char *endp = append_resumption (buf, buf + sizeof (buf), + ptid, step, siggnal); /* Check whether this new action would fit in the vCont packet along with previous actions. If not, send what we've got so far and start a new vCont packet. */ - rsize = endp - buf; - if (rsize > builder->endp - builder->p) + size_t rsize = endp - buf; + if (rsize > m_endp - m_p) { - vcont_builder_flush (builder); - vcont_builder_restart (builder); + flush (); + restart (); /* Should now fit. */ - gdb_assert (rsize <= builder->endp - builder->p); + gdb_assert (rsize <= m_endp - m_p); } - memcpy (builder->p, buf, rsize); - builder->p += rsize; - *builder->p = '\0'; + memcpy (m_p, buf, rsize); + m_p += rsize; + *m_p = '\0'; } /* to_commit_resume implementation. */ @@ -6184,7 +6190,6 @@ remote_target::commit_resume () struct thread_info *tp; int any_process_wildcard; int may_global_wildcard_vcont; - struct vcont_builder vcont_builder; /* If connected in all-stop mode, we'd send the remote resume request directly from remote_resume. Likewise if @@ -6282,7 +6287,8 @@ remote_target::commit_resume () we end up with too many actions for a single packet vcont_builder flushes the current vCont packet to the remote side and starts a new one. */ - vcont_builder_restart (&vcont_builder); + struct vcont_builder vcont_builder; + vcont_builder.restart (); /* Threads first. */ ALL_NON_EXITED_THREADS (tp) @@ -6303,7 +6309,7 @@ remote_target::commit_resume () continue; } - vcont_builder_push_action (&vcont_builder, tp->ptid, + vcont_builder.push_action (tp->ptid, remote_thr->last_resume_step, remote_thr->last_resume_sig); remote_thr->vcont_resumed = 1; @@ -6330,8 +6336,8 @@ remote_target::commit_resume () continue action for each running process, if any. */ if (may_global_wildcard_vcont) { - vcont_builder_push_action (&vcont_builder, minus_one_ptid, - 0, GDB_SIGNAL_0); + vcont_builder.push_action (minus_one_ptid, + false, GDB_SIGNAL_0); } else { @@ -6339,15 +6345,14 @@ remote_target::commit_resume () { if (get_remote_inferior (inf)->may_wildcard_vcont) { - vcont_builder_push_action (&vcont_builder, - pid_to_ptid (inf->pid), - 0, GDB_SIGNAL_0); + vcont_builder.push_action (pid_to_ptid (inf->pid), + false, GDB_SIGNAL_0); } } } } - vcont_builder_flush (&vcont_builder); + vcont_builder.flush (); } \f -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 09/10] remote: Make vcont_builder a class 2018-05-16 14:18 ` [PATCH 09/10] remote: Make vcont_builder a class Pedro Alves @ 2018-05-22 5:07 ` Simon Marchi 2018-05-22 21:33 ` Pedro Alves 0 siblings, 1 reply; 35+ messages in thread From: Simon Marchi @ 2018-05-22 5:07 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves <palves@redhat.com> > > * remote.c (vcont_builder): Now a class. Make all data members > private. > (vcont_builder) <vcont_builder, restart, flush, push_action>: > Declare methods. > (vcont_builder_restart): Rename to ... > (vcont_builder::restart): ... this. > (vcont_builder_flush): Rename to ... > (vcont_builder::flush): ... this. > (vcont_builder_push_action): Rename to ... > (vcont_builder::push_action): ... this. > (remote_target::commit_resume): Adjust. LGTM. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 09/10] remote: Make vcont_builder a class 2018-05-22 5:07 ` Simon Marchi @ 2018-05-22 21:33 ` Pedro Alves 0 siblings, 0 replies; 35+ messages in thread From: Pedro Alves @ 2018-05-22 21:33 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 05/22/2018 04:06 AM, Simon Marchi wrote: > On 2018-05-16 10:18 AM, Pedro Alves wrote: >> gdb/ChangeLog: >> yyyy-mm-dd Pedro Alves <palves@redhat.com> >> >> * remote.c (vcont_builder): Now a class. Make all data members >> private. >> (vcont_builder) <vcont_builder, restart, flush, push_action>: >> Declare methods. >> (vcont_builder_restart): Rename to ... >> (vcont_builder::restart): ... this. >> (vcont_builder_flush): Rename to ... >> (vcont_builder::flush): ... this. >> (vcont_builder_push_action): Rename to ... >> (vcont_builder::push_action): ... this. >> (remote_target::commit_resume): Adjust. > > LGTM. Here's the version that I'm landing -- it moves the restart() call to the ctor, as suggested in response to patch #10. From f5db4863f51b83bc6b54504d61e1731011cfdec2 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Tue, 22 May 2018 18:22:11 +0100 Subject: [PATCH] remote: Make vcont_builder a class gdb/ChangeLog: 2018-05-22 Pedro Alves <palves@redhat.com> * remote.c (vcont_builder): Now a class. Make all data members private. (vcont_builder) <vcont_builder, restart, flush, push_action>: Declare methods. (vcont_builder_restart): Rename to ... (vcont_builder::restart): ... this. (vcont_builder_flush): Rename to ... (vcont_builder::flush): ... this. (vcont_builder_push_action): Rename to ... (vcont_builder::push_action): ... this. (remote_target::commit_resume): Adjust. --- gdb/ChangeLog | 14 ++++++++++ gdb/remote.c | 83 ++++++++++++++++++++++++++++++++--------------------------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3504a3cf19..501ee86a2a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,17 @@ +2018-05-22 Pedro Alves <palves@redhat.com> + + * remote.c (vcont_builder): Now a class. Make all data members + private. + (vcont_builder) <vcont_builder, restart, flush, push_action>: + Declare methods. + (vcont_builder_restart): Rename to ... + (vcont_builder::restart): ... this. + (vcont_builder_flush): Rename to ... + (vcont_builder::flush): ... this. + (vcont_builder_push_action): Rename to ... + (vcont_builder::push_action): ... this. + (remote_target::commit_resume): Adjust. + 2018-05-22 Pedro Alves <palves@redhat.com> * remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE): Rename to ... diff --git a/gdb/remote.c b/gdb/remote.c index 72254dba31..33b2c906eb 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -6096,45 +6096,57 @@ get_remote_inferior (inferior *inf) return static_cast<remote_inferior *> (inf->priv.get ()); } -/* Structure used to track the construction of a vCont packet in the +/* Class used to track the construction of a vCont packet in the outgoing packet buffer. This is used to send multiple vCont packets if we have more actions than would fit a single packet. */ -struct vcont_builder +class vcont_builder { +public: + vcont_builder () + { + restart (); + } + + void flush (); + void push_action (ptid_t ptid, bool step, gdb_signal siggnal); + +private: + void restart (); + /* Pointer to the first action. P points here if no action has been appended yet. */ - char *first_action; + char *m_first_action; /* Where the next action will be appended. */ - char *p; + char *m_p; /* The end of the buffer. Must never write past this. */ - char *endp; + char *m_endp; }; /* Prepare the outgoing buffer for a new vCont packet. */ -static void -vcont_builder_restart (struct vcont_builder *builder) +void +vcont_builder::restart () { struct remote_state *rs = get_remote_state (); - builder->p = rs->buf; - builder->endp = rs->buf + get_remote_packet_size (); - builder->p += xsnprintf (builder->p, builder->endp - builder->p, "vCont"); - builder->first_action = builder->p; + m_p = rs->buf; + m_endp = rs->buf + get_remote_packet_size (); + m_p += xsnprintf (m_p, m_endp - m_p, "vCont"); + m_first_action = m_p; } /* If the vCont packet being built has any action, send it to the remote end. */ -static void -vcont_builder_flush (struct vcont_builder *builder) +void +vcont_builder::flush () { struct remote_state *rs; - if (builder->p == builder->first_action) + if (m_p == m_first_action) return; rs = get_remote_state (); @@ -6155,33 +6167,30 @@ vcont_builder_flush (struct vcont_builder *builder) what we've got so far to the remote end and start over a new vCont packet (with the new action). */ -static void -vcont_builder_push_action (struct vcont_builder *builder, - ptid_t ptid, int step, enum gdb_signal siggnal) +void +vcont_builder::push_action (ptid_t ptid, bool step, gdb_signal siggnal) { char buf[MAX_ACTION_SIZE + 1]; - char *endp; - size_t rsize; - endp = append_resumption (buf, buf + sizeof (buf), - ptid, step, siggnal); + char *endp = append_resumption (buf, buf + sizeof (buf), + ptid, step, siggnal); /* Check whether this new action would fit in the vCont packet along with previous actions. If not, send what we've got so far and start a new vCont packet. */ - rsize = endp - buf; - if (rsize > builder->endp - builder->p) + size_t rsize = endp - buf; + if (rsize > m_endp - m_p) { - vcont_builder_flush (builder); - vcont_builder_restart (builder); + flush (); + restart (); /* Should now fit. */ - gdb_assert (rsize <= builder->endp - builder->p); + gdb_assert (rsize <= m_endp - m_p); } - memcpy (builder->p, buf, rsize); - builder->p += rsize; - *builder->p = '\0'; + memcpy (m_p, buf, rsize); + m_p += rsize; + *m_p = '\0'; } /* to_commit_resume implementation. */ @@ -6193,7 +6202,6 @@ remote_target::commit_resume () struct thread_info *tp; int any_process_wildcard; int may_global_wildcard_vcont; - struct vcont_builder vcont_builder; /* If connected in all-stop mode, we'd send the remote resume request directly from remote_resume. Likewise if @@ -6291,7 +6299,7 @@ remote_target::commit_resume () we end up with too many actions for a single packet vcont_builder flushes the current vCont packet to the remote side and starts a new one. */ - vcont_builder_restart (&vcont_builder); + struct vcont_builder vcont_builder; /* Threads first. */ ALL_NON_EXITED_THREADS (tp) @@ -6312,7 +6320,7 @@ remote_target::commit_resume () continue; } - vcont_builder_push_action (&vcont_builder, tp->ptid, + vcont_builder.push_action (tp->ptid, remote_thr->last_resume_step, remote_thr->last_resume_sig); remote_thr->vcont_resumed = 1; @@ -6339,8 +6347,8 @@ remote_target::commit_resume () continue action for each running process, if any. */ if (may_global_wildcard_vcont) { - vcont_builder_push_action (&vcont_builder, minus_one_ptid, - 0, GDB_SIGNAL_0); + vcont_builder.push_action (minus_one_ptid, + false, GDB_SIGNAL_0); } else { @@ -6348,15 +6356,14 @@ remote_target::commit_resume () { if (get_remote_inferior (inf)->may_wildcard_vcont) { - vcont_builder_push_action (&vcont_builder, - pid_to_ptid (inf->pid), - 0, GDB_SIGNAL_0); + vcont_builder.push_action (pid_to_ptid (inf->pid), + false, GDB_SIGNAL_0); } } } } - vcont_builder_flush (&vcont_builder); + vcont_builder.flush (); } \f -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 10/10] remote: one struct remote_state per struct remote_target 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves 2018-05-16 14:18 ` [PATCH 01/10] remote: struct remote_state, use op new Pedro Alves 2018-05-16 14:18 ` [PATCH 09/10] remote: Make vcont_builder a class Pedro Alves @ 2018-05-16 14:18 ` Pedro Alves 2018-05-22 5:07 ` Simon Marchi 2018-05-16 14:18 ` [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup Pedro Alves ` (6 subsequent siblings) 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:18 UTC (permalink / raw) To: gdb-patches 'struct remote_state' today contains per-connection state, however there's only a single global instance of that type. In order to support multiple connections, we must have one such object per connection. Thus this patch eliminates the 'remote_state' global in favor of having a remote_state instance per remote_target instance. The get_remote_state free function is eliminated as well, by making it a remote_target method instead. The patch then fixes the fallout by making all free functions that refer to get_remote_state() directly or indirectly be methods of remote_target too. Likewise, remote-fileio.c and remote-notif.c routines are parameterized with a remote_target pointer too, so they can call into the right remote_target instance. References to the global 'get_remote_state ()->remote_desc' to tell whether the remote target is open (!= nullptr) must be replaced with something else: - Command implementations use a new get_current_remote_target free function. - remote_target::open_1 checks the exception type instead. Finally, remote_target and extended_remote_target are made heap-allocated targets. As with the earlier core target patches, it still won't be possible to have more than one remote_target instance in practice, but this puts us closer. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote-fileio.c (remote_fileio_reply, remote_fileio_ioerror) (remote_fileio_badfd, remote_fileio_return_errno) (remote_fileio_return_success, remote_fileio_func_open) (remote_fileio_func_open, remote_fileio_func_close) (remote_fileio_func_read, remote_fileio_func_write) (remote_fileio_func_lseek, remote_fileio_func_rename) (remote_fileio_func_unlink, remote_fileio_func_stat) (remote_fileio_func_fstat, remote_fileio_func_gettimeofday) (remote_fileio_func_isatty, remote_fileio_func_system): Add remote_target parameter. (remote_fio_func_map) <func>: Add remote_target parameter. (do_remote_fileio_request, remote_fileio_request): * remote-fileio.h (remote_fileio_request): * remote-notif.c (remote_notif_ack, remote_notif_parse, ): Add remote_target parameter. (remote_notif_process, handle_notification): Adjust to pass down the remote. (remote_notif_state_allocate): Add remote_target parameter. Save it. * remote-notif.h (struct remote_target): Forward declare. (struct notif_client) <parse, ack, can_get_pending_events>: Add remote_target parameter. (struct remote_notif_state) <remote>: New field. (remote_notif_ack, remote_notif_parse): Add remote_target parameter. (remote_notif_state_allocate, remote_notif_state_allocate): Add remote_target parameter. * remote.c (OPAQUETHREADBYTES, threadref, gdb_ext_thread_info) (threads_listing_context, rmt_thread_action, protocol_feature) (packet_reg, stop_reply, stop_reply_p, enum packet_support) (packet_result, struct threads_listing_context, remote_state): Move definitions and declarations higher up. (remote_target) <~remote_target>: Declare. (remote_download_command_source, remote_file_put, remote_file_get) (remote_file_delete, remote_hostio_pread, remote_hostio_pwrite) (remote_hostio_pread_vFile, remote_hostio_send_command) (remote_hostio_set_filesystem, remote_hostio_open) (remote_hostio_close, remote_hostio_unlink, remote_state) (get_remote_state, get_remote_packet_size, get_memory_packet_size) (get_memory_write_packet_size, get_memory_read_packet_size) (append_pending_thread_resumptions, remote_detach_1) (append_resumption, remote_resume_with_vcont) (add_current_inferior_and_thread, wait_ns, wait_as) (process_stop_reply, remote_notice_new_inferior) (process_initial_stop_replies, remote_add_thread) (btrace_sync_conf, remote_btrace_maybe_reopen) (remove_new_fork_children, kill_new_fork_children) (discard_pending_stop_replies, stop_reply_queue_length) (check_pending_events_prevent_wildcard_vcont) (discard_pending_stop_replies_in_queue, stop_reply) (remote_notif_remove_queued_reply, stop_reply *queued_stop_reply) (peek_stop_reply, remote_parse_stop_reply, remote_stop_ns) (remote_interrupt_as, remote_interrupt_ns) (remote_get_noisy_reply, remote_query_attached) (remote_add_inferior, remote_current_thread, get_current_thread) (set_thread, set_general_thread, set_continue_thread) (set_general_process, write_ptid) (remote_unpack_thread_info_response, remote_get_threadinfo) (parse_threadlist_response, remote_get_threadlist) (remote_threadlist_iterator, remote_get_threads_with_ql) (remote_get_threads_with_qxfer) (remote_get_threads_with_qthreadinfo, extended_remote_restart) (get_offsets, remote_check_symbols, remote_supported_packet) (remote_query_supported, remote_packet_size) (remote_serial_quit_handler, remote_detach_pid) (remote_vcont_probe, remote_resume_with_hc) (send_interrupt_sequence, interrupt_query) (remote_notif_get_pending_events, fetch_register_using_p) (send_g_packet, process_g_packet, fetch_registers_using_g) (store_register_using_P, store_registers_using_G) (set_remote_traceframe, check_binary_download) (remote_write_bytes_aux, remote_write_bytes, remote_read_bytes_1) (remote_xfer_live_readonly_partial, remote_read_bytes) (remote_send_printf, remote_flash_write, readchar) (remote_serial_write, putpkt, putpkt_binary, skip_frame) (read_frame, getpkt, getpkt_or_notif_sane_1, getpkt_sane) (getpkt_or_notif_sane, remote_vkill, remote_kill_k) (extended_remote_disable_randomization, extended_remote_run) (send_environment_packet, extended_remote_environment_support) (extended_remote_set_inferior_cwd, remote_write_qxfer) (remote_read_qxfer, push_stop_reply, vcont_r_supported) (packet_command): Now methods of ... (remote_target): ... this class. (m_remote_state) <remote_target>: New field. (struct remote_state) <stop_reply_queue, remote_async_inferior_event_token, wait_forever_enabled_p>: New fields. (remote_state::remote_state): Allocate stop_reply_queue. (remote_state): Delete global. (get_remote_state_raw): Delete. (remote_target::get_remote_state): Allocate m_remote_state on demand. (get_current_remote_target): New. (wait_forever_enabled_p, remote_async_inferior_event_token): Delete, moved to struct remote_state. (remote_target::close): Delete self. Destruction bits split to ... (remote_target::~remote_target): ... this. (show_memory_packet_size): Adjust to use get_current_remote_target. (struct protocol_feature) <func>: Add remote_target parameter. All callers adjusted. (curr_quit_handler_target): New. (remote_serial_quit_handler): Reimplement. (remote_target::open_1): Adjust to use get_current_remote_target. Heap-allocate remote_target/extended_remote_target instances. (vcont_builder::vcont_builder): Add remote_target parameter, and save it in m_remote. All callers adjusted. (vcont_builder::m_remote): New field. (vcont_builder::restart, vcont_builder::flush) (vcont_builder::push_action): Use it. (remote_target::commit_resume): Use it. (struct queue_iter_param) <remote>: New field. (remote_target::remove_new_fork_children): Fill in 'remote' field. (check_pending_event_prevents_wildcard_vcont_callback_data): New. (check_pending_event_prevents_wildcard_vcont_callback) (remote_target::check_pending_events_prevent_wildcard_vcont) (remote_target::discard_pending_stop_replies) (remote_target::discard_pending_stop_replies_in_queue) (remote_target::remote_notif_remove_queued_reply): Fill in 'remote' field. (remote_notif_get_pending_events): New. (remote_target::readchar, remote_target::remote_serial_write): Save/restore curr_quit_handler_target. (putpkt): New. (kill_new_fork_children): Fill in 'remote' field. (packet_command): Use get_current_remote_target, defer to remote_target method of same name. (scoped_remote_fd::scoped_remote_fd): Add 'remote_target' parameter, and save it in m_remote. All callers adjusted. (scoped_remote_fd::release): Use m_remote. (scoped_remote_fd::m_remote): New field. (remote_file_put, remote_file_get, remote_file_delete): Use get_current_remote_target, defer to remote_target method of same name. (remote_btrace_reset): Add remote_state paremeter. Update all callers. (remote_async_inferior_event_handler). Pass down 'data'. (remote_new_objfile): Use get_current_remote_target. (remote_target::vcont_r_supported): New. (set_range_stepping): Use get_current_remote_target and remote_target::vcont_r_supported. (_initialize_remote): Don't allocate 'remote_state' and 'stop_reply_queue' globals. * remote.h (struct remote_target): Forward declare. (getpkt, putpkt, remote_notif_get_pending_events): Add 'remote_target' parameter. --- gdb/remote-fileio.c | 214 ++++---- gdb/remote-fileio.h | 3 +- gdb/remote-notif.c | 22 +- gdb/remote-notif.h | 21 +- gdb/remote.c | 1527 ++++++++++++++++++++++++++++++--------------------- gdb/remote.h | 9 +- 6 files changed, 1058 insertions(+), 738 deletions(-) diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c index 4c648a9c83..313da642ea 100644 --- a/gdb/remote-fileio.c +++ b/gdb/remote-fileio.c @@ -313,7 +313,7 @@ remote_fileio_quit_handler (void) } static void -remote_fileio_reply (int retcode, int error) +remote_fileio_reply (remote_target *remote, int retcode, int error) { char buf[32]; int ctrl_c = check_quit_flag (); @@ -339,36 +339,36 @@ remote_fileio_reply (int retcode, int error) strcat (buf, ",C"); } quit_handler = remote_fileio_o_quit_handler; - putpkt (buf); + putpkt (remote, buf); } static void -remote_fileio_ioerror (void) +remote_fileio_ioerror (remote_target *remote) { - remote_fileio_reply (-1, FILEIO_EIO); + remote_fileio_reply (remote, -1, FILEIO_EIO); } static void -remote_fileio_badfd (void) +remote_fileio_badfd (remote_target *remote) { - remote_fileio_reply (-1, FILEIO_EBADF); + remote_fileio_reply (remote, -1, FILEIO_EBADF); } static void -remote_fileio_return_errno (int retcode) +remote_fileio_return_errno (remote_target *remote, int retcode) { - remote_fileio_reply (retcode, retcode < 0 + remote_fileio_reply (remote, retcode, retcode < 0 ? host_to_fileio_error (errno) : 0); } static void -remote_fileio_return_success (int retcode) +remote_fileio_return_success (remote_target *remote, int retcode) { - remote_fileio_reply (retcode, 0); + remote_fileio_reply (remote, retcode, 0); } static void -remote_fileio_func_open (char *buf) +remote_fileio_func_open (remote_target *remote, char *buf) { CORE_ADDR ptrval; int length; @@ -381,20 +381,20 @@ remote_fileio_func_open (char *buf) /* 1. Parameter: Ptr to pathname / length incl. trailing zero. */ if (remote_fileio_extract_ptr_w_len (&buf, &ptrval, &length)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } /* 2. Parameter: open flags */ if (remote_fileio_extract_int (&buf, &num)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } flags = remote_fileio_oflags_to_host (num); /* 3. Parameter: open mode */ if (remote_fileio_extract_int (&buf, &num)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } mode = remote_fileio_mode_to_host (num, 1); @@ -403,7 +403,7 @@ remote_fileio_func_open (char *buf) pathname = (char *) alloca (length); if (target_read_memory (ptrval, (gdb_byte *) pathname, length) != 0) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } @@ -414,13 +414,13 @@ remote_fileio_func_open (char *buf) { if (!S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode)) { - remote_fileio_reply (-1, FILEIO_ENODEV); + remote_fileio_reply (remote, -1, FILEIO_ENODEV); return; } if (S_ISDIR (st.st_mode) && ((flags & O_WRONLY) == O_WRONLY || (flags & O_RDWR) == O_RDWR)) { - remote_fileio_reply (-1, FILEIO_EISDIR); + remote_fileio_reply (remote, -1, FILEIO_EISDIR); return; } } @@ -428,16 +428,16 @@ remote_fileio_func_open (char *buf) fd = gdb_open_cloexec (pathname, flags, mode); if (fd < 0) { - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); return; } fd = remote_fileio_fd_to_targetfd (fd); - remote_fileio_return_success (fd); + remote_fileio_return_success (remote, fd); } static void -remote_fileio_func_close (char *buf) +remote_fileio_func_close (remote_target *remote, char *buf) { long num; int fd; @@ -445,24 +445,24 @@ remote_fileio_func_close (char *buf) /* Parameter: file descriptor */ if (remote_fileio_extract_int (&buf, &num)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } fd = remote_fileio_map_fd ((int) num); if (fd == FIO_FD_INVALID) { - remote_fileio_badfd (); + remote_fileio_badfd (remote); return; } if (fd != FIO_FD_CONSOLE_IN && fd != FIO_FD_CONSOLE_OUT && close (fd)) - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); remote_fileio_close_target_fd ((int) num); - remote_fileio_return_success (0); + remote_fileio_return_success (remote, 0); } static void -remote_fileio_func_read (char *buf) +remote_fileio_func_read (remote_target *remote, char *buf) { long target_fd, num; LONGEST lnum; @@ -475,26 +475,26 @@ remote_fileio_func_read (char *buf) /* 1. Parameter: file descriptor */ if (remote_fileio_extract_int (&buf, &target_fd)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } fd = remote_fileio_map_fd ((int) target_fd); if (fd == FIO_FD_INVALID) { - remote_fileio_badfd (); + remote_fileio_badfd (remote); return; } /* 2. Parameter: buffer pointer */ if (remote_fileio_extract_long (&buf, &lnum)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } ptrval = (CORE_ADDR) lnum; /* 3. Parameter: buffer length */ if (remote_fileio_extract_int (&buf, &num)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } length = (size_t) num; @@ -502,7 +502,7 @@ remote_fileio_func_read (char *buf) switch (fd) { case FIO_FD_CONSOLE_OUT: - remote_fileio_badfd (); + remote_fileio_badfd (remote); return; case FIO_FD_CONSOLE_IN: { @@ -580,15 +580,15 @@ remote_fileio_func_read (char *buf) } if (ret < 0) - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); else - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); xfree (buffer); } static void -remote_fileio_func_write (char *buf) +remote_fileio_func_write (remote_target *remote, char *buf) { long target_fd, num; LONGEST lnum; @@ -600,26 +600,26 @@ remote_fileio_func_write (char *buf) /* 1. Parameter: file descriptor */ if (remote_fileio_extract_int (&buf, &target_fd)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } fd = remote_fileio_map_fd ((int) target_fd); if (fd == FIO_FD_INVALID) { - remote_fileio_badfd (); + remote_fileio_badfd (remote); return; } /* 2. Parameter: buffer pointer */ if (remote_fileio_extract_long (&buf, &lnum)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } ptrval = (CORE_ADDR) lnum; /* 3. Parameter: buffer length */ if (remote_fileio_extract_int (&buf, &num)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } length = (size_t) num; @@ -628,14 +628,14 @@ remote_fileio_func_write (char *buf) if (target_read_memory (ptrval, buffer, length) != 0) { xfree (buffer); - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } switch (fd) { case FIO_FD_CONSOLE_IN: - remote_fileio_badfd (); + remote_fileio_badfd (remote); xfree (buffer); return; case FIO_FD_CONSOLE_OUT: @@ -653,15 +653,15 @@ remote_fileio_func_write (char *buf) } if (ret < 0) - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); else - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); xfree (buffer); } static void -remote_fileio_func_lseek (char *buf) +remote_fileio_func_lseek (remote_target *remote, char *buf) { long num; LONGEST lnum; @@ -671,50 +671,50 @@ remote_fileio_func_lseek (char *buf) /* 1. Parameter: file descriptor */ if (remote_fileio_extract_int (&buf, &num)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } fd = remote_fileio_map_fd ((int) num); if (fd == FIO_FD_INVALID) { - remote_fileio_badfd (); + remote_fileio_badfd (remote); return; } else if (fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT) { - remote_fileio_reply (-1, FILEIO_ESPIPE); + remote_fileio_reply (remote, -1, FILEIO_ESPIPE); return; } /* 2. Parameter: offset */ if (remote_fileio_extract_long (&buf, &lnum)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } offset = (off_t) lnum; /* 3. Parameter: flag */ if (remote_fileio_extract_int (&buf, &num)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } if (remote_fileio_seek_flag_to_host (num, &flag)) { - remote_fileio_reply (-1, FILEIO_EINVAL); + remote_fileio_reply (remote, -1, FILEIO_EINVAL); return; } ret = lseek (fd, offset, flag); if (ret == (off_t) -1) - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); else - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); } static void -remote_fileio_func_rename (char *buf) +remote_fileio_func_rename (remote_target *remote, char *buf) { CORE_ADDR old_ptr, new_ptr; int old_len, new_len; @@ -725,14 +725,14 @@ remote_fileio_func_rename (char *buf) /* 1. Parameter: Ptr to oldpath / length incl. trailing zero */ if (remote_fileio_extract_ptr_w_len (&buf, &old_ptr, &old_len)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } /* 2. Parameter: Ptr to newpath / length incl. trailing zero */ if (remote_fileio_extract_ptr_w_len (&buf, &new_ptr, &new_len)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } @@ -740,7 +740,7 @@ remote_fileio_func_rename (char *buf) oldpath = (char *) alloca (old_len); if (target_read_memory (old_ptr, (gdb_byte *) oldpath, old_len) != 0) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } @@ -748,7 +748,7 @@ remote_fileio_func_rename (char *buf) newpath = (char *) alloca (new_len); if (target_read_memory (new_ptr, (gdb_byte *) newpath, new_len) != 0) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } @@ -758,7 +758,7 @@ remote_fileio_func_rename (char *buf) if ((!of && !S_ISREG (ost.st_mode) && !S_ISDIR (ost.st_mode)) || (!nf && !S_ISREG (nst.st_mode) && !S_ISDIR (nst.st_mode))) { - remote_fileio_reply (-1, FILEIO_EACCES); + remote_fileio_reply (remote, -1, FILEIO_EACCES); return; } @@ -800,14 +800,14 @@ remote_fileio_func_rename (char *buf) } #endif - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); } else - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); } static void -remote_fileio_func_unlink (char *buf) +remote_fileio_func_unlink (remote_target *remote, char *buf) { CORE_ADDR ptrval; int length; @@ -818,14 +818,14 @@ remote_fileio_func_unlink (char *buf) /* Parameter: Ptr to pathname / length incl. trailing zero */ if (remote_fileio_extract_ptr_w_len (&buf, &ptrval, &length)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } /* Request pathname using 'm' packet */ pathname = (char *) alloca (length); if (target_read_memory (ptrval, (gdb_byte *) pathname, length) != 0) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } @@ -833,20 +833,20 @@ remote_fileio_func_unlink (char *buf) the correct return code). */ if (!stat (pathname, &st) && !S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode)) { - remote_fileio_reply (-1, FILEIO_ENODEV); + remote_fileio_reply (remote, -1, FILEIO_ENODEV); return; } ret = unlink (pathname); if (ret == -1) - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); else - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); } static void -remote_fileio_func_stat (char *buf) +remote_fileio_func_stat (remote_target *remote, char *buf) { CORE_ADDR statptr, nameptr; int ret, namelength; @@ -858,14 +858,14 @@ remote_fileio_func_stat (char *buf) /* 1. Parameter: Ptr to pathname / length incl. trailing zero */ if (remote_fileio_extract_ptr_w_len (&buf, &nameptr, &namelength)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } /* 2. Parameter: Ptr to struct stat */ if (remote_fileio_extract_long (&buf, &lnum)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } statptr = (CORE_ADDR) lnum; @@ -874,7 +874,7 @@ remote_fileio_func_stat (char *buf) pathname = (char *) alloca (namelength); if (target_read_memory (nameptr, (gdb_byte *) pathname, namelength) != 0) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } @@ -882,13 +882,13 @@ remote_fileio_func_stat (char *buf) if (ret == -1) { - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); return; } /* Only operate on regular files and directories. */ if (!ret && !S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode)) { - remote_fileio_reply (-1, FILEIO_EACCES); + remote_fileio_reply (remote, -1, FILEIO_EACCES); return; } if (statptr) @@ -899,15 +899,15 @@ remote_fileio_func_stat (char *buf) errno = target_write_memory (statptr, (gdb_byte *) &fst, sizeof fst); if (errno != 0) { - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); return; } } - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); } static void -remote_fileio_func_fstat (char *buf) +remote_fileio_func_fstat (remote_target *remote, char *buf) { CORE_ADDR ptrval; int fd, ret; @@ -920,19 +920,19 @@ remote_fileio_func_fstat (char *buf) /* 1. Parameter: file descriptor */ if (remote_fileio_extract_int (&buf, &target_fd)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } fd = remote_fileio_map_fd ((int) target_fd); if (fd == FIO_FD_INVALID) { - remote_fileio_badfd (); + remote_fileio_badfd (remote); return; } /* 2. Parameter: Ptr to struct stat */ if (remote_fileio_extract_long (&buf, &lnum)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } ptrval = (CORE_ADDR) lnum; @@ -966,7 +966,7 @@ remote_fileio_func_fstat (char *buf) if (ret == -1) { - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); return; } if (ptrval) @@ -976,15 +976,15 @@ remote_fileio_func_fstat (char *buf) errno = target_write_memory (ptrval, (gdb_byte *) &fst, sizeof fst); if (errno != 0) { - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); return; } } - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); } static void -remote_fileio_func_gettimeofday (char *buf) +remote_fileio_func_gettimeofday (remote_target *remote, char *buf) { LONGEST lnum; CORE_ADDR ptrval; @@ -995,20 +995,20 @@ remote_fileio_func_gettimeofday (char *buf) /* 1. Parameter: struct timeval pointer */ if (remote_fileio_extract_long (&buf, &lnum)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } ptrval = (CORE_ADDR) lnum; /* 2. Parameter: some pointer value... */ if (remote_fileio_extract_long (&buf, &lnum)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } /* ...which has to be NULL. */ if (lnum) { - remote_fileio_reply (-1, FILEIO_EINVAL); + remote_fileio_reply (remote, -1, FILEIO_EINVAL); return; } @@ -1016,7 +1016,7 @@ remote_fileio_func_gettimeofday (char *buf) if (ret == -1) { - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); return; } @@ -1027,15 +1027,15 @@ remote_fileio_func_gettimeofday (char *buf) errno = target_write_memory (ptrval, (gdb_byte *) &ftv, sizeof ftv); if (errno != 0) { - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); return; } } - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); } static void -remote_fileio_func_isatty (char *buf) +remote_fileio_func_isatty (remote_target *remote, char *buf) { long target_fd; int fd; @@ -1043,16 +1043,16 @@ remote_fileio_func_isatty (char *buf) /* Parameter: file descriptor */ if (remote_fileio_extract_int (&buf, &target_fd)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } fd = remote_fileio_map_fd ((int) target_fd); - remote_fileio_return_success (fd == FIO_FD_CONSOLE_IN || - fd == FIO_FD_CONSOLE_OUT ? 1 : 0); + int ret = fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT ? 1 : 0; + remote_fileio_return_success (remote, ret); } static void -remote_fileio_func_system (char *buf) +remote_fileio_func_system (remote_target *remote, char *buf) { CORE_ADDR ptrval; int ret, length; @@ -1061,7 +1061,7 @@ remote_fileio_func_system (char *buf) /* Parameter: Ptr to commandline / length incl. trailing zero */ if (remote_fileio_extract_ptr_w_len (&buf, &ptrval, &length)) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } @@ -1071,7 +1071,7 @@ remote_fileio_func_system (char *buf) cmdline = (char *) alloca (length); if (target_read_memory (ptrval, (gdb_byte *) cmdline, length) != 0) { - remote_fileio_ioerror (); + remote_fileio_ioerror (remote); return; } } @@ -1083,25 +1083,25 @@ remote_fileio_func_system (char *buf) if (!remote_fio_system_call_allowed) { if (!length) - remote_fileio_return_success (0); + remote_fileio_return_success (remote, 0); else - remote_fileio_reply (-1, FILEIO_EPERM); + remote_fileio_reply (remote, -1, FILEIO_EPERM); return; } ret = system (cmdline); if (!length) - remote_fileio_return_success (ret); + remote_fileio_return_success (remote, ret); else if (ret == -1) - remote_fileio_return_errno (-1); + remote_fileio_return_errno (remote, -1); else - remote_fileio_return_success (WEXITSTATUS (ret)); + remote_fileio_return_success (remote, WEXITSTATUS (ret)); } static struct { const char *name; - void (*func)(char *); + void (*func)(remote_target *remote, char *); } remote_fio_func_map[] = { { "open", remote_fileio_func_open }, { "close", remote_fileio_func_close }, @@ -1119,7 +1119,7 @@ static struct { }; static void -do_remote_fileio_request (char *buf) +do_remote_fileio_request (remote_target *remote, char *buf) { char *c; int idx; @@ -1135,9 +1135,9 @@ do_remote_fileio_request (char *buf) if (!strcmp (remote_fio_func_map[idx].name, buf)) break; if (!remote_fio_func_map[idx].name) - remote_fileio_reply (-1, FILEIO_ENOSYS); + remote_fileio_reply (remote, -1, FILEIO_ENOSYS); else - remote_fio_func_map[idx].func (c); + remote_fio_func_map[idx].func (remote, c); } /* Close any open descriptors, and reinitialize the file mapping. */ @@ -1167,7 +1167,7 @@ remote_fileio_reset (void) acknowledged the Ctrl-C sent asynchronously earlier. */ void -remote_fileio_request (char *buf, int ctrlc_pending_p) +remote_fileio_request (remote_target *remote, char *buf, int ctrlc_pending_p) { /* Save the previous quit handler, so we can restore it. No need for a cleanup since we catch all exceptions below. Note that the @@ -1181,20 +1181,20 @@ remote_fileio_request (char *buf, int ctrlc_pending_p) asynchronously earlier, take this opportunity to send the Ctrl-C synchronously. */ set_quit_flag (); - remote_fileio_reply (-1, FILEIO_EINTR); + remote_fileio_reply (remote, -1, FILEIO_EINTR); } else { TRY { - do_remote_fileio_request (buf); + do_remote_fileio_request (remote, buf); } CATCH (ex, RETURN_MASK_ALL) { if (ex.reason == RETURN_QUIT) - remote_fileio_reply (-1, FILEIO_EINTR); + remote_fileio_reply (remote, -1, FILEIO_EINTR); else - remote_fileio_reply (-1, FILEIO_EIO); + remote_fileio_reply (remote, -1, FILEIO_EIO); } END_CATCH } diff --git a/gdb/remote-fileio.h b/gdb/remote-fileio.h index 9f7cd982c6..ed553a4468 100644 --- a/gdb/remote-fileio.h +++ b/gdb/remote-fileio.h @@ -28,7 +28,8 @@ struct cmd_list_element; /* Unified interface to remote fileio, called in remote.c from remote_wait () and remote_async_wait (). */ -extern void remote_fileio_request (char *buf, int ctrlc_pending_p); +extern void remote_fileio_request (remote_target *remote, + char *buf, int ctrlc_pending_p); /* Cleanup any remote fileio state. */ extern void remote_fileio_reset (void); diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c index 85612b1e39..7b889801ca 100644 --- a/gdb/remote-notif.c +++ b/gdb/remote-notif.c @@ -58,7 +58,8 @@ static void do_notif_event_xfree (void *arg); acknowledge. */ void -remote_notif_ack (struct notif_client *nc, char *buf) +remote_notif_ack (remote_target *remote, + struct notif_client *nc, char *buf) { struct notif_event *event = nc->alloc_event (); struct cleanup *old_chain @@ -68,8 +69,8 @@ remote_notif_ack (struct notif_client *nc, char *buf) fprintf_unfiltered (gdb_stdlog, "notif: ack '%s'\n", nc->ack_command); - nc->parse (nc, buf, event); - nc->ack (nc, buf, event); + nc->parse (remote, nc, buf, event); + nc->ack (remote, nc, buf, event); discard_cleanups (old_chain); } @@ -77,7 +78,8 @@ remote_notif_ack (struct notif_client *nc, char *buf) /* Parse the BUF for the expected notification NC. */ struct notif_event * -remote_notif_parse (struct notif_client *nc, char *buf) +remote_notif_parse (remote_target *remote, + struct notif_client *nc, char *buf) { struct notif_event *event = nc->alloc_event (); struct cleanup *old_chain @@ -86,7 +88,7 @@ remote_notif_parse (struct notif_client *nc, char *buf) if (notif_debug) fprintf_unfiltered (gdb_stdlog, "notif: parse '%s'\n", nc->name); - nc->parse (nc, buf, event); + nc->parse (remote, nc, buf, event); discard_cleanups (old_chain); return event; @@ -108,8 +110,8 @@ remote_notif_process (struct remote_notif_state *state, gdb_assert (nc != except); - if (nc->can_get_pending_events (nc)) - remote_notif_get_pending_events (nc); + if (nc->can_get_pending_events (state->remote, nc)) + remote_notif_get_pending_events (state->remote, nc); } } @@ -157,7 +159,7 @@ handle_notification (struct remote_notif_state *state, char *buf) else { struct notif_event *event - = remote_notif_parse (nc, buf + strlen (nc->name) + 1); + = remote_notif_parse (state->remote, nc, buf + strlen (nc->name) + 1); /* Be careful to only set it after parsing, since an error may be thrown then. */ @@ -236,10 +238,12 @@ do_notif_event_xfree (void *arg) /* Return an allocated remote_notif_state. */ struct remote_notif_state * -remote_notif_state_allocate (void) +remote_notif_state_allocate (remote_target *remote) { struct remote_notif_state *notif_state = XCNEW (struct remote_notif_state); + notif_state->remote = remote; + notif_state->notif_queue = QUEUE_alloc (notif_client_p, NULL); /* Register async_event_handler for notification. */ diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h index 98d7a30cab..dafa177817 100644 --- a/gdb/remote-notif.h +++ b/gdb/remote-notif.h @@ -39,6 +39,8 @@ enum REMOTE_NOTIF_ID REMOTE_NOTIF_LAST, }; +struct remote_target; + /* A client to a sort of async remote notification. */ typedef struct notif_client @@ -52,17 +54,20 @@ typedef struct notif_client /* Parse BUF to get the expected event and update EVENT. This function may throw exception if contents in BUF is not the expected event. */ - void (*parse) (struct notif_client *self, char *buf, + void (*parse) (remote_target *remote, + struct notif_client *self, char *buf, struct notif_event *event); /* Send field <ack_command> to remote, and do some checking. If something wrong, throw an exception. */ - void (*ack) (struct notif_client *self, char *buf, + void (*ack) (remote_target *remote, + struct notif_client *self, char *buf, struct notif_event *event); /* Check this notification client can get pending events in 'remote_notif_process'. */ - int (*can_get_pending_events) (struct notif_client *self); + int (*can_get_pending_events) (remote_target *remote, + struct notif_client *self); /* Allocate an event. */ struct notif_event *(*alloc_event) (void); @@ -77,6 +82,9 @@ DECLARE_QUEUE_P (notif_client_p); struct remote_notif_state { + /* The remote target. */ + remote_target *remote; + /* Notification queue. */ QUEUE(notif_client_p) *notif_queue; @@ -98,8 +106,9 @@ struct remote_notif_state struct notif_event *pending_event[REMOTE_NOTIF_LAST]; }; -void remote_notif_ack (struct notif_client *nc, char *buf); -struct notif_event *remote_notif_parse (struct notif_client *nc, +void remote_notif_ack (remote_target *remote, notif_client *nc, char *buf); +struct notif_event *remote_notif_parse (remote_target *remote, + notif_client *nc, char *buf); void notif_event_xfree (struct notif_event *event); @@ -109,7 +118,7 @@ void handle_notification (struct remote_notif_state *notif_state, void remote_notif_process (struct remote_notif_state *state, struct notif_client *except); -struct remote_notif_state *remote_notif_state_allocate (void); +remote_notif_state *remote_notif_state_allocate (remote_target *remote); void remote_notif_state_xfree (struct remote_notif_state *state); extern struct notif_client notif_client_stop; diff --git a/gdb/remote.c b/gdb/remote.c index 4166598750..97c6e6e4fb 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -84,6 +84,47 @@ Use a remote computer via a serial line, using a gdb-specific protocol.\n\ Specify the serial device it is connected to\n\ (e.g. /dev/ttyS0, /dev/ttya, COM1, etc.)."); +#define OPAQUETHREADBYTES 8 + +/* a 64 bit opaque identifier */ +typedef unsigned char threadref[OPAQUETHREADBYTES]; + +struct gdb_ext_thread_info; +struct threads_listing_context; +typedef int (*rmt_thread_action) (threadref *ref, void *context); +struct protocol_feature; +struct packet_reg; + +struct stop_reply; +typedef struct stop_reply *stop_reply_p; + +DECLARE_QUEUE_P (stop_reply_p); +DEFINE_QUEUE_P (stop_reply_p); + +/* Generic configuration support for packets the stub optionally + supports. Allows the user to specify the use of the packet as well + as allowing GDB to auto-detect support in the remote stub. */ + +enum packet_support + { + PACKET_SUPPORT_UNKNOWN = 0, + PACKET_ENABLE, + PACKET_DISABLE + }; + +/* Analyze a packet's return value and update the packet config + accordingly. */ + +enum packet_result +{ + PACKET_ERROR, + PACKET_OK, + PACKET_UNKNOWN +}; + +struct threads_listing_context; +struct remote_state; + static const target_info remote_target_info = { "remote", N_("Remote serial target in gdb-specific protocol"), @@ -97,6 +138,7 @@ public: { to_stratum = process_stratum; } + ~remote_target () override; const target_info &info () const override { return remote_target_info; } @@ -378,9 +420,258 @@ public: int remove_exec_catchpoint (int) override; enum exec_direction_kind execution_direction () override; -protected: +public: /* Remote specific methods. */ + + void remote_download_command_source (int num, ULONGEST addr, + struct command_line *cmds); + + void remote_file_put (const char *local_file, const char *remote_file, + int from_tty); + void remote_file_get (const char *remote_file, const char *local_file, + int from_tty); + void remote_file_delete (const char *remote_file, int from_tty); + + int remote_hostio_pread (int fd, gdb_byte *read_buf, int len, + ULONGEST offset, int *remote_errno); + int remote_hostio_pwrite (int fd, const gdb_byte *write_buf, int len, + ULONGEST offset, int *remote_errno); + int remote_hostio_pread_vFile (int fd, gdb_byte *read_buf, int len, + ULONGEST offset, int *remote_errno); + + int remote_hostio_send_command (int command_bytes, int which_packet, + int *remote_errno, char **attachment, + int *attachment_len); + int remote_hostio_set_filesystem (struct inferior *inf, + int *remote_errno); + /* We should get rid of this and use fileio_open directly. */ + int remote_hostio_open (struct inferior *inf, const char *filename, + int flags, int mode, int warn_if_slow, + int *remote_errno); + int remote_hostio_close (int fd, int *remote_errno); + + int remote_hostio_unlink (inferior *inf, const char *filename, + int *remote_errno); + + struct remote_state *get_remote_state (); + + long get_remote_packet_size (void); + long get_memory_packet_size (struct memory_packet_config *config); + + long get_memory_write_packet_size (); + long get_memory_read_packet_size (); + + char *append_pending_thread_resumptions (char *p, char *endp, + ptid_t ptid); static void open_1 (const char *name, int from_tty, int extended_p); void start_remote (int from_tty, int extended_p); + void remote_detach_1 (int from_tty, struct inferior *inf); + + char *append_resumption (char *p, char *endp, + ptid_t ptid, int step, gdb_signal siggnal); + int remote_resume_with_vcont (ptid_t ptid, int step, + gdb_signal siggnal); + + void add_current_inferior_and_thread (char *wait_status); + + ptid_t wait_ns (ptid_t ptid, struct target_waitstatus *status, + int options); + ptid_t wait_as (ptid_t ptid, target_waitstatus *status, + int options); + + ptid_t process_stop_reply (struct stop_reply *stop_reply, + target_waitstatus *status); + + void remote_notice_new_inferior (ptid_t currthread, int executing); + + void process_initial_stop_replies (int from_tty); + + void remote_add_thread (ptid_t ptid, bool running, bool executing); + + void btrace_sync_conf (const btrace_config *conf); + + void remote_btrace_maybe_reopen (); + + void remove_new_fork_children (threads_listing_context *context); + void kill_new_fork_children (int pid); + void discard_pending_stop_replies (struct inferior *inf); + int stop_reply_queue_length (); + + void check_pending_events_prevent_wildcard_vcont + (int *may_global_wildcard_vcont); + + void discard_pending_stop_replies_in_queue (); + struct stop_reply *remote_notif_remove_queued_reply (ptid_t ptid); + struct stop_reply *queued_stop_reply (ptid_t ptid); + int peek_stop_reply (ptid_t ptid); + void remote_parse_stop_reply (char *buf, stop_reply *event); + + void remote_stop_ns (ptid_t ptid); + void remote_interrupt_as (); + void remote_interrupt_ns (); + + char *remote_get_noisy_reply (); + int remote_query_attached (int pid); + inferior *remote_add_inferior (int fake_pid_p, int pid, int attached, + int try_open_exec); + + ptid_t remote_current_thread (ptid_t oldpid); + ptid_t get_current_thread (char *wait_status); + + void set_thread (ptid_t ptid, int gen); + void set_general_thread (ptid_t ptid); + void set_continue_thread (ptid_t ptid); + void set_general_process (); + + char *write_ptid (char *buf, const char *endbuf, ptid_t ptid); + + int remote_unpack_thread_info_response (char *pkt, threadref *expectedref, + gdb_ext_thread_info *info); + int remote_get_threadinfo (threadref *threadid, int fieldset, + gdb_ext_thread_info *info); + + int parse_threadlist_response (char *pkt, int result_limit, + threadref *original_echo, + threadref *resultlist, + int *doneflag); + int remote_get_threadlist (int startflag, threadref *nextthread, + int result_limit, int *done, int *result_count, + threadref *threadlist); + + int remote_threadlist_iterator (rmt_thread_action stepfunction, + void *context, int looplimit); + + int remote_get_threads_with_ql (threads_listing_context *context); + int remote_get_threads_with_qxfer (threads_listing_context *context); + int remote_get_threads_with_qthreadinfo (threads_listing_context *context); + + void extended_remote_restart (); + + void get_offsets (); + + void remote_check_symbols (); + + void remote_supported_packet (const struct protocol_feature *feature, + enum packet_support support, + const char *argument); + + void remote_query_supported (); + + void remote_packet_size (const protocol_feature *feature, + packet_support support, const char *value); + + void remote_serial_quit_handler (); + + void remote_detach_pid (int pid); + + void remote_vcont_probe (); + + void remote_resume_with_hc (ptid_t ptid, int step, + gdb_signal siggnal); + + void send_interrupt_sequence (); + void interrupt_query (); + + void remote_notif_get_pending_events (notif_client *nc); + + int fetch_register_using_p (struct regcache *regcache, + packet_reg *reg); + int send_g_packet (); + void process_g_packet (struct regcache *regcache); + void fetch_registers_using_g (struct regcache *regcache); + int store_register_using_P (const struct regcache *regcache, + packet_reg *reg); + void store_registers_using_G (const struct regcache *regcache); + + void set_remote_traceframe (); + + void check_binary_download (CORE_ADDR addr); + + target_xfer_status remote_write_bytes_aux (const char *header, + CORE_ADDR memaddr, + const gdb_byte *myaddr, + ULONGEST len_units, + int unit_size, + ULONGEST *xfered_len_units, + char packet_format, + int use_length); + + target_xfer_status remote_write_bytes (CORE_ADDR memaddr, + const gdb_byte *myaddr, ULONGEST len, + int unit_size, ULONGEST *xfered_len); + + target_xfer_status remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, + ULONGEST len_units, + int unit_size, ULONGEST *xfered_len_units); + + target_xfer_status remote_xfer_live_readonly_partial (gdb_byte *readbuf, + ULONGEST memaddr, + ULONGEST len, + int unit_size, + ULONGEST *xfered_len); + + target_xfer_status remote_read_bytes (CORE_ADDR memaddr, + gdb_byte *myaddr, ULONGEST len, + int unit_size, + ULONGEST *xfered_len); + + packet_result remote_send_printf (const char *format, ...) + ATTRIBUTE_PRINTF (2, 3); + + target_xfer_status remote_flash_write (ULONGEST address, + ULONGEST length, ULONGEST *xfered_len, + const gdb_byte *data); + + int readchar (int timeout); + + void remote_serial_write (const char *str, int len); + + int putpkt (const char *buf); + int putpkt_binary (const char *buf, int cnt); + + void skip_frame (); + long read_frame (char **buf_p, long *sizeof_buf); + void getpkt (char **buf, long *sizeof_buf, int forever); + int getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever, + int expecting_notif, int *is_notif); + int getpkt_sane (char **buf, long *sizeof_buf, int forever); + int getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever, + int *is_notif); + int remote_vkill (int pid); + void remote_kill_k (); + + void extended_remote_disable_randomization (int val); + int extended_remote_run (const std::string &args); + + void send_environment_packet (const char *action, + const char *packet, + const char *value); + + void extended_remote_environment_support (); + void extended_remote_set_inferior_cwd (); + + target_xfer_status remote_write_qxfer (const char *object_name, + const char *annex, + const gdb_byte *writebuf, + ULONGEST offset, LONGEST len, + ULONGEST *xfered_len, + struct packet_config *packet); + + target_xfer_status remote_read_qxfer (const char *object_name, + const char *annex, + gdb_byte *readbuf, ULONGEST offset, + LONGEST len, + ULONGEST *xfered_len, + struct packet_config *packet); + + void push_stop_reply (struct stop_reply *new_event); + + bool vcont_r_supported (); + + void packet_command (const char *args, int from_tty); + +private: /* data fields */ + + std::unique_ptr<struct remote_state> m_remote_state; }; static const target_info extended_remote_target_info = { @@ -436,30 +727,6 @@ static char *remote_exec_file_var; enum { REMOTE_ALIGN_WRITES = 16 }; /* Prototypes for local functions. */ -static int getpkt_sane (char **buf, long *sizeof_buf, int forever); -static int getpkt_or_notif_sane (char **buf, long *sizeof_buf, - int forever, int *is_notif); - -struct remote_state; - -static int remote_vkill (int pid, struct remote_state *rs); - -static void remote_kill_k (void); - -static int readchar (int timeout); - -static void remote_serial_write (const char *str, int len); - -static void interrupt_query (void); - -static void set_general_thread (ptid_t ptid); -static void set_continue_thread (ptid_t ptid); - -static void get_offsets (void); - -static void skip_frame (void); - -static long read_frame (char **buf_p, long *sizeof_buf); static int hexnumlen (ULONGEST num); @@ -475,12 +742,6 @@ static void print_packet (const char *); static int stub_unpack_int (char *buff, int fieldlength); -static ptid_t remote_current_thread (ptid_t oldptid); - -static int putpkt_binary (const char *buf, int cnt); - -static void check_binary_download (CORE_ADDR addr); - struct packet_config; static void show_packet_config_cmd (struct packet_config *config); @@ -490,22 +751,10 @@ static void show_remote_protocol_packet_cmd (struct ui_file *file, struct cmd_list_element *c, const char *value); -static char *write_ptid (char *buf, const char *endbuf, ptid_t ptid); static ptid_t read_ptid (const char *buf, const char **obuf); -static void remote_query_supported (void); - -static void remote_check_symbols (void); - struct stop_reply; static void stop_reply_xfree (struct stop_reply *); -static void remote_parse_stop_reply (char *, struct stop_reply *); -static void push_stop_reply (struct stop_reply *); -static void discard_pending_stop_replies_in_queue (struct remote_state *); -static int peek_stop_reply (ptid_t ptid); - -struct threads_listing_context; -static void remove_new_fork_children (struct threads_listing_context *); static void remote_async_inferior_event_handler (gdb_client_data); @@ -513,16 +762,10 @@ static int remote_read_description_p (struct target_ops *target); static void remote_console_output (char *msg); -static void remote_btrace_reset (void); - -static void remote_btrace_maybe_reopen (void); - -static int stop_reply_queue_length (void); +static void remote_btrace_reset (remote_state *rs); static void remote_unpush_and_throw (void); -static struct remote_state *get_remote_state (void); - /* For "remote". */ static struct cmd_list_element *remote_cmdlist; @@ -556,11 +799,6 @@ struct vCont_action_support static int use_range_stepping = 1; -#define OPAQUETHREADBYTES 8 - -/* a 64 bit opaque identifier */ -typedef unsigned char threadref[OPAQUETHREADBYTES]; - /* About this many threadisds fit in a packet. */ #define MAXTHREADLISTRESULTS 32 @@ -783,6 +1021,28 @@ public: /* data */ file descriptor at a time. */ struct readahead_cache readahead_cache; + /* The list of already fetched and acknowledged stop events. This + queue is used for notification Stop, and other notifications + don't need queue for their events, because the notification + events of Stop can't be consumed immediately, so that events + should be queued first, and be consumed by remote_wait_{ns,as} + one per time. Other notifications can consume their events + immediately, so queue is not needed for them. */ + QUEUE (stop_reply_p) *stop_reply_queue; + + /* Asynchronous signal handle registered as event loop source for + when we have pending events ready to be passed to the core. */ + struct async_event_handler *remote_async_inferior_event_token = nullptr; + + /* FIXME: cagney/1999-09-23: Even though getpkt was called with + ``forever'' still use the normal timeout mechanism. This is + currently used by the ASYNC code to guarentee that target reads + during the initial connect always time-out. Once getpkt has been + modified to return a timeout indication and, in turn + remote_wait()/wait_for_inferior() have gained a timeout parameter + this can go away. */ + int wait_forever_enabled_p = 1; + private: /* Mapping of remote protocol data for each gdbarch. Usually there is only one entry here, though we may see more with stubs that @@ -834,24 +1094,16 @@ remote_state::remote_state () whenever a larger buffer is needed. */ this->buf_size = 400; this->buf = (char *) xmalloc (this->buf_size); + + this->stop_reply_queue = QUEUE_alloc (stop_reply_p, stop_reply_xfree); } remote_state::~remote_state () { xfree (this->last_pass_packet); xfree (this->last_program_signals_packet); -} -/* This data could be associated with a target, but we do not always - have access to the current target when we need it, so for now it is - static. This will be fine for as long as only one target is in use - at a time. */ -static struct remote_state *remote_state; - -static struct remote_state * -get_remote_state_raw (void) -{ - return remote_state; + QUEUE_free (stop_reply_p, this->stop_reply_queue); } /* Utility: generate error from an incoming stub packet. */ @@ -875,8 +1127,8 @@ trace_error (char *buf) /* Utility: wait for reply from stub, while accepting "O" packets. */ -static char * -remote_get_noisy_reply () +char * +remote_target::remote_get_noisy_reply () { struct remote_state *rs = get_remote_state (); @@ -978,19 +1230,20 @@ remote_state::get_remote_arch_state (struct gdbarch *gdbarch) /* Fetch the global remote target state. */ -static struct remote_state * -get_remote_state (void) +remote_state * +remote_target::get_remote_state () { - struct remote_state *rs = get_remote_state_raw (); + if (m_remote_state == nullptr) + m_remote_state.reset (new remote_state ()); /* Make sure that the remote architecture state has been initialized, because doing so might reallocate rs->buf. Any function which calls getpkt also needs to be mindful of changes to rs->buf, but this call limits the number of places which run into trouble. */ - rs->get_remote_arch_state (target_gdbarch ()); + m_remote_state->get_remote_arch_state (target_gdbarch ()); - return rs; + return m_remote_state.get (); } /* Cleanup routine for the remote module's pspace data. */ @@ -1165,11 +1418,21 @@ remote_arch_state::remote_arch_state (struct gdbarch *gdbarch) this->remote_packet_size = (this->sizeof_g_packet * 2 + 32); } +/* Get a pointer to the current remote target. If not connected to a + remote target, return NULL. */ + +static remote_target * +get_current_remote_target () +{ + target_ops *proc_target = find_target_at (process_stratum); + return dynamic_cast<remote_target *> (proc_target); +} + /* Return the current allowed size of a remote packet. This is inferred from the current architecture, and should be used to limit the length of outgoing packets. */ -static long -get_remote_packet_size (void) +long +remote_target::get_remote_packet_size () { struct remote_state *rs = get_remote_state (); remote_arch_state *rsa = rs->get_remote_arch_state (target_gdbarch ()); @@ -1215,15 +1478,6 @@ static remote_target remote_ops; static extended_remote_target extended_remote_ops; -/* FIXME: cagney/1999-09-23: Even though getpkt was called with - ``forever'' still use the normal timeout mechanism. This is - currently used by the ASYNC code to guarentee that target reads - during the initial connect always time-out. Once getpkt has been - modified to return a timeout indication and, in turn - remote_wait()/wait_for_inferior() have gained a timeout parameter - this can go away. */ -static int wait_forever_enabled_p = 1; - /* Allow the user to specify what sequence to send to the remote when he requests a program interruption: Although ^C is usually what remote systems expect (this is the default, here), it is @@ -1349,8 +1603,8 @@ get_fixed_memory_packet_size (struct memory_packet_config *config) /* Compute the current size of a read/write packet. Since this makes use of ``actual_register_packet_size'' the computation is dynamic. */ -static long -get_memory_packet_size (struct memory_packet_config *config) +long +remote_target::get_memory_packet_size (struct memory_packet_config *config) { struct remote_state *rs = get_remote_state (); remote_arch_state *rsa = rs->get_remote_arch_state (target_gdbarch ()); @@ -1443,11 +1697,11 @@ show_memory_packet_size (struct memory_packet_config *config) get_fixed_memory_packet_size (config)); else { - struct remote_state *rs = get_remote_state (); + remote_target *remote = get_current_remote_target (); - if (rs->remote_desc != NULL) + if (remote != NULL) printf_filtered (_("Packets are limited to %ld bytes.\n"), - get_memory_packet_size (config)); + remote->get_memory_packet_size (config)); else puts_filtered ("The actual limit will be further reduced " "dependent on the target.\n"); @@ -1471,8 +1725,8 @@ show_memory_write_packet_size (const char *args, int from_tty) show_memory_packet_size (&memory_write_packet_config); } -static long -get_memory_write_packet_size (void) +long +remote_target::get_memory_write_packet_size () { return get_memory_packet_size (&memory_write_packet_config); } @@ -1494,8 +1748,8 @@ show_memory_read_packet_size (const char *args, int from_tty) show_memory_packet_size (&memory_read_packet_config); } -static long -get_memory_read_packet_size (void) +long +remote_target::get_memory_read_packet_size () { long size = get_memory_packet_size (&memory_read_packet_config); @@ -1508,16 +1762,6 @@ get_memory_read_packet_size (void) } \f -/* Generic configuration support for packets the stub optionally - supports. Allows the user to specify the use of the packet as well - as allowing GDB to auto-detect support in the remote stub. */ - -enum packet_support - { - PACKET_SUPPORT_UNKNOWN = 0, - PACKET_ENABLE, - PACKET_DISABLE - }; struct packet_config { @@ -1535,16 +1779,6 @@ struct packet_config enum packet_support support; }; -/* Analyze a packet's return value and update the packet config - accordingly. */ - -enum packet_result -{ - PACKET_ERROR, - PACKET_OK, - PACKET_UNKNOWN -}; - static enum packet_support packet_config_support (struct packet_config *config); static enum packet_support packet_support (int packet); @@ -2039,12 +2273,6 @@ remote_target::remove_exec_catchpoint (int pid) } \f -/* Asynchronous signal handle registered as event loop source for - when we have pending events ready to be passed to the core. */ - -static struct async_event_handler *remote_async_inferior_event_token; - -\f static ptid_t magic_null_ptid; static ptid_t not_sent_ptid; @@ -2053,8 +2281,8 @@ static ptid_t any_thread_ptid; /* Find out if the stub attached to PID (and hence GDB should offer to detach instead of killing it when bailing out). */ -static int -remote_query_attached (int pid) +int +remote_target::remote_query_attached (int pid) { struct remote_state *rs = get_remote_state (); size_t size = get_remote_packet_size (); @@ -2098,9 +2326,9 @@ remote_query_attached (int pid) attempt to open this inferior's executable as the main executable if no main executable is open already. */ -static struct inferior * -remote_add_inferior (int fake_pid_p, int pid, int attached, - int try_open_exec) +inferior * +remote_target::remote_add_inferior (int fake_pid_p, int pid, int attached, + int try_open_exec) { struct inferior *inf; @@ -2147,8 +2375,8 @@ static remote_thread_info *get_remote_thread_info (thread_info *thread); /* Add thread PTID to GDB's thread list. Tag it as executing/running according to RUNNING. */ -static void -remote_add_thread (ptid_t ptid, int running, int executing) +void +remote_target::remote_add_thread (ptid_t ptid, bool running, bool executing) { struct remote_state *rs = get_remote_state (); struct thread_info *thread; @@ -2176,8 +2404,8 @@ remote_add_thread (ptid_t ptid, int running, int executing) GDB's inferior list as well. EXECUTING indicates whether the thread is (internally) executing or stopped. */ -static void -remote_notice_new_inferior (ptid_t currthread, int executing) +void +remote_target::remote_notice_new_inferior (ptid_t currthread, int executing) { /* In non-stop mode, we assume new found threads are (externally) running until proven otherwise with a stop reply. In all-stop, @@ -2473,8 +2701,8 @@ remote_target::program_signals (int numsigs, unsigned char *signals) MINUS_ONE_PTID, set the thread to -1, so the stub returns the thread. If GEN is set, set the general thread, if not, then set the step/continue thread. */ -static void -set_thread (ptid_t ptid, int gen) +void +remote_target::set_thread (ptid_t ptid, int gen) { struct remote_state *rs = get_remote_state (); ptid_t state = gen ? rs->general_thread : rs->continue_thread; @@ -2502,14 +2730,14 @@ set_thread (ptid_t ptid, int gen) rs->continue_thread = ptid; } -static void -set_general_thread (ptid_t ptid) +void +remote_target::set_general_thread (ptid_t ptid) { set_thread (ptid, 1); } -static void -set_continue_thread (ptid_t ptid) +void +remote_target::set_continue_thread (ptid_t ptid) { set_thread (ptid, 0); } @@ -2523,8 +2751,8 @@ set_continue_thread (ptid_t ptid) general operations is the process the selected general thread belongs to. */ -static void -set_general_process (void) +void +remote_target::set_general_process () { struct remote_state *rs = get_remote_state (); @@ -2666,38 +2894,10 @@ static int threadmatch (threadref *dest, threadref *src); static char *pack_threadinfo_request (char *pkt, int mode, threadref *id); -static int remote_unpack_thread_info_response (char *pkt, - threadref *expectedref, - struct gdb_ext_thread_info - *info); - - -static int remote_get_threadinfo (threadref *threadid, - int fieldset, /*TAG mask */ - struct gdb_ext_thread_info *info); - static char *pack_threadlist_request (char *pkt, int startflag, int threadcount, threadref *nextthread); -static int parse_threadlist_response (char *pkt, - int result_limit, - threadref *original_echo, - threadref *resultlist, - int *doneflag); - -static int remote_get_threadlist (int startflag, - threadref *nextthread, - int result_limit, - int *done, - int *result_count, - threadref *threadlist); - -typedef int (*rmt_thread_action) (threadref *ref, void *context); - -static int remote_threadlist_iterator (rmt_thread_action stepfunction, - void *context, int looplimit); - static int remote_newthread_step (threadref *ref, void *context); @@ -2705,8 +2905,8 @@ static int remote_newthread_step (threadref *ref, void *context); buffer we're allowed to write to. Returns BUF+CHARACTERS_WRITTEN. */ -static char * -write_ptid (char *buf, const char *endbuf, ptid_t ptid) +char * +remote_target::write_ptid (char *buf, const char *endbuf, ptid_t ptid) { int pid, tid; struct remote_state *rs = get_remote_state (); @@ -3003,9 +3203,10 @@ pack_threadinfo_request (char *pkt, int mode, threadref *id) #define TAG_MOREDISPLAY 16 /* Whatever the kernel wants to say about the process. */ -static int -remote_unpack_thread_info_response (char *pkt, threadref *expectedref, - struct gdb_ext_thread_info *info) +int +remote_target::remote_unpack_thread_info_response (char *pkt, + threadref *expectedref, + gdb_ext_thread_info *info) { struct remote_state *rs = get_remote_state (); int mask, length; @@ -3096,9 +3297,10 @@ remote_unpack_thread_info_response (char *pkt, threadref *expectedref, return retval; } -static int -remote_get_threadinfo (threadref *threadid, int fieldset, /* TAG mask */ - struct gdb_ext_thread_info *info) +int +remote_target::remote_get_threadinfo (threadref *threadid, + int fieldset, + gdb_ext_thread_info *info) { struct remote_state *rs = get_remote_state (); int result; @@ -3132,10 +3334,11 @@ pack_threadlist_request (char *pkt, int startflag, int threadcount, /* Encoding: 'q':8,'M':8,count:16,done:8,argthreadid:64,(threadid:64)* */ -static int -parse_threadlist_response (char *pkt, int result_limit, - threadref *original_echo, threadref *resultlist, - int *doneflag) +int +remote_target::parse_threadlist_response (char *pkt, int result_limit, + threadref *original_echo, + threadref *resultlist, + int *doneflag) { struct remote_state *rs = get_remote_state (); char *limit; @@ -3163,9 +3366,10 @@ parse_threadlist_response (char *pkt, int result_limit, /* Fetch the next batch of threads from the remote. Returns -1 if the qL packet is not supported, 0 on error and 1 on success. */ -static int -remote_get_threadlist (int startflag, threadref *nextthread, int result_limit, - int *done, int *result_count, threadref *threadlist) +int +remote_target::remote_get_threadlist (int startflag, threadref *nextthread, + int result_limit, int *done, int *result_count, + threadref *threadlist) { struct remote_state *rs = get_remote_state (); int result = 1; @@ -3225,9 +3429,9 @@ remote_get_threadlist (int startflag, threadref *nextthread, int result_limit, STEPFUNCTION returns false. If the packet is not supported, returns -1. */ -static int -remote_threadlist_iterator (rmt_thread_action stepfunction, void *context, - int looplimit) +int +remote_target::remote_threadlist_iterator (rmt_thread_action stepfunction, + void *context, int looplimit) { struct remote_state *rs = get_remote_state (); int done, i, result_count; @@ -3358,8 +3562,8 @@ remote_newthread_step (threadref *ref, void *data) #define CRAZY_MAX_THREADS 1000 -static ptid_t -remote_current_thread (ptid_t oldpid) +ptid_t +remote_target::remote_current_thread (ptid_t oldpid) { struct remote_state *rs = get_remote_state (); @@ -3383,9 +3587,8 @@ remote_current_thread (ptid_t oldpid) /* List remote threads using the deprecated qL packet. */ -static int -remote_get_threads_with_ql (struct target_ops *ops, - struct threads_listing_context *context) +int +remote_target::remote_get_threads_with_ql (threads_listing_context *context) { if (remote_threadlist_iterator (remote_newthread_step, context, CRAZY_MAX_THREADS) >= 0) @@ -3466,15 +3669,14 @@ const struct gdb_xml_element threads_elements[] = { /* List remote threads using qXfer:threads:read. */ -static int -remote_get_threads_with_qxfer (struct target_ops *ops, - struct threads_listing_context *context) +int +remote_target::remote_get_threads_with_qxfer (threads_listing_context *context) { #if defined(HAVE_LIBEXPAT) if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE) { gdb::optional<gdb::char_vector> xml - = target_read_stralloc (ops, TARGET_OBJECT_THREADS, NULL); + = target_read_stralloc (this, TARGET_OBJECT_THREADS, NULL); if (xml && (*xml)[0] != '\0') { @@ -3491,9 +3693,8 @@ remote_get_threads_with_qxfer (struct target_ops *ops, /* List remote threads using qfThreadInfo/qsThreadInfo. */ -static int -remote_get_threads_with_qthreadinfo (struct target_ops *ops, - struct threads_listing_context *context) +int +remote_target::remote_get_threads_with_qthreadinfo (threads_listing_context *context) { struct remote_state *rs = get_remote_state (); @@ -3542,9 +3743,9 @@ remote_target::update_thread_list () /* We have a few different mechanisms to fetch the thread list. Try them all, starting with the most preferred one first, falling back to older methods. */ - if (remote_get_threads_with_qxfer (this, &context) - || remote_get_threads_with_qthreadinfo (this, &context) - || remote_get_threads_with_ql (this, &context)) + if (remote_get_threads_with_qxfer (&context) + || remote_get_threads_with_qthreadinfo (&context) + || remote_get_threads_with_ql (&context)) { struct thread_info *tp, *tmp; @@ -3776,8 +3977,8 @@ remote_target::get_ada_task_ptid (long lwp, long thread) /* Restart the remote side; this is an extended protocol operation. */ -static void -extended_remote_restart (void) +void +remote_target::extended_remote_restart () { struct remote_state *rs = get_remote_state (); @@ -3794,17 +3995,9 @@ extended_remote_restart (void) void remote_target::close () { - struct remote_state *rs = get_remote_state (); - - if (rs->remote_desc == NULL) - return; /* already closed */ - /* Make sure we leave stdin registered in the event loop. */ terminal_ours (); - serial_close (rs->remote_desc); - rs->remote_desc = NULL; - /* We don't have a connection to the remote stub anymore. Get rid of all the inferiors and their threads we were controlling. Reset inferior_ptid to null_ptid first, as otherwise has_stack_frame @@ -3812,22 +4005,34 @@ remote_target::close () inferior_ptid = null_ptid; discard_all_inferiors (); - /* We are closing the remote target, so we should discard + trace_reset_local_state (); + + delete this; +} + +remote_target::~remote_target () +{ + struct remote_state *rs = get_remote_state (); + + /* Check for NULL because we may get here with a partially + constructed target/connection. */ + if (rs->remote_desc != nullptr) + serial_close (rs->remote_desc); + + /* We are destroying the remote target, so we should discard everything of this target. */ - discard_pending_stop_replies_in_queue (rs); + discard_pending_stop_replies_in_queue (); - if (remote_async_inferior_event_token) - delete_async_event_handler (&remote_async_inferior_event_token); + if (rs->remote_async_inferior_event_token) + delete_async_event_handler (&rs->remote_async_inferior_event_token); remote_notif_state_xfree (rs->notif_state); - - trace_reset_local_state (); } /* Query the remote side for the text, data and bss offsets. */ -static void -get_offsets (void) +void +remote_target::get_offsets () { struct remote_state *rs = get_remote_state (); char *buf; @@ -3984,8 +4189,9 @@ get_offsets (void) } /* Send interrupt_sequence to remote target. */ -static void -send_interrupt_sequence (void) + +void +remote_target::send_interrupt_sequence () { struct remote_state *rs = get_remote_state (); @@ -4048,8 +4254,8 @@ stop_reply_extract_thread (char *stop_reply) ask the remote which is the current thread with qC. The former method avoids a roundtrip. */ -static ptid_t -get_current_thread (char *wait_status) +ptid_t +remote_target::get_current_thread (char *wait_status) { ptid_t ptid = null_ptid; @@ -4075,8 +4281,8 @@ get_current_thread (char *wait_status) qC query, we infer the current thread from that stop reply, passed in in WAIT_STATUS, which may be NULL. */ -static void -add_current_inferior_and_thread (char *wait_status) +void +remote_target::add_current_inferior_and_thread (char *wait_status) { struct remote_state *rs = get_remote_state (); int fake_pid_p = 0; @@ -4139,8 +4345,8 @@ print_one_stopped_thread (struct thread_info *thread) on initial connection. We mark these threads as stopped and print their current frame before giving the user the prompt. */ -static void -process_initial_stop_replies (int from_tty) +void +remote_target::process_initial_stop_replies (int from_tty) { int pending_stop_replies = stop_reply_queue_length (); struct inferior *inf; @@ -4546,7 +4752,7 @@ remote_target::start_remote (int from_tty, int extended_p) /* remote_notif_get_pending_replies acks this one, and gets the rest out. */ rs->notif_state->pending_event[notif_client_stop.id] - = remote_notif_parse (notif, rs->buf); + = remote_notif_parse (this, notif, rs->buf); remote_notif_get_pending_events (notif); } @@ -4658,8 +4864,8 @@ init_all_packet_configs (void) /* Symbol look-up. */ -static void -remote_check_symbols (void) +void +remote_target::remote_check_symbols () { char *msg, *reply, *tmp; int end; @@ -4796,8 +5002,8 @@ struct protocol_feature not reported). The third argument may be NULL; if not NULL, it is a NUL-terminated string taken from the packet following this feature's name and an equals sign. */ - void (*func) (const struct protocol_feature *, enum packet_support, - const char *); + void (*func) (remote_target *remote, const struct protocol_feature *, + enum packet_support, const char *); /* The corresponding packet for this feature. Only used if FUNC is remote_supported_packet. */ @@ -4805,7 +5011,8 @@ struct protocol_feature }; static void -remote_supported_packet (const struct protocol_feature *feature, +remote_supported_packet (remote_target *remote, + const struct protocol_feature *feature, enum packet_support support, const char *argument) { @@ -4819,9 +5026,9 @@ remote_supported_packet (const struct protocol_feature *feature, remote_protocol_packets[feature->packet].support = support; } -static void -remote_packet_size (const struct protocol_feature *feature, - enum packet_support support, const char *value) +void +remote_target::remote_packet_size (const protocol_feature *feature, + enum packet_support support, const char *value) { struct remote_state *rs = get_remote_state (); @@ -4851,6 +5058,13 @@ remote_packet_size (const struct protocol_feature *feature, rs->explicit_packet_size = packet_size; } +void +remote_packet_size (remote_target *remote, const protocol_feature *feature, + enum packet_support support, const char *value) +{ + remote->remote_packet_size (feature, support, value); +} + static const struct protocol_feature remote_protocol_features[] = { { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 }, { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet, @@ -5002,8 +5216,8 @@ remote_query_supported_append (std::string *msg, const char *append) msg->append (append); } -static void -remote_query_supported (void) +void +remote_target::remote_query_supported () { struct remote_state *rs = get_remote_state (); char *next; @@ -5142,7 +5356,7 @@ remote_query_supported (void) seen[i] = 1; feature = &remote_protocol_features[i]; - feature->func (feature, is_supported, value); + feature->func (this, feature, is_supported, value); break; } } @@ -5164,7 +5378,7 @@ remote_query_supported (void) const struct protocol_feature *feature; feature = &remote_protocol_features[i]; - feature->func (feature, feature->default_support, NULL); + feature->func (this, feature, feature->default_support, NULL); } } @@ -5191,8 +5405,8 @@ remote_query_supported (void) is probably wedged --- offer to quit/disconnect. */ -static void -remote_serial_quit_handler (void) +void +remote_target::remote_serial_quit_handler () { struct remote_state *rs = get_remote_state (); @@ -5220,6 +5434,16 @@ remote_serial_quit_handler (void) } } +/* The remote_target that is current while the quit handler is + overridden with remote_serial_quit_handler. */ +static remote_target *curr_quit_handler_target; + +static void +remote_serial_quit_handler () +{ + curr_quit_handler_target->remote_serial_quit_handler (); +} + /* Remove any of the remote.c targets from target stack. Upper targets depend on it so remove them first. */ @@ -5239,21 +5463,17 @@ remote_unpush_and_throw (void) void remote_target::open_1 (const char *name, int from_tty, int extended_p) { - struct remote_state *rs = get_remote_state (); + remote_target *curr_remote = get_current_remote_target (); if (name == 0) error (_("To open a remote debug connection, you need to specify what\n" "serial device is attached to the remote system\n" "(e.g. /dev/ttyS0, /dev/ttya, COM1, etc.).")); - /* See FIXME above. */ - if (!target_async_permitted) - wait_forever_enabled_p = 1; - /* If we're connected to a running target, target_preopen will kill it. Ask this question first, before target_preopen has a chance to kill anything. */ - if (rs->remote_desc != NULL && !have_inferiors ()) + if (curr_remote != NULL && !have_inferiors ()) { if (from_tty && !query (_("Already connected to a remote target. Disconnect? "))) @@ -5263,19 +5483,20 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p) /* Here the possibly existing remote target gets unpushed. */ target_preopen (from_tty); - /* Make sure we send the passed signals list the next time we resume. */ - xfree (rs->last_pass_packet); - rs->last_pass_packet = NULL; - - /* Make sure we send the program signals list the next time we - resume. */ - xfree (rs->last_program_signals_packet); - rs->last_program_signals_packet = NULL; - remote_fileio_reset (); reopen_exec_file (); reread_symbols (); + remote_target *remote + = (extended_p ? new extended_remote_target () : new remote_target ()); + target_ops_up target_holder (remote); + + remote_state *rs = remote->get_remote_state (); + + /* See FIXME above. */ + if (!target_async_permitted) + rs->wait_forever_enabled_p = 1; + rs->remote_desc = remote_serial_open (name); if (!rs->remote_desc) perror_with_name (name); @@ -5308,15 +5529,16 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p) puts_filtered ("\n"); } - remote_target *target - = extended_p ? &extended_remote_ops : &remote_ops; - push_target (target); /* Switch to using remote target now. */ + /* Switch to using the remote target now. */ + push_target (remote); + /* The target stack owns the target now. */ + target_holder.release (); /* Register extra event sources in the event loop. */ - remote_async_inferior_event_token + rs->remote_async_inferior_event_token = create_async_event_handler (remote_async_inferior_event_handler, - NULL); - rs->notif_state = remote_notif_state_allocate (); + remote); + rs->notif_state = remote_notif_state_allocate (remote); /* Reset the target state; these things will be queried either by remote_query_supported or as they are needed. */ @@ -5350,7 +5572,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p) around this. Eventually a mechanism that allows wait_for_inferior() to expect/get timeouts will be implemented. */ - wait_forever_enabled_p = 0; + rs->wait_forever_enabled_p = 0; } /* First delete any symbols previously loaded from shared libraries. */ @@ -5378,31 +5600,29 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p) TRY { - target->start_remote (from_tty, extended_p); + remote->start_remote (from_tty, extended_p); } CATCH (ex, RETURN_MASK_ALL) { /* Pop the partially set up target - unless something else did already before throwing the exception. */ - if (rs->remote_desc != NULL) + if (ex.error != TARGET_CLOSE_ERROR) remote_unpush_target (); - if (target_async_permitted) - wait_forever_enabled_p = 1; throw_exception (ex); } END_CATCH } - remote_btrace_reset (); + remote_btrace_reset (rs); if (target_async_permitted) - wait_forever_enabled_p = 1; + rs->wait_forever_enabled_p = 1; } /* Detach the specified process. */ -static void -remote_detach_pid (int pid) +void +remote_target::remote_detach_pid (int pid) { struct remote_state *rs = get_remote_state (); @@ -5428,8 +5648,8 @@ remote_detach_pid (int pid) any breakpoints in the target program or it'll die when it hits one. */ -static void -remote_detach_1 (int from_tty, inferior *inf) +void +remote_target::remote_detach_1 (int from_tty, inferior *inf) { int pid = ptid_get_pid (inferior_ptid); struct remote_state *rs = get_remote_state (); @@ -5659,7 +5879,7 @@ extended_remote_target::attach (const char *args, int from_tty) if (target_can_async_p ()) { struct notif_event *reply - = remote_notif_parse (¬if_client_stop, wait_status); + = remote_notif_parse (this, ¬if_client_stop, wait_status); push_stop_reply ((struct stop_reply *) reply); @@ -5697,9 +5917,10 @@ extended_remote_target::post_attach (int pid) /* Check for the availability of vCont. This function should also check the response. */ -static void -remote_vcont_probe (struct remote_state *rs) +void +remote_target::remote_vcont_probe () { + remote_state *rs = get_remote_state (); char *buf; strcpy (rs->buf, "vCont?"); @@ -5757,9 +5978,9 @@ remote_vcont_probe (struct remote_state *rs) the thread to be stepped and/or signalled is given in the global INFERIOR_PTID. */ -static char * -append_resumption (char *p, char *endp, - ptid_t ptid, int step, enum gdb_signal siggnal) +char * +remote_target::append_resumption (char *p, char *endp, + ptid_t ptid, int step, gdb_signal siggnal) { struct remote_state *rs = get_remote_state (); @@ -5844,8 +6065,9 @@ resume_clear_thread_private_info (struct thread_info *thread) /* Append a vCont continue-with-signal action for threads that have a non-zero stop signal. */ -static char * -append_pending_thread_resumptions (char *p, char *endp, ptid_t ptid) +char * +remote_target::append_pending_thread_resumptions (char *p, char *endp, + ptid_t ptid) { struct thread_info *thread; @@ -5866,9 +6088,9 @@ append_pending_thread_resumptions (char *p, char *endp, ptid_t ptid) /* Set the target running, using the packets that use Hc (c/s/C/S). */ -static void -remote_resume_with_hc (struct target_ops *ops, - ptid_t ptid, int step, enum gdb_signal siggnal) +void +remote_target::remote_resume_with_hc (ptid_t ptid, int step, + gdb_signal siggnal) { struct remote_state *rs = get_remote_state (); struct thread_info *thread; @@ -5888,7 +6110,7 @@ remote_resume_with_hc (struct target_ops *ops, resume_clear_thread_private_info (thread); buf = rs->buf; - if (execution_direction == EXEC_REVERSE) + if (::execution_direction == EXEC_REVERSE) { /* We don't pass signals to the target in reverse exec mode. */ if (info_verbose && siggnal != GDB_SIGNAL_0) @@ -5925,19 +6147,20 @@ remote_resume_with_hc (struct target_ops *ops, This function issues a strict subset of all possible vCont commands at the moment. */ -static int -remote_resume_with_vcont (ptid_t ptid, int step, enum gdb_signal siggnal) +int +remote_target::remote_resume_with_vcont (ptid_t ptid, int step, + enum gdb_signal siggnal) { struct remote_state *rs = get_remote_state (); char *p; char *endp; /* No reverse execution actions defined for vCont. */ - if (execution_direction == EXEC_REVERSE) + if (::execution_direction == EXEC_REVERSE) return 0; if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN) - remote_vcont_probe (rs); + remote_vcont_probe (); if (packet_support (PACKET_vCont) == PACKET_DISABLE) return 0; @@ -6041,7 +6264,7 @@ remote_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal) /* Prefer vCont, and fallback to s/c/S/C, which use Hc. */ if (!remote_resume_with_vcont (ptid, step, siggnal)) - remote_resume_with_hc (this, ptid, step, siggnal); + remote_resume_with_hc (ptid, step, siggnal); /* We are about to start executing the inferior, let's register it with the event loop. NOTE: this is the one place where all the @@ -6064,8 +6287,6 @@ remote_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal) rs->waiting_for_stop_reply = 1; } -static void check_pending_events_prevent_wildcard_vcont - (int *may_global_wildcard_vcont); static int is_pending_fork_parent_thread (struct thread_info *thread); /* Private per-inferior info for target remote processes. */ @@ -6094,7 +6315,8 @@ get_remote_inferior (inferior *inf) class vcont_builder { public: - vcont_builder () + explicit vcont_builder (remote_target *remote) + : m_remote (remote) {} void restart (); @@ -6102,6 +6324,9 @@ public: void push_action (ptid_t ptid, bool step, gdb_signal siggnal); private: + /* The remote target. */ + remote_target *m_remote; + /* Pointer to the first action. P points here if no action has been appended yet. */ char *m_first_action; @@ -6118,10 +6343,10 @@ private: void vcont_builder::restart () { - struct remote_state *rs = get_remote_state (); + struct remote_state *rs = m_remote->get_remote_state (); m_p = rs->buf; - m_endp = rs->buf + get_remote_packet_size (); + m_endp = rs->buf + m_remote->get_remote_packet_size (); m_p += xsnprintf (m_p, m_endp - m_p, "vCont"); m_first_action = m_p; } @@ -6137,9 +6362,9 @@ vcont_builder::flush () if (m_p == m_first_action) return; - rs = get_remote_state (); - putpkt (rs->buf); - getpkt (&rs->buf, &rs->buf_size, 0); + rs = m_remote->get_remote_state (); + m_remote->putpkt (rs->buf); + m_remote->getpkt (&rs->buf, &rs->buf_size, 0); if (strcmp (rs->buf, "OK") != 0) error (_("Unexpected vCont reply in non-stop mode: %s"), rs->buf); } @@ -6160,8 +6385,8 @@ vcont_builder::push_action (ptid_t ptid, bool step, gdb_signal siggnal) { char buf[MAX_ACTION_SIZE + 1]; - char *endp = append_resumption (buf, buf + sizeof (buf), - ptid, step, siggnal); + char *endp = m_remote->append_resumption (buf, buf + sizeof (buf), + ptid, step, siggnal); /* Check whether this new action would fit in the vCont packet along with previous actions. If not, send what we've got so far and @@ -6287,7 +6512,7 @@ remote_target::commit_resume () we end up with too many actions for a single packet vcont_builder flushes the current vCont packet to the remote side and starts a new one. */ - struct vcont_builder vcont_builder; + struct vcont_builder vcont_builder (this); vcont_builder.restart (); /* Threads first. */ @@ -6361,15 +6586,15 @@ remote_target::commit_resume () thread, all threads of a remote process, or all threads of all processes. */ -static void -remote_stop_ns (ptid_t ptid) +void +remote_target::remote_stop_ns (ptid_t ptid) { struct remote_state *rs = get_remote_state (); char *p = rs->buf; char *endp = rs->buf + get_remote_packet_size (); if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN) - remote_vcont_probe (rs); + remote_vcont_probe (); if (!rs->supports_vCont.t) error (_("Remote server does not support stopping threads")); @@ -6412,8 +6637,8 @@ remote_stop_ns (ptid_t ptid) interrupt the remote target. It is undefined which thread of which process reports the interrupt. */ -static void -remote_interrupt_as (void) +void +remote_target::remote_interrupt_as () { struct remote_state *rs = get_remote_state (); @@ -6434,8 +6659,8 @@ remote_interrupt_as (void) reports the interrupt. Throws an error if the packet is not supported by the server. */ -static void -remote_interrupt_ns (void) +void +remote_target::remote_interrupt_ns () { struct remote_state *rs = get_remote_state (); char *p = rs->buf; @@ -6514,8 +6739,8 @@ remote_target::pass_ctrlc () /* Ask the user what to do when an interrupt is received. */ -static void -interrupt_query (void) +void +remote_target::interrupt_query () { struct remote_state *rs = get_remote_state (); @@ -6603,17 +6828,6 @@ typedef struct stop_reply int core; } *stop_reply_p; -DECLARE_QUEUE_P (stop_reply_p); -DEFINE_QUEUE_P (stop_reply_p); -/* The list of already fetched and acknowledged stop events. This - queue is used for notification Stop, and other notifications - don't need queue for their events, because the notification events - of Stop can't be consumed immediately, so that events should be - queued first, and be consumed by remote_wait_{ns,as} one per - time. Other notifications can consume their events immediately, - so queue is not needed for them. */ -static QUEUE (stop_reply_p) *stop_reply_queue; - static void stop_reply_xfree (struct stop_reply *r) { @@ -6622,44 +6836,51 @@ stop_reply_xfree (struct stop_reply *r) /* Return the length of the stop reply queue. */ -static int -stop_reply_queue_length (void) +int +remote_target::stop_reply_queue_length () { - return QUEUE_length (stop_reply_p, stop_reply_queue); + remote_state *rs = get_remote_state (); + return QUEUE_length (stop_reply_p, rs->stop_reply_queue); } -static void -remote_notif_stop_parse (struct notif_client *self, char *buf, +void +remote_notif_stop_parse (remote_target *remote, + struct notif_client *self, char *buf, struct notif_event *event) { - remote_parse_stop_reply (buf, (struct stop_reply *) event); + remote->remote_parse_stop_reply (buf, (struct stop_reply *) event); } static void -remote_notif_stop_ack (struct notif_client *self, char *buf, +remote_notif_stop_ack (remote_target *remote, + struct notif_client *self, char *buf, struct notif_event *event) { struct stop_reply *stop_reply = (struct stop_reply *) event; /* acknowledge */ - putpkt (self->ack_command); + putpkt (remote, self->ack_command); if (stop_reply->ws.kind == TARGET_WAITKIND_IGNORE) + { /* We got an unknown stop reply. */ error (_("Unknown stop reply")); + } - push_stop_reply (stop_reply); + remote->push_stop_reply (stop_reply); } static int -remote_notif_stop_can_get_pending_events (struct notif_client *self) +remote_notif_stop_can_get_pending_events (remote_target *remote, + struct notif_client *self) { /* We can't get pending events in remote_notif_process for notification stop, and we have to do this in remote_wait_ns instead. If we fetch all queued events from stub, remote stub may exit and we have no chance to process them back in remote_wait_ns. */ - mark_async_event_handler (remote_async_inferior_event_token); + remote_state *rs = remote->get_remote_state (); + mark_async_event_handler (rs->remote_async_inferior_event_token); return 0; } @@ -6706,6 +6927,7 @@ struct notif_client notif_client_stop = struct queue_iter_param { + remote_target *remote; void *input; struct stop_reply *output; }; @@ -6778,8 +7000,8 @@ remove_child_of_pending_fork (QUEUE (stop_reply_p) *q, and have not yet called follow_fork, which will set up the host-side data structures for the new process. */ -static void -remove_new_fork_children (struct threads_listing_context *context) +void +remote_target::remove_new_fork_children (threads_listing_context *context) { struct thread_info * thread; int pid = -1; @@ -6800,12 +7022,24 @@ remove_new_fork_children (struct threads_listing_context *context) in process PID and remove those fork child threads from the CONTEXT list as well. */ remote_notif_get_pending_events (notif); + param.remote = this; param.input = context; param.output = NULL; - QUEUE_iterate (stop_reply_p, stop_reply_queue, + QUEUE_iterate (stop_reply_p, get_remote_state ()->stop_reply_queue, remove_child_of_pending_fork, ¶m); } +/* Callback data for + check_pending_event_prevents_wildcard_vcont_callback. */ +struct check_pending_event_prevents_wildcard_vcont_callback_data +{ + /* The remote target. */ + remote_target *remote; + + /* Whether we can do a global wildcard (vCont;c) */ + int *may_global_wildcard_vcont; +}; + /* Check whether EVENT would prevent a global or process wildcard vCont action. */ @@ -6817,7 +7051,7 @@ check_pending_event_prevents_wildcard_vcont_callback void *data) { struct inferior *inf; - int *may_global_wildcard_vcont = (int *) data; + auto *cb_data = (check_pending_event_prevents_wildcard_vcont_callback_data *) data; if (event->ws.kind == TARGET_WAITKIND_NO_RESUMED || event->ws.kind == TARGET_WAITKIND_NO_HISTORY) @@ -6825,14 +7059,14 @@ check_pending_event_prevents_wildcard_vcont_callback if (event->ws.kind == TARGET_WAITKIND_FORKED || event->ws.kind == TARGET_WAITKIND_VFORKED) - *may_global_wildcard_vcont = 0; + *cb_data->may_global_wildcard_vcont = 0; inf = find_inferior_ptid (event->ptid); /* This may be the first time we heard about this process. Regardless, we must not do a global wildcard resume, otherwise we'd resume this process too. */ - *may_global_wildcard_vcont = 0; + *cb_data->may_global_wildcard_vcont = 0; if (inf != NULL) get_remote_inferior (inf)->may_wildcard_vcont = false; @@ -6845,15 +7079,18 @@ check_pending_event_prevents_wildcard_vcont_callback and clear the event inferior's may_wildcard_vcont flag if we can't do a process-wide wildcard resume (vCont;c:pPID.-1). */ -static void -check_pending_events_prevent_wildcard_vcont (int *may_global_wildcard) +void +remote_target::check_pending_events_prevent_wildcard_vcont + (int *may_global_wildcard) { struct notif_client *notif = ¬if_client_stop; + check_pending_event_prevents_wildcard_vcont_callback_data cb_data + {this, may_global_wildcard}; remote_notif_get_pending_events (notif); - QUEUE_iterate (stop_reply_p, stop_reply_queue, + QUEUE_iterate (stop_reply_p, get_remote_state ()->stop_reply_queue, check_pending_event_prevents_wildcard_vcont_callback, - may_global_wildcard); + &cb_data); } /* Remove stop replies in the queue if its pid is equal to the given @@ -6879,8 +7116,8 @@ remove_stop_reply_for_inferior (QUEUE (stop_reply_p) *q, /* Discard all pending stop replies of inferior INF. */ -static void -discard_pending_stop_replies (struct inferior *inf) +void +remote_target::discard_pending_stop_replies (struct inferior *inf) { struct queue_iter_param param; struct stop_reply *reply; @@ -6901,11 +7138,12 @@ discard_pending_stop_replies (struct inferior *inf) rns->pending_event[notif_client_stop.id] = NULL; } + param.remote = this; param.input = inf; param.output = NULL; /* Discard the stop replies we have already pulled with vStopped. */ - QUEUE_iterate (stop_reply_p, stop_reply_queue, + QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, remove_stop_reply_for_inferior, ¶m); } @@ -6932,16 +7170,18 @@ remove_stop_reply_of_remote_state (QUEUE (stop_reply_p) *q, /* Discard the stop replies for RS in stop_reply_queue. */ -static void -discard_pending_stop_replies_in_queue (struct remote_state *rs) +void +remote_target::discard_pending_stop_replies_in_queue () { + remote_state *rs = get_remote_state (); struct queue_iter_param param; + param.remote = this; param.input = rs; param.output = NULL; /* Discard the stop replies we have already pulled with vStopped. */ - QUEUE_iterate (stop_reply_p, stop_reply_queue, + QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, remove_stop_reply_of_remote_state, ¶m); } @@ -6969,15 +7209,16 @@ remote_notif_remove_once_on_match (QUEUE (stop_reply_p) *q, /* Remove the first reply in 'stop_reply_queue' which matches PTID. */ -static struct stop_reply * -remote_notif_remove_queued_reply (ptid_t ptid) +struct stop_reply * +remote_target::remote_notif_remove_queued_reply (ptid_t ptid) { struct queue_iter_param param; + param.remote = this; param.input = &ptid; param.output = NULL; - QUEUE_iterate (stop_reply_p, stop_reply_queue, + QUEUE_iterate (stop_reply_p, get_remote_state ()->stop_reply_queue, remote_notif_remove_once_on_match, ¶m); if (notif_debug) fprintf_unfiltered (gdb_stdlog, @@ -6992,14 +7233,17 @@ remote_notif_remove_queued_reply (ptid_t ptid) found. If there are still queued events left to process, tell the event loop to get back to target_wait soon. */ -static struct stop_reply * -queued_stop_reply (ptid_t ptid) +struct stop_reply * +remote_target::queued_stop_reply (ptid_t ptid) { struct stop_reply *r = remote_notif_remove_queued_reply (ptid); - if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue)) - /* There's still at least an event left. */ - mark_async_event_handler (remote_async_inferior_event_token); + if (!QUEUE_is_empty (stop_reply_p, get_remote_state ()->stop_reply_queue)) + { + remote_state *rs = get_remote_state (); + /* There's still at least an event left. */ + mark_async_event_handler (rs->remote_async_inferior_event_token); + } return r; } @@ -7008,19 +7252,20 @@ queued_stop_reply (ptid_t ptid) know that we now have at least one queued event left to pass to the core side, tell the event loop to get back to target_wait soon. */ -static void -push_stop_reply (struct stop_reply *new_event) +void +remote_target::push_stop_reply (struct stop_reply *new_event) { - QUEUE_enque (stop_reply_p, stop_reply_queue, new_event); + remote_state *rs = get_remote_state (); + QUEUE_enque (stop_reply_p, rs->stop_reply_queue, new_event); if (notif_debug) fprintf_unfiltered (gdb_stdlog, "notif: push 'Stop' %s to queue %d\n", target_pid_to_str (new_event->ptid), QUEUE_length (stop_reply_p, - stop_reply_queue)); + rs->stop_reply_queue)); - mark_async_event_handler (remote_async_inferior_event_token); + mark_async_event_handler (rs->remote_async_inferior_event_token); } static int @@ -7037,10 +7282,11 @@ stop_reply_match_ptid_and_ws (QUEUE (stop_reply_p) *q, /* Returns true if we have a stop reply for PTID. */ -static int -peek_stop_reply (ptid_t ptid) +int +remote_target::peek_stop_reply (ptid_t ptid) { - return !QUEUE_iterate (stop_reply_p, stop_reply_queue, + remote_state *rs = get_remote_state (); + return !QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, stop_reply_match_ptid_and_ws, &ptid); } @@ -7059,8 +7305,8 @@ strprefix (const char *p, const char *pend, const char *prefix) /* Parse the stop reply in BUF. Either the function succeeds, and the result is stored in EVENT, or throws an error. */ -static void -remote_parse_stop_reply (char *buf, struct stop_reply *event) +void +remote_target::remote_parse_stop_reply (char *buf, stop_reply *event) { remote_arch_state *rsa = NULL; ULONGEST addr; @@ -7458,7 +7704,7 @@ Packet: '%s'\n"), */ void -remote_notif_get_pending_events (struct notif_client *nc) +remote_target::remote_notif_get_pending_events (notif_client *nc) { struct remote_state *rs = get_remote_state (); @@ -7470,7 +7716,7 @@ remote_notif_get_pending_events (struct notif_client *nc) nc->name); /* acknowledge */ - nc->ack (nc, rs->buf, rs->notif_state->pending_event[nc->id]); + nc->ack (this, nc, rs->buf, rs->notif_state->pending_event[nc->id]); rs->notif_state->pending_event[nc->id] = NULL; while (1) @@ -7479,7 +7725,7 @@ remote_notif_get_pending_events (struct notif_client *nc) if (strcmp (rs->buf, "OK") == 0) break; else - remote_notif_ack (nc, rs->buf); + remote_notif_ack (this, nc, rs->buf); } } else @@ -7491,13 +7737,22 @@ remote_notif_get_pending_events (struct notif_client *nc) } } +/* Wrapper around remote_target::remote_notif_get_pending_events to + avoid having to export the whole remote_target class. */ + +void +remote_notif_get_pending_events (remote_target *remote, notif_client *nc) +{ + remote->remote_notif_get_pending_events (nc); +} + /* Called when it is decided that STOP_REPLY holds the info of the event that is to be returned to the core. This function always destroys STOP_REPLY. */ -static ptid_t -process_stop_reply (struct stop_reply *stop_reply, - struct target_waitstatus *status) +ptid_t +remote_target::process_stop_reply (struct stop_reply *stop_reply, + struct target_waitstatus *status) { ptid_t ptid; @@ -7546,8 +7801,8 @@ process_stop_reply (struct stop_reply *stop_reply, /* The non-stop mode version of target_wait. */ -static ptid_t -remote_wait_ns (ptid_t ptid, struct target_waitstatus *status, int options) +ptid_t +remote_target::wait_ns (ptid_t ptid, struct target_waitstatus *status, int options) { struct remote_state *rs = get_remote_state (); struct stop_reply *stop_reply; @@ -7605,8 +7860,8 @@ remote_wait_ns (ptid_t ptid, struct target_waitstatus *status, int options) /* Wait until the remote machine stops, then return, storing status in STATUS just as `wait' would. */ -static ptid_t -remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options) +ptid_t +remote_target::wait_as (ptid_t ptid, target_waitstatus *status, int options) { struct remote_state *rs = get_remote_state (); ptid_t event_ptid = null_ptid; @@ -7630,7 +7885,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options) int ret; int is_notif; int forever = ((options & TARGET_WNOHANG) == 0 - && wait_forever_enabled_p); + && rs->wait_forever_enabled_p); if (!rs->waiting_for_stop_reply) { @@ -7678,7 +7933,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options) for a stop reply. See the comments in putpkt_binary. Set waiting_for_stop_reply to 0 temporarily. */ rs->waiting_for_stop_reply = 0; - remote_fileio_request (buf, rs->ctrlc_pending_p); + remote_fileio_request (this, buf, rs->ctrlc_pending_p); rs->ctrlc_pending_p = 0; /* GDB handled the File-I/O request, and the target is running again. Keep waiting for events. */ @@ -7692,7 +7947,8 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options) rs->waiting_for_stop_reply = 0; stop_reply - = (struct stop_reply *) remote_notif_parse (¬if_client_stop, + = (struct stop_reply *) remote_notif_parse (this, + ¬if_client_stop, rs->buf); event_ptid = process_stop_reply (stop_reply, status); @@ -7758,16 +8014,18 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status, int options) ptid_t event_ptid; if (target_is_non_stop_p ()) - event_ptid = remote_wait_ns (ptid, status, options); + event_ptid = wait_ns (ptid, status, options); else - event_ptid = remote_wait_as (ptid, status, options); + event_ptid = wait_as (ptid, status, options); if (target_is_async_p ()) { + remote_state *rs = get_remote_state (); + /* If there are are events left in the queue tell the event loop to return here. */ - if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue)) - mark_async_event_handler (remote_async_inferior_event_token); + if (!QUEUE_is_empty (stop_reply_p, rs->stop_reply_queue)) + mark_async_event_handler (rs->remote_async_inferior_event_token); } return event_ptid; @@ -7775,8 +8033,9 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status, int options) /* Fetch a single register using a 'p' packet. */ -static int -fetch_register_using_p (struct regcache *regcache, struct packet_reg *reg) +int +remote_target::fetch_register_using_p (struct regcache *regcache, + packet_reg *reg) { struct gdbarch *gdbarch = regcache->arch (); struct remote_state *rs = get_remote_state (); @@ -7836,8 +8095,8 @@ fetch_register_using_p (struct regcache *regcache, struct packet_reg *reg) /* Fetch the registers included in the target's 'g' packet. */ -static int -send_g_packet (void) +int +remote_target::send_g_packet () { struct remote_state *rs = get_remote_state (); int buf_len; @@ -7872,8 +8131,8 @@ send_g_packet (void) return buf_len / 2; } -static void -process_g_packet (struct regcache *regcache) +void +remote_target::process_g_packet (struct regcache *regcache) { struct gdbarch *gdbarch = regcache->arch (); struct remote_state *rs = get_remote_state (); @@ -7977,8 +8236,8 @@ process_g_packet (struct regcache *regcache) } } -static void -fetch_registers_using_g (struct regcache *regcache) +void +remote_target::fetch_registers_using_g (struct regcache *regcache) { send_g_packet (); process_g_packet (regcache); @@ -7987,8 +8246,8 @@ fetch_registers_using_g (struct regcache *regcache) /* Make the remote selected traceframe match GDB's selected traceframe. */ -static void -set_remote_traceframe (void) +void +remote_target::set_remote_traceframe () { int newnum; struct remote_state *rs = get_remote_state (); @@ -8084,9 +8343,9 @@ remote_target::prepare_to_store (struct regcache *regcache) /* Helper: Attempt to store REGNUM using the P packet. Return fail IFF packet was not recognized. */ -static int -store_register_using_P (const struct regcache *regcache, - struct packet_reg *reg) +int +remote_target::store_register_using_P (const struct regcache *regcache, + packet_reg *reg) { struct gdbarch *gdbarch = regcache->arch (); struct remote_state *rs = get_remote_state (); @@ -8125,8 +8384,8 @@ store_register_using_P (const struct regcache *regcache, /* Store register REGNUM, or all registers if REGNUM == -1, from the contents of the register cache buffer. FIXME: ignores errors. */ -static void -store_registers_using_G (const struct regcache *regcache) +void +remote_target::store_registers_using_G (const struct regcache *regcache) { struct remote_state *rs = get_remote_state (); remote_arch_state *rsa = rs->get_remote_arch_state (regcache->arch ()); @@ -8286,8 +8545,8 @@ remote_address_masked (CORE_ADDR addr) clean. In cases like this, the user should clear "remote X-packet". */ -static void -check_binary_download (CORE_ADDR addr) +void +remote_target::check_binary_download (CORE_ADDR addr) { struct remote_state *rs = get_remote_state (); @@ -8377,11 +8636,13 @@ align_for_efficient_write (int todo, CORE_ADDR memaddr) -> $m1000,4#?? <- eeeeffffeeeedddd */ -static enum target_xfer_status -remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, - const gdb_byte *myaddr, ULONGEST len_units, - int unit_size, ULONGEST *xfered_len_units, - char packet_format, int use_length) +target_xfer_status +remote_target::remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, + const gdb_byte *myaddr, + ULONGEST len_units, + int unit_size, + ULONGEST *xfered_len_units, + char packet_format, int use_length) { struct remote_state *rs = get_remote_state (); char *p; @@ -8540,9 +8801,10 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, 'enum target_xfer_status' value). Save the number of bytes transferred in *XFERED_LEN. Only transfer a single packet. */ -static enum target_xfer_status -remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, ULONGEST len, - int unit_size, ULONGEST *xfered_len) +target_xfer_status +remote_target::remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, + ULONGEST len, int unit_size, + ULONGEST *xfered_len) { const char *packet_format = NULL; @@ -8583,9 +8845,10 @@ remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, ULONGEST len, See the comment of remote_write_bytes_aux for an example of memory read/write exchange between gdb and the stub. */ -static enum target_xfer_status -remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len_units, - int unit_size, ULONGEST *xfered_len_units) +target_xfer_status +remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, + ULONGEST len_units, + int unit_size, ULONGEST *xfered_len_units) { struct remote_state *rs = get_remote_state (); int buf_size_bytes; /* Max size of packet output buffer. */ @@ -8630,15 +8893,17 @@ remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len_units, For interface/parameters/return description see target.h, to_xfer_partial. */ -static enum target_xfer_status -remote_xfer_live_readonly_partial (struct target_ops *ops, gdb_byte *readbuf, - ULONGEST memaddr, ULONGEST len, - int unit_size, ULONGEST *xfered_len) +target_xfer_status +remote_target::remote_xfer_live_readonly_partial (gdb_byte *readbuf, + ULONGEST memaddr, + ULONGEST len, + int unit_size, + ULONGEST *xfered_len) { struct target_section *secp; struct target_section_table *table; - secp = target_section_by_addr (ops, memaddr); + secp = target_section_by_addr (this, memaddr); if (secp != NULL && (bfd_get_section_flags (secp->the_bfd_section->owner, secp->the_bfd_section) @@ -8647,7 +8912,7 @@ remote_xfer_live_readonly_partial (struct target_ops *ops, gdb_byte *readbuf, struct target_section *p; ULONGEST memend = memaddr + len; - table = target_get_section_table (ops); + table = target_get_section_table (this); for (p = table->sections; p < table->sections_end; p++) { @@ -8682,10 +8947,10 @@ remote_xfer_live_readonly_partial (struct target_ops *ops, gdb_byte *readbuf, first if the requested memory is unavailable in traceframe. Otherwise, fall back to remote_read_bytes_1. */ -static enum target_xfer_status -remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr, - gdb_byte *myaddr, ULONGEST len, int unit_size, - ULONGEST *xfered_len) +target_xfer_status +remote_target::remote_read_bytes (CORE_ADDR memaddr, + gdb_byte *myaddr, ULONGEST len, int unit_size, + ULONGEST *xfered_len) { if (len == 0) return TARGET_XFER_EOF; @@ -8715,7 +8980,7 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr, } /* This goes through the topmost target again. */ - res = remote_xfer_live_readonly_partial (ops, myaddr, memaddr, + res = remote_xfer_live_readonly_partial (myaddr, memaddr, len, unit_size, xfered_len); if (res == TARGET_XFER_OK) return TARGET_XFER_OK; @@ -8746,11 +9011,8 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr, FORMAT and the remaining arguments, then gets the reply. Returns whether the packet was a success, a failure, or unknown. */ -static enum packet_result remote_send_printf (const char *format, ...) - ATTRIBUTE_PRINTF (1, 2); - -static enum packet_result -remote_send_printf (const char *format, ...) +packet_result +remote_target::remote_send_printf (const char *format, ...) { struct remote_state *rs = get_remote_state (); int max_size = get_remote_packet_size (); @@ -8798,10 +9060,10 @@ remote_target::flash_erase (ULONGEST address, LONGEST length) } } -static enum target_xfer_status -remote_flash_write (struct target_ops *ops, ULONGEST address, - ULONGEST length, ULONGEST *xfered_len, - const gdb_byte *data) +target_xfer_status +remote_target::remote_flash_write (ULONGEST address, + ULONGEST length, ULONGEST *xfered_len, + const gdb_byte *data) { scoped_restore restore_timeout = make_scoped_restore (&remote_timeout, remote_flash_timeout); @@ -8860,15 +9122,17 @@ unpush_and_perror (const char *string) sequence, as that would break communication with the remote server. See remote_serial_quit_handler for more detail. */ -static int -readchar (int timeout) +int +remote_target::readchar (int timeout) { int ch; struct remote_state *rs = get_remote_state (); { + scoped_restore restore_quit_target + = make_scoped_restore (&curr_quit_handler_target, this); scoped_restore restore_quit - = make_scoped_restore (&quit_handler, remote_serial_quit_handler); + = make_scoped_restore (&quit_handler, ::remote_serial_quit_handler); rs->got_ctrlc_during_io = 0; @@ -8903,13 +9167,15 @@ readchar (int timeout) communication with the remote server. See remote_serial_quit_handler for more detail. */ -static void -remote_serial_write (const char *str, int len) +void +remote_target::remote_serial_write (const char *str, int len) { struct remote_state *rs = get_remote_state (); + scoped_restore restore_quit_target + = make_scoped_restore (&curr_quit_handler_target, this); scoped_restore restore_quit - = make_scoped_restore (&quit_handler, remote_serial_quit_handler); + = make_scoped_restore (&quit_handler, ::remote_serial_quit_handler); rs->got_ctrlc_during_io = 0; @@ -8947,19 +9213,28 @@ print_packet (const char *buf) } int -putpkt (const char *buf) +remote_target::putpkt (const char *buf) { return putpkt_binary (buf, strlen (buf)); } +/* Wrapper around remote_target::putpkt to avoid exporting + remote_target. */ + +int +putpkt (remote_target *remote, const char *buf) +{ + return remote->putpkt (buf); +} + /* Send a packet to the remote machine, with error checking. The data of the packet is in BUF. The string in BUF can be at most get_remote_packet_size () - 5 to account for the $, # and checksum, and for a possible /0 if we are debugging (remote_debug) and want to print the sent packet as a string. */ -static int -putpkt_binary (const char *buf, int cnt) +int +remote_target::putpkt_binary (const char *buf, int cnt) { struct remote_state *rs = get_remote_state (); int i; @@ -9164,8 +9439,8 @@ putpkt_binary (const char *buf, int cnt) /* Come here after finding the start of a frame when we expected an ack. Do our best to discard the rest of this packet. */ -static void -skip_frame (void) +void +remote_target::skip_frame () { int c; @@ -9206,9 +9481,8 @@ skip_frame (void) trailing NULL) on success. (could be extended to return one of the SERIAL status indications). */ -static long -read_frame (char **buf_p, - long *sizeof_buf) +long +remote_target::read_frame (char **buf_p, long *sizeof_buf) { unsigned char csum; long bc; @@ -9338,10 +9612,9 @@ read_frame (char **buf_p, don't have to change all the calls to getpkt to deal with the return value, because at the moment I don't know what the right thing to do it for those. */ + void -getpkt (char **buf, - long *sizeof_buf, - int forever) +remote_target::getpkt (char **buf, long *sizeof_buf, int forever) { getpkt_sane (buf, sizeof_buf, forever); } @@ -9359,9 +9632,10 @@ getpkt (char **buf, boolean that indicates whether *BUF holds a notification or not (a regular packet). */ -static int -getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever, - int expecting_notif, int *is_notif) +int +remote_target::getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, + int forever, int expecting_notif, + int *is_notif) { struct remote_state *rs = get_remote_state (); int c; @@ -9500,15 +9774,15 @@ getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever, } } -static int -getpkt_sane (char **buf, long *sizeof_buf, int forever) +int +remote_target::getpkt_sane (char **buf, long *sizeof_buf, int forever) { return getpkt_or_notif_sane_1 (buf, sizeof_buf, forever, 0, NULL); } -static int -getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever, - int *is_notif) +int +remote_target::getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever, + int *is_notif) { return getpkt_or_notif_sane_1 (buf, sizeof_buf, forever, 1, is_notif); @@ -9517,22 +9791,22 @@ getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever, /* Check whether EVENT is a fork event for the process specified by the pid passed in DATA, and if it is, kill the fork child. */ -static int -kill_child_of_pending_fork (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - stop_reply_p event, - void *data) +int +remote_kill_child_of_pending_fork (QUEUE (stop_reply_p) *q, + QUEUE_ITER (stop_reply_p) *iter, + stop_reply_p event, + void *data) { struct queue_iter_param *param = (struct queue_iter_param *) data; int parent_pid = *(int *) param->input; if (is_pending_fork_parent (&event->ws, parent_pid, event->ptid)) { - struct remote_state *rs = get_remote_state (); + remote_target *remote = param->remote; int child_pid = ptid_get_pid (event->ws.value.related_pid); int res; - res = remote_vkill (child_pid, rs); + res = remote->remote_vkill (child_pid); if (res != 0) error (_("Can't kill fork child process %d"), child_pid); } @@ -9543,9 +9817,10 @@ kill_child_of_pending_fork (QUEUE (stop_reply_p) *q, /* Kill any new fork children of process PID that haven't been processed by follow_fork. */ -static void -kill_new_fork_children (int pid, struct remote_state *rs) +void +remote_target::kill_new_fork_children (int pid) { + remote_state *rs = get_remote_state (); struct thread_info *thread; struct notif_client *notif = ¬if_client_stop; struct queue_iter_param param; @@ -9558,11 +9833,10 @@ kill_new_fork_children (int pid, struct remote_state *rs) if (is_pending_fork_parent (ws, pid, thread->ptid)) { - struct remote_state *rs = get_remote_state (); int child_pid = ptid_get_pid (ws->value.related_pid); int res; - res = remote_vkill (child_pid, rs); + res = remote_vkill (child_pid); if (res != 0) error (_("Can't kill fork child process %d"), child_pid); } @@ -9571,10 +9845,11 @@ kill_new_fork_children (int pid, struct remote_state *rs) /* Check for any pending fork events (not reported or processed yet) in process PID and kill those fork child threads as well. */ remote_notif_get_pending_events (notif); + param.remote = this; param.input = &pid; param.output = NULL; - QUEUE_iterate (stop_reply_p, stop_reply_queue, - kill_child_of_pending_fork, ¶m); + QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, + remote_kill_child_of_pending_fork, ¶m); } \f @@ -9593,9 +9868,9 @@ remote_target::kill () kill the child task. We need to do this before killing the parent task because if this is a vfork then the parent will be sleeping. */ - kill_new_fork_children (pid, rs); + kill_new_fork_children (pid); - res = remote_vkill (pid, rs); + res = remote_vkill (pid); if (res == 0) { target_mourn_inferior (inferior_ptid); @@ -9625,12 +9900,14 @@ remote_target::kill () /* Send a kill request to the target using the 'vKill' packet. */ -static int -remote_vkill (int pid, struct remote_state *rs) +int +remote_target::remote_vkill (int pid) { if (packet_support (PACKET_vKill) == PACKET_DISABLE) return -1; + remote_state *rs = get_remote_state (); + /* Tell the remote target to detach. */ xsnprintf (rs->buf, get_remote_packet_size (), "vKill;%x", pid); putpkt (rs->buf); @@ -9652,8 +9929,8 @@ remote_vkill (int pid, struct remote_state *rs) /* Send a kill request to the target using the 'k' packet. */ -static void -remote_kill_k (void) +void +remote_target::remote_kill_k () { /* Catch errors so the user can quit from gdb even when we aren't on speaking terms with the remote system. */ @@ -9760,8 +10037,8 @@ extended_remote_target::supports_disable_randomization () return packet_support (PACKET_QDisableRandomization) == PACKET_ENABLE; } -static void -extended_remote_disable_randomization (int val) +void +remote_target::extended_remote_disable_randomization (int val) { struct remote_state *rs = get_remote_state (); char *reply; @@ -9776,8 +10053,8 @@ extended_remote_disable_randomization (int val) error (_("Bogus QDisableRandomization reply from target: %s"), reply); } -static int -extended_remote_run (const std::string &args) +int +remote_target::extended_remote_run (const std::string &args) { struct remote_state *rs = get_remote_state (); int len; @@ -9840,12 +10117,13 @@ extended_remote_run (const std::string &args) or "QEnvironmentUnsetVariable". VALUE is the variable to be sent. */ -static void -send_environment_packet (struct remote_state *rs, - const char *action, - const char *packet, - const char *value) +void +remote_target::send_environment_packet (const char *action, + const char *packet, + const char *value) { + remote_state *rs = get_remote_state (); + /* Convert the environment variable to an hex string, which is the best format to be transmitted over the wire. */ std::string encoded_value = bin2hex ((const gdb_byte *) value, @@ -9863,9 +10141,11 @@ send_environment_packet (struct remote_state *rs, /* Helper function to handle the QEnvironment* packets. */ -static void -extended_remote_environment_support (struct remote_state *rs) +void +remote_target::extended_remote_environment_support () { + remote_state *rs = get_remote_state (); + if (packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE) { putpkt ("QEnvironmentReset"); @@ -9878,23 +10158,24 @@ extended_remote_environment_support (struct remote_state *rs) if (packet_support (PACKET_QEnvironmentHexEncoded) != PACKET_DISABLE) for (const std::string &el : e->user_set_env ()) - send_environment_packet (rs, "set", "QEnvironmentHexEncoded", + send_environment_packet ("set", "QEnvironmentHexEncoded", el.c_str ()); if (packet_support (PACKET_QEnvironmentUnset) != PACKET_DISABLE) for (const std::string &el : e->user_unset_env ()) - send_environment_packet (rs, "unset", "QEnvironmentUnset", el.c_str ()); + send_environment_packet ("unset", "QEnvironmentUnset", el.c_str ()); } /* Helper function to set the current working directory for the inferior in the remote target. */ -static void -extended_remote_set_inferior_cwd (struct remote_state *rs) +void +remote_target::extended_remote_set_inferior_cwd () { if (packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE) { const char *inferior_cwd = get_inferior_cwd (); + remote_state *rs = get_remote_state (); if (inferior_cwd != NULL) { @@ -9964,9 +10245,9 @@ Remote replied unexpectedly while setting startup-with-shell: %s"), rs->buf); } - extended_remote_environment_support (rs); + extended_remote_environment_support (); - extended_remote_set_inferior_cwd (rs); + extended_remote_set_inferior_cwd (); /* Now restart the remote server. */ run_worked = extended_remote_run (args) != -1; @@ -10593,11 +10874,12 @@ the loaded file\n")); into remote target. The number of bytes written to the remote target is returned, or -1 for error. */ -static enum target_xfer_status -remote_write_qxfer (const char *object_name, - const char *annex, const gdb_byte *writebuf, - ULONGEST offset, LONGEST len, ULONGEST *xfered_len, - struct packet_config *packet) +target_xfer_status +remote_target::remote_write_qxfer (const char *object_name, + const char *annex, const gdb_byte *writebuf, + ULONGEST offset, LONGEST len, + ULONGEST *xfered_len, + struct packet_config *packet) { int i, buf_len; ULONGEST n; @@ -10636,12 +10918,13 @@ remote_write_qxfer (const char *object_name, EOF. PACKET is checked and updated to indicate whether the remote target supports this object. */ -static enum target_xfer_status -remote_read_qxfer (const char *object_name, - const char *annex, - gdb_byte *readbuf, ULONGEST offset, LONGEST len, - ULONGEST *xfered_len, - struct packet_config *packet) +target_xfer_status +remote_target::remote_read_qxfer (const char *object_name, + const char *annex, + gdb_byte *readbuf, ULONGEST offset, + LONGEST len, + ULONGEST *xfered_len, + struct packet_config *packet) { struct remote_state *rs = get_remote_state (); LONGEST i, n, packet_len; @@ -10747,7 +11030,7 @@ remote_target::xfer_partial (enum target_object object, return remote_write_bytes (offset, writebuf, len, unit_size, xfered_len); else - return remote_read_bytes (this, offset, readbuf, len, unit_size, + return remote_read_bytes (offset, readbuf, len, unit_size, xfered_len); } @@ -10795,7 +11078,7 @@ remote_target::xfer_partial (enum target_object object, switch (object) { case TARGET_OBJECT_FLASH: - return remote_flash_write (this, offset, len, xfered_len, + return remote_flash_write (offset, len, xfered_len, writebuf); default: @@ -11114,11 +11397,17 @@ remote_target::memory_map () static void packet_command (const char *args, int from_tty) { - struct remote_state *rs = get_remote_state (); + remote_target *remote = get_current_remote_target (); - if (!rs->remote_desc) + if (remote == nullptr) error (_("command can only be used with remote target")); + remote->packet_command (args, from_tty); +} + +void +remote_target::packet_command (const char *args, int from_tty) +{ if (!args) error (_("remote-packet command requires packet text as argument")); @@ -11127,6 +11416,8 @@ packet_command (const char *args, int from_tty) puts_filtered ("\n"); putpkt (args); + remote_state *rs = get_remote_state (); + getpkt (&rs->buf, &rs->buf_size, 0); puts_filtered ("received: "); print_packet (rs->buf); @@ -11653,10 +11944,10 @@ remote_hostio_parse_result (char *buffer, int *retcode, the packet buffer and *ATTACHMENT_LEN will be set to the attachment's length. */ -static int -remote_hostio_send_command (int command_bytes, int which_packet, - int *remote_errno, char **attachment, - int *attachment_len) +int +remote_target::remote_hostio_send_command (int command_bytes, int which_packet, + int *remote_errno, char **attachment, + int *attachment_len) { struct remote_state *rs = get_remote_state (); int ret, bytes_read; @@ -11738,8 +12029,9 @@ readahead_cache::invalidate_fd (int fd) arguments will use. Return 0 on success, or -1 if an error occurs (and set *REMOTE_ERRNO). */ -static int -remote_hostio_set_filesystem (struct inferior *inf, int *remote_errno) +int +remote_target::remote_hostio_set_filesystem (struct inferior *inf, + int *remote_errno) { struct remote_state *rs = get_remote_state (); int required_pid = (inf == NULL || inf->fake_pid_p) ? 0 : inf->pid; @@ -11773,11 +12065,10 @@ remote_hostio_set_filesystem (struct inferior *inf, int *remote_errno) /* Implementation of to_fileio_open. */ -static int -remote_hostio_open (struct target_ops *self, - struct inferior *inf, const char *filename, - int flags, int mode, int warn_if_slow, - int *remote_errno) +int +remote_target::remote_hostio_open (inferior *inf, const char *filename, + int flags, int mode, int warn_if_slow, + int *remote_errno) { struct remote_state *rs = get_remote_state (); char *p = rs->buf; @@ -11822,16 +12113,15 @@ remote_target::fileio_open (struct inferior *inf, const char *filename, int flags, int mode, int warn_if_slow, int *remote_errno) { - return remote_hostio_open (this, inf, filename, flags, mode, warn_if_slow, + return remote_hostio_open (inf, filename, flags, mode, warn_if_slow, remote_errno); } /* Implementation of to_fileio_pwrite. */ -static int -remote_hostio_pwrite (struct target_ops *self, - int fd, const gdb_byte *write_buf, int len, - ULONGEST offset, int *remote_errno) +int +remote_target::remote_hostio_pwrite (int fd, const gdb_byte *write_buf, int len, + ULONGEST offset, int *remote_errno) { struct remote_state *rs = get_remote_state (); char *p = rs->buf; @@ -11859,16 +12149,15 @@ int remote_target::fileio_pwrite (int fd, const gdb_byte *write_buf, int len, ULONGEST offset, int *remote_errno) { - return remote_hostio_pwrite (this, fd, write_buf, len, offset, remote_errno); + return remote_hostio_pwrite (fd, write_buf, len, offset, remote_errno); } /* Helper for the implementation of to_fileio_pread. Read the file from the remote side with vFile:pread. */ -static int -remote_hostio_pread_vFile (struct target_ops *self, - int fd, gdb_byte *read_buf, int len, - ULONGEST offset, int *remote_errno) +int +remote_target::remote_hostio_pread_vFile (int fd, gdb_byte *read_buf, int len, + ULONGEST offset, int *remote_errno) { struct remote_state *rs = get_remote_state (); char *p = rs->buf; @@ -11926,10 +12215,9 @@ readahead_cache::pread (int fd, gdb_byte *read_buf, size_t len, /* Implementation of to_fileio_pread. */ -static int -remote_hostio_pread (struct target_ops *self, - int fd, gdb_byte *read_buf, int len, - ULONGEST offset, int *remote_errno) +int +remote_target::remote_hostio_pread (int fd, gdb_byte *read_buf, int len, + ULONGEST offset, int *remote_errno) { int ret; struct remote_state *rs = get_remote_state (); @@ -11956,7 +12244,7 @@ remote_hostio_pread (struct target_ops *self, cache->bufsize = get_remote_packet_size (); cache->buf = (gdb_byte *) xrealloc (cache->buf, cache->bufsize); - ret = remote_hostio_pread_vFile (self, cache->fd, cache->buf, cache->bufsize, + ret = remote_hostio_pread_vFile (cache->fd, cache->buf, cache->bufsize, cache->offset, remote_errno); if (ret <= 0) { @@ -11972,13 +12260,13 @@ int remote_target::fileio_pread (int fd, gdb_byte *read_buf, int len, ULONGEST offset, int *remote_errno) { - return remote_hostio_pread (this, fd, read_buf, len, offset, remote_errno); + return remote_hostio_pread (fd, read_buf, len, offset, remote_errno); } /* Implementation of to_fileio_close. */ -static int -remote_hostio_close (struct target_ops *self, int fd, int *remote_errno) +int +remote_target::remote_hostio_close (int fd, int *remote_errno) { struct remote_state *rs = get_remote_state (); char *p = rs->buf; @@ -11997,15 +12285,14 @@ remote_hostio_close (struct target_ops *self, int fd, int *remote_errno) int remote_target::fileio_close (int fd, int *remote_errno) { - return remote_hostio_close (this, fd, remote_errno); + return remote_hostio_close (fd, remote_errno); } /* Implementation of to_fileio_unlink. */ -static int -remote_hostio_unlink (struct target_ops *self, - struct inferior *inf, const char *filename, - int *remote_errno) +int +remote_target::remote_hostio_unlink (inferior *inf, const char *filename, + int *remote_errno) { struct remote_state *rs = get_remote_state (); char *p = rs->buf; @@ -12027,7 +12314,7 @@ int remote_target::fileio_unlink (struct inferior *inf, const char *filename, int *remote_errno) { - return remote_hostio_unlink (this, inf, filename, remote_errno); + return remote_hostio_unlink (inf, filename, remote_errno); } /* Implementation of to_fileio_readlink. */ @@ -12148,12 +12435,12 @@ remote_target::filesystem_is_local () /* Try opening a file to probe support. The supplied filename is irrelevant, we only care about whether the stub recognizes the packet or not. */ - fd = remote_hostio_open (this, NULL, "just probing", + fd = remote_hostio_open (NULL, "just probing", FILEIO_O_RDONLY, 0700, 0, &remote_errno); if (fd >= 0) - remote_hostio_close (this, fd, &remote_errno); + remote_hostio_close (fd, &remote_errno); ps = packet_support (PACKET_vFile_open); } @@ -12244,8 +12531,8 @@ remote_hostio_error (int errnum) class scoped_remote_fd { public: - explicit scoped_remote_fd (int fd) - : m_fd (fd) + scoped_remote_fd (remote_target *remote, int fd) + : m_remote (remote), m_fd (fd) { } @@ -12254,8 +12541,7 @@ public: if (m_fd != -1) { int remote_errno; - remote_hostio_close (find_target_at (process_stratum), - m_fd, &remote_errno); + m_remote->remote_hostio_close (m_fd, &remote_errno); } } @@ -12276,12 +12562,27 @@ public: } private: + /* The remote target. */ + remote_target *m_remote; + /* The owned remote I/O file descriptor. */ int m_fd; }; void remote_file_put (const char *local_file, const char *remote_file, int from_tty) +{ + remote_target *remote = get_current_remote_target (); + + if (remote == nullptr) + error (_("command can only be used with remote target")); + + remote->remote_file_put (local_file, remote_file, from_tty); +} + +void +remote_target::remote_file_put (const char *local_file, const char *remote_file, + int from_tty) { struct cleanup *back_to; int retcode, remote_errno, bytes, io_size; @@ -12289,20 +12590,16 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) int bytes_in_buffer; int saw_eof; ULONGEST offset; - struct remote_state *rs = get_remote_state (); - - if (!rs->remote_desc) - error (_("command can only be used with remote target")); gdb_file_up file = gdb_fopen_cloexec (local_file, "rb"); if (file == NULL) perror_with_name (local_file); scoped_remote_fd fd - (remote_hostio_open (find_target_at (process_stratum), NULL, - remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT - | FILEIO_O_TRUNC), - 0700, 0, &remote_errno)); + (this, remote_hostio_open (NULL, + remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT + | FILEIO_O_TRUNC), + 0700, 0, &remote_errno)); if (fd.get () == -1) remote_hostio_error (remote_errno); @@ -12342,8 +12639,7 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) bytes += bytes_in_buffer; bytes_in_buffer = 0; - retcode = remote_hostio_pwrite (find_target_at (process_stratum), - fd.get (), buffer, bytes, + retcode = remote_hostio_pwrite (fd.get (), buffer, bytes, offset, &remote_errno); if (retcode < 0) @@ -12361,8 +12657,7 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) offset += retcode; } - if (remote_hostio_close (find_target_at (process_stratum), - fd.release (), &remote_errno)) + if (remote_hostio_close (fd.release (), &remote_errno)) remote_hostio_error (remote_errno); if (from_tty) @@ -12372,20 +12667,28 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) void remote_file_get (const char *remote_file, const char *local_file, int from_tty) +{ + remote_target *remote = get_current_remote_target (); + + if (remote == nullptr) + error (_("command can only be used with remote target")); + + remote->remote_file_get (remote_file, local_file, from_tty); +} + +void +remote_target::remote_file_get (const char *remote_file, const char *local_file, + int from_tty) { struct cleanup *back_to; int remote_errno, bytes, io_size; gdb_byte *buffer; ULONGEST offset; - struct remote_state *rs = get_remote_state (); - - if (!rs->remote_desc) - error (_("command can only be used with remote target")); scoped_remote_fd fd - (remote_hostio_open (find_target_at (process_stratum), NULL, - remote_file, FILEIO_O_RDONLY, 0, 0, - &remote_errno)); + (this, remote_hostio_open (NULL, + remote_file, FILEIO_O_RDONLY, 0, 0, + &remote_errno)); if (fd.get () == -1) remote_hostio_error (remote_errno); @@ -12402,8 +12705,7 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty) offset = 0; while (1) { - bytes = remote_hostio_pread (find_target_at (process_stratum), - fd.get (), buffer, io_size, offset, + bytes = remote_hostio_pread (fd.get (), buffer, io_size, offset, &remote_errno); if (bytes == 0) /* Success, but no bytes, means end-of-file. */ @@ -12418,8 +12720,7 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty) perror_with_name (local_file); } - if (remote_hostio_close (find_target_at (process_stratum), - fd.release (), &remote_errno)) + if (remote_hostio_close (fd.release (), &remote_errno)) remote_hostio_error (remote_errno); if (from_tty) @@ -12430,14 +12731,20 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty) void remote_file_delete (const char *remote_file, int from_tty) { - int retcode, remote_errno; - struct remote_state *rs = get_remote_state (); + remote_target *remote = get_current_remote_target (); - if (!rs->remote_desc) + if (remote == nullptr) error (_("command can only be used with remote target")); - retcode = remote_hostio_unlink (find_target_at (process_stratum), - NULL, remote_file, &remote_errno); + remote->remote_file_delete (remote_file, from_tty); +} + +void +remote_target::remote_file_delete (const char *remote_file, int from_tty) +{ + int retcode, remote_errno; + + retcode = remote_hostio_unlink (NULL, remote_file, &remote_errno); if (retcode == -1) remote_hostio_error (remote_errno); @@ -12584,9 +12891,9 @@ remote_target::trace_init () /* Recursive routine to walk through command list including loops, and download packets for each command. */ -static void -remote_download_command_source (int num, ULONGEST addr, - struct command_line *cmds) +void +remote_target::remote_download_command_source (int num, ULONGEST addr, + struct command_line *cmds) { struct remote_state *rs = get_remote_state (); struct command_line *cmd; @@ -13426,17 +13733,15 @@ struct btrace_target_info /* Reset our idea of our target's btrace configuration. */ static void -remote_btrace_reset (void) +remote_btrace_reset (remote_state *rs) { - struct remote_state *rs = get_remote_state (); - memset (&rs->btrace_config, 0, sizeof (rs->btrace_config)); } /* Synchronize the configuration with the target. */ -static void -btrace_sync_conf (const struct btrace_config *conf) +void +remote_target::btrace_sync_conf (const btrace_config *conf) { struct packet_config *packet; struct remote_state *rs; @@ -13505,8 +13810,8 @@ btrace_read_config (struct btrace_config *conf) /* Maybe reopen target btrace. */ -static void -remote_btrace_maybe_reopen (void) +void +remote_target::remote_btrace_maybe_reopen () { struct remote_state *rs = get_remote_state (); struct thread_info *tp; @@ -13772,7 +14077,7 @@ remote_target::can_do_single_step () struct remote_state *rs = get_remote_state (); if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN) - remote_vcont_probe (rs); + remote_vcont_probe (); return rs->supports_vCont.s && rs->supports_vCont.S; } @@ -13863,7 +14168,7 @@ remote_async_serial_handler (struct serial *scb, void *context) static void remote_async_inferior_event_handler (gdb_client_data data) { - inferior_event_handler (INF_REG_EVENT, NULL); + inferior_event_handler (INF_REG_EVENT, data); } void @@ -13877,8 +14182,8 @@ remote_target::async (int enable) /* If there are pending events in the stop reply queue tell the event loop to process them. */ - if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue)) - mark_async_event_handler (remote_async_inferior_event_token); + if (!QUEUE_is_empty (stop_reply_p, rs->stop_reply_queue)) + mark_async_event_handler (rs->remote_async_inferior_event_token); /* For simplicity, below we clear the pending events token without remembering whether it is marked, so here we always mark it. If there's actually no pending notification to @@ -13893,7 +14198,7 @@ remote_target::async (int enable) /* If the core is disabling async, it doesn't want to be disturbed with target events. Clear all async event sources too. */ - clear_async_event_handler (remote_async_inferior_event_token); + clear_async_event_handler (rs->remote_async_inferior_event_token); if (target_is_non_stop_p ()) clear_async_event_handler (rs->notif_state->get_pending_events_token); } @@ -13969,10 +14274,10 @@ show_remote_cmd (const char *args, int from_tty) static void remote_new_objfile (struct objfile *objfile) { - struct remote_state *rs = get_remote_state (); + remote_target *remote = get_current_remote_target (); - if (rs->remote_desc != 0) /* Have a remote connection. */ - remote_check_symbols (); + if (remote != NULL) /* Have a remote connection. */ + remote->remote_check_symbols (); } /* Pull all the tracepoints defined on the target and create local @@ -14034,29 +14339,33 @@ show_range_stepping (struct ui_file *file, int from_tty, "is %s.\n"), value); } +/* Return true if the vCont;r action is supported by the remote + stub. */ + +bool +remote_target::vcont_r_supported () +{ + if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN) + remote_vcont_probe (); + + return (packet_support (PACKET_vCont) == PACKET_ENABLE + && get_remote_state ()->supports_vCont.r); +} + /* The "set/show range-stepping" set hook. */ static void set_range_stepping (const char *ignore_args, int from_tty, struct cmd_list_element *c) { - struct remote_state *rs = get_remote_state (); - - /* Whene enabling, check whether range stepping is actually - supported by the target, and warn if not. */ + /* When enabling, check whether range stepping is actually supported + by the target, and warn if not. */ if (use_range_stepping) { - if (rs->remote_desc != NULL) - { - if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN) - remote_vcont_probe (rs); - - if (packet_support (PACKET_vCont) == PACKET_ENABLE - && rs->supports_vCont.r) - return; - } - - warning (_("Range stepping is not supported by the current target")); + remote_target *remote = get_current_remote_target (); + if (remote == NULL + || !remote->vcont_r_supported ()) + warning (_("Range stepping is not supported by the current target")); } } @@ -14074,11 +14383,6 @@ _initialize_remote (void) = register_program_space_data_with_cleanup (NULL, remote_pspace_data_cleanup); - /* Initialize the per-target state. At the moment there is only one - of these, not one per target. Only one target is active at a - time. */ - remote_state = new struct remote_state (); - add_target (remote_target_info, remote_target::open); add_target (extended_remote_target_info, extended_remote_target::open); @@ -14089,7 +14393,6 @@ _initialize_remote (void) init_remote_threadtests (); #endif - stop_reply_queue = QUEUE_alloc (stop_reply_p, stop_reply_xfree); /* set/show remote ... */ add_prefix_cmd ("remote", class_maintenance, set_remote_cmd, _("\ diff --git a/gdb/remote.h b/gdb/remote.h index 85721986db..82c3736f37 100644 --- a/gdb/remote.h +++ b/gdb/remote.h @@ -22,6 +22,7 @@ #include "remote-notif.h" struct target_desc; +struct remote_target; /* Read a packet from the remote machine, with error checking, and store it in *BUF. Resize *BUF using xrealloc if necessary to hold @@ -29,7 +30,8 @@ struct target_desc; rather than timing out; this is used (in synchronous mode) to wait for a target that is is executing user code to stop. */ -extern void getpkt (char **buf, long *sizeof_buf, int forever); +extern void getpkt (remote_target *remote, + char **buf, long *sizeof_buf, int forever); /* Send a packet to the remote machine, with error checking. The data of the packet is in BUF. The string in BUF can be at most PBUFSIZ @@ -37,7 +39,7 @@ extern void getpkt (char **buf, long *sizeof_buf, int forever); we are debugging (remote_debug) and want to print the sent packet as a string. */ -extern int putpkt (const char *buf); +extern int putpkt (remote_target *remote, const char *buf); void register_remote_g_packet_guess (struct gdbarch *gdbarch, int bytes, const struct target_desc *tdesc); @@ -53,5 +55,6 @@ extern int remote_register_number_and_offset (struct gdbarch *gdbarch, int regnum, int *pnum, int *poffset); -extern void remote_notif_get_pending_events (struct notif_client *np); +extern void remote_notif_get_pending_events (remote_target *remote, + struct notif_client *np); #endif -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target 2018-05-16 14:18 ` [PATCH 10/10] remote: one struct remote_state per struct remote_target Pedro Alves @ 2018-05-22 5:07 ` Simon Marchi 2018-05-22 21:06 ` Pedro Alves 0 siblings, 1 reply; 35+ messages in thread From: Simon Marchi @ 2018-05-22 5:07 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > 'struct remote_state' today contains per-connection state, however > there's only a single global instance of that type. In order to > support multiple connections, we must have one such object per > connection. > > Thus this patch eliminates the 'remote_state' global in favor of > having a remote_state instance per remote_target instance. > > The get_remote_state free function is eliminated as well, by making it > a remote_target method instead. > > The patch then fixes the fallout by making all free functions that > refer to get_remote_state() directly or indirectly be methods of > remote_target too. > > Likewise, remote-fileio.c and remote-notif.c routines are > parameterized with a remote_target pointer too, so they can call into > the right remote_target instance. > > References to the global 'get_remote_state ()->remote_desc' to tell > whether the remote target is open (!= nullptr) must be replaced with > something else: > > - Command implementations use a new get_current_remote_target free > function. > > - remote_target::open_1 checks the exception type instead. > > Finally, remote_target and extended_remote_target are made > heap-allocated targets. As with the earlier core target patches, it > still won't be possible to have more than one remote_target instance > in practice, but this puts us closer. Hi Pedro, I took a rather quick look, because a lot of the changes are mecanical, once you have set the premises you pointed out in the commit message (and I think they are fine). Two nits: Is there a reason not to make the remote_state object a simple field of remote_target, does it have to be a pointer? You would have to shuffle things around a little bit more, but it seems to work fine. > @@ -6287,7 +6512,7 @@ remote_target::commit_resume () > we end up with too many actions for a single packet vcont_builder > flushes the current vCont packet to the remote side and starts a > new one. */ > - struct vcont_builder vcont_builder; > + struct vcont_builder vcont_builder (this); > vcont_builder.restart (); That's more a comment for the previous patch, but: I find it strange to have to call restart just after building the object. Couldn't the constructor leave it in a ready to use state? Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target 2018-05-22 5:07 ` Simon Marchi @ 2018-05-22 21:06 ` Pedro Alves 2018-05-24 17:00 ` [PATCH 11/10] remote_target::m_remote_state, pointer -> object (Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target) Pedro Alves 0 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-22 21:06 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 05/22/2018 04:37 AM, Simon Marchi wrote: > I took a rather quick look, because a lot of the changes are mecanical, > once you have set the premises you pointed out in the commit message > (and I think they are fine). > > Two nits: > > Is there a reason not to make the remote_state object a simple field > of remote_target, does it have to be a pointer? You would have to > shuffle things around a little bit more, but it seems to work fine. Yeah, no reason other than struct remote_state not being complete yet when the field is defined in struct remote_target. I was thinking the moving would be done as follow up, to avoid even more churn mixed in with changes, very much like patch #4 started with a pointer and then patch #5 moved to objects. > >> @@ -6287,7 +6512,7 @@ remote_target::commit_resume () >> we end up with too many actions for a single packet vcont_builder >> flushes the current vCont packet to the remote side and starts a >> new one. */ >> - struct vcont_builder vcont_builder; >> + struct vcont_builder vcont_builder (this); >> vcont_builder.restart (); > > That's more a comment for the previous patch, but: I find it strange to > have to call restart just after building the object. Couldn't the > constructor leave it in a ready to use state? Yeah, I guess it could. I've made that change. I'll post it in response to the previous patch. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 11/10] remote_target::m_remote_state, pointer -> object (Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target) 2018-05-22 21:06 ` Pedro Alves @ 2018-05-24 17:00 ` Pedro Alves 2018-05-25 5:23 ` Simon Marchi 0 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-24 17:00 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 05/22/2018 06:30 PM, Pedro Alves wrote: > On 05/22/2018 04:37 AM, Simon Marchi wrote: >> Is there a reason not to make the remote_state object a simple field >> of remote_target, does it have to be a pointer? You would have to >> shuffle things around a little bit more, but it seems to work fine. > > Yeah, no reason other than struct remote_state not being complete yet > when the field is defined in struct remote_target. I was thinking the > moving would be done as follow up, to avoid even more churn mixed in with > changes, very much like patch #4 started with a pointer and then patch #5 > moved to objects. Like this? (Note this is generated with "git format-patch --patience". The default diff algorithm produces a much less readable patch.) From 3ab58d94cb7b3b7039f3e02b5ed72571e546f80d Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Thu, 24 May 2018 16:33:23 +0100 Subject: [PATCH] remote_target::m_remote_state, pointer -> object The only reason remote_target::m_remote_state is a pointer is that struct remote_state is incomplete when struct remote_target is defined. This commit thus moves struct remote_state (and its dependencies) higher up and makes remote_target::m_remote_state an object instead of a pointer. gdb/ChangeLog: 2018-05-24 Pedro Alves <palves@redhat.com> * remote.c (struct vCont_action_support, MAXTHREADLISTRESULTS) (struct readahead_cache, struct packet_reg, struct remote_arch_state, class remote_state): Move higher up in the file. (remote_target::m_remote_state): Now an object instead of a pointer. (remote_target::get_remote_state): Adjust. --- gdb/remote.c | 546 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 272 insertions(+), 274 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 3e4566d985..38ba7b581c 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -123,7 +123,273 @@ enum packet_result }; struct threads_listing_context; -struct remote_state; + +/* Stub vCont actions support. + + Each field is a boolean flag indicating whether the stub reports + support for the corresponding action. */ + +struct vCont_action_support +{ + /* vCont;t */ + bool t = false; + + /* vCont;r */ + bool r = false; + + /* vCont;s */ + bool s = false; + + /* vCont;S */ + bool S = false; +}; + +/* About this many threadisds fit in a packet. */ + +#define MAXTHREADLISTRESULTS 32 + +/* Data for the vFile:pread readahead cache. */ + +struct readahead_cache +{ + /* Invalidate the readahead cache. */ + void invalidate (); + + /* Invalidate the readahead cache if it is holding data for FD. */ + void invalidate_fd (int fd); + + /* Serve pread from the readahead cache. Returns number of bytes + read, or 0 if the request can't be served from the cache. */ + int pread (int fd, gdb_byte *read_buf, size_t len, ULONGEST offset); + + /* The file descriptor for the file that is being cached. -1 if the + cache is invalid. */ + int fd = -1; + + /* The offset into the file that the cache buffer corresponds + to. */ + ULONGEST offset = 0; + + /* The buffer holding the cache contents. */ + gdb_byte *buf = nullptr; + /* The buffer's size. We try to read as much as fits into a packet + at a time. */ + size_t bufsize = 0; + + /* Cache hit and miss counters. */ + ULONGEST hit_count = 0; + ULONGEST miss_count = 0; +}; + +/* Description of the remote protocol for a given architecture. */ + +struct packet_reg +{ + long offset; /* Offset into G packet. */ + long regnum; /* GDB's internal register number. */ + LONGEST pnum; /* Remote protocol register number. */ + int in_g_packet; /* Always part of G packet. */ + /* long size in bytes; == register_size (target_gdbarch (), regnum); + at present. */ + /* char *name; == gdbarch_register_name (target_gdbarch (), regnum); + at present. */ +}; + +struct remote_arch_state +{ + explicit remote_arch_state (struct gdbarch *gdbarch); + + /* Description of the remote protocol registers. */ + long sizeof_g_packet; + + /* Description of the remote protocol registers indexed by REGNUM + (making an array gdbarch_num_regs in size). */ + std::unique_ptr<packet_reg[]> regs; + + /* This is the size (in chars) of the first response to the ``g'' + packet. It is used as a heuristic when determining the maximum + size of memory-read and memory-write packets. A target will + typically only reserve a buffer large enough to hold the ``g'' + packet. The size does not include packet overhead (headers and + trailers). */ + long actual_register_packet_size; + + /* This is the maximum size (in chars) of a non read/write packet. + It is also used as a cap on the size of read/write packets. */ + long remote_packet_size; +}; + +/* Description of the remote protocol state for the currently + connected target. This is per-target state, and independent of the + selected architecture. */ + +class remote_state +{ +public: + + remote_state (); + ~remote_state (); + + /* Get the remote arch state for GDBARCH. */ + struct remote_arch_state *get_remote_arch_state (struct gdbarch *gdbarch); + +public: /* data */ + + /* A buffer to use for incoming packets, and its current size. The + buffer is grown dynamically for larger incoming packets. + Outgoing packets may also be constructed in this buffer. + BUF_SIZE is always at least REMOTE_PACKET_SIZE; + REMOTE_PACKET_SIZE should be used to limit the length of outgoing + packets. */ + char *buf; + long buf_size; + + /* True if we're going through initial connection setup (finding out + about the remote side's threads, relocating symbols, etc.). */ + bool starting_up = false; + + /* If we negotiated packet size explicitly (and thus can bypass + heuristics for the largest packet size that will not overflow + a buffer in the stub), this will be set to that packet size. + Otherwise zero, meaning to use the guessed size. */ + long explicit_packet_size = 0; + + /* remote_wait is normally called when the target is running and + waits for a stop reply packet. But sometimes we need to call it + when the target is already stopped. We can send a "?" packet + and have remote_wait read the response. Or, if we already have + the response, we can stash it in BUF and tell remote_wait to + skip calling getpkt. This flag is set when BUF contains a + stop reply packet and the target is not waiting. */ + int cached_wait_status = 0; + + /* True, if in no ack mode. That is, neither GDB nor the stub will + expect acks from each other. The connection is assumed to be + reliable. */ + bool noack_mode = false; + + /* True if we're connected in extended remote mode. */ + bool extended = false; + + /* True if we resumed the target and we're waiting for the target to + stop. In the mean time, we can't start another command/query. + The remote server wouldn't be ready to process it, so we'd + timeout waiting for a reply that would never come and eventually + we'd close the connection. This can happen in asynchronous mode + because we allow GDB commands while the target is running. */ + bool waiting_for_stop_reply = false; + + /* The status of the stub support for the various vCont actions. */ + vCont_action_support supports_vCont; + + /* True if the user has pressed Ctrl-C, but the target hasn't + responded to that. */ + bool ctrlc_pending_p = false; + + /* True if we saw a Ctrl-C while reading or writing from/to the + remote descriptor. At that point it is not safe to send a remote + interrupt packet, so we instead remember we saw the Ctrl-C and + process it once we're done with sending/receiving the current + packet, which should be shortly. If however that takes too long, + and the user presses Ctrl-C again, we offer to disconnect. */ + bool got_ctrlc_during_io = false; + + /* Descriptor for I/O to remote machine. Initialize it to NULL so that + remote_open knows that we don't have a file open when the program + starts. */ + struct serial *remote_desc = nullptr; + + /* These are the threads which we last sent to the remote system. The + TID member will be -1 for all or -2 for not sent yet. */ + ptid_t general_thread = null_ptid; + ptid_t continue_thread = null_ptid; + + /* This is the traceframe which we last selected on the remote system. + It will be -1 if no traceframe is selected. */ + int remote_traceframe_number = -1; + + char *last_pass_packet = nullptr; + + /* The last QProgramSignals packet sent to the target. We bypass + sending a new program signals list down to the target if the new + packet is exactly the same as the last we sent. IOW, we only let + the target know about program signals list changes. */ + char *last_program_signals_packet = nullptr; + + gdb_signal last_sent_signal = GDB_SIGNAL_0; + + bool last_sent_step = false; + + /* The execution direction of the last resume we got. */ + exec_direction_kind last_resume_exec_dir = EXEC_FORWARD; + + char *finished_object = nullptr; + char *finished_annex = nullptr; + ULONGEST finished_offset = 0; + + /* Should we try the 'ThreadInfo' query packet? + + This variable (NOT available to the user: auto-detect only!) + determines whether GDB will use the new, simpler "ThreadInfo" + query or the older, more complex syntax for thread queries. + This is an auto-detect variable (set to true at each connect, + and set to false when the target fails to recognize it). */ + bool use_threadinfo_query = false; + bool use_threadextra_query = false; + + threadref echo_nextthread {}; + threadref nextthread {}; + threadref resultthreadlist[MAXTHREADLISTRESULTS] {}; + + /* The state of remote notification. */ + struct remote_notif_state *notif_state = nullptr; + + /* The branch trace configuration. */ + struct btrace_config btrace_config {}; + + /* The argument to the last "vFile:setfs:" packet we sent, used + to avoid sending repeated unnecessary "vFile:setfs:" packets. + Initialized to -1 to indicate that no "vFile:setfs:" packet + has yet been sent. */ + int fs_pid = -1; + + /* A readahead cache for vFile:pread. Often, reading a binary + involves a sequence of small reads. E.g., when parsing an ELF + file. A readahead cache helps mostly the case of remote + debugging on a connection with higher latency, due to the + request/reply nature of the RSP. We only cache data for a single + file descriptor at a time. */ + struct readahead_cache readahead_cache; + + /* The list of already fetched and acknowledged stop events. This + queue is used for notification Stop, and other notifications + don't need queue for their events, because the notification + events of Stop can't be consumed immediately, so that events + should be queued first, and be consumed by remote_wait_{ns,as} + one per time. Other notifications can consume their events + immediately, so queue is not needed for them. */ + QUEUE (stop_reply_p) *stop_reply_queue; + + /* Asynchronous signal handle registered as event loop source for + when we have pending events ready to be passed to the core. */ + struct async_event_handler *remote_async_inferior_event_token = nullptr; + + /* FIXME: cagney/1999-09-23: Even though getpkt was called with + ``forever'' still use the normal timeout mechanism. This is + currently used by the ASYNC code to guarentee that target reads + during the initial connect always time-out. Once getpkt has been + modified to return a timeout indication and, in turn + remote_wait()/wait_for_inferior() have gained a timeout parameter + this can go away. */ + int wait_forever_enabled_p = 1; + +private: + /* Mapping of remote protocol data for each gdbarch. Usually there + is only one entry here, though we may see more with stubs that + support multi-process. */ + std::unordered_map<struct gdbarch *, remote_arch_state> + m_arch_states; +}; static const target_info remote_target_info = { "remote", @@ -671,7 +937,9 @@ public: /* Remote specific methods. */ private: /* data fields */ - std::unique_ptr<struct remote_state> m_remote_state; + /* The remote state. Don't reference this directly. Use the + get_remote_state method instead. */ + remote_state m_remote_state; }; static const target_info extended_remote_target_info = { @@ -775,282 +1043,15 @@ static struct cmd_list_element *remote_cmdlist; static struct cmd_list_element *remote_set_cmdlist; static struct cmd_list_element *remote_show_cmdlist; -/* Stub vCont actions support. - - Each field is a boolean flag indicating whether the stub reports - support for the corresponding action. */ - -struct vCont_action_support -{ - /* vCont;t */ - bool t = false; - - /* vCont;r */ - bool r = false; - - /* vCont;s */ - bool s = false; - - /* vCont;S */ - bool S = false; -}; - /* Controls whether GDB is willing to use range stepping. */ static int use_range_stepping = 1; -/* About this many threadisds fit in a packet. */ - -#define MAXTHREADLISTRESULTS 32 - /* The max number of chars in debug output. The rest of chars are omitted. */ #define REMOTE_DEBUG_MAX_CHAR 512 -/* Data for the vFile:pread readahead cache. */ - -struct readahead_cache -{ - /* Invalidate the readahead cache. */ - void invalidate (); - - /* Invalidate the readahead cache if it is holding data for FD. */ - void invalidate_fd (int fd); - - /* Serve pread from the readahead cache. Returns number of bytes - read, or 0 if the request can't be served from the cache. */ - int pread (int fd, gdb_byte *read_buf, size_t len, ULONGEST offset); - - /* The file descriptor for the file that is being cached. -1 if the - cache is invalid. */ - int fd = -1; - - /* The offset into the file that the cache buffer corresponds - to. */ - ULONGEST offset = 0; - - /* The buffer holding the cache contents. */ - gdb_byte *buf = nullptr; - /* The buffer's size. We try to read as much as fits into a packet - at a time. */ - size_t bufsize = 0; - - /* Cache hit and miss counters. */ - ULONGEST hit_count = 0; - ULONGEST miss_count = 0; -}; - -/* Description of the remote protocol for a given architecture. */ - -struct packet_reg -{ - long offset; /* Offset into G packet. */ - long regnum; /* GDB's internal register number. */ - LONGEST pnum; /* Remote protocol register number. */ - int in_g_packet; /* Always part of G packet. */ - /* long size in bytes; == register_size (target_gdbarch (), regnum); - at present. */ - /* char *name; == gdbarch_register_name (target_gdbarch (), regnum); - at present. */ -}; - -struct remote_arch_state -{ - explicit remote_arch_state (struct gdbarch *gdbarch); - - /* Description of the remote protocol registers. */ - long sizeof_g_packet; - - /* Description of the remote protocol registers indexed by REGNUM - (making an array gdbarch_num_regs in size). */ - std::unique_ptr<packet_reg[]> regs; - - /* This is the size (in chars) of the first response to the ``g'' - packet. It is used as a heuristic when determining the maximum - size of memory-read and memory-write packets. A target will - typically only reserve a buffer large enough to hold the ``g'' - packet. The size does not include packet overhead (headers and - trailers). */ - long actual_register_packet_size; - - /* This is the maximum size (in chars) of a non read/write packet. - It is also used as a cap on the size of read/write packets. */ - long remote_packet_size; -}; - -/* Description of the remote protocol state for the currently - connected target. This is per-target state, and independent of the - selected architecture. */ - -class remote_state -{ -public: - - remote_state (); - ~remote_state (); - - /* Get the remote arch state for GDBARCH. */ - struct remote_arch_state *get_remote_arch_state (struct gdbarch *gdbarch); - -public: /* data */ - - /* A buffer to use for incoming packets, and its current size. The - buffer is grown dynamically for larger incoming packets. - Outgoing packets may also be constructed in this buffer. - BUF_SIZE is always at least REMOTE_PACKET_SIZE; - REMOTE_PACKET_SIZE should be used to limit the length of outgoing - packets. */ - char *buf; - long buf_size; - - /* True if we're going through initial connection setup (finding out - about the remote side's threads, relocating symbols, etc.). */ - bool starting_up = false; - - /* If we negotiated packet size explicitly (and thus can bypass - heuristics for the largest packet size that will not overflow - a buffer in the stub), this will be set to that packet size. - Otherwise zero, meaning to use the guessed size. */ - long explicit_packet_size = 0; - - /* remote_wait is normally called when the target is running and - waits for a stop reply packet. But sometimes we need to call it - when the target is already stopped. We can send a "?" packet - and have remote_wait read the response. Or, if we already have - the response, we can stash it in BUF and tell remote_wait to - skip calling getpkt. This flag is set when BUF contains a - stop reply packet and the target is not waiting. */ - int cached_wait_status = 0; - - /* True, if in no ack mode. That is, neither GDB nor the stub will - expect acks from each other. The connection is assumed to be - reliable. */ - bool noack_mode = false; - - /* True if we're connected in extended remote mode. */ - bool extended = false; - - /* True if we resumed the target and we're waiting for the target to - stop. In the mean time, we can't start another command/query. - The remote server wouldn't be ready to process it, so we'd - timeout waiting for a reply that would never come and eventually - we'd close the connection. This can happen in asynchronous mode - because we allow GDB commands while the target is running. */ - bool waiting_for_stop_reply = false; - - /* The status of the stub support for the various vCont actions. */ - vCont_action_support supports_vCont; - - /* True if the user has pressed Ctrl-C, but the target hasn't - responded to that. */ - bool ctrlc_pending_p = false; - - /* True if we saw a Ctrl-C while reading or writing from/to the - remote descriptor. At that point it is not safe to send a remote - interrupt packet, so we instead remember we saw the Ctrl-C and - process it once we're done with sending/receiving the current - packet, which should be shortly. If however that takes too long, - and the user presses Ctrl-C again, we offer to disconnect. */ - bool got_ctrlc_during_io = false; - - /* Descriptor for I/O to remote machine. Initialize it to NULL so that - remote_open knows that we don't have a file open when the program - starts. */ - struct serial *remote_desc = nullptr; - - /* These are the threads which we last sent to the remote system. The - TID member will be -1 for all or -2 for not sent yet. */ - ptid_t general_thread = null_ptid; - ptid_t continue_thread = null_ptid; - - /* This is the traceframe which we last selected on the remote system. - It will be -1 if no traceframe is selected. */ - int remote_traceframe_number = -1; - - char *last_pass_packet = nullptr; - - /* The last QProgramSignals packet sent to the target. We bypass - sending a new program signals list down to the target if the new - packet is exactly the same as the last we sent. IOW, we only let - the target know about program signals list changes. */ - char *last_program_signals_packet = nullptr; - - gdb_signal last_sent_signal = GDB_SIGNAL_0; - - bool last_sent_step = false; - - /* The execution direction of the last resume we got. */ - exec_direction_kind last_resume_exec_dir = EXEC_FORWARD; - - char *finished_object = nullptr; - char *finished_annex = nullptr; - ULONGEST finished_offset = 0; - - /* Should we try the 'ThreadInfo' query packet? - - This variable (NOT available to the user: auto-detect only!) - determines whether GDB will use the new, simpler "ThreadInfo" - query or the older, more complex syntax for thread queries. - This is an auto-detect variable (set to true at each connect, - and set to false when the target fails to recognize it). */ - bool use_threadinfo_query = false; - bool use_threadextra_query = false; - - threadref echo_nextthread {}; - threadref nextthread {}; - threadref resultthreadlist[MAXTHREADLISTRESULTS] {}; - - /* The state of remote notification. */ - struct remote_notif_state *notif_state = nullptr; - - /* The branch trace configuration. */ - struct btrace_config btrace_config {}; - - /* The argument to the last "vFile:setfs:" packet we sent, used - to avoid sending repeated unnecessary "vFile:setfs:" packets. - Initialized to -1 to indicate that no "vFile:setfs:" packet - has yet been sent. */ - int fs_pid = -1; - - /* A readahead cache for vFile:pread. Often, reading a binary - involves a sequence of small reads. E.g., when parsing an ELF - file. A readahead cache helps mostly the case of remote - debugging on a connection with higher latency, due to the - request/reply nature of the RSP. We only cache data for a single - file descriptor at a time. */ - struct readahead_cache readahead_cache; - - /* The list of already fetched and acknowledged stop events. This - queue is used for notification Stop, and other notifications - don't need queue for their events, because the notification - events of Stop can't be consumed immediately, so that events - should be queued first, and be consumed by remote_wait_{ns,as} - one per time. Other notifications can consume their events - immediately, so queue is not needed for them. */ - QUEUE (stop_reply_p) *stop_reply_queue; - - /* Asynchronous signal handle registered as event loop source for - when we have pending events ready to be passed to the core. */ - struct async_event_handler *remote_async_inferior_event_token = nullptr; - - /* FIXME: cagney/1999-09-23: Even though getpkt was called with - ``forever'' still use the normal timeout mechanism. This is - currently used by the ASYNC code to guarentee that target reads - during the initial connect always time-out. Once getpkt has been - modified to return a timeout indication and, in turn - remote_wait()/wait_for_inferior() have gained a timeout parameter - this can go away. */ - int wait_forever_enabled_p = 1; - -private: - /* Mapping of remote protocol data for each gdbarch. Usually there - is only one entry here, though we may see more with stubs that - support multi-process. */ - std::unordered_map<struct gdbarch *, remote_arch_state> - m_arch_states; -}; - /* Private data that we'll store in (struct thread_info)->priv. */ struct remote_thread_info : public private_thread_info { @@ -1237,17 +1238,14 @@ remote_state::get_remote_arch_state (struct gdbarch *gdbarch) remote_state * remote_target::get_remote_state () { - if (m_remote_state == nullptr) - m_remote_state.reset (new remote_state ()); - /* Make sure that the remote architecture state has been initialized, because doing so might reallocate rs->buf. Any function which calls getpkt also needs to be mindful of changes to rs->buf, but this call limits the number of places which run into trouble. */ - m_remote_state->get_remote_arch_state (target_gdbarch ()); + m_remote_state.get_remote_arch_state (target_gdbarch ()); - return m_remote_state.get (); + return &m_remote_state; } /* Cleanup routine for the remote module's pspace data. */ -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/10] remote_target::m_remote_state, pointer -> object (Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target) 2018-05-24 17:00 ` [PATCH 11/10] remote_target::m_remote_state, pointer -> object (Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target) Pedro Alves @ 2018-05-25 5:23 ` Simon Marchi 0 siblings, 0 replies; 35+ messages in thread From: Simon Marchi @ 2018-05-25 5:23 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On 2018-05-24 11:53, Pedro Alves wrote: > On 05/22/2018 06:30 PM, Pedro Alves wrote: >> On 05/22/2018 04:37 AM, Simon Marchi wrote: > >>> Is there a reason not to make the remote_state object a simple field >>> of remote_target, does it have to be a pointer? You would have to >>> shuffle things around a little bit more, but it seems to work fine. >> >> Yeah, no reason other than struct remote_state not being complete yet >> when the field is defined in struct remote_target. I was thinking the >> moving would be done as follow up, to avoid even more churn mixed in >> with >> changes, very much like patch #4 started with a pointer and then patch >> #5 >> moved to objects. > Like this? Yep, looks good! ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves ` (2 preceding siblings ...) 2018-05-16 14:18 ` [PATCH 10/10] remote: one struct remote_state per struct remote_target Pedro Alves @ 2018-05-16 14:18 ` Pedro Alves 2018-05-16 17:43 ` Tom Tromey 2018-05-18 21:04 ` Simon Marchi 2018-05-16 14:25 ` [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects Pedro Alves ` (5 subsequent siblings) 9 siblings, 2 replies; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:18 UTC (permalink / raw) To: gdb-patches gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (remote_hostio_close_cleanup): Delete. (class scoped_remote_fd): New. (remote_file_put, remote_file_get): Use it. --- gdb/remote.c | 89 ++++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index ca72c1a1c2..bc4815c67e 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -12198,20 +12198,52 @@ remote_hostio_error (int errnum) error (_("Remote I/O error: %s"), safe_strerror (host_error)); } -static void -remote_hostio_close_cleanup (void *opaque) +/* A RAII wrapper around a remote file descriptor. */ + +class scoped_remote_fd { - int fd = *(int *) opaque; - int remote_errno; +public: + explicit scoped_remote_fd (int fd) + : m_fd (fd) + { + } - remote_hostio_close (find_target_at (process_stratum), fd, &remote_errno); -} + ~scoped_remote_fd () + { + if (m_fd != -1) + { + int remote_errno; + remote_hostio_close (find_target_at (process_stratum), + m_fd, &remote_errno); + } + } + + DISABLE_COPY_AND_ASSIGN (scoped_remote_fd); + + /* Release ownership of the file descriptor, and return it. */ + int release () noexcept + { + int fd = m_fd; + m_fd = -1; + return fd; + } + + /* Return the owned file descriptor. */ + int get () const noexcept + { + return m_fd; + } + +private: + /* The owned remote I/O file descriptor. */ + int m_fd; +}; void remote_file_put (const char *local_file, const char *remote_file, int from_tty) { - struct cleanup *back_to, *close_cleanup; - int retcode, fd, remote_errno, bytes, io_size; + struct cleanup *back_to; + int retcode, remote_errno, bytes, io_size; gdb_byte *buffer; int bytes_in_buffer; int saw_eof; @@ -12225,11 +12257,12 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) if (file == NULL) perror_with_name (local_file); - fd = remote_hostio_open (find_target_at (process_stratum), NULL, - remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT - | FILEIO_O_TRUNC), - 0700, 0, &remote_errno); - if (fd == -1) + scoped_remote_fd fd + (remote_hostio_open (find_target_at (process_stratum), NULL, + remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT + | FILEIO_O_TRUNC), + 0700, 0, &remote_errno)); + if (fd.get () == -1) remote_hostio_error (remote_errno); /* Send up to this many bytes at once. They won't all fit in the @@ -12238,8 +12271,6 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) buffer = (gdb_byte *) xmalloc (io_size); back_to = make_cleanup (xfree, buffer); - close_cleanup = make_cleanup (remote_hostio_close_cleanup, &fd); - bytes_in_buffer = 0; saw_eof = 0; offset = 0; @@ -12271,7 +12302,7 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) bytes_in_buffer = 0; retcode = remote_hostio_pwrite (find_target_at (process_stratum), - fd, buffer, bytes, + fd.get (), buffer, bytes, offset, &remote_errno); if (retcode < 0) @@ -12289,8 +12320,8 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) offset += retcode; } - discard_cleanups (close_cleanup); - if (remote_hostio_close (find_target_at (process_stratum), fd, &remote_errno)) + if (remote_hostio_close (find_target_at (process_stratum), + fd.release (), &remote_errno)) remote_hostio_error (remote_errno); if (from_tty) @@ -12301,8 +12332,8 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty) void remote_file_get (const char *remote_file, const char *local_file, int from_tty) { - struct cleanup *back_to, *close_cleanup; - int fd, remote_errno, bytes, io_size; + struct cleanup *back_to; + int remote_errno, bytes, io_size; gdb_byte *buffer; ULONGEST offset; struct remote_state *rs = get_remote_state (); @@ -12310,10 +12341,11 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty) if (!rs->remote_desc) error (_("command can only be used with remote target")); - fd = remote_hostio_open (find_target_at (process_stratum), NULL, - remote_file, FILEIO_O_RDONLY, 0, 0, - &remote_errno); - if (fd == -1) + scoped_remote_fd fd + (remote_hostio_open (find_target_at (process_stratum), NULL, + remote_file, FILEIO_O_RDONLY, 0, 0, + &remote_errno)); + if (fd.get () == -1) remote_hostio_error (remote_errno); gdb_file_up file = gdb_fopen_cloexec (local_file, "wb"); @@ -12326,13 +12358,12 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty) buffer = (gdb_byte *) xmalloc (io_size); back_to = make_cleanup (xfree, buffer); - close_cleanup = make_cleanup (remote_hostio_close_cleanup, &fd); - offset = 0; while (1) { bytes = remote_hostio_pread (find_target_at (process_stratum), - fd, buffer, io_size, offset, &remote_errno); + fd.get (), buffer, io_size, offset, + &remote_errno); if (bytes == 0) /* Success, but no bytes, means end-of-file. */ break; @@ -12346,8 +12377,8 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty) perror_with_name (local_file); } - discard_cleanups (close_cleanup); - if (remote_hostio_close (find_target_at (process_stratum), fd, &remote_errno)) + if (remote_hostio_close (find_target_at (process_stratum), + fd.release (), &remote_errno)) remote_hostio_error (remote_errno); if (from_tty) -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup 2018-05-16 14:18 ` [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup Pedro Alves @ 2018-05-16 17:43 ` Tom Tromey 2018-05-16 18:53 ` Pedro Alves 2018-05-18 21:04 ` Simon Marchi 1 sibling, 1 reply; 35+ messages in thread From: Tom Tromey @ 2018-05-16 17:43 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> + ~scoped_remote_fd () Pedro> + { Pedro> + if (m_fd != -1) Pedro> + { Pedro> + int remote_errno; Pedro> + remote_hostio_close (find_target_at (process_stratum), Pedro> + m_fd, &remote_errno); The only danger here is if remote_hostio_close can throw. However, this was already a danger (in theory) before the patch -- there is a rule (perhaps unwritten and/or unenforced) that one cannot throw during a "do_cleanups". I didn't look but some assurance that this can't happen would be good to have. Tom ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup 2018-05-16 17:43 ` Tom Tromey @ 2018-05-16 18:53 ` Pedro Alves 2018-05-16 19:46 ` Tom Tromey 0 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 18:53 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 05/16/2018 06:37 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > Pedro> + ~scoped_remote_fd () > Pedro> + { > Pedro> + if (m_fd != -1) > Pedro> + { > Pedro> + int remote_errno; > Pedro> + remote_hostio_close (find_target_at (process_stratum), > Pedro> + m_fd, &remote_errno); > > The only danger here is if remote_hostio_close can throw. However, this > was already a danger (in theory) before the patch -- there is a rule > (perhaps unwritten and/or unenforced) that one cannot throw during a > "do_cleanups". Yeah, a cleanup throwing leaves things in a bad state. I've been ignoring this whole "destructors shouldn't throw" thing in RAII-fying patches for that reason, but it wouldn't hurt to start handling it properly. I don't think there's anything that can be done in this case other than swallowing the exception. We already call remote_hostio_close explicitly in the normal paths, so the destructor trying to close the file is just for the already-unwinding-due-to-some-other-exception case. > I didn't look but some assurance that this can't happen would be good to > have. So it seems to me that the below would be the way to go. WDYT? diff --git a/gdb/remote.c b/gdb/remote.c index bc4815c67e..9761332ee4 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -12213,8 +12213,17 @@ public: if (m_fd != -1) { int remote_errno; - remote_hostio_close (find_target_at (process_stratum), - m_fd, &remote_errno); + try + { + remote_hostio_close (find_target_at (process_stratum), + m_fd, &remote_errno); + } + catch (...) + { + /* Swallow exception before it escapes the dtor. If + something goes wrong, likely the connection is gone, + and there's nothing else that can be done. */ + } } } -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup 2018-05-16 18:53 ` Pedro Alves @ 2018-05-16 19:46 ` Tom Tromey 0 siblings, 0 replies; 35+ messages in thread From: Tom Tromey @ 2018-05-16 19:46 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> So it seems to me that the below would be the way to go. WDYT? Yeah, seems like a good idea. Tom ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup 2018-05-16 14:18 ` [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup Pedro Alves 2018-05-16 17:43 ` Tom Tromey @ 2018-05-18 21:04 ` Simon Marchi 1 sibling, 0 replies; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:04 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves <palves@redhat.com> > > * remote.c (remote_hostio_close_cleanup): Delete. > (class scoped_remote_fd): New. > (remote_file_put, remote_file_get): Use it. LGTM, including catching the exception in the dtor. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves ` (3 preceding siblings ...) 2018-05-16 14:18 ` [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup Pedro Alves @ 2018-05-16 14:25 ` Pedro Alves 2018-05-18 21:17 ` Simon Marchi 2018-05-16 14:25 ` [PATCH 03/10] remote: Make readahead_cache a C++ class Pedro Alves ` (4 subsequent siblings) 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:25 UTC (permalink / raw) To: gdb-patches The previous patch made the map store pointers to remote_arch_state instead of objects directly, simply because struct remote_arch_state is still incomplete where struct remote_state is declared. This patch thus moves the remote_arch_state declaration higher up in the file, and makes the map store remote_arch_state objects directly instead of pointers to objects. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (struct packet_reg, struct remote_arch_state): Move higher up in the file. (remote_state) <m_arch_states>: Store remote_arch_state values instead of remote_arch_state pointers. (remote_state::get_remote_arch_state): Adjust. --- gdb/remote.c | 93 ++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 49 insertions(+), 44 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 7903cb6344..8e1baf0856 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -603,6 +603,44 @@ struct readahead_cache ULONGEST miss_count = 0; }; +/* Description of the remote protocol for a given architecture. */ + +struct packet_reg +{ + long offset; /* Offset into G packet. */ + long regnum; /* GDB's internal register number. */ + LONGEST pnum; /* Remote protocol register number. */ + int in_g_packet; /* Always part of G packet. */ + /* long size in bytes; == register_size (target_gdbarch (), regnum); + at present. */ + /* char *name; == gdbarch_register_name (target_gdbarch (), regnum); + at present. */ +}; + +struct remote_arch_state +{ + explicit remote_arch_state (struct gdbarch *gdbarch); + + /* Description of the remote protocol registers. */ + long sizeof_g_packet; + + /* Description of the remote protocol registers indexed by REGNUM + (making an array gdbarch_num_regs in size). */ + std::unique_ptr<packet_reg[]> regs; + + /* This is the size (in chars) of the first response to the ``g'' + packet. It is used as a heuristic when determining the maximum + size of memory-read and memory-write packets. A target will + typically only reserve a buffer large enough to hold the ``g'' + packet. The size does not include packet overhead (headers and + trailers). */ + long actual_register_packet_size; + + /* This is the maximum size (in chars) of a non read/write packet. + It is also used as a cap on the size of read/write packets. */ + long remote_packet_size; +}; + /* Description of the remote protocol state for the currently connected target. This is per-target state, and independent of the selected architecture. */ @@ -749,8 +787,7 @@ private: /* Mapping of remote protocol data for each gdbarch. Usually there is only one entry here, though we may see more with stubs that support multi-process. */ - std::unordered_map<struct gdbarch *, - std::unique_ptr<struct remote_arch_state>> + std::unordered_map<struct gdbarch *, remote_arch_state> m_arch_states; }; @@ -817,44 +854,6 @@ get_remote_state_raw (void) return remote_state; } -/* Description of the remote protocol for a given architecture. */ - -struct packet_reg -{ - long offset; /* Offset into G packet. */ - long regnum; /* GDB's internal register number. */ - LONGEST pnum; /* Remote protocol register number. */ - int in_g_packet; /* Always part of G packet. */ - /* long size in bytes; == register_size (target_gdbarch (), regnum); - at present. */ - /* char *name; == gdbarch_register_name (target_gdbarch (), regnum); - at present. */ -}; - -struct remote_arch_state -{ - explicit remote_arch_state (struct gdbarch *gdbarch); - - /* Description of the remote protocol registers. */ - long sizeof_g_packet; - - /* Description of the remote protocol registers indexed by REGNUM - (making an array gdbarch_num_regs in size). */ - std::unique_ptr<packet_reg[]> regs; - - /* This is the size (in chars) of the first response to the ``g'' - packet. It is used as a heuristic when determining the maximum - size of memory-read and memory-write packets. A target will - typically only reserve a buffer large enough to hold the ``g'' - packet. The size does not include packet overhead (headers and - trailers). */ - long actual_register_packet_size; - - /* This is the maximum size (in chars) of a non read/write packet. - It is also used as a cap on the size of read/write packets. */ - long remote_packet_size; -}; - /* Utility: generate error from an incoming stub packet. */ static void trace_error (char *buf) @@ -955,10 +954,13 @@ remote_get_noisy_reply () struct remote_arch_state * remote_state::get_remote_arch_state (struct gdbarch *gdbarch) { - auto &rsa = this->m_arch_states[gdbarch]; - if (rsa == nullptr) + remote_arch_state *rsa; + + auto it = this->m_arch_states.find (gdbarch); + if (it == this->m_arch_states.end ()) { - rsa.reset (new remote_arch_state (gdbarch)); + auto p = this->m_arch_states.emplace (gdbarch, gdbarch); + rsa = &p.first->second; /* Make sure that the packet buffer is plenty big enough for this architecture. */ @@ -968,7 +970,10 @@ remote_state::get_remote_arch_state (struct gdbarch *gdbarch) this->buf = (char *) xrealloc (this->buf, this->buf_size); } } - return rsa.get (); + else + rsa = &it->second; + + return rsa; } /* Fetch the global remote target state. */ -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects 2018-05-16 14:25 ` [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects Pedro Alves @ 2018-05-18 21:17 ` Simon Marchi 2018-05-18 21:18 ` Simon Marchi 0 siblings, 1 reply; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:17 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > The previous patch made the map store pointers to remote_arch_state > instead of objects directly, simply because struct remote_arch_state > is still incomplete where struct remote_state is declared. This patch > thus moves the remote_arch_state declaration higher up in the file, > and makes the map store remote_arch_state objects directly instead of > pointers to objects. LGTM. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects 2018-05-18 21:17 ` Simon Marchi @ 2018-05-18 21:18 ` Simon Marchi 2018-05-21 16:12 ` Pedro Alves 0 siblings, 1 reply; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:18 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-18 05:05 PM, Simon Marchi wrote: > On 2018-05-16 10:18 AM, Pedro Alves wrote: >> The previous patch made the map store pointers to remote_arch_state >> instead of objects directly, simply because struct remote_arch_state >> is still incomplete where struct remote_state is declared. This patch >> thus moves the remote_arch_state declaration higher up in the file, >> and makes the map store remote_arch_state objects directly instead of >> pointers to objects. > > LGTM. > > Simon > Oops, spoke a bit too soon. It fails to build with g++ 5.4.0 (Ubuntu 16.04). It doesn't like the "this->m_arch_states.emplace (gdbarch, gdbarch)". With g++ 7.1.0 it builds fine. /home/emaisin/src/binutils-gdb/gdb/remote.c:962:61: required from here /usr/include/c++/5/ext/new_allocator.h:120:4: error: no matching function for call to ‘std::pair<gdbarch* const, remote_arch_state>::pair(gdbarch*&, gdbarch*&)’ { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } ^ Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects 2018-05-18 21:18 ` Simon Marchi @ 2018-05-21 16:12 ` Pedro Alves 0 siblings, 0 replies; 35+ messages in thread From: Pedro Alves @ 2018-05-21 16:12 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 05/18/2018 10:09 PM, Simon Marchi wrote: > Oops, spoke a bit too soon. It fails to build with g++ 5.4.0 (Ubuntu 16.04). > It doesn't like the "this->m_arch_states.emplace (gdbarch, gdbarch)". With > g++ 7.1.0 it builds fine. > > /home/emaisin/src/binutils-gdb/gdb/remote.c:962:61: required from here > /usr/include/c++/5/ext/new_allocator.h:120:4: error: no matching function for call to ‘std::pair<gdbarch* const, remote_arch_state>::pair(gdbarch*&, gdbarch*&)’ > { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } > ^ Bah. I tried with gcc 4.8 now, and it trips on the same issue. Using a different emplace overload works. From aecfc376df9b718c9f86898db925031151d16774 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Mon, 21 May 2018 16:47:03 +0100 Subject: [PATCH] remote: remote_arch_state pointers -> remote_arch_state objects The previous patch made the map store pointers to remote_arch_state instead of objects directly, simply because struct remote_arch_state is still incomplete where struct remote_state is declared. This patch thus moves the remote_arch_state declaration higher up in the file, and makes the map store remote_arch_state objects directly instead of pointers to objects. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (struct packet_reg, struct remote_arch_state): Move higher up in the file. (remote_state) <m_arch_states>: Store remote_arch_state values instead of remote_arch_state pointers. (remote_state::get_remote_arch_state): Adjust. --- gdb/remote.c | 95 ++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index ba054f5061..1d1819bac1 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -603,6 +603,44 @@ struct readahead_cache ULONGEST miss_count = 0; }; +/* Description of the remote protocol for a given architecture. */ + +struct packet_reg +{ + long offset; /* Offset into G packet. */ + long regnum; /* GDB's internal register number. */ + LONGEST pnum; /* Remote protocol register number. */ + int in_g_packet; /* Always part of G packet. */ + /* long size in bytes; == register_size (target_gdbarch (), regnum); + at present. */ + /* char *name; == gdbarch_register_name (target_gdbarch (), regnum); + at present. */ +}; + +struct remote_arch_state +{ + explicit remote_arch_state (struct gdbarch *gdbarch); + + /* Description of the remote protocol registers. */ + long sizeof_g_packet; + + /* Description of the remote protocol registers indexed by REGNUM + (making an array gdbarch_num_regs in size). */ + std::unique_ptr<packet_reg[]> regs; + + /* This is the size (in chars) of the first response to the ``g'' + packet. It is used as a heuristic when determining the maximum + size of memory-read and memory-write packets. A target will + typically only reserve a buffer large enough to hold the ``g'' + packet. The size does not include packet overhead (headers and + trailers). */ + long actual_register_packet_size; + + /* This is the maximum size (in chars) of a non read/write packet. + It is also used as a cap on the size of read/write packets. */ + long remote_packet_size; +}; + /* Description of the remote protocol state for the currently connected target. This is per-target state, and independent of the selected architecture. */ @@ -749,8 +787,7 @@ private: /* Mapping of remote protocol data for each gdbarch. Usually there is only one entry here, though we may see more with stubs that support multi-process. */ - std::unordered_map<struct gdbarch *, - std::unique_ptr<struct remote_arch_state>> + std::unordered_map<struct gdbarch *, remote_arch_state> m_arch_states; }; @@ -820,44 +857,6 @@ get_remote_state_raw (void) return remote_state; } -/* Description of the remote protocol for a given architecture. */ - -struct packet_reg -{ - long offset; /* Offset into G packet. */ - long regnum; /* GDB's internal register number. */ - LONGEST pnum; /* Remote protocol register number. */ - int in_g_packet; /* Always part of G packet. */ - /* long size in bytes; == register_size (target_gdbarch (), regnum); - at present. */ - /* char *name; == gdbarch_register_name (target_gdbarch (), regnum); - at present. */ -}; - -struct remote_arch_state -{ - explicit remote_arch_state (struct gdbarch *gdbarch); - - /* Description of the remote protocol registers. */ - long sizeof_g_packet; - - /* Description of the remote protocol registers indexed by REGNUM - (making an array gdbarch_num_regs in size). */ - std::unique_ptr<packet_reg[]> regs; - - /* This is the size (in chars) of the first response to the ``g'' - packet. It is used as a heuristic when determining the maximum - size of memory-read and memory-write packets. A target will - typically only reserve a buffer large enough to hold the ``g'' - packet. The size does not include packet overhead (headers and - trailers). */ - long actual_register_packet_size; - - /* This is the maximum size (in chars) of a non read/write packet. - It is also used as a cap on the size of read/write packets. */ - long remote_packet_size; -}; - /* Utility: generate error from an incoming stub packet. */ static void trace_error (char *buf) @@ -958,10 +957,15 @@ remote_get_noisy_reply () struct remote_arch_state * remote_state::get_remote_arch_state (struct gdbarch *gdbarch) { - auto &rsa = this->m_arch_states[gdbarch]; - if (rsa == nullptr) + remote_arch_state *rsa; + + auto it = this->m_arch_states.find (gdbarch); + if (it == this->m_arch_states.end ()) { - rsa.reset (new remote_arch_state (gdbarch)); + auto p = this->m_arch_states.emplace (std::piecewise_construct, + std::forward_as_tuple (gdbarch), + std::forward_as_tuple (gdbarch)); + rsa = &p.first->second; /* Make sure that the packet buffer is plenty big enough for this architecture. */ @@ -971,7 +975,10 @@ remote_state::get_remote_arch_state (struct gdbarch *gdbarch) this->buf = (char *) xrealloc (this->buf, this->buf_size); } } - return rsa.get (); + else + rsa = &it->second; + + return rsa; } /* Fetch the global remote target state. */ -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 03/10] remote: Make readahead_cache a C++ class 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves ` (4 preceding siblings ...) 2018-05-16 14:25 ` [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects Pedro Alves @ 2018-05-16 14:25 ` Pedro Alves 2018-05-18 21:06 ` Simon Marchi 2018-05-16 14:27 ` [PATCH 04/10] remote: multiple remote_arch_state instances per arch Pedro Alves ` (3 subsequent siblings) 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:25 UTC (permalink / raw) To: gdb-patches The idea here is eliminate the get_remote_state calls from within readahead_cache_invalidate, readahead_cache_invalidate_fd, remote_hostio_pread_from_cache by making those functions be class methods instead. Later on we'll have one readahead_cache instance per remote connection, and this change makes that easier. gdb/ChangeLog: 2018-05-07 Pedro Alves <palves@redhat.com> * remote.c (struct readahead_cache) <invalidate, invalidate_fd, pread>: New method declarations. (remote_target::open_1): Adjust. (readahead_cache_invalidate): Rename to ... (readahead_cache::invalidate): ... this, and adjust to be a class method. (readahead_cache_invalidate_fd): Rename to ... (readahead_cache::invalidate_fd): ... this, and adjust to be a class method. (remote_hostio_pwrite): Adjust. (remote_hostio_pread_from_cache): Rename to ... (readahead_cache::pread): ... this, and adjust to be a class method. (remote_hostio_close): Adjust. --- gdb/remote.c | 70 ++++++++++++++++++++++++++++++------------------------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index bc4815c67e..a416d9285b 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -518,8 +518,6 @@ static void remote_btrace_maybe_reopen (void); static int stop_reply_queue_length (void); -static void readahead_cache_invalidate (void); - static void remote_unpush_and_throw (void); static struct remote_state *get_remote_state (void); @@ -575,6 +573,16 @@ typedef unsigned char threadref[OPAQUETHREADBYTES]; struct readahead_cache { + /* Invalidate the readahead cache. */ + void invalidate (); + + /* Invalidate the readahead cache if it is holding data for FD. */ + void invalidate_fd (int fd); + + /* Serve pread from the readahead cache. Returns number of bytes + read, or 0 if the request can't be served from the cache. */ + int pread (int fd, gdb_byte *read_buf, size_t len, ULONGEST offset); + /* The file descriptor for the file that is being cached. -1 if the cache is invalid. */ int fd = -1; @@ -5299,7 +5307,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p) rs->use_threadinfo_query = 1; rs->use_threadextra_query = 1; - readahead_cache_invalidate (); + rs->readahead_cache.invalidate (); if (target_async_permitted) { @@ -11668,25 +11676,21 @@ remote_hostio_send_command (int command_bytes, int which_packet, return ret; } -/* Invalidate the readahead cache. */ +/* See declaration.h. */ -static void -readahead_cache_invalidate (void) +void +readahead_cache::invalidate () { - struct remote_state *rs = get_remote_state (); - - rs->readahead_cache.fd = -1; + this->fd = -1; } -/* Invalidate the readahead cache if it is holding data for FD. */ +/* See declaration.h. */ -static void -readahead_cache_invalidate_fd (int fd) +void +readahead_cache::invalidate_fd (int fd) { - struct remote_state *rs = get_remote_state (); - - if (rs->readahead_cache.fd == fd) - rs->readahead_cache.fd = -1; + if (this->fd == fd) + this->fd = -1; } /* Set the filesystem remote_hostio functions that take FILENAME @@ -11793,7 +11797,7 @@ remote_hostio_pwrite (struct target_ops *self, int left = get_remote_packet_size (); int out_len; - readahead_cache_invalidate_fd (fd); + rs->readahead_cache.invalidate_fd (fd); remote_buffer_add_string (&p, &left, "vFile:pwrite:"); @@ -11857,26 +11861,22 @@ remote_hostio_pread_vFile (struct target_ops *self, return ret; } -/* Serve pread from the readahead cache. Returns number of bytes - read, or 0 if the request can't be served from the cache. */ +/* See declaration.h. */ -static int -remote_hostio_pread_from_cache (struct remote_state *rs, - int fd, gdb_byte *read_buf, size_t len, - ULONGEST offset) +int +readahead_cache::pread (int fd, gdb_byte *read_buf, size_t len, + ULONGEST offset) { - struct readahead_cache *cache = &rs->readahead_cache; - - if (cache->fd == fd - && cache->offset <= offset - && offset < cache->offset + cache->bufsize) + if (this->fd == fd + && this->offset <= offset + && offset < this->offset + this->bufsize) { - ULONGEST max = cache->offset + cache->bufsize; + ULONGEST max = this->offset + this->bufsize; if (offset + len > max) len = max - offset; - memcpy (read_buf, cache->buf + offset - cache->offset, len); + memcpy (read_buf, this->buf + offset - this->offset, len); return len; } @@ -11892,9 +11892,9 @@ remote_hostio_pread (struct target_ops *self, { int ret; struct remote_state *rs = get_remote_state (); - struct readahead_cache *cache = &rs->readahead_cache; + readahead_cache *cache = &rs->readahead_cache; - ret = remote_hostio_pread_from_cache (rs, fd, read_buf, len, offset); + ret = cache->pread (fd, read_buf, len, offset); if (ret > 0) { cache->hit_count++; @@ -11919,12 +11919,12 @@ remote_hostio_pread (struct target_ops *self, cache->offset, remote_errno); if (ret <= 0) { - readahead_cache_invalidate_fd (fd); + cache->invalidate_fd (fd); return ret; } cache->bufsize = ret; - return remote_hostio_pread_from_cache (rs, fd, read_buf, len, offset); + return cache->pread (fd, read_buf, len, offset); } int @@ -11943,7 +11943,7 @@ remote_hostio_close (struct target_ops *self, int fd, int *remote_errno) char *p = rs->buf; int left = get_remote_packet_size () - 1; - readahead_cache_invalidate_fd (fd); + rs->readahead_cache.invalidate_fd (fd); remote_buffer_add_string (&p, &left, "vFile:close:"); -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 03/10] remote: Make readahead_cache a C++ class 2018-05-16 14:25 ` [PATCH 03/10] remote: Make readahead_cache a C++ class Pedro Alves @ 2018-05-18 21:06 ` Simon Marchi 0 siblings, 0 replies; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:06 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > The idea here is eliminate the get_remote_state calls from within > readahead_cache_invalidate, readahead_cache_invalidate_fd, > remote_hostio_pread_from_cache by making those functions be class > methods instead. Later on we'll have one readahead_cache instance per > remote connection, and this change makes that easier. LGTM. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 04/10] remote: multiple remote_arch_state instances per arch 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves ` (5 preceding siblings ...) 2018-05-16 14:25 ` [PATCH 03/10] remote: Make readahead_cache a C++ class Pedro Alves @ 2018-05-16 14:27 ` Pedro Alves 2018-05-18 21:09 ` Simon Marchi 2018-05-16 14:28 ` [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected Pedro Alves ` (2 subsequent siblings) 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:27 UTC (permalink / raw) To: gdb-patches Currently, we associate gdbarch-related remote protocol state on a per-gdbarch data object. Things like the size of the g/G packet, and the max remote packet size. If we'll support being connected to different remote servers at the same time, then we need to cope with each having their own packet sizes, even if they are each debugging programs of the same architecture. I.e., a single instance of remote_arch_state per arch is not sufficient. This patch moves the remote_arch_state object to a map of gdbarch-to-remote_arch_state saved in the remote_state structure. Usually there will only be one entry in the map, though we may see more with stubs that support multi-process and/or archs with multiple ABIs (e.g, one remote_arch_state for 64-bit inferiors and another for 32-bit inferiors). gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c: Include <unordered_map>. (remote_state): Now a class. (remote_state) <get_remote_arch_state>: Declare method. <get_remote_arch_state>: New field. (remote_arch_state) <remote_arch_state>: Declare ctor. <regs>: Now a unique_ptr. (remote_gdbarch_data_handle): Delete. (get_remote_arch_state): Delete. (remote_state::get_remote_arch_state): New. (get_remote_state): Adjust to call remote_state's get_remote_arch_state method. (init_remote_state): Delete, bits factored out to ... (remote_arch_state::remote_arch_state): ... this new method. (get_remote_packet_size, get_memory_packet_size) (process_g_packet, remote_target::fetch_registers) (remote_target::prepare_to_store, store_registers_using_G) (remote_target::store_registers, remote_target::get_trace_status): Adjust to call remote_state's method. (_initialize_remote): Remove reference to remote_gdbarch_data_handle. --- gdb/remote.c | 108 +++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 60 insertions(+), 48 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index a416d9285b..7903cb6344 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -75,6 +75,7 @@ #include "common/scoped_restore.h" #include "environ.h" #include "common/byte-vector.h" +#include <unordered_map> /* The remote target. */ @@ -606,11 +607,18 @@ struct readahead_cache connected target. This is per-target state, and independent of the selected architecture. */ -struct remote_state +class remote_state { +public: + remote_state (); ~remote_state (); + /* Get the remote arch state for GDBARCH. */ + struct remote_arch_state *get_remote_arch_state (struct gdbarch *gdbarch); + +public: /* data */ + /* A buffer to use for incoming packets, and its current size. The buffer is grown dynamically for larger incoming packets. Outgoing packets may also be constructed in this buffer. @@ -736,6 +744,14 @@ struct remote_state request/reply nature of the RSP. We only cache data for a single file descriptor at a time. */ struct readahead_cache readahead_cache; + +private: + /* Mapping of remote protocol data for each gdbarch. Usually there + is only one entry here, though we may see more with stubs that + support multi-process. */ + std::unordered_map<struct gdbarch *, + std::unique_ptr<struct remote_arch_state>> + m_arch_states; }; /* Private data that we'll store in (struct thread_info)->priv. */ @@ -817,12 +833,14 @@ struct packet_reg struct remote_arch_state { + explicit remote_arch_state (struct gdbarch *gdbarch); + /* Description of the remote protocol registers. */ long sizeof_g_packet; /* Description of the remote protocol registers indexed by REGNUM (making an array gdbarch_num_regs in size). */ - struct packet_reg *regs; + std::unique_ptr<packet_reg[]> regs; /* This is the size (in chars) of the first response to the ``g'' packet. It is used as a heuristic when determining the maximum @@ -934,15 +952,23 @@ remote_get_noisy_reply () while (1); } -/* Handle for retreving the remote protocol data from gdbarch. */ -static struct gdbarch_data *remote_gdbarch_data_handle; - -static struct remote_arch_state * -get_remote_arch_state (struct gdbarch *gdbarch) +struct remote_arch_state * +remote_state::get_remote_arch_state (struct gdbarch *gdbarch) { - gdb_assert (gdbarch != NULL); - return ((struct remote_arch_state *) - gdbarch_data (gdbarch, remote_gdbarch_data_handle)); + auto &rsa = this->m_arch_states[gdbarch]; + if (rsa == nullptr) + { + rsa.reset (new remote_arch_state (gdbarch)); + + /* Make sure that the packet buffer is plenty big enough for + this architecture. */ + if (this->buf_size < rsa->remote_packet_size) + { + this->buf_size = 2 * rsa->remote_packet_size; + this->buf = (char *) xrealloc (this->buf, this->buf_size); + } + } + return rsa.get (); } /* Fetch the global remote target state. */ @@ -950,14 +976,16 @@ get_remote_arch_state (struct gdbarch *gdbarch) static struct remote_state * get_remote_state (void) { + struct remote_state *rs = get_remote_state_raw (); + /* Make sure that the remote architecture state has been initialized, because doing so might reallocate rs->buf. Any function which calls getpkt also needs to be mindful of changes to rs->buf, but this call limits the number of places which run into trouble. */ - get_remote_arch_state (target_gdbarch ()); + rs->get_remote_arch_state (target_gdbarch ()); - return get_remote_state_raw (); + return rs; } /* Cleanup routine for the remote module's pspace data. */ @@ -1099,23 +1127,16 @@ remote_register_number_and_offset (struct gdbarch *gdbarch, int regnum, return *pnum != -1; } -static void * -init_remote_state (struct gdbarch *gdbarch) +remote_arch_state::remote_arch_state (struct gdbarch *gdbarch) { - struct remote_state *rs = get_remote_state_raw (); - struct remote_arch_state *rsa; - - rsa = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct remote_arch_state); - /* Use the architecture to build a regnum<->pnum table, which will be 1:1 unless a feature set specifies otherwise. */ - rsa->regs = GDBARCH_OBSTACK_CALLOC (gdbarch, - gdbarch_num_regs (gdbarch), - struct packet_reg); + this->regs.reset (new packet_reg [gdbarch_num_regs (gdbarch)] ()); /* Record the maximum possible size of the g packet - it may turn out to be smaller. */ - rsa->sizeof_g_packet = map_regcache_remote_table (gdbarch, rsa->regs); + this->sizeof_g_packet + = map_regcache_remote_table (gdbarch, this->regs.get ()); /* Default maximum number of characters in a packet body. Many remote stubs have a hardwired buffer size of 400 bytes @@ -1124,10 +1145,10 @@ init_remote_state (struct gdbarch *gdbarch) NUL character can always fit in the buffer. This stops GDB trashing stubs that try to squeeze an extra NUL into what is already a full buffer (As of 1999-12-04 that was most stubs). */ - rsa->remote_packet_size = 400 - 1; + this->remote_packet_size = 400 - 1; /* This one is filled in when a ``g'' packet is received. */ - rsa->actual_register_packet_size = 0; + this->actual_register_packet_size = 0; /* Should rsa->sizeof_g_packet needs more space than the default, adjust the size accordingly. Remember that each byte is @@ -1135,18 +1156,8 @@ init_remote_state (struct gdbarch *gdbarch) header / footer. NOTE: cagney/1999-10-26: I suspect that 8 (``$NN:G...#NN'') is a better guess, the below has been padded a little. */ - if (rsa->sizeof_g_packet > ((rsa->remote_packet_size - 32) / 2)) - rsa->remote_packet_size = (rsa->sizeof_g_packet * 2 + 32); - - /* Make sure that the packet buffer is plenty big enough for - this architecture. */ - if (rs->buf_size < rsa->remote_packet_size) - { - rs->buf_size = 2 * rsa->remote_packet_size; - rs->buf = (char *) xrealloc (rs->buf, rs->buf_size); - } - - return rsa; + if (this->sizeof_g_packet > ((this->remote_packet_size - 32) / 2)) + this->remote_packet_size = (this->sizeof_g_packet * 2 + 32); } /* Return the current allowed size of a remote packet. This is @@ -1156,7 +1167,7 @@ static long get_remote_packet_size (void) { struct remote_state *rs = get_remote_state (); - remote_arch_state *rsa = get_remote_arch_state (target_gdbarch ()); + remote_arch_state *rsa = rs->get_remote_arch_state (target_gdbarch ()); if (rs->explicit_packet_size) return rs->explicit_packet_size; @@ -1324,7 +1335,7 @@ static long get_memory_packet_size (struct memory_packet_config *config) { struct remote_state *rs = get_remote_state (); - remote_arch_state *rsa = get_remote_arch_state (target_gdbarch ()); + remote_arch_state *rsa = rs->get_remote_arch_state (target_gdbarch ()); long what_they_get; if (config->fixed_p) @@ -7246,7 +7257,7 @@ Packet: '%s'\n"), } event->arch = inf->gdbarch; - rsa = get_remote_arch_state (event->arch); + rsa = event->rs->get_remote_arch_state (event->arch); } packet_reg *reg @@ -7840,7 +7851,7 @@ process_g_packet (struct regcache *regcache) { struct gdbarch *gdbarch = regcache->arch (); struct remote_state *rs = get_remote_state (); - remote_arch_state *rsa = get_remote_arch_state (gdbarch); + remote_arch_state *rsa = rs->get_remote_arch_state (gdbarch); int i, buf_len; char *p; char *regs; @@ -7974,7 +7985,8 @@ void remote_target::fetch_registers (struct regcache *regcache, int regnum) { struct gdbarch *gdbarch = regcache->arch (); - remote_arch_state *rsa = get_remote_arch_state (gdbarch); + struct remote_state *rs = get_remote_state (); + remote_arch_state *rsa = rs->get_remote_arch_state (gdbarch); int i; set_remote_traceframe (); @@ -8024,7 +8036,8 @@ remote_target::fetch_registers (struct regcache *regcache, int regnum) void remote_target::prepare_to_store (struct regcache *regcache) { - remote_arch_state *rsa = get_remote_arch_state (regcache->arch ()); + struct remote_state *rs = get_remote_state (); + remote_arch_state *rsa = rs->get_remote_arch_state (regcache->arch ()); int i; /* Make sure the entire registers array is valid. */ @@ -8090,7 +8103,7 @@ static void store_registers_using_G (const struct regcache *regcache) { struct remote_state *rs = get_remote_state (); - remote_arch_state *rsa = get_remote_arch_state (regcache->arch ()); + remote_arch_state *rsa = rs->get_remote_arch_state (regcache->arch ()); gdb_byte *regs; char *p; @@ -8129,7 +8142,8 @@ void remote_target::store_registers (struct regcache *regcache, int regnum) { struct gdbarch *gdbarch = regcache->arch (); - remote_arch_state *rsa = get_remote_arch_state (gdbarch); + struct remote_state *rs = get_remote_state (); + remote_arch_state *rsa = rs->get_remote_arch_state (gdbarch); int i; set_remote_traceframe (); @@ -12906,7 +12920,7 @@ remote_target::get_trace_status (struct trace_status *ts) return -1; trace_regblock_size - = get_remote_arch_state (target_gdbarch ())->sizeof_g_packet; + = rs->get_remote_arch_state (target_gdbarch ())->sizeof_g_packet; putpkt ("qTStatus"); @@ -14026,8 +14040,6 @@ _initialize_remote (void) const char *cmd_name; /* architecture specific data */ - remote_gdbarch_data_handle = - gdbarch_data_register_post_init (init_remote_state); remote_g_packet_data_handle = gdbarch_data_register_pre_init (remote_g_packet_data_init); -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 04/10] remote: multiple remote_arch_state instances per arch 2018-05-16 14:27 ` [PATCH 04/10] remote: multiple remote_arch_state instances per arch Pedro Alves @ 2018-05-18 21:09 ` Simon Marchi 0 siblings, 0 replies; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:09 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > Currently, we associate gdbarch-related remote protocol state on a > per-gdbarch data object. Things like the size of the g/G packet, and > the max remote packet size. If we'll support being connected to > different remote servers at the same time, then we need to cope with > each having their own packet sizes, even if they are each debugging > programs of the same architecture. I.e., a single instance of > remote_arch_state per arch is not sufficient. > > This patch moves the remote_arch_state object to a map of > gdbarch-to-remote_arch_state saved in the remote_state structure. > Usually there will only be one entry in the map, though we may see > more with stubs that support multi-process and/or archs with multiple > ABIs (e.g, one remote_arch_state for 64-bit inferiors and another for > 32-bit inferiors). LGTM. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves ` (6 preceding siblings ...) 2018-05-16 14:27 ` [PATCH 04/10] remote: multiple remote_arch_state instances per arch Pedro Alves @ 2018-05-16 14:28 ` Pedro Alves 2018-05-18 21:42 ` Simon Marchi 2018-05-16 15:46 ` [PATCH 07/10] remote: Move discard_pending_stop_replies call Pedro Alves 2018-05-16 15:50 ` [PATCH 06/10] remote: Small cleanup in compare_section_command Pedro Alves 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 14:28 UTC (permalink / raw) To: gdb-patches Currently "show remote memory-write-packet-size" says that the packet size is limited to whatever is stored in the remote_state global, even if not connected to a target. When we get to support multiple instances of remote targets, there won't be a remote_state global anymore, so that must be replaced by something else. Since it doesn't make sense to print the limit of the packet size of a non-existing connection, this patch makes us say that the limit will be further reduced when we connect. The text is taken from the command's online help, which says: "The actual limit is further reduced dependent on the target." gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (get_fixed_memory_packet_size): New. (get_memory_packet_size): Use it. (show_memory_packet_size): Use it. Don't refer to get_memory_packet_size if not connected to a remote target. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * gdb.base/remote.exp: Adjust expected output of "show remote memory-write-packet-size". --- gdb/remote.c | 34 +++++++++++++++++++++++++--------- gdb/testsuite/gdb.base/remote.exp | 6 +++--- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 230288b727..7ec8d24a3b 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1333,6 +1333,19 @@ struct memory_packet_config can write at least one byte. */ #define MIN_MEMORY_PACKET_SIZE 20 +/* Get the memory packet size, assuming it is fixed. */ + +static long +get_fixed_memory_packet_size (struct memory_packet_config *config) +{ + gdb_assert (config->fixed_p); + + if (config->size <= 0) + return DEFAULT_MAX_MEMORY_PACKET_SIZE; + else + return config->size; +} + /* Compute the current size of a read/write packet. Since this makes use of ``actual_register_packet_size'' the computation is dynamic. */ @@ -1344,12 +1357,7 @@ get_memory_packet_size (struct memory_packet_config *config) long what_they_get; if (config->fixed_p) - { - if (config->size <= 0) - what_they_get = DEFAULT_MAX_MEMORY_PACKET_SIZE; - else - what_they_get = config->size; - } + what_they_get = get_fixed_memory_packet_size (config); else { what_they_get = get_remote_packet_size (); @@ -1432,10 +1440,18 @@ show_memory_packet_size (struct memory_packet_config *config) printf_filtered (_("The %s is %ld. "), config->name, config->size); if (config->fixed_p) printf_filtered (_("Packets are fixed at %ld bytes.\n"), - get_memory_packet_size (config)); + get_fixed_memory_packet_size (config)); else - printf_filtered (_("Packets are limited to %ld bytes.\n"), - get_memory_packet_size (config)); + { + struct remote_state *rs = get_remote_state (); + + if (rs->remote_desc != NULL) + printf_filtered (_("Packets are limited to %ld bytes.\n"), + get_memory_packet_size (config)); + else + puts_filtered ("The actual limit will be further reduced " + "dependent on the target.\n"); + } } static struct memory_packet_config memory_write_packet_config = diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp index 26361af9a5..4f0eb10ea4 100644 --- a/gdb/testsuite/gdb.base/remote.exp +++ b/gdb/testsuite/gdb.base/remote.exp @@ -35,7 +35,7 @@ if {$result != "" } then { # gdb_test "show remote memory-write-packet-size" \ - "The memory-write-packet-size is 0. Packets are limited to \[0-9\]+ bytes." \ + "The memory-write-packet-size is 0. The actual limit will be further reduced dependent on the target\." \ "write-packet default" gdb_test "set remote memory-write-packet-size" \ @@ -44,12 +44,12 @@ gdb_test "set remote memory-write-packet-size" \ gdb_test_no_output "set remote memory-write-packet-size 20" gdb_test "show remote memory-write-packet-size" \ - "The memory-write-packet-size is 20. Packets are limited to 20 bytes." \ + "The memory-write-packet-size is 20. The actual limit will be further reduced dependent on the target\." \ "set write-packet - small" gdb_test_no_output "set remote memory-write-packet-size 1" gdb_test "show remote memory-write-packet-size" \ - "The memory-write-packet-size is 1. Packets are limited to 20 bytes." \ + "The memory-write-packet-size is 1. The actual limit will be further reduced dependent on the target\." \ "set write-packet - very-small" # -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected 2018-05-16 14:28 ` [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected Pedro Alves @ 2018-05-18 21:42 ` Simon Marchi 2018-05-21 20:41 ` Pedro Alves 0 siblings, 1 reply; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:42 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > Currently "show remote memory-write-packet-size" says that the packet > size is limited to whatever is stored in the remote_state global, even > if not connected to a target. > > When we get to support multiple instances of remote targets, there > won't be a remote_state global anymore, so that must be replaced by > something else. > > Since it doesn't make sense to print the limit of the packet size of a > non-existing connection, this patch makes us say that the limit will > be further reduced when we connect. > > The text is taken from the command's online help, which says: > > "The actual limit is further reduced dependent on the target." The result sounds a bit weird: (gdb) show remote memory-read-packet-size The memory-read-packet-size is 0. The actual limit will be further reduced dependent on the target. (gdb) show remote memory-write-packet-size The memory-write-packet-size is 0. The actual limit will be further reduced dependent on the target. How can the limit be reduced if it is zero? I don't really know about this code, is zero a special value that means no limit? Perhaps it should be handled differently to make the message clearer. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected 2018-05-18 21:42 ` Simon Marchi @ 2018-05-21 20:41 ` Pedro Alves 2018-05-22 3:37 ` Simon Marchi 2018-05-22 21:55 ` Sergio Durigan Junior 0 siblings, 2 replies; 35+ messages in thread From: Pedro Alves @ 2018-05-21 20:41 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 05/18/2018 10:26 PM, Simon Marchi wrote: > On 2018-05-16 10:18 AM, Pedro Alves wrote: >> Currently "show remote memory-write-packet-size" says that the packet >> size is limited to whatever is stored in the remote_state global, even >> if not connected to a target. >> >> When we get to support multiple instances of remote targets, there >> won't be a remote_state global anymore, so that must be replaced by >> something else. >> >> Since it doesn't make sense to print the limit of the packet size of a >> non-existing connection, this patch makes us say that the limit will >> be further reduced when we connect. >> >> The text is taken from the command's online help, which says: >> >> "The actual limit is further reduced dependent on the target." > > The result sounds a bit weird: > > (gdb) show remote memory-read-packet-size > The memory-read-packet-size is 0. The actual limit will be further reduced dependent on the target. > (gdb) show remote memory-write-packet-size > The memory-write-packet-size is 0. The actual limit will be further reduced dependent on the target. > > How can the limit be reduced if it is zero? I don't really know about > this code, is zero a special value that means no limit? Perhaps it should be > handled differently to make the message clearer. Yeah, I think the command itself it pretty obscure/weird. "0" means "default packet size". I.e., whatever packet size the remote can handle. And if you do "set remote memory-write-packet fixed", then GDB picks an arbitrary default... I've spent a few hours today staring at this, trying to come up with something reasonable-ish, and this is what I came up with. I even caught a bug. I don't think it's worth it to polish this much more, as this is a pretty obscure command that most probably nobody uses nowadays. WDYT? ------------------ From dcca1d189d4e3bdd21af369fa8dc5b450d3effad Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Mon, 21 May 2018 19:46:43 +0100 Subject: [PATCH] Handle "show remote memory-write-packet-size" when not connected Currently "show remote memory-write-packet-size" says that the packet size is limited to whatever is stored in the remote_state global, even if not connected to a target. When we get to support multiple instances of remote targets, there won't be a remote_state global anymore, so that must be replaced by something else. Since it doesn't make sense to print the limit of the packet size of a non-existing connection, this patch makes us say that the limit will be further reduced when we connect. The text is taken from the command's online help, which says: "The actual limit is further reduced dependent on the target." Note that a value of "0" is special, as per "help set remote memory-write-packet-size": ~~~ Specify the number of bytes in a packet or 0 (zero) for the default packet size. ~~~ I've tweaked "show remote memory-write-packet-size" to include "(default)" in the output in that case, like this: (gdb) show remote memory-write-packet-size The memory-write-packet-size is 0 (default). The actual limit will be further reduced dependent on the target. While working on this, I noticed that an explicit "set remote write-packet-size 0" does not makes GDB go back to the exact same state as the default state when GDB starts up: (gdb) show remote memory-write-packet-size The memory-write-packet-size is 0. [...] ^^ (gdb) set remote memory-write-packet-size 0 (gdb) show remote memory-write-packet-size The memory-write-packet-size is 16384. [...] ^^^^^ The "16384" number comes from DEFAULT_MAX_MEMORY_PACKET_SIZE. I think this happens because git commit a5c0808e221c ("gdb: remove packet size limit") at <https://sourceware.org/ml/gdb-patches/2015-08/msg00743.html>, added this: /* So that the query shows the correct value. */ if (size <= 0) size = DEFAULT_MAX_MEMORY_PACKET_SIZE; to set_memory_packet_size, but despite what the comment suggests, that also has the side-effect of recording DEFAULT_MAX_MEMORY_PACKET_SIZE in config->size. Finally, DEFAULT_MAX_MEMORY_PACKET_SIZE only makes sense for "set remote memory-write-packet-size fixed", so I've renamed it accordingly, to make it a little bit clearer. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE): Rename to ... (DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED): ... this. (get_fixed_memory_packet_size): New. (get_memory_packet_size): Use it. (set_memory_packet_size): Don't override the config size with DEFAULT_MAX_MEMORY_PACKET_SIZE. (show_memory_packet_size): Use get_fixed_memory_packet_size. Don't refer to get_memory_packet_size if not connected to a remote target. Show "(default)" if configured size is 0. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * gdb.base/remote.exp: Adjust expected output of "show remote memory-write-packet-size". Add tests for "set remote memory-write-packet-size 0" and "set remote memory-write-packet-size fixed/limit". --- gdb/remote.c | 60 ++++++++++++++++++++++++++------------- gdb/testsuite/gdb.base/remote.exp | 30 ++++++++++++++++++-- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 59880a93a8..72254dba31 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1328,16 +1328,29 @@ struct memory_packet_config int fixed_p; }; -/* The default max memory-write-packet-size. The 16k is historical. - (It came from older GDB's using alloca for buffers and the - knowledge (folklore?) that some hosts don't cope very well with - large alloca calls.) */ -#define DEFAULT_MAX_MEMORY_PACKET_SIZE 16384 +/* The default max memory-write-packet-size, when the setting is + "fixed". The 16k is historical. (It came from older GDB's using + alloca for buffers and the knowledge (folklore?) that some hosts + don't cope very well with large alloca calls.) */ +#define DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED 16384 /* The minimum remote packet size for memory transfers. Ensures we can write at least one byte. */ #define MIN_MEMORY_PACKET_SIZE 20 +/* Get the memory packet size, assuming it is fixed. */ + +static long +get_fixed_memory_packet_size (struct memory_packet_config *config) +{ + gdb_assert (config->fixed_p); + + if (config->size <= 0) + return DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED; + else + return config->size; +} + /* Compute the current size of a read/write packet. Since this makes use of ``actual_register_packet_size'' the computation is dynamic. */ @@ -1349,12 +1362,7 @@ get_memory_packet_size (struct memory_packet_config *config) long what_they_get; if (config->fixed_p) - { - if (config->size <= 0) - what_they_get = DEFAULT_MAX_MEMORY_PACKET_SIZE; - else - what_they_get = config->size; - } + what_they_get = get_fixed_memory_packet_size (config); else { what_they_get = get_remote_packet_size (); @@ -1414,16 +1422,17 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config) something arbitrarily large. */ } - /* So that the query shows the correct value. */ - if (size <= 0) - size = DEFAULT_MAX_MEMORY_PACKET_SIZE; - /* Extra checks? */ if (fixed_p && !config->fixed_p) { + /* So that the query shows the correct value. */ + long query_size = (size <= 0 + ? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED + : size); + if (! query (_("The target may not be able to correctly handle a %s\n" "of %ld bytes. Change the packet size? "), - config->name, size)) + config->name, query_size)) error (_("Packet size not changed.")); } /* Update the config. */ @@ -1434,13 +1443,24 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config) static void show_memory_packet_size (struct memory_packet_config *config) { - printf_filtered (_("The %s is %ld. "), config->name, config->size); + if (config->size == 0) + printf_filtered (_("The %s is 0 (default). "), config->name); + else + printf_filtered (_("The %s is %ld. "), config->name, config->size); if (config->fixed_p) printf_filtered (_("Packets are fixed at %ld bytes.\n"), - get_memory_packet_size (config)); + get_fixed_memory_packet_size (config)); else - printf_filtered (_("Packets are limited to %ld bytes.\n"), - get_memory_packet_size (config)); + { + struct remote_state *rs = get_remote_state (); + + if (rs->remote_desc != NULL) + printf_filtered (_("Packets are limited to %ld bytes.\n"), + get_memory_packet_size (config)); + else + puts_filtered ("The actual limit will be further reduced " + "dependent on the target.\n"); + } } static struct memory_packet_config memory_write_packet_config = diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp index 26361af9a5..ba34441af2 100644 --- a/gdb/testsuite/gdb.base/remote.exp +++ b/gdb/testsuite/gdb.base/remote.exp @@ -35,7 +35,7 @@ if {$result != "" } then { # gdb_test "show remote memory-write-packet-size" \ - "The memory-write-packet-size is 0. Packets are limited to \[0-9\]+ bytes." \ + "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \ "write-packet default" gdb_test "set remote memory-write-packet-size" \ @@ -44,14 +44,38 @@ gdb_test "set remote memory-write-packet-size" \ gdb_test_no_output "set remote memory-write-packet-size 20" gdb_test "show remote memory-write-packet-size" \ - "The memory-write-packet-size is 20. Packets are limited to 20 bytes." \ + "The memory-write-packet-size is 20. The actual limit will be further reduced dependent on the target\." \ "set write-packet - small" gdb_test_no_output "set remote memory-write-packet-size 1" gdb_test "show remote memory-write-packet-size" \ - "The memory-write-packet-size is 1. Packets are limited to 20 bytes." \ + "The memory-write-packet-size is 1. The actual limit will be further reduced dependent on the target\." \ "set write-packet - very-small" +gdb_test_no_output "set remote memory-write-packet-size 0" +gdb_test "show remote memory-write-packet-size" \ + "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \ + "write-packet default again" + +set test "set remote memory-write-packet-size fixed" +gdb_test_multiple $test $test { + -re "Change the packet size. .y or n. " { + gdb_test_multiple "y" $test { + -re "$gdb_prompt $" { + pass $test + } + } + } +} +gdb_test "show remote memory-write-packet-size" \ + "The memory-write-packet-size is 0 \\(default\\). Packets are fixed at 16384 bytes\." \ + "write-packet default fixed" + +gdb_test_no_output "set remote memory-write-packet-size limit" +gdb_test "show remote memory-write-packet-size" \ + "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \ + "write-packet default limit again" + # # Part TWO: Check the download behavior. # -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected 2018-05-21 20:41 ` Pedro Alves @ 2018-05-22 3:37 ` Simon Marchi 2018-05-22 21:55 ` Sergio Durigan Junior 1 sibling, 0 replies; 35+ messages in thread From: Simon Marchi @ 2018-05-22 3:37 UTC (permalink / raw) To: Pedro Alves, Simon Marchi, gdb-patches On 2018-05-21 03:47 PM, Pedro Alves wrote: > Yeah, I think the command itself it pretty obscure/weird. > > "0" means "default packet size". I.e., whatever packet size > the remote can handle. And if you do > "set remote memory-write-packet fixed", then GDB picks > an arbitrary default... > I've spent a few hours today staring at this, trying to > come up with something reasonable-ish, and this is what > I came up with. I even caught a bug. I don't think it's worth > it to polish this much more, as this is a pretty obscure > command that most probably nobody uses nowadays. > > WDYT? This is completely reasonnable, thanks. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected 2018-05-21 20:41 ` Pedro Alves 2018-05-22 3:37 ` Simon Marchi @ 2018-05-22 21:55 ` Sergio Durigan Junior 2018-05-22 23:26 ` [pushed] Fix gdb.base/remote.exp with native-extended-gdbserver board (Re: [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected) Pedro Alves 1 sibling, 1 reply; 35+ messages in thread From: Sergio Durigan Junior @ 2018-05-22 21:55 UTC (permalink / raw) To: Pedro Alves; +Cc: Simon Marchi, gdb-patches On Monday, May 21 2018, Pedro Alves wrote: > On 05/18/2018 10:26 PM, Simon Marchi wrote: >> On 2018-05-16 10:18 AM, Pedro Alves wrote: >>> Currently "show remote memory-write-packet-size" says that the packet >>> size is limited to whatever is stored in the remote_state global, even >>> if not connected to a target. >>> >>> When we get to support multiple instances of remote targets, there >>> won't be a remote_state global anymore, so that must be replaced by >>> something else. >>> >>> Since it doesn't make sense to print the limit of the packet size of a >>> non-existing connection, this patch makes us say that the limit will >>> be further reduced when we connect. >>> >>> The text is taken from the command's online help, which says: >>> >>> "The actual limit is further reduced dependent on the target." >> >> The result sounds a bit weird: >> >> (gdb) show remote memory-read-packet-size >> The memory-read-packet-size is 0. The actual limit will be further reduced dependent on the target. >> (gdb) show remote memory-write-packet-size >> The memory-write-packet-size is 0. The actual limit will be further reduced dependent on the target. >> >> How can the limit be reduced if it is zero? I don't really know about >> this code, is zero a special value that means no limit? Perhaps it should be >> handled differently to make the message clearer. > > Yeah, I think the command itself it pretty obscure/weird. > > "0" means "default packet size". I.e., whatever packet size > the remote can handle. And if you do > "set remote memory-write-packet fixed", then GDB picks > an arbitrary default... > I've spent a few hours today staring at this, trying to > come up with something reasonable-ish, and this is what > I came up with. I even caught a bug. I don't think it's worth > it to polish this much more, as this is a pretty obscure > command that most probably nobody uses nowadays. > > WDYT? I'm seeing a few failures on Debian-s390x-native-extended-gdbserver-m64: PASS -> FAIL: gdb.base/remote.exp: write-packet default PASS -> FAIL: gdb.base/remote.exp: set write-packet - small PASS -> FAIL: gdb.base/remote.exp: set write-packet - very-small new FAIL: gdb.base/remote.exp: write-packet default again new FAIL: gdb.base/remote.exp: write-packet default limit again PASS -> FAIL: gdb.reverse/s390-mvcle.exp: check MVCLE record 2 PASS -> FAIL: gdb.reverse/s390-mvcle.exp: check MVCLE record 3 PASS -> KFAIL: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: inferior 1 exited gdb.log shows: ... show remote memory-write-packet-size The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes. (gdb) FAIL: gdb.base/remote.exp: write-packet default set remote memory-write-packet-size Argument required (integer, `fixed' or `limited'). (gdb) PASS: gdb.base/remote.exp: set write-packet - NULL set remote memory-write-packet-size 20 (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size 20 show remote memory-write-packet-size The memory-write-packet-size is 20. Packets are limited to 20 bytes. (gdb) FAIL: gdb.base/remote.exp: set write-packet - small set remote memory-write-packet-size 1 (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size 1 show remote memory-write-packet-size The memory-write-packet-size is 1. Packets are limited to 20 bytes. (gdb) FAIL: gdb.base/remote.exp: set write-packet - very-small set remote memory-write-packet-size 0 (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size 0 show remote memory-write-packet-size The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes. (gdb) FAIL: gdb.base/remote.exp: write-packet default again set remote memory-write-packet-size fixed The target may not be able to correctly handle a memory-write-packet-size of 16384 bytes. Change the packet size? (y or n) y (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size fixed show remote memory-write-packet-size The memory-write-packet-size is 0 (default). Packets are fixed at 16384 bytes. (gdb) PASS: gdb.base/remote.exp: write-packet default fixed set remote memory-write-packet-size limit (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size limit show remote memory-write-packet-size The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes. (gdb) FAIL: gdb.base/remote.exp: write-packet default limit again set remote memory-write-packet-size 398 (gdb) PASS: gdb.base/remote.exp: timed download `remote' - limit, 398 - set packet size set remote memory-write-packet-size limit ... This is on a try build; unfortunately our BuildBot has a very big queue right now and the "official" commit hasn't been tested yet. Please let me know if you need more info. I haven't actually tested to see if this was the exact commit that caused the regression, but it seems to be the one. > ------------------ > From dcca1d189d4e3bdd21af369fa8dc5b450d3effad Mon Sep 17 00:00:00 2001 > From: Pedro Alves <palves@redhat.com> > Date: Mon, 21 May 2018 19:46:43 +0100 > Subject: [PATCH] Handle "show remote memory-write-packet-size" when not > connected > > Currently "show remote memory-write-packet-size" says that the packet > size is limited to whatever is stored in the remote_state global, even > if not connected to a target. > > When we get to support multiple instances of remote targets, there > won't be a remote_state global anymore, so that must be replaced by > something else. > > Since it doesn't make sense to print the limit of the packet size of a > non-existing connection, this patch makes us say that the limit will > be further reduced when we connect. > > The text is taken from the command's online help, which says: > > "The actual limit is further reduced dependent on the target." > > Note that a value of "0" is special, as per "help set remote > memory-write-packet-size": > > ~~~ > Specify the number of bytes in a packet or 0 (zero) for the > default packet size. > ~~~ > > I've tweaked "show remote memory-write-packet-size" to include > "(default)" in the output in that case, like this: > > (gdb) show remote memory-write-packet-size > The memory-write-packet-size is 0 (default). The actual limit will be further reduced dependent on the target. > > While working on this, I noticed that an explicit "set remote > write-packet-size 0" does not makes GDB go back to the exact same > state as the default state when GDB starts up: > > (gdb) show remote memory-write-packet-size > The memory-write-packet-size is 0. [...] > ^^ > > (gdb) set remote memory-write-packet-size 0 > (gdb) show remote memory-write-packet-size > The memory-write-packet-size is 16384. [...] > ^^^^^ > > The "16384" number comes from DEFAULT_MAX_MEMORY_PACKET_SIZE. > > I think this happens because git commit a5c0808e221c ("gdb: remove > packet size limit") at > <https://sourceware.org/ml/gdb-patches/2015-08/msg00743.html>, added > this: > > /* So that the query shows the correct value. */ > if (size <= 0) > size = DEFAULT_MAX_MEMORY_PACKET_SIZE; > > to set_memory_packet_size, but despite what the comment suggests, that > also has the side-effect of recording DEFAULT_MAX_MEMORY_PACKET_SIZE > in config->size. > > Finally, DEFAULT_MAX_MEMORY_PACKET_SIZE only makes sense for "set > remote memory-write-packet-size fixed", so I've renamed it > accordingly, to make it a little bit clearer. > > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves <palves@redhat.com> > > * remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE): Rename to ... > (DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED): ... this. > (get_fixed_memory_packet_size): New. > (get_memory_packet_size): Use it. > (set_memory_packet_size): Don't override the config size with > DEFAULT_MAX_MEMORY_PACKET_SIZE. > (show_memory_packet_size): Use get_fixed_memory_packet_size. > Don't refer to get_memory_packet_size if not connected to a remote > target. Show "(default)" if configured size is 0. > > gdb/testsuite/ChangeLog: > yyyy-mm-dd Pedro Alves <palves@redhat.com> > > * gdb.base/remote.exp: Adjust expected output of "show remote > memory-write-packet-size". Add tests for "set remote > memory-write-packet-size 0" and "set remote > memory-write-packet-size fixed/limit". > --- > gdb/remote.c | 60 ++++++++++++++++++++++++++------------- > gdb/testsuite/gdb.base/remote.exp | 30 ++++++++++++++++++-- > 2 files changed, 67 insertions(+), 23 deletions(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index 59880a93a8..72254dba31 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -1328,16 +1328,29 @@ struct memory_packet_config > int fixed_p; > }; > > -/* The default max memory-write-packet-size. The 16k is historical. > - (It came from older GDB's using alloca for buffers and the > - knowledge (folklore?) that some hosts don't cope very well with > - large alloca calls.) */ > -#define DEFAULT_MAX_MEMORY_PACKET_SIZE 16384 > +/* The default max memory-write-packet-size, when the setting is > + "fixed". The 16k is historical. (It came from older GDB's using > + alloca for buffers and the knowledge (folklore?) that some hosts > + don't cope very well with large alloca calls.) */ > +#define DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED 16384 > > /* The minimum remote packet size for memory transfers. Ensures we > can write at least one byte. */ > #define MIN_MEMORY_PACKET_SIZE 20 > > +/* Get the memory packet size, assuming it is fixed. */ > + > +static long > +get_fixed_memory_packet_size (struct memory_packet_config *config) > +{ > + gdb_assert (config->fixed_p); > + > + if (config->size <= 0) > + return DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED; > + else > + return config->size; > +} > + > /* Compute the current size of a read/write packet. Since this makes > use of ``actual_register_packet_size'' the computation is dynamic. */ > > @@ -1349,12 +1362,7 @@ get_memory_packet_size (struct memory_packet_config *config) > > long what_they_get; > if (config->fixed_p) > - { > - if (config->size <= 0) > - what_they_get = DEFAULT_MAX_MEMORY_PACKET_SIZE; > - else > - what_they_get = config->size; > - } > + what_they_get = get_fixed_memory_packet_size (config); > else > { > what_they_get = get_remote_packet_size (); > @@ -1414,16 +1422,17 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config) > something arbitrarily large. */ > } > > - /* So that the query shows the correct value. */ > - if (size <= 0) > - size = DEFAULT_MAX_MEMORY_PACKET_SIZE; > - > /* Extra checks? */ > if (fixed_p && !config->fixed_p) > { > + /* So that the query shows the correct value. */ > + long query_size = (size <= 0 > + ? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED > + : size); > + > if (! query (_("The target may not be able to correctly handle a %s\n" > "of %ld bytes. Change the packet size? "), > - config->name, size)) > + config->name, query_size)) > error (_("Packet size not changed.")); > } > /* Update the config. */ > @@ -1434,13 +1443,24 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config) > static void > show_memory_packet_size (struct memory_packet_config *config) > { > - printf_filtered (_("The %s is %ld. "), config->name, config->size); > + if (config->size == 0) > + printf_filtered (_("The %s is 0 (default). "), config->name); > + else > + printf_filtered (_("The %s is %ld. "), config->name, config->size); > if (config->fixed_p) > printf_filtered (_("Packets are fixed at %ld bytes.\n"), > - get_memory_packet_size (config)); > + get_fixed_memory_packet_size (config)); > else > - printf_filtered (_("Packets are limited to %ld bytes.\n"), > - get_memory_packet_size (config)); > + { > + struct remote_state *rs = get_remote_state (); > + > + if (rs->remote_desc != NULL) > + printf_filtered (_("Packets are limited to %ld bytes.\n"), > + get_memory_packet_size (config)); > + else > + puts_filtered ("The actual limit will be further reduced " > + "dependent on the target.\n"); > + } > } > > static struct memory_packet_config memory_write_packet_config = > diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp > index 26361af9a5..ba34441af2 100644 > --- a/gdb/testsuite/gdb.base/remote.exp > +++ b/gdb/testsuite/gdb.base/remote.exp > @@ -35,7 +35,7 @@ if {$result != "" } then { > # > > gdb_test "show remote memory-write-packet-size" \ > - "The memory-write-packet-size is 0. Packets are limited to \[0-9\]+ bytes." \ > + "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \ > "write-packet default" > > gdb_test "set remote memory-write-packet-size" \ > @@ -44,14 +44,38 @@ gdb_test "set remote memory-write-packet-size" \ > > gdb_test_no_output "set remote memory-write-packet-size 20" > gdb_test "show remote memory-write-packet-size" \ > - "The memory-write-packet-size is 20. Packets are limited to 20 bytes." \ > + "The memory-write-packet-size is 20. The actual limit will be further reduced dependent on the target\." \ > "set write-packet - small" > > gdb_test_no_output "set remote memory-write-packet-size 1" > gdb_test "show remote memory-write-packet-size" \ > - "The memory-write-packet-size is 1. Packets are limited to 20 bytes." \ > + "The memory-write-packet-size is 1. The actual limit will be further reduced dependent on the target\." \ > "set write-packet - very-small" > > +gdb_test_no_output "set remote memory-write-packet-size 0" > +gdb_test "show remote memory-write-packet-size" \ > + "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \ > + "write-packet default again" > + > +set test "set remote memory-write-packet-size fixed" > +gdb_test_multiple $test $test { > + -re "Change the packet size. .y or n. " { > + gdb_test_multiple "y" $test { > + -re "$gdb_prompt $" { > + pass $test > + } > + } > + } > +} > +gdb_test "show remote memory-write-packet-size" \ > + "The memory-write-packet-size is 0 \\(default\\). Packets are fixed at 16384 bytes\." \ > + "write-packet default fixed" > + > +gdb_test_no_output "set remote memory-write-packet-size limit" > +gdb_test "show remote memory-write-packet-size" \ > + "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \ > + "write-packet default limit again" > + > # > # Part TWO: Check the download behavior. > # > -- > 2.14.3 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* [pushed] Fix gdb.base/remote.exp with native-extended-gdbserver board (Re: [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected) 2018-05-22 21:55 ` Sergio Durigan Junior @ 2018-05-22 23:26 ` Pedro Alves 0 siblings, 0 replies; 35+ messages in thread From: Pedro Alves @ 2018-05-22 23:26 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: Simon Marchi, gdb-patches On 05/22/2018 10:43 PM, Sergio Durigan Junior wrote: > On Monday, May 21 2018, Pedro Alves wrote: > > I'm seeing a few failures on Debian-s390x-native-extended-gdbserver-m64: > ... > show remote memory-write-packet-size > The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes. > (gdb) FAIL: gdb.base/remote.exp: write-packet default > ... > Ah, I forgot testing with extended-remote. I've pushed in the fix below. Thanks! From 035522c0228be33b2124ada2f178187e7e53aa6c Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Tue, 22 May 2018 23:26:46 +0100 Subject: [PATCH] Fix gdb.base/remote.exp with native-extended-gdbserver board This fixes gdb.base/remote.exp regressions caused by the previous commit to the testcase, when tested with --target_board=native-extended-gdbserver. For example: ... show remote memory-write-packet-size The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes. (gdb) FAIL: gdb.base/remote.exp: write-packet default ... With that board, GDB connects to GDBserver at gdb_start time, so GDB is showing the actual remote/gdbserver packet size limits. Fix it using the usual "disconnect" pattern. While at it, there's no need to start GDB before compiling the testcase. gdb/testsuite/ChangeLog: 2018-05-22 Pedro Alves <palves@redhat.com> * gdb.base/remote.exp: Only gdb_start after compiling the testcase. Issue "disconnect" before testing "set remote" command defaults. Issue clean_restart before running to main. --- gdb/testsuite/ChangeLog | 6 ++++++ gdb/testsuite/gdb.base/remote.exp | 9 +++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 25df82b4e7..24955f6530 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2018-05-22 Pedro Alves <palves@redhat.com> + + * gdb.base/remote.exp: Only gdb_start after compiling the + testcase. Issue "disconnect" before testing "set remote" command + defaults. Issue clean_restart before running to main. + 2018-05-22 Pedro Alves <palves@redhat.com> * gdb.base/remote.exp: Adjust expected output of "show remote diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp index ba34441af2..edcfac824f 100644 --- a/gdb/testsuite/gdb.base/remote.exp +++ b/gdb/testsuite/gdb.base/remote.exp @@ -21,14 +21,17 @@ if {[target_info gdb_protocol] != "remote" standard_testfile .c -gdb_start - set result [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] if {$result != "" } then { untested "failed to compile" return -1 } +gdb_start + +# Make sure we're disconnected, in case we're testing with an +# extended-remote board, therefore already connected. +gdb_test "disconnect" ".*" # # Part ONE: Check the down load commands @@ -126,6 +129,8 @@ proc gdb_load_timed {executable class writesize} { pass $test } +clean_restart $binfile + # These download tests won't actually download anything on !is_remote # target boards, but we run them anyway because it's simpler, and # harmless. -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 07/10] remote: Move discard_pending_stop_replies call 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves ` (7 preceding siblings ...) 2018-05-16 14:28 ` [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected Pedro Alves @ 2018-05-16 15:46 ` Pedro Alves 2018-05-18 21:29 ` Simon Marchi 2018-05-16 15:50 ` [PATCH 06/10] remote: Small cleanup in compare_section_command Pedro Alves 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 15:46 UTC (permalink / raw) To: gdb-patches This helps because discard_pending_stop_replies will later become a method of remote_target. Otherwise, when we have multiple instances of remote_target, we'd have to make discard_pending_stop_replies find the inferior's target_ops, and upcast it to remote_target (if indeed a remote) to find the pending stop replies queue to clear. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (remote_target::mourn_inferior): Move discard_pending_stop_replies call here from ... (_initialize_remote): ... here. --- gdb/remote.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index ef15eafe2a..230288b727 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -9666,6 +9666,10 @@ remote_target::mourn_inferior () { struct remote_state *rs = get_remote_state (); + /* We're no longer interested in notification events of an inferior + that exited or was killed/detached. */ + discard_pending_stop_replies (current_inferior ()); + /* In 'target remote' mode with one inferior, we close the connection. */ if (!rs->extended && number_of_live_inferiors () <= 1) { @@ -14059,9 +14063,6 @@ _initialize_remote (void) /* Hook into new objfile notification. */ gdb::observers::new_objfile.attach (remote_new_objfile); - /* We're no longer interested in notification events of an inferior - when it exits. */ - gdb::observers::inferior_exit.attach (discard_pending_stop_replies); #if 0 init_remote_threadtests (); -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 07/10] remote: Move discard_pending_stop_replies call 2018-05-16 15:46 ` [PATCH 07/10] remote: Move discard_pending_stop_replies call Pedro Alves @ 2018-05-18 21:29 ` Simon Marchi 0 siblings, 0 replies; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:29 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > This helps because discard_pending_stop_replies will later become a > method of remote_target. Otherwise, when we have multiple instances > of remote_target, we'd have to make discard_pending_stop_replies find > the inferior's target_ops, and upcast it to remote_target (if indeed a > remote) to find the pending stop replies queue to clear. > > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves <palves@redhat.com> > > * remote.c (remote_target::mourn_inferior): Move > discard_pending_stop_replies call here from ... > (_initialize_remote): ... here. > --- > gdb/remote.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index ef15eafe2a..230288b727 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -9666,6 +9666,10 @@ remote_target::mourn_inferior () > { > struct remote_state *rs = get_remote_state (); > > + /* We're no longer interested in notification events of an inferior > + that exited or was killed/detached. */ > + discard_pending_stop_replies (current_inferior ()); > + > /* In 'target remote' mode with one inferior, we close the connection. */ > if (!rs->extended && number_of_live_inferiors () <= 1) > { > @@ -14059,9 +14063,6 @@ _initialize_remote (void) > > /* Hook into new objfile notification. */ > gdb::observers::new_objfile.attach (remote_new_objfile); > - /* We're no longer interested in notification events of an inferior > - when it exits. */ > - gdb::observers::inferior_exit.attach (discard_pending_stop_replies); > > #if 0 > init_remote_threadtests (); > LGTM. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 06/10] remote: Small cleanup in compare_section_command 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves ` (8 preceding siblings ...) 2018-05-16 15:46 ` [PATCH 07/10] remote: Move discard_pending_stop_replies call Pedro Alves @ 2018-05-16 15:50 ` Pedro Alves 2018-05-18 21:26 ` Simon Marchi 9 siblings, 1 reply; 35+ messages in thread From: Pedro Alves @ 2018-05-16 15:50 UTC (permalink / raw) To: gdb-patches The set_general_process call in compare_sections_command isn't actually needed. remote_target::verify_memory and remote_target::xfer_partial already handle making sure the remote is pointing at the right process or thread. Getting this out of the way helps a bit with further elimination of the remote_state global, because we have to handle the case of a user invoking the command even if not connect to a remote target. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * remote.c (compare_section_command): Remove set_general_process call. --- gdb/remote.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 8e1baf0856..ef15eafe2a 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -10511,9 +10511,6 @@ compare_sections_command (const char *args, int from_tty) if (!exec_bfd) error (_("command cannot be used without an exec file")); - /* Make sure the remote is pointing at the right process. */ - set_general_process (); - if (args != NULL && strcmp (args, "-r") == 0) { read_only = 1; -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 06/10] remote: Small cleanup in compare_section_command 2018-05-16 15:50 ` [PATCH 06/10] remote: Small cleanup in compare_section_command Pedro Alves @ 2018-05-18 21:26 ` Simon Marchi 0 siblings, 0 replies; 35+ messages in thread From: Simon Marchi @ 2018-05-18 21:26 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2018-05-16 10:18 AM, Pedro Alves wrote: > The set_general_process call in compare_sections_command isn't > actually needed. remote_target::verify_memory and > remote_target::xfer_partial already handle making sure the remote is > pointing at the right process or thread. > > Getting this out of the way helps a bit with further elimination of > the remote_state global, because we have to handle the case of a user > invoking the command even if not connect to a remote target. > > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves <palves@redhat.com> > > * remote.c (compare_section_command): Remove set_general_process > call. > --- > gdb/remote.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index 8e1baf0856..ef15eafe2a 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -10511,9 +10511,6 @@ compare_sections_command (const char *args, int from_tty) > if (!exec_bfd) > error (_("command cannot be used without an exec file")); > > - /* Make sure the remote is pointing at the right process. */ > - set_general_process (); > - > if (args != NULL && strcmp (args, "-r") == 0) > { > read_only = 1; > LGTM. Simon ^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2018-05-25 3:31 UTC | newest] Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-16 14:18 [PATCH 00/10] remote: More multi-target preparation Pedro Alves 2018-05-16 14:18 ` [PATCH 01/10] remote: struct remote_state, use op new Pedro Alves 2018-05-18 20:57 ` Simon Marchi 2018-05-21 15:36 ` [PATCH 1.2 01/10] remote: struct remote_state, use op new, fix leaks Pedro Alves 2018-05-16 14:18 ` [PATCH 09/10] remote: Make vcont_builder a class Pedro Alves 2018-05-22 5:07 ` Simon Marchi 2018-05-22 21:33 ` Pedro Alves 2018-05-16 14:18 ` [PATCH 10/10] remote: one struct remote_state per struct remote_target Pedro Alves 2018-05-22 5:07 ` Simon Marchi 2018-05-22 21:06 ` Pedro Alves 2018-05-24 17:00 ` [PATCH 11/10] remote_target::m_remote_state, pointer -> object (Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target) Pedro Alves 2018-05-25 5:23 ` Simon Marchi 2018-05-16 14:18 ` [PATCH 02/10] remote: Eliminate remote_hostio_close_cleanup Pedro Alves 2018-05-16 17:43 ` Tom Tromey 2018-05-16 18:53 ` Pedro Alves 2018-05-16 19:46 ` Tom Tromey 2018-05-18 21:04 ` Simon Marchi 2018-05-16 14:25 ` [PATCH 05/10] remote: remote_arch_state pointers -> remote_arch_state objects Pedro Alves 2018-05-18 21:17 ` Simon Marchi 2018-05-18 21:18 ` Simon Marchi 2018-05-21 16:12 ` Pedro Alves 2018-05-16 14:25 ` [PATCH 03/10] remote: Make readahead_cache a C++ class Pedro Alves 2018-05-18 21:06 ` Simon Marchi 2018-05-16 14:27 ` [PATCH 04/10] remote: multiple remote_arch_state instances per arch Pedro Alves 2018-05-18 21:09 ` Simon Marchi 2018-05-16 14:28 ` [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected Pedro Alves 2018-05-18 21:42 ` Simon Marchi 2018-05-21 20:41 ` Pedro Alves 2018-05-22 3:37 ` Simon Marchi 2018-05-22 21:55 ` Sergio Durigan Junior 2018-05-22 23:26 ` [pushed] Fix gdb.base/remote.exp with native-extended-gdbserver board (Re: [PATCH 08/10] Handle "show remote memory-write-packet-size" when not connected) Pedro Alves 2018-05-16 15:46 ` [PATCH 07/10] remote: Move discard_pending_stop_replies call Pedro Alves 2018-05-18 21:29 ` Simon Marchi 2018-05-16 15:50 ` [PATCH 06/10] remote: Small cleanup in compare_section_command Pedro Alves 2018-05-18 21:26 ` Simon Marchi
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).