[PATCH] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

Andrew Burgess
When using gdb.lookup_static_symbol I think that GDB should find
static symbols (global symbol with static linkage) from the current
object file ahead of static symbols from other object files.

This means that if we have two source files f1.c and f2.c, and both
files contains 'static int foo;', then when we are stopped in f1.c a
call to 'gdb.lookup_static_symbol ("foo")' will find f1.c::foo, and if
we are stopped in f2.c we would find 'f2.c::foo'.

Given that gdb.lookup_static_symbol always returns a single symbol,
but there can be multiple static symbols with the same name GDB is
always making a choice about which symbols to return.  I think that it
makes sense for the choice GDB makes in this case to match what a user
would get on the command line if they asked to 'print foo'.

gdb/testsuite/ChangeLog:

        * gdb.python/py-symbol.c: Declare and call function from new
        py-symbol-2.c file.
        * gdb.python/py-symbol.exp: Compile both source files, and add new
        tests for gdb.lookup_static_symbol.
        * gdb.python/py-symbol-2.c: New file.

gdb/doc/ChangeLog:

        * python.texi (Symbols In Python): Extend documentation for
        gdb.lookup_static_symbol.

gdb/ChangeLog:

        * python/py-symbol.c (gdbpy_lookup_static_symbol): Lookup in
        static block of current object file first.
---
 gdb/ChangeLog                          |  5 +++++
 gdb/doc/ChangeLog                      |  5 +++++
 gdb/doc/python.texi                    |  8 ++++++++
 gdb/python/py-symbol.c                 | 25 +++++++++++++++++++++++-
 gdb/testsuite/ChangeLog                |  8 ++++++++
 gdb/testsuite/gdb.python/py-symbol-2.c | 26 +++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-symbol.c   |  9 +++++++++
 gdb/testsuite/gdb.python/py-symbol.exp | 35 ++++++++++++++++++++++------------
 8 files changed, 108 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-symbol-2.c

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 832283dede8..12f169afa78 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4869,6 +4869,14 @@
 up such variables, iterate over the variables of the function's
 @code{gdb.Block} and check that @code{block.addr_class} is
 @code{gdb.SYMBOL_LOC_STATIC}.
+
+There can be multiple global symbols with static linkage with the same
+name.  This function will only return the first matching symbol that
+it finds.  Which symbol is found depends on where @value{GDBN} is
+currently stopped, as @value{GDBN} will first search for matching
+symbols in the current object file, and then search all other object
+files.  If the application is not yet running then @value{GDBN} will
+search all object files.
 @end defun
 
 A @code{gdb.Symbol} object has the following attributes:
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 2b10e21d872..ae9aca6c5d0 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -487,9 +487,32 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
  &domain))
     return NULL;
 
+  /* In order to find static symbols associated with the "current" object
+     file ahead of those from other object files, we first need to see if
+     we can acquire a current block.  If this fails however, then we still
+     want to search all static symbols, so don't throw an exception just
+     yet.  */
+  const struct block *block = NULL;
   try
     {
-      symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
+      struct frame_info *selected_frame
+ = get_selected_frame (_("No frame selected."));
+      block = get_frame_block (selected_frame, NULL);
+    }
+  catch (const gdb_exception &except)
+    {
+      /* Nothing.  */
+    }
+
+  try
+    {
+      if (block != nullptr)
+ symbol
+  = lookup_symbol_in_static_block (name, block,
+   (domain_enum) domain).symbol;
+
+      if (symbol == nullptr)
+ symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
     }
   catch (const gdb_exception &except)
     {
diff --git a/gdb/testsuite/gdb.python/py-symbol-2.c b/gdb/testsuite/gdb.python/py-symbol-2.c
new file mode 100644
index 00000000000..12872efe4b7
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-symbol-2.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010-2019 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/>.  */
+
+static int rr = 99; /* line of other rr */
+
+void
+function_in_other_file (void)
+{
+  /* Nothing.  */
+}
+
+
diff --git a/gdb/testsuite/gdb.python/py-symbol.c b/gdb/testsuite/gdb.python/py-symbol.c
index 06a931bf5d9..51325dcd450 100644
--- a/gdb/testsuite/gdb.python/py-symbol.c
+++ b/gdb/testsuite/gdb.python/py-symbol.c
@@ -38,6 +38,10 @@ namespace {
 };
 #endif
 
+#ifdef USE_TWO_FILES
+extern void function_in_other_file (void);
+#endif
+
 int qq = 72; /* line of qq */
 static int rr = 42; /* line of rr */
 
@@ -70,5 +74,10 @@ int main (int argc, char *argv[])
   sclass.seti (42);
   sclass.valueofi ();
 #endif
+
+#ifdef USE_TWO_FILES
+  function_in_other_file ();
+#endif
+
   return 0; /* Break at end.  */
 }
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index 5617f127e5b..296845783b1 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -18,9 +18,11 @@
 
 load_lib gdb-python.exp
 
-standard_testfile
+standard_testfile py-symbol.c py-symbol-2.c
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+set opts { debug additional_flags=-DUSE_TWO_FILES }
+if {[prepare_for_testing "failed to prepare" $testfile \
+ [list $srcfile $srcfile2] $opts]} {
     return -1
 }
 
@@ -48,6 +50,7 @@ gdb_test "python print (gdb.lookup_global_symbol('qq').needs_frame)" \
     "False" \
     "print whether qq needs a frame"
 
+# Similarly, test looking up a static symbol before we runto_main.
 set rr_line [gdb_get_line_number "line of rr"]
 gdb_test "python print (gdb.lookup_global_symbol ('rr') is None)" "True" \
     "lookup_global_symbol for static var"
@@ -100,10 +103,23 @@ gdb_test "python print (func.print_name)" "func" "test func.print_name"
 gdb_test "python print (func.linkage_name)" "func" "test func.linkage_name"
 gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test func.addr_class"
 
-gdb_breakpoint [gdb_get_line_number "Break at end."]
+# Stop in a second file and ensure we find its local static symbol.
+gdb_breakpoint "function_in_other_file"
+gdb_continue_to_breakpoint "function_in_other_file"
+gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
+    "print value of rr from other file"
+
+# Now continue back to the first source file.
+set linenum [gdb_get_line_number "Break at end."]
+gdb_breakpoint "$srcfile:$linenum"
 gdb_continue_to_breakpoint "Break at end for variable a" ".*Break at end.*"
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 
+# Check that we find the static sybol local to this file over the
+# static symbol from the second source file.
+gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
+    "print value of rr from main file"
+
 # Test is_variable attribute.
 gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0
 gdb_test "python print (a\[0\].is_variable)" "True" "test a.is_variable"
@@ -145,17 +161,12 @@ gdb_test "python print (t\[0\].symtab)" "${py_symbol_c}" "get symtab"
 
 # C++ tests
 # Recompile binary.
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}-cxx" executable "debug c++"] != "" } {
-    untested "failed to compile in C++ mode"
+lappend opts { c++ }
+if {[prepare_for_testing "failed to prepare" "${binfile}-cxx" \
+ [list $srcfile $srcfile2] $opts]} {
     return -1
 }
 
-# Start with a fresh gdb.
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}-cxx
-
 gdb_test "python print (gdb.lookup_global_symbol ('(anonymous namespace)::anon') is None)" \
     "True" "anon is None"
 gdb_test "python print (gdb.lookup_static_symbol ('(anonymous namespace)::anon').value ())" \
@@ -189,7 +200,7 @@ gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "t
 # Test is_valid when the objfile is unloaded.  This must be the last
 # test as it unloads the object file in GDB.
 # Start with a fresh gdb.
-clean_restart ${testfile}
+clean_restart ${binfile}
 if ![runto_main] then {
     fail "cannot run to main."
     return 0
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

Eli Zaretskii
> From: Andrew Burgess <[hidden email]>
> Cc: Christian Biesinger <[hidden email]>, Andrew Burgess <[hidden email]>
> Date: Mon, 23 Sep 2019 17:46:38 +0100
>
> +There can be multiple global symbols with static linkage with the same
> +name.  This function will only return the first matching symbol that
> +it finds.  Which symbol is found depends on where @value{GDBN} is
> +currently stopped, as @value{GDBN} will first search for matching
> +symbols in the current object file, and then search all other object
> +files.  If the application is not yet running then @value{GDBN} will
> +search all object files.

I don't understand the last sentence: AFAIU, GDB will search "all"
object files even if the program runs, it just will start from the
current object file.  So the last sentence should instead explain in
which order will GDB search the object files when the debuggee is not
yet running, i.e. what will be the first examined object file.  Am I
missing something?

Otherwise, the documentation part is approved.

Thanks.
Reply | Threaded
Open this post in threaded view
|

[PATCHv2] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

Andrew Burgess
Changes since v1:

  - Minor tweak to docs to address Eli's feedback,
  - Rebase to current HEAD,
  - Fixed a small bug in the test script that caused some failures in
    C++ part of the test - no idea how I missed this originally.

Thanks,
Andrew

---

When using gdb.lookup_static_symbol I think that GDB should find
static symbols (global symbol with static linkage) from the current
object file ahead of static symbols from other object files.

This means that if we have two source files f1.c and f2.c, and both
files contains 'static int foo;', then when we are stopped in f1.c a
call to 'gdb.lookup_static_symbol ("foo")' will find f1.c::foo, and if
we are stopped in f2.c we would find 'f2.c::foo'.

Given that gdb.lookup_static_symbol always returns a single symbol,
but there can be multiple static symbols with the same name GDB is
always making a choice about which symbols to return.  I think that it
makes sense for the choice GDB makes in this case to match what a user
would get on the command line if they asked to 'print foo'.

gdb/testsuite/ChangeLog:

        * gdb.python/py-symbol.c: Declare and call function from new
        py-symbol-2.c file.
        * gdb.python/py-symbol.exp: Compile both source files, and add new
        tests for gdb.lookup_static_symbol.
        * gdb.python/py-symbol-2.c: New file.

gdb/doc/ChangeLog:

        * python.texi (Symbols In Python): Extend documentation for
        gdb.lookup_static_symbol.

gdb/ChangeLog:

        * python/py-symbol.c (gdbpy_lookup_static_symbol): Lookup in
        static block of current object file first.
---
 gdb/ChangeLog                          |  5 +++++
 gdb/doc/ChangeLog                      |  5 +++++
 gdb/doc/python.texi                    |  9 +++++++++
 gdb/python/py-symbol.c                 | 25 +++++++++++++++++++++++-
 gdb/testsuite/ChangeLog                |  8 ++++++++
 gdb/testsuite/gdb.python/py-symbol-2.c | 26 +++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-symbol.c   |  9 +++++++++
 gdb/testsuite/gdb.python/py-symbol.exp | 35 ++++++++++++++++++++++------------
 8 files changed, 109 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-symbol-2.c

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 48adad4e97b..0f12de94bba 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4869,6 +4869,15 @@
 up such variables, iterate over the variables of the function's
 @code{gdb.Block} and check that @code{block.addr_class} is
 @code{gdb.SYMBOL_LOC_STATIC}.
+
+There can be multiple global symbols with static linkage with the same
+name.  This function will only return the first matching symbol that
+it finds.  Which symbol is found depends on where @value{GDBN} is
+currently stopped, as @value{GDBN} will first search for matching
+symbols in the current object file, and then search all other object
+files.  If the application is not yet running then @value{GDBN} will
+search all object files in the order they appear in the debug
+information.
 @end defun
 
 A @code{gdb.Symbol} object has the following attributes:
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 2b10e21d872..ae9aca6c5d0 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -487,9 +487,32 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
  &domain))
     return NULL;
 
+  /* In order to find static symbols associated with the "current" object
+     file ahead of those from other object files, we first need to see if
+     we can acquire a current block.  If this fails however, then we still
+     want to search all static symbols, so don't throw an exception just
+     yet.  */
+  const struct block *block = NULL;
   try
     {
-      symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
+      struct frame_info *selected_frame
+ = get_selected_frame (_("No frame selected."));
+      block = get_frame_block (selected_frame, NULL);
+    }
+  catch (const gdb_exception &except)
+    {
+      /* Nothing.  */
+    }
+
+  try
+    {
+      if (block != nullptr)
+ symbol
+  = lookup_symbol_in_static_block (name, block,
+   (domain_enum) domain).symbol;
+
+      if (symbol == nullptr)
+ symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
     }
   catch (const gdb_exception &except)
     {
diff --git a/gdb/testsuite/gdb.python/py-symbol-2.c b/gdb/testsuite/gdb.python/py-symbol-2.c
new file mode 100644
index 00000000000..12872efe4b7
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-symbol-2.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010-2019 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/>.  */
+
+static int rr = 99; /* line of other rr */
+
+void
+function_in_other_file (void)
+{
+  /* Nothing.  */
+}
+
+
diff --git a/gdb/testsuite/gdb.python/py-symbol.c b/gdb/testsuite/gdb.python/py-symbol.c
index 06a931bf5d9..51325dcd450 100644
--- a/gdb/testsuite/gdb.python/py-symbol.c
+++ b/gdb/testsuite/gdb.python/py-symbol.c
@@ -38,6 +38,10 @@ namespace {
 };
 #endif
 
+#ifdef USE_TWO_FILES
+extern void function_in_other_file (void);
+#endif
+
 int qq = 72; /* line of qq */
 static int rr = 42; /* line of rr */
 
@@ -70,5 +74,10 @@ int main (int argc, char *argv[])
   sclass.seti (42);
   sclass.valueofi ();
 #endif
+
+#ifdef USE_TWO_FILES
+  function_in_other_file ();
+#endif
+
   return 0; /* Break at end.  */
 }
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index 5617f127e5b..61960075565 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -18,9 +18,11 @@
 
 load_lib gdb-python.exp
 
-standard_testfile
+standard_testfile py-symbol.c py-symbol-2.c
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+set opts { debug additional_flags=-DUSE_TWO_FILES }
+if {[prepare_for_testing "failed to prepare" $testfile \
+ [list $srcfile $srcfile2] $opts]} {
     return -1
 }
 
@@ -48,6 +50,7 @@ gdb_test "python print (gdb.lookup_global_symbol('qq').needs_frame)" \
     "False" \
     "print whether qq needs a frame"
 
+# Similarly, test looking up a static symbol before we runto_main.
 set rr_line [gdb_get_line_number "line of rr"]
 gdb_test "python print (gdb.lookup_global_symbol ('rr') is None)" "True" \
     "lookup_global_symbol for static var"
@@ -100,10 +103,23 @@ gdb_test "python print (func.print_name)" "func" "test func.print_name"
 gdb_test "python print (func.linkage_name)" "func" "test func.linkage_name"
 gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test func.addr_class"
 
