[patch]: User choice for multiply-defined symbols

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

Hello,

the following patch makes use of the new "multiple-symbols" command introduced by Joel.

If there is more than one symbol associated with a name and the multiple-symbols flag is set
to "ask", the user is asked to choose which of the symbols 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
> 3
Breakpoint 6 at 0x100004b0: file ../../../../src/gdb/testsuite/gdb.base/multiple_symbols.c, line 14.

I extended "search_symbols" by a distinct flag. If set the new function "distinct_search_syms" is called
before returning the search result. As the name says, this function removes duplicates from the search result.

For exmaple:

If GDB debugs a threaded application with a symbol "foo" in every thread, there would be a user choice like:

[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_mod.c:5
[4] foo at ../../../../src/gdb/testsuite/gdb.base/multiple_symbols_mod.c:5

If the user then chooses for example 2, a new breakpoint for "multiple_symbols_mod.c:5" would be set, which results
in a multi-location breakpoint in every thread. This is not such a big problem, but if the user chooses all symbols,
a multi-location breakpoint is created for each of the symbols above :-) This results in n*n breakpoints.

This patch also contains a new testcase to cover this functionality. I'd like to hear your opinion about this feature.
Is it ok for mainline?


ChangeLog:

	* linespec.c (decode_variable): Allow user to choose multiply-defined
	symbols if found.
	* symtab.c (symbol_search_equal, distinct_search_syms): New function.
	(search_symbols): Add new parameter distinct. Call distinct_search_syms
	if set. Update caller.
	* symtab.h (search_symbols): Add new parameter distinct.

ChangeLog Testsuite:

	* gdb.base/multiple_symbols.exp: New testcase for multiply-definded
	symbols.
	* gdb.base/multiple_symbols.c: Likewise.
	* gdb.base/multiple_symbols_mod.c: Likewise.

Regards,
Markus

--
 Markus Deuling
 GNU Toolchain for Linux on Cell BE
 deuling@de.ibm.com

diff -urpN src-orig/gdb/linespec.c src/gdb/linespec.c
--- src-orig/gdb/linespec.c	2008-04-28 07:51:31.000000000 +0200
+++ src/gdb/linespec.c	2008-05-05 09:38:45.000000000 +0200
@@ -1734,9 +1734,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,9 +1745,41 @@ decode_variable (char *copy, int funfirs
   struct symbol *sym;
   /* The symtab that SYM was found in.  */
   struct symtab *sym_symtab;
-
   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),
diff -urpN src-orig/gdb/symtab.c src/gdb/symtab.c
--- src-orig/gdb/symtab.c	2008-04-28 07:51:31.000000000 +0200
+++ src/gdb/symtab.c	2008-04-30 10:59:00.000000000 +0200
@@ -2898,6 +2899,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
@@ -2959,10 +3001,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;
@@ -3254,6 +3298,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
@@ -3329,7 +3375,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
@@ -3398,7 +3444,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-orig/gdb/symtab.h src/gdb/symtab.h
--- src-orig/gdb/symtab.h	2008-04-28 07:51:31.000000000 +0200
+++ src/gdb/symtab.h	2008-04-30 09:55:00.000000000 +0200
@@ -1392,7 +1392,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-orig/gdb/testsuite/gdb.base/multiple_symbols.c src/gdb/testsuite/gdb.base/multiple_symbols.c
--- src-orig/gdb/testsuite/gdb.base/multiple_symbols.c	1970-01-01 01:00:00.000000000 +0100
+++ src/gdb/testsuite/gdb.base/multiple_symbols.c	2008-05-05 08:03:18.000000000 +0200
@@ -0,0 +1,14 @@
+#include <stdio.h>
+void foo ();
+
+int main ()
+{
+  foo ();
+  return 0;
+}
+
+void
+foo ()
+{
+  return;
+}
diff -urpN src-orig/gdb/testsuite/gdb.base/multiple_symbols.exp src/gdb/testsuite/gdb.base/multiple_symbols.exp
--- src-orig/gdb/testsuite/gdb.base/multiple_symbols.exp	1970-01-01 01:00:00.000000000 +0100
+++ src/gdb/testsuite/gdb.base/multiple_symbols.exp	2008-05-05 09:25:00.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-orig/gdb/testsuite/gdb.base/multiple_symbols_mod.c src/gdb/testsuite/gdb.base/multiple_symbols_mod.c
--- src-orig/gdb/testsuite/gdb.base/multiple_symbols_mod.c	1970-01-01 01:00:00.000000000 +0100
+++ src/gdb/testsuite/gdb.base/multiple_symbols_mod.c	2008-05-05 08:03:26.000000000 +0200
@@ -0,0 +1,5 @@
+static void
+foo ()
+{
+  return;
+}