public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 00/25] Many updates to DAP implementation
@ 2023-05-24 16:36 Tom Tromey
  2023-05-24 16:36 ` [PATCH 01/25] Stop gdb in gnat_runtime_has_debug_info Tom Tromey
                   ` (25 more replies)
  0 siblings, 26 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

I've been working steadily on the DAP implementation.  This series
combines a number of patches.

It starts with some general refactoring of Ada catchpoints, to make
them work more like C++ catchpoints.  In particular, now they can be
set before the inferior has started.  This is then used to implement
the DAP setExceptionBreakpoints request.

Other patches add DAP features.  In some cases a new Python API is
added so that it can be used by DAP.

I do have some more patches to come after this series goes in.

Regression tested on x86-64 Fedora 36.

---
Tom Tromey (25):
      Stop gdb in gnat_runtime_has_debug_info
      Use gnat_runtime_has_debug_info in Ada catchpoint tests
      Pass tempflag to ada_catchpoint constructor
      Transfer ownership of exception string to ada_catchpoint
      Combine create_excep_cond_exprs and ada_catchpoint::re_set
      Turn should_stop_exception into a method of ada_catchpoint
      Mark members of ada_catchpoint "private"
      Don't require inferior execution for Ada catchpoints
      Implement DAP setExceptionBreakpoints request
      Implement DAP attach request
      Implement DAP stepOut request
      Add singleThread support to some DAP requests
      Rename one DAP function
      Add test for DAP pause request
      Fix a latent bug in DAP request decorator
      Use tuples for default arguments in DAP
      Add type-checking to DAP requests
      Add gdb.Value.assign method
      Implement DAP setExpression request
      Handle DAP supportsVariableType capability
      Add "target" parameter to DAP attach request
      Add "stop at main" extension to DAP launch request
      Implement DAP breakpointLocations request
      Do not report totalFrames from DAP stackTrace request
      Implement DAP conditional breakpoints

 gdb/NEWS                                       |   2 +
 gdb/ada-lang.c                                 | 210 +++++++++----------------
 gdb/ada-lang.h                                 |   2 +-
 gdb/data-directory/Makefile.in                 |   2 +
 gdb/doc/gdb.texinfo                            |  17 ++
 gdb/doc/python.texi                            |   6 +
 gdb/mi/mi-cmd-catch.c                          |   4 +-
 gdb/python/lib/gdb/dap/__init__.py             |   1 +
 gdb/python/lib/gdb/dap/breakpoint.py           | 143 ++++++++++++++---
 gdb/python/lib/gdb/dap/bt.py                   |  14 +-
 gdb/python/lib/gdb/dap/disassemble.py          |   7 +-
 gdb/python/lib/gdb/dap/evaluate.py             |  38 ++++-
 gdb/python/lib/gdb/dap/launch.py               |  42 ++++-
 gdb/python/lib/gdb/dap/locations.py            |  45 ++++++
 gdb/python/lib/gdb/dap/memory.py               |   4 +-
 gdb/python/lib/gdb/dap/next.py                 |  51 ++++--
 gdb/python/lib/gdb/dap/scopes.py               |   2 +-
 gdb/python/lib/gdb/dap/server.py               |  24 ++-
 gdb/python/lib/gdb/dap/typecheck.py            |  88 +++++++++++
 gdb/python/lib/gdb/dap/varref.py               |   3 +
 gdb/python/py-value.c                          |  30 ++++
 gdb/testsuite/gdb.ada/catch_assert_if.exp      |  28 +---
 gdb/testsuite/gdb.ada/catch_ex.exp             |  18 +--
 gdb/testsuite/gdb.ada/catch_ex_std.exp         |  27 +---
 gdb/testsuite/gdb.ada/excep_handle.exp         |  24 +--
 gdb/testsuite/gdb.ada/mi_catch_assert.exp      |  33 +---
 gdb/testsuite/gdb.ada/mi_catch_ex.exp          |  31 +---
 gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp     |  32 +---
 gdb/testsuite/gdb.ada/mi_ex_cond.exp           |  33 +---
 gdb/testsuite/gdb.dap/attach.c                 |  25 +++
 gdb/testsuite/gdb.dap/attach.exp               |  36 +++++
 gdb/testsuite/gdb.dap/basic-dap.c              |  12 +-
 gdb/testsuite/gdb.dap/basic-dap.exp            |  35 ++++-
 gdb/testsuite/gdb.dap/catch-exception.exp      |  66 ++++++++
 gdb/testsuite/gdb.dap/catch-exception/pck.ads  |  18 +++
 gdb/testsuite/gdb.dap/catch-exception/prog.adb |  44 ++++++
 gdb/testsuite/gdb.dap/cond-bp.c                |  26 +++
 gdb/testsuite/gdb.dap/cond-bp.exp              |  65 ++++++++
 gdb/testsuite/gdb.dap/pause.exp                |  41 +++++
 gdb/testsuite/gdb.dap/remote-dap.exp           |  49 ++++++
 gdb/testsuite/gdb.dap/stop-at-main.exp         |  37 +++++
 gdb/testsuite/gdb.dap/type_check.exp           |  29 ++++
 gdb/testsuite/gdb.dap/type_check.py            |  96 +++++++++++
 gdb/testsuite/gdb.python/py-value.exp          |  14 ++
 gdb/testsuite/lib/ada.exp                      |   2 +
 gdb/testsuite/lib/dap-support.exp              |  43 ++++-
 46 files changed, 1199 insertions(+), 400 deletions(-)
---
base-commit: 389971df23ca74092314dbde1303310a33766ba7
change-id: 20230427-ada-catch-exception-89d3649d3cad

Best regards,
-- 
Tom Tromey <tromey@adacore.com>


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

* [PATCH 01/25] Stop gdb in gnat_runtime_has_debug_info
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-05-24 16:36 ` [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests Tom Tromey
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

gnat_runtime_has_debug_info starts a new gdb to do its work.  However,
it also leaves this gdb running, which can potentially confuse the
calling test -- I encountered this when writing a new DAP test.  This
patch changes the proc to shut down gdb.
---
 gdb/testsuite/lib/ada.exp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp
index b728e3a2aba..b4a93faefa5 100644
--- a/gdb/testsuite/lib/ada.exp
+++ b/gdb/testsuite/lib/ada.exp
@@ -204,5 +204,7 @@ gdb_caching_proc gnat_runtime_has_debug_info {} {
 	}
     }
 
+    gdb_exit
+
     return $has_debug_info
 }

-- 
2.40.0


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

* [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
  2023-05-24 16:36 ` [PATCH 01/25] Stop gdb in gnat_runtime_has_debug_info Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-06-13 20:10   ` Tom de Vries
  2023-05-24 16:36 ` [PATCH 03/25] Pass tempflag to ada_catchpoint constructor Tom Tromey
                   ` (23 subsequent siblings)
  25 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

This changes the Ada catchpoint tests to use
gnat_runtime_has_debug_info.  This simplifies the code.
---
 gdb/testsuite/gdb.ada/catch_assert_if.exp  | 28 +------------------------
 gdb/testsuite/gdb.ada/catch_ex.exp         | 18 ++++------------
 gdb/testsuite/gdb.ada/catch_ex_std.exp     | 27 ++++++++----------------
 gdb/testsuite/gdb.ada/excep_handle.exp     | 24 +++++-----------------
 gdb/testsuite/gdb.ada/mi_catch_assert.exp  | 33 +-----------------------------
 gdb/testsuite/gdb.ada/mi_catch_ex.exp      | 31 ++--------------------------
 gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp | 32 +----------------------------
 gdb/testsuite/gdb.ada/mi_ex_cond.exp       | 33 +++---------------------------
 8 files changed, 25 insertions(+), 201 deletions(-)

diff --git a/gdb/testsuite/gdb.ada/catch_assert_if.exp b/gdb/testsuite/gdb.ada/catch_assert_if.exp
index f5b6096ecb0..4078fa408d1 100644
--- a/gdb/testsuite/gdb.ada/catch_assert_if.exp
+++ b/gdb/testsuite/gdb.ada/catch_assert_if.exp
@@ -15,7 +15,7 @@
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile bla
 
@@ -25,35 +25,9 @@ if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional
 
 clean_restart ${testfile}
 
-######################################################################
-# 1. Try catching all exceptions to check that runtime supports it.  #
-######################################################################
-
 set eol "\[\r\n\]+"
 set sp "\[ \t\]*"
 
-if {![runto_main]} {
-   return 0
-}
-
-set msg "insert catchpoint on all Ada exceptions"
-gdb_test_multiple "catch exception" $msg {
-    -re "Catchpoint $decimal: all Ada exceptions$eol$gdb_prompt $" {
-	pass $msg
-    }
-    -re "Your Ada runtime appears to be missing some debugging information.*$eol$gdb_prompt $" {
-	# If the runtime was not built with enough debug information,
-	# or if it was stripped, we can not test exception
-	# catchpoints.
-	unsupported $msg
-	return -1
-    }
-}
-
-##################################################
-# 2. Try catching conditional failed assertion.  #
-##################################################
-
 # Here is the scenario:
 #  - Restart the debugger from scratch, runto_main
 #    We'll catch assertions if Global_Var = 2
diff --git a/gdb/testsuite/gdb.ada/catch_ex.exp b/gdb/testsuite/gdb.ada/catch_ex.exp
index de1e53243fb..388eb949e37 100644
--- a/gdb/testsuite/gdb.ada/catch_ex.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex.exp
@@ -15,7 +15,7 @@
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile foo
 
@@ -44,19 +44,9 @@ if {![runto_main]} {
    return 0
 }
 
-set msg "insert catchpoint on all Ada exceptions"
-gdb_test_multiple "catch exception" $msg {
-    -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
-	pass $msg
-    }
-    -re "Your Ada runtime appears to be missing some debugging information.*$eol$gdb_prompt $" {
-	# If the runtime was not built with enough debug information,
-	# or if it was stripped, we can not test exception
-	# catchpoints.
-	unsupported $msg
-	return -1
-    }
-}
+gdb_test "catch exception" \
+    "Catchpoint $any_nb: all Ada exceptions" \
+    "insert catchpoint on all Ada exceptions"
 
 gdb_test "info break" \
          "$info_break_header$eol.*$catch_exception_info" \
diff --git a/gdb/testsuite/gdb.ada/catch_ex_std.exp b/gdb/testsuite/gdb.ada/catch_ex_std.exp
index 73cbdaf90ca..3a2b10a8c13 100644
--- a/gdb/testsuite/gdb.ada/catch_ex_std.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex_std.exp
@@ -17,7 +17,7 @@ require allow_shlib_tests
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile foo
 
@@ -69,23 +69,12 @@ if {![runto_main]} {
    return 0
 }
 
-set can_catch_exceptions 0
-gdb_test_multiple "catch exception some_kind_of_error" "" {
-    -re "Catchpoint \[0-9\]+: `some_kind_of_error' Ada exception\r\n$gdb_prompt $" {
-	pass $gdb_test_name
-	set can_catch_exceptions 1
-    }
+gdb_test "catch exception some_kind_of_error" \
+    "Catchpoint \[0-9\]+: `some_kind_of_error' Ada exception"
 
-    -re "Your Ada runtime appears to be missing some debugging information.\r\nCannot insert Ada exception catchpoint in this configuration.\r\n$gdb_prompt $" {
-	    unsupported $gdb_test_name
-    }
-}
-
-if { $can_catch_exceptions } {
-    gdb_test "cont" \
-	"Catchpoint \[0-9\]+, .* at .*foo\.adb:\[0-9\]+.*" \
-	"caught the exception"
+gdb_test "cont" \
+    "Catchpoint \[0-9\]+, .* at .*foo\.adb:\[0-9\]+.*" \
+    "caught the exception"
 
-    gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
-	" = true"
-}
+gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
+    " = true"
diff --git a/gdb/testsuite/gdb.ada/excep_handle.exp b/gdb/testsuite/gdb.ada/excep_handle.exp
index 2cc80fbb988..deb3ace2987 100644
--- a/gdb/testsuite/gdb.ada/excep_handle.exp
+++ b/gdb/testsuite/gdb.ada/excep_handle.exp
@@ -15,7 +15,7 @@
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile foo
 
@@ -40,32 +40,18 @@ set catchpoint_program_error_msg \
 set catchpoint_storage_error_msg \
   "Catchpoint $decimal, exception at $hex in foo \\\(\\\).*at .*foo.adb:$decimal$eol$decimal$sp$when Storage_Error =>"
 
-############################################
-# Check that runtime supports catchpoint.  #
-############################################
-
 if {![runto_main]} {
    return 0
 }
 
-set msg "insert catchpoint on all Ada exceptions handlers"
-gdb_test_multiple "catch handlers" $msg {
-    -re "Catchpoint $decimal: all Ada exceptions handlers$eol$gdb_prompt $" {
-	pass $msg
-    }
-    -re "Your Ada runtime appears to be missing some debugging information.*$eol$gdb_prompt $" {
-	# If the runtime was not built with enough debug information,
-	# or if it was stripped, we can not test exception handlers
-	# catchpoints.
-	unsupported $msg
-	return -1
-    }
-}
-
 ############################################
 # 1. Try catching all exceptions handlers. #
 ############################################
 
+gdb_test "catch handlers" \
+    "Catchpoint $decimal: all Ada exceptions handlers" \
+    "insert catchpoint on all Ada exceptions handlers"
+
 # Continue.  The program should stop at first exception handling.
 
 gdb_test "continue" \
diff --git a/gdb/testsuite/gdb.ada/mi_catch_assert.exp b/gdb/testsuite/gdb.ada/mi_catch_assert.exp
index c3673fab8e8..7586e0a4256 100644
--- a/gdb/testsuite/gdb.ada/mi_catch_assert.exp
+++ b/gdb/testsuite/gdb.ada/mi_catch_assert.exp
@@ -15,7 +15,7 @@
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile bla
 
@@ -23,37 +23,6 @@ if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional
   return -1
 }
 
-# Some global variables used to simplify the maintenance of some of
-# the regular expressions below.
-set eol "\[\r\n\]+"
-set sp "\[ \t\]*"
-
-# Before going any further, verify that we can insert exception
-# catchpoints...  That way, we won't have to do this while doing
-# the actual GDB/MI testing.
-
-clean_restart ${testfile}
-
-if {![runto_main]} {
-   return 0
-}
-
-set msg "insert catchpoint on all Ada exceptions"
-gdb_test_multiple "catch exception" $msg {
-    -re "Catchpoint $decimal: all Ada exceptions$eol$gdb_prompt $" {
-	pass $msg
-    }
-    -re "Your Ada runtime appears to be missing some debugging information.*\[\r\n\]+$gdb_prompt $" {
-	# If the runtime was not built with enough debug information,
-	# or if it was stripped, we can not test exception
-	# catchpoints.
-	unsupported $msg
-	return -1
-    }
-}
-
-# Now, we can start the GDB/MI testing itself...
-
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
 
diff --git a/gdb/testsuite/gdb.ada/mi_catch_ex.exp b/gdb/testsuite/gdb.ada/mi_catch_ex.exp
index d0dfbd59764..8cbcbe7e1a8 100644
--- a/gdb/testsuite/gdb.ada/mi_catch_ex.exp
+++ b/gdb/testsuite/gdb.ada/mi_catch_ex.exp
@@ -15,7 +15,7 @@
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile foo
 
@@ -23,36 +23,9 @@ if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional
   return -1
 }
 
-# Some global variables used to simplify the maintenance of some of
+# A global variable used to simplify the maintenance of some of
 # the regular expressions below.
 set any_nb "\[0-9\]+"
-set eol "\[\r\n\]+"
-
-# Before going any further, verify that we can insert exception
-# catchpoints...  That way, we won't have to do this while doing
-# the actual GDB/MI testing.
-
-clean_restart ${testfile}
-
-if {![runto_main]} {
-   return 0
-}
-
-set msg "insert catchpoint on all Ada exceptions"
-gdb_test_multiple "catch exception" $msg {
-    -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
-	pass $msg
-    }
-    -re "Your Ada runtime appears to be missing some debugging information.*\[\r\n\]+$gdb_prompt $" {
-	# If the runtime was not built with enough debug information,
-	# or if it was stripped, we can not test exception
-	# catchpoints.
-	unsupported $msg
-	return -1
-    }
-}
-
-# Now, we can start the GDB/MI testing itself...
 
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
diff --git a/gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp b/gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp
index 2cd175f098b..ae081448553 100644
--- a/gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp
+++ b/gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp
@@ -15,7 +15,7 @@
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile foo
 
@@ -23,36 +23,6 @@ if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional
   return -1
 }
 
-# A global variable used to simplify the maintenance of some of
-# the regular expressions below.
-set eol "\[\r\n\]+"
-
-# Before going any further, verify that we can insert exception
-# handlers catchpoints...  That way, we won't have to do this while
-# doing the actual GDB/MI testing.
-
-clean_restart ${testfile}
-
-if {![runto_main]} {
-   return 0
-}
-
-set msg "insert catchpoint on all Ada exceptions handlers"
-gdb_test_multiple "catch handlers" $msg {
-    -re "Catchpoint $decimal: all Ada exceptions handlers$eol$gdb_prompt $" {
-	pass $msg
-    }
-    -re "Your Ada runtime appears to be missing some debugging information.*\[\r\n\]+$gdb_prompt $" {
-	# If the runtime was not built with enough debug information,
-	# or if it was stripped, we can not test exception
-	# catchpoints.
-	unsupported $msg
-	return -1
-    }
-}
-
-# Now, we can start the GDB/MI testing itself...
-
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
 
diff --git a/gdb/testsuite/gdb.ada/mi_ex_cond.exp b/gdb/testsuite/gdb.ada/mi_ex_cond.exp
index 54aa6b54ee1..c0bc079f5f9 100644
--- a/gdb/testsuite/gdb.ada/mi_ex_cond.exp
+++ b/gdb/testsuite/gdb.ada/mi_ex_cond.exp
@@ -15,7 +15,7 @@
 
 load_lib "ada.exp"
 
-require allow_ada_tests
+require allow_ada_tests gnat_runtime_has_debug_info
 
 standard_ada_testfile foo
 
@@ -23,36 +23,9 @@ if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional
   return -1
 }
 
-# # Some global variables used to simplify the maintenance of some of
-# # the regular expressions below.
+# A global variable used to simplify the maintenance of some of
+# the regular expressions below.
 set any_nb "\[0-9\]+"
-set eol "\[\r\n\]+"
-
-# Before going any further, verify that we can insert exception
-# catchpoints...  That way, we won't have to do this while doing
-# the actual GDB/MI testing.
-
-clean_restart ${testfile}
-
-if {![runto_main]} {
-   return 0
-}
-
-set msg "insert catchpoint on all Ada exceptions"
-gdb_test_multiple "catch exception" $msg {
-    -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
-	pass $msg
-    }
-    -re "Your Ada runtime appears to be missing some debugging information.*\[\r\n\]+$gdb_prompt $" {
-	# If the runtime was not built with enough debug information,
-	# or if it was stripped, we can not test exception
-	# catchpoints.
-	unsupported $msg
-	return -1
-    }
-}
-
-# Now, we can start the GDB/MI testing itself...
 
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"

-- 
2.40.0


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

* [PATCH 03/25] Pass tempflag to ada_catchpoint constructor
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
  2023-05-24 16:36 ` [PATCH 01/25] Stop gdb in gnat_runtime_has_debug_info Tom Tromey
  2023-05-24 16:36 ` [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-05-24 16:36 ` [PATCH 04/25] Transfer ownership of exception string to ada_catchpoint Tom Tromey
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

This is a minor cleanup to pass tempflag to the ada_catchpoint
constructor.
---
 gdb/ada-lang.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 21f3348a161..50eea4860ba 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12140,7 +12140,7 @@ struct ada_catchpoint : public code_breakpoint
 		  bool tempflag,
 		  bool enabled,
 		  bool from_tty)
-    : code_breakpoint (gdbarch_, bp_catchpoint),
+    : code_breakpoint (gdbarch_, bp_catchpoint, tempflag),
       m_kind (kind)
   {
     add_location (sal);
@@ -12169,7 +12169,6 @@ struct ada_catchpoint : public code_breakpoint
       }
 
     enable_state = enabled ? bp_enabled : bp_disabled;
-    disposition = tempflag ? disp_del : disp_donttouch;
     locspec = string_to_location_spec (&addr_string_,
 				       language_def (language_ada));
     language = language_ada;

-- 
2.40.0


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

* [PATCH 04/25] Transfer ownership of exception string to ada_catchpoint
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (2 preceding siblings ...)
  2023-05-24 16:36 ` [PATCH 03/25] Pass tempflag to ada_catchpoint constructor Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-05-24 16:36 ` [PATCH 05/25] Combine create_excep_cond_exprs and ada_catchpoint::re_set Tom Tromey
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

This changes the ada_catchpoint to require an rvalue ref, so that
ownership of the exception string can be transferred to the catchpoint
object.
---
 gdb/ada-lang.c        | 16 +++++++++-------
 gdb/ada-lang.h        |  2 +-
 gdb/mi/mi-cmd-catch.c |  4 ++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 50eea4860ba..374cec57090 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12139,8 +12139,10 @@ struct ada_catchpoint : public code_breakpoint
 		  const char *addr_string_,
 		  bool tempflag,
 		  bool enabled,
-		  bool from_tty)
+		  bool from_tty,
+		  std::string &&excep_string_)
     : code_breakpoint (gdbarch_, bp_catchpoint, tempflag),
+      excep_string (std::move (excep_string_)),
       m_kind (kind)
   {
     add_location (sal);
@@ -12807,7 +12809,7 @@ ada_exception_sal (enum ada_exception_catchpoint_kind ex,
 void
 create_ada_exception_catchpoint (struct gdbarch *gdbarch,
 				 enum ada_exception_catchpoint_kind ex_kind,
-				 const std::string &excep_string,
+				 std::string &&excep_string,
 				 const std::string &cond_string,
 				 int tempflag,
 				 int enabled,
@@ -12818,8 +12820,8 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
 
   std::unique_ptr<ada_catchpoint> c
     (new ada_catchpoint (gdbarch, ex_kind, sal, addr_string.c_str (),
-			 tempflag, enabled, from_tty));
-  c->excep_string = excep_string;
+			 tempflag, enabled, from_tty,
+			 std::move (excep_string)));
   create_excep_cond_exprs (c.get (), ex_kind);
   if (!cond_string.empty ())
     set_breakpoint_condition (c.get (), cond_string.c_str (), from_tty, false);
@@ -12846,7 +12848,7 @@ catch_ada_exception_command (const char *arg_entry, int from_tty,
   catch_ada_exception_command_split (arg, false, &ex_kind, &excep_string,
 				     &cond_string);
   create_ada_exception_catchpoint (gdbarch, ex_kind,
-				   excep_string, cond_string,
+				   std::move (excep_string), cond_string,
 				   tempflag, 1 /* enabled */,
 				   from_tty);
 }
@@ -12871,7 +12873,7 @@ catch_ada_handlers_command (const char *arg_entry, int from_tty,
   catch_ada_exception_command_split (arg, true, &ex_kind, &excep_string,
 				     &cond_string);
   create_ada_exception_catchpoint (gdbarch, ex_kind,
-				   excep_string, cond_string,
+				   std::move (excep_string), cond_string,
 				   tempflag, 1 /* enabled */,
 				   from_tty);
 }
@@ -12938,7 +12940,7 @@ catch_assert_command (const char *arg_entry, int from_tty,
     arg = "";
   catch_ada_assert_command_split (arg, cond_string);
   create_ada_exception_catchpoint (gdbarch, ada_catch_assert,
-				   "", cond_string,
+				   {}, cond_string,
 				   tempflag, 1 /* enabled */,
 				   from_tty);
 }
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index f5bb54c7d36..08620d7ed7c 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -341,7 +341,7 @@ extern const char *ada_main_name ();
 
 extern void create_ada_exception_catchpoint
   (struct gdbarch *gdbarch, enum ada_exception_catchpoint_kind ex_kind,
-   const std::string &excep_string, const std::string &cond_string, int tempflag,
+   std::string &&excep_string, const std::string &cond_string, int tempflag,
    int enabled, int from_tty);
 
 /* Return true if BP is an Ada catchpoint.  */
diff --git a/gdb/mi/mi-cmd-catch.c b/gdb/mi/mi-cmd-catch.c
index 082703740af..dcd48a84a5d 100644
--- a/gdb/mi/mi-cmd-catch.c
+++ b/gdb/mi/mi-cmd-catch.c
@@ -153,7 +153,7 @@ mi_cmd_catch_exception (const char *cmd, char *argv[], int argc)
 
   scoped_restore restore_breakpoint_reporting = setup_breakpoint_reporting ();
   create_ada_exception_catchpoint (gdbarch, ex_kind,
-				   exception_name,
+				   std::move (exception_name),
 				   condition, temp, enabled, 0);
 }
 
@@ -217,7 +217,7 @@ mi_cmd_catch_handlers (const char *cmd, char *argv[], int argc)
   scoped_restore restore_breakpoint_reporting
     = setup_breakpoint_reporting ();
   create_ada_exception_catchpoint (gdbarch, ada_catch_handlers,
-				   exception_name,
+				   std::move (exception_name),
 				   condition, temp, enabled, 0);
 }
 

-- 
2.40.0


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

* [PATCH 05/25] Combine create_excep_cond_exprs and ada_catchpoint::re_set
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (3 preceding siblings ...)
  2023-05-24 16:36 ` [PATCH 04/25] Transfer ownership of exception string to ada_catchpoint Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-05-24 16:36 ` [PATCH 06/25] Turn should_stop_exception into a method of ada_catchpoint Tom Tromey
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

This patch merges create_excep_cond_exprs into ada_catchpoint::re_set.
This is less verbose and is also a step toward making ada_catchpoint
work more like the other code_breakpoint-based exception catchpoints.
---
 gdb/ada-lang.c | 44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 374cec57090..24ed93f2004 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12174,6 +12174,8 @@ struct ada_catchpoint : public code_breakpoint
     locspec = string_to_location_spec (&addr_string_,
 				       language_def (language_ada));
     language = language_ada;
+
+    re_set ();
   }
 
   struct bp_location *allocate_location () override;
@@ -12207,29 +12209,35 @@ class ada_catchpoint_location : public bp_location
   expression_up excep_cond_expr;
 };
 
-/* Parse the exception condition string in the context of each of the
-   catchpoint's locations, and store them for later evaluation.  */
+/* Implement the RE_SET method in the structure for all exception
+   catchpoint kinds.  */
 
-static void
-create_excep_cond_exprs (struct ada_catchpoint *c,
-			 enum ada_exception_catchpoint_kind ex)
+void
+ada_catchpoint::re_set ()
 {
+  /* Call the base class's method.  This updates the catchpoint's
+     locations.  */
+  this->code_breakpoint::re_set ();
+
+  /* Reparse the exception conditional expressions.  One for each
+     location.  */
+
   /* Nothing to do if there's no specific exception to catch.  */
-  if (c->excep_string.empty ())
+  if (excep_string.empty ())
     return;
 
   /* Same if there are no locations... */
-  if (c->loc == NULL)
+  if (loc == NULL)
     return;
 
   /* Compute the condition expression in text form, from the specific
      expection we want to catch.  */
   std::string cond_string
-    = ada_exception_catchpoint_cond_string (c->excep_string.c_str (), ex);
+    = ada_exception_catchpoint_cond_string (excep_string.c_str (), m_kind);
 
   /* Iterate over all the catchpoint's locations, and parse an
      expression for each.  */
-  for (bp_location *bl : c->locations ())
+  for (bp_location *bl : locations ())
     {
       struct ada_catchpoint_location *ada_loc
 	= (struct ada_catchpoint_location *) bl;
@@ -12250,7 +12258,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c,
 	    {
 	      warning (_("failed to reevaluate internal exception condition "
 			 "for catchpoint %d: %s"),
-		       c->number, e.what ());
+		       number, e.what ());
 	    }
 	}
 
@@ -12267,21 +12275,6 @@ ada_catchpoint::allocate_location ()
   return new ada_catchpoint_location (this);
 }
 
-/* Implement the RE_SET method in the structure for all exception
-   catchpoint kinds.  */
-
-void
-ada_catchpoint::re_set ()
-{
-  /* Call the base class's method.  This updates the catchpoint's
-     locations.  */
-  this->code_breakpoint::re_set ();
-
-  /* Reparse the exception conditional expressions.  One for each
-     location.  */
-  create_excep_cond_exprs (this, m_kind);
-}
-
 /* Returns true if we should stop for this breakpoint hit.  If the
    user specified a specific exception, we only want to cause a stop
    if the program thrown that exception.  */
@@ -12822,7 +12815,6 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
     (new ada_catchpoint (gdbarch, ex_kind, sal, addr_string.c_str (),
 			 tempflag, enabled, from_tty,
 			 std::move (excep_string)));
-  create_excep_cond_exprs (c.get (), ex_kind);
   if (!cond_string.empty ())
     set_breakpoint_condition (c.get (), cond_string.c_str (), from_tty, false);
   install_breakpoint (0, std::move (c), 1);

-- 
2.40.0


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

* [PATCH 06/25] Turn should_stop_exception into a method of ada_catchpoint
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (4 preceding siblings ...)
  2023-05-24 16:36 ` [PATCH 05/25] Combine create_excep_cond_exprs and ada_catchpoint::re_set Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-05-24 16:36 ` [PATCH 07/25] Mark members of ada_catchpoint "private" Tom Tromey
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

This turns the should_stop_exception function in ada-lang.c into a
method of ada_catchpoint.
---
 gdb/ada-lang.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 24ed93f2004..d8c7492551c 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12186,6 +12186,12 @@ struct ada_catchpoint : public code_breakpoint
   void print_mention () const override;
   void print_recreate (struct ui_file *fp) const override;
 
+  /* A helper function for check_status.  Returns true if we should
+     stop for this breakpoint hit.  If the user specified a specific
+     exception, we only want to cause a stop if the program thrown
+     that exception.  */
+  bool should_stop_exception (const struct bp_location *bl) const;
+
   /* The name of the specific exception the user specified.  */
   std::string excep_string;
 
@@ -12275,12 +12281,10 @@ ada_catchpoint::allocate_location ()
   return new ada_catchpoint_location (this);
 }
 
-/* Returns true if we should stop for this breakpoint hit.  If the
-   user specified a specific exception, we only want to cause a stop
-   if the program thrown that exception.  */
+/* See declaration.  */
 
-static bool
-should_stop_exception (const struct bp_location *bl)
+bool
+ada_catchpoint::should_stop_exception (const struct bp_location *bl) const
 {
   struct ada_catchpoint *c = (struct ada_catchpoint *) bl->owner;
   const struct ada_catchpoint_location *ada_loc

-- 
2.40.0


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

* [PATCH 07/25] Mark members of ada_catchpoint "private"
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (5 preceding siblings ...)
  2023-05-24 16:36 ` [PATCH 06/25] Turn should_stop_exception into a method of ada_catchpoint Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-05-24 16:36 ` [PATCH 08/25] Don't require inferior execution for Ada catchpoints Tom Tromey
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

This changes the members of ada_catchpoint to be private.
---
 gdb/ada-lang.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d8c7492551c..6ee507147cb 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12142,7 +12142,7 @@ struct ada_catchpoint : public code_breakpoint
 		  bool from_tty,
 		  std::string &&excep_string_)
     : code_breakpoint (gdbarch_, bp_catchpoint, tempflag),
-      excep_string (std::move (excep_string_)),
+      m_excep_string (std::move (excep_string_)),
       m_kind (kind)
   {
     add_location (sal);
@@ -12186,6 +12186,8 @@ struct ada_catchpoint : public code_breakpoint
   void print_mention () const override;
   void print_recreate (struct ui_file *fp) const override;
 
+private:
+
   /* A helper function for check_status.  Returns true if we should
      stop for this breakpoint hit.  If the user specified a specific
      exception, we only want to cause a stop if the program thrown
@@ -12193,7 +12195,7 @@ struct ada_catchpoint : public code_breakpoint
   bool should_stop_exception (const struct bp_location *bl) const;
 
   /* The name of the specific exception the user specified.  */
-  std::string excep_string;
+  std::string m_excep_string;
 
   /* What kind of catchpoint this is.  */
   enum ada_exception_catchpoint_kind m_kind;
@@ -12229,7 +12231,7 @@ ada_catchpoint::re_set ()
      location.  */
 
   /* Nothing to do if there's no specific exception to catch.  */
-  if (excep_string.empty ())
+  if (m_excep_string.empty ())
     return;
 
   /* Same if there are no locations... */
@@ -12239,7 +12241,7 @@ ada_catchpoint::re_set ()
   /* Compute the condition expression in text form, from the specific
      expection we want to catch.  */
   std::string cond_string
-    = ada_exception_catchpoint_cond_string (excep_string.c_str (), m_kind);
+    = ada_exception_catchpoint_cond_string (m_excep_string.c_str (), m_kind);
 
   /* Iterate over all the catchpoint's locations, and parse an
      expression for each.  */
@@ -12316,7 +12318,7 @@ ada_catchpoint::should_stop_exception (const struct bp_location *bl) const
     }
 
   /* With no specific exception, should always stop.  */
-  if (c->excep_string.empty ())
+  if (c->m_excep_string.empty ())
     return true;
 
   if (ada_loc->excep_cond_expr == NULL)
@@ -12456,10 +12458,10 @@ ada_catchpoint::print_one (bp_location **last_loc) const
   switch (m_kind)
     {
       case ada_catch_exception:
-	if (!excep_string.empty ())
+	if (!m_excep_string.empty ())
 	  {
 	    std::string msg = string_printf (_("`%s' Ada exception"),
-					     excep_string.c_str ());
+					     m_excep_string.c_str ());
 
 	    uiout->field_string ("what", msg);
 	  }
