ping [Fwd: [RFC] User choice for multiply-defined symbols]

[Markus Deuling <deuling at de dot ibm dot com>]

Hi,

any opinions on this patch?

Regards,
Markus

-------- Original-Nachricht --------
Betreff: [RFC] User choice for multiply-defined symbols
Datum: Thu, 14 Aug 2008 17:34:45 +0200
Von: Markus Deuling <deuling@de.ibm.com>
An: GDB Patches <gdb-patches@sourceware.org>
CC: Ulrich Weigand <uweigand@de.ibm.com>

Hi,

I'd like to continue the discussion about a patch I began here: http://sourceware.org/ml/gdb-patches/2008-05/msg00190.html

When looking up a symbol (for example invoked by "break foo") GDB returns the first symbol it finds.  The 
search normally begins in the main executable (there is a special case when an application was built using 
the -Bsymbolic linker option.  Then the lookup prefers the symbol definition in the current shared library ).

Multi-location breakpoints handle the case where a single instance of a function (at a particular source file/line)
is implemented at multiple locations in the object file(s).  This new feature is intended to handle the case where 
we have *multiple* definitions of functions with the same name, so that just specifying the name does not fully identify 
the target function.  With this patch, GDB would query the user which of those definitions was meant.

What I try to achieve with this patch is to do a complete lookup in all object file(s) as well.  If this 
search finds more than one symbol "foo" like seen in the following example, GDB should let the user choose, which
one (or even more than one symbol) to use.

(gdb) break foo
[0] cancel
[1] all
[2] foo at ../../../../src/gdb/testsuite/gdb.base/multiple_symbols_mod.c:5
[3] foo at ../../../../src/gdb/testsuite/gdb.base/multiple_symbols.c:14

GDB's default behaviour isn't changed by the patch. User has to "set multiple-symbols ask" to invoke the 
search.  If > 1 symbols are found, the user is given the choice as seen above.

Attached you find the rebased patch. I'd like to hear your opinion about this patch.

Regards,
Markus

