[RFA] massively speed up "info var foo" on large programs

[RFA] massively speed up "info var foo" on large programs

[Doug Evans]

Hi.

I'm not entirely sure this patch is correct, but it feels correct (*1),
and is a massive win.
"info var Task" in one large program goes from 350 seconds to 28 seconds.

The problem is the two loops over all minsyms of all objfiles.
The call to lookup_symbol looks up all those minsyms in all symtabs
of all objfiles.
Given an early test in the code of where the minsym lives
(e.g. for variables the code checks mst_data, mst_bss, mst_file_data,
mst_file_bss), it doesn't feel right to look in other objfiles.
It's possible there's a good reason though.  Alas I can't think of one
(I'm probably just blinded by the massive speedup ...).

This issue has been looked at before.
E.g., http://sourceware.org/ml/gdb-patches/2011-09/msg00461.html

(*1): There's a FIXME in the patch that I want to remove prior to check-in,
I'm just not sure what the right fix is.
For functions: why call both find_pc_symtab and lookup_symbol?
The patch leaves the code as is, so I could just change the FIXME
into a comment explaining why both calls are made.
I'd rather remove the call to find_pc_symtab (in the second minsym loop).
An alternative is to remove both find_pc_symtab calls and just
call lookup_msymbol_in_objfile for variables and functions,
I'm guessing the find_pc_symtab is used in the first minsym loop because
it's faster, but I wish that weren't just a guess.
[Gotta love comments that explain why things are they way they are. :-)]

One thing the patch does is change the test in the first minsym loop
to not call find_pc_symtab for variables.  I can put the test back,
but the call seems unnecessary for variables given that lookup_symbol
(or now lookup_msymbol_in_objfile) is also called.

The patch gives identical results for the cases I tried (with multi-1000 line
output), which raises my confidence that it's correct, but not entirely so.

Regression-tested on amd64-linux.

What's the right way to fix the FIXME?
And ok to check in after that's fixed?

2012-05-23  Doug Evans  <dje@google.com>

	* symtab.c (lookup_msymbol_in_objfile): New function.
	(search_symbols): Call it.

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.306
diff -u -p -r1.306 symtab.c
--- symtab.c	24 May 2012 02:51:48 -0000	1.306
+++ symtab.c	24 May 2012 04:19:31 -0000
@@ -1559,6 +1559,30 @@ lookup_symbol_aux_symtabs (int block_ind
   return NULL;
 }
 
+/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
+   Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE.  */
+
+static struct symbol *
+lookup_msymbol_in_objfile (struct objfile *objfile,
+			   struct minimal_symbol *msymbol,
+			   domain_enum domain)
+{
+  const char *name = SYMBOL_LINKAGE_NAME (msymbol);
+  enum language lang = current_language->la_language;
+  const char *modified_name;
+  struct cleanup *cleanup = demangle_for_lookup (name, lang, &modified_name);
+  struct symbol *returnval;
+
+  returnval = lookup_symbol_aux_objfile (objfile, GLOBAL_BLOCK,
+					 modified_name, domain);
+  if (returnval == NULL)
+    returnval = lookup_symbol_aux_objfile (objfile, STATIC_BLOCK,
+					   modified_name, domain);
+
+  do_cleanups (cleanup);
+  return returnval;
+}
+
 /* A helper function for lookup_symbol_aux that interfaces with the
    "quick" symbol table functions.  */
 
@@ -3463,21 +3487,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
-		  {
-		    /* FIXME: carlton/2003-02-04: Given that the
-		       semantics of lookup_symbol keeps on changing
-		       slightly, it would be a nice idea if we had a
-		       function lookup_symbol_minsym that found the
-		       symbol associated to a given minimal symbol (if
-		       any).  */
-		    if (kind == FUNCTIONS_DOMAIN
-			|| lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-					  (struct block *) NULL,
-					  VAR_DOMAIN, 0)
-			== NULL)
-		      found_misc = 1;
-		  }
+		/* Note: An important side-effect of these lookup functions
+		   is to expand the symbol table if msymbol is found.  */
+		if (kind == FUNCTIONS_DOMAIN
+		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
+		    : lookup_msymbol_in_objfile (objfile, msymbol,
+						 VAR_DOMAIN) == NULL)
+		  found_misc = 1;
 	      }
 	  }
       }
@@ -3570,13 +3586,13 @@ search_symbols (char *regexp, enum searc
 			    NULL, 0) == 0)
 	      {
 		/* Functions:  Look up by address.  */
-		if (kind != FUNCTIONS_DOMAIN ||
-		    (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))))
+		if (kind != FUNCTIONS_DOMAIN
+		    || find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL)
 		  {
 		    /* Variables/Absolutes:  Look up by name.  */
-		    if (lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-				       (struct block *) NULL, VAR_DOMAIN, 0)
-			 == NULL)
+		    /* FIXME: Why do we also look up fns by name?  */
+		    if (lookup_msymbol_in_objfile (objfile, msymbol,
+						   VAR_DOMAIN) == NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)

Re: [RFA] massively speed up "info var foo" on large programs

[Doug Evans]

On Thu, May 24, 2012 at 10:58 AM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> I'm not entirely sure this patch is correct, but it feels correct (*1),
> and is a massive win.
> "info var Task" in one large program goes from 350 seconds to 28 seconds.
>
> [...]
>
> 2012-05-23  Doug Evans  <dje@google.com>
>
>        * symtab.c (lookup_msymbol_in_objfile): New function.
>        (search_symbols): Call it.

Hmmm.
One thing that occurs to me is separate debug objfiles.
lookup_msymbol_in_objfile should probably search them.

Re: [RFA] massively speed up "info var foo" on large programs

[Matt Rice]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Thu, May 24, 2012 at 2:28 PM, Doug Evans <dje@google.com> wrote:
> On Thu, May 24, 2012 at 10:58 AM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> I'm not entirely sure this patch is correct, but it feels correct (*1),
>> and is a massive win.
>> "info var Task" in one large program goes from 350 seconds to 28 seconds.

FWIW i don't have anything with enough objfiles for the above to matter,
but here's something that can apply on top of your patch which
helps somewhat for objfiles with lots of symtabs, it still contains
the same worst case,
but helps in the 'info var' case with no arguments in e.g. ./gdb ./gdb
-batch -ex 'info var' case
where there is a lot of symtabs per objfile, and many symbols returned
by info var.

it could be faster if we had a way to know if a msymbol has no symbol
associated with it,
but this was as good as I could get without any major refactoring,
though maybe it is a little too ad-hoc.

anyhow if you don't mind having a look/testing it out.

it does come with a change of behaviour (IMO bugfix)
in that it compares the symbol/msymbol addresses,
this is for the case of ambiguous variable names, where previously it
would not output a msymbol if it found a symbol of the same name.

anyhow it speeds up the aforementioned ./gdb ./gdb -batch -ex 'info
var' case a bit.
though i doubt it will affect 'info var foo' much if at all.

thanks