@@ -12473,11 +12475,11 @@ ada_catchpoint::print_one (bp_location **last_loc) const
 	break;
       
       case ada_catch_handlers:
-	if (!excep_string.empty ())
+	if (!m_excep_string.empty ())
 	  {
 	    uiout->field_fmt ("what",
 			      _("`%s' Ada exception handlers"),
-			      excep_string.c_str ());
+			      m_excep_string.c_str ());
 	  }
 	else
 	  uiout->field_string ("what", "all Ada exceptions handlers");
@@ -12511,10 +12513,10 @@ ada_catchpoint::print_mention () const
   switch (m_kind)
     {
       case ada_catch_exception:
-	if (!excep_string.empty ())
+	if (!m_excep_string.empty ())
 	  {
 	    std::string info = string_printf (_("`%s' Ada exception"),
-					      excep_string.c_str ());
+					      m_excep_string.c_str ());
 	    uiout->text (info);
 	  }
 	else
@@ -12526,11 +12528,11 @@ ada_catchpoint::print_mention () const
 	break;
 
       case ada_catch_handlers:
-	if (!excep_string.empty ())
+	if (!m_excep_string.empty ())
 	  {
 	    std::string info
 	      = string_printf (_("`%s' Ada exception handlers"),
-			       excep_string.c_str ());
+			       m_excep_string.c_str ());
 	    uiout->text (info);
 	  }
 	else
