public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 6/7] Handle the static link in FrameDecorator
Date: Wed, 01 Nov 2023 11:09:38 -0600	[thread overview]
Message-ID: <20231101-dap-nested-function-v1-6-0b0c3b228ac7@adacore.com> (raw)
In-Reply-To: <20231101-dap-nested-function-v1-0-0b0c3b228ac7@adacore.com>

A co-worker requested that the DAP scope for a nested function's frame
also show the variables from outer frames.  DAP doesn't directly
support this notion, so this patch arranges to put these variables
into the inner frames "Locals" scope.

I chose to do this only for DAP.  For CLI and MI, gdb currently does
not do this, so this preserves the behavior.

Note that an earlier patch (see commit 4a1311ba) removed some code
that seemed to do something similar.  However, that code did not
actually work.
---
 gdb/python/lib/gdb/FrameDecorator.py      | 64 +++++++++++++++++-----
 gdb/testsuite/gdb.dap/ada-nested.exp      | 91 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/ada-nested/prog.adb | 32 +++++++++++
 3 files changed, 174 insertions(+), 13 deletions(-)

diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index 39ee2e2547c..b6eb1ab1de0 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -213,18 +213,36 @@ class DAPFrameDecorator(_FrameDecoratorBase):
             return sal.symtab.fullname()
         return None
 
+    def frame_locals(self):
+        """Return an iterable of local variables for this frame, if
+        any.  The iterable object contains objects conforming with the
+        Symbol/Value interface.  If there are no frame locals, or if
+        this frame is deemed to be a special case, return None."""
+
+        if hasattr(self._base, "frame_locals"):
+            return self._base.frame_locals()
+
+        frame = self.inferior_frame()
+        args = FrameVars(frame)
+        return args.fetch_frame_locals(True)
+
 
 class SymValueWrapper(object):
     """A container class conforming to the Symbol/Value interface
     which holds frame locals or frame arguments."""
 
-    def __init__(self, symbol, value):
+    # The FRAME argument is needed here because gdb.Symbol doesn't
+    # carry the block with it, and so read_var can't find symbols from
+    # outer (static link) frames.
+    def __init__(self, frame, symbol):
+        self.frame = frame
         self.sym = symbol
-        self.val = value
 
     def value(self):
         """Return the value associated with this symbol, or None"""