[-- Attachment #2: foo.diff --]
[-- Type: application/octet-stream, Size: 5453 bytes --]

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 868bcf2..dd5e73f 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1559,26 +1559,132 @@ lookup_symbol_aux_symtabs (int block_index, const char *name,
   return NULL;
 }
 
+/* Helper for lookup_symbol_msymbol_blockvector */
+struct symbol *
+lookup_block_symbol_for_msymbol (const struct block *block,
+                                 struct minimal_symbol *msym,
+				 const char *name);
+struct symbol *
+lookup_block_symbol_for_msymbol (const struct block *block,
+                                 struct minimal_symbol *msym,
+				 const char *name)
+{
+  struct dict_iterator iter;
+  struct symbol *sym;
+
+  for (sym = dict_iter_name_first (BLOCK_DICT (block),
+                                   name, &iter);
+       sym != NULL;
+       sym = dict_iter_name_next (name, &iter))
+    {
+      if (SYMBOL_VALUE_ADDRESS (msym) == SYMBOL_VALUE_ADDRESS (sym))
+	{
+	  return sym;
+	}
+    }
+  return NULL;
+}
+
+struct symbol *
+lookup_symbol_msymbol_block_vector (const char *name,
+				    struct minimal_symbol *msym,
+                                    struct blockvector *bv);
+
+struct symbol *
+lookup_symbol_msymbol_block_vector (const char *name,
+				    struct minimal_symbol *msym,
+                                    struct blockvector *bv)
+{
+  struct block *block;
+  struct symbol *sym = NULL;
+
+  block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
+  sym = lookup_block_symbol_for_msymbol (block, msym, name);
+  if (sym)
+    return sym;
+
+  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
+  sym = lookup_block_symbol_for_msymbol (block, msym, name);
+
+  return sym;
+}
+
+struct symtab_cache {
+  struct objfile *last_objfile;
+  struct symtab *last_symtab;
+};
+
+void symtab_cache_init (struct symtab_cache *cache);
+void symtab_cache_init (struct symtab_cache *cache)
+{
+  cache->last_objfile = NULL;
+  cache->last_symtab = NULL;
+}
+
 /* Wrapper around lookup_symbol_aux_objfile for search_symbols.
    Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE.  */
 
 static struct symbol *
 lookup_msymbol_in_objfile (struct objfile *objfile,
 			   struct minimal_symbol *msymbol,
-			   domain_enum domain)
+			   domain_enum domain,
+			   struct symtab_cache *cache)
 {
   const char *name = SYMBOL_LINKAGE_NAME (msymbol);
   enum language lang = current_language->la_language;
   const char *modified_name;
-  struct cleanup *cleanup = demangle_for_lookup (name, lang, &modified_name);
-  struct symbol *returnval;
+  struct cleanup *cleanup;
+  struct symbol *returnval = NULL;
+  struct symtab *current_symtab;
+  struct symtab *orig_symtab;
+  struct symtab *next_symtab;
+  struct symtab *stop_at_this_symtab;
+
+  /* FIXME: can this kill the symtab expansion side-effect? */
+  if (!objfile_has_symbols (objfile))
+    return NULL;
+
+  cleanup = demangle_for_lookup (name, lang, &modified_name);
+
+  if (cache->last_objfile != objfile)
+    current_symtab = objfile->symtabs;
+  else
+    current_symtab = cache->last_symtab;
+
+  orig_symtab = current_symtab;
+  stop_at_this_symtab = NULL;
+  next_symtab = objfile->symtabs;
+
+  while (current_symtab != stop_at_this_symtab)
+    {
+      if (current_symtab->primary)
+        {
+          struct blockvector *bv = BLOCKVECTOR (current_symtab);
 
-  returnval = lookup_symbol_aux_objfile (objfile, GLOBAL_BLOCK,
-					 modified_name, domain);
-  if (returnval == NULL)
-    returnval = lookup_symbol_aux_objfile (objfile, STATIC_BLOCK,
-					   modified_name, domain);
+          returnval = lookup_symbol_msymbol_block_vector (name, msymbol, bv);
 
+          if (returnval)
+            {
+              if (cache->last_objfile != objfile)
+		cache->last_objfile = objfile;
+
+	      if (cache->last_symtab != current_symtab)
+		cache->last_symtab = current_symtab;
+
+	      goto ret_sym;
+	    }
+	}
+      if (current_symtab->next == NULL)
+	{
+	  current_symtab = next_symtab;
+	  stop_at_this_symtab = orig_symtab;
+	  next_symtab = orig_symtab;
+	}
+      else
+        current_symtab = current_symtab->next;
+    }
+
+ ret_sym:
   do_cleanups (cleanup);
   return returnval;
 }
@@ -3474,6 +3580,10 @@ search_symbols (char *regexp, enum search_domain kind,
 
   if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
     {
+      struct symtab_cache st_cache;
+
+      symtab_cache_init(&st_cache);
+
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
@@ -3492,7 +3602,8 @@ search_symbols (char *regexp, enum search_domain kind,
 		if (kind == FUNCTIONS_DOMAIN
 		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
 		    : lookup_msymbol_in_objfile (objfile, msymbol,
-						 VAR_DOMAIN) == NULL)
+						 VAR_DOMAIN,
+						 &st_cache) == NULL)
 		  found_misc = 1;
 	      }
 	  }
@@ -3572,6 +3683,10 @@ search_symbols (char *regexp, enum search_domain kind,
 
   if (found_misc || kind != FUNCTIONS_DOMAIN)
     {
+      struct symtab_cache st_cache;
+
+      symtab_cache_init(&st_cache);
+
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
@@ -3592,7 +3707,8 @@ search_symbols (char *regexp, enum search_domain kind,
 		    /* Variables/Absolutes:  Look up by name.  */
 		    /* FIXME: Why do we also look up fns by name?  */
 		    if (lookup_msymbol_in_objfile (objfile, msymbol,
-						   VAR_DOMAIN) == NULL)
+						   VAR_DOMAIN,
+						   &st_cache) == NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)

Re: [RFA] massively speed up "info var foo" on large programs

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 2011 bytes --]

On Thu, May 24, 2012 at 2:28 PM, Doug Evans <dje@google.com> wrote:
> On Thu, May 24, 2012 at 10:58 AM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> I'm not entirely sure this patch is correct, but it feels correct (*1),
>> and is a massive win.
>> "info var Task" in one large program goes from 350 seconds to 28 seconds.
>>
>> [...]
>>
>> 2012-05-23  Doug Evans  <dje@google.com>
>>
>>        * symtab.c (lookup_msymbol_in_objfile): New function.
>>        (search_symbols): Call it.
>
> Hmmm.
> One thing that occurs to me is separate debug objfiles.
> lookup_msymbol_in_objfile should probably search them.

This is a revised patch.
It scans separate debug files.
I think I understand the code better so I've removed the FIXME: The
comments in the code were misleading, find_pc_symtab is, I think, an
optimization to avoid unnecessarily calling lookup_symbol.
This patch also adds a (nfiles == 0) check to the second minsym loop:
there's no point in scanning minsyms for specific files.

The output is different from the previous code, I didn't take into
account the symbols that gdb creates for @plt entries.  I think if we
want to continue to provide the current output, we should add an
option to "info var|fun|type" to produce it: the normal case shouldn't
be that slow.

This patch removes the gdb-created minsyms from the output.
Another way to go is to print them.  I don't have a strong opinion on
either choice.
[The different with the current behaviour is that if the minsym is
found in any objfile then the current code won't print it in the
"Non-debugging symbols" section of the output.]

Ok to check in?

2012-05-25  Doug Evans  <dje@google.com>

        * symtab.c (minimal_symbol): New member created_by_gdb.
        * elfread.c (elf_symtab_read): Set created_by_gdb for @plt minsym
        created by gdb.
        * symtab.c (lookup_msymbol_in_objfile): New function.
        (search_symbols): Call it.  Only scan minsyms if nfiles == 0.

