From patchwork Tue Dec 12 17:44:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 81993 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 82907385B51F for ; Tue, 12 Dec 2023 17:45:01 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-il1-x130.google.com (mail-il1-x130.google.com [IPv6:2607:f8b0:4864:20::130]) by sourceware.org (Postfix) with ESMTPS id E59BA3858C2A for ; Tue, 12 Dec 2023 17:44:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E59BA3858C2A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E59BA3858C2A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::130 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702403087; cv=none; b=jOYoiwZzA5QKqHAsXSxSBOX5zxfllH+Zbyuny9OXkmlHYvstkWyzS82HfYXW1sep92so8cscNkNr18V+y4PU6gVNtm/HOVbyOozXDAvZzLW3dfAtYiWm4VJSOe1a7zPweg7RrCq3lk9sxV/hjhNO4IQOPAazwlhC8g6Xg+ihsDg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702403087; c=relaxed/simple; bh=0T4oB+uxB+DNZQEgzhTC0VlIrKG6jrehcv/EnJC9NOk=; h=DKIM-Signature:From:Date:Subject:MIME-Version:Message-Id:To; b=XNr7GOW3ELlnzWkQ6if/6xOoqXKTGr0fG59GjtzNQlcP3PUl3uhfu7PCDUXM5klToXUi9ghWljw8L7sj+fGgepj5+W7+TyAZ2wcqRXaidN36ayu+P8uxjwzP44LBYaxbjeX5E3zJClH8YxzKw+bJoM6mZs/TyGn0dEdjDT6S5p4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-il1-x130.google.com with SMTP id e9e14a558f8ab-35d74cf427cso23430175ab.1 for ; Tue, 12 Dec 2023 09:44:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1702403084; x=1703007884; darn=sourceware.org; h=to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=cc4h28pQyTSMdG7L9KCN+/83VrbA2cdDWNuKbiKWFbw=; b=Tx7+IajNcrOyVxUyNddxUAQB8cidazWW6J73f+Ecs5V/qi3kgSxP+BkHdZzGythoXd Ct5ZK/uHHipU83cyxbO7V5sUgrXmcNVhq4XsBa4vZgpH1+7U4RHuBKeiAqZ74+hJJeoK wT6fhvjlX4vCmmPlLipn8K9iSXBztW2zLjFRjbCFrdQOq/SW9+a8FaGyZHL3jABnowD8 mYLN8MNnEUGtP8KlDoDLnnoe3Xa2ivOuww9uR6n/2MXhfoDCWlI48MhFLMBAWLvfHwLF XTnRPROeh0DFa4PXyfQT8wtGXWIlMF6csr9h55QhKQ+lA0OqMQXbq+UwVjemruTXbBES k1eg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702403084; x=1703007884; h=to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cc4h28pQyTSMdG7L9KCN+/83VrbA2cdDWNuKbiKWFbw=; b=Hmqv5tF3FsuPm+rk/iYLrZwkKFGc6crJphlAMlCzHvDLJSXDwz5mfAv2z8Pi3wTBx9 xiXq1fKI94xR9tIhPcpG1wZ0XM1N2WeUHMlfs7lY0yoKxMrv69KSd6T1h86vemdzOB+T 99OAUY8gsPLZIiXSUeATfgzIl1znY3CNMYsDn1nSheYUFjRTkjZHFQoBqPQ9S2pb8wTQ +4KYwuWBTE35vS9X6lvEJQe0Rib0Oz3neSE4vz25MN/E4n0/slLZqgNGqZAS0EiLfurP hz2dkGz3JQFQy5Gvo2ITbBJaz8jSSlj5VFy7hcwNJySMuXU4l8XHVUDTOikao6TUm60p xZGQ== X-Gm-Message-State: AOJu0YysUbnMfIRfZYrz/rChv+Ej7/DbDLkwFXBFp7WXoi9Rirg/OnLb R/svxdKQavoY2fZTYgRn3Og05amBK7ceHQ30Hn4= X-Google-Smtp-Source: AGHT+IHdzHwKxIOlbK35506zpte49RegANQOXYpillm+9I/92jdzphnXP1XZtRRJp2hqRG9VtP3wqg== X-Received: by 2002:a05:6e02:17cf:b0:35d:a8d0:6e31 with SMTP id z15-20020a056e0217cf00b0035da8d06e31mr8889440ilu.26.1702403084083; Tue, 12 Dec 2023 09:44:44 -0800 (PST) Received: from localhost.localdomain (71-211-161-25.hlrn.qwest.net. [71.211.161.25]) by smtp.gmail.com with ESMTPSA id u26-20020a02aa9a000000b004693a30f295sm2479819jai.170.2023.12.12.09.44.43 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Dec 2023 09:44:43 -0800 (PST) From: Tom Tromey Date: Tue, 12 Dec 2023 10:44:42 -0700 Subject: [PATCH 1/4] Introduce and use DAPException MIME-Version: 1.0 Message-Id: <20231212-dap-no-test-exceptions-v1-1-af0e33f10093@adacore.com> References: <20231212-dap-no-test-exceptions-v1-0-af0e33f10093@adacore.com> In-Reply-To: <20231212-dap-no-test-exceptions-v1-0-af0e33f10093@adacore.com> To: gdb-patches@sourceware.org X-Mailer: b4 0.12.4 X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org 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 . 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]