diff -urpN src/gdb/linespec.c dev/gdb/linespec.c
--- src/gdb/linespec.c	2008-05-24 01:39:03.000000000 +0200
+++ dev/gdb/linespec.c	2008-08-13 18:17:40.000000000 +0200
@@ -1733,9 +1733,9 @@ decode_dollar (char *copy, int funfirstl
 
 
 /* Decode a linespec that's a variable.  If FILE_SYMTAB is non-NULL,
-   look in that symtab's static variables first.  If NOT_FOUND_PTR is not NULL and
-   the function cannot be found, store boolean true in the location pointed to
-   and do not issue an error message.  */ 
+   look in that symtab's static variables first.  If NOT_FOUND_PTR is not NULL
+   and the function cannot be found, store boolean true in the location pointed 
+   to and do not issue an error message.  */ 
 
 static struct symtabs_and_lines
 decode_variable (char *copy, int funfirstline, char ***canonical,
@@ -1745,6 +1745,39 @@ decode_variable (char *copy, int funfirs
 
   struct minimal_symbol *msymbol;
 
+  if (!file_symtab && multiple_symbols_select_mode () == multiple_symbols_ask)
+    {
+      int nelts = 0; 
+      struct symbol_search *symbols, *p;
+      struct cleanup *chain;
+      struct symbol **sym_arr;
+      char *regexp = xmalloc (strlen (copy) + 3); 
+ 
+      /* Perform a distinct search that matches exactly COPY.  */
+      xsnprintf (regexp, strlen (copy) + 3, "^%s$", copy);
+      search_symbols (regexp, FUNCTIONS_DOMAIN, 0, (char **) NULL, &symbols, 1);
+      xfree (regexp);
+      chain = make_cleanup_free_search_symbols (symbols);
+ 
+      for (p = symbols; p != NULL && p->symbol; p = p->next, nelts++);
+      if (nelts > 1)
+ 	{
+ 	  int idx = 0;
+ 	  struct symtabs_and_lines result;
+ 
+ 	  sym_arr =  xmalloc ((nelts) * sizeof (struct symbol *));
+ 	  make_cleanup (xfree, sym_arr);
+ 
+ 	  for (p = symbols; p != NULL; p = p->next)
+ 	    if (p->symbol)
+ 	      sym_arr[idx++] = p->symbol;
+ 	  result = decode_line_2 (sym_arr, idx, funfirstline, canonical);
+ 	  do_cleanups (chain);
+ 	  return result;
+ 	}
+       do_cleanups (chain);
+     }
+ 
   sym = lookup_symbol (copy,
 		       (file_symtab
 			? BLOCKVECTOR_BLOCK (BLOCKVECTOR (file_symtab),
Binary files src/gdb/.linespec.c.swp and dev/gdb/.linespec.c.swp differ
diff -urpN src/gdb/symtab.c dev/gdb/symtab.c
--- src/gdb/symtab.c	2008-07-21 18:47:10.000000000 +0200
+++ dev/gdb/symtab.c	2008-08-13 18:14:07.000000000 +0200
@@ -2930,6 +2930,47 @@ make_cleanup_free_search_symbols (struct
   return make_cleanup (do_free_search_symbols_cleanup, symbols);
 }
 
+/* Returns TRUE if SYM1 == SYM2.  This is true if the symbol name, the symbol's
+   associated file name and line no are equal.  */
+static int
+symbol_search_equal (struct symbol_search *sym1, struct symbol_search *sym2)
+{
+  return (!strcmp (SYMBOL_PRINT_NAME (sym1->symbol),
+		   SYMBOL_PRINT_NAME (sym2->symbol))
+	  && (sym1->symbol->line == sym2->symbol->line)
+	  && (!strcmp (sym1->symtab->filename, sym2->symtab->filename)));
+}
+
+/* Helper function for search results.  This function ensures that only
+   distinct symbols are left in SYMBOLS.  Every duplicate is being deleted.  */
+static void
+distinct_search_syms (struct symbol_search *symbols)
+{
+  struct symbol_search *p;
+
+  for (p = symbols; p != NULL; p = p->next)
+    {
+      struct symbol_search *next, *prev = p, *p2 = p->next;
+
+     /* Check if current p has a duplicate in the following part of the search
+	result.  If so delete it.  */
+      for (; p2 != NULL; p2 = next)
+	{
+	  next = p2->next;
+	  if (symbol_search_equal (p, p2))
+	    {
+	      prev->next = p2->next;
+	      xfree (p2); 
+	    }
+	  else
+	    prev = prev->next;
+	}      
+    }
+
+  return;
+}
+
+
 /* Helper function for sort_search_symbols and qsort.  Can only
    sort symbols, not minimal symbols.  */
 static int
@@ -2991,10 +3032,12 @@ sort_search_symbols (struct symbol_searc
 
    The results are sorted locally; each symtab's global and static blocks are
    separately alphabetized.
- */
+
+   If DISTINCT then the search result is distinct, which means there are no
+   duplicates.  */
 void
 search_symbols (char *regexp, domain_enum kind, int nfiles, char *files[],
-		struct symbol_search **matches)
+		struct symbol_search **matches, int distinct)
 {
   struct symtab *s;
   struct partial_symtab *ps;
@@ -3285,6 +3328,8 @@ search_symbols (char *regexp, domain_enu
   *matches = sr;
   if (sr != NULL)
     discard_cleanups (old_chain);
+  if (distinct)
+    distinct_search_syms (*matches);
 }
 
 /* Helper function for symtab_symbol_info, this function uses
@@ -3360,7 +3405,7 @@ symtab_symbol_info (char *regexp, domain
   int first = 1;
 
   /* must make sure that if we're interrupted, symbols gets freed */
-  search_symbols (regexp, kind, 0, (char **) NULL, &symbols);
+  search_symbols (regexp, kind, 0, (char **) NULL, &symbols, 0);
   old_chain = make_cleanup_free_search_symbols (symbols);
 
   printf_filtered (regexp
@@ -3429,7 +3474,7 @@ rbreak_command (char *regexp, int from_t
   struct symbol_search *p;
   struct cleanup *old_chain;
 
-  search_symbols (regexp, FUNCTIONS_DOMAIN, 0, (char **) NULL, &ss);
+  search_symbols (regexp, FUNCTIONS_DOMAIN, 0, (char **) NULL, &ss, 0);
   old_chain = make_cleanup_free_search_symbols (ss);
 
   for (p = ss; p != NULL; p = p->next)
diff -urpN src/gdb/symtab.h dev/gdb/symtab.h
--- src/gdb/symtab.h	2008-05-27 21:29:51.000000000 +0200
+++ dev/gdb/symtab.h	2008-08-13 18:14:07.000000000 +0200
@@ -1312,7 +1312,7 @@ struct symbol_search
 };
 
 extern void search_symbols (char *, domain_enum, int, char **,
-			    struct symbol_search **);
+			    struct symbol_search **, int);
 extern void free_search_symbols (struct symbol_search *);
 extern struct cleanup *make_cleanup_free_search_symbols (struct symbol_search
 							 *);
diff -urpN src/gdb/testsuite/gdb.base/multiple_symbols.c dev/gdb/testsuite/gdb.base/multiple_symbols.c
--- src/gdb/testsuite/gdb.base/multiple_symbols.c	1970-01-01 01:00:00.000000000 +0100
+++ dev/gdb/testsuite/gdb.base/multiple_symbols.c	2008-08-13 18:14:07.000000000 +0200
@@ -0,0 +1,14 @@
+#include <stdio.h>
+void foo ();
+
+int main ()
+{
+  foo ();
+  return 0;
+}
+
+void
+foo ()
+{
+  return;
+}
diff -urpN src/gdb/testsuite/gdb.base/multiple_symbols.exp dev/gdb/testsuite/gdb.base/multiple_symbols.exp
--- src/gdb/testsuite/gdb.base/multiple_symbols.exp	1970-01-01 01:00:00.000000000 +0100
+++ dev/gdb/testsuite/gdb.base/multiple_symbols.exp	2008-08-13 18:14:07.000000000 +0200
@@ -0,0 +1,110 @@
+#   Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999,
+#   2000, 2002, 2003, 2007, 2008 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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# This file was contributed by Markus Deuling (deuling@de.ibm.com)
+
+set testfile1 "multiple_symbols"
+set testfile2 "multiple_symbols_mod"
+set srcfile1 ${srcdir}/${subdir}/${testfile1}.c
+set srcfile2 ${srcdir}/${subdir}/${testfile2}.c
+set binfile ${objdir}/${subdir}/${testfile1}
+
+if [get_compiler_info ${binfile}] {
+    return -1;
+}
+
+
+if { [gdb_compile "${srcfile1} ${srcfile2}" "${binfile}" executable debug ] != "" } {
+     untested multiple_symbols.exp
+     return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+delete_breakpoints
+gdb_test "break foo" \
+    "Breakpoint.*at.* file .*$srcfile1, line.*" \
+    "break foo"
+
+gdb_test "set multiple-symbols ask" "" ""
+
+# Cancel selection.
+delete_breakpoints
+gdb_test_multiple "break foo" "break foo" {
+  -re ".*0.*cancel.*1.*all.*2.*foo at.*$srcfile2.*3.*foo at.*$srcfile1.*>.*" {
+    gdb_test "0" ".*canceled.*" "cancel"
+  }
+}
+
+# Choose all possible breakpoints.
+delete_breakpoints
+gdb_test_multiple "break foo" "break foo" {
+  -re ".*0.*cancel.*1.*all.*2.*foo at.*$srcfile2.*3.*foo at.*$srcfile1.*>.*" {
+    gdb_test "1" "Breakpoint.*at.* file .*$srcfile2, line.*Breakpoint.*at.* file .*$srcfile1, line.*" "break all"
+  }
+}
+
+# Break on a single symbol.
+delete_breakpoints
+gdb_test_multiple "break foo" "break foo" {
+  -re ".*0.*cancel.*1.*all.*2.*foo at.*$srcfile2.*3.*foo at.*$srcfile1.*>.*" {
+    gdb_test "2" "Breakpoint.*at.* file .*$srcfile2, line.*" "break 2"
+  }
+}
+
+# Break on the other symbol.
+delete_breakpoints
+gdb_test_multiple "break foo" "break foo" {
+  -re ".*0.*cancel.*1.*all.*2.*foo at.*$srcfile2.*3.*foo at.*$srcfile1.*>.*" {
+    gdb_test "3" "Breakpoint.*at.* file .*$srcfile1, line.*" "break 3"
+  }
+}
+
+# Choose two symbols for creating breakpoints.
+delete_breakpoints
+gdb_test_multiple "break foo" "break foo" {
+  -re ".*0.*cancel.*1.*all.*2.*foo at.*$srcfile2.*3.*foo at.*$srcfile1.*>.*" {
+    gdb_test "2 3" "Breakpoint.*at.* file .*$srcfile2, line.*Breakpoint.*at.* file .*$srcfile1, line.*" "break 2 3"
+  }
+}
+
+# Choose a non-existing symbol.
+delete_breakpoints
+gdb_test_multiple "break foo" "break foo" {
+  -re ".*0.*cancel.*1.*all.*2.*foo at.*$srcfile2.*3.*foo at.*$srcfile1.*>.*" {
+    gdb_test "4" ".*No choice number 4.*" "break non-existing"
+  }
+}
+
+# Enter a non-numeric choice.
+delete_breakpoints
+gdb_test_multiple "break foo" "break foo" {
+  -re ".*0.*cancel.*1.*all.*2.*foo at.*$srcfile2.*3.*foo at.*$srcfile1.*>.*" {
+    gdb_test "a" ".*Arguments must be choice numbers.*" "break non-numeric choice"
+  }
+}
+
diff -urpN src/gdb/testsuite/gdb.base/multiple_symbols_mod.c dev/gdb/testsuite/gdb.base/multiple_symbols_mod.c
--- src/gdb/testsuite/gdb.base/multiple_symbols_mod.c	1970-01-01 01:00:00.000000000 +0100
+++ dev/gdb/testsuite/gdb.base/multiple_symbols_mod.c	2008-08-13 18:14:07.000000000 +0200
@@ -0,0 +1,5 @@
+static void
+foo ()
+{
+  return;
+}