[-- Attachment #2: gdb-120525-search-symbols-speedup-2.patch.txt --]
[-- Type: text/plain, Size: 5697 bytes --]

2012-05-25  Doug Evans  <dje@google.com>

	* symtab.c (minimal_symbol): New member created_by_gdb.
	* elfread.c (elf_symtab_read): Set created_by_gdb for @plt minsym
	created by gdb.
	* symtab.c (lookup_msymbol_in_objfile): New function.
	(search_symbols): Call it.  Only scan minsyms if nfiles == 0.

Index: elfread.c
===================================================================
RCS file: /cvs/src/src/gdb/elfread.c,v
retrieving revision 1.131
diff -u -p -r1.131 elfread.c
--- elfread.c	18 May 2012 21:02:47 -0000	1.131
+++ elfread.c	25 May 2012 07:13:41 -0000
@@ -594,6 +594,7 @@ elf_symtab_read (struct objfile *objfile
 		  if (mtramp)
 		    {
 		      MSYMBOL_SIZE (mtramp) = MSYMBOL_SIZE (msym);
+		      mtramp->created_by_gdb = 1;
 		      mtramp->filename = filesymname;
 		      gdbarch_elf_make_msymbol_special (gdbarch, sym, mtramp);
 		    }
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.306
diff -u -p -r1.306 symtab.c
--- symtab.c	24 May 2012 02:51:48 -0000	1.306
+++ symtab.c	25 May 2012 07:13:41 -0000
@@ -1559,6 +1559,48 @@ lookup_symbol_aux_symtabs (int block_ind
   return NULL;
 }
 
+/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
+   Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE
+   and all related objfiles.  */
+
+static struct symbol *
+lookup_msymbol_in_objfile (struct objfile *objfile,
+			   struct minimal_symbol *msymbol,
+			   domain_enum domain)
+{
+  const char *name = SYMBOL_LINKAGE_NAME (msymbol);
+  enum language lang = current_language->la_language;
+  const char *modified_name;
+  struct cleanup *cleanup = demangle_for_lookup (name, lang, &modified_name);
+  struct objfile *main_objfile, *cur_objfile;
+
+  if (objfile->separate_debug_objfile_backlink)
+    main_objfile = objfile->separate_debug_objfile_backlink;
+  else
+    main_objfile = objfile;
+
+  for (cur_objfile = main_objfile;
+       cur_objfile;
+       cur_objfile = objfile_separate_debug_iterate (main_objfile, cur_objfile))
+    {
+      struct symbol *sym;
+
+      sym = lookup_symbol_aux_objfile (cur_objfile, GLOBAL_BLOCK,
+				       modified_name, domain);
+      if (sym == NULL)
+	sym = lookup_symbol_aux_objfile (cur_objfile, STATIC_BLOCK,
+					 modified_name, domain);
+      if (sym != NULL)
+	{
+	  do_cleanups (cleanup);
+	  return sym;
+	}
+    }
+
+  do_cleanups (cleanup);
+  return NULL;
+}
+
 /* A helper function for lookup_symbol_aux that interfaces with the
    "quick" symbol table functions.  */
 
@@ -3463,21 +3505,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
-		  {
-		    /* FIXME: carlton/2003-02-04: Given that the
-		       semantics of lookup_symbol keeps on changing
-		       slightly, it would be a nice idea if we had a
-		       function lookup_symbol_minsym that found the
-		       symbol associated to a given minimal symbol (if
-		       any).  */
-		    if (kind == FUNCTIONS_DOMAIN
-			|| lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-					  (struct block *) NULL,
-					  VAR_DOMAIN, 0)
-			== NULL)
-		      found_misc = 1;
-		  }
+		/* Note: An important side-effect of these lookup functions
+		   is to expand the symbol table if msymbol is found.  */
+		if (kind == FUNCTIONS_DOMAIN
+		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
+		    : lookup_msymbol_in_objfile (objfile, msymbol,
+						 VAR_DOMAIN) == NULL)
+		  found_misc = 1;
 	      }
 	  }
       }
@@ -3554,12 +3588,15 @@ search_symbols (char *regexp, enum searc
   /* If there are no eyes, avoid all contact.  I mean, if there are
      no debug symbols, then print directly from the msymbol_vector.  */
 
-  if (found_misc || kind != FUNCTIONS_DOMAIN)
+  if (found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
     {
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
 
+	if (msymbol->created_by_gdb)
+	  continue;
+
 	if (MSYMBOL_TYPE (msymbol) == ourtype
 	    || MSYMBOL_TYPE (msymbol) == ourtype2
 	    || MSYMBOL_TYPE (msymbol) == ourtype3
@@ -3569,14 +3606,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		/* Functions:  Look up by address.  */
-		if (kind != FUNCTIONS_DOMAIN ||
-		    (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))))
+		/* For functions we can do a quick check of whether the
+		   symbol might be found via find_pc_symtab.  */
+		if (kind != FUNCTIONS_DOMAIN
+		    || find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL)
 		  {
-		    /* Variables/Absolutes:  Look up by name.  */
-		    if (lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-				       (struct block *) NULL, VAR_DOMAIN, 0)
-			 == NULL)
+		    if (lookup_msymbol_in_objfile (objfile, msymbol,
+						   VAR_DOMAIN) == NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.206
diff -u -p -r1.206 symtab.h
--- symtab.h	10 May 2012 20:04:00 -0000	1.206
+++ symtab.h	25 May 2012 07:13:42 -0000
@@ -339,6 +339,10 @@ struct minimal_symbol
 
   ENUM_BITFIELD(minimal_symbol_type) type : 8;
 
+  /* Non-zero if this symbol was created by gdb.
+     Such symbols do not appear in the output of "info var|fun".  */
+  unsigned int created_by_gdb : 1;
+
   /* Two flag bits provided for the use of the target.  */
   unsigned int target_flag_1 : 1;
   unsigned int target_flag_2 : 1;

Re: [RFA] massively speed up "info var foo" on large programs

[Pedro Alves]

On 05/25/2012 09:21 AM, Doug Evans wrote:

> 
> The output is different from the previous code, I didn't take into
> account the symbols that gdb creates for @plt entries.  I think if we
> want to continue to provide the current output, we should add an
> option to "info var|fun|type" to produce it: the normal case shouldn't
> be that slow.


Different how?  The patch has no testsuite updates, so the email reader is
left wondering.  ;-)

-- 
Pedro Alves

Re: [RFA] massively speed up "info var foo" on large programs

[Matt Rice]

On Fri, May 25, 2012 at 1:21 AM, Doug Evans <dje@google.com> wrote:

> It scans separate debug files.
>
>+  if (objfile->separate_debug_objfile_backlink)
>+    main_objfile = objfile->separate_debug_objfile_backlink;
>+  else
>+    main_objfile = objfile;

I'm wondering if this is OK/necessary from objfiles.h:

Note that separate debug object are in the main chain and therefore
will be visited by ALL_OBJFILES & co iterators.

>       ALL_MSYMBOLS (objfile, msymbol)
>       {
>         QUIT;
>
>+	if (msymbol->created_by_gdb)
>+	  continue;
>+

so shouldn't ALL_MSYMBOLS (which calls ALL_OBJFILES) cover it,
and the separate debuginfo code above potentially cause symbols in
separate debuginfo objfiles to be listed twice?

Re: [RFA] massively speed up "info var foo" on large programs

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]

On Fri, May 25, 2012 at 1:50 AM, Pedro Alves <palves@redhat.com> wrote:
> On 05/25/2012 09:21 AM, Doug Evans wrote:
>
>>
>> The output is different from the previous code, I didn't take into
>> account the symbols that gdb creates for @plt entries.  I think if we
>> want to continue to provide the current output, we should add an
>> option to "info var|fun|type" to produce it: the normal case shouldn't
>> be that slow.
>
>
> Different how?  The patch has no testsuite updates, so the email reader is
> left wondering.  ;-)

In the "Non-debugging symbols" section of the output, when a symbol
would have been found in another objfile, the code would have not
printed the non-@plt form of the function name.
With this patch we have a decision to make.  Searching all the other
objfiles is not reasonable (IMO) so what to do?  I can think of two
possibilities: always print it or never print it.  Since the symbol in
question is an artificial symbol created by gdb I have opted for never
printing it.

Thus instead of seeing this in the "Non-debugging symbols" section of
the output:

0x1234 foo@plt
0x1234 foo

the output will contain:

0x1234 foo@plt

Here is v3 of the patch.  I added a testcase.
Regression tested on amd64-linux.

Ok to check in?

2012-05-27  Doug Evans  <dje@google.com>

        * symtab.c (minimal_symbol): New member created_by_gdb.
        * elfread.c (elf_symtab_read): Set created_by_gdb for @plt
minsym
        created by gdb.
        * symtab.c (lookup_msymbol_in_objfile): New function.
        (search_symbols): Call it.  Only scan minsyms if nfiles == 0.

        testsuite:
        * gdb.base/info-fun.exp: New file.
        * gdb.base/info-fun.c: New file.
        * gdb.base/info-fun-solib.c: New file.

[-- Attachment #2: gdb-120527-search-symbols-speedup-3.patch.txt --]
[-- Type: text/plain, Size: 10749 bytes --]

2012-05-27  Doug Evans  <dje@google.com>

	* symtab.c (minimal_symbol): New member created_by_gdb.
	* elfread.c (elf_symtab_read): Set created_by_gdb for @plt minsym
	created by gdb.
	* symtab.c (lookup_msymbol_in_objfile): New function.
	(search_symbols): Call it.  Only scan minsyms if nfiles == 0.

	testsuite:
	* gdb.base/info-fun.exp: New file.
	* gdb.base/info-fun.c: New file.
	* gdb.base/info-fun-solib.c: New file.

Index: elfread.c
===================================================================
RCS file: /cvs/src/src/gdb/elfread.c,v
retrieving revision 1.131
diff -u -p -r1.131 elfread.c
--- elfread.c	18 May 2012 21:02:47 -0000	1.131
+++ elfread.c	28 May 2012 04:13:19 -0000
@@ -594,6 +594,7 @@ elf_symtab_read (struct objfile *objfile
 		  if (mtramp)
 		    {
 		      MSYMBOL_SIZE (mtramp) = MSYMBOL_SIZE (msym);
+		      mtramp->created_by_gdb = 1;
 		      mtramp->filename = filesymname;
 		      gdbarch_elf_make_msymbol_special (gdbarch, sym, mtramp);
 		    }
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.306
diff -u -p -r1.306 symtab.c
--- symtab.c	24 May 2012 02:51:48 -0000	1.306
+++ symtab.c	28 May 2012 04:13:19 -0000
@@ -1559,6 +1559,48 @@ lookup_symbol_aux_symtabs (int block_ind
   return NULL;
 }
 
+/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
+   Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE
+   and all related objfiles.  */
+
+static struct symbol *
+lookup_msymbol_in_objfile (struct objfile *objfile,
+			   struct minimal_symbol *msymbol,
+			   domain_enum domain)
+{
+  const char *name = SYMBOL_LINKAGE_NAME (msymbol);
+  enum language lang = current_language->la_language;
+  const char *modified_name;
+  struct cleanup *cleanup = demangle_for_lookup (name, lang, &modified_name);
+  struct objfile *main_objfile, *cur_objfile;
+
+  if (objfile->separate_debug_objfile_backlink)
+    main_objfile = objfile->separate_debug_objfile_backlink;
+  else
+    main_objfile = objfile;
+
+  for (cur_objfile = main_objfile;
+       cur_objfile;
+       cur_objfile = objfile_separate_debug_iterate (main_objfile, cur_objfile))
+    {
+      struct symbol *sym;
+
+      sym = lookup_symbol_aux_objfile (cur_objfile, GLOBAL_BLOCK,
+				       modified_name, domain);
+      if (sym == NULL)
+	sym = lookup_symbol_aux_objfile (cur_objfile, STATIC_BLOCK,
+					 modified_name, domain);
+      if (sym != NULL)
+	{
+	  do_cleanups (cleanup);
+	  return sym;
+	}
+    }
+
+  do_cleanups (cleanup);
+  return NULL;
+}
+
 /* A helper function for lookup_symbol_aux that interfaces with the
    "quick" symbol table functions.  */
 
@@ -3463,21 +3505,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
-		  {
-		    /* FIXME: carlton/2003-02-04: Given that the
-		       semantics of lookup_symbol keeps on changing
-		       slightly, it would be a nice idea if we had a
-		       function lookup_symbol_minsym that found the
-		       symbol associated to a given minimal symbol (if
-		       any).  */
-		    if (kind == FUNCTIONS_DOMAIN
-			|| lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-					  (struct block *) NULL,
-					  VAR_DOMAIN, 0)
-			== NULL)
-		      found_misc = 1;
-		  }
+		/* Note: An important side-effect of these lookup functions
+		   is to expand the symbol table if msymbol is found.  */
+		if (kind == FUNCTIONS_DOMAIN
+		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
+		    : lookup_msymbol_in_objfile (objfile, msymbol,
+						 VAR_DOMAIN) == NULL)
+		  found_misc = 1;
 	      }
 	  }
       }
