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

  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(-)
  

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
  

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