Discussion:
[RFAv2 0/2] Fix regression 'info variables' does not show minimal symbols.
Philippe Waroquiers
2018-11-10 15:00:46 UTC
Permalink
This patch series fixes the regression introduced by

12615cba8411c8 Add [-q] [-t TYPEREGEXP] [NAMEREGEXP] args to info [args|functions|locals|variables]

and adds a test case.


Compared to the first version, this fixes the remarks
given by Simon:
* Fix indentation in gdb/symtab.c.
* Add license header in info_minsym.c.
* Tight down the regexp in info_minsym.exp.

Some more eyes on the fix are expected before pushing.

Thanks
Philippe Waroquiers
2018-11-10 15:00:47 UTC
Permalink
12615cba8411c8 Add [-q] [-t TYPEREGEXP] [NAMEREGEXP] args to info [args|functions|locals|variables]
introduced a regression that minimal symbols were not listed anymore, due to a wrong
condition checking the absence of a type regexp in the loop scanning the minimal symbols.

Instead, before entering the loop scanning the minimal symbols, check that we
do not have a type regexp, as we will never match a minimal symbol with
this type regexp.

With the fix in this patch, for this part of the code, we basically go back
to the GDB 8.2 logic, with just the addition of
&& !treg.has_value ())
to 'enter' in the minsym case.
This should ensure that at least there is no regression compared to 8.2,
when not using the new type matching argument, as there was no treg in 8.2.

2018-11-10 Philippe Waroquiers <***@skynet.be>

* symtab.c (search_symbols): Properly check absence of type regexp
before entering the loop scanning the minimal symbols.
---
gdb/symtab.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 2e9e6325ad..7a77bcfb18 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4544,9 +4544,12 @@ search_symbols (const char *regexp, enum search_domain kind,
sort_search_symbols_remove_dups (&result);

/* If there are no eyes, avoid all contact. I mean, if there are
- no debug symbols, then add matching minsyms. */
+ no debug symbols, then add matching minsyms. But if the user wants
+ to see symbols matching a type regexp, then never give a minimal symbol,
+ as we assume that a minimal symbol does not have a type. */

- if (found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
+ if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
+ && !treg.has_value ())
{
ALL_MSYMBOLS (objfile, msymbol)
{
@@ -4560,13 +4563,9 @@ search_symbols (const char *regexp, enum search_domain kind,
|| MSYMBOL_TYPE (msymbol) == ourtype3
|| MSYMBOL_TYPE (msymbol) == ourtype4)
{
- /* If the user wants to see var matching a type regexp,
- then never give a minimal symbol. */
- if (kind != VARIABLES_DOMAIN
- && !treg.has_value () /* minimal symbol has never a type ???? */
- && (!preg.has_value ()
- || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0,
- NULL, 0) == 0))
+ if (!preg.has_value ()
+ || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0,
+ NULL, 0) == 0)
{
/* For functions we can do a quick check of whether the
symbol might be found via find_pc_symtab. */
--
2.19.1
Philippe Waroquiers
2018-11-10 15:00:48 UTC
Permalink
2018-11-10 Philippe Waroquiers <***@skynet.be>

* gdb.base/info_minsym.c: New file.
* gdb.base/info_minsym.exp: New file.
---
gdb/testsuite/gdb.base/info_minsym.c | 29 +++++++++++++++++++++
gdb/testsuite/gdb.base/info_minsym.exp | 36 ++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/info_minsym.c
create mode 100644 gdb/testsuite/gdb.base/info_minsym.exp

diff --git a/gdb/testsuite/gdb.base/info_minsym.c b/gdb/testsuite/gdb.base/info_minsym.c
new file mode 100644
index 0000000000..3e28fe1b78
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info_minsym.c
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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 minsym_var;
+
+static int minsym_fun (void)
+{
+ minsym_var++;
+}
+
+int
+main (void)
+{
+ return minsym_fun ();
+}
diff --git a/gdb/testsuite/gdb.base/info_minsym.exp b/gdb/testsuite/gdb.base/info_minsym.exp
new file mode 100644
index 0000000000..be4e33132f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info_minsym.exp
@@ -0,0 +1,36 @@
+# Copyright (C) 2018 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/>.
+
+
+# Verify 'info variables|functions'
+# shows minimal symbols when no type matching is requested
+# does not show minimal symbols when type matching is requested.
+
+set testfile info_minsym
+
+standard_testfile info_minsym.c
+
+# Compile the program without debugging information, to have minimal symbols.
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {c}]} {
+ return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test_no_output "info variables -q -t int minsym" "minsym variables do not match type"
+gdb_test_no_output "info functions -q -t int minsym" "minsym functions do not match type"
+
+gdb_test "info variables -q minsym" "$hex minsym_var" "minsym variables found"
+gdb_test "info functions -q minsym" "$hex minsym_fun" "minsym functions found"
--
2.19.1
Tom Tromey
2018-11-19 22:11:19 UTC
Permalink
Philippe> 2018-11-10 Philippe Waroquiers <***@skynet.be>
Philippe> * gdb.base/info_minsym.c: New file.
Philippe> * gdb.base/info_minsym.exp: New file.

...

Philippe> +gdb_test_no_output "info variables -q -t int minsym" "minsym variables do not match type"
Philippe> +gdb_test_no_output "info functions -q -t int minsym" "minsym functions do not match type"

I think these lines are too long and should be split.
Otherwise the patch is ok.

Tom
Philippe Waroquiers
2018-11-20 21:19:41 UTC
Permalink
Post by Tom Tromey
Philippe> * gdb.base/info_minsym.c: New file.
Philippe> * gdb.base/info_minsym.exp: New file.
...
Philippe> +gdb_test_no_output "info variables -q -t int minsym" "minsym variables do not match type"
Philippe> +gdb_test_no_output "info functions -q -t int minsym" "minsym functions do not match type"
I think these lines are too long and should be split.
Otherwise the patch is ok.
Thanks, pushed after having split the linesĀ 
(and re-run the tests on debian/x86_64).

Philippe

Tom Tromey
2018-11-19 22:11:28 UTC
Permalink
Philippe> This patch series fixes the regression introduced by
Philippe> 12615cba8411c8 Add [-q] [-t TYPEREGEXP] [NAMEREGEXP] args to info [args|functions|locals|variables]

Philippe> and adds a test case.


Philippe> Compared to the first version, this fixes the remarks
Philippe> given by Simon:
Philippe> * Fix indentation in gdb/symtab.c.
Philippe> * Add license header in info_minsym.c.
Philippe> * Tight down the regexp in info_minsym.exp.

Philippe> Some more eyes on the fix are expected before pushing.

This looks good to me.

Tom
Loading...