-gdb_breakpoint [gdb_get_line_number "Break at end."]
+# Stop in a second file and ensure we find its local static symbol.
+gdb_breakpoint "function_in_other_file"
+gdb_continue_to_breakpoint "function_in_other_file"
+gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
+    "print value of rr from other file"
+
+# Now continue back to the first source file.
+set linenum [gdb_get_line_number "Break at end."]
+gdb_breakpoint "$srcfile:$linenum"
 gdb_continue_to_breakpoint "Break at end for variable a" ".*Break at end.*"
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 
+# Check that we find the static sybol local to this file over the
+# static symbol from the second source file.
+gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
+    "print value of rr from main file"
+
 # Test is_variable attribute.
 gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0
 gdb_test "python print (a\[0\].is_variable)" "True" "test a.is_variable"
@@ -145,17 +161,12 @@ gdb_test "python print (t\[0\].symtab)" "${py_symbol_c}" "get symtab"
 
 # C++ tests
 # Recompile binary.
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}-cxx" executable "debug c++"] != "" } {
-    untested "failed to compile in C++ mode"
+lappend opts c++
+if {[prepare_for_testing "failed to prepare" "${binfile}-cxx" \
+ [list $srcfile $srcfile2] $opts]} {
     return -1
 }
 
-# Start with a fresh gdb.
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}-cxx
-
 gdb_test "python print (gdb.lookup_global_symbol ('(anonymous namespace)::anon') is None)" \
     "True" "anon is None"
 gdb_test "python print (gdb.lookup_static_symbol ('(anonymous namespace)::anon').value ())" \
@@ -189,7 +200,7 @@ gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "t
 # Test is_valid when the objfile is unloaded.  This must be the last
 # test as it unloads the object file in GDB.
 # Start with a fresh gdb.
-clean_restart ${testfile}
+clean_restart ${binfile}
 if ![runto_main] then {
     fail "cannot run to main."
     return 0
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCHv2] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

Eli Zaretskii
> From: Andrew Burgess <[hidden email]>
> Cc: Christian Biesinger <[hidden email]>, Andrew Burgess <[hidden email]>
> Date: Tue,  8 Oct 2019 12:07:37 +0100
>
> Changes since v1:
>
>   - Minor tweak to docs to address Eli's feedback,
>   - Rebase to current HEAD,
>   - Fixed a small bug in the test script that caused some failures in
>     C++ part of the test - no idea how I missed this originally.

OK for the documentation part.

Thanks.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHv2] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

Sourceware - gdb-patches mailing list
In reply to this post by Andrew Burgess
On Tue, Oct 8, 2019 at 6:07 AM Andrew Burgess
<[hidden email]> wrote:

>
> Changes since v1:
>
>   - Minor tweak to docs to address Eli's feedback,
>   - Rebase to current HEAD,
>   - Fixed a small bug in the test script that caused some failures in
>     C++ part of the test - no idea how I missed this originally.
>
> Thanks,
> Andrew
>
> ---
>
> When using gdb.lookup_static_symbol I think that GDB should find
> static symbols (global symbol with static linkage) from the current
> object file ahead of static symbols from other object files.

So I am not sure how I feel about this. Since this is a programmatic
API, I'm not sure it should be affected by the current program status
like this. Maybe this should be optional behavior, or maybe it should
be possible to pass in a block? (At the same time, it is already
possible to find static variables through the block, if you have one)

Christian

> This means that if we have two source files f1.c and f2.c, and both
> files contains 'static int foo;', then when we are stopped in f1.c a
> call to 'gdb.lookup_static_symbol ("foo")' will find f1.c::foo, and if
> we are stopped in f2.c we would find 'f2.c::foo'.
>
> Given that gdb.lookup_static_symbol always returns a single symbol,
> but there can be multiple static symbols with the same name GDB is
> always making a choice about which symbols to return.  I think that it
> makes sense for the choice GDB makes in this case to match what a user
> would get on the command line if they asked to 'print foo'.
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.python/py-symbol.c: Declare and call function from new
>         py-symbol-2.c file.
>         * gdb.python/py-symbol.exp: Compile both source files, and add new
>         tests for gdb.lookup_static_symbol.
>         * gdb.python/py-symbol-2.c: New file.
>
> gdb/doc/ChangeLog:
>
>         * python.texi (Symbols In Python): Extend documentation for
>         gdb.lookup_static_symbol.
>
> gdb/ChangeLog:
>
>         * python/py-symbol.c (gdbpy_lookup_static_symbol): Lookup in
>         static block of current object file first.
> ---
>  gdb/ChangeLog                          |  5 +++++
>  gdb/doc/ChangeLog                      |  5 +++++
>  gdb/doc/python.texi                    |  9 +++++++++
>  gdb/python/py-symbol.c                 | 25 +++++++++++++++++++++++-
>  gdb/testsuite/ChangeLog                |  8 ++++++++
>  gdb/testsuite/gdb.python/py-symbol-2.c | 26 +++++++++++++++++++++++++
>  gdb/testsuite/gdb.python/py-symbol.c   |  9 +++++++++
>  gdb/testsuite/gdb.python/py-symbol.exp | 35 ++++++++++++++++++++++------------
>  8 files changed, 109 insertions(+), 13 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/py-symbol-2.c
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 48adad4e97b..0f12de94bba 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4869,6 +4869,15 @@
>  up such variables, iterate over the variables of the function's
>  @code{gdb.Block} and check that @code{block.addr_class} is
>  @code{gdb.SYMBOL_LOC_STATIC}.
> +
> +There can be multiple global symbols with static linkage with the same
> +name.  This function will only return the first matching symbol that
> +it finds.  Which symbol is found depends on where @value{GDBN} is
> +currently stopped, as @value{GDBN} will first search for matching
> +symbols in the current object file, and then search all other object
> +files.  If the application is not yet running then @value{GDBN} will
> +search all object files in the order they appear in the debug
> +information.
>  @end defun
>
>  A @code{gdb.Symbol} object has the following attributes:
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 2b10e21d872..ae9aca6c5d0 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -487,9 +487,32 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
>                                         &domain))
>      return NULL;
>
> +  /* In order to find static symbols associated with the "current" object
> +     file ahead of those from other object files, we first need to see if
> +     we can acquire a current block.  If this fails however, then we still
> +     want to search all static symbols, so don't throw an exception just
> +     yet.  */
> +  const struct block *block = NULL;
>    try
>      {
> -      symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
> +      struct frame_info *selected_frame
> +       = get_selected_frame (_("No frame selected."));
> +      block = get_frame_block (selected_frame, NULL);
> +    }
> +  catch (const gdb_exception &except)
> +    {
> +      /* Nothing.  */
> +    }
> +
> +  try
> +    {
> +      if (block != nullptr)
> +       symbol
> +         = lookup_symbol_in_static_block (name, block,
> +                                          (domain_enum) domain).symbol;
> +
> +      if (symbol == nullptr)
> +       symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
>      }
>    catch (const gdb_exception &except)
>      {
> diff --git a/gdb/testsuite/gdb.python/py-symbol-2.c b/gdb/testsuite/gdb.python/py-symbol-2.c
> new file mode 100644
> index 00000000000..12872efe4b7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-symbol-2.c
> @@ -0,0 +1,26 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2010-2019 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/>.  */
> +
> +static int rr = 99;            /* line of other rr */
> +
> +void
> +function_in_other_file (void)
> +{
> +  /* Nothing.  */
> +}
> +
> +
> diff --git a/gdb/testsuite/gdb.python/py-symbol.c b/gdb/testsuite/gdb.python/py-symbol.c
> index 06a931bf5d9..51325dcd450 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.c
> +++ b/gdb/testsuite/gdb.python/py-symbol.c
> @@ -38,6 +38,10 @@ namespace {
>  };
>  #endif
>
> +#ifdef USE_TWO_FILES
> +extern void function_in_other_file (void);
> +#endif
> +
>  int qq = 72;                   /* line of qq */
>  static int rr = 42;            /* line of rr */
>
> @@ -70,5 +74,10 @@ int main (int argc, char *argv[])
>    sclass.seti (42);
>    sclass.valueofi ();
>  #endif
> +
> +#ifdef USE_TWO_FILES
> +  function_in_other_file ();
> +#endif
> +
>    return 0; /* Break at end.  */
>  }
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index 5617f127e5b..61960075565 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -18,9 +18,11 @@
>
>  load_lib gdb-python.exp
>
> -standard_testfile
> +standard_testfile py-symbol.c py-symbol-2.c
>
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> +set opts { debug additional_flags=-DUSE_TWO_FILES }
> +if {[prepare_for_testing "failed to prepare" $testfile \
> +        [list $srcfile $srcfile2] $opts]} {
>      return -1
>  }
>
> @@ -48,6 +50,7 @@ gdb_test "python print (gdb.lookup_global_symbol('qq').needs_frame)" \
>      "False" \
>      "print whether qq needs a frame"
>
> +# Similarly, test looking up a static symbol before we runto_main.
>  set rr_line [gdb_get_line_number "line of rr"]
>  gdb_test "python print (gdb.lookup_global_symbol ('rr') is None)" "True" \
>      "lookup_global_symbol for static var"
> @@ -100,10 +103,23 @@ gdb_test "python print (func.print_name)" "func" "test func.print_name"
>  gdb_test "python print (func.linkage_name)" "func" "test func.linkage_name"
>  gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test func.addr_class"
>
> -gdb_breakpoint [gdb_get_line_number "Break at end."]
> +# Stop in a second file and ensure we find its local static symbol.
> +gdb_breakpoint "function_in_other_file"
> +gdb_continue_to_breakpoint "function_in_other_file"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
> +    "print value of rr from other file"
> +
> +# Now continue back to the first source file.
> +set linenum [gdb_get_line_number "Break at end."]
> +gdb_breakpoint "$srcfile:$linenum"
>  gdb_continue_to_breakpoint "Break at end for variable a" ".*Break at end.*"
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
>
> +# Check that we find the static sybol local to this file over the
> +# static symbol from the second source file.
> +gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
> +    "print value of rr from main file"
> +
>  # Test is_variable attribute.
>  gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0
>  gdb_test "python print (a\[0\].is_variable)" "True" "test a.is_variable"
> @@ -145,17 +161,12 @@ gdb_test "python print (t\[0\].symtab)" "${py_symbol_c}" "get symtab"
>
>  # C++ tests
>  # Recompile binary.
> -if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}-cxx" executable "debug c++"] != "" } {
> -    untested "failed to compile in C++ mode"
> +lappend opts c++
> +if {[prepare_for_testing "failed to prepare" "${binfile}-cxx" \
> +        [list $srcfile $srcfile2] $opts]} {
>      return -1
>  }
>
> -# Start with a fresh gdb.
> -gdb_exit
> -gdb_start
> -gdb_reinitialize_dir $srcdir/$subdir
> -gdb_load ${binfile}-cxx
> -
>  gdb_test "python print (gdb.lookup_global_symbol ('(anonymous namespace)::anon') is None)" \
>      "True" "anon is None"
>  gdb_test "python print (gdb.lookup_static_symbol ('(anonymous namespace)::anon').value ())" \
> @@ -189,7 +200,7 @@ gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "t
>  # Test is_valid when the objfile is unloaded.  This must be the last
>  # test as it unloads the object file in GDB.
>  # Start with a fresh gdb.
> -clean_restart ${testfile}
> +clean_restart ${binfile}
>  if ![runto_main] then {
>      fail "cannot run to main."
>      return 0
> --
> 2.14.5
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHv2] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

Andrew Burgess
* Christian Biesinger <[hidden email]> [2019-10-08 11:00:56 -0500]:

> On Tue, Oct 8, 2019 at 6:07 AM Andrew Burgess
> <[hidden email]> wrote:
> >
> > Changes since v1:
> >
> >   - Minor tweak to docs to address Eli's feedback,
> >   - Rebase to current HEAD,
> >   - Fixed a small bug in the test script that caused some failures in
> >     C++ part of the test - no idea how I missed this originally.
> >
> > Thanks,
> > Andrew
> >
> > ---
> >
> > When using gdb.lookup_static_symbol I think that GDB should find
> > static symbols (global symbol with static linkage) from the current
> > object file ahead of static symbols from other object files.
>
> So I am not sure how I feel about this. Since this is a programmatic
> API, I'm not sure it should be affected by the current program status
> like this. Maybe this should be optional behavior, or maybe it should
> be possible to pass in a block? (At the same time, it is already
> possible to find static variables through the block, if you have one)

