[COMMITTED] Revert fix to symtab/17602, bigger fix needed
Commit Message
Hi.
This patch caused a regression in gdb.ada/operator_bp.exp.
https://sourceware.org/ml/gdb-patches/2014-11/msg00642.html
There's still a bug here, but the deeper I dig the bigger the problem I find.
Wrong comments, confusing code, and on and on ...
[More than once I've gotten to the point of wondering "How does this
work at all?" :-(]
So I'm going to revert this patch for now and work on a better fix.
2014-12-05 Doug Evans <xdje42@gmail.com>
Revert:
PR symtab/17602
* linespec.c (iterate_name_matcher): Fix arguments to symbol_name_cmp.
Comments
On Fri, Dec 5, 2014 at 1:16 AM, Doug Evans <xdje42@gmail.com> wrote:
> Hi.
>
> This patch caused a regression in gdb.ada/operator_bp.exp.
>
> https://sourceware.org/ml/gdb-patches/2014-11/msg00642.html
>
> There's still a bug here, but the deeper I dig the bigger the problem I find.
> Wrong comments, confusing code, and on and on ...
> [More than once I've gotten to the point of wondering "How does this
> work at all?" :-(]
>
> So I'm going to revert this patch for now and work on a better fix.
>
> 2014-12-05 Doug Evans <xdje42@gmail.com>
>
> Revert:
> 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 af958dc..82384ca 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -982,12 +982,7 @@ iterate_name_matcher (const char *name, void *d)
> {
> const struct symbol_matcher_data *data = d;
>
> - /* 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)
> + if (data->symbol_name_cmp (name, data->lookup_name) == 0)
> return 1; /* Expand this symbol's symbol table. */
> return 0; /* Skip this symbol. */
> }
fyi,
I've added text to the PR
to show how to see the bug.
https://sourceware.org/bugzilla/show_bug.cgi?id=17602#c4
If anyone has thoughts on how to fix this (beyond a simple "quick fix")
I'm all ears. I've got a dozen symtab-related sandboxes in flight,
and I need to set this one aside for now.
ISTM a proper fix will involve a significant cleanup including
either getting rid of SYMBOL_SEARCH_NAME, and/or
establishing a lot more clarification on how symbol
comparisons work throughout gdb.
For starters, we have both symbol_compare_ftype
and symbol_name_cmp_ftype.
Let's get rid of one of them.
[But this needs to be more than just a simple
mechanical removal of symbol_name_cmp_type,
the one I think we should remove.
It also needs to involve establishing clarity
on what the arguments to symbol_compare_ftype
REALLY are, and are not.
And then auditing all of gdb to verify the code
adheres to those rules.]
On Fri, Dec 5, 2014 at 9:20 AM, Doug Evans <xdje42@gmail.com> wrote:
> For starters, we have both symbol_compare_ftype
> and symbol_name_cmp_ftype.
> Let's get rid of one of them.
[sorry for the followup]
FAOD, Or, if we do need two,
then let's at least add a lot more clarity
to distinguish the different use-cases.
@@ -982,12 +982,7 @@ iterate_name_matcher (const char *name, void *d)
{
const struct symbol_matcher_data *data = d;
- /* 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)
+ if (data->symbol_name_cmp (name, data->lookup_name) == 0)
return 1; /* Expand this symbol's symbol table. */
return 0; /* Skip this symbol. */
}