@@ -12557,8 +12559,8 @@ ada_catchpoint::print_recreate (struct ui_file *fp) const
     {
       case ada_catch_exception:
 	gdb_printf (fp, "catch exception");
-	if (!excep_string.empty ())
-	  gdb_printf (fp, " %s", excep_string.c_str ());
+	if (!m_excep_string.empty ())
+	  gdb_printf (fp, " %s", m_excep_string.c_str ());
 	break;
 
       case ada_catch_exception_unhandled:

-- 
2.40.0


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

* [PATCH 08/25] Don't require inferior execution for Ada catchpoints
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (6 preceding siblings ...)
  2023-05-24 16:36 ` [PATCH 07/25] Mark members of ada_catchpoint "private" Tom Tromey
@ 2023-05-24 16:36 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 09/25] Implement DAP setExceptionBreakpoints request Tom Tromey
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:36 UTC (permalink / raw)
  To: gdb-patches

Currently, Ada catchpoints require that the inferior be running.
However, there's no deep reason for this -- for example, C++ exception
catchpoints do not have this requirement.  Instead, those work like
ordinary breakpoints: they are pending until the needed runtime
locations are seen.

This patch changes Ada catchpoints to work the same way.
---
 gdb/ada-lang.c | 115 +++++++++++++++------------------------------------------
 1 file changed, 29 insertions(+), 86 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 6ee507147cb..f9d5965f055 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -790,18 +790,6 @@ ada_get_decoded_type (struct type *type)
 
 				/* Language Selection */
 
-/* If the main program is in Ada, return language_ada, otherwise return LANG
-   (the main program is in Ada iif the adainit symbol is found).  */
-
-static enum language
-ada_update_initial_language (enum language lang)
-{
-  if (lookup_minimal_symbol ("adainit", NULL, NULL).minsym != NULL)
-    return language_ada;
-
-  return lang;
-}
-
 /* If the main procedure is written in Ada, then return its name.
    The result is good until the next call.  Return NULL if the main
    procedure doesn't appear to be in Ada.  */
@@ -11827,31 +11815,8 @@ ada_exception_support_info_sniffer (void)
       return;
     }
 
-  /* Sometimes, it is normal for us to not be able to find the routine
-     we are looking for.  This happens when the program is linked with
-     the shared version of the GNAT runtime, and the program has not been
-     started yet.  Inform the user of these two possible causes if
-     applicable.  */
-
-  if (ada_update_initial_language (language_unknown) != language_ada)
-    error (_("Unable to insert catchpoint.  Is this an Ada main program?"));
-
-  /* If the symbol does not exist, then check that the program is
-     already started, to make sure that shared libraries have been
-     loaded.  If it is not started, this may mean that the symbol is
-     in a shared library.  */
-
-  if (inferior_ptid.pid () == 0)
-    error (_("Unable to insert catchpoint. Try to start the program first."));
-
-  /* At this point, we know that we are debugging an Ada program and
-     that the inferior has been started, but we still are not able to
-     find the run-time symbols.  That can mean that we are in
-     configurable run time mode, or that a-except as been optimized
-     out by the linker...  In any case, at this point it is not worth
-     supporting this feature.  */
-
-  error (_("Cannot insert Ada exception catchpoints in this configuration."));
+  throw_error (NOT_FOUND_ERROR,
+	       _("Could not find Ada runtime exception support"));
 }
 
 /* True iff FRAME is very likely to be that of a function that is
@@ -12135,44 +12100,19 @@ struct ada_catchpoint : public code_breakpoint
 {
   ada_catchpoint (struct gdbarch *gdbarch_,
 		  enum ada_exception_catchpoint_kind kind,
-		  struct symtab_and_line sal,
-		  const char *addr_string_,
+		  const char *cond_string,
 		  bool tempflag,
 		  bool enabled,
 		  bool from_tty,
 		  std::string &&excep_string_)
-    : code_breakpoint (gdbarch_, bp_catchpoint, tempflag),
+    : code_breakpoint (gdbarch_, bp_catchpoint, tempflag, cond_string),
       m_excep_string (std::move (excep_string_)),
       m_kind (kind)
   {
-    add_location (sal);
-
     /* Unlike most code_breakpoint types, Ada catchpoints are
        pspace-specific.  */
-    gdb_assert (sal.pspace != nullptr);
-    this->pspace = sal.pspace;
-
-    if (from_tty)
-      {
-	struct gdbarch *loc_gdbarch = get_sal_arch (sal);
-	if (!loc_gdbarch)
-	  loc_gdbarch = gdbarch;
-
-	describe_other_breakpoints (loc_gdbarch,
-				    sal.pspace, sal.pc, sal.section, -1);
-	/* FIXME: brobecker/2006-12-28: Actually, re-implement a special
-	   version for exception catchpoints, because two catchpoints
-	   used for different exception names will use the same address.
-	   In this case, a "breakpoint ... also set at..." warning is
-	   unproductive.  Besides, the warning phrasing is also a bit
-	   inappropriate, we should use the word catchpoint, and tell
-	   the user what type of catchpoint it is.  The above is good
-	   enough for now, though.  */
-      }
-
+    pspace = current_program_space;
     enable_state = enabled ? bp_enabled : bp_disabled;
-    locspec = string_to_location_spec (&addr_string_,
-				       language_def (language_ada));
     language = language_ada;
 
     re_set ();
@@ -12217,15 +12157,29 @@ class ada_catchpoint_location : public bp_location
   expression_up excep_cond_expr;
 };
 
+static struct symtab_and_line ada_exception_sal
+     (enum ada_exception_catchpoint_kind ex);
+
 /* Implement the RE_SET method in the structure for all exception
    catchpoint kinds.  */
 
 void
 ada_catchpoint::re_set ()
 {
-  /* Call the base class's method.  This updates the catchpoint's
-     locations.  */
-  this->code_breakpoint::re_set ();
+  std::vector<symtab_and_line> sals;
+  try
+    {
+      struct symtab_and_line sal = ada_exception_sal (m_kind);
+      sals.push_back (sal);
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      /* For NOT_FOUND_ERROR, the breakpoint will be pending.  */
+      if (ex.error != NOT_FOUND_ERROR)
+	throw;
+    }
+
+  update_breakpoint_locations (this, pspace, sals, {});
 
   /* Reparse the exception conditional expressions.  One for each
      location.  */
@@ -12756,16 +12710,11 @@ ada_exception_catchpoint_cond_string (const char *excep_string,
   return result;
 }
 
-/* Return the symtab_and_line that should be used to insert an exception
-   catchpoint of the TYPE kind.
-
-   ADDR_STRING returns the name of the function where the real
-   breakpoint that implements the catchpoints is set, depending on the
-   type of catchpoint we need to create.  */
+/* Return the symtab_and_line that should be used to insert an
+   exception catchpoint of the TYPE kind.  */
 
 static struct symtab_and_line
-ada_exception_sal (enum ada_exception_catchpoint_kind ex,
-		   std::string *addr_string)
+ada_exception_sal (enum ada_exception_catchpoint_kind ex)
 {
   const char *sym_name;
   struct symbol *sym;
@@ -12779,14 +12728,12 @@ ada_exception_sal (enum ada_exception_catchpoint_kind ex,
   sym = standard_lookup (sym_name, NULL, VAR_DOMAIN);
 
   if (sym == NULL)
-    error (_("Catchpoint symbol not found: %s"), sym_name);
+    throw_error (NOT_FOUND_ERROR, _("Catchpoint symbol not found: %s"),
+		 sym_name);
 
   if (sym->aclass () != LOC_BLOCK)
     error (_("Unable to insert catchpoint. %s is not a function."), sym_name);
 
-  /* Set ADDR_STRING.  */
-  *addr_string = sym_name;
-
   return find_function_start_sal (sym, 1);
 }
 
@@ -12814,15 +12761,11 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
 				 int enabled,
 				 int from_tty)
 {
-  std::string addr_string;
-  struct symtab_and_line sal = ada_exception_sal (ex_kind, &addr_string);
-
   std::unique_ptr<ada_catchpoint> c
-    (new ada_catchpoint (gdbarch, ex_kind, sal, addr_string.c_str (),
+    (new ada_catchpoint (gdbarch, ex_kind,
+			 cond_string.empty () ? nullptr : cond_string.c_str (),
 			 tempflag, enabled, from_tty,
 			 std::move (excep_string)));
-  if (!cond_string.empty ())
-    set_breakpoint_condition (c.get (), cond_string.c_str (), from_tty, false);
   install_breakpoint (0, std::move (c), 1);
 }
 

-- 
2.40.0


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

* [PATCH 09/25] Implement DAP setExceptionBreakpoints request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (7 preceding siblings ...)
  2023-05-24 16:36 ` [PATCH 08/25] Don't require inferior execution for Ada catchpoints Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 10/25] Implement DAP attach request Tom Tromey
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

This implements the DAP setExceptionBreakpoints request for Ada.  This
is a somewhat minimal implementation, in that "exceptionOptions" are
not implemented (or advertised) -- I wasn't completely sure how this
feature is supposed to work.

I haven't added C++ exception handling here, but it's easy to do if
needed.

This patch relies on the new MI command execution support to do its
work.
---
 gdb/python/lib/gdb/dap/breakpoint.py           | 65 ++++++++++++++++++++++++--
 gdb/python/lib/gdb/dap/server.py               |  4 +-
 gdb/testsuite/gdb.dap/catch-exception.exp      | 65 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/catch-exception/pck.ads  | 18 +++++++
 gdb/testsuite/gdb.dap/catch-exception/prog.adb | 44 +++++++++++++++++
 5 files changed, 189 insertions(+), 7 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index f0e1f103d1b..877069f79a5 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -37,12 +37,11 @@ def breakpoint_descriptor(bp):
         # https://github.com/microsoft/debug-adapter-protocol/issues/13
         loc = bp.locations[0]
         (basename, line) = loc.source
-        return {
+        result = {
             "id": bp.number,
             "verified": True,
             "source": {
                 "name": os.path.basename(basename),
-                "path": loc.fullname,
                 # We probably don't need this but it doesn't hurt to
                 # be explicit.
                 "sourceReference": 0,
@@ -50,6 +49,10 @@ def breakpoint_descriptor(bp):
             "line": line,
             "instructionReference": hex(loc.address),
         }
+        path = loc.fullname
+        if path is not None:
+            result["source"]["path"] = path
+        return result
     else:
         return {
             "id": bp.number,
@@ -58,9 +61,10 @@ def breakpoint_descriptor(bp):
 
 
 # Helper function to set some breakpoints according to a list of
-# specifications.
+# specifications and a callback function to do the work of creating
+# the breakpoint.
 @in_gdb_thread
-def _set_breakpoints(kind, specs):
+def _set_breakpoints_callback(kind, specs, creator):
     global breakpoint_map
     # Try to reuse existing breakpoints if possible.
     if kind in breakpoint_map:
@@ -75,7 +79,7 @@ def _set_breakpoints(kind, specs):
             bp = saved_map.pop(keyspec)
         else:
             # FIXME handle exceptions here
-            bp = gdb.Breakpoint(**spec)
+            bp = creator(**spec)
         breakpoint_map[kind][keyspec] = bp
         result.append(breakpoint_descriptor(bp))
     # Delete any breakpoints that were not reused.
@@ -84,6 +88,13 @@ def _set_breakpoints(kind, specs):
     return result
 
 
+# Helper function to set odinary breakpoints according to a list of
+# specifications.
+@in_gdb_thread
+def _set_breakpoints(kind, specs):
+    return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
+
+
 @request("setBreakpoints")
 def set_breakpoint(*, source, breakpoints=[], **args):
     if "path" not in source:
@@ -141,3 +152,47 @@ def set_insn_breakpoints(*, breakpoints, offset=None, **args):
     return {
         "breakpoints": result,
     }
+
+
+@in_gdb_thread
+def _catch_exception(filterId, condition=None, **args):
+    if filterId == "assert":
+        args = ["-catch-assert"]
+    elif filterId == "exception":
+        args = ["-catch-exception"]
+    else:
+        raise Exception(f"Invalid exception filterID: {filterId}")
+    if condition is not None:
+        args.extend(["-c", condition])
+    result = gdb.execute_mi(*args)
+    # A little lame that there's no more direct way.
+    for bp in gdb.breakpoints():
+        if bp.number == result["bkptno"]:
+            return bp
+    raise Exception("Could not find catchpoint after creating")
+
+
+@in_gdb_thread
+def _set_exception_catchpoints(filter_options):
+    return _set_breakpoints_callback("exception", filter_options, _catch_exception)
+
+
+@request("setExceptionBreakpoints")
+@capability("supportsExceptionFilterOptions")
+@capability("exceptionBreakpointFilters", ({
+    "filter": "assert",
+    "label": "Ada assertions",
+    "supportsCondition": True,
+}, {
+    "filter": "exception",
+    "label": "Ada exceptions",
+    "supportsCondition": True,
+}))
+def set_exception_breakpoints(*, filters, filterOptions=[], **args):
+    # Convert the 'filters' to the filter-options style.
+    options = [{"filterId": filter} for filter in filters]
+    options.extend(filterOptions)
+    result = send_gdb_with_response(lambda: _set_exception_catchpoints(options))
+    return {
+        "breakpoints": result,
+    }
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index ff88282049f..f27fa9caa4f 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -171,13 +171,13 @@ def request(name):
     return wrap
 
 
-def capability(name):
+def capability(name, value=True):
     """A decorator that indicates that the wrapper function implements
     the DAP capability NAME."""
 
     def wrap(func):
         global _capabilities
-        _capabilities[name] = True
+        _capabilities[name] = value
         return func
 
     return wrap
diff --git a/gdb/testsuite/gdb.dap/catch-exception.exp b/gdb/testsuite/gdb.dap/catch-exception.exp
new file mode 100644
index 00000000000..6bfeb3ed87e
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/catch-exception.exp
@@ -0,0 +1,65 @@
+# Copyright 2023 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib ada.exp
+load_lib dap-support.exp
+
+require allow_ada_tests allow_dap_tests gnat_runtime_has_debug_info
+
+standard_ada_testfile prog
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable \
+	 {debug additional_flags=-gnata}] != ""} {
+    return -1
+}
+
+if {[dap_launch $binfile] == ""} {
+    return
+}
+
+set obj [dap_check_request_and_response "set exception catchpoints" \
+	     setExceptionBreakpoints \
+	     {o filters [a [s assert]] \
+		  filterOptions [a [o filterId [s exception] \
+					condition [s "Global_Var = 23"]]]}]
+
+set bps [dict get [lindex $obj 0] body breakpoints]
+gdb_assert {[llength $bps] == 2} "two breakpoints"
+
+# The "path" should never be "null".
+set i 1
+foreach spec $bps {
+    # If "path" does not exist, then that is fine as well.
+    if {![dict exists $spec source path]} {
+	pass "breakpoint $i path"
+    } else {
+	gdb_assert {[dict get $spec source path] != "null"} \
+	    "breakpoint $i path"
+    }
+    incr i
+}
+
+dap_check_request_and_response "start inferior" configurationDone
+
+dap_wait_for_event_and_check "stopped at first raise" stopped \
+    "body reason" breakpoint \
+    "body hitBreakpointIds" 2
+
+dap_check_request_and_response "continue to assert" continue
+dap_wait_for_event_and_check "stopped at assert" stopped \
+    "body reason" breakpoint \
+    "body hitBreakpointIds" 1
+
+dap_shutdown
diff --git a/gdb/testsuite/gdb.dap/catch-exception/pck.ads b/gdb/testsuite/gdb.dap/catch-exception/pck.ads
new file mode 100644
index 00000000000..e82a54fa60a
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/catch-exception/pck.ads
@@ -0,0 +1,18 @@
+--  Copyright 2023 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Pck is
+   Global_Var : Integer := 91;
+end Pck;
diff --git a/gdb/testsuite/gdb.dap/catch-exception/prog.adb b/gdb/testsuite/gdb.dap/catch-exception/prog.adb
new file mode 100644
index 00000000000..287eb2492f9
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/catch-exception/prog.adb
@@ -0,0 +1,44 @@
+--  Copyright 2023 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Prog is
+begin
+
+   begin
+      raise Program_Error;
+   exception
+      when others =>
+         null;
+   end;
+
+   begin
+      Global_Var := 23;
+      raise Program_Error;
+   exception
+      when others =>
+         null;
+   end;
+
+   begin
+      pragma Assert (False);
+      null;
+   exception
+      when others =>
+         null;
+   end;
+
+end Prog;

-- 
2.40.0


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

* [PATCH 10/25] Implement DAP attach request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (8 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 09/25] Implement DAP setExceptionBreakpoints request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:53   ` Eli Zaretskii
  2023-05-24 16:37 ` [PATCH 11/25] Implement DAP stepOut request Tom Tromey
                   ` (15 subsequent siblings)
  25 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

This implements the DAP "attach" request.

Note that the copyright dates on the new test source file are not
incorrect -- this was copied verbatim from another directory.
---
 gdb/doc/gdb.texinfo               |  8 ++++++++
 gdb/python/lib/gdb/dap/launch.py  | 13 ++++++++++++-
 gdb/testsuite/gdb.dap/attach.c    | 25 +++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/attach.exp  | 36 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/lib/dap-support.exp | 19 ++++++++++++++++---
 5 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d1059e0cb7f..1b967485bc6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39020,6 +39020,14 @@ If provided, this is a string that specifies the program to use.  This
 corresponds to the @code{file} command.  @xref{Files}.
 @end table
 
+@value{GDBN} defines a parameter that can be passed to the
+@code{attach} request:
+
+@table @code
+@item pid
+The process ID to which @value{GDBN} should attach.  @xref{Attach}.
+@end table
+
 @node JIT Interface
 @chapter JIT Compilation Interface
 @cindex just-in-time compilation
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index 21499a339e1..a46be1dcca2 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -16,7 +16,7 @@
 import gdb
 from .events import ExecutionInvoker
 from .server import request, capability
-from .startup import send_gdb, in_gdb_thread
+from .startup import send_gdb, send_gdb_with_response, in_gdb_thread
 
 
 _program = None
@@ -45,6 +45,17 @@ def launch(*, program=None, args=[], env=None, **extra):
         send_gdb(lambda: _set_args_env(args, env))
 
 
+@request("attach")
+def attach(*, pid, **args):
+    # Ensure configurationDone does not try to run.
+    global _program
+    _program = None
+    # Use send_gdb_with_response to ensure we get an error if the
+    # attach fails.
+    send_gdb_with_response("attach " + str(pid))
+    return None
+
+
 @capability("supportsConfigurationDoneRequest")
 @request("configurationDone")
 def config_done(**args):
diff --git a/gdb/testsuite/gdb.dap/attach.c b/gdb/testsuite/gdb.dap/attach.c
new file mode 100644
index 00000000000..24c5283d1e6
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/attach.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+int
+main ()
+{
+  sleep (600);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dap/attach.exp b/gdb/testsuite/gdb.dap/attach.exp
new file mode 100644
index 00000000000..5b308f94975
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/attach.exp
@@ -0,0 +1,36 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test "attach" in DAP.
+
+require can_spawn_for_attach allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+    return
+}
+
+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] != ""} {
+    dap_shutdown true
+}
+
+kill_wait_spawned_process $test_spawn_id
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index ead295bdbfe..8667164fa11 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -270,9 +270,22 @@ proc dap_launch {file {args {}} {env {}}} {
     return [dap_check_request_and_response "startup - launch" launch $params]
 }
 
-# Cleanly shut down gdb.  NAME is used as the test name.
-proc dap_shutdown {{name shutdown}} {
-    dap_check_request_and_response $name disconnect
+# Start gdb, send a DAP initialize request, and then an attach request
+# specifying PID as the inferior process ID.  Returns the empty string
+# on failure, or the response object from the attach request.
+proc dap_attach {pid} {
+    if {[_dap_initialize "startup - initialize"] == ""} {
+	return ""
+    }
+    return [dap_check_request_and_response "startup - attach" attach \
+		[format {o pid [i %s]} $pid]]
+}
+
+# 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]
 }
 
 # Search the event list EVENTS for an output event matching the regexp

-- 
2.40.0


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

* [PATCH 11/25] Implement DAP stepOut request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (9 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 10/25] Implement DAP attach request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 12/25] Add singleThread support to some DAP requests Tom Tromey
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

This implements the DAP "stepOut" request.
---
 gdb/python/lib/gdb/dap/next.py      |  6 ++++++
 gdb/testsuite/gdb.dap/basic-dap.c   | 10 +++++++++-
 gdb/testsuite/gdb.dap/basic-dap.exp |  5 +++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/gdb/python/lib/gdb/dap/next.py b/gdb/python/lib/gdb/dap/next.py
index 636dfce997d..232b1529fe5 100644
--- a/gdb/python/lib/gdb/dap/next.py
+++ b/gdb/python/lib/gdb/dap/next.py
@@ -44,6 +44,12 @@ def stepIn(*, threadId, granularity="statement", **args):
     send_gdb(ExecutionInvoker(cmd, StopKinds.STEP))
 
 
+@request("stepOut")
+def step_out(*, threadId):
+    _handle_thread_step(threadId)
+    send_gdb(ExecutionInvoker("finish", StopKinds.STEP))
+
+
 @request("continue")
 def continue_request(**args):
     send_gdb(ExecutionInvoker("continue", None))
diff --git a/gdb/testsuite/gdb.dap/basic-dap.c b/gdb/testsuite/gdb.dap/basic-dap.c
index a8327141fb0..2570b8b0702 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.c
+++ b/gdb/testsuite/gdb.dap/basic-dap.c
@@ -35,10 +35,18 @@ address_breakpoint_here ()
 {
 }
 
-int main ()
+int
+line_breakpoint_here ()
 {
   do_not_stop_here ();
   function_breakpoint_here ();
   address_breakpoint_here ();
   return 0;			/* BREAK */
 }
+
+
+int
+main ()
+{
+  return line_breakpoint_here ();
+}
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index f28239d8268..d4dbdac70ff 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -146,6 +146,11 @@ dap_wait_for_event_and_check "stopped at line breakpoint" stopped \
     "body reason" breakpoint \
     "body hitBreakpointIds" $line_bpno
 
+dap_check_request_and_response "return from function" stepOut \
+    {o threadId [i 1]}
+dap_wait_for_event_and_check "stopped after return" stopped \
+    "body reason" step
+
 set obj [dap_check_request_and_response "evaluate global in main" \
 	     evaluate {o expression [s global_variable]}]
 dap_match_values "global value in main" [lindex $obj 0] \

-- 
2.40.0


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

* [PATCH 12/25] Add singleThread support to some DAP requests
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (10 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 11/25] Implement DAP stepOut request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 13/25] Rename one DAP function Tom Tromey
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

A few DAP requests support a "singleThread" parameter, which is
somewhat similar to scheduler-locking.  This patch implements support
for this.
---
 gdb/python/lib/gdb/dap/next.py            | 45 ++++++++++++++++++++++---------
 gdb/testsuite/gdb.dap/basic-dap.exp       |  9 ++++---
 gdb/testsuite/gdb.dap/catch-exception.exp |  3 ++-
 3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/next.py b/gdb/python/lib/gdb/dap/next.py
index 232b1529fe5..290b9b855ba 100644
--- a/gdb/python/lib/gdb/dap/next.py
+++ b/gdb/python/lib/gdb/dap/next.py
@@ -13,21 +13,40 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import gdb
+
 from .events import StopKinds, ExecutionInvoker
 from .server import capability, request
-from .startup import send_gdb
+from .startup import in_gdb_thread, send_gdb, send_gdb_with_response
 from .state import set_thread
 
 
-# Helper function to set the current thread.
-def _handle_thread_step(threadId):
+# Helper function to set the current thread and the scheduler-locking
+# mode.  Returns True if scheduler-locking was successfully set to
+# 'on', False in all other cases, including error.
+@in_gdb_thread
+def _handle_thread_step(thread_id, single_thread):
     # Ensure we're going to step the correct thread.
-    send_gdb(lambda: set_thread(threadId))
+    set_thread(thread_id)
+    if single_thread:
+        result = True
+        arg = "on"
+    else:
+        result = False
+        arg = "off"
+    try:
+        # This can fail, depending on the target, so catch the error
+        # and report to our caller.  We can't use exec_and_log because
+        # that does not propagate exceptions.
+        gdb.execute("set scheduler-locking " + arg, from_tty=True, to_string=True)
+    except gdb.error:
+        result = False
+    return result
 
 
 @request("next")
-def next(*, threadId, granularity="statement", **args):
-    _handle_thread_step(threadId)
+def next(*, threadId, singleThread=False, granularity="statement", **args):
+    send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     cmd = "next"
     if granularity == "instruction":
         cmd += "i"
@@ -35,9 +54,10 @@ def next(*, threadId, granularity="statement", **args):
 
 
 @capability("supportsSteppingGranularity")
+@capability("supportsSingleThreadExecutionRequests")
 @request("stepIn")
-def stepIn(*, threadId, granularity="statement", **args):
-    _handle_thread_step(threadId)
+def stepIn(*, threadId, singleThread=False, granularity="statement", **args):
+    send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     cmd = "step"
     if granularity == "instruction":
         cmd += "i"
@@ -45,12 +65,13 @@ def stepIn(*, threadId, granularity="statement", **args):
 
 
 @request("stepOut")
-def step_out(*, threadId):
-    _handle_thread_step(threadId)
+def step_out(*, threadId, singleThread=False):
+    send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("finish", StopKinds.STEP))
 
 
 @request("continue")
-def continue_request(**args):
+def continue_request(*, threadId, singleThread=False, **args):
+    locked = send_gdb_with_response(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("continue", None))
-    return {"allThreadsContinued": True}
+    return {"allThreadsContinued": not locked}
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index d4dbdac70ff..ce739875404 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -136,15 +136,18 @@ set obj [dap_check_request_and_response "evaluate global second time" \
 dap_match_values "global value after step" [lindex $obj 0] \
     "body result" 24
 
-dap_check_request_and_response "continue to address" continue
+dap_check_request_and_response "continue to address" continue \
+    {o threadId [i 1]}
 dap_wait_for_event_and_check "stopped at address breakpoint" stopped \
     "body reason" breakpoint \
     "body hitBreakpointIds" $insn_bpno
 
-dap_check_request_and_response "continue to line" continue
+dap_check_request_and_response "continue to line" continue \
+    {o threadId [i 1]}
 dap_wait_for_event_and_check "stopped at line breakpoint" stopped \
     "body reason" breakpoint \
-    "body hitBreakpointIds" $line_bpno
+    "body hitBreakpointIds" $line_bpno \
+    "body allThreadsStopped" true
 
 dap_check_request_and_response "return from function" stepOut \
     {o threadId [i 1]}
diff --git a/gdb/testsuite/gdb.dap/catch-exception.exp b/gdb/testsuite/gdb.dap/catch-exception.exp
index 6bfeb3ed87e..7f2e750b32e 100644
--- a/gdb/testsuite/gdb.dap/catch-exception.exp
+++ b/gdb/testsuite/gdb.dap/catch-exception.exp
@@ -57,7 +57,8 @@ dap_wait_for_event_and_check "stopped at first raise" stopped \
     "body reason" breakpoint \
     "body hitBreakpointIds" 2
 
-dap_check_request_and_response "continue to assert" continue
+dap_check_request_and_response "continue to assert" continue \
+    {o threadId [i 1]}
 dap_wait_for_event_and_check "stopped at assert" stopped \
     "body reason" breakpoint \
     "body hitBreakpointIds" 1

-- 
2.40.0


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

* [PATCH 13/25] Rename one DAP function
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (11 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 12/25] Add singleThread support to some DAP requests Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 14/25] Add test for DAP pause request Tom Tromey
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

When I first started implementing DAP, I had some vague plan of having
the implementation functions use the same name as the request.  I
abandoned this idea, but one vestige remained.  This patch renames the
one remaining function to be gdb-ish.
---
 gdb/python/lib/gdb/dap/next.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/python/lib/gdb/dap/next.py b/gdb/python/lib/gdb/dap/next.py
index 290b9b855ba..75f2fa6f31a 100644
--- a/gdb/python/lib/gdb/dap/next.py
+++ b/gdb/python/lib/gdb/dap/next.py
@@ -56,7 +56,7 @@ def next(*, threadId, singleThread=False, granularity="statement", **args):
 @capability("supportsSteppingGranularity")
 @capability("supportsSingleThreadExecutionRequests")
 @request("stepIn")
-def stepIn(*, threadId, singleThread=False, granularity="statement", **args):
+def step_in(*, threadId, singleThread=False, granularity="statement", **args):
     send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     cmd = "step"
     if granularity == "instruction":

-- 
2.40.0


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

* [PATCH 14/25] Add test for DAP pause request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (12 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 13/25] Rename one DAP function Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 15/25] Fix a latent bug in DAP request decorator Tom Tromey
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

I neglected to write a test for the DAP "pause" request.  This patch
adds one.
---
 gdb/testsuite/gdb.dap/pause.exp | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/gdb/testsuite/gdb.dap/pause.exp b/gdb/testsuite/gdb.dap/pause.exp
new file mode 100644
index 00000000000..27955d31526
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/pause.exp
@@ -0,0 +1,41 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test "pause" in DAP.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile attach.c
+
+if {[build_executable ${testfile}.exp $testfile $srcfile] == -1} {
+    return
+}
+
+if {[dap_launch $testfile] == ""} {
+    return
+}
+
+dap_check_request_and_response "start inferior" configurationDone
+dap_wait_for_event_and_check "inferior started" thread "body reason" started
+
+dap_check_request_and_response pause pause \
+    {o threadId [i 1]}
+
+dap_wait_for_event_and_check "stopped by pause" stopped \
+    "body reason" pause
+
+dap_shutdown

-- 
2.40.0


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

* [PATCH 15/25] Fix a latent bug in DAP request decorator
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (13 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 14/25] Add test for DAP pause request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 16/25] Use tuples for default arguments in DAP Tom Tromey
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

The 'request' decorator is intended to also ensure that the request
function runs in the DAP thread.  However, the unwrapped function is
installed in the global request map, so the wrapped version is never
called.  This patch fixes the bug.
---
 gdb/python/lib/gdb/dap/server.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index f27fa9caa4f..8abe475b031 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -164,9 +164,10 @@ def request(name):
 
     def wrap(func):
         global _commands
-        _commands[name] = func
         # All requests must run in the DAP thread.
-        return in_dap_thread(func)
+        func = in_dap_thread(func)
+        _commands[name] = func
+        return func
 
     return wrap
 

-- 
2.40.0


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

* [PATCH 16/25] Use tuples for default arguments in DAP
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (14 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 15/25] Fix a latent bug in DAP request decorator Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 17/25] Add type-checking to DAP requests Tom Tromey
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

My co-worker Kévin taught me that using a mutable object as a default
argument in Python is somewhat dangerous, because the object is
created a single time (when the function is defined), and so if it is
mutated in the body of the function, the changes will stick around.

This patch changes the cases like this in DAP to use () rather than []
as the default.  This patch is merely preventative, as no bugs like
this are in the code.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 4 ++--
 gdb/python/lib/gdb/dap/launch.py     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 877069f79a5..8ffa1829031 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -96,7 +96,7 @@ def _set_breakpoints(kind, specs):
 
 
 @request("setBreakpoints")
-def set_breakpoint(*, source, breakpoints=[], **args):
+def set_breakpoint(*, source, breakpoints=(), **args):
     if "path" not in source:
         result = []
     else:
@@ -188,7 +188,7 @@ def _set_exception_catchpoints(filter_options):
     "label": "Ada exceptions",
     "supportsCondition": True,
 }))
-def set_exception_breakpoints(*, filters, filterOptions=[], **args):
+def set_exception_breakpoints(*, filters, filterOptions=(), **args):
     # Convert the 'filters' to the filter-options style.
     options = [{"filterId": filter} for filter in filters]
     options.extend(filterOptions)
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index a46be1dcca2..dc4bf3c6d15 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -36,7 +36,7 @@ def _set_args_env(args, env):
 # from implementations.  Any additions or changes here should be
 # documented in the gdb manual.
 @request("launch")
-def launch(*, program=None, args=[], env=None, **extra):
+def launch(*, program=None, args=(), env=None, **extra):
     if program is not None:
         global _program
         _program = program

-- 
2.40.0


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

* [PATCH 17/25] Add type-checking to DAP requests
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (15 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 16/25] Use tuples for default arguments in DAP Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 18/25] Add gdb.Value.assign method Tom Tromey
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

It occurred to me recently that gdb's DAP implementation should
probably check the types of objects coming from the client.  This
patch implements this idea by reusing Python's existing type
annotations, and supplying a decorator that verifies these at runtime.

Python doesn't make it very easy to do runtime type-checking, so the
core of the checker is written by hand.  I haven't tried to make a
fully generic runtime type checker.  Instead, this only checks the
subset that is needed by DAP.  For example, only keyword-only
functions are handled.

Furthermore, in a few spots, it wasn't convenient to spell out the
type that is accepted.  I've added a couple of comments to this effect
in breakpoint.py.

I've tried to make this code compatible with older versions of Python,
but I've only been able to try it with 3.9 and 3.10.
---
 gdb/data-directory/Makefile.in        |  1 +
 gdb/python/lib/gdb/dap/breakpoint.py  | 41 ++++++++++-----
 gdb/python/lib/gdb/dap/bt.py          |  2 +-
 gdb/python/lib/gdb/dap/disassemble.py |  7 ++-
 gdb/python/lib/gdb/dap/evaluate.py    | 13 ++++-
 gdb/python/lib/gdb/dap/launch.py      | 14 ++++-
 gdb/python/lib/gdb/dap/memory.py      |  4 +-
 gdb/python/lib/gdb/dap/next.py        | 12 +++--
 gdb/python/lib/gdb/dap/scopes.py      |  2 +-
 gdb/python/lib/gdb/dap/server.py      |  6 ++-
 gdb/python/lib/gdb/dap/typecheck.py   | 88 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/type_check.exp  | 29 +++++++++++
 gdb/testsuite/gdb.dap/type_check.py   | 96 +++++++++++++++++++++++++++++++++++
 13 files changed, 287 insertions(+), 28 deletions(-)

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index a95c2d7ab37..9e34d4cffd3 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -105,6 +105,7 @@ PYTHON_FILE_LIST = \
 	gdb/dap/startup.py \
 	gdb/dap/state.py \
 	gdb/dap/threads.py \
+	gdb/dap/typecheck.py \
 	gdb/dap/varref.py \
 	gdb/function/__init__.py \
 	gdb/function/as_string.py \
diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 8ffa1829031..ad019333fea 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -16,6 +16,9 @@
 import gdb
 import os
 
+# These are deprecated in 3.9, but required in older versions.
+from typing import Optional, Sequence
+
 from .server import request, capability
 from .startup import send_gdb_with_response, in_gdb_thread
 
@@ -95,8 +98,10 @@ def _set_breakpoints(kind, specs):
     return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
 
 
+# FIXME we do not specify a type for 'source'.
+# FIXME 'breakpoints' is really a list[SourceBreakpoint].
 @request("setBreakpoints")
-def set_breakpoint(*, source, breakpoints=(), **args):
+def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
     else:
@@ -119,7 +124,7 @@ def set_breakpoint(*, source, breakpoints=(), **args):
 
 @request("setFunctionBreakpoints")
 @capability("supportsFunctionBreakpoints")
-def set_fn_breakpoint(*, breakpoints, **args):
+def set_fn_breakpoint(*, breakpoints: Sequence, **args):
     specs = []
     for bp in breakpoints:
         specs.append(
@@ -135,7 +140,9 @@ def set_fn_breakpoint(*, breakpoints, **args):
 
 @request("setInstructionBreakpoints")
 @capability("supportsInstructionBreakpoints")
-def set_insn_breakpoints(*, breakpoints, offset=None, **args):
+def set_insn_breakpoints(
+    *, breakpoints: Sequence, offset: Optional[int] = None, **args
+):
     specs = []
     for bp in breakpoints:
         # There's no way to set an explicit address breakpoint
@@ -179,16 +186,24 @@ def _set_exception_catchpoints(filter_options):
 
 @request("setExceptionBreakpoints")
 @capability("supportsExceptionFilterOptions")
-@capability("exceptionBreakpointFilters", ({
-    "filter": "assert",
-    "label": "Ada assertions",
-    "supportsCondition": True,
-}, {
-    "filter": "exception",
-    "label": "Ada exceptions",
-    "supportsCondition": True,
-}))
-def set_exception_breakpoints(*, filters, filterOptions=(), **args):
+@capability(
+    "exceptionBreakpointFilters",
+    (
+        {
+            "filter": "assert",
+            "label": "Ada assertions",
+            "supportsCondition": True,
+        },
+        {
+            "filter": "exception",
+            "label": "Ada exceptions",
+            "supportsCondition": True,
+        },
+    ),
+)
+def set_exception_breakpoints(
+    *, filters: Sequence[str], filterOptions: Sequence = (), **args
+):
     # Convert the 'filters' to the filter-options style.
     options = [{"filterId": filter} for filter in filters]
     options.extend(filterOptions)
diff --git a/gdb/python/lib/gdb/dap/bt.py b/gdb/python/lib/gdb/dap/bt.py
index 21cedb73d26..a38573fbba8 100644
--- a/gdb/python/lib/gdb/dap/bt.py
+++ b/gdb/python/lib/gdb/dap/bt.py
@@ -88,5 +88,5 @@ def _backtrace(thread_id, levels, startFrame):
 
 @request("stackTrace")
 @capability("supportsDelayedStackTraceLoading")
-def stacktrace(*, levels=0, startFrame=0, threadId, **extra):
+def stacktrace(*, levels: int = 0, startFrame: int = 0, threadId: int, **extra):
     return send_gdb_with_response(lambda: _backtrace(threadId, levels, startFrame))
diff --git a/gdb/python/lib/gdb/dap/disassemble.py b/gdb/python/lib/gdb/dap/disassemble.py
index 900a3c6adbb..bc091eb2c89 100644
--- a/gdb/python/lib/gdb/dap/disassemble.py
+++ b/gdb/python/lib/gdb/dap/disassemble.py
@@ -43,7 +43,12 @@ def _disassemble(pc, skip_insns, count):
 @request("disassemble")
 @capability("supportsDisassembleRequest")
 def disassemble(
-    *, memoryReference, offset=0, instructionOffset=0, instructionCount, **extra
+    *,
+    memoryReference: str,
+    offset: int = 0,
+    instructionOffset: int = 0,
+    instructionCount: int,
+    **extra
 ):
     pc = int(memoryReference, 0) + offset
     return send_gdb_with_response(
diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index fffd255417b..4fc0f31486c 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -16,6 +16,9 @@
 import gdb
 import gdb.printing
 
+# This is deprecated in 3.9, but required in older versions.
+from typing import Optional
+
 from .frames import frame_for_id
 from .server import request
 from .startup import send_gdb_with_response, in_gdb_thread
@@ -55,7 +58,13 @@ def _repl(command, frame_id):
 
 # FIXME supportsVariableType handling
 @request("evaluate")
-def eval_request(*, expression, frameId=None, context="variables", **args):
+def eval_request(
+    *,
+    expression: str,
+    frameId: Optional[int] = None,
+    context: str = "variables",
+    **args,
+):
     if context in ("watch", "variables"):
         # These seem to be expression-like.
         return send_gdb_with_response(lambda: _evaluate(expression, frameId))
@@ -75,7 +84,7 @@ def _variables(ref, start, count):
 @request("variables")
 # Note that we ignore the 'filter' field.  That seems to be
 # specific to javascript.
-def variables(*, variablesReference, start=0, count=0, **args):
+def variables(*, variablesReference: int, start: int = 0, count: int = 0, **args):
     result = send_gdb_with_response(
         lambda: _variables(variablesReference, start, count)
     )
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index dc4bf3c6d15..e187c3144d7 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -14,6 +14,10 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import gdb
+
+# These are deprecated in 3.9, but required in older versions.
+from typing import Mapping, Optional, Sequence
+
 from .events import ExecutionInvoker
 from .server import request, capability
 from .startup import send_gdb, send_gdb_with_response, in_gdb_thread
@@ -36,7 +40,13 @@ def _set_args_env(args, env):
 # from implementations.  Any additions or changes here should be
 # documented in the gdb manual.
 @request("launch")
-def launch(*, program=None, args=(), env=None, **extra):
+def launch(
+    *,
+    program: Optional[str] = None,
+    args: Sequence[str] = (),
+    env: Optional[Mapping[str, str]] = None,
+    **extra,
+):
     if program is not None:
         global _program
         _program = program
@@ -46,7 +56,7 @@ def launch(*, program=None, args=(), env=None, **extra):
 
 
 @request("attach")
-def attach(*, pid, **args):
+def attach(*, pid: int, **args):
     # Ensure configurationDone does not try to run.
     global _program
     _program = None
diff --git a/gdb/python/lib/gdb/dap/memory.py b/gdb/python/lib/gdb/dap/memory.py
index 7eb8a27ce47..85948bda9f4 100644
--- a/gdb/python/lib/gdb/dap/memory.py
+++ b/gdb/python/lib/gdb/dap/memory.py
@@ -28,7 +28,7 @@ def _read_memory(addr, count):
 
 @request("readMemory")
 @capability("supportsReadMemoryRequest")
-def read_memory(*, memoryReference, offset=0, count, **extra):
+def read_memory(*, memoryReference: str, offset: int = 0, count: int, **extra):
     addr = int(memoryReference, 0) + offset
     buf = send_gdb_with_response(lambda: _read_memory(addr, count))
     return {
@@ -45,7 +45,7 @@ def _write_memory(addr, contents):
 
 @request("writeMemory")
 @capability("supportsWriteMemoryRequest")
-def write_memory(*, memoryReference, offset=0, data, **extra):
+def write_memory(*, memoryReference: str, offset: int = 0, data: str, **extra):
     addr = int(memoryReference, 0) + offset
     send_gdb_with_response(lambda: _write_memory(addr, data))
     return {}
diff --git a/gdb/python/lib/gdb/dap/next.py b/gdb/python/lib/gdb/dap/next.py
index 75f2fa6f31a..8b112770a0c 100644
--- a/gdb/python/lib/gdb/dap/next.py
+++ b/gdb/python/lib/gdb/dap/next.py
@@ -45,7 +45,9 @@ def _handle_thread_step(thread_id, single_thread):
 
 
 @request("next")
-def next(*, threadId, singleThread=False, granularity="statement", **args):
+def next(
+    *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
+):
     send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     cmd = "next"
     if granularity == "instruction":
@@ -56,7 +58,9 @@ def next(*, threadId, singleThread=False, granularity="statement", **args):
 @capability("supportsSteppingGranularity")
 @capability("supportsSingleThreadExecutionRequests")
 @request("stepIn")
-def step_in(*, threadId, singleThread=False, granularity="statement", **args):
+def step_in(
+    *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
+):
     send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     cmd = "step"
     if granularity == "instruction":
@@ -65,13 +69,13 @@ def step_in(*, threadId, singleThread=False, granularity="statement", **args):
 
 
 @request("stepOut")
-def step_out(*, threadId, singleThread=False):
+def step_out(*, threadId: int, singleThread: bool = False):
     send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("finish", StopKinds.STEP))
 
 
 @request("continue")
-def continue_request(*, threadId, singleThread=False, **args):
+def continue_request(*, threadId: int, singleThread: bool = False, **args):
     locked = send_gdb_with_response(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("continue", None))
     return {"allThreadsContinued": not locked}
diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py
index 8e9af5064c2..9b80dd9ce80 100644
--- a/gdb/python/lib/gdb/dap/scopes.py
+++ b/gdb/python/lib/gdb/dap/scopes.py
@@ -109,6 +109,6 @@ def _get_scope(id):
 
 
 @request("scopes")
-def scopes(*, frameId, **extra):
+def scopes(*, frameId: int, **extra):
     scopes = send_gdb_with_response(lambda: _get_scope(frameId))
     return {"scopes": scopes}
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index 8abe475b031..be66676f730 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -25,6 +25,7 @@ from .startup import (
     log_stack,
     send_gdb_with_response,
 )
+from .typecheck import type_check
 
 
 # Map capability names to values.
@@ -165,7 +166,8 @@ def request(name):
     def wrap(func):
         global _commands
         # All requests must run in the DAP thread.
-        func = in_dap_thread(func)
+        # Also type-check the calls.
+        func = in_dap_thread(type_check(func))
         _commands[name] = func
         return func
 
@@ -202,7 +204,7 @@ def terminate(**args):
 
 @request("disconnect")
 @capability("supportTerminateDebuggee")
-def disconnect(*, terminateDebuggee=False, **args):
+def disconnect(*, terminateDebuggee: bool = False, **args):
     if terminateDebuggee:
         terminate()
     _server.shutdown()
diff --git a/gdb/python/lib/gdb/dap/typecheck.py b/gdb/python/lib/gdb/dap/typecheck.py
new file mode 100644
index 00000000000..791dc75f7b3
--- /dev/null
+++ b/gdb/python/lib/gdb/dap/typecheck.py
@@ -0,0 +1,88 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# A simple runtime type checker.
+
+import collections.abc
+import functools
+import typing
+
+
+# 'isinstance' won't work in general for type variables, so we
+# implement the subset that is needed by DAP.
+def _check_instance(value, typevar):
+    base = typing.get_origin(typevar)
+    if base is None:
+        return isinstance(value, typevar)
+    arg_types = typing.get_args(typevar)
+    if base == collections.abc.Mapping or base == typing.Mapping:
+        if not isinstance(value, collections.abc.Mapping):
+            return False
+        assert len(arg_types) == 2
+        (keytype, valuetype) = arg_types
+        return all(
+            _check_instance(k, keytype) and _check_instance(v, valuetype)
+            for k, v in value.items()
+        )
+    elif base == collections.abc.Sequence or base == typing.Sequence:
+        # In some places we simply use 'Sequence' without arguments.
+        if not isinstance(value, base):
+            return False
+        if len(arg_types) == 0:
+            return True
+        assert len(arg_types) == 1
+        arg_type = arg_types[0]
+        return all(_check_instance(item, arg_type) for item in value)
+    elif base == typing.Union:
+        return any(_check_instance(value, arg_type) for arg_type in arg_types)
+    raise TypeError("unsupported type variable '" + str(typevar) + "'")
+
+
+def type_check(func):
+    """A decorator that checks FUNC's argument types at runtime."""
+
+    # The type checker relies on 'typing.get_origin', which was added
+    # in Python 3.8.  (It also relies on 'typing.get_args', but that
+    # was added at the same time.)
+    if not hasattr(typing, "get_origin"):
+        return func
+
+    hints = typing.get_type_hints(func)
+    # We don't check the return type, but we allow it in case someone
+    # wants to use it on a function definition.
+    if "return" in hints:
+        del hints["return"]
+
+    # Note that keyword-only is fine for our purposes, because this is
+    # only used for DAP requests, and those are always called this
+    # way.
+    @functools.wraps(func)
+    def check_arguments(**kwargs):
+        for key in hints:
+            # The argument might not be passed in; we could type-check
+            # any default value here, but it seems fine to just rely
+            # on the code being correct -- the main goal of this
+            # checking is to verify JSON coming from the client.
+            if key in kwargs and not _check_instance(kwargs[key], hints[key]):
+                raise TypeError(
+                    "value for '"
+                    + key
+                    + "' does not have expected type '"
+                    + str(hints[key])
+                    + "'"
+                )
+        return func(**kwargs)
+
+    return check_arguments
diff --git a/gdb/testsuite/gdb.dap/type_check.exp b/gdb/testsuite/gdb.dap/type_check.exp
new file mode 100644
index 00000000000..346b4ba75b1
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/type_check.exp
@@ -0,0 +1,29 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test the DAP type-checker.
+
+require allow_dap_tests allow_python_tests
+
+load_lib dap-support.exp
+load_lib gdb-python.exp
+
+clean_restart
+
+set remote_python_file \
+    [gdb_remote_download host ${srcdir}/${subdir}/${gdb_test_file_name}.py]
+gdb_test_no_output "source ${remote_python_file}" "load python file"
+
+gdb_test "python check_everything()" OK "type checker"
diff --git a/gdb/testsuite/gdb.dap/type_check.py b/gdb/testsuite/gdb.dap/type_check.py
new file mode 100644
index 00000000000..1fdb595f3c6
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/type_check.py
@@ -0,0 +1,96 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test the type checker.
+
+import typing
+from gdb.dap.typecheck import type_check
+
+
+# A wrapper to call a function that should cause a type-checking
+# failure.  Returns if the exception was seen.  Throws an exception if
+# a TypeError was not seen.
+def should_fail(func, **args):
+    try:
+        func(**args)
+    except TypeError:
+        return
+    raise RuntimeError("function failed to throw TypeError")
+
+
+# Also specify a return type to make sure return types do not confuse
+# the checker.
+@type_check
+def simple_types(*, b: bool, s: str, i: int = 23) -> int:
+    return i
+
+
+def check_simple():
+    simple_types(b=True, s="DEI", i=97)
+    # Check the absence of a defaulted argument.
+    simple_types(b=True, s="DEI")
+    simple_types(b=False, s="DEI", i=97)
+    should_fail(simple_types, b=97, s="DEI", i=97)
+    should_fail(simple_types, b=True, s=None, i=97)
+    should_fail(simple_types, b=True, s="DEI", i={})
+
+
+@type_check
+def sequence_type(*, s: typing.Sequence[str]):
+    pass
+
+
+def check_sequence():
+    sequence_type(s=("hi", "out", "there"))
+    sequence_type(s=["hi", "out", "there"])
+    sequence_type(s=())
+    sequence_type(s=[])
+    should_fail(sequence_type, s=23)
+    should_fail(sequence_type, s=["hi", 97])
+
+
+@type_check
+def map_type(*, m: typing.Mapping[str, int]):
+    pass
+
+
+def check_map():
+    map_type(m={})
+    map_type(m={"dei": 23})
+    should_fail(map_type, m=[1, 2, 3])
+    should_fail(map_type, m=None)
+    should_fail(map_type, m={"dei": "string"})
+
+
+@type_check
+def opt_type(*, u: typing.Optional[int]):
+    pass
+
+
+def check_opt():
+    opt_type(u=23)
+    opt_type(u=None)
+    should_fail(opt_type, u="string")
+
+
+def check_everything():
+    # Older versions of Python can't really implement this.
+    if hasattr(typing, "get_origin"):
+        # Just let any exception propagate and end up in the log.
+        check_simple()
+        check_sequence()
+        check_map()
+        check_opt()
+        print("OK")

-- 
2.40.0


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

* [PATCH 18/25] Add gdb.Value.assign method
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (16 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 17/25] Add type-checking to DAP requests Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:54   ` Eli Zaretskii
  2023-05-24 16:37 ` [PATCH 19/25] Implement DAP setExpression request Tom Tromey
                   ` (7 subsequent siblings)
  25 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

This adds an 'assign' method to gdb.Value.  This allows for assignment
without requiring the use of parse_and_eval.
---
 gdb/NEWS                              |  2 ++
 gdb/doc/python.texi                   |  6 ++++++
 gdb/python/py-value.c                 | 30 ++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-value.exp | 14 ++++++++++++++
 4 files changed, 52 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index d97e3c15a87..4c4df060b62 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -209,6 +209,8 @@ info main
      "unset_env".  These can be used to modify the inferior's
      environment before it is started.
 
+  ** gdb.Value now has the 'assign' method.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 69755e96143..64ddbe21fc5 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -947,6 +947,12 @@ If @var{type} is @code{None} then this version of @code{__init__}
 behaves as though @var{type} was not passed at all.
 @end defun
 
+@defun Value.assign (rhs)
+Assign @var{rhs} to this value, and return @code{None}.  If this value
+cannot be assigned to, or if the assignment is invalid for some reason
+(for example a type-checking failure), an exception will be thrown.
+@end defun
+
 @defun Value.cast (type)
 Return a new instance of @code{gdb.Value} that is the result of
 casting this instance to the type described by @var{type}, which must
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 6c62820c63b..2be223da4ea 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -856,6 +856,33 @@ valpy_reinterpret_cast (PyObject *self, PyObject *args)
   return valpy_do_cast (self, args, UNOP_REINTERPRET_CAST);
 }
 
+/* Implementation of the "assign" method.  */
+
+static PyObject *
+valpy_assign (PyObject *self_obj, PyObject *args)
+{
+  PyObject *val_obj;
+
+  if (! PyArg_ParseTuple (args, "O", &val_obj))
+    return nullptr;
+
+  struct value *val = convert_value_from_python (val_obj);
+  if (val == nullptr)
+    return nullptr;
+
+  try
+    {
+      value_object *self = (value_object *) self_obj;
+      value_assign (self->value, val);
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+
+  Py_RETURN_NONE;
+}
+
 static Py_ssize_t
 valpy_length (PyObject *self)
 {
@@ -2121,6 +2148,9 @@ Return Unicode string representation of the value." },
     "format_string (...) -> string\n\
 Return a string representation of the value using the specified\n\
 formatting options" },
+  { "assign", (PyCFunction) valpy_assign, METH_VARARGS,
+    "assign (VAL) -> None\n\
+Assign VAL to this value." },
   {NULL}  /* Sentinel */
 };
 
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 9fc25814721..cdfcd414cd4 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -633,6 +633,19 @@ proc test_history_count {} {
     }
 }
 
