diff options
| -rw-r--r-- | gdb/ChangeLog | 55 | ||||
| -rw-r--r-- | gdb/infcmd.c | 16 | ||||
| -rw-r--r-- | gdb/infrun.c | 226 | ||||
| -rw-r--r-- | gdb/infrun.h | 98 | ||||
| -rw-r--r-- | gdb/mi/mi-main.c | 4 | ||||
| -rw-r--r-- | gdb/process-stratum-target.h | 32 | ||||
| -rw-r--r-- | gdb/record-full.c | 10 | ||||
| -rw-r--r-- | gdb/remote.c | 149 | ||||
| -rw-r--r-- | gdb/target-delegates.c | 18 | ||||
| -rw-r--r-- | gdb/target.c | 27 | ||||
| -rw-r--r-- | gdb/target.h | 31 | ||||
| -rw-r--r-- | gdb/top.c | 7 |
12 files changed, 575 insertions, 98 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7f60d3c..e3ae1c4 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,58 @@ +2021-03-26 Simon Marchi <simon.marchi@efficios.com> + Pedro Alves <pedro@palves.net> + + * infcmd.c (run_command_1, attach_command, detach_command) + (interrupt_target_1): Use scoped_disable_commit_resumed. + * infrun.c (do_target_resume): Remove + target_commit_resume call. + (commit_resume_all_targets): Remove. + (maybe_set_commit_resumed_all_targets): New. + (maybe_call_commit_resumed_all_targets): New. + (enable_commit_resumed): New. + (scoped_disable_commit_resumed::scoped_disable_commit_resumed) + (scoped_disable_commit_resumed::~scoped_disable_commit_resumed) + (scoped_disable_commit_resumed::reset) + (scoped_disable_commit_resumed::reset_and_commit) + (scoped_enable_commit_resumed::scoped_enable_commit_resumed) + (scoped_enable_commit_resumed::~scoped_enable_commit_resumed): + New. + (proceed): Use scoped_disable_commit_resumed and + maybe_call_commit_resumed_all_targets. + (fetch_inferior_event): Use scoped_disable_commit_resumed. + * infrun.h (struct scoped_disable_commit_resumed): New. + (maybe_call_commit_resumed_all_process_targets): New. + (struct scoped_enable_commit_resumed): New. + * mi/mi-main.c (exec_continue): Use scoped_disable_commit_resumed. + * process-stratum-target.h (class process_stratum_target): + <commit_resumed_state>: New. + * record-full.c (record_full_wait_1): Change commit_resumed_state + around calling commit_resumed. + * remote.c (class remote_target) <commit_resume>: Rename to... + <commit_resumed>: ... this. + (struct stop_reply): Move up. + (remote_target::commit_resume): Rename to... + (remote_target::commit_resumed): ... this. Check if there is any + thread pending vCont resume. + (remote_target::remote_stop_ns): Generate stop replies for resumed + but pending vCont threads. + (remote_target::wait_ns): Add gdb_assert. + * target-delegates.c: Regenerate. + * target.c (target_wait, target_resume): Assert that the current + process_stratum target isn't in commit-resumed state. + (defer_target_commit_resume): Remove. + (target_commit_resume): Remove. + (target_commit_resumed): New. + (make_scoped_defer_target_commit_resume): Remove. + (target_stop): Assert that the current process_stratum target + isn't in commit-resumed state. + * target.h (struct target_ops) <commit_resume>: Rename to ... + <commit_resumed>: ... this. + (target_commit_resume): Remove. + (target_commit_resumed): New. + (make_scoped_defer_target_commit_resume): Remove. + * top.c (wait_sync_command_done): Use + scoped_enable_commit_resumed. + 2021-03-26 Pedro Alves <pedro@palves.net> * target.c (target_always_non_stop_p): Also check whether the diff --git a/gdb/infcmd.c b/gdb/infcmd.c index a6e9572..9b0186d 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -419,6 +419,8 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) dont_repeat (); + scoped_disable_commit_resumed disable_commit_resumed ("running"); + kill_if_already_running (from_tty); init_wait_for_inferior (); @@ -538,6 +540,8 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) /* Since there was no error, there's no need to finish the thread states here. */ finish_state.release (); + + disable_commit_resumed.reset_and_commit (); } static void @@ -2565,6 +2569,8 @@ attach_command (const char *args, int from_tty) dont_repeat (); /* Not for the faint of heart */ + scoped_disable_commit_resumed disable_commit_resumed ("attaching"); + if (gdbarch_has_global_solist (target_gdbarch ())) /* Don't complain if all processes share the same symbol space. */ @@ -2673,6 +2679,8 @@ attach_command (const char *args, int from_tty) } else attach_post_wait (args, from_tty, mode); + + disable_commit_resumed.reset_and_commit (); } /* We had just found out that the target was already attached to an @@ -2746,6 +2754,8 @@ detach_command (const char *args, int from_tty) if (inferior_ptid == null_ptid) error (_("The program is not being run.")); + scoped_disable_commit_resumed disable_commit_resumed ("detaching"); + query_if_trace_running (from_tty); disconnect_tracing (); @@ -2779,6 +2789,8 @@ detach_command (const char *args, int from_tty) if (!was_non_stop_p) restart_after_all_stop_detach (as_process_stratum_target (target_ref.get ())); + + disable_commit_resumed.reset_and_commit (); } /* Disconnect from the current target without resuming it (leaving it @@ -2827,6 +2839,8 @@ stop_current_target_threads_ns (ptid_t ptid) void interrupt_target_1 (bool all_threads) { + scoped_disable_commit_resumed disable_commit_resumed ("interrupting"); + if (non_stop) { if (all_threads) @@ -2844,6 +2858,8 @@ interrupt_target_1 (bool all_threads) } else target_interrupt (); + + disable_commit_resumed.reset_and_commit (); } /* interrupt [-a] diff --git a/gdb/infrun.c b/gdb/infrun.c index 20035a0..347eefb 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2173,8 +2173,6 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig) target_resume (resume_ptid, step, sig); - target_commit_resume (); - if (target_can_async_p ()) target_async (1); } @@ -2761,28 +2759,208 @@ schedlock_applies (struct thread_info *tp) execution_direction))); } -/* Calls target_commit_resume on all targets. */ +/* Set process_stratum_target::COMMIT_RESUMED_STATE in all target + stacks that have threads executing and don't have threads with + pending events. */ static void -commit_resume_all_targets () +maybe_set_commit_resumed_all_targets () +{ + for (inferior *inf : all_non_exited_inferiors ()) + { + process_stratum_target *proc_target = inf->process_target (); + + if (proc_target->commit_resumed_state) + { + /* We already set this in a previous iteration, via another + inferior sharing the process_stratum target. */ + continue; + } + + /* If the target has no resumed threads, it would be useless to + ask it to commit the resumed threads. */ + if (!proc_target->threads_executing) + { + infrun_debug_printf ("not requesting commit-resumed for target " + "%s, no resumed threads", + proc_target->shortname ()); + continue; + } + + /* As an optimization, if a thread from this target has some + status to report, handle it before requiring the target to + commit its resumed threads: handling the status might lead to + resuming more threads. */ + bool has_thread_with_pending_status = false; + for (thread_info *thread : all_non_exited_threads (proc_target)) + if (thread->resumed && thread->suspend.waitstatus_pending_p) + { + has_thread_with_pending_status = true; + break; + } + + if (has_thread_with_pending_status) + { + infrun_debug_printf ("not requesting commit-resumed for target %s, a" + " thread has a pending waitstatus", + proc_target->shortname ()); + continue; + } + + infrun_debug_printf ("enabling commit-resumed for target %s", + proc_target->shortname ()); + + proc_target->commit_resumed_state = true; + } +} + +/* See infrun.h. */ + +void +maybe_call_commit_resumed_all_targets () { scoped_restore_current_thread restore_thread; - /* Map between process_target and a representative inferior. This - is to avoid committing a resume in the same target more than - once. Resumptions must be idempotent, so this is an - optimization. */ - std::unordered_map<process_stratum_target *, inferior *> conn_inf; + for (inferior *inf : all_non_exited_inferiors ()) + { + process_stratum_target *proc_target = inf->process_target (); + + if (!proc_target->commit_resumed_state) + continue; + + switch_to_inferior_no_thread (inf); + + infrun_debug_printf ("calling commit_resumed for target %s", + proc_target->shortname()); + + target_commit_resumed (); + } +} + +/* To track nesting of scoped_disable_commit_resumed objects, ensuring + that only the outermost one attempts to re-enable + commit-resumed. */ +static bool enable_commit_resumed = true; + +/* See infrun.h. */ + +scoped_disable_commit_resumed::scoped_disable_commit_resumed + (const char *reason) + : m_reason (reason), + m_prev_enable_commit_resumed (enable_commit_resumed) +{ + infrun_debug_printf ("reason=%s", m_reason); + + enable_commit_resumed = false; for (inferior *inf : all_non_exited_inferiors ()) - if (inf->has_execution ()) - conn_inf[inf->process_target ()] = inf; + { + process_stratum_target *proc_target = inf->process_target (); + + if (m_prev_enable_commit_resumed) + { + /* This is the outermost instance: force all + COMMIT_RESUMED_STATE to false. */ + proc_target->commit_resumed_state = false; + } + else + { + /* This is not the outermost instance, we expect + COMMIT_RESUMED_STATE to have been cleared by the + outermost instance. */ + gdb_assert (!proc_target->commit_resumed_state); + } + } +} - for (const auto &ci : conn_inf) +/* See infrun.h. */ + +void +scoped_disable_commit_resumed::reset () +{ + if (m_reset) + return; + m_reset = true; + + infrun_debug_printf ("reason=%s", m_reason); + + gdb_assert (!enable_commit_resumed); + + enable_commit_resumed = m_prev_enable_commit_resumed; + + if (m_prev_enable_commit_resumed) { - inferior *inf = ci.second; - switch_to_inferior_no_thread (inf); - target_commit_resume (); + /* This is the outermost instance, re-enable + COMMIT_RESUMED_STATE on the targets where it's possible. */ + maybe_set_commit_resumed_all_targets (); + } + else + { + /* This is not the outermost instance, we expect + COMMIT_RESUMED_STATE to still be false. */ + for (inferior *inf : all_non_exited_inferiors ()) + { + process_stratum_target *proc_target = inf->process_target (); + gdb_assert (!proc_target->commit_resumed_state); + } + } +} + +/* See infrun.h. */ + +scoped_disable_commit_resumed::~scoped_disable_commit_resumed () +{ + reset (); +} + +/* See infrun.h. */ + +void +scoped_disable_commit_resumed::reset_and_commit () +{ + reset (); + maybe_call_commit_resumed_all_targets (); +} + +/* See infrun.h. */ + +scoped_enable_commit_resumed::scoped_enable_commit_resumed + (const char *reason) + : m_reason (reason), + m_prev_enable_commit_resumed (enable_commit_resumed) +{ + infrun_debug_printf ("reason=%s", m_reason); + + if (!enable_commit_resumed) + { + enable_commit_resumed = true; + + /* Re-enable COMMIT_RESUMED_STATE on the targets where it's + possible. */ + maybe_set_commit_resumed_all_targets (); + + maybe_call_commit_resumed_all_targets (); + } +} + +/* See infrun.h. */ + +scoped_enable_commit_resumed::~scoped_enable_commit_resumed () +{ + infrun_debug_printf ("reason=%s", m_reason); + + gdb_assert (enable_commit_resumed); + + enable_commit_resumed = m_prev_enable_commit_resumed; + + if (!enable_commit_resumed) + { + /* Force all COMMIT_RESUMED_STATE back to false. */ + for (inferior *inf : all_non_exited_inferiors ()) + { + process_stratum_target *proc_target = inf->process_target (); + proc_target->commit_resumed_state = false; + } } } @@ -3006,7 +3184,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) cur_thr->prev_pc = regcache_read_pc_protected (regcache); { - scoped_restore save_defer_tc = make_scoped_defer_target_commit_resume (); + scoped_disable_commit_resumed disable_commit_resumed ("proceeding"); started = start_step_over (); @@ -3074,9 +3252,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) if (!ecs->wait_some_more) error (_("Command aborted.")); } - } - commit_resume_all_targets (); + disable_commit_resumed.reset_and_commit (); + } finish_state.release (); @@ -3878,8 +4056,16 @@ fetch_inferior_event () = make_scoped_restore (&execution_direction, target_execution_direction ()); + /* Allow targets to pause their resumed threads while we handle + the event. */ + scoped_disable_commit_resumed disable_commit_resumed ("handling event"); + if (!do_target_wait (minus_one_ptid, ecs, TARGET_WNOHANG)) - return; + { + infrun_debug_printf ("do_target_wait returned no event"); + disable_commit_resumed.reset_and_commit (); + return; + } gdb_assert (ecs->ws.kind != TARGET_WAITKIND_IGNORE); @@ -3970,6 +4156,8 @@ fetch_inferior_event () /* No error, don't finish the thread states yet. */ finish_state.release (); + disable_commit_resumed.reset_and_commit (); + /* This scope is used to ensure that readline callbacks are reinstalled here. */ } diff --git a/gdb/infrun.h b/gdb/infrun.h index e643c84..220ccc7 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -273,4 +273,102 @@ extern void all_uis_on_sync_execution_starting (void); detach. */ extern void restart_after_all_stop_detach (process_stratum_target *proc_target); +/* RAII object to temporarily disable the requirement for target + stacks to commit their resumed threads. + + On construction, set process_stratum_target::commit_resumed_state + to false for all process_stratum targets in all target + stacks. + + On destruction (or if reset_and_commit() is called), set + process_stratum_target::commit_resumed_state to true for all + process_stratum targets in all target stacks, except those that: + + - have no resumed threads + - have a resumed thread with a pending status + + target_commit_resumed is not called in the destructor, because its + implementations could throw, and we don't to swallow that error in + a destructor. Instead, the caller should call the + reset_and_commit_resumed() method so that an eventual exception can + propagate. "reset" in the method name refers to the fact that this + method has the same effect as the destructor, in addition to + committing resumes. + + The creation of nested scoped_disable_commit_resumed objects is + tracked, such that only the outermost instance actually does + something, for cases like this: + + void + inner_func () + { + scoped_disable_commit_resumed disable; + + // do stuff + + disable.reset_and_commit (); + } + + void + outer_func () + { + scoped_disable_commit_resumed disable; + + for (... each thread ...) + inner_func (); + + disable.reset_and_commit (); + } + + In this case, we don't want the `disable` destructor in + `inner_func` to require targets to commit resumed threads, so that + the `reset_and_commit()` call in `inner_func` doesn't actually + resume threads. */ + +struct scoped_disable_commit_resumed +{ + explicit scoped_disable_commit_resumed (const char *reason); + ~scoped_disable_commit_resumed (); + + DISABLE_COPY_AND_ASSIGN (scoped_disable_commit_resumed); + + /* Undoes the disabling done by the ctor, and calls + maybe_call_commit_resumed_all_targets(). */ + void reset_and_commit (); + +private: + /* Undoes the disabling done by the ctor. */ + void reset (); + + /* Whether this object has been reset. */ + bool m_reset = false; + + const char *m_reason; + bool m_prev_enable_commit_resumed; +}; + +/* Call target_commit_resumed method on all target stacks whose + process_stratum target layer has COMMIT_RESUME_STATE set. */ + +extern void maybe_call_commit_resumed_all_targets (); + +/* RAII object to temporarily enable the requirement for target stacks + to commit their resumed threads. This is the inverse of + scoped_disable_commit_resumed. The constructor calls the + maybe_call_commit_resumed_all_targets function itself, since it's + OK to throw from a constructor. */ + +struct scoped_enable_commit_resumed +{ + explicit scoped_enable_commit_resumed (const char *reason); + ~scoped_enable_commit_resumed (); + + DISABLE_COPY_AND_ASSIGN (scoped_enable_commit_resumed); + +private: + const char *m_reason; + bool m_prev_enable_commit_resumed; +}; + + #endif /* INFRUN_H */ diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index c6f1ab4..988db55 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -266,6 +266,8 @@ exec_continue (char **argv, int argc) { prepare_execution_command (current_inferior ()->top_target (), mi_async_p ()); + scoped_disable_commit_resumed disable_commit_resumed ("mi continue"); + if (non_stop) { /* In non-stop mode, 'resume' always resumes a single thread. @@ -311,6 +313,8 @@ exec_continue (char **argv, int argc) continue_1 (1); } } + + disable_commit_resumed.reset_and_commit (); } static void diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h index b513c26..6fddbba 100644 --- a/gdb/process-stratum-target.h +++ b/gdb/process-stratum-target.h @@ -72,6 +72,38 @@ public: /* The connection number. Visible in "info connections". */ int connection_number = 0; + + /* Whether resumed threads must be committed to the target. + + When true, resumed threads must be committed to the execution + target. + + When false, the target may leave resumed threads stopped when + it's convenient or efficient to do so. When the core requires + resumed threads to be committed again, this is set back to true + and calls the `commit_resumed` method to allow the target to do + so. + + To simplify the implementation of targets, the following methods + are guaranteed to be called with COMMIT_RESUMED_STATE set to + false: + + - resume + - stop + - wait + + Knowing this, the target doesn't need to implement different + behaviors depending on the COMMIT_RESUMED_STATE, and can simply + assume that it is false. + + Targets can take advantage of this to batch resumption requests, + for example. In that case, the target doesn't actually resume in + its `resume` implementation. Instead, it takes note of the + resumption intent in `resume` and defers the actual resumption to + `commit_resumed`. For example, the remote target uses this to + coalesce multiple resumption requests in a single vCont + packet. */ + bool commit_resumed_state = false; }; /* Downcast TARGET to process_stratum_target. */ diff --git a/gdb/record-full.c b/gdb/record-full.c index 8310b7b..23cbdcb 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -1242,11 +1242,11 @@ record_full_wait_1 (struct target_ops *ops, break; } + process_stratum_target *proc_target + = current_inferior ()->process_target (); + if (gdbarch_software_single_step_p (gdbarch)) { - process_stratum_target *proc_target - = current_inferior ()->process_target (); - /* Try to insert the software single step breakpoint. If insert success, set step to 0. */ set_executing (proc_target, inferior_ptid, false); @@ -1263,7 +1263,9 @@ record_full_wait_1 (struct target_ops *ops, "issuing one more step in the " "target beneath\n"); ops->beneath ()->resume (ptid, step, GDB_SIGNAL_0); - ops->beneath ()->commit_resume (); + proc_target->commit_resumed_state = true; + proc_target->commit_resumed (); + proc_target->commit_resumed_state = false; continue; } } diff --git a/gdb/remote.c b/gdb/remote.c index 188f507..1036ffd 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -426,7 +426,7 @@ public: void detach (inferior *, int) override; void disconnect (const char *, int) override; - void commit_resume () override; + void commit_resumed () override; void resume (ptid_t, int, enum gdb_signal) override; ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override; @@ -6499,6 +6499,36 @@ get_remote_inferior (inferior *inf) return static_cast<remote_inferior *> (inf->priv.get ()); } +struct stop_reply : public notif_event +{ + ~stop_reply (); + + /* The identifier of the thread about this event */ + ptid_t ptid; + + /* The remote state this event is associated with. When the remote + connection, represented by a remote_state object, is closed, + all the associated stop_reply events should be released. */ + struct remote_state *rs; + + struct target_waitstatus ws; + + /* The architecture associated with the expedited registers. */ + gdbarch *arch; + + /* Expedited registers. This makes remote debugging a bit more + efficient for those targets that provide critical registers as + part of their normal status mechanism (as another roundtrip to + fetch them is avoided). */ + std::vector<cached_reg_t> regcache; + + enum target_stop_reason stop_reason; + + CORE_ADDR watch_data_address; + + int core; +}; + /* 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. */ @@ -6603,7 +6633,7 @@ vcont_builder::push_action (ptid_t ptid, bool step, gdb_signal siggnal) /* to_commit_resume implementation. */ void -remote_target::commit_resume () +remote_target::commit_resumed () { int any_process_wildcard; int may_global_wildcard_vcont; @@ -6678,6 +6708,8 @@ remote_target::commit_resume () disable process and global wildcard resumes appropriately. */ check_pending_events_prevent_wildcard_vcont (&may_global_wildcard_vcont); + bool any_pending_vcont_resume = false; + for (thread_info *tp : all_non_exited_threads (this)) { remote_thread_info *priv = get_remote_thread_info (tp); @@ -6694,6 +6726,9 @@ remote_target::commit_resume () continue; } + if (priv->get_resume_state () == resume_state::RESUMED_PENDING_VCONT) + any_pending_vcont_resume = true; + /* If a thread is the parent of an unfollowed fork, then we can't do a global wildcard, as that would resume the fork child. */ @@ -6701,6 +6736,11 @@ remote_target::commit_resume () may_global_wildcard_vcont = 0; } + /* We didn't have any resumed thread pending a vCont resume, so nothing to + do. */ + if (!any_pending_vcont_resume) + return; + /* Now let's build the vCont packet(s). Actions must be appended from narrower to wider scopes (thread -> process -> global). If we end up with too many actions for a single packet vcont_builder @@ -6721,6 +6761,13 @@ remote_target::commit_resume () gdb_assert (!thread_is_in_step_over_chain (tp)); + /* We should never be commit-resuming a thread that has a stop reply. + Otherwise, we would end up reporting a stop event for a thread while + it is running on the remote target. */ + remote_state *rs = get_remote_state (); + for (const auto &stop_reply : rs->stop_reply_queue) + gdb_assert (stop_reply->ptid != tp->ptid); + const resumed_pending_vcont_info &info = remote_thr->resumed_pending_vcont_info (); @@ -6786,6 +6833,74 @@ remote_target::remote_stop_ns (ptid_t ptid) char *p = rs->buf.data (); char *endp = p + get_remote_packet_size (); + /* If any thread that needs to stop was resumed but pending a vCont + resume, generate a phony stop_reply. However, first check + whether the thread wasn't resumed with a signal. Generating a + phony stop in that case would result in losing the signal. */ + bool needs_commit = false; + for (thread_info *tp : all_non_exited_threads (this, ptid)) + { + remote_thread_info *remote_thr = get_remote_thread_info (tp); + + if (remote_thr->get_resume_state () + == resume_state::RESUMED_PENDING_VCONT) + { + const resumed_pending_vcont_info &info + = remote_thr->resumed_pending_vcont_info (); + if (info.sig != GDB_SIGNAL_0) + { + /* This signal must be forwarded to the inferior. We + could commit-resume just this thread, but its simpler + to just commit-resume everything. */ + needs_commit = true; + break; + } + } + } + + if (needs_commit) + commit_resumed (); + else + for (thread_info *tp : all_non_exited_threads (this, ptid)) + { + remote_thread_info *remote_thr = get_remote_thread_info (tp); + + if (remote_thr->get_resume_state () + == resume_state::RESUMED_PENDING_VCONT) + { + remote_debug_printf ("Enqueueing phony stop reply for thread pending " + "vCont-resume (%d, %ld, %ld)", tp->ptid.pid(), + tp->ptid.lwp (), tp->ptid.tid ()); + + /* Check that the thread wasn't resumed with a signal. + Generating a phony stop would result in losing the + signal. */ + const resumed_pending_vcont_info &info + = remote_thr->resumed_pending_vcont_info (); + gdb_assert (info.sig == GDB_SIGNAL_0); + + stop_reply *sr = new stop_reply (); + sr->ptid = tp->ptid; + sr->rs = rs; + sr->ws.kind = TARGET_WAITKIND_STOPPED; + sr->ws.value.sig = GDB_SIGNAL_0; + sr->arch = tp->inf->gdbarch; + sr->stop_reason = TARGET_STOPPED_BY_NO_REASON; + sr->watch_data_address = 0; + sr->core = 0; + this->push_stop_reply (sr); + + /* Pretend that this thread was actually resumed on the + remote target, then stopped. If we leave it in the + RESUMED_PENDING_VCONT state and the commit_resumed + method is called while the stop reply is still in the + queue, we'll end up reporting a stop event to the core + for that thread while it is running on the remote + target... that would be bad. */ + remote_thr->set_resumed (); + } + } + /* FIXME: This supports_vCont_probed check is a workaround until packet_support is per-connection. */ if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN @@ -6990,36 +7105,6 @@ remote_console_output (const char *msg) gdb_stdtarg->flush (); } -struct stop_reply : public notif_event -{ - ~stop_reply (); - - /* The identifier of the thread about this event */ - ptid_t ptid; - - /* The remote state this event is associated with. When the remote - connection, represented by a remote_state object, is closed, - all the associated stop_reply events should be released. */ - struct remote_state *rs; - - struct target_waitstatus ws; - - /* The architecture associated with the expedited registers. */ - gdbarch *arch; - - /* Expedited registers. This makes remote debugging a bit more - efficient for those targets that provide critical registers as - part of their normal status mechanism (as another roundtrip to - fetch them is avoided). */ - std::vector<cached_reg_t> regcache; - - enum target_stop_reason stop_reason; - - CORE_ADDR watch_data_address; - - int core; -}; - /* Return the length of the stop reply queue. */ int diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 74103bf..efbb31b 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -14,7 +14,7 @@ struct dummy_target : public target_ops void detach (inferior *arg0, int arg1) override; void disconnect (const char *arg0, int arg1) override; void resume (ptid_t arg0, int arg1, enum gdb_signal arg2) override; - void commit_resume () override; + void commit_resumed () override; ptid_t wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2) override; void fetch_registers (struct regcache *arg0, int arg1) override; void store_registers (struct regcache *arg0, int arg1) override; @@ -188,7 +188,7 @@ struct debug_target : public target_ops void detach (inferior *arg0, int arg1) override; void disconnect (const char *arg0, int arg1) override; void resume (ptid_t arg0, int arg1, enum gdb_signal arg2) override; - void commit_resume () override; + void commit_resumed () override; ptid_t wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2) override; void fetch_registers (struct regcache *arg0, int arg1) override; void store_registers (struct regcache *arg0, int arg1) override; @@ -447,22 +447,22 @@ debug_target::resume (ptid_t arg0, int arg1, enum gdb_signal arg2) } void -target_ops::commit_resume () +target_ops::commit_resumed () { - this->beneath ()->commit_resume (); + this->beneath ()->commit_resumed (); } void -dummy_target::commit_resume () +dummy_target::commit_resumed () { } void -debug_target::commit_resume () +debug_target::commit_resumed () { - fprintf_unfiltered (gdb_stdlog, "-> %s->commit_resume (...)\n", this->beneath ()->shortname ()); - this->beneath ()->commit_resume (); - fprintf_unfiltered (gdb_stdlog, "<- %s->commit_resume (", this->beneath ()->shortname ()); + fprintf_unfiltered (gdb_stdlog, "-> %s->commit_resumed (...)\n", this->beneath ()->shortname ()); + this->beneath ()->commit_resumed (); + fprintf_unfiltered (gdb_stdlog, "<- %s->commit_resumed (", this->beneath ()->shortname ()); fputs_unfiltered (")\n", gdb_stdlog); } diff --git a/gdb/target.c b/gdb/target.c index 51832c5..0da035e 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2597,6 +2597,9 @@ target_wait (ptid_t ptid, struct target_waitstatus *status, target_wait_flags options) { target_ops *target = current_inferior ()->top_target (); + process_stratum_target *proc_target = current_inferior ()->process_target (); + + gdb_assert (!proc_target->commit_resumed_state); if (!target->can_async_p ()) gdb_assert ((options & TARGET_WNOHANG) == 0); @@ -2653,6 +2656,7 @@ void target_resume (ptid_t ptid, int step, enum gdb_signal signal) { process_stratum_target *curr_target = current_inferior ()->process_target (); + gdb_assert (!curr_target->commit_resumed_state); target_dcache_invalidate (); @@ -2666,26 +2670,13 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal) clear_inline_frame_state (curr_target, ptid); } -/* If true, target_commit_resume is a nop. */ -static int defer_target_commit_resume; - /* See target.h. */ void -target_commit_resume (void) -{ - if (defer_target_commit_resume) - return; - - current_inferior ()->top_target ()->commit_resume (); -} - -/* See target.h. */ - -scoped_restore_tmpl<int> -make_scoped_defer_target_commit_resume () +target_commit_resumed () { - return make_scoped_restore (&defer_target_commit_resume, 1); + gdb_assert (current_inferior ()->process_target ()->commit_resumed_state); + current_inferior ()->top_target ()->commit_resumed (); } void @@ -3761,6 +3752,10 @@ target_update_thread_list (void) void target_stop (ptid_t ptid) { + process_stratum_target *proc_target = current_inferior ()->process_target (); + + gdb_assert (!proc_target->commit_resumed_state); + if (!may_stop) { warning (_("May not interrupt or stop the target, ignoring attempt")); diff --git a/gdb/target.h b/gdb/target.h index 5d14edb..0aef372 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -485,8 +485,15 @@ struct target_ops int TARGET_DEBUG_PRINTER (target_debug_print_step), enum gdb_signal) TARGET_DEFAULT_NORETURN (noprocess ()); - virtual void commit_resume () + + /* Ensure that all resumed threads are committed to the target. + + See the description of + process_stratum_target::commit_resumed_state for more + details. */ + virtual void commit_resumed () TARGET_DEFAULT_IGNORE (); + /* See target_wait's description. Note that implementations of this method must not assume that inferior_ptid on entry is pointing at the thread or inferior that ends up reporting an @@ -1463,23 +1470,11 @@ extern void target_disconnect (const char *, int); target_commit_resume below. */ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal); -/* Commit a series of resumption requests previously prepared with - target_resume calls. - - GDB always calls target_commit_resume after calling target_resume - one or more times. A target may thus use this method in - coordination with the target_resume method to batch target-side - resumption requests. In that case, the target doesn't actually - resume in its target_resume implementation. Instead, it prepares - the resumption in target_resume, and defers the actual resumption - to target_commit_resume. E.g., the remote target uses this to - coalesce multiple resumption requests in a single vCont packet. */ -extern void target_commit_resume (); - -/* Setup to defer target_commit_resume calls, and reactivate - target_commit_resume on destruction, if it was previously - active. */ -extern scoped_restore_tmpl<int> make_scoped_defer_target_commit_resume (); +/* Ensure that all resumed threads are committed to the target. + + See the description of process_stratum_target::commit_resumed_state + for more details. */ +extern void target_commit_resumed (); /* For target_read_memory see target/target.h. */ @@ -517,6 +517,13 @@ wait_sync_command_done (void) scoped_restore save_ui = make_scoped_restore (¤t_ui); struct ui *ui = current_ui; + /* We're about to wait until the target stops after having resumed + it so must force-commit resumptions, in case we're being called + in some context where a scoped_disable_commit_resumed object is + active. I.e., this function is a commit-resumed sync/flush + point. */ + scoped_enable_commit_resumed enable ("sync wait"); + while (gdb_do_one_event () >= 0) if (ui->prompt_state != PROMPT_BLOCKED) break; |