[7/9] Rewrite lookup_static_symbol to use gdbarch routine

Hi.

This patch rewrites lookup_static_symbol to use
gdbarch_iterate_over_objfiles_in_search_order.

This function is currently implemented like this:
[This is before the function is renamed by this patch series.]

struct symbol *
lookup_static_symbol_aux (const char *name, const domain_enum domain)
{
  struct objfile *objfile;
  struct symbol *sym;

  sym = lookup_symbol_aux_symtabs (STATIC_BLOCK, name, domain);
  if (sym != NULL)
    return sym;

  ALL_OBJFILES (objfile)
  {
    sym = lookup_symbol_aux_quick (objfile, STATIC_BLOCK, name, domain);
    if (sym != NULL)
      return sym;
  }

  return NULL;
}

Note what we're doing here.
First we're searching over all expanded symtabs in all objfiles,
and then we search partial/gdb_index tables in all objfiles.  Eh?

Normally when looking up a symbol in a particular objfile
we first list in expanded symtabs and then look in partial/gdb_index
tables.  And *then* we try the next objfile.

I can't think of any justification for the current behaviour.

This patch changes things to be consistent,
but it is a behavioural change.
The testsuite passes, and any noticeable difference
by this change would be dependent on the order in which
symtabs got expanded.  Thus I can't think of a reason to not
apply this change.

