[PATCH 2/2] [PR symtab/17602] Fix arguments to symbol_name_cmp

[PATCH 2/2] [PR symtab/17602] Fix arguments to symbol_name_cmp

[Doug Evans]

Hi.

This patch fixes pr 17602, the arguments to symbol_name_cmp are
reversed, as explained in the comment in this patch.

Regression tested on amd64-linux.

2014-11-25  Doug Evans  <dje@google.com>

	PR symtab/17602
	* linespec.c (iterate_name_matcher): Fix arguments to symbol_name_cmp.

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 5325702..35b0205 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -982,7 +982,12 @@ iterate_name_matcher (const char *name, void *d)
 {
   const struct symbol_matcher_data *data = d;
 
-  if (data->symbol_name_cmp (name, data->lookup_name) == 0)
+  /* The order of arguments we pass to symbol_name_cmp is important as
+     strcmp_iw, a typical value for symbol_name_cmp, only performs special
+     processing of '(' to remove overload info on the first argument and not
+     the second.  The first argument is what the user provided, the second
+     argument is what came from partial syms / .gdb_index.  */
+  if (data->symbol_name_cmp (data->lookup_name, name) == 0)
     return 1; /* Expand this symbol's symbol table.  */
   return 0; /* Skip this symbol.  */
 }

Re: [PATCH 2/2] [PR symtab/17602] Fix arguments to symbol_name_cmp

[Yao Qi]

Doug Evans <dje@google.com> writes:

> -  if (data->symbol_name_cmp (name, data->lookup_name) == 0)
> +  /* The order of arguments we pass to symbol_name_cmp is important as
> +     strcmp_iw, a typical value for symbol_name_cmp, only performs special
> +     processing of '(' to remove overload info on the first argument and not
> +     the second.  The first argument is what the user provided, the second
> +     argument is what came from partial syms / .gdb_index.  */
> +  if (data->symbol_name_cmp (data->lookup_name, name) == 0)
>      return 1; /* Expand this symbol's symbol table.  */
>    return 0; /* Skip this symbol.  */

Such odd feature is documented in the comments to strcmp_iw:

   As an extra hack, string1=="FOO(ARGS)" matches string2=="FOO".
   This "feature" is useful when searching for matching C++ function names
   (such as if the user types 'break FOO', where FOO is a mangled C++
   function).

A question not related to this patch too much, do you consider to move
such c++ specific hack into c++ specific routine la_get_symbol_name_cmp?

-- 
Yao (齐尧)

Re: [PATCH 2/2] [PR symtab/17602] Fix arguments to symbol_name_cmp

[Andreas Arnez]

On Wed, Nov 26 2014, Doug Evans wrote:

> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 5325702..35b0205 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -982,7 +982,12 @@ iterate_name_matcher (const char *name, void *d)
>  {
>    const struct symbol_matcher_data *data = d;
>
> -  if (data->symbol_name_cmp (name, data->lookup_name) == 0)
> +  /* The order of arguments we pass to symbol_name_cmp is important as
> +     strcmp_iw, a typical value for symbol_name_cmp, only performs special
> +     processing of '(' to remove overload info on the first argument and not
> +     the second.  The first argument is what the user provided, the second
> +     argument is what came from partial syms / .gdb_index.  */
> +  if (data->symbol_name_cmp (data->lookup_name, name) == 0)
>      return 1; /* Expand this symbol's symbol table.  */
>    return 0; /* Skip this symbol.  */
>  }

This seems to cause a regression for the Ada testcase "operator_bp.exp":

> [...]
> FAIL: gdb.ada/operator_bp.exp: break "+" (got interactive prompt)
> FAIL: gdb.ada/operator_bp.exp: break "-" (got interactive prompt)
> FAIL: gdb.ada/operator_bp.exp: break "*" (got interactive prompt)
> FAIL: gdb.ada/operator_bp.exp: break "/" (got interactive prompt)
> FAIL: gdb.ada/operator_bp.exp: break "mod" (got interactive prompt)
> FAIL: gdb.ada/operator_bp.exp: break "rem" (got interactive prompt)
> FAIL: gdb.ada/operator_bp.exp: break "**" (got interactive prompt)
> [...]

See https://sourceware.org/ml/gdb-testers/2014-q4/msg00126.html

The problem occurs like this:

  (gdb) break "+"
  Function ""+"" not defined.
  Make breakpoint pending on future shared library load? (y or [n]) n
  (gdb) FAIL: gdb.ada/operator_bp.exp: break "+" (got interactive prompt) 

When reverting the patch, the test succeeds again.

Re: [PATCH 2/2] [PR symtab/17602] Fix arguments to symbol_name_cmp

[Doug Evans]

On Thu, Dec 4, 2014 at 3:48 AM, Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:
> On Wed, Nov 26 2014, Doug Evans wrote:
>
>> diff --git a/gdb/linespec.c b/gdb/linespec.c
>> index 5325702..35b0205 100644
>> --- a/gdb/linespec.c
>> +++ b/gdb/linespec.c
>> @@ -982,7 +982,12 @@ iterate_name_matcher (const char *name, void *d)
>>  {
>>    const struct symbol_matcher_data *data = d;
>>
>> -  if (data->symbol_name_cmp (name, data->lookup_name) == 0)
>> +  /* The order of arguments we pass to symbol_name_cmp is important as
>> +     strcmp_iw, a typical value for symbol_name_cmp, only performs special
>> +     processing of '(' to remove overload info on the first argument and not
>> +     the second.  The first argument is what the user provided, the second
>> +     argument is what came from partial syms / .gdb_index.  */
>> +  if (data->symbol_name_cmp (data->lookup_name, name) == 0)
>>      return 1; /* Expand this symbol's symbol table.  */
>>    return 0; /* Skip this symbol.  */
>>  }
>
> This seems to cause a regression for the Ada testcase "operator_bp.exp":
>
>> [...]
>> FAIL: gdb.ada/operator_bp.exp: break "+" (got interactive prompt)
>> FAIL: gdb.ada/operator_bp.exp: break "-" (got interactive prompt)
>> FAIL: gdb.ada/operator_bp.exp: break "*" (got interactive prompt)
>> FAIL: gdb.ada/operator_bp.exp: break "/" (got interactive prompt)
>> FAIL: gdb.ada/operator_bp.exp: break "mod" (got interactive prompt)
>> FAIL: gdb.ada/operator_bp.exp: break "rem" (got interactive prompt)
>> FAIL: gdb.ada/operator_bp.exp: break "**" (got interactive prompt)
>> [...]
>
> See https://sourceware.org/ml/gdb-testers/2014-q4/msg00126.html
>
> The problem occurs like this:
>
>   (gdb) break "+"
>   Function ""+"" not defined.
>   Make breakpoint pending on future shared library load? (y or [n]) n
>   (gdb) FAIL: gdb.ada/operator_bp.exp: break "+" (got interactive prompt)
>
> When reverting the patch, the test succeeds again.

Yeah, found that last night.

The problem is ada-lang.c:wild_match takes arguments in the opposite
order of strcmp_iw.
Working on a patch.

Re: [PATCH 2/2] [PR symtab/17602] Fix arguments to symbol_name_cmp

[Doug Evans]

On Wed, Nov 26, 2014 at 1:00 AM, Yao Qi <yao@codesourcery.com> wrote:
> A question not related to this patch too much, do you consider to move
> such c++ specific hack into c++ specific routine la_get_symbol_name_cmp?

I hadn't planned on it, at least not soon.