Given that gdb.lookup_static_symbol returns a single symbol, I'm
struggling to see how having it return something other than the one
GDB would "naturally" see (if the user took control and just typed 'p
some_variable') would ever be useful.

I think we should either change the API to return the same thing the
CLI would see, or have the API return a list of symbols.  Simply
picking the "first" seems pretty arbitrary, and is (I would guess)
going to be wrong more often than not.

I don't really understand why this being a "programmatic API" makes
any difference - the CLI is programmable (though admittedly it's much
inferior to python scripting) - they both serve the same purpose,
allow the user to send commands to GDB and see the program state.  The
question I think is more, does it make sense to have the symbol return
change based on the current program state.   Given the role of GDB I'd
say yes.

How would you feel if I changed the patch to have
'gdb.lookup_static_symbol' return the one symbol that is the current
value, and 'gdb.lookup_all_static_symbols' return a list of all
matching static symbols?

Thanks,
Andrew


>
> > This means that if we have two source files f1.c and f2.c, and both
> > files contains 'static int foo;', then when we are stopped in f1.c a
> > call to 'gdb.lookup_static_symbol ("foo")' will find f1.c::foo, and if
> > we are stopped in f2.c we would find 'f2.c::foo'.
> >
> > Given that gdb.lookup_static_symbol always returns a single symbol,
> > but there can be multiple static symbols with the same name GDB is
> > always making a choice about which symbols to return.  I think that it
> > makes sense for the choice GDB makes in this case to match what a user
> > would get on the command line if they asked to 'print foo'.
> >
> > gdb/testsuite/ChangeLog:
> >
> >         * gdb.python/py-symbol.c: Declare and call function from new
> >         py-symbol-2.c file.
> >         * gdb.python/py-symbol.exp: Compile both source files, and add new
> >         tests for gdb.lookup_static_symbol.
> >         * gdb.python/py-symbol-2.c: New file.
> >
> > gdb/doc/ChangeLog:
> >
> >         * python.texi (Symbols In Python): Extend documentation for
> >         gdb.lookup_static_symbol.
> >
> > gdb/ChangeLog:
> >
> >         * python/py-symbol.c (gdbpy_lookup_static_symbol): Lookup in
> >         static block of current object file first.
> > ---
> >  gdb/ChangeLog                          |  5 +++++
> >  gdb/doc/ChangeLog                      |  5 +++++
> >  gdb/doc/python.texi                    |  9 +++++++++
> >  gdb/python/py-symbol.c                 | 25 +++++++++++++++++++++++-
> >  gdb/testsuite/ChangeLog                |  8 ++++++++
> >  gdb/testsuite/gdb.python/py-symbol-2.c | 26 +++++++++++++++++++++++++
> >  gdb/testsuite/gdb.python/py-symbol.c   |  9 +++++++++
> >  gdb/testsuite/gdb.python/py-symbol.exp | 35 ++++++++++++++++++++++------------
> >  8 files changed, 109 insertions(+), 13 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.python/py-symbol-2.c
> >
> > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> > index 48adad4e97b..0f12de94bba 100644
> > --- a/gdb/doc/python.texi
> > +++ b/gdb/doc/python.texi
> > @@ -4869,6 +4869,15 @@
> >  up such variables, iterate over the variables of the function's
> >  @code{gdb.Block} and check that @code{block.addr_class} is
> >  @code{gdb.SYMBOL_LOC_STATIC}.
> > +
> > +There can be multiple global symbols with static linkage with the same
> > +name.  This function will only return the first matching symbol that
> > +it finds.  Which symbol is found depends on where @value{GDBN} is
> > +currently stopped, as @value{GDBN} will first search for matching
> > +symbols in the current object file, and then search all other object
> > +files.  If the application is not yet running then @value{GDBN} will
> > +search all object files in the order they appear in the debug
> > +information.
> >  @end defun
> >
> >  A @code{gdb.Symbol} object has the following attributes:
> > diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> > index 2b10e21d872..ae9aca6c5d0 100644
> > --- a/gdb/python/py-symbol.c
> > +++ b/gdb/python/py-symbol.c
> > @@ -487,9 +487,32 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
> >                                         &domain))
> >      return NULL;
> >
> > +  /* In order to find static symbols associated with the "current" object
> > +     file ahead of those from other object files, we first need to see if
> > +     we can acquire a current block.  If this fails however, then we still
> > +     want to search all static symbols, so don't throw an exception just
> > +     yet.  */
> > +  const struct block *block = NULL;
> >    try
> >      {
> > -      symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
> > +      struct frame_info *selected_frame
> > +       = get_selected_frame (_("No frame selected."));
> > +      block = get_frame_block (selected_frame, NULL);
> > +    }
> > +  catch (const gdb_exception &except)
> > +    {
> > +      /* Nothing.  */
> > +    }
> > +
> > +  try
> > +    {
> > +      if (block != nullptr)
> > +       symbol
> > +         = lookup_symbol_in_static_block (name, block,
> > +                                          (domain_enum) domain).symbol;
> > +
> > +      if (symbol == nullptr)
> > +       symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
> >      }
> >    catch (const gdb_exception &except)
> >      {
> > diff --git a/gdb/testsuite/gdb.python/py-symbol-2.c b/gdb/testsuite/gdb.python/py-symbol-2.c
> > new file mode 100644
> > index 00000000000..12872efe4b7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.python/py-symbol-2.c
> > @@ -0,0 +1,26 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2010-2019 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/>.  */
> > +
> > +static int rr = 99;            /* line of other rr */
> > +
> > +void
> > +function_in_other_file (void)
> > +{
> > +  /* Nothing.  */
> > +}
> > +
> > +
> > diff --git a/gdb/testsuite/gdb.python/py-symbol.c b/gdb/testsuite/gdb.python/py-symbol.c
> > index 06a931bf5d9..51325dcd450 100644
> > --- a/gdb/testsuite/gdb.python/py-symbol.c
> > +++ b/gdb/testsuite/gdb.python/py-symbol.c
> > @@ -38,6 +38,10 @@ namespace {
> >  };
> >  #endif
> >
> > +#ifdef USE_TWO_FILES
> > +extern void function_in_other_file (void);
> > +#endif
> > +
> >  int qq = 72;                   /* line of qq */
> >  static int rr = 42;            /* line of rr */
> >
> > @@ -70,5 +74,10 @@ int main (int argc, char *argv[])
> >    sclass.seti (42);
> >    sclass.valueofi ();
> >  #endif
> > +
> > +#ifdef USE_TWO_FILES
> > +  function_in_other_file ();
> > +#endif
> > +
> >    return 0; /* Break at end.  */
> >  }
> > diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> > index 5617f127e5b..61960075565 100644
> > --- a/gdb/testsuite/gdb.python/py-symbol.exp
> > +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> > @@ -18,9 +18,11 @@
> >
> >  load_lib gdb-python.exp
> >
> > -standard_testfile
> > +standard_testfile py-symbol.c py-symbol-2.c
> >
> > -if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> > +set opts { debug additional_flags=-DUSE_TWO_FILES }
> > +if {[prepare_for_testing "failed to prepare" $testfile \
> > +        [list $srcfile $srcfile2] $opts]} {
> >      return -1
> >  }
> >
> > @@ -48,6 +50,7 @@ gdb_test "python print (gdb.lookup_global_symbol('qq').needs_frame)" \
> >      "False" \
> >      "print whether qq needs a frame"
> >
> > +# Similarly, test looking up a static symbol before we runto_main.
> >  set rr_line [gdb_get_line_number "line of rr"]
> >  gdb_test "python print (gdb.lookup_global_symbol ('rr') is None)" "True" \
> >      "lookup_global_symbol for static var"
> > @@ -100,10 +103,23 @@ gdb_test "python print (func.print_name)" "func" "test func.print_name"
> >  gdb_test "python print (func.linkage_name)" "func" "test func.linkage_name"
> >  gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test func.addr_class"
> >
> > -gdb_breakpoint [gdb_get_line_number "Break at end."]
> > +# Stop in a second file and ensure we find its local static symbol.
> > +gdb_breakpoint "function_in_other_file"
> > +gdb_continue_to_breakpoint "function_in_other_file"
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
> > +    "print value of rr from other file"
> > +
> > +# Now continue back to the first source file.
> > +set linenum [gdb_get_line_number "Break at end."]
> > +gdb_breakpoint "$srcfile:$linenum"
> >  gdb_continue_to_breakpoint "Break at end for variable a" ".*Break at end.*"
> >  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> >
> > +# Check that we find the static sybol local to this file over the
> > +# static symbol from the second source file.
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
> > +    "print value of rr from main file"
> > +
> >  # Test is_variable attribute.
> >  gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0
> >  gdb_test "python print (a\[0\].is_variable)" "True" "test a.is_variable"
> > @@ -145,17 +161,12 @@ gdb_test "python print (t\[0\].symtab)" "${py_symbol_c}" "get symtab"
> >
> >  # C++ tests
> >  # Recompile binary.
> > -if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}-cxx" executable "debug c++"] != "" } {
> > -    untested "failed to compile in C++ mode"
> > +lappend opts c++
> > +if {[prepare_for_testing "failed to prepare" "${binfile}-cxx" \
> > +        [list $srcfile $srcfile2] $opts]} {
> >      return -1
> >  }
> >
> > -# Start with a fresh gdb.
> > -gdb_exit
> > -gdb_start
> > -gdb_reinitialize_dir $srcdir/$subdir
> > -gdb_load ${binfile}-cxx
> > -
> >  gdb_test "python print (gdb.lookup_global_symbol ('(anonymous namespace)::anon') is None)" \
> >      "True" "anon is None"
> >  gdb_test "python print (gdb.lookup_static_symbol ('(anonymous namespace)::anon').value ())" \
> > @@ -189,7 +200,7 @@ gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "t
> >  # Test is_valid when the objfile is unloaded.  This must be the last
> >  # test as it unloads the object file in GDB.
> >  # Start with a fresh gdb.
> > -clean_restart ${testfile}
> > +clean_restart ${binfile}
> >  if ![runto_main] then {
> >      fail "cannot run to main."
> >      return 0
> > --
> > 2.14.5
> >
Reply | Threaded
Open this post in threaded view
|

[PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Andrew Burgess
If gdb.lookup_static_symbol is going to return a single symbol then it
makes sense (I think) for it to return a context sensitive choice of
symbol, that is the static symbol that would be visible to the program
at that point.

However, if the user of the python API wants to instead get a
consistent set of static symbols, no matter where they stop, then they
have to instead consider all static symbols with a given name - there
could be many.  That is what this new API function offers, it returns
a list (possibly empty) of all static symbols matching a given
name (and optionally a given symbol domain).

gdb/ChangeLog:

        * python/py-symbol.c (gdbpy_lookup_all_static_symbols): New
        function.
        * python/python-internal.h (gdbpy_lookup_all_static_symbols):
        Declare new function.
        * python/python.c (python_GdbMethods): Add
        gdb.lookup_all_static_symbols method.

gdb/testsuite/ChangeLog:

        * gdb.python/py-symbol.exp: Add test for
        gdb.lookup_all_static_symbols.

gdb/doc/ChangeLog:

        * python.texi (Symbols In Python): Add documentation for
        gdb.lookup_all_static_symbols.

Change-Id: I1153b0ae5bcbc43b3dcf139043c7a48bf791e1a3
---
 gdb/ChangeLog                          |  9 ++++++
 gdb/doc/ChangeLog                      |  5 +++
 gdb/doc/python.texi                    | 35 +++++++++++++++++++++
 gdb/python/py-symbol.c                 | 56 ++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h           |  2 ++
 gdb/python/python.c                    |  4 +++
 gdb/testsuite/ChangeLog                |  5 +++
 gdb/testsuite/gdb.python/py-symbol.exp |  8 +++++
 8 files changed, 124 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 0f12de94bba..2126effa12f 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4880,6 +4880,41 @@
 information.
 @end defun
 
+@findex gdb.lookup_global_symbol
+@defun gdb.lookup_global_symbol (name @r{[}, domain@r{]})
+This function searches for a global symbol by name.
+The search scope can be restricted to by the domain argument.
+
+@var{name} is the name of the symbol.  It must be a string.
+The optional @var{domain} argument restricts the search to the domain type.
+The @var{domain} argument must be a domain constant defined in the @code{gdb}
+module and described later in this chapter.
+
+The result is a @code{gdb.Symbol} object or @code{None} if the symbol
+is not found.
+@end defun
+
+@findex gdb.lookup_all_static_symbols
+@defun gdb.lookup_all_static_symbols (name @r{[}, domain@r{]})
+Similar to @code{gdb.lookup_static_symbol}, this function searches for
+global symbols with static linkage by name, and optionally restricted
+by the domain argument.  However, this function returns a list of all
+matching symbols found, not just the first one.
+
+@var{name} is the name of the symbol.  It must be a string.
+The optional @var{domain} argument restricts the search to the domain type.
+The @var{domain} argument must be a domain constant defined in the @code{gdb}
+module and described later in this chapter.
+
+The result is a list of @code{gdb.Symbol} objects which could be empty
+if no matching symbols were found.
+
+Note that this function will not find function-scoped static variables. To look
+up such variables, iterate over the variables of the function's
+@code{gdb.Block} and check that @code{block.addr_class} is
+@code{gdb.SYMBOL_LOC_STATIC}.
+@end defun
+
 A @code{gdb.Symbol} object has the following attributes:
 
 @defvar Symbol.type
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index ae9aca6c5d0..6e126f9f377 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -534,6 +534,62 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
   return sym_obj;
 }
 
+/* Implementation of
+   gdb.lookup_all_static_symbols (name [, domain) -> [symbol] or None.
+
+   Returns a list of all static symbols matching NAME in DOMAIN.  */
+
+PyObject *
+gdbpy_lookup_all_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
+{
+  const char *name;
+  int domain = VAR_DOMAIN;
+  static const char *keywords[] = { "name", "domain", NULL };
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
+ &domain))
+    return NULL;
+
+  gdbpy_ref<> return_list (PyList_New (0));
+  if (return_list == NULL)
+    return NULL;
+
+  try
+    {
+      for (objfile *objfile : current_program_space->objfiles ())
+ {
+  for (compunit_symtab *cust : objfile->compunits ())
+    {
+      const struct blockvector *bv;
+      const struct block *block;
+
+      bv = COMPUNIT_BLOCKVECTOR (cust);
+      block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
+
+      if (block != nullptr)
+ {
+  symbol *symbol = lookup_symbol_in_static_block
+    (name, block, (domain_enum) domain).symbol;
+
+  if (symbol != nullptr)
+    {
+      PyObject *sym_obj
+ = symbol_to_symbol_object (symbol);
+      if (PyList_Append (return_list.get (), sym_obj) == -1)
+ return NULL;
+    }
+ }
+    }
+ }
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+
+  return return_list.release ();
+}
+
 /* This function is called when an objfile is about to be freed.
    Invalidate the symbol as further actions on the symbol would result
    in bad data.  All access to obj->symbol should be gated by
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c5578430cff..e29e018a9d8 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -426,6 +426,8 @@ PyObject *gdbpy_lookup_global_symbol (PyObject *self, PyObject *args,
       PyObject *kw);
 PyObject *gdbpy_lookup_static_symbol (PyObject *self, PyObject *args,
       PyObject *kw);
+PyObject *gdbpy_lookup_all_static_symbols (PyObject *self, PyObject *args,
+   PyObject *kw);
 PyObject *gdbpy_start_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_current_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_stop_recording (PyObject *self, PyObject *args);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ddf0e72d26f..80936e2ab7b 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1994,6 +1994,10 @@ Return the symbol corresponding to the given name (or None)." },
     METH_VARARGS | METH_KEYWORDS,
     "lookup_static_symbol (name [, domain]) -> symbol\n\
 Return the static-linkage symbol corresponding to the given name (or None)." },
+  { "lookup_all_static_symbols", (PyCFunction) gdbpy_lookup_all_static_symbols,
+    METH_VARARGS | METH_KEYWORDS,
+    "lookup_all_static_symbols (name [, domain]) -> symbol\n\
+Return a list of all static-linkage symbols corresponding to the given name." },
 
   { "lookup_objfile", (PyCFunction) gdbpy_lookup_objfile,
     METH_VARARGS | METH_KEYWORDS,
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index 61960075565..116c32441f8 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -108,6 +108,10 @@ gdb_breakpoint "function_in_other_file"
 gdb_continue_to_breakpoint "function_in_other_file"
 gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
     "print value of rr from other file"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[0\].value ())" "99" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[0\], from the other file"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[1\].value ())" "42" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[1\], from the other file"
 
 # Now continue back to the first source file.
 set linenum [gdb_get_line_number "Break at end."]
@@ -119,6 +123,10 @@ gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 # static symbol from the second source file.
 gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
     "print value of rr from main file"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[0\].value ())" "99" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[0\], from the main file"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[1\].value ())" "42" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[1\], from the main file"
 
 # Test is_variable attribute.
 gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0
--
2.14.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Eli Zaretskii
> From: Andrew Burgess <[hidden email]>
> Cc: Christian Biesinger <[hidden email]>, Andrew Burgess <[hidden email]>
> Date: Tue, 15 Oct 2019 17:46:47 +0100
>
> gdb/ChangeLog:
>
> * python/py-symbol.c (gdbpy_lookup_all_static_symbols): New
> function.
> * python/python-internal.h (gdbpy_lookup_all_static_symbols):
> Declare new function.
> * python/python.c (python_GdbMethods): Add
> gdb.lookup_all_static_symbols method.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.python/py-symbol.exp: Add test for
> gdb.lookup_all_static_symbols.
>
> gdb/doc/ChangeLog:
>
> * python.texi (Symbols In Python): Add documentation for
> gdb.lookup_all_static_symbols.

The python.texi part is OK, but I think this change also needs a NEWS
entry.

Thanks.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Simon Marchi-4
In reply to this post by Andrew Burgess
On 2019-10-15 12:46 p.m., Andrew Burgess wrote:
> If gdb.lookup_static_symbol is going to return a single symbol then it
> makes sense (I think) for it to return a context sensitive choice of
> symbol, that is the static symbol that would be visible to the program
> at that point.

I remember discussing this with Christian, and I didn't have a strong option.  But
now, I tend to agree with Andrew.

I see two use cases here:

1. I want to get all static symbols named `foo`.  In which case, I'd use the
   function Andrew proposes in this patch.
2. I want to get the static symbol named `foo` that's visible from a certain
   point, perhaps a given block or where my program is currently stopped at.
   Ideally, we would have a "CompilationUnit" object type in Python, such that
   I could use

    block.compunit.lookup_static_symbol('foo')

  or

    gdb.current_compunit().lookup_static_symbol('foo')

If we had that, we wouldn't need gdb.lookup_static_symbol.  However, we don't
have compilation unit objects in Python, so I see gdb.lookup_static_symbol
as a crutch until then.  If gdb.lookup_static_symbol returns in priority the
symbol that's visible from the current context, it partially covers use case #2.

If we have gdb.lookup_all_static_symbols, I think it's not useful to also have
gdb.lookup_static_symbol returning the first element, because it's equivalent
to gdb.lookup_all_static_symbols(...)[0].

> However, if the user of the python API wants to instead get a
> consistent set of static symbols, no matter where they stop, then they
> have to instead consider all static symbols with a given name - there
> could be many.  That is what this new API function offers, it returns
> a list (possibly empty) of all static symbols matching a given
> name (and optionally a given symbol domain).

Bikeshed time: what do you think of naming the new function `lookup_static_symbols`?

> gdb/ChangeLog:
>
> * python/py-symbol.c (gdbpy_lookup_all_static_symbols): New
> function.
> * python/python-internal.h (gdbpy_lookup_all_static_symbols):
> Declare new function.
> * python/python.c (python_GdbMethods): Add
> gdb.lookup_all_static_symbols method.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.python/py-symbol.exp: Add test for
> gdb.lookup_all_static_symbols.
>
> gdb/doc/ChangeLog:
>
> * python.texi (Symbols In Python): Add documentation for
> gdb.lookup_all_static_symbols.
>
> Change-Id: I1153b0ae5bcbc43b3dcf139043c7a48bf791e1a3
> ---
>  gdb/ChangeLog                          |  9 ++++++
>  gdb/doc/ChangeLog                      |  5 +++
>  gdb/doc/python.texi                    | 35 +++++++++++++++++++++
>  gdb/python/py-symbol.c                 | 56 ++++++++++++++++++++++++++++++++++
>  gdb/python/python-internal.h           |  2 ++
>  gdb/python/python.c                    |  4 +++
>  gdb/testsuite/ChangeLog                |  5 +++
>  gdb/testsuite/gdb.python/py-symbol.exp |  8 +++++
>  8 files changed, 124 insertions(+)
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 0f12de94bba..2126effa12f 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4880,6 +4880,41 @@
>  information.
>  @end defun
>  
> +@findex gdb.lookup_global_symbol
> +@defun gdb.lookup_global_symbol (name @r{[}, domain@r{]})
> +This function searches for a global symbol by name.
> +The search scope can be restricted to by the domain argument.
> +
> +@var{name} is the name of the symbol.  It must be a string.
> +The optional @var{domain} argument restricts the search to the domain type.
> +The @var{domain} argument must be a domain constant defined in the @code{gdb}
> +module and described later in this chapter.
> +
> +The result is a @code{gdb.Symbol} object or @code{None} if the symbol
> +is not found.
> +@end defun
> +
> +@findex gdb.lookup_all_static_symbols
> +@defun gdb.lookup_all_static_symbols (name @r{[}, domain@r{]})
> +Similar to @code{gdb.lookup_static_symbol}, this function searches for
> +global symbols with static linkage by name, and optionally restricted
> +by the domain argument.  However, this function returns a list of all
> +matching symbols found, not just the first one.
> +
> +@var{name} is the name of the symbol.  It must be a string.
> +The optional @var{domain} argument restricts the search to the domain type.
> +The @var{domain} argument must be a domain constant defined in the @code{gdb}
> +module and described later in this chapter.
> +
> +The result is a list of @code{gdb.Symbol} objects which could be empty
> +if no matching symbols were found.
> +
> +Note that this function will not find function-scoped static variables. To look
> +up such variables, iterate over the variables of the function's
> +@code{gdb.Block} and check that @code{block.addr_class} is
> +@code{gdb.SYMBOL_LOC_STATIC}.
> +@end defun
> +
>  A @code{gdb.Symbol} object has the following attributes:
>  
>  @defvar Symbol.type
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index ae9aca6c5d0..6e126f9f377 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -534,6 +534,62 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
>    return sym_obj;
>  }
>  
> +/* Implementation of
> +   gdb.lookup_all_static_symbols (name [, domain) -> [symbol] or None.
> +
> +   Returns a list of all static symbols matching NAME in DOMAIN.  */
> +
> +PyObject *
> +gdbpy_lookup_all_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
> +{
> +  const char *name;
> +  int domain = VAR_DOMAIN;
> +  static const char *keywords[] = { "name", "domain", NULL };
> +
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
> + &domain))
> +    return NULL;
> +
> +  gdbpy_ref<> return_list (PyList_New (0));
> +  if (return_list == NULL)
> +    return NULL;
> +
> +  try
> +    {
> +      for (objfile *objfile : current_program_space->objfiles ())
> + {
> +  for (compunit_symtab *cust : objfile->compunits ())

This list only contains the already expanded compilation units.  So it won't find
the static symbols in CUs that are not yet expanded to full compunit symtabs.  Here's
an example:

test.c:

```
static int yo = 2;

int foo(int x);

int main() {
        return foo(4) + yo;
}
```

test2.c:

```
static int yo = 6;

int foo(int x) {
        return yo * x;
}
```

Compiled with

  gcc test.c test2.c -g3 -O0

$ ./gdb --data-directory=data-directory a.out -batch -ex "python print(gdb.lookup_all_static_symbols('yo'))"
[<gdb.Symbol object at 0x7f6223978e68>]
$ ./gdb --data-directory=data-directory a.out -batch -ex "python print(gdb.lookup_all_static_symbols('yo'))" -readnow
[<gdb.Symbol object at 0x7fef0b649e68>, <gdb.Symbol object at 0x7fef0b649f58>]

So I guess there should be a step where we expand CUs with matching symbols first.

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Andrew Burgess
This version:

  1. Renames lookup_all_static_symbols to lookup_static_symbols,
  2. Expands symtabs containing symbols with a matching name before
     finding matches, and
  3. Includes a test similar to Simon's the highlights the need for
     the previous change.

Thanks,
Andrew

---


commit f9cd67c7287475e1cb0a654a6e4e1841e5a78bae
Author: Andrew Burgess <[hidden email]>
Date:   Tue Oct 15 16:18:26 2019 +0100

    gdb/python: Introduce gdb.lookup_static_symbols
   
    If gdb.lookup_static_symbol is going to return a single symbol then it
    makes sense (I think) for it to return a context sensitive choice of
    symbol, that is the static symbol that would be visible to the program
    at that point.
   
    However, if the user of the python API wants to instead get a
    consistent set of static symbols, no matter where they stop, then they
    have to instead consider all static symbols with a given name - there
    could be many.  That is what this new API function offers, it returns
    a list (possibly empty) of all static symbols matching a given
    name (and optionally a given symbol domain).
   
    gdb/ChangeLog:
   
            * python/py-symbol.c (gdbpy_lookup_static_symbols): New
            function.
            * python/python-internal.h (gdbpy_lookup_static_symbols):
            Declare new function.
            * python/python.c (python_GdbMethods): Add
            gdb.lookup_static_symbols method.
   
    gdb/testsuite/ChangeLog:
   
            * gdb.python/py-symbol.exp: Add test for
            gdb.lookup_static_symbols.
   
    gdb/doc/ChangeLog:
   
            * python.texi (Symbols In Python): Add documentation for
            gdb.lookup_static_symbols.
   
    Change-Id: I1153b0ae5bcbc43b3dcf139043c7a48bf791e1a3

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 0f12de94bba..a30c8ae4f03 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4880,6 +4880,41 @@
 information.
 @end defun
 
+@findex gdb.lookup_global_symbol
+@defun gdb.lookup_global_symbol (name @r{[}, domain@r{]})
+This function searches for a global symbol by name.
+The search scope can be restricted to by the domain argument.
+
+@var{name} is the name of the symbol.  It must be a string.
+The optional @var{domain} argument restricts the search to the domain type.
+The @var{domain} argument must be a domain constant defined in the @code{gdb}
+module and described later in this chapter.
+
+The result is a @code{gdb.Symbol} object or @code{None} if the symbol
+is not found.
+@end defun
+
+@findex gdb.lookup_static_symbols
+@defun gdb.lookup_static_symbols (name @r{[}, domain@r{]})
+Similar to @code{gdb.lookup_static_symbol}, this function searches for
+global symbols with static linkage by name, and optionally restricted
+by the domain argument.  However, this function returns a list of all
+matching symbols found, not just the first one.
+
+@var{name} is the name of the symbol.  It must be a string.
+The optional @var{domain} argument restricts the search to the domain type.
+The @var{domain} argument must be a domain constant defined in the @code{gdb}
+module and described later in this chapter.
+
+The result is a list of @code{gdb.Symbol} objects which could be empty
+if no matching symbols were found.
+
+Note that this function will not find function-scoped static variables. To look
+up such variables, iterate over the variables of the function's
+@code{gdb.Block} and check that @code{block.addr_class} is
+@code{gdb.SYMBOL_LOC_STATIC}.
+@end defun
+
 A @code{gdb.Symbol} object has the following attributes:
 
 @defvar Symbol.type
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index ae9aca6c5d0..119b1f2cbbf 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -23,6 +23,7 @@
 #include "symtab.h"
 #include "python-internal.h"
 #include "objfiles.h"
+#include "symfile.h"
 
 typedef struct sympy_symbol_object {
   PyObject_HEAD
@@ -534,6 +535,66 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
   return sym_obj;
 }
 
+/* Implementation of
+   gdb.lookup_static_symbols (name [, domain) -> [symbol] or None.
+
+   Returns a list of all static symbols matching NAME in DOMAIN.  */
+
+PyObject *
+gdbpy_lookup_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
+{
+  const char *name;
+  int domain = VAR_DOMAIN;
+  static const char *keywords[] = { "name", "domain", NULL };
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
+ &domain))
+    return NULL;
+
+  gdbpy_ref<> return_list (PyList_New (0));
+  if (return_list == NULL)
+    return NULL;
+
+  try
+    {
+      /* Expand any symtabs that contain potentially matching symbols.  */
+      lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
+      expand_symtabs_matching (NULL, lookup_name, NULL, NULL, ALL_DOMAIN);
+
+      for (objfile *objfile : current_program_space->objfiles ())
+ {
+  for (compunit_symtab *cust : objfile->compunits ())
+    {
+      const struct blockvector *bv;
+      const struct block *block;
+
+      bv = COMPUNIT_BLOCKVECTOR (cust);
+      block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
+
+      if (block != nullptr)
+ {
+  symbol *symbol = lookup_symbol_in_static_block
+    (name, block, (domain_enum) domain).symbol;
+
+  if (symbol != nullptr)
+    {
+      PyObject *sym_obj
+ = symbol_to_symbol_object (symbol);
+      if (PyList_Append (return_list.get (), sym_obj) == -1)
+ return NULL;
+    }
+ }
+    }
+ }
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+
+  return return_list.release ();
+}
+
 /* This function is called when an objfile is about to be freed.
    Invalidate the symbol as further actions on the symbol would result
    in bad data.  All access to obj->symbol should be gated by
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c5578430cff..703c60032c0 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -426,6 +426,8 @@ PyObject *gdbpy_lookup_global_symbol (PyObject *self, PyObject *args,
       PyObject *kw);
 PyObject *gdbpy_lookup_static_symbol (PyObject *self, PyObject *args,
       PyObject *kw);
+PyObject *gdbpy_lookup_static_symbols (PyObject *self, PyObject *args,
+   PyObject *kw);
 PyObject *gdbpy_start_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_current_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_stop_recording (PyObject *self, PyObject *args);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ddf0e72d26f..f94214e1b24 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1994,6 +1994,10 @@ Return the symbol corresponding to the given name (or None)." },
     METH_VARARGS | METH_KEYWORDS,
     "lookup_static_symbol (name [, domain]) -> symbol\n\
 Return the static-linkage symbol corresponding to the given name (or None)." },
+  { "lookup_static_symbols", (PyCFunction) gdbpy_lookup_static_symbols,
+    METH_VARARGS | METH_KEYWORDS,
+    "lookup_static_symbols (name [, domain]) -> symbol\n\
+Return a list of all static-linkage symbols corresponding to the given name." },
 
   { "lookup_objfile", (PyCFunction) gdbpy_lookup_objfile,
     METH_VARARGS | METH_KEYWORDS,
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index 61960075565..ea41297f54f 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -29,6 +29,15 @@ if {[prepare_for_testing "failed to prepare" $testfile \
 # Skip all tests if Python scripting is not enabled.
 if { [skip_python_tests] } { continue }
 
+# Check that we find all static symbols before the inferior has
+# started, at which point some of the symtabs might not have been
+# expanded.
+gdb_test "python print (len (gdb.lookup_static_symbols ('rr')))" \
+    "2" "print (len (gdb.lookup_static_symbols ('rr')))"
+
+# Restart so we don't have expanded symtabs after the previous test.
+clean_restart ${binfile}
+
 # Test looking up a global symbol before we runto_main as this is the
 # point where we don't have a current frame, and we don't want to
 # require one.
@@ -108,6 +117,10 @@ gdb_breakpoint "function_in_other_file"
 gdb_continue_to_breakpoint "function_in_other_file"
 gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
     "print value of rr from other file"
+gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
+    "print value of gdb.lookup_static_symbols ('rr')\[0\], from the other file"
+gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
+    "print value of gdb.lookup_static_symbols ('rr')\[1\], from the other file"
 
 # Now continue back to the first source file.
 set linenum [gdb_get_line_number "Break at end."]
@@ -119,6 +132,10 @@ gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 # static symbol from the second source file.
 gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
     "print value of rr from main file"
+gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
+    "print value of gdb.lookup_static_symbols ('rr')\[0\], from the main file"
+gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
+    "print value of gdb.lookup_static_symbols ('rr')\[1\], from the main file"
 
 # Test is_variable attribute.
 gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHv2] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol

Sourceware - gdb-patches mailing list
In reply to this post by Andrew Burgess
On Tue, Oct 15, 2019 at 4:15 PM Andrew Burgess
<[hidden email]> wrote:

>
> * Christian Biesinger <[hidden email]> [2019-10-08 11:00:56 -0500]:
>
> > On Tue, Oct 8, 2019 at 6:07 AM Andrew Burgess
> > <[hidden email]> wrote:
> > >
> > > Changes since v1:
> > >
> > >   - Minor tweak to docs to address Eli's feedback,
> > >   - Rebase to current HEAD,
> > >   - Fixed a small bug in the test script that caused some failures in
> > >     C++ part of the test - no idea how I missed this originally.
> > >
> > > Thanks,
> > > Andrew
> > >
> > > ---
> > >
> > > When using gdb.lookup_static_symbol I think that GDB should find
> > > static symbols (global symbol with static linkage) from the current
> > > object file ahead of static symbols from other object files.
> >
> > So I am not sure how I feel about this. Since this is a programmatic
> > API, I'm not sure it should be affected by the current program status
> > like this. Maybe this should be optional behavior, or maybe it should
> > be possible to pass in a block? (At the same time, it is already
> > possible to find static variables through the block, if you have one)
>
> Given that gdb.lookup_static_symbol returns a single symbol, I'm
> struggling to see how having it return something other than the one
> GDB would "naturally" see (if the user took control and just typed 'p
> some_variable') would ever be useful.
>
> I think we should either change the API to return the same thing the
> CLI would see, or have the API return a list of symbols.  Simply
> picking the "first" seems pretty arbitrary, and is (I would guess)
> going to be wrong more often than not.
>
> I don't really understand why this being a "programmatic API" makes
> any difference - the CLI is programmable (though admittedly it's much
> inferior to python scripting) - they both serve the same purpose,
> allow the user to send commands to GDB and see the program state.  The
> question I think is more, does it make sense to have the symbol return
> change based on the current program state.   Given the role of GDB I'd
> say yes.
>
> How would you feel if I changed the patch to have
> 'gdb.lookup_static_symbol' return the one symbol that is the current
> value, and 'gdb.lookup_all_static_symbols' return a list of all
> matching static symbols?

Fair enough -- you convinced me. I like this plan.

Christian

> > > This means that if we have two source files f1.c and f2.c, and both
> > > files contains 'static int foo;', then when we are stopped in f1.c a
> > > call to 'gdb.lookup_static_symbol ("foo")' will find f1.c::foo, and if
> > > we are stopped in f2.c we would find 'f2.c::foo'.
> > >
> > > Given that gdb.lookup_static_symbol always returns a single symbol,
> > > but there can be multiple static symbols with the same name GDB is
> > > always making a choice about which symbols to return.  I think that it
> > > makes sense for the choice GDB makes in this case to match what a user
> > > would get on the command line if they asked to 'print foo'.
> > >
> > > gdb/testsuite/ChangeLog:
> > >
> > >         * gdb.python/py-symbol.c: Declare and call function from new
> > >         py-symbol-2.c file.
> > >         * gdb.python/py-symbol.exp: Compile both source files, and add new
> > >         tests for gdb.lookup_static_symbol.
> > >         * gdb.python/py-symbol-2.c: New file.
> > >
> > > gdb/doc/ChangeLog:
> > >
> > >         * python.texi (Symbols In Python): Extend documentation for
> > >         gdb.lookup_static_symbol.
> > >
> > > gdb/ChangeLog:
> > >
> > >         * python/py-symbol.c (gdbpy_lookup_static_symbol): Lookup in
> > >         static block of current object file first.
> > > ---
> > >  gdb/ChangeLog                          |  5 +++++
> > >  gdb/doc/ChangeLog                      |  5 +++++
> > >  gdb/doc/python.texi                    |  9 +++++++++
> > >  gdb/python/py-symbol.c                 | 25 +++++++++++++++++++++++-
> > >  gdb/testsuite/ChangeLog                |  8 ++++++++
> > >  gdb/testsuite/gdb.python/py-symbol-2.c | 26 +++++++++++++++++++++++++
> > >  gdb/testsuite/gdb.python/py-symbol.c   |  9 +++++++++
> > >  gdb/testsuite/gdb.python/py-symbol.exp | 35 ++++++++++++++++++++++------------
> > >  8 files changed, 109 insertions(+), 13 deletions(-)
> > >  create mode 100644 gdb/testsuite/gdb.python/py-symbol-2.c
> > >
> > > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> > > index 48adad4e97b..0f12de94bba 100644
> > > --- a/gdb/doc/python.texi
> > > +++ b/gdb/doc/python.texi
> > > @@ -4869,6 +4869,15 @@
> > >  up such variables, iterate over the variables of the function's
> > >  @code{gdb.Block} and check that @code{block.addr_class} is
> > >  @code{gdb.SYMBOL_LOC_STATIC}.
> > > +
> > > +There can be multiple global symbols with static linkage with the same
> > > +name.  This function will only return the first matching symbol that
> > > +it finds.  Which symbol is found depends on where @value{GDBN} is
> > > +currently stopped, as @value{GDBN} will first search for matching
> > > +symbols in the current object file, and then search all other object
> > > +files.  If the application is not yet running then @value{GDBN} will
> > > +search all object files in the order they appear in the debug
> > > +information.
> > >  @end defun
> > >
> > >  A @code{gdb.Symbol} object has the following attributes:
> > > diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> > > index 2b10e21d872..ae9aca6c5d0 100644
> > > --- a/gdb/python/py-symbol.c
> > > +++ b/gdb/python/py-symbol.c
> > > @@ -487,9 +487,32 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
> > >                                         &domain))
> > >      return NULL;
> > >
> > > +  /* In order to find static symbols associated with the "current" object
> > > +     file ahead of those from other object files, we first need to see if
> > > +     we can acquire a current block.  If this fails however, then we still
> > > +     want to search all static symbols, so don't throw an exception just
> > > +     yet.  */
> > > +  const struct block *block = NULL;
> > >    try
> > >      {
> > > -      symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
> > > +      struct frame_info *selected_frame
> > > +       = get_selected_frame (_("No frame selected."));
> > > +      block = get_frame_block (selected_frame, NULL);
> > > +    }
> > > +  catch (const gdb_exception &except)
> > > +    {
> > > +      /* Nothing.  */
> > > +    }
> > > +
> > > +  try
> > > +    {
> > > +      if (block != nullptr)
> > > +       symbol
> > > +         = lookup_symbol_in_static_block (name, block,
> > > +                                          (domain_enum) domain).symbol;
> > > +
> > > +      if (symbol == nullptr)
> > > +       symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
> > >      }
> > >    catch (const gdb_exception &except)
> > >      {
> > > diff --git a/gdb/testsuite/gdb.python/py-symbol-2.c b/gdb/testsuite/gdb.python/py-symbol-2.c
> > > new file mode 100644
> > > index 00000000000..12872efe4b7
> > > --- /dev/null
> > > +++ b/gdb/testsuite/gdb.python/py-symbol-2.c
> > > @@ -0,0 +1,26 @@
> > > +/* This testcase is part of GDB, the GNU debugger.
> > > +
> > > +   Copyright 2010-2019 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/>.  */
> > > +
> > > +static int rr = 99;            /* line of other rr */
> > > +
> > > +void
> > > +function_in_other_file (void)
> > > +{
> > > +  /* Nothing.  */
> > > +}
> > > +
> > > +
> > > diff --git a/gdb/testsuite/gdb.python/py-symbol.c b/gdb/testsuite/gdb.python/py-symbol.c
> > > index 06a931bf5d9..51325dcd450 100644
> > > --- a/gdb/testsuite/gdb.python/py-symbol.c
> > > +++ b/gdb/testsuite/gdb.python/py-symbol.c
> > > @@ -38,6 +38,10 @@ namespace {
> > >  };
> > >  #endif
> > >
> > > +#ifdef USE_TWO_FILES
> > > +extern void function_in_other_file (void);
> > > +#endif
> > > +
> > >  int qq = 72;                   /* line of qq */
> > >  static int rr = 42;            /* line of rr */
> > >
> > > @@ -70,5 +74,10 @@ int main (int argc, char *argv[])
> > >    sclass.seti (42);
> > >    sclass.valueofi ();
> > >  #endif
> > > +
> > > +#ifdef USE_TWO_FILES
> > > +  function_in_other_file ();
> > > +#endif
> > > +
> > >    return 0; /* Break at end.  */
> > >  }
> > > diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> > > index 5617f127e5b..61960075565 100644
> > > --- a/gdb/testsuite/gdb.python/py-symbol.exp
> > > +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> > > @@ -18,9 +18,11 @@
> > >
> > >  load_lib gdb-python.exp
> > >
> > > -standard_testfile
> > > +standard_testfile py-symbol.c py-symbol-2.c
> > >
> > > -if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> > > +set opts { debug additional_flags=-DUSE_TWO_FILES }
> > > +if {[prepare_for_testing "failed to prepare" $testfile \
> > > +        [list $srcfile $srcfile2] $opts]} {
> > >      return -1
> > >  }
> > >
> > > @@ -48,6 +50,7 @@ gdb_test "python print (gdb.lookup_global_symbol('qq').needs_frame)" \
> > >      "False" \
> > >      "print whether qq needs a frame"
> > >
> > > +# Similarly, test looking up a static symbol before we runto_main.
> > >  set rr_line [gdb_get_line_number "line of rr"]
> > >  gdb_test "python print (gdb.lookup_global_symbol ('rr') is None)" "True" \
> > >      "lookup_global_symbol for static var"
> > > @@ -100,10 +103,23 @@ gdb_test "python print (func.print_name)" "func" "test func.print_name"
> > >  gdb_test "python print (func.linkage_name)" "func" "test func.linkage_name"
> > >  gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test func.addr_class"
> > >
> > > -gdb_breakpoint [gdb_get_line_number "Break at end."]
> > > +# Stop in a second file and ensure we find its local static symbol.
> > > +gdb_breakpoint "function_in_other_file"
> > > +gdb_continue_to_breakpoint "function_in_other_file"
> > > +gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
> > > +    "print value of rr from other file"
> > > +
> > > +# Now continue back to the first source file.
> > > +set linenum [gdb_get_line_number "Break at end."]
> > > +gdb_breakpoint "$srcfile:$linenum"
> > >  gdb_continue_to_breakpoint "Break at end for variable a" ".*Break at end.*"
> > >  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> > >
> > > +# Check that we find the static sybol local to this file over the
> > > +# static symbol from the second source file.
> > > +gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
> > > +    "print value of rr from main file"
> > > +
> > >  # Test is_variable attribute.
> > >  gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0
> > >  gdb_test "python print (a\[0\].is_variable)" "True" "test a.is_variable"
> > > @@ -145,17 +161,12 @@ gdb_test "python print (t\[0\].symtab)" "${py_symbol_c}" "get symtab"
> > >
> > >  # C++ tests
> > >  # Recompile binary.
> > > -if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}-cxx" executable "debug c++"] != "" } {
> > > -    untested "failed to compile in C++ mode"
> > > +lappend opts c++
> > > +if {[prepare_for_testing "failed to prepare" "${binfile}-cxx" \
> > > +        [list $srcfile $srcfile2] $opts]} {
> > >      return -1
> > >  }
> > >
> > > -# Start with a fresh gdb.
> > > -gdb_exit
> > > -gdb_start
> > > -gdb_reinitialize_dir $srcdir/$subdir
> > > -gdb_load ${binfile}-cxx
> > > -
> > >  gdb_test "python print (gdb.lookup_global_symbol ('(anonymous namespace)::anon') is None)" \
> > >      "True" "anon is None"
> > >  gdb_test "python print (gdb.lookup_static_symbol ('(anonymous namespace)::anon').value ())" \
> > > @@ -189,7 +200,7 @@ gdb_test "python print (cplusfunc.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "t
> > >  # Test is_valid when the objfile is unloaded.  This must be the last
> > >  # test as it unloads the object file in GDB.
> > >  # Start with a fresh gdb.
> > > -clean_restart ${testfile}
> > > +clean_restart ${binfile}
> > >  if ![runto_main] then {
> > >      fail "cannot run to main."
> > >      return 0
> > > --
> > > 2.14.5
> > >
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Sourceware - gdb-patches mailing list
In reply to this post by Simon Marchi-4
On Tue, Oct 15, 2019 at 2:28 PM Simon Marchi <[hidden email]> wrote:

>
> On 2019-10-15 12:46 p.m., Andrew Burgess wrote:
> > If gdb.lookup_static_symbol is going to return a single symbol then it
> > makes sense (I think) for it to return a context sensitive choice of
> > symbol, that is the static symbol that would be visible to the program
> > at that point.
>
> I remember discussing this with Christian, and I didn't have a strong option.  But
> now, I tend to agree with Andrew.
>
> I see two use cases here:
>
> 1. I want to get all static symbols named `foo`.  In which case, I'd use the
>    function Andrew proposes in this patch.
> 2. I want to get the static symbol named `foo` that's visible from a certain
>    point, perhaps a given block or where my program is currently stopped at.
>    Ideally, we would have a "CompilationUnit" object type in Python, such that
>    I could use
>
>     block.compunit.lookup_static_symbol('foo')
>
>   or
>
>     gdb.current_compunit().lookup_static_symbol('foo')

So technically, those don't give you "the static symbol named `foo`
that's visible from [this] point", because there could be static
variable in the function.

Since we already have block objects, should this new function
optionally take a block to start the lookup in?

Either way, I'm happy with this patch; I've come around to y'all's view.

Christian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Andrew Burgess
* Christian Biesinger <[hidden email]> [2019-10-21 17:36:37 -0500]:

> On Tue, Oct 15, 2019 at 2:28 PM Simon Marchi <[hidden email]> wrote:
> >
> > On 2019-10-15 12:46 p.m., Andrew Burgess wrote:
> > > If gdb.lookup_static_symbol is going to return a single symbol then it
> > > makes sense (I think) for it to return a context sensitive choice of
> > > symbol, that is the static symbol that would be visible to the program
> > > at that point.
> >
> > I remember discussing this with Christian, and I didn't have a strong option.  But
> > now, I tend to agree with Andrew.
> >
> > I see two use cases here:
> >
> > 1. I want to get all static symbols named `foo`.  In which case, I'd use the
> >    function Andrew proposes in this patch.
> > 2. I want to get the static symbol named `foo` that's visible from a certain
> >    point, perhaps a given block or where my program is currently stopped at.
> >    Ideally, we would have a "CompilationUnit" object type in Python, such that
> >    I could use
> >
> >     block.compunit.lookup_static_symbol('foo')
> >
> >   or
> >
> >     gdb.current_compunit().lookup_static_symbol('foo')
>
> So technically, those don't give you "the static symbol named `foo`
> that's visible from [this] point", because there could be static
> variable in the function.
>
> Since we already have block objects, should this new function
> optionally take a block to start the lookup in?

When you say "new function", I assume you mean
'gdb.lookup_static_symbol' not 'gdb.lookup_static_symbols', as the
second one always returns all symbols.

Anyway, I took a stab at adding a block parameter to the first of
these functions - is this what you were thinking?

Thanks,
Andrew

---

commit 62d488878b467dd83a9abffc6dc3c67c2da82e85
Author: Andrew Burgess <[hidden email]>
Date:   Wed Oct 23 15:08:42 2019 +0100

    gdb/python: New block parameter for gdb.lookup_static_symbol
   
    Adds a new block parameter to gdb.lookup_static_symbol.  Currently
    gdb.lookup_static_symbol first looks in the global static scope of the
    current block for a matching symbol, and if non is found it then looks
    in all other global static scopes.
   
    The new block parameter for gdb.lookup_static_symbol allows the user
    to search the global static scope for some other block rather than the
    current block if they so desire.
   
    If the block parameter is no given, or is given as None, then GDB will
    search using the current block first (this matches the CLI behaviour).
   
    I added the block parameter to the middle of the parameter list, so
    the parameter order is 'symbol block domain', this matches the order
    of the other symbol lookup function that takes a block 'lookup_symbol'
    which also has the order 'symbol block domain'.  I think changing the
    API like this is fine as the 'gdb.lookup_static_symbol' has not been
    in any previous GDB release, so there should be no existing user
    scripts that use this function.
   
    The documentation for gdb.lookup_static_symbol has been reordered so
    that the block parameter can be explained early on, and I've added
    some tests.
   
    gdb/ChangeLog:
   
            * python/py-symbol.c (gdbpy_lookup_static_symbol): Accept and
            handle a new block parameter.
   
    gdb/testsuite/ChangeLog:
   
            * gdb.python/py-symbol.exp: Add tests for passing a block
            parameter to gdb.lookup_static_symbol.
   
    gdb/doc/ChangeLog:
   
            * python.texi (Symbols In Python): Rewrite documentation for
            gdb.lookup_static_symbol, including adding a description of the
            new block parameter.
   
    Change-Id: I132a7f0934ba029c9f32058e9341a6c7cb975606

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index a30c8ae4f03..fe984a06b98 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4853,31 +4853,41 @@
 @end defun
 
 @findex gdb.lookup_static_symbol
-@defun gdb.lookup_static_symbol (name @r{[}, domain@r{]})
-This function searches for a global symbol with static linkage by name.
-The search scope can be restricted to by the domain argument.
+@defun gdb.lookup_static_symbol (name @r{[}, block @r{[}, domain@r{]]})
+This function searches for a global symbol with static linkage by
+name.
 
 @var{name} is the name of the symbol.  It must be a string.
-The optional @var{domain} argument restricts the search to the domain type.
-The @var{domain} argument must be a domain constant defined in the @code{gdb}
-module and described later in this chapter.
-
-The result is a @code{gdb.Symbol} object or @code{None} if the symbol
-is not found.
-
-Note that this function will not find function-scoped static variables. To look
-up such variables, iterate over the variables of the function's
-@code{gdb.Block} and check that @code{block.addr_class} is
-@code{gdb.SYMBOL_LOC_STATIC}.
 
 There can be multiple global symbols with static linkage with the same
 name.  This function will only return the first matching symbol that
 it finds.  Which symbol is found depends on where @value{GDBN} is
 currently stopped, as @value{GDBN} will first search for matching
-symbols in the current object file, and then search all other object
-files.  If the application is not yet running then @value{GDBN} will
-search all object files in the order they appear in the debug
-information.
+symbols in the global static scope of the current object file, and
+then search all other object files.  This corresponds to the behaviour
+you would see from the CLI if you tried to print the symbol by name.
+
+If the application is not yet running then @value{GDBN} will search
+all object files in the order they appear in the debug information.
+
+The optional @var{domain} argument restricts the search to the domain
+type.  The @var{domain} argument must be a domain constant defined in
+the @code{gdb} module and described later in this chapter.
+
+The optional @var{block} argument can be used to change which global
+static scope @value{GDBN} will search first.  If @var{block} is not
+given, or is passed the value @code{None} then @value{GDBN} will first
+search the global static scope corresponding to the current block.  If
+@var{block} is passed a @code{gdb.Block} then @value{GDBN} will first
+search the global static scope corresponding to the supplied block.
+
+The result is a @code{gdb.Symbol} object or @code{None} if the symbol
+is not found.
+
+Note that this function will not find function-scoped static
+variables. To look up such variables, iterate over the variables of
+the function's @code{gdb.Block} and check that @code{block.addr_class}
+is @code{gdb.SYMBOL_LOC_STATIC}.
 @end defun
 
 @findex gdb.lookup_global_symbol
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 647c54b0a5b..24b3f6f4376 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -473,19 +473,20 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
 }
 
 /* Implementation of
-   gdb.lookup_static_symbol (name [, domain]) -> symbol or None.  */
+   gdb.lookup_static_symbol (name [, block] [, domain]) -> symbol or None.  */
 
 PyObject *
 gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
 {
   const char *name;
   int domain = VAR_DOMAIN;
-  static const char *keywords[] = { "name", "domain", NULL };
+  static const char *keywords[] = { "name", "block", "domain", NULL };
   struct symbol *symbol = NULL;
+  PyObject *block_obj = NULL;
   PyObject *sym_obj;
 
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
- &domain))
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O|i", keywords, &name,
+ &block_obj, &domain))
     return NULL;
 
   /* In order to find static symbols associated with the "current" object
@@ -493,16 +494,22 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
      we can acquire a current block.  If this fails however, then we still
      want to search all static symbols, so don't throw an exception just
      yet.  */