@@ -3554,12 +3588,15 @@ search_symbols (char *regexp, enum searc
   /* If there are no eyes, avoid all contact.  I mean, if there are
      no debug symbols, then print directly from the msymbol_vector.  */
 
-  if (found_misc || kind != FUNCTIONS_DOMAIN)
+  if (found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
     {
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
 
+	if (msymbol->created_by_gdb)
+	  continue;
+
 	if (MSYMBOL_TYPE (msymbol) == ourtype
 	    || MSYMBOL_TYPE (msymbol) == ourtype2
 	    || MSYMBOL_TYPE (msymbol) == ourtype3
@@ -3569,14 +3606,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		/* Functions:  Look up by address.  */
-		if (kind != FUNCTIONS_DOMAIN ||
-		    (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))))
+		/* For functions we can do a quick check of whether the
+		   symbol might be found via find_pc_symtab.  */
+		if (kind != FUNCTIONS_DOMAIN
+		    || find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL)
 		  {
-		    /* Variables/Absolutes:  Look up by name.  */
-		    if (lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-				       (struct block *) NULL, VAR_DOMAIN, 0)
-			 == NULL)
+		    if (lookup_msymbol_in_objfile (objfile, msymbol,
+						   VAR_DOMAIN) == NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.206
diff -u -p -r1.206 symtab.h
--- symtab.h	10 May 2012 20:04:00 -0000	1.206
+++ symtab.h	28 May 2012 04:13:19 -0000
@@ -339,6 +339,10 @@ struct minimal_symbol
 
   ENUM_BITFIELD(minimal_symbol_type) type : 8;
 
+  /* Non-zero if this symbol was created by gdb.
+     Such symbols do not appear in the output of "info var|fun".  */
+  unsigned int created_by_gdb : 1;
+
   /* Two flag bits provided for the use of the target.  */
   unsigned int target_flag_1 : 1;
   unsigned int target_flag_2 : 1;
Index: testsuite/gdb.base/info-fun-solib.c
===================================================================
RCS file: testsuite/gdb.base/info-fun-solib.c
diff -N testsuite/gdb.base/info-fun-solib.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/info-fun-solib.c	28 May 2012 04:13:20 -0000
@@ -0,0 +1,20 @@
+/* Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+foo (void)
+{
+  return 0;
+}
Index: testsuite/gdb.base/info-fun.c
===================================================================
RCS file: testsuite/gdb.base/info-fun.c
diff -N testsuite/gdb.base/info-fun.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/info-fun.c	28 May 2012 04:13:20 -0000
@@ -0,0 +1,22 @@
+/* Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern int foo (void);
+
+int
+main ()
+{
+  return foo ();
+}
Index: testsuite/gdb.base/info-fun.exp
===================================================================
RCS file: testsuite/gdb.base/info-fun.exp
diff -N testsuite/gdb.base/info-fun.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/info-fun.exp	28 May 2012 04:13:20 -0000
@@ -0,0 +1,76 @@
+# Copyright 2012 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if { [skip_shlib_tests] || [is_remote target] } {
+    return 0
+}
+
+# Library file.
+set libname "info-fun-solib"
+set srcfile_lib ${srcdir}/${subdir}/${libname}.c
+set binfile_lib ${objdir}/${subdir}/${libname}.so
+set lib_flags {}
+# Binary file.
+set testfile "info-fun"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+set bin_flags [list debug shlib=${binfile_lib}]
+
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+# SEP must be last for the possible `unsupported' error path.
+foreach libsepdebug {NO IN SEP} { with_test_prefix "$libsepdebug" {
+
+    set sep_lib_flags $lib_flags
+    if {$libsepdebug != "NO"} {
+	lappend sep_lib_flags {debug}
+    }
+    if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $sep_lib_flags] != ""
+	 || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+      untested "Could not compile $binfile_lib or $binfile."
+      return -1
+    }
+
+    if {$libsepdebug == "SEP"} {
+	if {[gdb_gnu_strip_debug $binfile_lib] != 0} {
+	    unsupported "Could not split debug of $binfile_lib."
+	    return
+	} else {
+	    pass "split solib"
+	}
+    }
+
+    clean_restart $executable
+
+    if ![runto_main] then {
+      fail "Can't run to main"
+      return 0
+    }
+
+    set match_str {All functions matching regular expression "foo":[\r\n]*}
+    if { "$libsepdebug" != "NO"  } {
+	append match_str {File .*/info-fun-solib[.]c:[\r\n]*}
+	append match_str {int foo\(void\);[\r\n]*}
+    }
+    append match_str {Non-debugging symbols:[\r\n]*}
+    append match_str "$hex *foo(@plt)?\[\r\n\]*"
+    if { "$libsepdebug" == "NO"  } {
+	append match_str "$hex *foo\[\r\n\]*"
+    }
+
+    gdb_test "info fun foo" "$match_str"
+}}

Re: [RFA] massively speed up "info var foo" on large programs

[Doug Evans]

