[RFA 2/2] Test case for 'info variables|functions' with minimal symbols.

[RFA 2/2] Test case for 'info variables|functions' with minimal symbols.

[Philippe Waroquiers]

2018-10-30  Philippe Waroquiers  <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   | 12 +++++++++
 gdb/testsuite/gdb.base/info_minsym.exp | 37 ++++++++++++++++++++++++++
 2 files changed, 49 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..e71ce7abf9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info_minsym.c
@@ -0,0 +1,12 @@
+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..8ec98b6071
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info_minsym.exp
@@ -0,0 +1,37 @@
+# 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" ".* minsym_var" "minsym variables found"
+gdb_test "info functions -q minsym" ".* minsym_fun" "minsym functions found"
+
-- 
2.19.1

[RFA 0/2] Fix regression 'info variables' does not show minimal symbols + test

[Philippe Waroquiers]

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.

Ok to apply ?

[RFA 1/2] Fix regression 'info variables' does not show minimal symbols.

[Philippe Waroquiers]

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.

2018-10-30  Philippe Waroquiers  <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 | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index cd27a75e8c..55e8296418 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 ()
+	    if (!preg.has_value ()
 		    || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0,
-				   NULL, 0) == 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

Re: [RFA 2/2] Test case for 'info variables|functions' with minimal symbols.

[Simon Marchi]

On 2018-10-30 5:23 p.m., Philippe Waroquiers wrote:
> 2018-10-30  Philippe Waroquiers  <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   | 12 +++++++++
>  gdb/testsuite/gdb.base/info_minsym.exp | 37 ++++++++++++++++++++++++++
>  2 files changed, 49 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..e71ce7abf9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/info_minsym.c
> @@ -0,0 +1,12 @@
> +static int minsym_var;
> +
> +static int minsym_fun (void)
> +{
> +   minsym_var++;
> +}
> +
> +int
> +main (void)
> +{
> +  return minsym_fun ();
> +}

Please add a license header to this file.

> diff --git a/gdb/testsuite/gdb.base/info_minsym.exp b/gdb/testsuite/gdb.base/info_minsym.exp
> new file mode 100644
> index 0000000000..8ec98b6071
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/info_minsym.exp
> @@ -0,0 +1,37 @@
> +# 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" ".* minsym_var" "minsym variables found"
> +gdb_test "info functions -q minsym" ".* minsym_fun" "minsym functions found"

Perhaps you could tighten down the regex a bit.  For example if the commands wrongfully print
more than one result, but the output ends with minsym_var/minsym_fun, it won't be caught.
This should work:

gdb_test "info variables -q minsym" "$hex  minsym_var" "minsym variables found"
gdb_test "info functions -q minsym" "$hex  minsym_fun" "minsym functions found"

Otherwise, the test LGTM.

Simon

Re: [RFA 1/2] Fix regression 'info variables' does not show minimal symbols.

[Simon Marchi]

On 2018-10-30 5:23 p.m., Philippe Waroquiers wrote:
> 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.

I don't feel qualified to approve, since I haven't followed the original story
and this code is rather tortuous.  I just have a small formatting nit.

> 
> 2018-10-30  Philippe Waroquiers  <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 | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index cd27a75e8c..55e8296418 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 ()
> +	    if (!preg.has_value ()
>  		    || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0,
> -				   NULL, 0) == 0))
> +				   NULL, 0) == 0)

The indentation of the "preg->exec" line should be decreased.

Simon

Re: [RFA 1/2] Fix regression 'info variables' does not show minimal symbols.

[Philippe Waroquiers]

On Fri, 2018-11-09 at 20:41 +0000, Simon Marchi wrote:
> I don't feel qualified to approve, since I haven't followed the original story
> and this code is rather tortuous.
Yes, the code there is not very trivial, and I have added the wrong condition:
         if (kind != VARIABLES_DOMAIN> 
            && !treg.has_value () /* minimal symbol has never a type ???? */
in the very first version of the patch, and I cannot make any sense of it
now.

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.

Pedro reviewed this patch series, so might shed some more lights on
this fix.


>   I just have a small formatting nit.
which I have fixed, waiting for Pedro's feedback.

Thanks for looking at the patch

Philippe