+# Test Value.assign.
+proc test_assign {} {
+    gdb_test_no_output "python i_value = gdb.parse_and_eval('i')" \
+	"evaluate i"
+    gdb_test_no_output "python i_value.assign(27)" \
+	"set i to 27"
+    gdb_test "print i" " = 27"
+    gdb_test_no_output "python i_value = gdb.Value(27)" \
+	"reset i_value"
+    gdb_test "python i_value.assign(89)" "not an lvalue.*" \
+	"cannot assign to integer"
+}
+
 # Build C version of executable.  C++ is built later.
 if { [build_inferior "${binfile}" "c"] < 0 } {
     return -1
@@ -663,6 +676,7 @@ test_value_in_inferior
 test_value_from_buffer
 test_value_sub_classes
 test_inferior_function_call
+test_assign
 test_value_after_death
 
 # Test either C or C++ values. 

-- 
2.40.0


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

* [PATCH 19/25] Implement DAP setExpression request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (17 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 18/25] Add gdb.Value.assign method Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 20/25] Handle DAP supportsVariableType capability Tom Tromey
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

This implements the DAP setExpression request.
---
 gdb/python/lib/gdb/dap/evaluate.py  | 24 +++++++++++++++++++++++-
 gdb/testsuite/gdb.dap/basic-dap.exp |  5 +++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index 4fc0f31486c..1db6962f8e0 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -20,7 +20,7 @@ import gdb.printing
 from typing import Optional
 
 from .frames import frame_for_id
