diff mbox series

Add DAP scope cache

Message ID 20231002183615.161787-1-tromey@adacore.com
State New
Headers
Series Add DAP scope cache |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey Oct. 2, 2023, 6:36 p.m. UTC 
  Andry Ogorodnik, a co-worker, noticed that multiple "scopes" requests
with the same frame would yield different variableReference values in
the response.

This patch adds a regression test for this, and adds a scope cache in
scopes.py, ensuring that multiple identical requests will get the same
response.
---
 gdb/python/lib/gdb/dap/scopes.py | 38 ++++++++++++++++++++++++--------
 gdb/testsuite/gdb.dap/scopes.exp |  7 ++++++
 2 files changed, 36 insertions(+), 9 deletions(-)

Comments

Alexandra Petlanova Hajkova Oct. 3, 2023, 12:16 p.m. UTC | #1 
On Mon, Oct 2, 2023 at 8:36 PM Tom Tromey via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> Andry Ogorodnik, a co-worker, noticed that multiple "scopes" requests
> with the same frame would yield different variableReference values in
> the response.
>
> This patch adds a regression test for this, and adds a scope cache in
> scopes.py, ensuring that multiple identical requests will get the same
> response.
>
> 2.40.1
>
>
I tested this on ppc64le (Power 10) with Fedora Rawhide and I can confirm
there're no regressions and the gdb.dap/scopes.exp all passes.
Tom Tromey Oct. 16, 2023, 2:41 p.m. UTC | #2 
>>>>> "Alexandra" == Alexandra Petlanova Hajkova via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Andry Ogorodnik, a co-worker, noticed that multiple "scopes" requests
>> with the same frame would yield different variableReference values in
>> the response.
>> 
>> This patch adds a regression test for this, and adds a scope cache in
>> scopes.py, ensuring that multiple identical requests will get the same
>> response.

Alexandra> I tested this on ppc64le (Power 10) with Fedora Rawhide and I can confirm
Alexandra> there're no regressions and the gdb.dap/scopes.exp all passes.

Thanks.  I've added a Tested-By with your email, and I'm going to check
this in.  I'm also going to push it to the gdb 14 branch -- I plan to do
this with the DAP fixes generally, since it is a new feature.

Tom
diff mbox series

Patch

diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py
index 4874d001216..5dde060f44f 100644
--- a/gdb/python/lib/gdb/dap/scopes.py
+++ b/gdb/python/lib/gdb/dap/scopes.py
@@ -21,6 +21,21 @@  from .server import request
 from .varref import BaseReference
 
 
+# Map DAP frame IDs to scopes.  This ensures that scopes are re-used.
+frame_to_scope = {}
+
+
+# When the inferior is re-started, we erase all scope references.  See
+# the section "Lifetime of Objects References" in the spec.
+@in_gdb_thread
+def clear_scopes(event):
+    global frame_to_scope
+    frame_to_scope = {}
+
+
+gdb.events.cont.connect(clear_scopes)
+
+
 class _ScopeReference(BaseReference):
     def __init__(self, name, hint, frame, var_list):
         super().__init__(name)
@@ -83,15 +98,20 @@  class _RegisterReference(_ScopeReference):
 # Helper function to create a DAP scopes for a given frame ID.
 @in_gdb_thread
 def _get_scope(id):
-    frame = frame_for_id(id)
-    scopes = []
-    args = frame.frame_args()
-    if args:
-        scopes.append(_ScopeReference("Arguments", "arguments", frame, args))
-    locs = frame.frame_locals()
-    if locs:
-        scopes.append(_ScopeReference("Locals", "locals", frame, locs))
-    scopes.append(_RegisterReference("Registers", frame))
+    global frame_to_scope
+    if id in frame_to_scope:
+        scopes = frame_to_scope[id]
+    else:
+        frame = frame_for_id(id)
+        scopes = []
+        args = frame.frame_args()
+        if args:
+            scopes.append(_ScopeReference("Arguments", "arguments", frame, args))
+        locs = frame.frame_locals()
+        if locs:
+            scopes.append(_ScopeReference("Locals", "locals", frame, locs))
+        scopes.append(_RegisterReference("Registers", frame))
+        frame_to_scope[id] = scopes
     return [x.to_object() for x in scopes]
 
 
diff --git a/gdb/testsuite/gdb.dap/scopes.exp b/gdb/testsuite/gdb.dap/scopes.exp
index 6937badcca0..003557cf323 100644
--- a/gdb/testsuite/gdb.dap/scopes.exp
+++ b/gdb/testsuite/gdb.dap/scopes.exp
@@ -52,6 +52,13 @@  set scopes [dap_check_request_and_response "get scopes" scopes \
 		[format {o frameId [i %d]} $frame_id]]
 set scopes [dict get [lindex $scopes 0] body scopes]
 
+# Request the scopes twice, and verify that the results are identical.
+# GDB previously had a bug where it would return new scopes each time.
+set scopes2 [dap_check_request_and_response "get scopes again" scopes \
+		 [format {o frameId [i %d]} $frame_id]]
+set scopes2 [dict get [lindex $scopes2 0] body scopes]
+gdb_assert {$scopes2 == $scopes} "identical scopes requests yield same body"
+
 gdb_assert {[llength $scopes] == 2} "two scopes"
 
 lassign $scopes scope reg_scope