Change handling of over-long DAP variables requests
Commit Message
In PR dap/33228, we changed gdb to gracefully report an error if a DAP
'variables' request asked for more variables than had been reported.
This behavior was clarified in the spec, see
https://github.com/microsoft/debug-adapter-protocol/issues/571
This patch changes gdb to conform to the specified approach, namely
truncating the list rather than erroring.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33228
---
gdb/python/lib/gdb/dap/varref.py | 8 +++++---
gdb/testsuite/gdb.dap/scopes.exp | 15 +++++++++------
2 files changed, 14 insertions(+), 9 deletions(-)
base-commit: b056a39914ed08da28f18f9b6eecf44447e069de
Comments
Tom Tromey <tromey@adacore.com> writes:
> In PR dap/33228, we changed gdb to gracefully report an error if a DAP
> 'variables' request asked for more variables than had been reported.
>
> This behavior was clarified in the spec, see
>
> https://github.com/microsoft/debug-adapter-protocol/issues/571
>
> This patch changes gdb to conform to the specified approach, namely
> truncating the list rather than erroring.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33228
LGTM.
Approved-By: Andrew Burgess <aburgess@redhat.com>
I wonder if this should also be merged into gdb-17-branch so that it
lands in 17.2?
Thanks,
Andrew
> ---
> gdb/python/lib/gdb/dap/varref.py | 8 +++++---
> gdb/testsuite/gdb.dap/scopes.exp | 15 +++++++++------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
> index f5ffe34f7f5..e7d114c9d51 100644
> --- a/gdb/python/lib/gdb/dap/varref.py
> +++ b/gdb/python/lib/gdb/dap/varref.py
> @@ -146,10 +146,12 @@ class BaseReference(ABC):
> if self._children is None:
> self._children = [None] * self.child_count()
> for idx in range(start, start + count):
> + # DAP was clarified to say that if too many children are
> + # requested, this is not an error, but instead the result
> + # is just truncated.
> + # https://github.com/microsoft/debug-adapter-protocol/issues/571
> if idx >= len(self._children):
> - raise DAPException(
> - f"requested child {idx} outside range of variable {self._ref}"
> - )
> + break
> if self._children[idx] is None:
> (name, value) = self.fetch_one_child(idx)
> name = self._compute_name(name)
> diff --git a/gdb/testsuite/gdb.dap/scopes.exp b/gdb/testsuite/gdb.dap/scopes.exp
> index 52efa683c4c..b3026263a90 100644
> --- a/gdb/testsuite/gdb.dap/scopes.exp
> +++ b/gdb/testsuite/gdb.dap/scopes.exp
> @@ -133,12 +133,15 @@ set refs [lindex [dap_check_request_and_response "fetch contents of dei" \
> set deivals [dict get $refs body variables]
> gdb_assert {[llength $deivals] == 2} "dei has two members"
>
> -# Request more children than exist. See PR dap/33228.
> -set seq [dap_send_request variables \
> - [format {o variablesReference [i %d] count [i 100]} $dei_ref]]
> -lassign [dap_read_response variables $seq] response ignore
> -gdb_assert {[dict get $response success] == "false"} \
> - "variables with invalid count"
> +# Request more children than exist. See PR dap/33228 and
> +# https://github.com/microsoft/debug-adapter-protocol/issues/571.
> +set refs [lindex [dap_check_request_and_response "fetch too many variables" \
> + "variables" \
> + [format {o variablesReference [i %d] count [i 100]} \
> + $dei_ref]] \
> + 0]
> +set deivals [dict get $refs body variables]
> +gdb_assert {[llength $deivals] == 2} "still just two members"
>
> set num [dict get $reg_scope variablesReference]
> lassign [dap_check_request_and_response "fetch all registers" \
>
> base-commit: b056a39914ed08da28f18f9b6eecf44447e069de
> --
> 2.52.0
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> Approved-By: Andrew Burgess <aburgess@redhat.com>
Andrew> I wonder if this should also be merged into gdb-17-branch so that it
Andrew> lands in 17.2?
Makes sense, I'll do that.
Tom
@@ -146,10 +146,12 @@ class BaseReference(ABC):
if self._children is None:
self._children = [None] * self.child_count()
for idx in range(start, start + count):
+ # DAP was clarified to say that if too many children are
+ # requested, this is not an error, but instead the result
+ # is just truncated.
+ # https://github.com/microsoft/debug-adapter-protocol/issues/571
if idx >= len(self._children):
- raise DAPException(
- f"requested child {idx} outside range of variable {self._ref}"
- )
+ break
if self._children[idx] is None:
(name, value) = self.fetch_one_child(idx)
name = self._compute_name(name)
@@ -133,12 +133,15 @@ set refs [lindex [dap_check_request_and_response "fetch contents of dei" \
set deivals [dict get $refs body variables]
gdb_assert {[llength $deivals] == 2} "dei has two members"
-# Request more children than exist. See PR dap/33228.
-set seq [dap_send_request variables \
- [format {o variablesReference [i %d] count [i 100]} $dei_ref]]
-lassign [dap_read_response variables $seq] response ignore
-gdb_assert {[dict get $response success] == "false"} \
- "variables with invalid count"
+# Request more children than exist. See PR dap/33228 and
+# https://github.com/microsoft/debug-adapter-protocol/issues/571.
+set refs [lindex [dap_check_request_and_response "fetch too many variables" \
+ "variables" \
+ [format {o variablesReference [i %d] count [i 100]} \
+ $dei_ref]] \
+ 0]
+set deivals [dict get $refs body variables]
+gdb_assert {[llength $deivals] == 2} "still just two members"
set num [dict get $reg_scope variablesReference]
lassign [dap_check_request_and_response "fetch all registers" \