Contrary to lookup_global_symbol where at least for linux one might
not care what order objfiles are searched, for static symbols there
is more reason to care.  However, the current API doesn't provide
for this (we don't pass in BLOCK of OBJFILE as an argument) because
this routine is used as a last resort anyways.

2014-10-26  Doug Evans  <xdje42@gmail.com>

	* symtab.c (lookup_static_symbol): Rewrite to use
	gdbarch_iterate_over_objfiles_in_search_order.
	(iterated_sym_lookup_data): Renamed from global_sym_lookup_data.
	New member block_index.
	(lookup_symbol_iterator_cb): Renamed from
	lookup_symbol_global_iterator_cb.  Use block_index from
	iterated_sym_lookup_data.
	(lookup_symbol_global): Update.
	(lookup_symbol_aux_symtabs): Delete.

Doug Evans <xdje42@gmail.com> writes:

> struct symbol *
> lookup_static_symbol_aux (const char *name, const domain_enum domain)
> {
>   struct objfile *objfile;
>   struct symbol *sym;
>
>   sym = lookup_symbol_aux_symtabs (STATIC_BLOCK, name, domain);
>   if (sym != NULL)
>     return sym;
>
>   ALL_OBJFILES (objfile)
>   {
>     sym = lookup_symbol_aux_quick (objfile, STATIC_BLOCK, name, domain);
>     if (sym != NULL)
>       return sym;
>   }
>
>   return NULL;
> }
>
> Note what we're doing here.
> First we're searching over all expanded symtabs in all objfiles,
> and then we search partial/gdb_index tables in all objfiles.  Eh?
>
> Normally when looking up a symbol in a particular objfile
> we first list in expanded symtabs and then look in partial/gdb_index
> tables.  And *then* we try the next objfile.
>
> I can't think of any justification for the current behaviour.
>
> This patch changes things to be consistent,
> but it is a behavioural change.

Yes, it changes the behavior.  Here is an example in my mind, but not
sure it is correct or not, say, we have a static int foo defined in two
objfiles respectively (1.c and 2.c), foo is in the partial table of two
objfiles, but is only expanded to the full symtab of *one* objfile (2.c).

             1.c    2.c
  partial    foo    foo
  full              foo

before your patch, GDB gets foo from 2.c full table, and after it, GDB
gets foo from 1.c partial table.  Is it possible?

Regarding the change, we also need to update the comments to
iterate_over_objfiles_in_search_order in gdbarch.sh,

# Iterate over all objfiles in the order that makes the most sense
# for the architecture to make global symbol searches.
                               ^^^^^^^^^^^^^
> The testsuite passes, and any noticeable difference
> by this change would be dependent on the order in which
> symtabs got expanded.  Thus I can't think of a reason to not
> apply this change.

If read this
<https://www.sourceware.org/ml/gdb-patches/2012-05/msg00204.html>
correctly, Joel expressed the same intention there.

Since gdbarch method iterate_over_objfiles_in_search_order was added for
windows target and this patch uses it, we need to test it on windows target.
If you don't have mingw testing env, let me know, I'll see if I can do
the test.

Yao Qi <yao@codesourcery.com> writes:
> Doug Evans <xdje42@gmail.com> writes:
>
>> struct symbol *
>> lookup_static_symbol_aux (const char *name, const domain_enum domain)
>> {
>>   struct objfile *objfile;
>>   struct symbol *sym;
>>
>>   sym = lookup_symbol_aux_symtabs (STATIC_BLOCK, name, domain);
>>   if (sym != NULL)
>>     return sym;
>>
>>   ALL_OBJFILES (objfile)
>>   {
>>     sym = lookup_symbol_aux_quick (objfile, STATIC_BLOCK, name, domain);
>>     if (sym != NULL)
>>       return sym;
>>   }
>>
>>   return NULL;
>> }
>>
>> Note what we're doing here.
>> First we're searching over all expanded symtabs in all objfiles,
>> and then we search partial/gdb_index tables in all objfiles.  Eh?
>>
>> Normally when looking up a symbol in a particular objfile
>> we first list in expanded symtabs and then look in partial/gdb_index
>> tables.  And *then* we try the next objfile.
>>
>> I can't think of any justification for the current behaviour.
>>
>> This patch changes things to be consistent,
>> but it is a behavioural change.
>
> Yes, it changes the behavior.  Here is an example in my mind, but not
> sure it is correct or not, say, we have a static int foo defined in two
> objfiles respectively (1.c and 2.c), foo is in the partial table of two
> objfiles, but is only expanded to the full symtab of *one* objfile (2.c).
>
>              1.c    2.c
>   partial    foo    foo
>   full              foo

Also note that the context is searching across objfiles,
so 1.c and 2.c are also 1.so and 2.so.

> before your patch, GDB gets foo from 2.c full table, and after it, GDB
> gets foo from 1.c partial table.  Is it possible?

The question isn't whether it is possible,
the question is whether this is a behavioural change
on which we make some kind of guarantee.
For the task at hand, we should be making a guarantee
related to library search order before making
any kind of guarantee related to internal
implementation details (partial vs full syms).
[I'm not saying we can or should make search order
guarantees, per se.  Rather, such things should
at least be coherent.]
With this patch we now perform a proper full search
of libraries in a partcular order
(however that order is defined which is a separate discussion).
Whereas today we could find foo in the last library in the search order,
even if every library has foo, just because someone accessed
some other symbol in the last library and caused the
symtab with foo to be expanded.

> Regarding the change, we also need to update the comments to
> iterate_over_objfiles_in_search_order in gdbarch.sh,
>
> # Iterate over all objfiles in the order that makes the most sense
> # for the architecture to make global symbol searches.
>                                ^^^^^^^^^^^^^

Ah, righto.

>> The testsuite passes, and any noticeable difference
>> by this change would be dependent on the order in which
>> symtabs got expanded.  Thus I can't think of a reason to not
>> apply this change.
>
> If read this
> <https://www.sourceware.org/ml/gdb-patches/2012-05/msg00204.html>
> correctly, Joel expressed the same intention there.

Righto.

> Since gdbarch method iterate_over_objfiles_in_search_order was added for
> windows target and this patch uses it, we need to test it on windows target.
> If you don't have mingw testing env, let me know, I'll see if I can do
> the test.

I'm going to be making more symtab changes so I might as well
get mingw testing going here.
Testing in progress.