-        return self.val
+        if self.frame is None:
+            return None
+        return self.frame.read_var(self.sym)
 
     def symbol(self):
         """Return the symbol, or Python text, associated with this
@@ -240,32 +258,50 @@ class FrameVars(object):
     def __init__(self, frame):
         self.frame = frame
 
-    def fetch_frame_locals(self):
+    def fetch_frame_locals(self, follow_link=False):
         """Public utility method to fetch frame local variables for
         the stored frame.  Frame arguments are not fetched.  If there
         are no frame local variables, return an empty list."""
         lvars = []
 
+        frame = self.frame
         try:
-            block = self.frame.block()
+            block = frame.block()
         except RuntimeError:
             block = None
 
+        traversed_link = False
         while block is not None:
             if block.is_global or block.is_static:
                 break
             for sym in block:
+                # Exclude arguments from the innermost function, but
+                # if we found and traversed a static link, just treat
+                # all such variables as "local".
                 if sym.is_argument:
+                    if not traversed_link:
+                        continue
+                elif not sym.is_variable:
+                    # We use an 'elif' here because is_variable
+                    # returns False for arguments as well.  Anyway,
+                    # don't include non-variables here.
                     continue
-                if sym.is_variable:
-                    lvars.append(SymValueWrapper(sym, None))
+                lvars.append(SymValueWrapper(frame, sym))
 
-            # Stop when the function itself is seen, to avoid showing
-            # variables from outer functions in a nested function.
             if block.function is not None:
-                break
-
-            block = block.superblock
+                if not follow_link:
+                    break
+                # If the frame has a static link, follow it here.
+                traversed_link = True
+                frame = frame.static_link()
+                if frame is None:
+                    break
+                try:
+                    block = frame.block()
+                except RuntimeError:
+                    block = None
+            else:
+                block = block.superblock
 
         return lvars
 
@@ -287,10 +323,12 @@ class FrameVars(object):
             for sym in block:
                 if not sym.is_argument:
                     continue
-                args.append(SymValueWrapper(sym, None))
+                args.append(SymValueWrapper(None, sym))
 
             # Stop when the function itself is seen, to avoid showing
             # variables from outer functions in a nested function.
+            # Note that we don't traverse the static link for
+            # arguments, only for locals.
             if block.function is not None:
                 break
 
diff --git a/gdb/testsuite/gdb.dap/ada-nested.exp b/gdb/testsuite/gdb.dap/ada-nested.exp
new file mode 100644
index 00000000000..1a02f4f352e
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/ada-nested.exp
@@ -0,0 +1,91 @@
+# 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/>.
+
+# Check the scope of a nested function.
+
+load_lib ada.exp
+load_lib dap-support.exp
+
+require allow_ada_tests allow_dap_tests
+
+standard_ada_testfile prog
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable \
+	 {debug additional_flags=-gnata}] != ""} {
+    return -1
+}
+
+if {[dap_launch $binfile] == ""} {
+    return
+}
+
+set line [gdb_get_line_number "STOP"]
+set obj [dap_check_request_and_response "set breakpoint" \
+	     setBreakpoints \
+	     [format {o source [o path [%s]] \
+			  breakpoints [a [o line [i %d]]]} \
+		  [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 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 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]
+
+# This is what the implementation does, so we can assume it, but check
+# just in case something changes.
+lassign $scopes args locals _ignore
+gdb_assert {[dict get $args name] == "Arguments"} "argument scope"
+gdb_assert {[dict get $locals name] == "Locals"} "local scope"
+
+gdb_assert {[dict get $locals namedVariables] == 3} "two locals"
+
+set num [dict get $locals variablesReference]
+set refs [lindex [dap_check_request_and_response "fetch variables" \
+		      "variables" \
+		      [format {o variablesReference [i %d] count [i 3]} \
+			   $num]] \
+	      0]
+
+foreach var [dict get $refs body variables] {
+    set name [dict get $var name]
+
+    switch $name {
+	"i" {
+	    gdb_assert {[dict get $var value] == "1"} "check value of i"
+	}
+	"x" {
+	    gdb_assert {[dict get $var value] == "12"} "check value of x"
+	}
+	"outer_arg" {
+	    gdb_assert {[dict get $var value] == "1"} "check value of outer_arg"
+	}
+	default {
+	    fail "unknown variable $name"
+	}
+    }
+}
+
+dap_shutdown
diff --git a/gdb/testsuite/gdb.dap/ada-nested/prog.adb b/gdb/testsuite/gdb.dap/ada-nested/prog.adb
new file mode 100644
index 00000000000..6b38835a94c
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/ada-nested/prog.adb
@@ -0,0 +1,32 @@
+--  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/>.
+
+procedure Foo is
+   X  : Integer := 12;
+
+   procedure Outer (Outer_Arg : Integer) is
+      procedure Bump (Stride : Integer) is
+      begin
+         X := X + Stride;          -- STOP
+      end;
+   begin
+      Bump (Outer_Arg);
+   end;
+
+begin
+   for I in 1 .. 20 loop
+      Outer (1);
+   end loop;
+end Foo;

-- 
2.40.1


  parent reply	other threads:[~2023-11-01 17:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-01 17:09 [PATCH 0/7] Handle nested functions in DAP Tom Tromey
2023-11-01 17:09 ` [PATCH 1/7] Add two convenience methods to block Tom Tromey
2023-11-01 17:09 ` [PATCH 2/7] Add block::function_block Tom Tromey
2023-11-01 17:09 ` [PATCH 3/7] Move follow_static_link to frame.c Tom Tromey
2023-11-01 17:09 ` [PATCH 4/7] Add gdb.Frame.static_link method Tom Tromey
2023-11-01 17:17   ` Eli Zaretskii
2023-11-01 17:09 ` [PATCH 5/7] Fix a bug in DAP scopes code Tom Tromey
2023-11-01 17:09 ` Tom Tromey [this message]
2023-11-15 12:31   ` [PATCH 6/7] Handle the static link in FrameDecorator Tom de Vries
2023-11-15 13:58     ` Tom Tromey
2023-11-15 14:07       ` Tom de Vries
2023-11-15 14:58         ` Tom Tromey
2023-11-01 17:09 ` [PATCH 7/7] Update gdb.Symbol.is_variable documentation Tom Tromey
2023-11-01 17:15   ` Eli Zaretskii
2023-11-14 15:39 ` [PATCH 0/7] Handle nested functions in DAP Tom Tromey
2023-11-14 16:11   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231101-dap-nested-function-v1-6-0b0c3b228ac7@adacore.com \
    --to=tromey@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).