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

[Doug Evans <dje@google.com>]

[-- 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"
+}}