Ping.
I'm happy with this patch enough to check it in in a few days if there
are no objections.
[Note that the "interesting" case only comes up when a PLT entry
refers to a function that isn't present in any objfile.  If it is
present in another objfile then it will be printed there.]

On Sun, May 27, 2012 at 9:49 PM, Doug Evans <dje@google.com> wrote:
> On Fri, May 25, 2012 at 1:50 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 05/25/2012 09:21 AM, Doug Evans wrote:
>>
>>>
>>> The output is different from the previous code, I didn't take into
>>> account the symbols that gdb creates for @plt entries.  I think if we
>>> want to continue to provide the current output, we should add an
>>> option to "info var|fun|type" to produce it: the normal case shouldn't
>>> be that slow.
>>
>>
>> Different how?  The patch has no testsuite updates, so the email reader is
>> left wondering.  ;-)
>
> In the "Non-debugging symbols" section of the output, when a symbol
> would have been found in another objfile, the code would have not
> printed the non-@plt form of the function name.
> With this patch we have a decision to make.  Searching all the other
> objfiles is not reasonable (IMO) so what to do?  I can think of two
> possibilities: always print it or never print it.  Since the symbol in
> question is an artificial symbol created by gdb I have opted for never
> printing it.
>
> Thus instead of seeing this in the "Non-debugging symbols" section of
> the output:
>
> 0x1234 foo@plt
> 0x1234 foo
>
> the output will contain:
>
> 0x1234 foo@plt
>
> Here is v3 of the patch.  I added a testcase.
> Regression tested on amd64-linux.
>
> Ok to check in?
>
> 2012-05-27  Doug Evans  <dje@google.com>
>
>        * symtab.c (minimal_symbol): New member created_by_gdb.
>        * elfread.c (elf_symtab_read): Set created_by_gdb for @plt
> minsym
>        created by gdb.
>        * symtab.c (lookup_msymbol_in_objfile): New function.
>        (search_symbols): Call it.  Only scan minsyms if nfiles == 0.
>
>        testsuite:
>        * gdb.base/info-fun.exp: New file.
>        * gdb.base/info-fun.c: New file.
>        * gdb.base/info-fun-solib.c: New file.

Re: [RFA] massively speed up "info var foo" on large programs

[Pedro Alves]

On 05/28/2012 05:49 AM, Doug Evans wrote:

> On Fri, May 25, 2012 at 1:50 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 05/25/2012 09:21 AM, Doug Evans wrote:
>>
>>>
>>> The output is different from the previous code, I didn't take into
>>> account the symbols that gdb creates for @plt entries.  I think if we
>>> want to continue to provide the current output, we should add an
>>> option to "info var|fun|type" to produce it: the normal case shouldn't
>>> be that slow.
>>
>>
>> Different how?  The patch has no testsuite updates, so the email reader is
>> left wondering.  ;-)
> 
> In the "Non-debugging symbols" section of the output, when a symbol
> would have been found in another objfile, the code would have not
> printed the non-@plt form of the function name.
> With this patch we have a decision to make.  Searching all the other
> objfiles is not reasonable (IMO) so what to do?  I can think of two
> possibilities: always print it or never print it.  Since the symbol in
> question is an artificial symbol created by gdb I have opted for never
> printing it.
> 
> Thus instead of seeing this in the "Non-debugging symbols" section of
> the output:
> 
> 0x1234 foo@plt
> 0x1234 foo
> 
> the output will contain:
> 
> 0x1234 foo@plt
> 
> Here is v3 of the patch.  I added a testcase.
> Regression tested on amd64-linux.
> 


Took me a while, and a gdb.log diff of the new testcase between
patched and unpatched gdb to grok this.  I think this output change
is okay.

> @@ -3463,21 +3505,13 @@ search_symbols (char *regexp, enum searc
>  		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
>  			    NULL, 0) == 0)
>  	      {
> -		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
> -		  {
> -		    /* FIXME: carlton/2003-02-04: Given that the
> -		       semantics of lookup_symbol keeps on changing
> -		       slightly, it would be a nice idea if we had a
> -		       function lookup_symbol_minsym that found the
> -		       symbol associated to a given minimal symbol (if
> -		       any).  */
> -		    if (kind == FUNCTIONS_DOMAIN
> -			|| lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
> -					  (struct block *) NULL,
> -					  VAR_DOMAIN, 0)

There's a comment above all this that talks about "lookup_symbol" that needs
updating.

> -			== NULL)
> -		      found_misc = 1;
> -		  }
> +		/* Note: An important side-effect of these lookup functions
> +		   is to expand the symbol table if msymbol is found.  */

Okay, it's an important side-effect, but why do we want the side effect
should IMO be mentioned. It's not obvious to me (a casual reader).

> +		if (kind == FUNCTIONS_DOMAIN
> +		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
> +		    : lookup_msymbol_in_objfile (objfile, msymbol,
> +						 VAR_DOMAIN) == NULL)

The "lookup_msymbol_in_objfile" function name threw me off for a few
minutes, as in, "if we already know the msymbol exists in objfile, wth
are we looking it up again?  For those important side-effects perhaps?"
More below.

> +		  found_misc = 1;
>  	      }
>
> +/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
> + Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE
> + and all related objfiles. */
> +
> +static struct symbol *
> +lookup_msymbol_in_objfile (struct objfile *objfile,
> + struct minimal_symbol *msymbol,
> + domain_enum domain)
> +{
> + const char *name = SYMBOL_LINKAGE_NAME (msymbol);

Ah, MSYMBOL is the input, not the output!  The lookup_symbol_xxx
functions return a symbol, so I was naively expecting this to return
an msymbol.  IMO it'd be clearer if it'd be called something like
lookup_symbol_of_msymbol_in_objfile then.  But, the only usage of MSYMBOL
in the entire function is extracting its name. This would then be much
simpler, clearer and reusable IMO if the prototype and name of
the function was:

static struct symbol *
lookup_symbol_in_objfile (struct objfile *objfile,
                          const char *name,
                          domain_enum domain);

And the caller did the SYMBOL_LINKAGE_NAME (msymbol) thing.

-- 
Pedro Alves

Re: [RFA] massively speed up "info var foo" on large programs

[Doug Evans]

On Fri, Jun 1, 2012 at 12:38 PM, Pedro Alves <palves@redhat.com> wrote:
> On 05/28/2012 05:49 AM, Doug Evans wrote:
>
>> On Fri, May 25, 2012 at 1:50 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 05/25/2012 09:21 AM, Doug Evans wrote:
>>>
>>>>
>>>> The output is different from the previous code, I didn't take into
>>>> account the symbols that gdb creates for @plt entries.  I think if we
>>>> want to continue to provide the current output, we should add an
>>>> option to "info var|fun|type" to produce it: the normal case shouldn't
>>>> be that slow.
>>>
>>>
>>> Different how?  The patch has no testsuite updates, so the email reader is
>>> left wondering.  ;-)
>>
>> In the "Non-debugging symbols" section of the output, when a symbol
>> would have been found in another objfile, the code would have not
>> printed the non-@plt form of the function name.
>> With this patch we have a decision to make.  Searching all the other
>> objfiles is not reasonable (IMO) so what to do?  I can think of two
>> possibilities: always print it or never print it.  Since the symbol in
>> question is an artificial symbol created by gdb I have opted for never
>> printing it.
>>
>> Thus instead of seeing this in the "Non-debugging symbols" section of
>> the output:
>>
>> 0x1234 foo@plt
>> 0x1234 foo
>>
>> the output will contain:
>>
>> 0x1234 foo@plt
>>
>> Here is v3 of the patch.  I added a testcase.
>> Regression tested on amd64-linux.
>>
>
>
> Took me a while, and a gdb.log diff of the new testcase between
> patched and unpatched gdb to grok this.  I think this output change
> is okay.
>
>> @@ -3463,21 +3505,13 @@ search_symbols (char *regexp, enum searc
>>               || regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
>>                           NULL, 0) == 0)
>>             {
>> -             if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
>> -               {
>> -                 /* FIXME: carlton/2003-02-04: Given that the
>> -                    semantics of lookup_symbol keeps on changing
>> -                    slightly, it would be a nice idea if we had a
>> -                    function lookup_symbol_minsym that found the
>> -                    symbol associated to a given minimal symbol (if
>> -                    any).  */
>> -                 if (kind == FUNCTIONS_DOMAIN
>> -                     || lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
>> -                                       (struct block *) NULL,
>> -                                       VAR_DOMAIN, 0)
>
> There's a comment above all this that talks about "lookup_symbol" that needs
> updating.

Yeah, already updated in my sandbox.

>> -                     == NULL)
>> -                   found_misc = 1;
>> -               }
>> +             /* Note: An important side-effect of these lookup functions
>> +                is to expand the symbol table if msymbol is found.  */
>
> Okay, it's an important side-effect, but why do we want the side effect
> should IMO be mentioned. It's not obvious to me (a casual reader).

Most of gdb's symbol handling is arguably non-obvious.   1/2 :-)
I've expanded the comment.

>> +             if (kind == FUNCTIONS_DOMAIN
>> +                 ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
>> +                 : lookup_msymbol_in_objfile (objfile, msymbol,
>> +                                              VAR_DOMAIN) == NULL)
>
> The "lookup_msymbol_in_objfile" function name threw me off for a few
> minutes, as in, "if we already know the msymbol exists in objfile, wth
> are we looking it up again?  For those important side-effects perhaps?"
> More below.

The comment for the function is pretty unambiguous.

>
>> +               found_misc = 1;
>>             }
>>
>> +/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
>> + Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE
>> + and all related objfiles. */
>> +
>> +static struct symbol *
>> +lookup_msymbol_in_objfile (struct objfile *objfile,
>> + struct minimal_symbol *msymbol,
>> + domain_enum domain)
>> +{
>> + const char *name = SYMBOL_LINKAGE_NAME (msymbol);
>
> Ah, MSYMBOL is the input, not the output!  The lookup_symbol_xxx
> functions return a symbol, so I was naively expecting this to return
> an msymbol.  IMO it'd be clearer if it'd be called something like
> lookup_symbol_of_msymbol_in_objfile then.  But, the only usage of MSYMBOL
> in the entire function is extracting its name. This would then be much
> simpler, clearer and reusable IMO if the prototype and name of
> the function was:
>
> static struct symbol *
> lookup_symbol_in_objfile (struct objfile *objfile,
>                          const char *name,
>                          domain_enum domain);
>
> And the caller did the SYMBOL_LINKAGE_NAME (msymbol) thing.

Actually, I had a similarly named function and prototype early on, but
I didn't like it.  GDB's symbol support is a mess (IMO), and I wanted
a name and usage to restrict it to the task at hand.  Anything more
general and I was pretty sure this patch would get bogged down.
Later on I want to revamp the symbol API, but I don't want this patch
tied down by that.
(For example, I don't want to bubble up the semantics of
demangle_for_lookup to the caller of this function.   Here we have an
msymbol, we know we have a mangled name.  If you want, I can go with
lookup_symbol_in_objfile but rename it to
lookup_symbol_in_objfile_from_linkage_name or some such.)

Re: [RFA] massively speed up "info var foo" on large programs

[Pedro Alves]

On 06/04/2012 05:06 AM, Doug Evans wrote:

> GDB's symbol support is a mess (IMO), and I wanted
> a name and usage to restrict it to the task at hand.  Anything more
> general and I was pretty sure this patch would get bogged down.


That'd be a bit beyond what I requested.  ;-)

> Later on I want to revamp the symbol API, but I don't want this patch
> tied down by that.


Certainly.  That'd clearly need to be a separate change.

> (For example, I don't want to bubble up the semantics of
> demangle_for_lookup to the caller of this function.   Here we have an
> msymbol, we know we have a mangled name.  If you want, I can go with
> lookup_symbol_in_objfile but rename it to
> lookup_symbol_in_objfile_from_linkage_name or some such.)


That'd be fine with me.  Thanks.

-- 
Pedro Alves

Re: [RFA] massively speed up "info var foo" on large programs

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

On Mon, Jun 4, 2012 at 8:03 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/04/2012 05:06 AM, Doug Evans wrote:
>
>> GDB's symbol support is a mess (IMO), and I wanted
>> a name and usage to restrict it to the task at hand.  Anything more
>> general and I was pretty sure this patch would get bogged down.
>
>
> That'd be a bit beyond what I requested.  ;-)
>
>> Later on I want to revamp the symbol API, but I don't want this patch
>> tied down by that.
>
>
> Certainly.  That'd clearly need to be a separate change.
>
>> (For example, I don't want to bubble up the semantics of
>> demangle_for_lookup to the caller of this function.   Here we have an
>> msymbol, we know we have a mangled name.  If you want, I can go with
>> lookup_symbol_in_objfile but rename it to
>> lookup_symbol_in_objfile_from_linkage_name or some such.)
>
>
> That'd be fine with me.  Thanks.

Here is what I checked in.

2012-06-18  Doug Evans  <dje@google.com>

        * symtab.h (minimal_symbol): New member created_by_gdb.
        * elfread.c (elf_symtab_read): Set created_by_gdb for @plt minsym
        created by gdb.
        * symtab.c (lookup_symbol_in_objfile_from_linkage_name): New function.
        (search_symbols): Call it instead of lookup_symbol.
        Skip symbols created by gdb.  Only scan minsyms if nfiles == 0.

        testsuite:
        * gdb.base/info-fun.exp: New file.
        * gdb.base/info-fun.c: New file.
        * gdb.base/info-fun-solib.c: New file.

[-- Attachment #2: gdb-120618-search-symbols-speedup-4.patch.txt --]
[-- Type: text/plain, Size: 12159 bytes --]

2012-06-18  Doug Evans  <dje@google.com>

	* symtab.h (minimal_symbol): New member created_by_gdb.
	* elfread.c (elf_symtab_read): Set created_by_gdb for @plt minsym
	created by gdb.
	* symtab.c (lookup_symbol_in_objfile_from_linkage_name): New function.
	(search_symbols): Call it instead of lookup_symbol.
	Skip symbols created by gdb.  Only scan minsyms if nfiles == 0.

	testsuite:
	* gdb.base/info-fun.exp: New file.
	* gdb.base/info-fun.c: New file.
	* gdb.base/info-fun-solib.c: New file.

Index: elfread.c
===================================================================
RCS file: /cvs/src/src/gdb/elfread.c,v
retrieving revision 1.131
diff -u -p -r1.131 elfread.c
--- elfread.c	18 May 2012 21:02:47 -0000	1.131
+++ elfread.c	19 Jun 2012 00:20:48 -0000
@@ -594,6 +594,7 @@ elf_symtab_read (struct objfile *objfile
 		  if (mtramp)
 		    {
 		      MSYMBOL_SIZE (mtramp) = MSYMBOL_SIZE (msym);
+		      mtramp->created_by_gdb = 1;
 		      mtramp->filename = filesymname;
 		      gdbarch_elf_make_msymbol_special (gdbarch, sym, mtramp);
 		    }
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.312
diff -u -p -r1.312 symtab.c
--- symtab.c	13 Jun 2012 15:47:14 -0000	1.312
+++ symtab.c	19 Jun 2012 00:20:48 -0000
@@ -1552,6 +1552,48 @@ lookup_symbol_aux_symtabs (int block_ind
   return NULL;
 }
 
+/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
+   Look up LINKAGE_NAME in DOMAIN in the global and static blocks of OBJFILE
+   and all related objfiles.  */
+
+static struct symbol *
+lookup_symbol_in_objfile_from_linkage_name (struct objfile *objfile,
+					    const char *linkage_name,
+					    domain_enum domain)
+{
+  enum language lang = current_language->la_language;
+  const char *modified_name;
+  struct cleanup *cleanup = demangle_for_lookup (linkage_name, lang,
+						 &modified_name);
+  struct objfile *main_objfile, *cur_objfile;
+
+  if (objfile->separate_debug_objfile_backlink)
+    main_objfile = objfile->separate_debug_objfile_backlink;
+  else
+    main_objfile = objfile;
+
+  for (cur_objfile = main_objfile;
+       cur_objfile;
+       cur_objfile = objfile_separate_debug_iterate (main_objfile, cur_objfile))
+    {
+      struct symbol *sym;
+
+      sym = lookup_symbol_aux_objfile (cur_objfile, GLOBAL_BLOCK,
+				       modified_name, domain);
+      if (sym == NULL)
+	sym = lookup_symbol_aux_objfile (cur_objfile, STATIC_BLOCK,
+					 modified_name, domain);
+      if (sym != NULL)
+	{
+	  do_cleanups (cleanup);
+	  return sym;
+	}
+    }
+
+  do_cleanups (cleanup);
+  return NULL;
+}
+
 /* A helper function for lookup_symbol_aux that interfaces with the
    "quick" symbol table functions.  */
 
@@ -3450,10 +3492,14 @@ search_symbols (char *regexp, enum searc
      The symbol will then be found during the scan of symtabs below.
 
      For functions, find_pc_symtab should succeed if we have debug info
-     for the function, for variables we have to call lookup_symbol
-     to determine if the variable has debug info.
+     for the function, for variables we have to call
+     lookup_symbol_in_objfile_from_linkage_name to determine if the variable
+     has debug info.
      If the lookup fails, set found_misc so that we will rescan to print
-     any matching symbols without debug info.  */
+     any matching symbols without debug info.
+     We only search the objfile the msymbol came from, we no longer search
+     all objfiles.  In large programs (1000s of shared libs) searching all
+     objfiles is not worth the pain.  */
 
   if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
     {
@@ -3461,6 +3507,9 @@ search_symbols (char *regexp, enum searc
       {
         QUIT;
 
+	if (msymbol->created_by_gdb)
+	  continue;
+
 	if (MSYMBOL_TYPE (msymbol) == ourtype
 	    || MSYMBOL_TYPE (msymbol) == ourtype2
 	    || MSYMBOL_TYPE (msymbol) == ourtype3
@@ -3470,21 +3519,15 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
-		  {
-		    /* FIXME: carlton/2003-02-04: Given that the
-		       semantics of lookup_symbol keeps on changing
-		       slightly, it would be a nice idea if we had a
-		       function lookup_symbol_minsym that found the
-		       symbol associated to a given minimal symbol (if
-		       any).  */
-		    if (kind == FUNCTIONS_DOMAIN
-			|| lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-					  (struct block *) NULL,
-					  VAR_DOMAIN, 0)
-			== NULL)
-		      found_misc = 1;
-		  }
+		/* Note: An important side-effect of these lookup functions
+		   is to expand the symbol table if msymbol is found, for the
+		   benefit of the next loop on ALL_PRIMARY_SYMTABS.  */
+		if (kind == FUNCTIONS_DOMAIN
+		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
+		    : (lookup_symbol_in_objfile_from_linkage_name
+		       (objfile, SYMBOL_LINKAGE_NAME (msymbol), VAR_DOMAIN)
+		       == NULL))
+		  found_misc = 1;
 	      }
 	  }
       }
@@ -3561,12 +3604,15 @@ search_symbols (char *regexp, enum searc
   /* If there are no eyes, avoid all contact.  I mean, if there are
      no debug symbols, then print directly from the msymbol_vector.  */
 
-  if (found_misc || kind != FUNCTIONS_DOMAIN)
+  if (found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
     {
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
 
+	if (msymbol->created_by_gdb)
+	  continue;
+
 	if (MSYMBOL_TYPE (msymbol) == ourtype
 	    || MSYMBOL_TYPE (msymbol) == ourtype2
 	    || MSYMBOL_TYPE (msymbol) == ourtype3
@@ -3576,14 +3622,14 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		/* Functions:  Look up by address.  */
-		if (kind != FUNCTIONS_DOMAIN ||
-		    (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))))
+		/* For functions we can do a quick check of whether the
+		   symbol might be found via find_pc_symtab.  */
+		if (kind != FUNCTIONS_DOMAIN
+		    || find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL)
 		  {
-		    /* Variables/Absolutes:  Look up by name.  */
-		    if (lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-				       (struct block *) NULL, VAR_DOMAIN, 0)
-			 == NULL)
+		    if (lookup_symbol_in_objfile_from_linkage_name
+			(objfile, SYMBOL_LINKAGE_NAME (msymbol), VAR_DOMAIN)
+			== NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.207
diff -u -p -r1.207 symtab.h
--- symtab.h	13 Jun 2012 15:47:15 -0000	1.207
+++ symtab.h	19 Jun 2012 00:20:48 -0000
@@ -340,6 +340,10 @@ struct minimal_symbol
 
   ENUM_BITFIELD(minimal_symbol_type) type : 8;
 
+  /* Non-zero if this symbol was created by gdb.
+     Such symbols do not appear in the output of "info var|fun".  */
+  unsigned int created_by_gdb : 1;
+
   /* Two flag bits provided for the use of the target.  */
   unsigned int target_flag_1 : 1;
   unsigned int target_flag_2 : 1;
Index: testsuite/gdb.base/info-fun-solib.c
===================================================================
RCS file: testsuite/gdb.base/info-fun-solib.c
diff -N testsuite/gdb.base/info-fun-solib.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/info-fun-solib.c	19 Jun 2012 00:20:49 -0000
@@ -0,0 +1,20 @@
+/* Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+foo (void)
+{
+  return 0;
+}
Index: testsuite/gdb.base/info-fun.c
===================================================================
RCS file: testsuite/gdb.base/info-fun.c
diff -N testsuite/gdb.base/info-fun.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/info-fun.c	19 Jun 2012 00:20:49 -0000
@@ -0,0 +1,22 @@
+/* Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern int foo (void);
+
+int
+main ()
+{
+  return foo ();
+}
Index: testsuite/gdb.base/info-fun.exp
===================================================================
RCS file: testsuite/gdb.base/info-fun.exp
diff -N testsuite/gdb.base/info-fun.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/info-fun.exp	19 Jun 2012 00:20:49 -0000
@@ -0,0 +1,76 @@
+# Copyright 2012 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if { [skip_shlib_tests] || [is_remote target] } {
+    return 0
+}
+
+# Library file.
+set libname "info-fun-solib"
+set srcfile_lib ${srcdir}/${subdir}/${libname}.c
+set binfile_lib ${objdir}/${subdir}/${libname}.so
+set lib_flags {}
+# Binary file.
+set testfile "info-fun"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+set bin_flags [list debug shlib=${binfile_lib}]
+
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+# SEP must be last for the possible `unsupported' error path.
+foreach libsepdebug {NO IN SEP} { with_test_prefix "$libsepdebug" {
+
+    set sep_lib_flags $lib_flags
+    if {$libsepdebug != "NO"} {
+	lappend sep_lib_flags {debug}
+    }
+    if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $sep_lib_flags] != ""
+	 || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+      untested "Could not compile $binfile_lib or $binfile."
+      return -1
+    }
+
+    if {$libsepdebug == "SEP"} {
+	if {[gdb_gnu_strip_debug $binfile_lib] != 0} {
+	    unsupported "Could not split debug of $binfile_lib."
+	    return
+	} else {
+	    pass "split solib"
+	}
+    }
+
+    clean_restart $executable
+
+    if ![runto_main] then {
+      fail "Can't run to main"
+      return 0
+    }
+
+    set match_str {All functions matching regular expression "foo":[\r\n]*}
+    if { "$libsepdebug" != "NO"  } {
+	append match_str {File .*/info-fun-solib[.]c:[\r\n]*}
+	append match_str {int foo\(void\);[\r\n]*}
+    }
+    append match_str {Non-debugging symbols:[\r\n]*}
+    append match_str "$hex *foo(@plt)?\[\r\n\]*"
+    if { "$libsepdebug" == "NO"  } {
+	append match_str "$hex *foo\[\r\n\]*"
+    }
+
+    gdb_test "info fun foo" "$match_str"
+}}

Re: [RFA] massively speed up "info var foo" on large programs

[Andreas Schwab]

Doug Evans <dje@google.com> writes:

> +    set match_str {All functions matching regular expression "foo":[\r\n]*}
> +    if { "$libsepdebug" != "NO"  } {
> +	append match_str {File .*/info-fun-solib[.]c:[\r\n]*}
> +	append match_str {int foo\(void\);[\r\n]*}
> +    }
> +    append match_str {Non-debugging symbols:[\r\n]*}
> +    append match_str "$hex *foo(@plt)?\[\r\n\]*"
> +    if { "$libsepdebug" == "NO"  } {
> +	append match_str "$hex *foo\[\r\n\]*"
> +    }
> +
> +    gdb_test "info fun foo" "$match_str"
> +}}

(gdb) info fun foo
All functions matching regular expression "foo":

Non-debugging symbols:
0x0000000010000578  00000011.plt_call.foo+0
0x00000000100008e8  foo@plt
0x00000fffb7fae73c  .foo
(gdb) FAIL: gdb.base/info-fun.exp: NO: info fun foo

(gdb) info fun foo
All functions matching regular expression "foo":

File ./gdb.base/info-fun-solib.c:
int foo(void);

Non-debugging symbols:
0x0000000010000578  00000011.plt_call.foo+0
0x00000000100008e8  foo@plt
(gdb) FAIL: gdb.base/info-fun.exp: IN: info fun foo

(gdb) info fun foo
All functions matching regular expression "foo":

File ./gdb.base/info-fun-solib.c:
int foo(void);

Non-debugging symbols:
0x0000000010000578  00000011.plt_call.foo+0
0x00000000100008e8  foo@plt
(gdb) FAIL: gdb.base/info-fun.exp: SEP: info fun foo

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFA] massively speed up "info var foo" on large programs