-  const struct block *block = NULL;
-  try
-    {
-      struct frame_info *selected_frame
- = get_selected_frame (_("No frame selected."));
-      block = get_frame_block (selected_frame, NULL);
-    }
-  catch (const gdb_exception &except)
+  const struct block *block = nullptr;
+  if (block_obj)
+    block = block_object_to_block (block_obj);
+
+  if (block == nullptr)
     {
-      /* Nothing.  */
+      try
+ {
+  struct frame_info *selected_frame
+    = get_selected_frame (_("No frame selected."));
+  block = get_frame_block (selected_frame, NULL);
+ }
+      catch (const gdb_exception &except)
+ {
+  /* Nothing.  */
+ }
     }
 
   try
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index ea41297f54f..0ffe07df5a8 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -87,6 +87,10 @@ if ![runto_main] then {
 
 global hex decimal
 
+# Grab the value of $pc, we'll use this later.
+set pc_val_file_1 [get_integer_valueof "\$pc" 0 \
+       "get value of \$pc in main file"]
+
 gdb_breakpoint [gdb_get_line_number "Block break here."]
 gdb_continue_to_breakpoint "Block break here."
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
@@ -116,7 +120,18 @@ gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test f
 gdb_breakpoint "function_in_other_file"
 gdb_continue_to_breakpoint "function_in_other_file"
 gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
-    "print value of rr from other file"
+    "print value of rr from other file, don't pass a block value"
+gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "99" \
+    "print value of rr from other file, pass block value of None"
+
+# Test gdb.lookup_static_symbol passing a block parameter
+set pc_val_file_2 [get_integer_valueof "\$pc" 0 \
+       "get \$pc in other file"]
+gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
+    "print value of rr from other file, pass block for other file"
+gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
+    "print value of rr from other file, pass block for main file"
+
 gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
     "print value of gdb.lookup_static_symbols ('rr')\[0\], from the other file"
 gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
@@ -131,7 +146,14 @@ gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 # Check that we find the static sybol local to this file over the
 # static symbol from the second source file.
 gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
-    "print value of rr from main file"
+    "print value of rr from main file, passing no block parameter"
+gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "42" \
+    "print value of rr from main file, passing None block parameter"
+gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
+    "print value of rr from main file, pass block for other file"
+gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
+    "print value of rr from main file, pass block for main file"
+
 gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
     "print value of gdb.lookup_static_symbols ('rr')\[0\], from the main file"
 gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Andrew Burgess
In reply to this post by Eli Zaretskii
* Eli Zaretskii <[hidden email]> [2019-10-15 20:07:41 +0300]:

> > From: Andrew Burgess <[hidden email]>
> > Cc: Christian Biesinger <[hidden email]>, Andrew Burgess <[hidden email]>
> > Date: Tue, 15 Oct 2019 17:46:47 +0100
> >
> > gdb/ChangeLog:
> >
> > * python/py-symbol.c (gdbpy_lookup_all_static_symbols): New
> > function.
> > * python/python-internal.h (gdbpy_lookup_all_static_symbols):
> > Declare new function.
> > * python/python.c (python_GdbMethods): Add
> > gdb.lookup_all_static_symbols method.
> >
> > gdb/testsuite/ChangeLog:
> >
> > * gdb.python/py-symbol.exp: Add test for
> > gdb.lookup_all_static_symbols.
> >
> > gdb/doc/ChangeLog:
> >
> > * python.texi (Symbols In Python): Add documentation for
> > gdb.lookup_all_static_symbols.
>
> The python.texi part is OK, but I think this change also needs a NEWS
> entry.

Thanks for the review, I'll include the NEWS entry below when I merge
this patch.

Thanks,
Andrew

--

diff --git a/gdb/NEWS b/gdb/NEWS
index 25e67e43c88..465f96aed18 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -65,6 +65,9 @@
   ** The new function gdb.lookup_static_symbol can be used to look up
      symbols with static linkage.
 
+  ** The new function gdb.lookup_static_symbols can be used to look up
+     all static symbols with static linkage.
+
   ** gdb.Objfile has new methods 'lookup_global_symbol' and
      'lookup_static_symbol' to lookup a symbol from this objfile only.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Eli Zaretskii
> Date: Wed, 23 Oct 2019 20:15:28 +0100
> From: Andrew Burgess <[hidden email]>
> Cc: [hidden email], [hidden email]
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 25e67e43c88..465f96aed18 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -65,6 +65,9 @@
>    ** The new function gdb.lookup_static_symbol can be used to look up
>       symbols with static linkage.
>  
> +  ** The new function gdb.lookup_static_symbols can be used to look up
> +     all static symbols with static linkage.
> +

OK.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Simon Marchi-4
In reply to this post by Andrew Burgess
On 2019-10-23 3:13 p.m., Andrew Burgess wrote:

>>> I see two use cases here:
>>>
>>> 1. I want to get all static symbols named `foo`.  In which case, I'd use the
>>>    function Andrew proposes in this patch.
>>> 2. I want to get the static symbol named `foo` that's visible from a certain
>>>    point, perhaps a given block or where my program is currently stopped at.
>>>    Ideally, we would have a "CompilationUnit" object type in Python, such that
>>>    I could use
>>>
>>>     block.compunit.lookup_static_symbol('foo')
>>>
>>>   or
>>>
>>>     gdb.current_compunit().lookup_static_symbol('foo')
>> So technically, those don't give you "the static symbol named `foo`
>> that's visible from [this] point", because there could be static
>> variable in the function.
>>
>> Since we already have block objects, should this new function
>> optionally take a block to start the lookup in?
> When you say "new function", I assume you mean
> 'gdb.lookup_static_symbol' not 'gdb.lookup_static_symbols', as the
> second one always returns all symbols.
>
> Anyway, I took a stab at adding a block parameter to the first of
> these functions - is this what you were thinking?
>
> Thanks,
> Andrew

I'm confused, I don't gdb.lookup_static_symbol(s) only look through
static symbols, as in file-level static variables?

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Andrew Burgess
* Simon Marchi <[hidden email]> [2019-10-23 23:11:30 -0400]:

> On 2019-10-23 3:13 p.m., Andrew Burgess wrote:
> >>> I see two use cases here:
> >>>
> >>> 1. I want to get all static symbols named `foo`.  In which case, I'd use the
> >>>    function Andrew proposes in this patch.
> >>> 2. I want to get the static symbol named `foo` that's visible from a certain
> >>>    point, perhaps a given block or where my program is currently stopped at.
> >>>    Ideally, we would have a "CompilationUnit" object type in Python, such that
> >>>    I could use
> >>>
> >>>     block.compunit.lookup_static_symbol('foo')
> >>>
> >>>   or
> >>>
> >>>     gdb.current_compunit().lookup_static_symbol('foo')
> >> So technically, those don't give you "the static symbol named `foo`
> >> that's visible from [this] point", because there could be static
> >> variable in the function.
> >>
> >> Since we already have block objects, should this new function
> >> optionally take a block to start the lookup in?
> > When you say "new function", I assume you mean
> > 'gdb.lookup_static_symbol' not 'gdb.lookup_static_symbols', as the
> > second one always returns all symbols.
> >
> > Anyway, I took a stab at adding a block parameter to the first of
> > these functions - is this what you were thinking?
> >
> > Thanks,
> > Andrew
>
> I'm confused, I don't gdb.lookup_static_symbol(s) only look through
> static symbols, as in file-level static variables?

So, before patch 3 the situation was:

  gdb.lookup_static_symbol

  Return the file level static symbol matching a given name.  Will
  return the static from the current file first, before searching
  through all other files.  This matches the CLI behaviour for if you
  just typed: 'print var_name' (with var_name being file-level
  static).

  gdb.lookup_static_symbols

  Returns all of the file level static symbols matching a given name,
  the order corresponds to the order of the compilation units as found
  by GDB (I guess).  In general a user shouldn't read any meaning into
  the order, just that these are all file level statics with that
  name.

The last patch in the series modifies the first of these to take an
optional block parameter.  GDB will then find the file level static
that would be seen from that block.

This allows the user to ask the question, what file-level static would
I see from over there, without actually having to be "over there".

This is demonstrated in the testsuite changes where I lookup a block
based on a $pc from some other function in a different file, I can
then query the file-level static that would be visible from that
function.

Is this super useful? I don't know, I think this is what Christian was
suggesting and adding the feature was both easy to implement and easy
to test, so I figured why not.

Thanks,
Andrew
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Sourceware - gdb-patches mailing list
In reply to this post by Andrew Burgess
On Wed, Oct 23, 2019 at 2:13 PM Andrew Burgess
<[hidden email]> wrote:

>
> * Christian Biesinger <[hidden email]> [2019-10-21 17:36:37 -0500]:
>
> > On Tue, Oct 15, 2019 at 2:28 PM Simon Marchi <[hidden email]> wrote:
> > >
> > > On 2019-10-15 12:46 p.m., Andrew Burgess wrote:
> > > > If gdb.lookup_static_symbol is going to return a single symbol then it
> > > > makes sense (I think) for it to return a context sensitive choice of
> > > > symbol, that is the static symbol that would be visible to the program
> > > > at that point.
> > >
> > > I remember discussing this with Christian, and I didn't have a strong option.  But
> > > now, I tend to agree with Andrew.
> > >
> > > I see two use cases here:
> > >
> > > 1. I want to get all static symbols named `foo`.  In which case, I'd use the
> > >    function Andrew proposes in this patch.
> > > 2. I want to get the static symbol named `foo` that's visible from a certain
> > >    point, perhaps a given block or where my program is currently stopped at.
> > >    Ideally, we would have a "CompilationUnit" object type in Python, such that
> > >    I could use
> > >
> > >     block.compunit.lookup_static_symbol('foo')
> > >
> > >   or
> > >
> > >     gdb.current_compunit().lookup_static_symbol('foo')
> >
> > So technically, those don't give you "the static symbol named `foo`
> > that's visible from [this] point", because there could be static
> > variable in the function.
> >
> > Since we already have block objects, should this new function
> > optionally take a block to start the lookup in?
>
> When you say "new function", I assume you mean
> 'gdb.lookup_static_symbol' not 'gdb.lookup_static_symbols', as the
> second one always returns all symbols.
>
> Anyway, I took a stab at adding a block parameter to the first of
> these functions - is this what you were thinking?

I'm sorry, I think I was a bit confused when I wrote that. Here's what
I was thinking about -- with your change to make lookup_static_symbol
produce the same result as "print var", it will also find block-level
statics. However, lookup_static_symbols only looks at file-level
static variables, which means that lookup_static_symbol can find a
variable that's not in lookup_static_symbols, which seems confusing to
users. My suggestion to give it a block to start in didn't really make
sense, I think, but it would be good to think about that a bit more?

Christian

>
> Thanks,
> Andrew
>
> ---
>
> commit 62d488878b467dd83a9abffc6dc3c67c2da82e85
> Author: Andrew Burgess <[hidden email]>
> Date:   Wed Oct 23 15:08:42 2019 +0100
>
>     gdb/python: New block parameter for gdb.lookup_static_symbol
>
>     Adds a new block parameter to gdb.lookup_static_symbol.  Currently
>     gdb.lookup_static_symbol first looks in the global static scope of the
>     current block for a matching symbol, and if non is found it then looks
>     in all other global static scopes.
>
>     The new block parameter for gdb.lookup_static_symbol allows the user
>     to search the global static scope for some other block rather than the
>     current block if they so desire.
>
>     If the block parameter is no given, or is given as None, then GDB will
>     search using the current block first (this matches the CLI behaviour).
>
>     I added the block parameter to the middle of the parameter list, so
>     the parameter order is 'symbol block domain', this matches the order
>     of the other symbol lookup function that takes a block 'lookup_symbol'
>     which also has the order 'symbol block domain'.  I think changing the
>     API like this is fine as the 'gdb.lookup_static_symbol' has not been
>     in any previous GDB release, so there should be no existing user
>     scripts that use this function.
>
>     The documentation for gdb.lookup_static_symbol has been reordered so
>     that the block parameter can be explained early on, and I've added
>     some tests.
>
>     gdb/ChangeLog:
>
>             * python/py-symbol.c (gdbpy_lookup_static_symbol): Accept and
>             handle a new block parameter.
>
>     gdb/testsuite/ChangeLog:
>
>             * gdb.python/py-symbol.exp: Add tests for passing a block
>             parameter to gdb.lookup_static_symbol.
>
>     gdb/doc/ChangeLog:
>
>             * python.texi (Symbols In Python): Rewrite documentation for
>             gdb.lookup_static_symbol, including adding a description of the
>             new block parameter.
>
>     Change-Id: I132a7f0934ba029c9f32058e9341a6c7cb975606
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index a30c8ae4f03..fe984a06b98 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4853,31 +4853,41 @@
>  @end defun
>
>  @findex gdb.lookup_static_symbol
> -@defun gdb.lookup_static_symbol (name @r{[}, domain@r{]})
> -This function searches for a global symbol with static linkage by name.
> -The search scope can be restricted to by the domain argument.
> +@defun gdb.lookup_static_symbol (name @r{[}, block @r{[}, domain@r{]]})
> +This function searches for a global symbol with static linkage by
> +name.
>
>  @var{name} is the name of the symbol.  It must be a string.
> -The optional @var{domain} argument restricts the search to the domain type.
> -The @var{domain} argument must be a domain constant defined in the @code{gdb}
> -module and described later in this chapter.
> -
> -The result is a @code{gdb.Symbol} object or @code{None} if the symbol
> -is not found.
> -
> -Note that this function will not find function-scoped static variables. To look
> -up such variables, iterate over the variables of the function's
> -@code{gdb.Block} and check that @code{block.addr_class} is
> -@code{gdb.SYMBOL_LOC_STATIC}.
>
>  There can be multiple global symbols with static linkage with the same
>  name.  This function will only return the first matching symbol that
>  it finds.  Which symbol is found depends on where @value{GDBN} is
>  currently stopped, as @value{GDBN} will first search for matching
> -symbols in the current object file, and then search all other object
> -files.  If the application is not yet running then @value{GDBN} will
> -search all object files in the order they appear in the debug
> -information.
> +symbols in the global static scope of the current object file, and
> +then search all other object files.  This corresponds to the behaviour
> +you would see from the CLI if you tried to print the symbol by name.
> +
> +If the application is not yet running then @value{GDBN} will search
> +all object files in the order they appear in the debug information.
> +
> +The optional @var{domain} argument restricts the search to the domain
> +type.  The @var{domain} argument must be a domain constant defined in
> +the @code{gdb} module and described later in this chapter.
> +
> +The optional @var{block} argument can be used to change which global
> +static scope @value{GDBN} will search first.  If @var{block} is not
> +given, or is passed the value @code{None} then @value{GDBN} will first
> +search the global static scope corresponding to the current block.  If
> +@var{block} is passed a @code{gdb.Block} then @value{GDBN} will first
> +search the global static scope corresponding to the supplied block.
> +
> +The result is a @code{gdb.Symbol} object or @code{None} if the symbol
> +is not found.
> +
> +Note that this function will not find function-scoped static
> +variables. To look up such variables, iterate over the variables of
> +the function's @code{gdb.Block} and check that @code{block.addr_class}
> +is @code{gdb.SYMBOL_LOC_STATIC}.
>  @end defun
>
>  @findex gdb.lookup_global_symbol
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 647c54b0a5b..24b3f6f4376 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -473,19 +473,20 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
>  }
>
>  /* Implementation of
> -   gdb.lookup_static_symbol (name [, domain]) -> symbol or None.  */
> +   gdb.lookup_static_symbol (name [, block] [, domain]) -> symbol or None.  */
>
>  PyObject *
>  gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
>  {
>    const char *name;
>    int domain = VAR_DOMAIN;
> -  static const char *keywords[] = { "name", "domain", NULL };
> +  static const char *keywords[] = { "name", "block", "domain", NULL };
>    struct symbol *symbol = NULL;
> +  PyObject *block_obj = NULL;
>    PyObject *sym_obj;
>
> -  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
> -                                       &domain))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O|i", keywords, &name,
> +                                       &block_obj, &domain))
>      return NULL;
>
>    /* In order to find static symbols associated with the "current" object
> @@ -493,16 +494,22 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
>       we can acquire a current block.  If this fails however, then we still
>       want to search all static symbols, so don't throw an exception just
>       yet.  */
> -  const struct block *block = NULL;
> -  try
> -    {
> -      struct frame_info *selected_frame
> -       = get_selected_frame (_("No frame selected."));
> -      block = get_frame_block (selected_frame, NULL);
> -    }
> -  catch (const gdb_exception &except)
> +  const struct block *block = nullptr;
> +  if (block_obj)
> +    block = block_object_to_block (block_obj);
> +
> +  if (block == nullptr)
>      {
> -      /* Nothing.  */
> +      try
> +       {
> +         struct frame_info *selected_frame
> +           = get_selected_frame (_("No frame selected."));
> +         block = get_frame_block (selected_frame, NULL);
> +       }
> +      catch (const gdb_exception &except)
> +       {
> +         /* Nothing.  */
> +       }
>      }
>
>    try
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index ea41297f54f..0ffe07df5a8 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -87,6 +87,10 @@ if ![runto_main] then {
>
>  global hex decimal
>
> +# Grab the value of $pc, we'll use this later.
> +set pc_val_file_1 [get_integer_valueof "\$pc" 0 \
> +                      "get value of \$pc in main file"]
> +
>  gdb_breakpoint [gdb_get_line_number "Block break here."]
>  gdb_continue_to_breakpoint "Block break here."
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> @@ -116,7 +120,18 @@ gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test f
>  gdb_breakpoint "function_in_other_file"
>  gdb_continue_to_breakpoint "function_in_other_file"
>  gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
> -    "print value of rr from other file"
> +    "print value of rr from other file, don't pass a block value"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "99" \
> +    "print value of rr from other file, pass block value of None"
> +
> +# Test gdb.lookup_static_symbol passing a block parameter
> +set pc_val_file_2 [get_integer_valueof "\$pc" 0 \
> +                      "get \$pc in other file"]
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
> +    "print value of rr from other file, pass block for other file"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
> +    "print value of rr from other file, pass block for main file"
> +
>  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
>      "print value of gdb.lookup_static_symbols ('rr')\[0\], from the other file"
>  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
> @@ -131,7 +146,14 @@ gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
>  # Check that we find the static sybol local to this file over the
>  # static symbol from the second source file.
>  gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
> -    "print value of rr from main file"
> +    "print value of rr from main file, passing no block parameter"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "42" \
> +    "print value of rr from main file, passing None block parameter"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
> +    "print value of rr from main file, pass block for other file"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
> +    "print value of rr from main file, pass block for main file"
> +
>  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
>      "print value of gdb.lookup_static_symbols ('rr')\[0\], from the main file"
>  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Andrew Burgess
* Christian Biesinger <[hidden email]> [2019-10-28 00:07:17 -0500]:

> On Wed, Oct 23, 2019 at 2:13 PM Andrew Burgess
> <[hidden email]> wrote:
> >
> > * Christian Biesinger <[hidden email]> [2019-10-21 17:36:37 -0500]:
> >
> > > On Tue, Oct 15, 2019 at 2:28 PM Simon Marchi <[hidden email]> wrote:
> > > >
> > > > On 2019-10-15 12:46 p.m., Andrew Burgess wrote:
> > > > > If gdb.lookup_static_symbol is going to return a single symbol then it
> > > > > makes sense (I think) for it to return a context sensitive choice of
> > > > > symbol, that is the static symbol that would be visible to the program
> > > > > at that point.
> > > >
> > > > I remember discussing this with Christian, and I didn't have a strong option.  But
> > > > now, I tend to agree with Andrew.
> > > >
> > > > I see two use cases here:
> > > >
> > > > 1. I want to get all static symbols named `foo`.  In which case, I'd use the
> > > >    function Andrew proposes in this patch.
> > > > 2. I want to get the static symbol named `foo` that's visible from a certain
> > > >    point, perhaps a given block or where my program is currently stopped at.
> > > >    Ideally, we would have a "CompilationUnit" object type in Python, such that
> > > >    I could use
> > > >
> > > >     block.compunit.lookup_static_symbol('foo')
> > > >
> > > >   or
> > > >
> > > >     gdb.current_compunit().lookup_static_symbol('foo')
> > >
> > > So technically, those don't give you "the static symbol named `foo`
> > > that's visible from [this] point", because there could be static
> > > variable in the function.
> > >
> > > Since we already have block objects, should this new function
> > > optionally take a block to start the lookup in?
> >
> > When you say "new function", I assume you mean
> > 'gdb.lookup_static_symbol' not 'gdb.lookup_static_symbols', as the
> > second one always returns all symbols.
> >
> > Anyway, I took a stab at adding a block parameter to the first of
> > these functions - is this what you were thinking?
>
> I'm sorry, I think I was a bit confused when I wrote that. Here's what
> I was thinking about -- with your change to make lookup_static_symbol
> produce the same result as "print var", it will also find block-level
> statics. However, lookup_static_symbols only looks at file-level
> static variables, which means that lookup_static_symbol can find a
> variable that's not in lookup_static_symbols, which seems confusing to
> users. My suggestion to give it a block to start in didn't really make
> sense, I think, but it would be good to think about that a bit more?

This is not correct, both lookup_static_symbol AND
lookup_static_symbols BOTH only lookup file level static globals.

[ SIDE-ISSUE : The name for both of these is obviously a possible
  sauce of confusion, maybe before this function makes it into a
  release we should consider renaming it/them?  How about
  lookup_static_global(s) ? ]

Here's a demo session using the above 3 patches, the test file is:

  static int global_v1 = 1;
  static int global_v2 = 2;

  int
  use_them ()
  {
    return global_v1 + global_v2;
  }

  int
  main ()
  {
    static int global_v2 = 3;
    static int global_v3 = 4;

    return use_them () + global_v2 + global_v3;
  }

And my GDB session:

  (gdb) start
  Temporary breakpoint 1 at 0x40049a: file test.c, line 16.
  Starting program: /projects/gdb/tmp/static-syms/test
 
  Temporary breakpoint 1, main () at test.c:16
  16  return use_them () + global_v2 + global_v3;
  (gdb) python print (gdb.lookup_static_symbol ('global_v1').value ())
  1
  (gdb) python print (gdb.lookup_static_symbol ('global_v2').value ())
  2
  (gdb) python print (gdb.lookup_static_symbol ('global_v3').value ())
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
  AttributeError: 'NoneType' object has no attribute 'value'
  Error while executing Python code.
  (gdb)

Notice that despite being inside main, we see the file level
'global_v2' and completely miss the 'global_v3'.  This is inline with
the original behaviour of this function before I started messing with
it.

One source of confusion may have been that I added a call to
lookup_symbol_in_static_block, this doesn't find a static symbol in a
block, or the first static symbol between block and the global scope,
but instead finds the static scope for a block and looks for a
matching symbol in that block.

The other source of confusion was my talking about 'print
symbol_name'.  You are correct that in the above test if I did 'print
global_v2' I would see the static within main, so in that sense what
I've implemented is _not_ like 'print symbol_name'.  The only
comparison I meant to draw with 'print symbol_name' was that if I do
try to access 'global_v1' while in main I know I'll get the global_v1
from _this_ file, not a global_v1 from some other file.

With the new 3rd patch a user can now from Python say, if I was in
some other block, which static global would I see, which I think for a
scripting interface is helpful.  Then with the second patch the user
can also find all static globals.  However, anything you can find with
lookup_static_symbol will _always_ be in the list returned by
lookup_static_symbols.

Hope this clears things up,

Thanks,
Andrew





>
> Christian
>
> >
> > Thanks,
> > Andrew
> >
> > ---
> >
> > commit 62d488878b467dd83a9abffc6dc3c67c2da82e85
> > Author: Andrew Burgess <[hidden email]>
> > Date:   Wed Oct 23 15:08:42 2019 +0100
> >
> >     gdb/python: New block parameter for gdb.lookup_static_symbol
> >
> >     Adds a new block parameter to gdb.lookup_static_symbol.  Currently
> >     gdb.lookup_static_symbol first looks in the global static scope of the
> >     current block for a matching symbol, and if non is found it then looks
> >     in all other global static scopes.
> >
> >     The new block parameter for gdb.lookup_static_symbol allows the user
> >     to search the global static scope for some other block rather than the
> >     current block if they so desire.
> >
> >     If the block parameter is no given, or is given as None, then GDB will
> >     search using the current block first (this matches the CLI behaviour).
> >
> >     I added the block parameter to the middle of the parameter list, so
> >     the parameter order is 'symbol block domain', this matches the order
> >     of the other symbol lookup function that takes a block 'lookup_symbol'
> >     which also has the order 'symbol block domain'.  I think changing the
> >     API like this is fine as the 'gdb.lookup_static_symbol' has not been
> >     in any previous GDB release, so there should be no existing user
> >     scripts that use this function.
> >
> >     The documentation for gdb.lookup_static_symbol has been reordered so
> >     that the block parameter can be explained early on, and I've added
> >     some tests.
> >
> >     gdb/ChangeLog:
> >
> >             * python/py-symbol.c (gdbpy_lookup_static_symbol): Accept and
> >             handle a new block parameter.
> >
> >     gdb/testsuite/ChangeLog:
> >
> >             * gdb.python/py-symbol.exp: Add tests for passing a block
> >             parameter to gdb.lookup_static_symbol.
> >
> >     gdb/doc/ChangeLog:
> >
> >             * python.texi (Symbols In Python): Rewrite documentation for
> >             gdb.lookup_static_symbol, including adding a description of the
> >             new block parameter.
> >
> >     Change-Id: I132a7f0934ba029c9f32058e9341a6c7cb975606
> >
> > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> > index a30c8ae4f03..fe984a06b98 100644
> > --- a/gdb/doc/python.texi
> > +++ b/gdb/doc/python.texi
> > @@ -4853,31 +4853,41 @@
> >  @end defun
> >
> >  @findex gdb.lookup_static_symbol
> > -@defun gdb.lookup_static_symbol (name @r{[}, domain@r{]})
> > -This function searches for a global symbol with static linkage by name.
> > -The search scope can be restricted to by the domain argument.
> > +@defun gdb.lookup_static_symbol (name @r{[}, block @r{[}, domain@r{]]})
> > +This function searches for a global symbol with static linkage by
> > +name.
> >
> >  @var{name} is the name of the symbol.  It must be a string.
> > -The optional @var{domain} argument restricts the search to the domain type.
> > -The @var{domain} argument must be a domain constant defined in the @code{gdb}
> > -module and described later in this chapter.
> > -
> > -The result is a @code{gdb.Symbol} object or @code{None} if the symbol
> > -is not found.
> > -
> > -Note that this function will not find function-scoped static variables. To look
> > -up such variables, iterate over the variables of the function's
> > -@code{gdb.Block} and check that @code{block.addr_class} is
> > -@code{gdb.SYMBOL_LOC_STATIC}.
> >
> >  There can be multiple global symbols with static linkage with the same
> >  name.  This function will only return the first matching symbol that
> >  it finds.  Which symbol is found depends on where @value{GDBN} is
> >  currently stopped, as @value{GDBN} will first search for matching
> > -symbols in the current object file, and then search all other object
> > -files.  If the application is not yet running then @value{GDBN} will
> > -search all object files in the order they appear in the debug
> > -information.
> > +symbols in the global static scope of the current object file, and
> > +then search all other object files.  This corresponds to the behaviour
> > +you would see from the CLI if you tried to print the symbol by name.
> > +
> > +If the application is not yet running then @value{GDBN} will search
> > +all object files in the order they appear in the debug information.
> > +
> > +The optional @var{domain} argument restricts the search to the domain
> > +type.  The @var{domain} argument must be a domain constant defined in
> > +the @code{gdb} module and described later in this chapter.
> > +
> > +The optional @var{block} argument can be used to change which global
> > +static scope @value{GDBN} will search first.  If @var{block} is not
> > +given, or is passed the value @code{None} then @value{GDBN} will first
> > +search the global static scope corresponding to the current block.  If
> > +@var{block} is passed a @code{gdb.Block} then @value{GDBN} will first
> > +search the global static scope corresponding to the supplied block.
> > +
> > +The result is a @code{gdb.Symbol} object or @code{None} if the symbol
> > +is not found.
> > +
> > +Note that this function will not find function-scoped static
> > +variables. To look up such variables, iterate over the variables of
> > +the function's @code{gdb.Block} and check that @code{block.addr_class}
> > +is @code{gdb.SYMBOL_LOC_STATIC}.
> >  @end defun
> >
> >  @findex gdb.lookup_global_symbol
> > diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> > index 647c54b0a5b..24b3f6f4376 100644
> > --- a/gdb/python/py-symbol.c
> > +++ b/gdb/python/py-symbol.c
> > @@ -473,19 +473,20 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
> >  }
> >
> >  /* Implementation of
> > -   gdb.lookup_static_symbol (name [, domain]) -> symbol or None.  */
> > +   gdb.lookup_static_symbol (name [, block] [, domain]) -> symbol or None.  */
> >
> >  PyObject *
> >  gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
> >  {
> >    const char *name;
> >    int domain = VAR_DOMAIN;
> > -  static const char *keywords[] = { "name", "domain", NULL };
> > +  static const char *keywords[] = { "name", "block", "domain", NULL };
> >    struct symbol *symbol = NULL;
> > +  PyObject *block_obj = NULL;
> >    PyObject *sym_obj;
> >
> > -  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
> > -                                       &domain))
> > +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O|i", keywords, &name,
> > +                                       &block_obj, &domain))
> >      return NULL;
> >
> >    /* In order to find static symbols associated with the "current" object
> > @@ -493,16 +494,22 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
> >       we can acquire a current block.  If this fails however, then we still
> >       want to search all static symbols, so don't throw an exception just
> >       yet.  */
> > -  const struct block *block = NULL;
> > -  try
> > -    {
> > -      struct frame_info *selected_frame
> > -       = get_selected_frame (_("No frame selected."));
> > -      block = get_frame_block (selected_frame, NULL);
> > -    }
> > -  catch (const gdb_exception &except)
> > +  const struct block *block = nullptr;
> > +  if (block_obj)
> > +    block = block_object_to_block (block_obj);
> > +
> > +  if (block == nullptr)
> >      {
> > -      /* Nothing.  */
> > +      try
> > +       {
> > +         struct frame_info *selected_frame
> > +           = get_selected_frame (_("No frame selected."));
> > +         block = get_frame_block (selected_frame, NULL);
> > +       }
> > +      catch (const gdb_exception &except)
> > +       {
> > +         /* Nothing.  */
> > +       }
> >      }
> >
> >    try
> > diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> > index ea41297f54f..0ffe07df5a8 100644
> > --- a/gdb/testsuite/gdb.python/py-symbol.exp
> > +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> > @@ -87,6 +87,10 @@ if ![runto_main] then {
> >
> >  global hex decimal
> >
> > +# Grab the value of $pc, we'll use this later.
> > +set pc_val_file_1 [get_integer_valueof "\$pc" 0 \
> > +                      "get value of \$pc in main file"]
> > +
> >  gdb_breakpoint [gdb_get_line_number "Block break here."]
> >  gdb_continue_to_breakpoint "Block break here."
> >  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> > @@ -116,7 +120,18 @@ gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test f
> >  gdb_breakpoint "function_in_other_file"
> >  gdb_continue_to_breakpoint "function_in_other_file"
> >  gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
> > -    "print value of rr from other file"
> > +    "print value of rr from other file, don't pass a block value"
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "99" \
> > +    "print value of rr from other file, pass block value of None"
> > +
> > +# Test gdb.lookup_static_symbol passing a block parameter
> > +set pc_val_file_2 [get_integer_valueof "\$pc" 0 \
> > +                      "get \$pc in other file"]
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
> > +    "print value of rr from other file, pass block for other file"
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
> > +    "print value of rr from other file, pass block for main file"
> > +
> >  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
> >      "print value of gdb.lookup_static_symbols ('rr')\[0\], from the other file"
> >  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
> > @@ -131,7 +146,14 @@ gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> >  # Check that we find the static sybol local to this file over the
> >  # static symbol from the second source file.
> >  gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
> > -    "print value of rr from main file"
> > +    "print value of rr from main file, passing no block parameter"
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "42" \
> > +    "print value of rr from main file, passing None block parameter"
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
> > +    "print value of rr from main file, pass block for other file"
> > +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
> > +    "print value of rr from main file, pass block for main file"
> > +
> >  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
> >      "print value of gdb.lookup_static_symbols ('rr')\[0\], from the main file"
> >  gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Simon Marchi-4
On 2019-11-01 7:53 a.m., Andrew Burgess wrote:

>> I'm sorry, I think I was a bit confused when I wrote that. Here's what
>> I was thinking about -- with your change to make lookup_static_symbol
>> produce the same result as "print var", it will also find block-level
>> statics. However, lookup_static_symbols only looks at file-level
>> static variables, which means that lookup_static_symbol can find a
>> variable that's not in lookup_static_symbols, which seems confusing to
>> users. My suggestion to give it a block to start in didn't really make
>> sense, I think, but it would be good to think about that a bit more?
>
> This is not correct, both lookup_static_symbol AND
> lookup_static_symbols BOTH only lookup file level static globals.
>
> [ SIDE-ISSUE : The name for both of these is obviously a possible
>   sauce of confusion, maybe before this function makes it into a
>   release we should consider renaming it/them?  How about
>   lookup_static_global(s) ? ]

I would at least make sure the naming is consistent with gdb.lookup_global_symbol,
so have the _symbol(s) suffix.  So, lookup_static_global_symbol(s).

I don't think that lookup_static_symbols is particularly confusing.  As a user, I
don't think I would expect it to return static symbols from local scopes.

I think what makes lookup_static_symbol ambiguous is that we're considering making
it take a block as parameter.  Then, I agree, a user could think that it will find
static symbols from local scopes.

But AFAIK, we're considering passing a block to lookup_static_symbol only because
of a current shortcoming in the API, that we don't have an object type
representing a compilation unit.  If lookup_static_symbol accepted a compilation
unit object instead, then I don't think there would be a similar confusion
(especially that it is singular, and returns a single symbol).

Would it be an option to not add lookup_static_symbol for now, and instead work
on having a type representing compilation units, and then think about introducing
lookup_static_symbol again?  Christian, can you achieve what you wanted using
lookup_static_symbols plus some filtering instead?

> Here's a demo session using the above 3 patches, the test file is:
>
>   static int global_v1 = 1;
>   static int global_v2 = 2;
>
>   int
>   use_them ()
>   {
>     return global_v1 + global_v2;
>   }
>
>   int
>   main ()
>   {
>     static int global_v2 = 3;
>     static int global_v3 = 4;
>
>     return use_them () + global_v2 + global_v3;
>   }
>
> And my GDB session:
>
>   (gdb) start
>   Temporary breakpoint 1 at 0x40049a: file test.c, line 16.
>   Starting program: /projects/gdb/tmp/static-syms/test
>  
>   Temporary breakpoint 1, main () at test.c:16
>   16  return use_them () + global_v2 + global_v3;
>   (gdb) python print (gdb.lookup_static_symbol ('global_v1').value ())
>   1
>   (gdb) python print (gdb.lookup_static_symbol ('global_v2').value ())
>   2
>   (gdb) python print (gdb.lookup_static_symbol ('global_v3').value ())
>   Traceback (most recent call last):
>     File "<string>", line 1, in <module>
>   AttributeError: 'NoneType' object has no attribute 'value'
>   Error while executing Python code.
>   (gdb)
>
> Notice that despite being inside main, we see the file level
> 'global_v2' and completely miss the 'global_v3'.  This is inline with
> the original behaviour of this function before I started messing with
> it.
>
> One source of confusion may have been that I added a call to
> lookup_symbol_in_static_block, this doesn't find a static symbol in a
> block, or the first static symbol between block and the global scope,
> but instead finds the static scope for a block and looks for a
> matching symbol in that block.
>
> The other source of confusion was my talking about 'print
> symbol_name'.  You are correct that in the above test if I did 'print
> global_v2' I would see the static within main, so in that sense what
> I've implemented is _not_ like 'print symbol_name'.  The only
> comparison I meant to draw with 'print symbol_name' was that if I do
> try to access 'global_v1' while in main I know I'll get the global_v1
> from _this_ file, not a global_v1 from some other file.
>
> With the new 3rd patch a user can now from Python say, if I was in
> some other block, which static global would I see, which I think for a
> scripting interface is helpful.  Then with the second patch the user
> can also find all static globals.  However, anything you can find with
> lookup_static_symbol will _always_ be in the list returned by
> lookup_static_symbols.

Ok, that's the behavior I expected (to not find static local symbols), thanks
for clearing that up.

Simon
12