* [PATCH 1/4] Introduce and use DAPException
2023-12-12 17:44 [PATCH 0/4] Check for rogue DAP exceptions Tom Tromey
@ 2023-12-12 17:44 ` Tom Tromey
2023-12-12 17:44 ` [PATCH 2/4] Add DAP log level parameter Tom Tromey
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2023-12-12 17:44 UTC (permalink / raw)
To: gdb-patches
This introduces a new DAPException class, and then changes various
spots in the DAP implementation to wrap "expected" exceptions in this.
This class will help detect rogue exceptions caused by bugs in the
implementation.
---
gdb/python/lib/gdb/dap/breakpoint.py | 8 +++++---
gdb/python/lib/gdb/dap/evaluate.py | 12 ++++++------
gdb/python/lib/gdb/dap/launch.py | 4 ++--
gdb/python/lib/gdb/dap/sources.py | 6 +++---
gdb/python/lib/gdb/dap/startup.py | 18 ++++++++++++++++++
gdb/python/lib/gdb/dap/varref.py | 6 +++---
6 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 33aa18e65bc..c67bb471daf 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -24,7 +24,7 @@ from typing import Optional, Sequence
from .server import request, capability, send_event
from .sources import make_source
-from .startup import in_gdb_thread, log_stack
+from .startup import in_gdb_thread, log_stack, parse_and_eval, DAPException
from .typecheck import type_check
@@ -168,7 +168,7 @@ def _set_breakpoints_callback(kind, specs, creator):
bp.ignore_count = 0
else:
bp.ignore_count = int(
- gdb.parse_and_eval(hit_condition, global_context=True)
+ parse_and_eval(hit_condition, global_context=True)
)
# Reaching this spot means success.
@@ -212,6 +212,7 @@ class _PrintBreakpoint(gdb.Breakpoint):
# have already been stripped by the placement of the
# regex capture in the 'split' call.
try:
+ # No real need to use the DAP parse_and_eval here.
val = gdb.parse_and_eval(item)
output += str(val)
except Exception as e:
@@ -360,12 +361,13 @@ def _catch_exception(filterId, **args):
if filterId in ("assert", "exception", "throw", "rethrow", "catch"):
cmd = "-catch-" + filterId
else:
- raise Exception("Invalid exception filterID: " + str(filterId))
+ raise DAPException("Invalid exception filterID: " + str(filterId))
result = gdb.execute_mi(cmd)
# A little lame that there's no more direct way.
for bp in gdb.breakpoints():
if bp.number == result["bkptno"]:
return bp
+ # Not a DAPException because this is definitely unexpected.
raise Exception("Could not find catchpoint after creating")
diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index d39e7879205..eb38baf3973 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -20,7 +20,7 @@ from typing import Optional
from .frames import select_frame
from .server import capability, request, client_bool_capability
-from .startup import in_gdb_thread
+from .startup import in_gdb_thread, parse_and_eval, DAPException
from .varref import find_variable, VariableReference, apply_format
@@ -37,7 +37,7 @@ def _evaluate(expr, frame_id, value_format):
if frame_id is not None:
select_frame(frame_id)
global_context = False
- val = gdb.parse_and_eval(expr, global_context=global_context)
+ val = parse_and_eval(expr, global_context=global_context)
ref = EvaluateResult(val)
return ref.to_object()
@@ -89,7 +89,7 @@ def eval_request(
# Ignore the format for repl evaluation.
return _repl(expression, frameId)
else:
- raise Exception('unknown evaluate context "' + context + '"')
+ raise DAPException('unknown evaluate context "' + context + '"')
@request("variables")
@@ -119,8 +119,8 @@ def set_expression(
if frameId is not None:
select_frame(frameId)
global_context = False
- lhs = gdb.parse_and_eval(expression, global_context=global_context)
- rhs = gdb.parse_and_eval(value, global_context=global_context)
+ lhs = parse_and_eval(expression, global_context=global_context)
+ rhs = parse_and_eval(value, global_context=global_context)
lhs.assign(rhs)
return _SetResult(lhs).to_object()
@@ -133,6 +133,6 @@ def set_variable(
with apply_format(format):
var = find_variable(variablesReference)
lhs = var.find_child_by_name(name)
- rhs = gdb.parse_and_eval(value)
+ rhs = parse_and_eval(value)
lhs.assign(rhs)
return lhs.to_object()
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index 7014047ff51..a8adb125707 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -20,7 +20,7 @@ from typing import Mapping, Optional, Sequence
from .events import exec_and_expect_stop, expect_process
from .server import request, capability
-from .startup import exec_and_log
+from .startup import exec_and_log, DAPException
# The program being launched, or None. This should only be accessed
@@ -69,7 +69,7 @@ def attach(*, pid: Optional[int] = None, target: Optional[str] = None, **args):
elif target is not None:
cmd = "target remote " + target
else:
- raise Exception("attach requires either 'pid' or 'target'")
+ raise DAPException("attach requires either 'pid' or 'target'")
expect_process("attach")
exec_and_log(cmd)
diff --git a/gdb/python/lib/gdb/dap/sources.py b/gdb/python/lib/gdb/dap/sources.py
index 821205cedd1..d73a3f8c5b1 100644
--- a/gdb/python/lib/gdb/dap/sources.py
+++ b/gdb/python/lib/gdb/dap/sources.py
@@ -18,7 +18,7 @@ import os
import gdb
from .server import request, capability
-from .startup import in_gdb_thread
+from .startup import in_gdb_thread, DAPException
# The next available source reference ID. Must be greater than 0.
@@ -68,11 +68,11 @@ def decode_source(source):
if "path" in source:
return source["path"]
if "sourceReference" not in source:
- raise Exception("either 'path' or 'sourceReference' must appear in Source")
+ raise DAPException("either 'path' or 'sourceReference' must appear in Source")
ref = source["sourceReference"]
global _id_map
if ref not in _id_map:
- raise Exception("no sourceReference " + str(ref))
+ raise DAPException("no sourceReference " + str(ref))
return _id_map[ref]["path"]
diff --git a/gdb/python/lib/gdb/dap/startup.py b/gdb/python/lib/gdb/dap/startup.py
index 1d3b94762a6..64a46597bf4 100644
--- a/gdb/python/lib/gdb/dap/startup.py
+++ b/gdb/python/lib/gdb/dap/startup.py
@@ -39,6 +39,24 @@ _gdb_thread = threading.current_thread()
_dap_thread = None
+# "Known" exceptions are wrapped in a DAP exception, so that, by
+# default, only rogue exceptions are logged -- this is then used by
+# the test suite.
+class DAPException(Exception):
+ pass
+
+
+# Wrapper for gdb.parse_and_eval that turns exceptions into
+# DAPException.
+def parse_and_eval(expression, global_context=False):
+ try:
+ return gdb.parse_and_eval(expression, global_context=global_context)
+ except Exception as e:
+ # Be sure to preserve the summary, as this can propagate to
+ # the client.
+ raise DAPException(str(e)) from e
+
+
def start_thread(name, target, args=()):
"""Start a new thread, invoking TARGET with *ARGS there.
This is a helper function that ensures that any GDB signals are
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 8f0a070498c..c75afe3848e 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -14,7 +14,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import gdb
-from .startup import in_gdb_thread
+from .startup import in_gdb_thread, DAPException
from .server import client_bool_capability
from abc import ABC, abstractmethod
from collections import defaultdict
@@ -165,7 +165,7 @@ class BaseReference(ABC):
# map here.
if name in self.by_name:
return self.by_name[name]
- raise Exception("no variable named '" + name + "'")
+ raise DAPException("no variable named '" + name + "'")
class VariableReference(BaseReference):
@@ -271,5 +271,5 @@ def find_variable(ref):
# Variable references are offset by 1.
ref = ref - 1
if ref < 0 or ref > len(all_variables):
- raise Exception("invalid variablesReference")
+ raise DAPException("invalid variablesReference")
return all_variables[ref]
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 2/4] Add DAP log level parameter
2023-12-12 17:44 [PATCH 0/4] Check for rogue DAP exceptions Tom Tromey
2023-12-12 17:44 ` [PATCH 1/4] Introduce and use DAPException Tom Tromey
@ 2023-12-12 17:44 ` Tom Tromey
2023-12-12 18:39 ` Eli Zaretskii
2023-12-13 16:35 ` Kévin Le Gouguec
2023-12-12 17:44 ` [PATCH 3/4] Avoid exception from attach in DAP Tom Tromey
` (3 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Tom Tromey @ 2023-12-12 17:44 UTC (permalink / raw)
To: gdb-patches
This adds a new parameter to control the DAP logging level. By
default, "expected" exceptions are not logged, but the parameter lets
the user change this when more logging is desired.
This also changes a couple of spots to avoid logging the stack trace
for a DAPException.
This patch also documents the existing DAP logging parameter. I
forgot to document this before.
---
gdb/NEWS | 5 +++++
gdb/doc/gdb.texinfo | 18 ++++++++++++++++++
gdb/python/lib/gdb/dap/breakpoint.py | 6 ++++--
gdb/python/lib/gdb/dap/server.py | 10 ++++++++++
gdb/python/lib/gdb/dap/startup.py | 29 +++++++++++++++++++++++++----
5 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 534e2e7f364..6d35722cc23 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -88,6 +88,11 @@ show remote thread-options-packet
** GDB now supports the "cancel" request.
+ ** New command "set debug dap-log-level" controls DAP logging.
+
+ ** The "set debug dap-log-file" command is now documented. This
+ command was available in GDB 14 but not documented.
+
* New remote packets
New stop reason: clone
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6e4adf512ee..fd60e41091a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39611,6 +39611,24 @@ Evaluations like this can be interrupted using the DAP @code{cancel}
request. (In fact, @code{cancel} should work for any request, but it
is unlikely to be useful for most of them.)
+@value{GDBN} provides a couple of logging settings that can be used in
+DAP mode. These can be set on the command line using the @code{-iex}
+option (@pxref{File Options}).
+
+@table @code
+@item set debug dap-log-file @r{[}@var{filename}@r{]}
+Enable DAP logging. Logs are written to the specified file. If no
+file is given, logging is stopped.
+
+@item set debug dap-log-level @var{level}
+Set the DAP logging level. The default is @samp{1}, which logs the
+DAP protocol, whatever debug messages the developers thought were
+useful, and unexpected exceptions. Level @samp{2} can be used to log
+all exceptions, including ones that are considered to be expected.
+For example, a failure to parse an expression would be considered a
+normal exception and not normally be logged.
+@end table
+
@node JIT Interface
@chapter JIT Compilation Interface
@cindex just-in-time compilation
diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index c67bb471daf..cef3df910f0 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -24,7 +24,7 @@ from typing import Optional, Sequence
from .server import request, capability, send_event
from .sources import make_source
-from .startup import in_gdb_thread, log_stack, parse_and_eval, DAPException
+from .startup import in_gdb_thread, log_stack, parse_and_eval, LOG_FULL, DAPException
from .typecheck import type_check
@@ -176,7 +176,9 @@ def _set_breakpoints_callback(kind, specs, creator):
result.append(_breakpoint_descriptor(bp))
# Exceptions other than gdb.error are possible here.
except Exception as e:
- log_stack()
+ # Don't normally want to see this, as it interferes with
+ # the test suite.
+ log_stack(LOG_FULL)
# Maybe the breakpoint was made but setting an attribute
# failed. We still want this to fail.
if bp is not None:
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index 44dffb1b809..e682c0faf84 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -23,6 +23,7 @@ import threading
from .io import start_json_writer, read_json
from .startup import (
exec_and_log,
+ DAPException,
DAPQueue,
in_dap_thread,
in_gdb_thread,
@@ -31,6 +32,7 @@ from .startup import (
start_thread,
log,
log_stack,
+ LOG_FULL,
)
from .typecheck import type_check
@@ -139,12 +141,20 @@ class Server:
result["body"] = body
result["success"] = True
except NotStoppedException:
+ # This is an expected exception, and the result is clearly
+ # visible in the log, so do not log it.
result["success"] = False
result["message"] = "notStopped"
except KeyboardInterrupt:
# This can only happen when a request has been canceled.
result["success"] = False
result["message"] = "cancelled"
+ except DAPException as e:
+ # Don't normally want to see this, as it interferes with
+ # the test suite.
+ log_stack(LOG_FULL)
+ result["success"] = False
+ result["message"] = str(e)
except BaseException as e:
log_stack()
result["success"] = False
diff --git a/gdb/python/lib/gdb/dap/startup.py b/gdb/python/lib/gdb/dap/startup.py
index 64a46597bf4..548fedf4ac9 100644
--- a/gdb/python/lib/gdb/dap/startup.py
+++ b/gdb/python/lib/gdb/dap/startup.py
@@ -101,6 +101,27 @@ def in_dap_thread(func):
return ensure_dap_thread
+# Logging levels.
+LOG_DEFAULT = 1
+LOG_FULL = 2
+
+
+class LogLevel(gdb.Parameter):
+ """DAP logging level."""
+
+ set_doc = "Set the DAP logging level."
+ show_doc = "Show the DAP logging level."
+
+ def __init__(self):
+ super().__init__(
+ "debug dap-log-level", gdb.COMMAND_MAINTENANCE, gdb.PARAM_ZUINTEGER
+ )
+ self.value = LOG_DEFAULT
+
+
+_log_level = LogLevel()
+
+
class LoggingParam(gdb.Parameter):
"""Whether DAP logging is enabled."""
@@ -128,16 +149,16 @@ class LoggingParam(gdb.Parameter):
dap_log = LoggingParam()
-def log(something):
+def log(something, level=LOG_DEFAULT):
"""Log SOMETHING to the log file, if logging is enabled."""
- if dap_log.log_file is not None:
+ if dap_log.log_file is not None and level <= _log_level.value:
print(something, file=dap_log.log_file)
dap_log.log_file.flush()
-def log_stack():
+def log_stack(level=LOG_DEFAULT):
"""Log a stack trace to the log file, if logging is enabled."""
- if dap_log.log_file is not None:
+ if dap_log.log_file is not None and level <= _log_level.value:
traceback.print_exc(file=dap_log.log_file)
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] Add DAP log level parameter
2023-12-12 17:44 ` [PATCH 2/4] Add DAP log level parameter Tom Tromey
@ 2023-12-12 18:39 ` Eli Zaretskii
2023-12-14 19:02 ` Tom Tromey
2023-12-13 16:35 ` Kévin Le Gouguec
1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2023-12-12 18:39 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> From: Tom Tromey <tromey@adacore.com>
> Date: Tue, 12 Dec 2023 10:44:43 -0700
>
> This adds a new parameter to control the DAP logging level. By
> default, "expected" exceptions are not logged, but the parameter lets
> the user change this when more logging is desired.
>
> This also changes a couple of spots to avoid logging the stack trace
> for a DAPException.
>
> This patch also documents the existing DAP logging parameter. I
> forgot to document this before.
> ---
> gdb/NEWS | 5 +++++
> gdb/doc/gdb.texinfo | 18 ++++++++++++++++++
> gdb/python/lib/gdb/dap/breakpoint.py | 6 ++++--
> gdb/python/lib/gdb/dap/server.py | 10 ++++++++++
> gdb/python/lib/gdb/dap/startup.py | 29 +++++++++++++++++++++++++----
> 5 files changed, 62 insertions(+), 6 deletions(-)
Thanks, the documentation parts are okay, with one comment:
> +@table @code
> +@item set debug dap-log-file @r{[}@var{filename}@r{]}
> +Enable DAP logging. Logs are written to the specified file. If no
> +file is given, logging is stopped.
I suggest to use "@var{filename}" instead of "file" here, on both
occasions.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] Add DAP log level parameter
2023-12-12 18:39 ` Eli Zaretskii
@ 2023-12-14 19:02 ` Tom Tromey
0 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2023-12-14 19:02 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches
>> +@table @code
>> +@item set debug dap-log-file @r{[}@var{filename}@r{]}
>> +Enable DAP logging. Logs are written to the specified file. If no
>> +file is given, logging is stopped.
Eli> I suggest to use "@var{filename}" instead of "file" here, on both
Eli> occasions.
I made this change.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] Add DAP log level parameter
2023-12-12 17:44 ` [PATCH 2/4] Add DAP log level parameter Tom Tromey
2023-12-12 18:39 ` Eli Zaretskii
@ 2023-12-13 16:35 ` Kévin Le Gouguec
2023-12-14 19:06 ` Tom Tromey
1 sibling, 1 reply; 13+ messages in thread
From: Kévin Le Gouguec @ 2023-12-13 16:35 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Tom Tromey <tromey@adacore.com> writes:
> +# Logging levels.
> +LOG_DEFAULT = 1
> +LOG_FULL = 2
> +
> +
> +class LogLevel(gdb.Parameter):
> + """DAP logging level."""
> +
> + set_doc = "Set the DAP logging level."
> + show_doc = "Show the DAP logging level."
> +
> + def __init__(self):
> + super().__init__(
> + "debug dap-log-level", gdb.COMMAND_MAINTENANCE, gdb.PARAM_ZUINTEGER
> + )
> + self.value = LOG_DEFAULT
> +
> +
> +_log_level = LogLevel()
I might suggest an enum here:
from enum import IntEnum, auto
class LogLevel(IntEnum):
DEFAULT = auto()
FULL = auto()
class LogLevelParam(gdb.Parameter):
"""..."""
def __init__(self):
...
self.value = LogLevel.DEFAULT
But the benefits of Python enums are somewhat marginal (even more so in
this instance; FWIW I _think_ IntEnum & auto are all available since
3.4), so no strong feelings about this.
Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] Add DAP log level parameter
2023-12-13 16:35 ` Kévin Le Gouguec
@ 2023-12-14 19:06 ` Tom Tromey
0 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2023-12-14 19:06 UTC (permalink / raw)
To: Kévin Le Gouguec; +Cc: Tom Tromey, gdb-patches
>>>>> "Kévin" == Kévin Le Gouguec <legouguec@adacore.com> writes:
Kévin> I might suggest an enum here:
...
Kévin> But the benefits of Python enums are somewhat marginal (even more so in
Kévin> this instance; FWIW I _think_ IntEnum & auto are all available since
Kévin> 3.4), so no strong feelings about this.
I went ahead and did it, it seems a little nicer maybe.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] Avoid exception from attach in DAP
2023-12-12 17:44 [PATCH 0/4] Check for rogue DAP exceptions Tom Tromey
2023-12-12 17:44 ` [PATCH 1/4] Introduce and use DAPException Tom Tromey
2023-12-12 17:44 ` [PATCH 2/4] Add DAP log level parameter Tom Tromey
@ 2023-12-12 17:44 ` Tom Tromey
2023-12-13 16:36 ` Kévin Le Gouguec
2023-12-12 17:44 ` [PATCH 4/4] Check for rogue DAP exceptions in test suite Tom Tromey
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Tom Tromey @ 2023-12-12 17:44 UTC (permalink / raw)
To: gdb-patches
I noticed that the DAP attach test case (and similarly
remoted-dap.exp) had a rogue exception stack trace in the log. It
turns out that an attach will generate a stop that does not have a
reason.
This patch fixes the problem in the _on_stop event listener by making
it a bit more careful when examining the event reason. It also adds
some machinery so that attach stops can be suppressed, which I think
is the right thing to do.
---
gdb/python/lib/gdb/dap/events.py | 43 +++++++++++++++++++++++++++++++---------
gdb/python/lib/gdb/dap/launch.py | 3 ++-
2 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/gdb/python/lib/gdb/dap/events.py b/gdb/python/lib/gdb/dap/events.py
index cbefe90e4ca..82fe99632d5 100644
--- a/gdb/python/lib/gdb/dap/events.py
+++ b/gdb/python/lib/gdb/dap/events.py
@@ -147,6 +147,16 @@ def _cont(event):
)
+_suppress_stop = False
+
+
+@in_gdb_thread
+def suppress_stop():
+ """Indicate that the next stop should not emit an event."""
+ global _suppress_stop
+ _suppress_stop = True
+
+
_expected_pause = False
@@ -198,6 +208,13 @@ stop_reason_map = {
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))
obj = {
@@ -206,17 +223,25 @@ def _on_stop(event):
}
if isinstance(event, gdb.BreakpointEvent):
obj["hitBreakpointIds"] = [x.number for x in event.breakpoints]
- global stop_reason_map
- reason = event.details["reason"]
global _expected_pause
- if (
- _expected_pause
- and reason == "signal-received"
- and event.details["signal-name"] in ("SIGINT", "SIGSTOP")
- ):
- obj["reason"] = "pause"
+ # Some stop events still do not emit details. For example,
+ # 'attach' causes a reason-less stop.
+ if "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.
+ obj["reason"] = "stopped"
else:
- obj["reason"] = stop_reason_map[reason]
+ reason = event.details["reason"]
+ if (
+ _expected_pause
+ and reason == "signal-received"
+ and event.details["signal-name"] in ("SIGINT", "SIGSTOP")
+ ):
+ obj["reason"] = "pause"
+ else:
+ global stop_reason_map
+ obj["reason"] = stop_reason_map[reason]
_expected_pause = False
send_event("stopped", obj)
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index a8adb125707..a20009c190d 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
+from .events import exec_and_expect_stop, expect_process, suppress_stop
from .server import request, capability
from .startup import exec_and_log, DAPException
@@ -71,6 +71,7 @@ def attach(*, pid: Optional[int] = None, target: Optional[str] = None, **args):
else:
raise DAPException("attach requires either 'pid' or 'target'")
expect_process("attach")
+ suppress_stop()
exec_and_log(cmd)
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] Avoid exception from attach in DAP
2023-12-12 17:44 ` [PATCH 3/4] Avoid exception from attach in DAP Tom Tromey
@ 2023-12-13 16:36 ` Kévin Le Gouguec
2023-12-14 18:52 ` Tom Tromey
0 siblings, 1 reply; 13+ messages in thread
From: Kévin Le Gouguec @ 2023-12-13 16:36 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Tom Tromey <tromey@adacore.com> writes:
> + if "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.
> + obj["reason"] = "stopped"
> else:
> - obj["reason"] = stop_reason_map[reason]
> + reason = event.details["reason"]
> + if (
> + _expected_pause
> + and reason == "signal-received"
> + and event.details["signal-name"] in ("SIGINT", "SIGSTOP")
> + ):
> + obj["reason"] = "pause"
> + else:
> + global stop_reason_map
> + obj["reason"] = stop_reason_map[reason]
> _expected_pause = False
> send_event("stopped", obj)
Nit: I'd find
if "reason" not in event.details:
# ...
obj["reason"] = "stopped"
elif (
_expected_pause
and event.details["reason"] == "signal-received"
and event.details["signal-name"] in ("SIGINT", "SIGSTOP")
):
obj["reason"] = "pause"
else:
global stop_reason_map
obj["reason"] = stop_reason_map[event.details["reason"]]
slightly more legible (less nesting; easier to understand that the
purpose of each branch is to set obj["reason"]), even if it means
repeating event.details["reason"]. Really not a huge deal though, good
to go as-is for me.
Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] Avoid exception from attach in DAP
2023-12-13 16:36 ` Kévin Le Gouguec
@ 2023-12-14 18:52 ` Tom Tromey
0 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2023-12-14 18:52 UTC (permalink / raw)
To: Kévin Le Gouguec; +Cc: Tom Tromey, gdb-patches
>>>>> "Kévin" == Kévin Le Gouguec <legouguec@adacore.com> writes:
Kévin> Nit: I'd find
...
Kévin> slightly more legible (less nesting; easier to understand that the
Kévin> purpose of each branch is to set obj["reason"]), even if it means
Kévin> repeating event.details["reason"]. Really not a huge deal though, good
Kévin> to go as-is for me.
I like it, I've made this change here.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] Check for rogue DAP exceptions in test suite
2023-12-12 17:44 [PATCH 0/4] Check for rogue DAP exceptions Tom Tromey
` (2 preceding siblings ...)
2023-12-12 17:44 ` [PATCH 3/4] Avoid exception from attach in DAP Tom Tromey
@ 2023-12-12 17:44 ` Tom Tromey
2023-12-13 16:36 ` [PATCH 0/4] Check for rogue DAP exceptions Kévin Le Gouguec
2023-12-22 16:57 ` Tom Tromey
5 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2023-12-12 17:44 UTC (permalink / raw)
To: gdb-patches
This changes the test suite to look for rogue DAP exceptions in the
log file, and issue a "fail" if one is found.
---
gdb/testsuite/lib/dap-support.exp | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index b9ac314fee5..4aaae637c45 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -16,6 +16,11 @@
# The JSON parser.
load_lib ton.tcl
+# The sequence number for the currently-executing instance of gdb
+# launched via dap_gdb_start. This is used for log-file checking
+# after the run is complete. Zero means gdb hasn't started yet.
+set dap_gdb_instance 0
+
# The sequence number for the next DAP request. This is used by the
# automatic sequence-counting code below. It is reset each time GDB
# is restarted.
@@ -29,10 +34,13 @@ proc dap_gdb_start {} {
gdb_stdin_log_init
+ global dap_gdb_instance
+ incr dap_gdb_instance
+
global GDBFLAGS stty_init
save_vars { GDBFLAGS stty_init } {
set stty_init "-echo raw"
- set logfile [standard_output_file "dap.log.$gdb_instances"]
+ set logfile [standard_output_file "dap.log.$dap_gdb_instance"]
append GDBFLAGS " -iex \"set debug dap-log-file $logfile\" -q -i=dap"
set res [gdb_spawn]
if {$res != 0} {
@@ -325,11 +333,35 @@ proc dap_target_remote {target} {
[format {o target [s %s]} $target]]
}
+# Read the most recent DAP log file and check it for exceptions.
+proc _dap_check_log_file {} {
+ global dap_gdb_instance
+
+ set fd [open [standard_output_file "dap.log.$dap_gdb_instance"]]
+ set contents [read $fd]
+ close $fd
+
+ set ok 1
+ foreach line [split $contents "\n"] {
+ if {[regexp "^Traceback" $line]} {
+ set ok 0
+ break
+ }
+ }
+
+ if {$ok} {
+ pass "exceptions in log file"
+ } else {
+ fail "exceptions in log file"
+ }
+}
+
# Cleanly shut down gdb. TERMINATE is passed as the terminateDebuggee
# parameter to the request.
proc dap_shutdown {{terminate false}} {
dap_check_request_and_response "shutdown" disconnect \
[format {o terminateDebuggee [l %s]} $terminate]
+ _dap_check_log_file
}
# Search the event list EVENTS for an output event matching the regexp
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/4] Check for rogue DAP exceptions
2023-12-12 17:44 [PATCH 0/4] Check for rogue DAP exceptions Tom Tromey
` (3 preceding siblings ...)
2023-12-12 17:44 ` [PATCH 4/4] Check for rogue DAP exceptions in test suite Tom Tromey
@ 2023-12-13 16:36 ` Kévin Le Gouguec
2023-12-22 16:57 ` Tom Tromey
5 siblings, 0 replies; 13+ messages in thread
From: Kévin Le Gouguec @ 2023-12-13 16:36 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Tom Tromey <tromey@adacore.com> writes:
> While looking into a question I got about DAP, I found out that the
> DAP 'attach' test had some unexpected exceptions in the log file,
> caused by an incorrect assumption in the stop event listener.
>
> This series tries to make the DAP implementation a bit more robust.
> After these patches, exceptions that are expected in the normal course
> of operation are not logged by default. Then, the test suite is
> updated to check that no rogue exceptions are seen.
>
> ---
> Tom Tromey (4):
> Introduce and use DAPException
> Add DAP log level parameter
> Avoid exception from attach in DAP
> Check for rogue DAP exceptions in test suite
>
> gdb/NEWS | 5 ++++
> gdb/doc/gdb.texinfo | 18 ++++++++++++++
> gdb/python/lib/gdb/dap/breakpoint.py | 12 ++++++---
> gdb/python/lib/gdb/dap/evaluate.py | 12 ++++-----
> gdb/python/lib/gdb/dap/events.py | 43 ++++++++++++++++++++++++++-------
> gdb/python/lib/gdb/dap/launch.py | 7 +++---
> gdb/python/lib/gdb/dap/server.py | 10 ++++++++
> gdb/python/lib/gdb/dap/sources.py | 6 ++---
> gdb/python/lib/gdb/dap/startup.py | 47 +++++++++++++++++++++++++++++++++---
> gdb/python/lib/gdb/dap/varref.py | 6 ++---
> gdb/testsuite/lib/dap-support.exp | 34 +++++++++++++++++++++++++-
> 11 files changed, 167 insertions(+), 33 deletions(-)
> ---
Left a couple of comments on some of the patches, but the series could
go in as-is IMO 👌
Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/4] Check for rogue DAP exceptions
2023-12-12 17:44 [PATCH 0/4] Check for rogue DAP exceptions Tom Tromey
` (4 preceding siblings ...)
2023-12-13 16:36 ` [PATCH 0/4] Check for rogue DAP exceptions Kévin Le Gouguec
@ 2023-12-22 16:57 ` Tom Tromey
5 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2023-12-22 16:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
Tom> While looking into a question I got about DAP, I found out that the
Tom> DAP 'attach' test had some unexpected exceptions in the log file,
Tom> caused by an incorrect assumption in the stop event listener.
Tom> This series tries to make the DAP implementation a bit more robust.
Tom> After these patches, exceptions that are expected in the normal course
Tom> of operation are not logged by default. Then, the test suite is
Tom> updated to check that no rogue exceptions are seen.
I'm checking this in.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread