diff mbox

[7/9] Rewrite lookup_static_symbol to use gdbarch routine

Message ID m3a9439y60.fsf@sspiff.org
State New
Headers show

Commit Message

Doug Evans Nov. 7, 2014, 9:07 a.m. UTC
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.
diff mbox

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 2aae04c..ec1c033 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -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.  */