[dje]

Andreas Schwab writes:
 > Doug Evans <dje@google.com> writes:
 > 
 > > +    set match_str {All functions matching regular expression "foo":[\r\n]*}
 > > +    if { "$libsepdebug" != "NO"  } {
 > > +	append match_str {File .*/info-fun-solib[.]c:[\r\n]*}
 > > +	append match_str {int foo\(void\);[\r\n]*}
 > > +    }
 > > +    append match_str {Non-debugging symbols:[\r\n]*}
 > > +    append match_str "$hex *foo(@plt)?\[\r\n\]*"
 > > +    if { "$libsepdebug" == "NO"  } {
 > > +	append match_str "$hex *foo\[\r\n\]*"
 > > +    }
 > > +
 > > +    gdb_test "info fun foo" "$match_str"
 > > +}}
 > 
 > (gdb) info fun foo
 > All functions matching regular expression "foo":
 > 
 > Non-debugging symbols:
 > 0x0000000010000578  00000011.plt_call.foo+0
 > 0x00000000100008e8  foo@plt
 > 0x00000fffb7fae73c  .foo
 > (gdb) FAIL: gdb.base/info-fun.exp: NO: info fun foo
 > 
 > (gdb) info fun foo
 > All functions matching regular expression "foo":
 > 
 > File ./gdb.base/info-fun-solib.c:
 > int foo(void);
 > 
 > Non-debugging symbols:
 > 0x0000000010000578  00000011.plt_call.foo+0
 > 0x00000000100008e8  foo@plt
 > (gdb) FAIL: gdb.base/info-fun.exp: IN: info fun foo
 > 
 > (gdb) info fun foo
 > All functions matching regular expression "foo":
 > 
 > File ./gdb.base/info-fun-solib.c:
 > int foo(void);
 > 
 > Non-debugging symbols:
 > 0x0000000010000578  00000011.plt_call.foo+0
 > 0x00000000100008e8  foo@plt
 > (gdb) FAIL: gdb.base/info-fun.exp: SEP: info fun foo
 > 
 > Andreas.
 > 
 > -- 
 > Andreas Schwab, schwab@linux-m68k.org
 > GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
 > "And now for something completely different."