-from .server import request
+from .server import capability, request
 from .startup import send_gdb_with_response, in_gdb_thread
 from .varref import find_variable, VariableReference
 
@@ -43,6 +43,20 @@ def _evaluate(expr, frame_id):
     return ref.to_object()
 
 
+# Helper function to perform an assignment.
+@in_gdb_thread
+def _set_expression(expression, value, frame_id):
+    global_context = True
+    if frame_id is not None:
+        frame = frame_for_id(frame_id)
+        frame.select()
+        global_context = False
+    lhs = gdb.parse_and_eval(expression, global_context=global_context)
+    rhs = gdb.parse_and_eval(value, global_context=global_context)
+    lhs.assign(rhs)
+    return EvaluateResult(lhs).to_object()
+
+
 # Helper function to evaluate a gdb command in a certain frame.
 @in_gdb_thread
 def _repl(command, frame_id):
@@ -89,3 +103,11 @@ def variables(*, variablesReference: int, start: int = 0, count: int = 0, **args
         lambda: _variables(variablesReference, start, count)
     )
     return {"variables": result}
+
+
+@capability("supportsSetExpression")
+@request("setExpression")
+def set_expression(
+    *, expression: str, value: str, frameId: Optional[int] = None, **args
+):
+    return send_gdb_with_response(lambda: _set_expression(expression, value, frameId))
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index ce739875404..9aaa94051e6 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -159,6 +159,11 @@ set obj [dap_check_request_and_response "evaluate global in main" \
 dap_match_values "global value in main" [lindex $obj 0] \
     "body result" 25
 
+set obj [dap_check_request_and_response "set global in main" \
+	     setExpression {o expression [s global_variable] value [s 23]}]
+dap_match_values "global value in main after set" [lindex $obj 0] \
+    "body result" 23
+
 set obj [dap_request_and_response \
 	     evaluate {o expression [s nosuchvariable]}]
 set response [lindex $obj 0]

-- 
2.40.0


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

* [PATCH 20/25] Handle DAP supportsVariableType capability
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (18 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 19/25] Implement DAP setExpression request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 21/25] Add "target" parameter to DAP attach request Tom Tromey
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

A DAP client can report the supportsVariableType in the initialize
request.  In this case, gdb can include the type of a variable or
expression in various results.
---
 gdb/python/lib/gdb/dap/evaluate.py  |  1 -
 gdb/python/lib/gdb/dap/server.py    | 11 +++++++++++
 gdb/python/lib/gdb/dap/varref.py    |  3 +++
 gdb/testsuite/gdb.dap/basic-dap.exp |  3 ++-
 gdb/testsuite/lib/dap-support.exp   |  4 +++-
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index 1db6962f8e0..2b400651b67 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -70,7 +70,6 @@ def _repl(command, frame_id):
     }
 
 
