[PATCH] Implement windows_nat_target::stop

[PATCH] Implement windows_nat_target::stop

[Patrick Monnerat]

Insight makes a difference between hitting Ctrl-C (that triggers
target::pass_ctrlc) and clicking the GUI stop button, triggering
target::stop).

As windows_nat did not implement the stop method, the GUI stop button
was inoperant on Windows.
---
 gdb/windows-nat.c | 6 ++++++
 gdb/windows-nat.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index e25ae81f054..d08faaa89ba 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -909,6 +909,12 @@ windows_nat_target::pass_ctrlc ()
   interrupt ();
 }
 
+void
+windows_nat_target::stop (ptid_t ptid)
+{
+  interrupt ();
+}
+
 /* Get the next event from the child.  Returns the thread ptid.  */
 
 ptid_t
diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h
index 8263729554b..4368a29b2bb 100644
--- a/gdb/windows-nat.h
+++ b/gdb/windows-nat.h
@@ -141,6 +141,7 @@ struct windows_nat_target : public inf_child_target
 
   void interrupt () override;
   void pass_ctrlc () override;
+  void stop (ptid_t ptid) override;
 
   const char *pid_to_exec_file (int pid) override;
 
-- 
2.53.0

Re: [PATCH] Implement windows_nat_target::stop

[Tom Tromey]

>>>>> "Patrick" == Patrick Monnerat <patrick@monnerat.net> writes:

Patrick> Insight makes a difference between hitting Ctrl-C (that triggers
Patrick> target::pass_ctrlc) and clicking the GUI stop button, triggering
Patrick> target::stop).

I wonder why.

Also I see Insight calls target_stop and not target_stop_and_wait, which
seems a bit weird.

Patrick> As windows_nat did not implement the stop method, the GUI stop button
Patrick> was inoperant on Windows.

Patrick> +void
Patrick> +windows_nat_target::stop (ptid_t ptid)
Patrick> +{
Patrick> +  interrupt ();
Patrick> +}

I don't really know the contract here but is interrupt guaranteed to
stop PTID?  Like is the relevant inferior always selected?  How does
this interact with non-stop?

Right now it's probably somewhat moot but there's ongoing work for
non-stop and multi-inferior on Windows.

Tom

Re: [PATCH] Implement windows_nat_target::stop

[Patrick Monnerat]

On 4/14/26 5:47 PM, Tom Tromey wrote:
> Patrick> Insight makes a difference between hitting Ctrl-C (that triggers
> Patrick> target::pass_ctrlc) and clicking the GUI stop button, triggering
> Patrick> target::stop).
>
> I wonder why.

I can't tell you exactly: the use of target_stop() has been introduced 
in 1994 without detailed explanation!

As a pure conjecture for the initial motivation, I can imagine:

- the target program may have no associated console thus may use SIGINT 
for another purpose

- Bare gdb has only a console interface, so it borrows SIGINT for 
stopping the target: a gui can do better (!).

Latest reworks of this are dated around 1997-1999.

Maybe this should be modernized.

>
> Also I see Insight calls target_stop and not target_stop_and_wait, which
> seems a bit weird.

Yes, you're right. target_stop_and_wait() introduction (2014) is 
posterior to target_stop() and missed from insight point of view.

This is a perverse effect of having splitted out insight: not many gdb 
developers dive into its code nowadays.

In all cases, a target_stop_and_wait() call ends in target_stop(), so 
this change, although wise, would not fix the current problem.

>
> Patrick> As windows_nat did not implement the stop method, the GUI stop button
> Patrick> was inoperant on Windows.
>
> Patrick> +void
> Patrick> +windows_nat_target::stop (ptid_t ptid)
> Patrick> +{
> Patrick> +  interrupt ();
> Patrick> +}
>
> I don't really know the contract here but is interrupt guaranteed to
> stop PTID?  Like is the relevant inferior always selected?  How does
> this interact with non-stop?
? Really out of my skills, sorry.
>
> Right now it's probably somewhat moot but there's ongoing work for
> non-stop and multi-inferior on Windows.
>
So you mean I should wait for this work to land and steady ?

In all cases, it should provide a target stop method compatible with 
other targets.


Thanks for your review,

Patrick