[7/9] Rewrite lookup_static_symbol to use gdbarch routine
Commit Message
Doug Evans <xdje42@gmail.com> writes:
> 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.
Hi.
I was looking at the callers of lookup_static_symbol,
and I think there is more cleanup possible here,
but I'd rather not take that on at the moment.
So I'm going with a simplified version of my previous patch.
Regression tested on amd64-linux.
2014-11-07 Doug Evans <xdje42@gmail.com>
* symtab.c (lookup_symbol_in_all_objfiles): Delete.
(lookup_static_symbol): Move definition to new location and rewrite.
(lookup_symbol_in_objfile): New function.
(lookup_symbol_global_iterator_cb): Call it.
@@ -80,11 +80,6 @@ struct symbol *lookup_local_symbol (const char *name,
enum language language);
static
-struct symbol *lookup_symbol_in_all_objfiles (int block_index,
- const char *name,
- const domain_enum domain);
-
-static
struct symbol *lookup_symbol_via_quick_fns (struct objfile *objfile,
int block_index,
const char *name,
@@ -1474,28 +1469,6 @@ lookup_symbol_aux (const char *name, const struct block *block,
return lookup_static_symbol (name, domain);
}
-/* See symtab.h. */
-
-struct symbol *
-lookup_static_symbol (const char *name, const domain_enum domain)
-{
- struct objfile *objfile;
- struct symbol *sym;
-
- sym = lookup_symbol_in_all_objfiles (STATIC_BLOCK, name, domain);
- if (sym != NULL)
- return sym;
-
- ALL_OBJFILES (objfile)
- {
- sym = lookup_symbol_via_quick_fns (objfile, STATIC_BLOCK, name, domain);
- if (sym != NULL)
- return sym;
- }
-
- return NULL;
-}
-
/* Check to see if the symbol is defined in BLOCK or its superiors.
Don't search STATIC_BLOCK or GLOBAL_BLOCK. */
@@ -1650,27 +1623,6 @@ lookup_symbol_in_objfile_symtabs (struct objfile *objfile, int block_index,
return NULL;
}
-/* Wrapper around lookup_symbol_in_objfile_symtabs to search all objfiles.
- Returns the first match found. */
-
-static struct symbol *
-lookup_symbol_in_all_objfiles (int block_index, const char *name,
- const domain_enum domain)
-{
- struct symbol *sym;
- struct objfile *objfile;
-
- ALL_OBJFILES (objfile)
- {
- sym = lookup_symbol_in_objfile_symtabs (objfile, block_index, name,
- domain);
- if (sym)
- return sym;
- }
-
- return NULL;
-}
-
/* Wrapper around lookup_symbol_in_objfile_symtabs for search_symbols.
Look up LINKAGE_NAME in DOMAIN in the global and static blocks of OBJFILE
and all related objfiles. */
@@ -1774,7 +1726,7 @@ basic_lookup_symbol_nonlocal (const char *name,
not it would be appropriate to search the current global block
here as well. (That's what this code used to do before the
is_a_field_of_this check was moved up.) On the one hand, it's
- redundant with the lookup_symbol_in_all_objfiles search that happens
+ redundant with the lookup in all objfiles search that happens
next. On the other hand, if decode_line_1 is passed an argument
like filename:var, then the user presumably wants 'var' to be
searched for in filename. On the third hand, there shouldn't be
@@ -1818,6 +1770,46 @@ lookup_symbol_in_static_block (const char *name,
return NULL;
}
+/* Perform the standard symbol lookup of NAME in OBJFILE:
+ 1) First search expanded symtabs, and if not found
+ 2) Search the "quick" symtabs (partial or .gdb_index).
+ BLOCK_INDEX is one of GLOBAL_BLOCK or STATIC_BLOCK. */
+
+static struct symbol *
+lookup_symbol_in_objfile (struct objfile *objfile, int block_index,
+ const char *name, const domain_enum domain)
+{
+ struct symbol *result;
+
+ result = lookup_symbol_in_objfile_symtabs (objfile, block_index,
+ name, domain);
+ if (result == NULL)
+ {
+ result = lookup_symbol_via_quick_fns (objfile, block_index,
+ name, domain);
+ }
+
+ return result;
+}
+
+/* See symtab.h. */
+
+struct symbol *
+lookup_static_symbol (const char *name, const domain_enum domain)
+{
+ struct objfile *objfile;
+ struct symbol *result;
+
+ ALL_OBJFILES (objfile)
+ {
+ result = lookup_symbol_in_objfile (objfile, STATIC_BLOCK, name, domain);
+ if (result != NULL)
+ return result;
+ }
+
+ return NULL;
+}
+
/* Private data to be used with lookup_symbol_global_iterator_cb. */
struct global_sym_lookup_data
@@ -1847,11 +1839,8 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
gdb_assert (data->result == NULL);
- data->result = lookup_symbol_in_objfile_symtabs (objfile, GLOBAL_BLOCK,
- data->name, data->domain);
- if (data->result == NULL)
- data->result = lookup_symbol_via_quick_fns (objfile, GLOBAL_BLOCK,
- data->name, data->domain);
+ data->result = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK,
+ data->name, data->domain);
/* If we found a match, tell the iterator to stop. Otherwise,
keep going. */