public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Emit stopped event for DAP attach request
@ 2024-01-17 16:20 Tom Tromey
  2024-01-24 15:31 ` Tom Tromey
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Tromey @ 2024-01-17 16:20 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

In an earlier patch, I wrote:

    ...  It also adds some machinery so that attach stops can be
    suppressed, which I think is the right thing to do.

However, after some discussions here at AdaCore, I now believe this to
be incorrect -- while DAP says that expected "continue" events should
be suppressed, there is no corresponding language for expected "stop"
events, and indeed "stop" events explicitly mention cases like "step".

This patch arranges for the stop event to be emitted again.
---
 gdb/python/lib/gdb/dap/events.py | 28 +++++++++++++---------------
 gdb/python/lib/gdb/dap/launch.py |  4 ++--
 gdb/testsuite/gdb.dap/attach.exp | 16 +++++++++++++---
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/events.py b/gdb/python/lib/gdb/dap/events.py
index bb2d1c9d342..41302229ee5 100644
--- a/gdb/python/lib/gdb/dap/events.py
+++ b/gdb/python/lib/gdb/dap/events.py
@@ -147,14 +147,14 @@ def _cont(event):
         )
 
 
-_suppress_stop = False
+_expected_stop_reason = None
 
 
 @in_gdb_thread
-def suppress_stop():
-    """Indicate that the next stop should not emit an event."""
-    global _suppress_stop
-    _suppress_stop = True
+def expect_stop(reason: str):
+    """Indicate that the next stop should be for REASON."""
+    global _expected_stop_reason
+    _expected_stop_reason = reason
 
 
 _expected_pause = False
@@ -209,24 +209,22 @@ def _on_stop(event):
     global inferior_running
     inferior_running = False
 
-    global _suppress_stop
-    if _suppress_stop:
-        _suppress_stop = False
-        log("suppressing stop in _on_stop")
-        return
-
     log("entering _on_stop: " + repr(event))
-    log("   details: " + repr(event.details))
+    if hasattr(event, "details"):
+        log("   details: " + repr(event.details))
     obj = {
         "threadId": gdb.selected_thread().global_num,
         "allThreadsStopped": True,
     }
     if isinstance(event, gdb.BreakpointEvent):
         obj["hitBreakpointIds"] = [x.number for x in event.breakpoints]
+
     global _expected_pause
-    # Some stop events still do not emit details.  For example,
-    # 'attach' causes a reason-less stop.
-    if "reason" not in event.details:
+    global _expected_stop_reason
+    if _expected_stop_reason is not None:
+        obj["reason"] = _expected_stop_reason
+        _expected_stop_reason = None
+    elif "reason" not in event.details:
         # This can only really happen via a "repl" evaluation of
         # something like "attach".  In this case just emit a generic
         # stop.
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index bf9640a4d92..184af16955c 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -18,7 +18,7 @@ import gdb
 # These are deprecated in 3.9, but required in older versions.
 from typing import Mapping, Optional, Sequence
 
-from .events import exec_and_expect_stop, expect_process, suppress_stop
+from .events import exec_and_expect_stop, expect_process, expect_stop
 from .server import request, capability
 from .startup import exec_and_log, DAPException
 
@@ -88,7 +88,7 @@ def attach(
     else:
         raise DAPException("attach requires either 'pid' or 'target'")
     expect_process("attach")
-    suppress_stop()
+    expect_stop("attach")
     exec_and_log(cmd)
 
 
diff --git a/gdb/testsuite/gdb.dap/attach.exp b/gdb/testsuite/gdb.dap/attach.exp
index 2b30e53cce0..ce2a16c1193 100644
--- a/gdb/testsuite/gdb.dap/attach.exp
+++ b/gdb/testsuite/gdb.dap/attach.exp
@@ -28,9 +28,19 @@ if {[build_executable ${testfile}.exp $testfile] == -1} {
 set test_spawn_id [spawn_wait_for_attach $binfile]
 set testpid [spawn_id_get_pid $test_spawn_id]
 
-# We just want to test that attaching works at all.
-if {[dap_attach $testpid $binfile] != ""} {
-    dap_shutdown true
+# Test that attaching works at all.
+set result [dap_attach $testpid $binfile]
+
+set found 0
+foreach ev [lindex $result 1] {
+    if {[dict get $ev type] == "event"
+	&& [dict get $ev event] == "stopped"
+	&& [dict get $ev body reason] == "attach"} {
+	set found 1
+    }
 }
+gdb_assert {$found} "saw stopped event for attach"
+
+dap_shutdown true
 
 kill_wait_spawned_process $test_spawn_id
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Emit stopped event for DAP attach request
  2024-01-17 16:20 [PATCH] Emit stopped event for DAP attach request Tom Tromey
@ 2024-01-24 15:31 ` Tom Tromey
  0 siblings, 0 replies; 2+ messages in thread
From: Tom Tromey @ 2024-01-24 15:31 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> In an earlier patch, I wrote:
Tom>     ...  It also adds some machinery so that attach stops can be
Tom>     suppressed, which I think is the right thing to do.

Tom> However, after some discussions here at AdaCore, I now believe this to
Tom> be incorrect -- while DAP says that expected "continue" events should
Tom> be suppressed, there is no corresponding language for expected "stop"
Tom> events, and indeed "stop" events explicitly mention cases like "step".

Tom> This patch arranges for the stop event to be emitted again.

I'm checking this in.

Tom

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-01-24 15:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-17 16:20 [PATCH] Emit stopped event for DAP attach request Tom Tromey
2024-01-24 15:31 ` Tom Tromey

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).