Hi.

I'm not sure how much effort to put into handling all the variations here.
I'm happy with this, but I'm also happy with anything that fixes your regression.

This is regression tested on amd64-linux.
Can you try this on m68k-linux?  Thanks.

2012-07-30  Doug Evans  <dje@google.com>

	* gdb.base/info-fun.exp: Fix failures on m68k-linux.

Index: info-fun.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/info-fun.exp,v
retrieving revision 1.2
diff -u -p -r1.2 info-fun.exp
--- info-fun.exp	21 Jun 2012 20:46:22 -0000	1.2
+++ info-fun.exp	30 Jul 2012 17:23:47 -0000
@@ -67,9 +67,16 @@ foreach libsepdebug {NO IN SEP} { with_t
 	append match_str {int foo\(void\);[\r\n]*}
     }
     append match_str {Non-debugging symbols:[\r\n]*}
+    # Note: Targets like m68k-linux also have, e.g., 00000011.plt_call.foo+0.
+    set plt_foo_match "($hex \[^\r\n\]*plt\[^\r\n\]*foo\[^\r\n\]*\[\r\n\]*)?"
+    append match_str $plt_foo_match
+    # This text we want to match more precisely.
     append match_str "$hex *foo(@plt)?\[\r\n\]*"
+    # Watch for again to not have to worry about the order of appearance.
+    append match_str $plt_foo_match
     if { "$libsepdebug" == "NO"  } {
-	append match_str "$hex *foo\[\r\n\]*"
+	# Note: The ".?" is for targets like m68k-linux that have ".foo" here.
+	append match_str "$hex *.?foo\[\r\n\]*"
     }
 
     gdb_test "info fun foo" "$match_str"

Re: [RFA] massively speed up "info var foo" on large programs

[Sergio Durigan Junior]

On Monday, July 30 2012, dje@google.com wrote:

> I'm not sure how much effort to put into handling all the variations here.
> I'm happy with this, but I'm also happy with anything that fixes your regression.
>
> This is regression tested on amd64-linux.
> Can you try this on m68k-linux?  Thanks.

I was seeing the same failures as Andreas on PPC64 and s390x (in fact I
was working on a patch for it now, but you were faster), when I
regtested 7.4 against 7.5 on RHEL 6.3.

I have just tested your patch on ppc64 and it works OK; I don't have a
s390x machine right now but I'll check tomorrow when I wake up.

> 2012-07-30  Doug Evans  <dje@google.com>
>
> 	* gdb.base/info-fun.exp: Fix failures on m68k-linux.

Given that these failures also happen on ppc64 and s390x due to the same
reason, I believe this and the other sentences specifically mentioning
m68k-linux can be more generic.

I also vote for committing this in the 7.5 branch.

Thanks,

-- 
Sergio

Re: [RFA] massively speed up "info var foo" on large programs

[Sergio Durigan Junior]

On Tuesday, July 31 2012, I wrote:

> I have just tested your patch on ppc64 and it works OK; I don't have a
> s390x machine right now but I'll check tomorrow when I wake up.

FWIW the patch also works OK on s390x RHEL 6.3.

-- 
Sergio

Re: [RFA] massively speed up "info var foo" on large programs

[dje]

Sergio Durigan Junior writes:
 > On Tuesday, July 31 2012, I wrote:
 > 
 > > I have just tested your patch on ppc64 and it works OK; I don't have a
 > > s390x machine right now but I'll check tomorrow when I wake up.
 > 
 > FWIW the patch also works OK on s390x RHEL 6.3.
 > 
 > -- 
 > Sergio

Thanks.

How about this?

I will check it into cvs head and 7.5 in a few days if there are no objections.

2012-08-01  Doug Evans  <dje@google.com>

	* gdb.base/info-fun.exp: Fix failures on m68k, ppc64, s390x.

Index: info-fun.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/info-fun.exp,v
retrieving revision 1.2
diff -u -p -r1.2 info-fun.exp
--- info-fun.exp	21 Jun 2012 20:46:22 -0000	1.2
+++ info-fun.exp	1 Aug 2012 18:34:44 -0000
@@ -67,9 +67,17 @@ foreach libsepdebug {NO IN SEP} { with_t
 	append match_str {int foo\(void\);[\r\n]*}
     }
     append match_str {Non-debugging symbols:[\r\n]*}
+    # Note: Targets like {m68k,ppc64,s390x}-linux also have, e.g.,
+    # 00000011.plt_call.foo+0 (m68k).
+    set plt_foo_match "($hex \[^\r\n\]*plt\[^\r\n\]*foo\[^\r\n\]*\[\r\n\]*)?"
+    append match_str $plt_foo_match
+    # This text we want to match precisely.
     append match_str "$hex *foo(@plt)?\[\r\n\]*"
+    # Watch for again to not have to worry about the order of appearance.
+    append match_str $plt_foo_match
     if { "$libsepdebug" == "NO"  } {
-	append match_str "$hex *foo\[\r\n\]*"
+	# Note: The ".?" is for targets like m68k-linux that have ".foo" here.
+	append match_str "$hex *.?foo\[\r\n\]*"
     }
 
     gdb_test "info fun foo" "$match_str"