-# FIXME supportsVariableType handling
 @request("evaluate")
 def eval_request(
     *,
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index be66676f730..b1c75ab967f 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -186,6 +186,17 @@ def capability(name, value=True):
     return wrap
 
 
+def client_bool_capability(name):
+    """Return the value of a boolean client capability.
+
+    If the capability was not specified, or did not have boolean type,
+    False is returned."""
+    global _server
+    if name in _server.config and isinstance(_server.config[name], bool):
+        return _server.config[name]
+    return False
+
+
 @request("initialize")
 def initialize(**args):
     global _server, _capabilities
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 88c34c20468..23f18d647c3 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -15,6 +15,7 @@
 
 import gdb
 from .startup import in_gdb_thread
+from .server import client_bool_capability
 from abc import abstractmethod
 
 
@@ -165,6 +166,8 @@ class VariableReference(BaseReference):
             result["memoryReference"] = hex(int(self.value))
         elif self.value.address is not None:
             result["memoryReference"] = hex(int(self.value.address))
+        if client_bool_capability("supportsVariableType"):
+            result["type"] = str(self.value.type)
         return result
 
     @in_gdb_thread
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index 9aaa94051e6..ca0d1be9f12 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -162,7 +162,8 @@ dap_match_values "global value in main" [lindex $obj 0] \
 set obj [dap_check_request_and_response "set global in main" \
 	     setExpression {o expression [s global_variable] value [s 23]}]
 dap_match_values "global value in main after set" [lindex $obj 0] \
-    "body result" 23
+    "body result" 23 \
+    "body type" int
 
 set obj [dap_request_and_response \
 	     evaluate {o expression [s nosuchvariable]}]
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index 8667164fa11..5c547480d09 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -230,7 +230,9 @@ proc _dap_initialize {name} {
     if {[dap_gdb_start]} {
 	return ""
     }
-    return [dap_check_request_and_response $name initialize]
+    return [dap_check_request_and_response $name initialize \
+		{o clientID [s "gdb testsuite"] \
+		     supportsVariableType [l true]}]
 }
 
 # Start gdb, send a DAP initialize request, and then a launch request

-- 
2.40.0


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

* [PATCH 21/25] Add "target" parameter to DAP attach request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (19 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 20/25] Handle DAP supportsVariableType capability Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:55   ` Eli Zaretskii
  2023-05-24 16:37 ` [PATCH 22/25] Add "stop at main" extension to DAP launch request Tom Tromey
                   ` (4 subsequent siblings)
  25 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

This adds a new "target" to the DAP attach request.  This is passed to
"target remote".  I thought "attach" made the most sense for this,
because in some sense gdb is attaching to a running process.  It's
worth noting that all DAP "attach" parameters are defined by the
implementation.
---
 gdb/doc/gdb.texinfo                  |  8 ++++--
 gdb/python/lib/gdb/dap/launch.py     | 10 ++++++--
 gdb/testsuite/gdb.dap/remote-dap.exp | 49 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/lib/dap-support.exp    | 11 ++++++++
 4 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1b967485bc6..f66dd4df3cf 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39020,12 +39020,16 @@ If provided, this is a string that specifies the program to use.  This
 corresponds to the @code{file} command.  @xref{Files}.
 @end table
 
-@value{GDBN} defines a parameter that can be passed to the
-@code{attach} request:
+@value{GDBN} defines some parameters that can be passed to the
+@code{attach} request.  One of these must be specified.
 
 @table @code
 @item pid
 The process ID to which @value{GDBN} should attach.  @xref{Attach}.
+
+@item target
+The target to which @value{GDBN} should connect.  This is a string and
+is passed to the @code{target remote} command.  @xref{Connecting}.
 @end table
 
 @node JIT Interface
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index e187c3144d7..2fec3267cbb 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -56,13 +56,19 @@ def launch(
 
 
 @request("attach")
-def attach(*, pid: int, **args):
+def attach(*, pid: Optional[int] = None, target: Optional[str] = None, **args):
     # Ensure configurationDone does not try to run.
     global _program
     _program = None
+    if pid is not None:
+        cmd = "attach " + str(pid)
+    elif target is not None:
+        cmd = "target remote " + target
+    else:
+        raise Exception("attach requires either 'pid' or 'target'")
     # Use send_gdb_with_response to ensure we get an error if the
     # attach fails.
-    send_gdb_with_response("attach " + str(pid))
+    send_gdb_with_response(cmd)
     return None
 
 
diff --git a/gdb/testsuite/gdb.dap/remote-dap.exp b/gdb/testsuite/gdb.dap/remote-dap.exp
new file mode 100644
index 00000000000..33524082051
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/remote-dap.exp
@@ -0,0 +1,49 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test "attach" with a remote target in DAP.
+
+load_lib gdbserver-support.exp
+load_lib dap-support.exp
+
+require allow_dap_tests allow_gdbserver_tests
+# We want to have control over where we start gdbserver.
+require {!is_remote target}
+
+# This test is only for remote targets.
+if {[target_info exists gdb_protocol]
+    && [target_info gdb_protocol] != "remote"} {
+    unsupported "requires remote"
+    return
+}
+
+standard_testfile attach.c
+
+if {[build_executable ${testfile}.exp $testfile $srcfile] == -1} {
+    return
+}
+
+set target_exec [gdb_remote_download target [standard_output_file $testfile]]
+
+lassign [gdbserver_start "" $target_exec] protocol port
+# Really should have been caught up above.
+gdb_assert {$protocol == "remote"}
+
+# We just want to test that attaching works at all.
+if {[dap_target_remote $port] != ""} {
+    dap_shutdown true
+}
+
+close_gdbserver
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index 5c547480d09..e1fada8b703 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -283,6 +283,17 @@ proc dap_attach {pid} {
 		[format {o pid [i %s]} $pid]]
 }
 
+# Start gdb, send a DAP initialize request, and then an attach request
+# specifying TARGET as the remote target.  Returns the empty string on
+# failure, or the response object from the attach request.
+proc dap_target_remote {target} {
+    if {[_dap_initialize "startup - initialize"] == ""} {
+	return ""
+    }
+    return [dap_check_request_and_response "startup - target" attach \
+		[format {o target [s %s]} $target]]
+}
+
 # Cleanly shut down gdb.  TERMINATE is passed as the terminateDebuggee
 # parameter to the request.
 proc dap_shutdown {{terminate false}} {

-- 
2.40.0


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

* [PATCH 22/25] Add "stop at main" extension to DAP launch request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (20 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 21/25] Add "target" parameter to DAP attach request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:56   ` Eli Zaretskii
  2023-05-24 16:37 ` [PATCH 23/25] Implement DAP breakpointLocations request Tom Tromey
                   ` (3 subsequent siblings)
  25 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

Co-workers who work on a program that uses DAP asked for the ability
to have gdb stop at the main subprogram when launching.  This patch
implements this extension.
---
 gdb/doc/gdb.texinfo                    |  5 +++++
 gdb/python/lib/gdb/dap/launch.py       | 13 +++++++++++-
 gdb/testsuite/gdb.dap/stop-at-main.exp | 37 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/lib/dap-support.exp      |  9 +++++++--
 4 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f66dd4df3cf..eb95273d93d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39018,6 +39018,11 @@ inferior will be set to exactly as passed in.  @xref{Environment}.
 @item program
 If provided, this is a string that specifies the program to use.  This
 corresponds to the @code{file} command.  @xref{Files}.
+
+@item stopAtBeginningOfMainSubprogram
+If provided, this must be a boolean.  When @samp{True}, @value{GDBN}
+will set a temporary breakpoint at the program's main procedure, using
+the same approach as the @code{start} command.  @xref{Starting}.
 @end table
 
 @value{GDBN} defines some parameters that can be passed to the
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index 2fec3267cbb..aee8c2f9ae6 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 ExecutionInvoker
 from .server import request, capability
-from .startup import send_gdb, send_gdb_with_response, in_gdb_thread
+from .startup import send_gdb, send_gdb_with_response, in_gdb_thread, exec_and_log
 
 
 _program = None
@@ -36,6 +36,14 @@ def _set_args_env(args, env):
             inf.set_env(name, value)
 
 
+@in_gdb_thread
+def _break_at_main():
+    inf = gdb.selected_inferior()
+    main = inf.main_name
+    if main is not None:
+        exec_and_log("tbreak " + main)
+
+
 # Any parameters here are necessarily extensions -- DAP requires this
 # from implementations.  Any additions or changes here should be
 # documented in the gdb manual.
@@ -45,12 +53,15 @@ def launch(
     program: Optional[str] = None,
     args: Sequence[str] = (),
     env: Optional[Mapping[str, str]] = None,
+    stopAtBeginningOfMainSubprogram: bool = False,
     **extra,
 ):
     if program is not None:
         global _program
         _program = program
         send_gdb(f"file {_program}")
+    if stopAtBeginningOfMainSubprogram:
+        send_gdb(_break_at_main)
     if len(args) > 0 or env is not None:
         send_gdb(lambda: _set_args_env(args, env))
 
diff --git a/gdb/testsuite/gdb.dap/stop-at-main.exp b/gdb/testsuite/gdb.dap/stop-at-main.exp
new file mode 100644
index 00000000000..80a9b81e152
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/stop-at-main.exp
@@ -0,0 +1,37 @@
+# Copyright 2022-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test the stop-at-main extension.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile attach.c
+
+if {[build_executable ${testfile}.exp $testfile $srcfile] == -1} {
+    return
+}
+
+if {[dap_launch $testfile {} {} 1] == ""} {
+    return
+}
+
+dap_check_request_and_response "start inferior" configurationDone
+# We didn't explicitly set a breakpoint, so if we hit one, it worked.
+dap_wait_for_event_and_check "stopped at function breakpoint" stopped \
+    "body reason" breakpoint
+
+dap_shutdown
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index e1fada8b703..92484bfdb8d 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -241,8 +241,9 @@ proc _dap_initialize {name} {
 # request.  If specified, ARGS is a list of command-line arguments,
 # and ENV is a list of pairs of the form {VAR VALUE} that is used to
 # populate the inferior's environment.  After this is called, gdb will
-# be ready to accept breakpoint requests.
-proc dap_launch {file {args {}} {env {}}} {
+# be ready to accept breakpoint requests.  If STOP_AT_MAIN is nonzero,
+# pass "stopAtBeginningOfMainSubprogram" to the launch request.
+proc dap_launch {file {args {}} {env {}} {stop_at_main 0}} {
     if {[_dap_initialize "startup - initialize"] == ""} {
 	return ""
     }
@@ -269,6 +270,10 @@ proc dap_launch {file {args {}} {env {}}} {
 	append params " \[o $envlist\]"
     }
 
+    if {$stop_at_main} {
+	append params { stopAtBeginningOfMainSubprogram [l true]}
+    }
+
     return [dap_check_request_and_response "startup - launch" launch $params]
 }
 

-- 
2.40.0


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

* [PATCH 23/25] Implement DAP breakpointLocations request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (21 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 22/25] Add "stop at main" extension to DAP launch request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 24/25] Do not report totalFrames from DAP stackTrace request Tom Tromey
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

This implements the DAP breakpointLocations request.
---
 gdb/data-directory/Makefile.in      |  1 +
 gdb/python/lib/gdb/dap/__init__.py  |  1 +
 gdb/python/lib/gdb/dap/locations.py | 45 +++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/basic-dap.c   |  2 +-
 gdb/testsuite/gdb.dap/basic-dap.exp | 15 +++++++++++++
 5 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 9e34d4cffd3..a3775a4a666 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -96,6 +96,7 @@ PYTHON_FILE_LIST = \
 	gdb/dap/__init__.py \
 	gdb/dap/io.py \
 	gdb/dap/launch.py \
+	gdb/dap/locations.py \
 	gdb/dap/memory.py \
 	gdb/dap/next.py \
 	gdb/dap/pause.py \
diff --git a/gdb/python/lib/gdb/dap/__init__.py b/gdb/python/lib/gdb/dap/__init__.py
index f07228e46ce..f3dd3ff7ea8 100644
--- a/gdb/python/lib/gdb/dap/__init__.py
+++ b/gdb/python/lib/gdb/dap/__init__.py
@@ -25,6 +25,7 @@ from . import bt
 from . import disassemble
 from . import evaluate
 from . import launch
+from . import locations
 from . import memory
 from . import next
 from . import pause
diff --git a/gdb/python/lib/gdb/dap/locations.py b/gdb/python/lib/gdb/dap/locations.py
new file mode 100644
index 00000000000..6c591579920
--- /dev/null
+++ b/gdb/python/lib/gdb/dap/locations.py
@@ -0,0 +1,45 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb
+
+# This is deprecated in 3.9, but required in older versions.
+from typing import Optional
+
+from .server import request
+from .startup import in_gdb_thread, send_gdb_with_response
+
+
+@in_gdb_thread
+def _find_lines(filename, start_line, end_line):
+    lines = set()
+    for entry in gdb.execute_mi("-symbol-list-lines", filename)["lines"]:
+        line = entry["line"]
+        if line >= start_line and line <= end_line:
+            lines.add(line)
+    return {"breakpoints": [{"line": x} for x in sorted(lines)]}
+
+
+@request("breakpointLocations")
+def breakpoint_locations(*, source, line: int, endLine: Optional[int] = None, **extra):
+    if endLine is None:
+        endLine = line
+    if "path" in source:
+        filename = source["path"]
+    elif "name" in source:
+        filename = source["name"]
+    else:
+        raise Exception("")
+    return send_gdb_with_response(lambda: _find_lines(filename, line, endLine))
diff --git a/gdb/testsuite/gdb.dap/basic-dap.c b/gdb/testsuite/gdb.dap/basic-dap.c
index 2570b8b0702..5fb11fae41c 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.c
+++ b/gdb/testsuite/gdb.dap/basic-dap.c
@@ -38,7 +38,7 @@ address_breakpoint_here ()
 int
 line_breakpoint_here ()
 {
-  do_not_stop_here ();
+  do_not_stop_here ();		/* FIRST */
   function_breakpoint_here ();
   address_breakpoint_here ();
   return 0;			/* BREAK */
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index ca0d1be9f12..df9afcfcdfc 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -71,6 +71,21 @@ if {!$ok} {
     fail "check new breakpoint event"
 }
 
+# Check that there are breakpoint locations on each line between FIRST
+# and BREAK.
+set first_line [gdb_get_line_number "FIRST"]
+set last_line [expr {$line - 1}]
+set obj [dap_check_request_and_response "breakpoint locations" \
+	     breakpointLocations \
+	     [format {o source [o path [%s]] line [i %d] endLine [i %d]} \
+		  [list s $srcfile] $first_line $last_line]]
+# We know gdb returns the lines in sorted order.
+foreach entry [dict get [lindex $obj 0] body breakpoints] {
+    gdb_assert {[dict get $entry line] == $first_line} \
+	"line $first_line in result"
+    incr first_line
+}
+
 set obj [dap_check_request_and_response "reset breakpoint by line number" \
 	     setBreakpoints \
 	     [format {o source [o path [%s]] breakpoints [a [o line [i %d]]]} \

-- 
2.40.0


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

* [PATCH 24/25] Do not report totalFrames from DAP stackTrace request
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (22 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 23/25] Implement DAP breakpointLocations request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-05-24 16:37 ` [PATCH 25/25] Implement DAP conditional breakpoints Tom Tromey
  2023-06-12 18:11 ` [PATCH 00/25] Many updates to DAP implementation Tom Tromey
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

Currently, gdb will unwind the entire stack in response to the
stackTrace request.  I had erroneously thought that the totalFrames
attribute was required in the response.  However, the spec says:

    If omitted or if `totalFrames` is larger than the available
    frames, a client is expected to request frames until a request
    returns less frames than requested (which indicates the end of the
    stack).

This patch removes this from the response in order to improve
performance when the stack trace is very long.
---
 gdb/python/lib/gdb/dap/bt.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/bt.py b/gdb/python/lib/gdb/dap/bt.py
index a38573fbba8..4439b428926 100644
--- a/gdb/python/lib/gdb/dap/bt.py
+++ b/gdb/python/lib/gdb/dap/bt.py
@@ -50,13 +50,9 @@ def _backtrace(thread_id, levels, startFrame):
         current_frame = gdb.newest_frame()
     except gdb.error:
         current_frame = None
-    # Note that we always iterate over all frames, which is lame, but
-    # seemingly necessary to support the totalFrames response.
-    # FIXME maybe the mildly mysterious note about "monotonically
-    # increasing totalFrames values" would let us fix this.
-    while current_frame is not None:
+    while current_frame is not None and (levels == 0 or len(frames) < levels):
         # This condition handles the startFrame==0 case as well.
-        if current_number >= startFrame and (levels == 0 or len(frames) < levels):
+        if current_number >= startFrame:
             newframe = {
                 "id": frame_id(current_frame),
                 "name": _frame_name(current_frame),
@@ -80,9 +76,11 @@ def _backtrace(thread_id, levels, startFrame):
             frames.append(newframe)
         current_number = current_number + 1
         current_frame = current_frame.older()
+    # Note that we do not calculate totalFrames here.  Its absence
+    # tells the client that it may simply ask for frames until a
+    # response yields fewer frames than requested.
     return {
         "stackFrames": frames,
-        "totalFrames": current_number,
     }
 
 

-- 
2.40.0


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

* [PATCH 25/25] Implement DAP conditional breakpoints
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (23 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 24/25] Do not report totalFrames from DAP stackTrace request Tom Tromey
@ 2023-05-24 16:37 ` Tom Tromey
  2023-06-12 18:11 ` [PATCH 00/25] Many updates to DAP implementation Tom Tromey
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-05-24 16:37 UTC (permalink / raw)
  To: gdb-patches

I realized that I had only implemented DAP breakpoint conditions for
exception breakpoints, and not other kinds of breakpoints.  This patch
corrects the oversight.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 69 +++++++++++++++++++++++++++---------
 gdb/testsuite/gdb.dap/cond-bp.c      | 26 ++++++++++++++
 gdb/testsuite/gdb.dap/cond-bp.exp    | 65 +++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+), 16 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index ad019333fea..20e65aa0e61 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -63,6 +63,14 @@ def breakpoint_descriptor(bp):
         }
 
 
+# Extract entries from a hash table and return a list of them.  Each
+# entry is a string.  If a key of that name appears in the hash table,
+# it is removed and pushed on the result list; if it does not appear,
+# None is pushed on the list.
+def _remove_entries(table, *names):
+    return [table.pop(name, None) for name in names]
+
+
 # Helper function to set some breakpoints according to a list of
 # specifications and a callback function to do the work of creating
 # the breakpoint.
@@ -78,11 +86,20 @@ def _set_breakpoints_callback(kind, specs, creator):
     result = []
     for spec in specs:
         keyspec = frozenset(spec.items())
+
+        (condition, hit_condition) = _remove_entries(spec, "condition", "hitCondition")
+
         if keyspec in saved_map:
             bp = saved_map.pop(keyspec)
         else:
             # FIXME handle exceptions here
             bp = creator(**spec)
+
+        if condition is not None:
+            bp.condition = condition
+        if hit_condition is not None:
+            bp.ignore_count = hit_condition
+
         breakpoint_map[kind][keyspec] = bp
         result.append(breakpoint_descriptor(bp))
     # Delete any breakpoints that were not reused.
@@ -98,9 +115,22 @@ def _set_breakpoints(kind, specs):
     return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
 
 
+# Turn a DAP SourceBreakpoint, FunctionBreakpoint, or
+# InstructionBreakpoint into a "spec" that is used by
+# _set_breakpoints.  SPEC is a dictionary of parameters that is used
+# as the base of the result; it is modified in place.
+def _basic_spec(bp_info, spec):
+    for name in ("condition", "hitCondition"):
+        if name in bp_info:
+            spec[name] = bp_info[name]
+    return spec
+
+
 # FIXME we do not specify a type for 'source'.
 # FIXME 'breakpoints' is really a list[SourceBreakpoint].
 @request("setBreakpoints")
+@capability("supportsHitConditionalBreakpoints")
+@capability("supportsConditionalBreakpoints")
 def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
@@ -108,10 +138,13 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
         specs = []
         for obj in breakpoints:
             specs.append(
-                {
-                    "source": source["path"],
-                    "line": obj["line"],
-                }
+                _basic_spec(
+                    obj,
+                    {
+                        "source": source["path"],
+                        "line": obj["line"],
+                    },
+                )
             )
         # Be sure to include the path in the key, so that we only
         # clear out breakpoints coming from this same source.
@@ -128,9 +161,12 @@ def set_fn_breakpoint(*, breakpoints: Sequence, **args):
     specs = []
     for bp in breakpoints:
         specs.append(
-            {
-                "function": bp["name"],
-            }
+            _basic_spec(
+                bp,
+                {
+                    "function": bp["name"],
+                },
+            )
         )
     result = send_gdb_with_response(lambda: _set_breakpoints("function", specs))
     return {
@@ -151,9 +187,12 @@ def set_insn_breakpoints(
         if offset is not None:
             val = val + " + " + str(offset)
         specs.append(
-            {
-                "spec": val,
-            }
+            _basic_spec(
+                bp,
+                {
+                    "spec": val,
+                },
+            )
         )
     result = send_gdb_with_response(lambda: _set_breakpoints("instruction", specs))
     return {
@@ -162,16 +201,14 @@ def set_insn_breakpoints(
 
 
 @in_gdb_thread
-def _catch_exception(filterId, condition=None, **args):
+def _catch_exception(filterId, **args):
     if filterId == "assert":
-        args = ["-catch-assert"]
+        cmd = "-catch-assert"
     elif filterId == "exception":
-        args = ["-catch-exception"]
+        cmd = "-catch-exception"
     else:
         raise Exception(f"Invalid exception filterID: {filterId}")
-    if condition is not None:
-        args.extend(["-c", condition])
-    result = gdb.execute_mi(*args)
+    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"]:
diff --git a/gdb/testsuite/gdb.dap/cond-bp.c b/gdb/testsuite/gdb.dap/cond-bp.c
new file mode 100644
index 00000000000..535b15d1420
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/cond-bp.c
@@ -0,0 +1,26 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  int acc = 0, i, j;
+  for (i = 0; i < 5; ++i)
+    for (j = 0; j < 5; ++j)
+      acc += i + j;		/* STOP */
+  return acc;
+}
diff --git a/gdb/testsuite/gdb.dap/cond-bp.exp b/gdb/testsuite/gdb.dap/cond-bp.exp
new file mode 100644
index 00000000000..376db4b3548
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/cond-bp.exp
@@ -0,0 +1,65 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test DAP breakpoint conditions.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+    return
+}
+
+if {[dap_launch $testfile] == ""} {
+    return
+}
+
+set line [gdb_get_line_number "STOP"]
+set obj [dap_check_request_and_response "set conditional breakpoint" \
+	     setBreakpoints \
+	     [format {o source [o path [%s]] \
+			  breakpoints [a [o line [i %d] \
+					      condition [s "i == 3"] \
+					      hitCondition [i 3]]]} \
+		  [list s $srcfile] $line]]
+set fn_bpno [dap_get_breakpoint_number $obj]
+
+dap_check_request_and_response "start inferior" configurationDone
+
+dap_wait_for_event_and_check "stopped at function breakpoint" stopped \
+    "body reason" breakpoint \
+    "body hitBreakpointIds" $fn_bpno
+
+set bt [lindex [dap_check_request_and_response "backtrace" stackTrace \
+		    {o threadId [i 1]}] \
+	    0]
+set frame_id [dict get [lindex [dict get $bt body stackFrames] 0] id]
+
+set obj [dap_check_request_and_response "evaluate i" \
+	     evaluate \
+	     [format {o expression [s i] frameId [i %s]} $frame_id]]
+dap_match_values "value of i" [lindex $obj 0] \
+    "body result" 3
+
+set obj [dap_check_request_and_response "evaluate j" \
+	     evaluate \
+	     [format {o expression [s j] frameId [i %s]} $frame_id]]
+dap_match_values "value of j" [lindex $obj 0] \
+    "body result" 3
+
+dap_shutdown

-- 
2.40.0


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

* Re: [PATCH 10/25] Implement DAP attach request
  2023-05-24 16:37 ` [PATCH 10/25] Implement DAP attach request Tom Tromey
@ 2023-05-24 16:53   ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-05-24 16:53 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Date: Wed, 24 May 2023 10:37:01 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> This implements the DAP "attach" request.
> 
> Note that the copyright dates on the new test source file are not
> incorrect -- this was copied verbatim from another directory.
> ---
>  gdb/doc/gdb.texinfo               |  8 ++++++++
>  gdb/python/lib/gdb/dap/launch.py  | 13 ++++++++++++-
>  gdb/testsuite/gdb.dap/attach.c    | 25 +++++++++++++++++++++++++
>  gdb/testsuite/gdb.dap/attach.exp  | 36 ++++++++++++++++++++++++++++++++++++
>  gdb/testsuite/lib/dap-support.exp | 19 ++++++++++++++++---
>  5 files changed, 97 insertions(+), 4 deletions(-)

OK for the documentation part.  Thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

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

* Re: [PATCH 18/25] Add gdb.Value.assign method
  2023-05-24 16:37 ` [PATCH 18/25] Add gdb.Value.assign method Tom Tromey
@ 2023-05-24 16:54   ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-05-24 16:54 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Date: Wed, 24 May 2023 10:37:09 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> This adds an 'assign' method to gdb.Value.  This allows for assignment
> without requiring the use of parse_and_eval.
> ---
>  gdb/NEWS                              |  2 ++
>  gdb/doc/python.texi                   |  6 ++++++
>  gdb/python/py-value.c                 | 30 ++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.python/py-value.exp | 14 ++++++++++++++
>  4 files changed, 52 insertions(+)

Thanks, the documentation parts are OK.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

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

* Re: [PATCH 21/25] Add "target" parameter to DAP attach request
  2023-05-24 16:37 ` [PATCH 21/25] Add "target" parameter to DAP attach request Tom Tromey
@ 2023-05-24 16:55   ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-05-24 16:55 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Date: Wed, 24 May 2023 10:37:12 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> This adds a new "target" to the DAP attach request.  This is passed to
> "target remote".  I thought "attach" made the most sense for this,
> because in some sense gdb is attaching to a running process.  It's
> worth noting that all DAP "attach" parameters are defined by the
> implementation.
> ---
>  gdb/doc/gdb.texinfo                  |  8 ++++--
>  gdb/python/lib/gdb/dap/launch.py     | 10 ++++++--
>  gdb/testsuite/gdb.dap/remote-dap.exp | 49 ++++++++++++++++++++++++++++++++++++
>  gdb/testsuite/lib/dap-support.exp    | 11 ++++++++
>  4 files changed, 74 insertions(+), 4 deletions(-)

Thanks, the documentation part is okay.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

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

* Re: [PATCH 22/25] Add "stop at main" extension to DAP launch request
  2023-05-24 16:37 ` [PATCH 22/25] Add "stop at main" extension to DAP launch request Tom Tromey
@ 2023-05-24 16:56   ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-05-24 16:56 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Date: Wed, 24 May 2023 10:37:13 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> Co-workers who work on a program that uses DAP asked for the ability
> to have gdb stop at the main subprogram when launching.  This patch
> implements this extension.
> ---
>  gdb/doc/gdb.texinfo                    |  5 +++++
>  gdb/python/lib/gdb/dap/launch.py       | 13 +++++++++++-
>  gdb/testsuite/gdb.dap/stop-at-main.exp | 37 ++++++++++++++++++++++++++++++++++
>  gdb/testsuite/lib/dap-support.exp      |  9 +++++++--
>  4 files changed, 61 insertions(+), 3 deletions(-)

OK for the documentation part, thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

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

* Re: [PATCH 00/25] Many updates to DAP implementation
  2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
                   ` (24 preceding siblings ...)
  2023-05-24 16:37 ` [PATCH 25/25] Implement DAP conditional breakpoints Tom Tromey
@ 2023-06-12 18:11 ` Tom Tromey
  25 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-06-12 18:11 UTC (permalink / raw)
  To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> I've been working steadily on the DAP implementation.  This series
Tom> combines a number of patches.

Tom> It starts with some general refactoring of Ada catchpoints, to make
Tom> them work more like C++ catchpoints.  In particular, now they can be
Tom> set before the inferior has started.  This is then used to implement
Tom> the DAP setExceptionBreakpoints request.

Tom> Other patches add DAP features.  In some cases a new Python API is
Tom> added so that it can be used by DAP.

Tom> I do have some more patches to come after this series goes in.

Tom> Regression tested on x86-64 Fedora 36.

I rebased this and re-tested it.  I'm checking it in now.

I have some more DAP patches to send after this, but not very many now.

Tom

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

* Re: [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests
  2023-05-24 16:36 ` [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests Tom Tromey
@ 2023-06-13 20:10   ` Tom de Vries
  2023-06-19  9:49     ` Tom de Vries
  0 siblings, 1 reply; 34+ messages in thread
From: Tom de Vries @ 2023-06-13 20:10 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 5/24/23 18:36, Tom Tromey via Gdb-patches wrote:
> This changes the Ada catchpoint tests to use
> gnat_runtime_has_debug_info.  This simplifies the code.

I see this regression:
...
FAIL: gdb.ada/catch_ex_std.exp: catch exception some_kind_of_error
FAIL: gdb.ada/catch_ex_std.exp: caught the exception (the program exited)
FAIL: gdb.ada/catch_ex_std.exp: print $_ada_exception = 
some_package.some_kind_of_error'Address
...

I'm assuming it's due to this commit.

At first glance, this PR ( 
https://sourceware.org/bugzilla/show_bug.cgi?id=30094 ) explains why.

Thanks,
- Tom




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

* Re: [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests
  2023-06-13 20:10   ` Tom de Vries
@ 2023-06-19  9:49     ` Tom de Vries
  2023-06-20 13:56       ` Tom Tromey
  0 siblings, 1 reply; 34+ messages in thread
From: Tom de Vries @ 2023-06-19  9:49 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 6/13/23 22:10, Tom de Vries wrote:
> On 5/24/23 18:36, Tom Tromey via Gdb-patches wrote:
>> This changes the Ada catchpoint tests to use
>> gnat_runtime_has_debug_info.  This simplifies the code.
> 
> I see this regression:
> ...
> FAIL: gdb.ada/catch_ex_std.exp: catch exception some_kind_of_error
> FAIL: gdb.ada/catch_ex_std.exp: caught the exception (the program exited)
> FAIL: gdb.ada/catch_ex_std.exp: print $_ada_exception = 
> some_package.some_kind_of_error'Address
> ...
> 
> I'm assuming it's due to this commit.
> 
> At first glance, this PR ( 
> https://sourceware.org/bugzilla/show_bug.cgi?id=30094 ) explains why.

Fixed by this commit ( 
https://sourceware.org/pipermail/gdb-patches/2023-June/200318.html ).

Thanks,
- Tom



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

* Re: [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests
  2023-06-19  9:49     ` Tom de Vries
@ 2023-06-20 13:56       ` Tom Tromey
  0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2023-06-20 13:56 UTC (permalink / raw)
  To: Tom de Vries via Gdb-patches; +Cc: Tom Tromey, Tom de Vries

>> FAIL: gdb.ada/catch_ex_std.exp: catch exception some_kind_of_error
>> FAIL: gdb.ada/catch_ex_std.exp: caught the exception (the program exited)
>> FAIL: gdb.ada/catch_ex_std.exp: print $_ada_exception =
>> some_package.some_kind_of_error'Address
>> ...
>> I'm assuming it's due to this commit.
>> At first glance, this PR (
>> https://sourceware.org/bugzilla/show_bug.cgi?id=30094 ) explains
>> why.

Tom> Fixed by this commit (
Tom> https://sourceware.org/pipermail/gdb-patches/2023-June/200318.html ).

Thanks for doing this.

Tom

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

end of thread, other threads:[~2023-06-20 13:58 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-24 16:36 [PATCH 00/25] Many updates to DAP implementation Tom Tromey
2023-05-24 16:36 ` [PATCH 01/25] Stop gdb in gnat_runtime_has_debug_info Tom Tromey
2023-05-24 16:36 ` [PATCH 02/25] Use gnat_runtime_has_debug_info in Ada catchpoint tests Tom Tromey
2023-06-13 20:10   ` Tom de Vries
2023-06-19  9:49     ` Tom de Vries
2023-06-20 13:56       ` Tom Tromey
2023-05-24 16:36 ` [PATCH 03/25] Pass tempflag to ada_catchpoint constructor Tom Tromey
2023-05-24 16:36 ` [PATCH 04/25] Transfer ownership of exception string to ada_catchpoint Tom Tromey
2023-05-24 16:36 ` [PATCH 05/25] Combine create_excep_cond_exprs and ada_catchpoint::re_set Tom Tromey
2023-05-24 16:36 ` [PATCH 06/25] Turn should_stop_exception into a method of ada_catchpoint Tom Tromey
2023-05-24 16:36 ` [PATCH 07/25] Mark members of ada_catchpoint "private" Tom Tromey
2023-05-24 16:36 ` [PATCH 08/25] Don't require inferior execution for Ada catchpoints Tom Tromey
2023-05-24 16:37 ` [PATCH 09/25] Implement DAP setExceptionBreakpoints request Tom Tromey
2023-05-24 16:37 ` [PATCH 10/25] Implement DAP attach request Tom Tromey
2023-05-24 16:53   ` Eli Zaretskii
2023-05-24 16:37 ` [PATCH 11/25] Implement DAP stepOut request Tom Tromey
2023-05-24 16:37 ` [PATCH 12/25] Add singleThread support to some DAP requests Tom Tromey
2023-05-24 16:37 ` [PATCH 13/25] Rename one DAP function Tom Tromey
2023-05-24 16:37 ` [PATCH 14/25] Add test for DAP pause request Tom Tromey
2023-05-24 16:37 ` [PATCH 15/25] Fix a latent bug in DAP request decorator Tom Tromey
2023-05-24 16:37 ` [PATCH 16/25] Use tuples for default arguments in DAP Tom Tromey
2023-05-24 16:37 ` [PATCH 17/25] Add type-checking to DAP requests Tom Tromey
2023-05-24 16:37 ` [PATCH 18/25] Add gdb.Value.assign method Tom Tromey
2023-05-24 16:54   ` Eli Zaretskii
2023-05-24 16:37 ` [PATCH 19/25] Implement DAP setExpression request Tom Tromey
2023-05-24 16:37 ` [PATCH 20/25] Handle DAP supportsVariableType capability Tom Tromey
2023-05-24 16:37 ` [PATCH 21/25] Add "target" parameter to DAP attach request Tom Tromey
2023-05-24 16:55   ` Eli Zaretskii
2023-05-24 16:37 ` [PATCH 22/25] Add "stop at main" extension to DAP launch request Tom Tromey
2023-05-24 16:56   ` Eli Zaretskii
2023-05-24 16:37 ` [PATCH 23/25] Implement DAP breakpointLocations request Tom Tromey
2023-05-24 16:37 ` [PATCH 24/25] Do not report totalFrames from DAP stackTrace request Tom Tromey
2023-05-24 16:37 ` [PATCH 25/25] Implement DAP conditional breakpoints Tom Tromey
2023-06-12 18:11 ` [PATCH 00/25] Many updates to DAP implementation 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).