public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] c++/11734 revisited
@ 2010-12-09  0:50 Keith Seitz
  2010-12-09  4:02 ` Eli Zaretskii
  2010-12-09 21:45 ` Tom Tromey
  0 siblings, 2 replies; 29+ messages in thread
From: Keith Seitz @ 2010-12-09  0:50 UTC (permalink / raw)
  To: gdb-patches

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

Hi,

Refresher: This is the bug that relates to psymtabs not having overload 
information and how gdb can't find the right psymtab to expand.

I originally proposed fixing this by switching to pre-expansion. Tom 
reports that this really mucks up indexing. As a result, I have 
rewritten this patch.

I *think* I have addressed almost all of the original issues that were 
reported. One that I have not addressed is Doug's complaint about 
removing the single-quote in decode_compound. Quite frankly, the 
single/double quote handling is a mess. It's such a nightmare, it would 
actually a primary reason to rewrite linespec.c entirely. One of these 
days, I'll find the time to do it. [Although with the countless hours 
I've spent on linespec.c in the past year, I'm sure I could have 
rewritten it three times already.]

This patch also fixes Jan's expand-psymtabs-cxx test which he posted on 
11/21.

Yes, I realize this is actually two patches, one for psymtab.c and one 
for linespec.c, but the linespec.c patch is really small this time and 
the tests for both are in the same file (because that's how I discovered 
the linespec bug).

Tested on x86_64-linux. No regressions.

Comments?

Keith

ChangeLog
2010-12-08  Keith Seitz  <keiths@redhat.com>

	* linespec.c (decode_compound): Rename SAVED_ARG to
	THE_REAL_SAVED_ARG.
	Make a copy of THE_REAL_SAVED_ARG in SAVED_ARG and strip
	single-quotes.

	* psymtab.c (lookup_symbol_aux_psymtabs): Don't assume that
	the psymtab we found was the right one. Search for the desired
	symbol in the symtab to be certain.
	(psymtab_search_name): New function.
	(lookup_partial_symbol): Use psymtab_search_name.

testsuite/ChangeLog
2010-12-08  Keith Seitz  <keiths@redhat.com>

	* gdb.cp/pr11734.exp: New test.
	* gdb.cp/pr11734.h: New file.
	* gdb.cp/pr11734-1.cc: New file.
	* gdb.cp/pr11734-2.cc: New file.
	* gdb.cp/pr11734-3.cc: New file.
	* gdb.cp/pr11734-4.cc: New file.

[-- Attachment #2: pr11734-4.patch --]
[-- Type: text/plain, Size: 14475 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.105
diff -u -p -r1.105 linespec.c
--- linespec.c	2 Dec 2010 20:05:59 -0000	1.105
+++ linespec.c	9 Dec 2010 00:49:09 -0000
@@ -1219,7 +1219,7 @@ decode_objc (char **argptr, int funfirst
 
 static struct symtabs_and_lines
 decode_compound (char **argptr, int funfirstline, char ***canonical,
-		 char *saved_arg, char *p, int *not_found_ptr)
+		 char *the_real_saved_arg, char *p, int *not_found_ptr)
 {
   struct symtabs_and_lines values;
   char *p2;
@@ -1230,7 +1230,23 @@ decode_compound (char **argptr, int funf
   struct symbol *sym_class;
   struct type *t;
   char *saved_java_argptr = NULL;
+  char *saved_arg;
 
+  /* THE_REAL_SAVED_ARG cannot be altered, so make a copy that can be.  */
+  saved_arg = alloca (strlen (the_real_saved_arg) + 1);
+  strcpy (saved_arg, the_real_saved_arg);
+
+  /* If the user specified "'foo::bar(baz)'" (note the qutoes -- often
+     added to workaround completer issues) -- saved_arg will be
+     encapsulated in single-quotes.  They are superfluous, so just strip
+     them off.  */
+  if (*saved_arg == '\'')
+    {
+      char *end = skip_quoted (saved_arg);
+      memcpy (saved_arg, saved_arg + 1, end - saved_arg);
+      memcpy (end - 2, end, strlen (saved_arg) + 1);
+    }
+      
   /* First check for "global" namespace specification, of the form
      "::foo".  If found, skip over the colons and jump to normal
      symbol processing.  I.e. the whole line specification starts with
@@ -1481,7 +1497,7 @@ decode_compound (char **argptr, int funf
      up.  The quotes are important if copy is empty.  */
   if (not_found_ptr)
     *not_found_ptr = 1;
-  cplusplus_error (saved_arg,
+  cplusplus_error (the_real_saved_arg,
 		   "Can't find member of namespace, class, struct, or union named \"%s\"\n",
 		   copy);
 }
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.19
diff -u -p -r1.19 psymtab.c
--- psymtab.c	24 Nov 2010 19:01:51 -0000	1.19
+++ psymtab.c	9 Dec 2010 00:49:10 -0000
@@ -33,6 +33,8 @@
 #include "readline/readline.h"
 #include "gdb_regex.h"
 #include "dictionary.h"
+#include "language.h"
+#include "cp-support.h"
 
 #ifndef DEV_TTY
 #define DEV_TTY "/dev/tty"
@@ -426,7 +428,26 @@ lookup_symbol_aux_psymtabs (struct objfi
   ALL_OBJFILE_PSYMTABS (objfile, ps)
   {
     if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
+      {
+	struct symbol *sym;
+	struct symtab *stab = PSYMTAB_TO_SYMTAB (ps);
+	sym = NULL;
+
+	/* Some caution must be observed with overloaded functions
+	   and methods, since the psymtab will not contain any overload
+	   information (but NAME might contain it).  */
+	if (stab->primary)
+	  {
+	    struct blockvector *bv = BLOCKVECTOR (stab);
+	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
+	    sym = lookup_block_symbol (block, name, domain);
+	  }
+
+	if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
+	  return stab;
+
+	/* Keep looking through other psymtabs.  */
+      }
   }
 
   return NULL;
@@ -519,6 +540,37 @@ pre_expand_symtabs_matching_psymtabs (st
   /* Nothing.  */
 }
 
+/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
+   not contain any method/function instance information (since this would
+   force reading type information while reading psymtabs).  Therefore,
+   if NAME contains overload information, it must be stripped before searching
+   psymtabs.
+
+   The caller is responsible for freeing the return result.  */
+
+static const char *
+psymtab_search_name (const char *name)
+{
+  switch (current_language->la_language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+       if (strchr (name, '('))
+         {
+           char *ret = cp_remove_params (name);
+           if (ret)
+             return ret;
+         }
+      }
+
+    default:
+      break;
+    }
+
+  return xstrdup (name);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    Check the global symbols if GLOBAL, the static symbols if not.  */
 
@@ -530,11 +582,16 @@ lookup_partial_symbol (struct partial_sy
   struct partial_symbol **top, **real_top, **bottom, **center;
   int length = (global ? pst->n_global_syms : pst->n_static_syms);
   int do_linear_search = 1;
+  const char *search_name;
+  struct cleanup *cleanup;
 
   if (length == 0)
     {
       return (NULL);
     }
+
+  search_name = psymtab_search_name (name);
+  cleanup = make_cleanup (xfree, (void *) search_name);
   start = (global ?
 	   pst->objfile->global_psymbols.list + pst->globals_offset :
 	   pst->objfile->static_psymbols.list + pst->statics_offset);
@@ -562,7 +619,8 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
+				 search_name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -575,11 +633,14 @@ lookup_partial_symbol (struct partial_sy
 	internal_error (__FILE__, __LINE__, _("failed internal consistency check"));
 
       while (top <= real_top
-	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
 				     SYMBOL_DOMAIN (*top), domain))
-	    return (*top);
+	    {
+	      do_cleanups (cleanup);
+	      return (*top);
+	    }
 	  top++;
 	}
     }
@@ -594,10 +655,14 @@ lookup_partial_symbol (struct partial_sy
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
 				     SYMBOL_DOMAIN (*psym), domain)
 	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
-	    return (*psym);
+	    {
+	      do_cleanups (cleanup);
+	      return (*psym);
+	    }
 	}
     }
 
+  do_cleanups (cleanup);
   return (NULL);
 }
 
Index: testsuite/gdb.cp/pr11734-1.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-1.cc
diff -N testsuite/gdb.cp/pr11734-1.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-1.cc	9 Dec 2010 00:49:10 -0000
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+int
+main ()
+{
+  pr11734 *p = new pr11734;
+  p->foo ();
+  return 0;
+}
+
Index: testsuite/gdb.cp/pr11734-2.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-2.cc
diff -N testsuite/gdb.cp/pr11734-2.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-2.cc	9 Dec 2010 00:49:10 -0000
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+void
+pr11734::foo(void)
+{
+}
+
Index: testsuite/gdb.cp/pr11734-3.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-3.cc
diff -N testsuite/gdb.cp/pr11734-3.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-3.cc	9 Dec 2010 00:49:10 -0000
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+void
+pr11734::foo (int a)
+{
+}
+
Index: testsuite/gdb.cp/pr11734-4.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-4.cc
diff -N testsuite/gdb.cp/pr11734-4.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-4.cc	9 Dec 2010 00:49:10 -0000
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+void
+pr11734::foo (char *a)
+{
+}
+
Index: testsuite/gdb.cp/pr11734.exp
===================================================================
RCS file: testsuite/gdb.cp/pr11734.exp
diff -N testsuite/gdb.cp/pr11734.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734.exp	9 Dec 2010 00:49:10 -0000
@@ -0,0 +1,67 @@
+# Copyright 2010 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+if { [skip_cplus_tests] } { continue }
+
+set testfile "pr11734"
+set class $testfile
+set binfile [file join $objdir $subdir $testfile]
+
+set srcfiles {}
+for {set i 1} {$i < 5} {incr i} {
+    lappend srcfiles [file join $srcdir $subdir $testfile-$i.cc]
+}
+if  {[gdb_compile $srcfiles $binfile executable {debug c++}] != "" } {
+     untested pr11734.exp
+     return -1
+}
+
+if {[get_compiler_info $binfile "c++"]} {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+
+if {![runto_main]} {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# An array holding the overload types for the method pr11734::foo.  The
+# first element is the overloaded method parameter.  The second element
+# is the expected source file number, e.g. "pr11734-?.cc".
+array set tests {
+    "char*"  4
+    "int"    3
+    ""       2
+}
+
+# Test each overload instance twice: once quoted, once unquoted
+foreach ovld [array names tests] {
+    set method "${class}::foo\($ovld\)"
+    set result "Breakpoint (\[0-9\]).*file .*/$class-$tests($ovld).*"
+    gdb_test "break $method" $result
+    gdb_test "break '$method'" $result
+}
+
+gdb_exit
+return 0
Index: testsuite/gdb.cp/pr11734.h
===================================================================
RCS file: testsuite/gdb.cp/pr11734.h
diff -N testsuite/gdb.cp/pr11734.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734.h	9 Dec 2010 00:49:10 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+class pr11734
+{
+ public:
+  void foo ();
+  void foo (int);
+  void foo (char *);
+};
+

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2010-12-09  0:50 [RFA] c++/11734 revisited Keith Seitz
@ 2010-12-09  4:02 ` Eli Zaretskii
  2010-12-09 21:45 ` Tom Tromey
  1 sibling, 0 replies; 29+ messages in thread
From: Eli Zaretskii @ 2010-12-09  4:02 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

> Date: Wed, 08 Dec 2010 16:44:34 -0800
> From: Keith Seitz <keiths@redhat.com>
> 
> +  /* If the user specified "'foo::bar(baz)'" (note the qutoes -- often
                                                          ^^^^^^
A typo.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2010-12-09  0:50 [RFA] c++/11734 revisited Keith Seitz
  2010-12-09  4:02 ` Eli Zaretskii
@ 2010-12-09 21:45 ` Tom Tromey
  2010-12-09 21:52   ` Jan Kratochvil
  2010-12-14 20:03   ` Keith Seitz
  1 sibling, 2 replies; 29+ messages in thread
From: Tom Tromey @ 2010-12-09 21:45 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> I originally proposed fixing this by switching to pre-expansion. Tom
Keith> reports that this really mucks up indexing. As a result, I have
Keith> rewritten this patch.

I think I must have been unclear.

The problem with using pre-expansion in the index code was that the
index didn't include all the same info as the psymtabs, so there was no
good way to avoid expanding too many CUs when asked to look up a common
type like "int".

I don't think this problem would affect psymtab.c.  So, you could
probably still use pre-expansion if you think that is better.

And, true to form, I misremembered the many different attempts at fixing
this that I went through -- apparently in the tree we currently do use
pre-expansion for the index.

Sorry about all that.

Keith> It's such a nightmare, it would actually a primary reason to
Keith> rewrite linespec.c entirely. One of these days, I'll find the
Keith> time to do it.

That will be a happy day.  For us anyway ;-)

Keith> 2010-12-08  Keith Seitz  <keiths@redhat.com>
Keith>	* linespec.c (decode_compound): Rename SAVED_ARG to
Keith>	THE_REAL_SAVED_ARG.

The ChangeLog entry should mention the PR, so the commit will end up in
bugzilla.

Keith> +  if (*saved_arg == '\'')
Keith> +    {
Keith> +      char *end = skip_quoted (saved_arg);
Keith> +      memcpy (saved_arg, saved_arg + 1, end - saved_arg);
Keith> +      memcpy (end - 2, end, strlen (saved_arg) + 1);

For overlapping copies you must use memmove.

Keith> +static const char *
Keith> +psymtab_search_name (const char *name)
Keith> +{
Keith> +  switch (current_language->la_language)

I don't think psymtab.c should refer to current_language.

I guess the search language could be a parameter.  I realize that at
some point we'll need to use current_language, and that symtab.c isn't
exactly clean in this regard.  It doesn't seem too hard to avoid letting
the rot creep into this code, though.

I suspect this same bug is going to bite the index code, too, and we'll
have to export this function for use there too.  I'll check that out
after your patch goes in.

Keith> +  return xstrdup (name);

The return type of this function should be just "char *".
Otherwise you will need casts when freeing.

Keith> +  cleanup = make_cleanup (xfree, (void *) search_name);

... like this one.

Keith> +if  {[gdb_compile $srcfiles $binfile executable {debug c++}] != "" } {
Keith> +     untested pr11734.exp
Keith> +     return -1
Keith> +}

Keith> +if {[get_compiler_info $binfile "c++"]} {
Keith> +    return -1
Keith> +}
Keith> +
Keith> +gdb_exit
Keith> +gdb_start
Keith> +gdb_reinitialize_dir $srcdir/$subdir
Keith> +gdb_load $binfile

I don't know if you can use prepare_for_testing here, but
if not you can at least use clean_restart.

Tom

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2010-12-09 21:45 ` Tom Tromey
@ 2010-12-09 21:52   ` Jan Kratochvil
  2010-12-10 15:21     ` Keith Seitz
  2010-12-14 20:03   ` Keith Seitz
  1 sibling, 1 reply; 29+ messages in thread
From: Jan Kratochvil @ 2010-12-09 21:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Keith Seitz, gdb-patches

On Thu, 09 Dec 2010 22:44:55 +0100, Tom Tromey wrote:
> Keith> +if  {[gdb_compile $srcfiles $binfile executable {debug c++}] != "" } {
> Keith> +     untested pr11734.exp
> Keith> +     return -1
> Keith> +}
> 
> Keith> +if {[get_compiler_info $binfile "c++"]} {
> Keith> +    return -1
> Keith> +}
> Keith> +
> Keith> +gdb_exit
> Keith> +gdb_start
> Keith> +gdb_reinitialize_dir $srcdir/$subdir
> Keith> +gdb_load $binfile
> 
> I don't know if you can use prepare_for_testing here, but
> if not you can at least use clean_restart.

If the reason was get_compiler_info it does not use $binfile anyway.
(And a lot of gdb testsuite code expects so.)


Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2010-12-09 21:52   ` Jan Kratochvil
@ 2010-12-10 15:21     ` Keith Seitz
  0 siblings, 0 replies; 29+ messages in thread
From: Keith Seitz @ 2010-12-10 15:21 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches

On 12/09/2010 01:52 PM, Jan Kratochvil wrote:

>> Keith>  +if {[get_compiler_info $binfile "c++"]} {
>> Keith>  +    return -1
>> Keith>  +}

> If the reason was get_compiler_info it does not use $binfile anyway.
> (And a lot of gdb testsuite code expects so.)

I admit, I copy & pasted that bit from other gdb.cp tests. I'm not even 
sure why it is necessary, since we have skip_cplus_tests already. Is 
there any case where we would not have a C++ compiler and 
skip_cplus_tests != true?

For now, I've simply removed this and converted the whole thing over to 
using prepare_for_testing. I noted one or two other gdb.cp tests which 
don't check get_compiler_info.

Keith

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2010-12-09 21:45 ` Tom Tromey
  2010-12-09 21:52   ` Jan Kratochvil
@ 2010-12-14 20:03   ` Keith Seitz
  2011-01-24 18:15     ` Jan Kratochvil
                       ` (5 more replies)
  1 sibling, 6 replies; 29+ messages in thread
From: Keith Seitz @ 2010-12-14 20:03 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

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

On 12/09/2010 01:44 PM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz<keiths@redhat.com>  writes:

> I don't think this problem would affect psymtab.c.  So, you could
> probably still use pre-expansion if you think that is better.

I don't think it really matters. This patch seems simple enough to 
understand, and it works. I prefer to err on the side of least invasive. 
I fear that pre-expansion might have unforeseen consequences. I don't 
really need another dwarf2_physname! :-)

> Keith>  It's such a nightmare, it would actually a primary reason to
> Keith>  rewrite linespec.c entirely. One of these days, I'll find the
> Keith>  time to do it.
>
> That will be a happy day.  For us anyway ;-)

Or for me, since I seem to keep ending up dealing with problems in 
decode_line_1/decode_compound!


> The ChangeLog entry should mention the PR, so the commit will end up in
> bugzilla.

Yes, of course. I completely forgot. Fixed.

> For overlapping copies you must use memmove.

Fixed.

> I guess the search language could be a parameter.  I realize that at
> some point we'll need to use current_language, and that symtab.c isn't
> exactly clean in this regard.  It doesn't seem too hard to avoid letting
> the rot creep into this code, though.

I've added this parameter and pushed the language setting up a level 
into symtab.c.

> The return type of this function should be just "char *".
> Otherwise you will need casts when freeing.

Okay.

> I don't know if you can use prepare_for_testing here, but
> if not you can at least use clean_restart.

I've switched to prepare_for_testing, as I mentioned in my follow-up to 
Jan's message.

Revised patch attached.

Keith

ChangeLog
2010-12-14  Keith Seitz  <keiths@redhat.com>

	PR c++/11734
	* linespec.c (decode_compound): Rename SAVED_ARG to
	THE_REAL_SAVED_ARG.
	Make a copy of THE_REAL_SAVED_ARG in SAVED_ARG and strip
	single-quotes.

	* psymtab.c (lookup_partial_symbol): Add language parameter.
	(lookup_symbol_aux_psymtabs): Likewise.
	Don't assume that the psymtab we found was the right one. Search
	for the desired symbol in the symtab to be certain.
	(psymtab_search_name): New function.
	(lookup_partial_symbol): Use psymtab_search_name.
	Add language parameter.
	(read_symtabs_for_function): Add language parameter and pass to
	lookup_partial_symbol.
	(find_symbol_file_from_partial): Likewise.
	* symfile.h (struct quick_symbol_functions): Add language parameter
	to lookup_symbol, expand_symtabs_for_function, and find_symbol_file.
	* cp-support.c (make_symbol_overload_list): Update above API
	changes.
	* symtab.c (lookup_symbol_aux_quick): Pass the current language
	to the quick symbol functions.
	(basic_lookup_transparent_type_quick): Likewise.
	(find_main_filename): Likewise.
	* dwarf2_read.c (dw2_lookup_symbol): Add langauge parameter.
	(dw2_expand_symtabs_for_function): Likewise.
	(dw2_find_symbol_file): Likewise.

testsuite/ChangeLog
2010-12-14  Keith Seitz  <keiths@redhat.com>

	PR c++/11734
	* gdb.cp/pr11734.exp: New test.
	* gdb.cp/pr11734.h: New file.
	* gdb.cp/pr11734-1.cc: New file.
	* gdb.cp/pr11734-2.cc: New file.
	* gdb.cp/pr11734-3.cc: New file.
	* gdb.cp/pr11734-4.cc: New file.

[-- Attachment #2: pr11734-5.patch --]
[-- Type: text/plain, Size: 22326 bytes --]

Index: cp-support.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-support.c,v
retrieving revision 1.44
diff -u -p -r1.44 cp-support.c
--- cp-support.c	25 Jun 2010 16:16:44 -0000	1.44
+++ cp-support.c	14 Dec 2010 19:43:50 -0000
@@ -35,6 +35,7 @@
 #include "exceptions.h"
 #include "expression.h"
 #include "value.h"
+#include "language.h"
 
 #include "safe-ctype.h"
 
@@ -917,7 +918,8 @@ make_symbol_overload_list_qualified (con
   ALL_OBJFILES (objfile)
   {
     if (objfile->sf)
-      objfile->sf->qf->expand_symtabs_for_function (objfile, func_name);
+      objfile->sf->qf->expand_symtabs_for_function (objfile, func_name,
+						    language_cplus);
   }
 
   /* Search upwards from currently selected frame (so that we can
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.489
diff -u -p -r1.489 dwarf2read.c
--- dwarf2read.c	12 Dec 2010 19:19:27 -0000	1.489
+++ dwarf2read.c	14 Dec 2010 19:43:53 -0000
@@ -2334,7 +2334,8 @@ dw2_lookup_symtab (struct objfile *objfi
 
 static struct symtab *
 dw2_lookup_symbol (struct objfile *objfile, int block_index,
-		   const char *name, domain_enum domain)
+		   const char *name, domain_enum domain,
+		   enum language language)
 {
   /* We do all the work in the pre_expand_symtabs_matching hook
      instead.  */
@@ -2410,7 +2411,8 @@ dw2_relocate (struct objfile *objfile, s
 
 static void
 dw2_expand_symtabs_for_function (struct objfile *objfile,
-				 const char *func_name)
+				 const char *func_name,
+				 enum language language)
 {
   dw2_do_expand_symtabs_matching (objfile, func_name);
 }
@@ -2470,7 +2472,8 @@ dw2_expand_symtabs_with_filename (struct
 }
 
 static const char *
-dw2_find_symbol_file (struct objfile *objfile, const char *name)
+dw2_find_symbol_file (struct objfile *objfile, const char *name,
+		      enum language language)
 {
   struct dwarf2_per_cu_data *per_cu;
   offset_type *vec;
Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.105
diff -u -p -r1.105 linespec.c
--- linespec.c	2 Dec 2010 20:05:59 -0000	1.105
+++ linespec.c	14 Dec 2010 19:43:54 -0000
@@ -1219,7 +1219,7 @@ decode_objc (char **argptr, int funfirst
 
 static struct symtabs_and_lines
 decode_compound (char **argptr, int funfirstline, char ***canonical,
-		 char *saved_arg, char *p, int *not_found_ptr)
+		 char *the_real_saved_arg, char *p, int *not_found_ptr)
 {
   struct symtabs_and_lines values;
   char *p2;
@@ -1230,7 +1230,23 @@ decode_compound (char **argptr, int funf
   struct symbol *sym_class;
   struct type *t;
   char *saved_java_argptr = NULL;
+  char *saved_arg;
 
+  /* THE_REAL_SAVED_ARG cannot be altered, so make a copy that can be.  */
+  saved_arg = alloca (strlen (the_real_saved_arg) + 1);
+  strcpy (saved_arg, the_real_saved_arg);
+
+  /* If the user specified "'foo::bar(baz)'" (note the quotes -- often
+     added to workaround completer issues) -- saved_arg will be
+     encapsulated in single-quotes.  They are superfluous, so just strip
+     them off.  */
+  if (*saved_arg == '\'')
+    {
+      char *end = skip_quoted (saved_arg);
+      memmove (saved_arg, saved_arg + 1, end - saved_arg);
+      memmove (end - 2, end, strlen (saved_arg) + 1);
+    }
+      
   /* First check for "global" namespace specification, of the form
      "::foo".  If found, skip over the colons and jump to normal
      symbol processing.  I.e. the whole line specification starts with
@@ -1481,7 +1497,7 @@ decode_compound (char **argptr, int funf
      up.  The quotes are important if copy is empty.  */
   if (not_found_ptr)
     *not_found_ptr = 1;
-  cplusplus_error (saved_arg,
+  cplusplus_error (the_real_saved_arg,
 		   "Can't find member of namespace, class, struct, or union named \"%s\"\n",
 		   copy);
 }
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.19
diff -u -p -r1.19 psymtab.c
--- psymtab.c	24 Nov 2010 19:01:51 -0000	1.19
+++ psymtab.c	14 Dec 2010 19:43:54 -0000
@@ -33,6 +33,8 @@
 #include "readline/readline.h"
 #include "gdb_regex.h"
 #include "dictionary.h"
+#include "language.h"
+#include "cp-support.h"
 
 #ifndef DEV_TTY
 #define DEV_TTY "/dev/tty"
@@ -55,7 +57,8 @@ static struct partial_symbol *match_part
 
 static struct partial_symbol *lookup_partial_symbol (struct partial_symtab *,
 						     const char *, int,
-						     domain_enum);
+						     domain_enum,
+						     enum language);
 
 static char *psymtab_to_fullname (struct partial_symtab *ps);
 
@@ -418,15 +421,35 @@ fixup_psymbol_section (struct partial_sy
 static struct symtab *
 lookup_symbol_aux_psymtabs (struct objfile *objfile,
 			    int block_index, const char *name,
-			    const domain_enum domain)
+			    const domain_enum domain, enum language language)
 {
   struct partial_symtab *ps;
   const int psymtab_index = (block_index == GLOBAL_BLOCK ? 1 : 0);
 
   ALL_OBJFILE_PSYMTABS (objfile, ps)
   {
-    if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
+    if (!ps->readin
+	&& lookup_partial_symbol (ps, name, psymtab_index, domain, language))
+      {
+	struct symbol *sym;
+	struct symtab *stab = PSYMTAB_TO_SYMTAB (ps);
+	sym = NULL;
+
+	/* Some caution must be observed with overloaded functions
+	   and methods, since the psymtab will not contain any overload
+	   information (but NAME might contain it).  */
+	if (stab->primary)
+	  {
+	    struct blockvector *bv = BLOCKVECTOR (stab);
+	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
+	    sym = lookup_block_symbol (block, name, domain);
+	  }
+
+	if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
+	  return stab;
+
+	/* Keep looking through other psymtabs.  */
+      }
   }
 
   return NULL;
@@ -519,22 +542,58 @@ pre_expand_symtabs_matching_psymtabs (st
   /* Nothing.  */
 }
 
+/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
+   not contain any method/function instance information (since this would
+   force reading type information while reading psymtabs).  Therefore,
+   if NAME contains overload information, it must be stripped before searching
+   psymtabs.
+
+   The caller is responsible for freeing the return result.  */
+
+static const char *
+psymtab_search_name (const char *name, enum language language)
+{
+  switch (language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+       if (strchr (name, '('))
+         {
+           char *ret = cp_remove_params (name);
+           if (ret)
+             return ret;
+         }
+      }
+
+    default:
+      break;
+    }
+
+  return xstrdup (name);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    Check the global symbols if GLOBAL, the static symbols if not.  */
 
 static struct partial_symbol *
 lookup_partial_symbol (struct partial_symtab *pst, const char *name,
-		       int global, domain_enum domain)
+		       int global, domain_enum domain, enum language language)
 {
   struct partial_symbol **start, **psym;
   struct partial_symbol **top, **real_top, **bottom, **center;
   int length = (global ? pst->n_global_syms : pst->n_static_syms);
   int do_linear_search = 1;
+  const char *search_name;
+  struct cleanup *cleanup;
 
   if (length == 0)
     {
       return (NULL);
     }
+
+  search_name = psymtab_search_name (name, language);
+  cleanup = make_cleanup (xfree, (void *) search_name);
   start = (global ?
 	   pst->objfile->global_psymbols.list + pst->globals_offset :
 	   pst->objfile->static_psymbols.list + pst->statics_offset);
@@ -562,7 +621,8 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
+				 search_name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -575,11 +635,14 @@ lookup_partial_symbol (struct partial_sy
 	internal_error (__FILE__, __LINE__, _("failed internal consistency check"));
 
       while (top <= real_top
-	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
 				     SYMBOL_DOMAIN (*top), domain))
-	    return (*top);
+	    {
+	      do_cleanups (cleanup);
+	      return (*top);
+	    }
 	  top++;
 	}
     }
@@ -594,10 +657,14 @@ lookup_partial_symbol (struct partial_sy
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
 				     SYMBOL_DOMAIN (*psym), domain)
 	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
-	    return (*psym);
+	    {
+	      do_cleanups (cleanup);
+	      return (*psym);
+	    }
 	}
     }
 
+  do_cleanups (cleanup);
   return (NULL);
 }
 
@@ -909,7 +976,8 @@ dump_psymtabs_for_objfile (struct objfil
    by matching FUNC_NAME.  Make sure we read that symbol table in. */
 
 static void
-read_symtabs_for_function (struct objfile *objfile, const char *func_name)
+read_symtabs_for_function (struct objfile *objfile, const char *func_name,
+			   enum language language)
 {
   struct partial_symtab *ps;
 
@@ -918,9 +986,9 @@ read_symtabs_for_function (struct objfil
     if (ps->readin)
       continue;
 
-    if ((lookup_partial_symbol (ps, func_name, 1, VAR_DOMAIN)
+    if ((lookup_partial_symbol (ps, func_name, 1, VAR_DOMAIN, language)
 	 != NULL)
-	|| (lookup_partial_symbol (ps, func_name, 0, VAR_DOMAIN)
+	|| (lookup_partial_symbol (ps, func_name, 0, VAR_DOMAIN, language)
 	    != NULL))
       psymtab_to_symtab (ps);
   }
@@ -1040,13 +1108,14 @@ psymtab_to_fullname (struct partial_symt
 }
 
 static const char *
-find_symbol_file_from_partial (struct objfile *objfile, const char *name)
+find_symbol_file_from_partial (struct objfile *objfile, const char *name,
+			       enum language language)
 {
   struct partial_symtab *pst;
 
   ALL_OBJFILE_PSYMTABS (objfile, pst)
     {
-      if (lookup_partial_symbol (pst, name, 1, VAR_DOMAIN))
+      if (lookup_partial_symbol (pst, name, 1, VAR_DOMAIN, language))
 	return pst->filename;
     }
   return NULL;
Index: symfile.h
===================================================================
RCS file: /cvs/src/src/gdb/symfile.h,v
retrieving revision 1.80
diff -u -p -r1.80 symfile.h
--- symfile.h	9 Dec 2010 19:52:23 -0000	1.80
+++ symfile.h	14 Dec 2010 19:43:54 -0000
@@ -167,14 +167,15 @@ struct quick_symbol_functions
   /* Check to see if the symbol is defined in a "partial" symbol table
      of OBJFILE.  KIND should be either GLOBAL_BLOCK or STATIC_BLOCK,
      depending on whether we want to search global symbols or static
-     symbols.  NAME is the name of the symbol to look for.  DOMAIN
-     indicates what sort of symbol to search for.
+     symbols.  NAME (valid in LANGUAGE) is the name of the symbol to look for.
+     DOMAIN indicates what sort of symbol to search for.
 
      Returns the newly-expanded symbol table in which the symbol is
      defined, or NULL if no such symbol table exists.  */
   struct symtab *(*lookup_symbol) (struct objfile *objfile,
 				   int kind, const char *name,
-				   domain_enum domain);
+				   domain_enum domain,
+				   enum language language);
 
   /* This is called to expand symbol tables before looking up a
      symbol.  A backend can choose to implement this and then have its
@@ -200,10 +201,11 @@ struct quick_symbol_functions
 		    struct section_offsets *new_offsets,
 		    struct section_offsets *delta);
 
-  /* Find all the symbols in OBJFILE named FUNC_NAME, and ensure that
-     the corresponding symbol tables are loaded.  */
+  /* Find all the symbols in OBJFILE named FUNC_NAME (valid in LANGUAGE),
+     and ensure that the corresponding symbol tables are loaded.  */
   void (*expand_symtabs_for_function) (struct objfile *objfile,
-				       const char *func_name);
+				       const char *func_name,
+				       enum language language);
 
   /* Read all symbol tables associated with OBJFILE.  */
   void (*expand_all_symtabs) (struct objfile *objfile);
@@ -217,8 +219,10 @@ struct quick_symbol_functions
 					const char *filename);
 
   /* Return the file name of the file holding the symbol in OBJFILE
-     named NAME.  If no such symbol exists in OBJFILE, return NULL.  */
-  const char *(*find_symbol_file) (struct objfile *objfile, const char *name);
+     named NAME (valid in LANGUAGE).  If no such symbol exists in OBJFILE,
+     return NULL.  */
+  const char *(*find_symbol_file) (struct objfile *objfile, const char *name,
+				   enum language language);
 
   /* Find global or static symbols in all tables that are in NAMESPACE 
      and for which MATCH (symbol name, NAME) == 0, passing each to 
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.254
diff -u -p -r1.254 symtab.c
--- symtab.c	17 Oct 2010 18:49:46 -0000	1.254
+++ symtab.c	14 Dec 2010 19:43:55 -0000
@@ -1380,7 +1380,8 @@ lookup_symbol_aux_quick (struct objfile 
 
   if (!objfile->sf)
     return NULL;
-  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, domain);
+  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, domain,
+					   current_language->la_language);
   if (!symtab)
     return NULL;
 
@@ -1551,7 +1552,8 @@ basic_lookup_transparent_type_quick (str
 
   if (!objfile->sf)
     return NULL;
-  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, STRUCT_DOMAIN);
+  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, STRUCT_DOMAIN,
+					   current_language->la_language);
   if (!symtab)
     return NULL;
 
@@ -1683,7 +1685,8 @@ find_main_filename (void)
 
     if (!objfile->sf)
       continue;
-    result = objfile->sf->qf->find_symbol_file (objfile, name);
+    result = objfile->sf->qf->find_symbol_file (objfile, name,
+						current_language->la_language);
     if (result)
       return result;
   }
Index: testsuite/gdb.cp/pr11734-1.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-1.cc
diff -N testsuite/gdb.cp/pr11734-1.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-1.cc	14 Dec 2010 19:43:56 -0000
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+int
+main ()
+{
+  pr11734 *p = new pr11734;
+  p->foo ();
+  return 0;
+}
+
Index: testsuite/gdb.cp/pr11734-2.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-2.cc
diff -N testsuite/gdb.cp/pr11734-2.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-2.cc	14 Dec 2010 19:43:56 -0000
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+void
+pr11734::foo(void)
+{
+}
+
Index: testsuite/gdb.cp/pr11734-3.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-3.cc
diff -N testsuite/gdb.cp/pr11734-3.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-3.cc	14 Dec 2010 19:43:56 -0000
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+void
+pr11734::foo (int a)
+{
+}
+
Index: testsuite/gdb.cp/pr11734-4.cc
===================================================================
RCS file: testsuite/gdb.cp/pr11734-4.cc
diff -N testsuite/gdb.cp/pr11734-4.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734-4.cc	14 Dec 2010 19:43:56 -0000
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+#include "pr11734.h"
+
+void
+pr11734::foo (char *a)
+{
+}
+
Index: testsuite/gdb.cp/pr11734.exp
===================================================================
RCS file: testsuite/gdb.cp/pr11734.exp
diff -N testsuite/gdb.cp/pr11734.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734.exp	14 Dec 2010 19:43:56 -0000
@@ -0,0 +1,55 @@
+# Copyright 2010 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+if { [skip_cplus_tests] } { continue }
+
+set testfile "pr11734"
+set class $testfile
+
+set srcfiles {}
+for {set i 1} {$i < 5} {incr i} {
+    lappend srcfiles $testfile-$i.cc
+}
+
+prepare_for_testing pr11734 $testfile $srcfiles {c++ debug}
+
+if {![runto_main]} {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# An array holding the overload types for the method pr11734::foo.  The
+# first element is the overloaded method parameter.  The second element
+# is the expected source file number, e.g. "pr11734-?.cc".
+array set tests {
+    "char*"  4
+    "int"    3
+    ""       2
+}
+
+# Test each overload instance twice: once quoted, once unquoted
+foreach ovld [array names tests] {
+    set method "${class}::foo\($ovld\)"
+    set result "Breakpoint (\[0-9\]).*file .*/$class-$tests($ovld).*"
+    gdb_test "break $method" $result
+    gdb_test "break '$method'" $result
+}
+
+gdb_exit
+return 0
Index: testsuite/gdb.cp/pr11734.h
===================================================================
RCS file: testsuite/gdb.cp/pr11734.h
diff -N testsuite/gdb.cp/pr11734.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr11734.h	14 Dec 2010 19:43:56 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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@gnu.org  */
+
+class pr11734
+{
+ public:
+  void foo ();
+  void foo (int);
+  void foo (char *);
+};
+

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2010-12-14 20:03   ` Keith Seitz
@ 2011-01-24 18:15     ` Jan Kratochvil
  2011-01-26 23:14       ` Jan Kratochvil
  2011-02-06 22:04     ` Jan Kratochvil
                       ` (4 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Jan Kratochvil @ 2011-01-24 18:15 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

On Tue, 14 Dec 2010 21:02:48 +0100, Keith Seitz wrote:
> On 12/09/2010 01:44 PM, Tom Tromey wrote:
> >>>>>>"Keith" == Keith Seitz<keiths@redhat.com>  writes:
> >The return type of this function should be just "char *".
> >Otherwise you will need casts when freeing.
> 
> Okay.

psymtab_search_name still inappropriately returns `const char *'; plus the part
at the caller.


> Revised patch attached.

As a summary:

"physname" patch I call:
	42284fdf9d8cdb20c8e833bdbdb2b56977fea525
	http://sourceware.org/ml/gdb-cvs/2010-03/msg00082.html
	dwarf2_physname patchset:
	[RFA] dwarf2_physname FINAL
	http://sourceware.org/ml/gdb-patches/2010-03/msg00220.html

Original pre-physname code for pr11734::foo() did not call decode_compound but
called decode_variable which resolved it from the fully qualified name.

post-physname code tried to resolve pr11734::foo() via decode_compound which
did not support such case and it fails now on HEAD.

Alternatively to your patch the current HEAD can be modified to call
decode_variable again as in pre-physname era which resolves it correctly but
for some reason you prefer to resolve pr11734::foo() via decode_compound
instead which makes sense.



> testsuite/ChangeLog
> 2010-12-14  Keith Seitz  <keiths@redhat.com>
> 
> 	PR c++/11734
> 	* gdb.cp/pr11734.exp: New test.
> 	* gdb.cp/pr11734.h: New file.
> 	* gdb.cp/pr11734-1.cc: New file.
> 	* gdb.cp/pr11734-2.cc: New file.
> 	* gdb.cp/pr11734-3.cc: New file.
> 	* gdb.cp/pr11734-4.cc: New file.

Please choose arbitrary words instead of prNUMBER for the testcase filename.


>  decode_compound (char **argptr, int funfirstline, char ***canonical,
[...]
> +  /* If the user specified "'foo::bar(baz)'" (note the quotes -- often
> +     added to workaround completer issues) -- saved_arg will be
> +     encapsulated in single-quotes.  They are superfluous, so just strip
> +     them off.  */
> +  if (*saved_arg == '\'')
> +    {
> +      char *end = skip_quoted (saved_arg);

Missing empty line (when it has been already patched by Michael Snyder in the
"white space" series).

> +      memmove (saved_arg, saved_arg + 1, end - saved_arg);
> +      memmove (end - 2, end, strlen (saved_arg) + 1);

Here is overrun as skip_quoted does not need a trailing quote.
For: (gdb) break '.
*******
mudflap violation 16 (check/read): time=1295814377.349729 ptr=0x320e6e2 size=2
pc=0x7f2af65f89a1 location=`(memmove src)'
      /usr/lib64/libmudflap.so.0(__mf_check+0x41) [0x7f2af65f89a1]
      /usr/lib64/libmudflap.so.0(__mfwrap_memmove+0x170) [0x7f2af65fa1a0]
      ../gdb() [0x69faa3]
Nearby object 1: checked region begins 2B into and ends 1B after
mudflap object 0x88759c0: name=`alloca region'
bounds=[0x320e6e0,0x320e6e2] size=3 area=heap check=3r/1w liveness=4
alloc time=1295814377.349463 pc=0x7f2af65f80e1
      /usr/lib64/libmudflap.so.0(__mf_register+0x41) [0x7f2af65f80e1]
      /usr/lib64/libmudflap.so.0(__mf_wrap_alloca_indirect+0x1b5) [0x7f2af65f9c25]
      ../gdb() [0x69f932]
      ../gdb(decode_line_1+0x804) [0x69bb1b]
number of nearby objects: 1
saved_arg=<>
*******


>  lookup_symbol_aux_psymtabs (struct objfile *objfile,
[...]
> +	if (stab->primary)
> +	  {
> +	    struct blockvector *bv = BLOCKVECTOR (stab);
> +	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);

Empty line.

> +	    sym = lookup_block_symbol (block, name, domain);
> +	  }



> +static const char *

The `const char *' code cleanup discussed above.

> +psymtab_search_name (const char *name, enum language language)
> +{
> +  switch (language)
> +    {
> +    case language_cplus:
> +    case language_java:
> +      {
> +       if (strchr (name, '('))
> +         {
> +           char *ret = cp_remove_params (name);

Empty line.

> +           if (ret)
> +             return ret;
> +         }
> +      }

Missing `break;' here, it is a bit fragile against future modifications.

> +
> +    default:
> +      break;
> +    }
> +
> +  return xstrdup (name);
> +}
> +
>  /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
>     Check the global symbols if GLOBAL, the static symbols if not.  */
>  
>  static struct partial_symbol *
>  lookup_partial_symbol (struct partial_symtab *pst, const char *name,
> -		       int global, domain_enum domain)
> +		       int global, domain_enum domain, enum language language)
>  {
>    struct partial_symbol **start, **psym;
>    struct partial_symbol **top, **real_top, **bottom, **center;
>    int length = (global ? pst->n_global_syms : pst->n_static_syms);
>    int do_linear_search = 1;
> +  const char *search_name;

The `const char *' code cleanup discussed above.


> +  struct cleanup *cleanup;
>  
>    if (length == 0)
>      {
>        return (NULL);
>      }
> +
> +  search_name = psymtab_search_name (name, language);
> +  cleanup = make_cleanup (xfree, (void *) search_name);

The `const char *' code cleanup discussed above.


> +   Please email any bugs, comments, and/or additions to this file to:
> +   bug-gdb@gnu.org  */

This part is obsolete (and should be removed in few places).


AFAIK Tom was also reviewing it.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2011-01-24 18:15     ` Jan Kratochvil
@ 2011-01-26 23:14       ` Jan Kratochvil
  0 siblings, 0 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-01-26 23:14 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

On Mon, 24 Jan 2011 18:37:45 +0100, Jan Kratochvil wrote:
> > Revised patch attached.
> 
> As a summary:
> 
> "physname" patch I call:
> 	42284fdf9d8cdb20c8e833bdbdb2b56977fea525
> 	http://sourceware.org/ml/gdb-cvs/2010-03/msg00082.html
> 	dwarf2_physname patchset:
> 	[RFA] dwarf2_physname FINAL
> 	http://sourceware.org/ml/gdb-patches/2010-03/msg00220.html
> 
> Original pre-physname code for pr11734::foo() did not call decode_compound but
> called decode_variable which resolved it from the fully qualified name.
> 
> post-physname code tried to resolve pr11734::foo() via decode_compound which
> did not support such case and it fails now on HEAD.
> 
> Alternatively to your patch the current HEAD can be modified to call
> decode_variable again as in pre-physname era which resolves it correctly but
> for some reason you prefer to resolve pr11734::foo() via decode_compound
> instead which makes sense.

I have to withdraw my preliminary approval.  Together with
	[RFA] c++/12273
	http://sourceware.org/ml/gdb-patches/2010-12/msg00264.html

both patches duplicate the decode_variable functionality by the new
implementation in decode_compound.

But this duplication is still not complete, for example it does not handle
properly namespaces.  It had only a luck the existing namespace tests
(nsusing.exp) do not exploit it.

I have attached such testcase on top of your this patch:
	Re: [RFA] c++/11734 revisited
	http://sourceware.org/ml/gdb-patches/2010-12/msg00263.html

with results:
PASS: gdb.cp/pr11734.exp: break func
PASS: gdb.cp/pr11734.exp: break 'func'
^^^^ = test like nsusing.exp
FAIL: gdb.cp/pr11734.exp: break pr11734::foo() (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break 'pr11734::foo()' (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break pr11734::foo(char*) (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break 'pr11734::foo(char*)' (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break pr11734::foo(int) (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break 'pr11734::foo(int)' (got interactive prompt)
^^^^ = these funcs get called by the C++ inferior but GDB cannot resolve them.

decode_compound itself should comply to namespacing, just dropping to
decode_variable as before is not enough anyway as decode_variable cannot
resolve class fields which have no global symbols/minsym.

The new plan of G++ parser/plugin is probably not considerable now as a fix.

With the very hacky drop into decode_variable the testcase works but this
patch of mine is sure not usable.



Thanks,
Jan


--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -783,7 +783,7 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   /* Does it look like there actually were two parts?  */
 
-  if (p[0] == ':' || p[0] == '.')
+if (0) //  if (p[0] == ':' || p[0] == '.')
     {
       /* Is it a C++ or Java compound data structure?
 	 The check on p[1] == ':' is capturing the case of "::",
@@ -868,7 +868,22 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
     }
   else
     {
-      p = skip_quoted (*argptr);
+      const char *lang_breakchars;
+      char *breakchars, *s, *end;
+
+      lang_breakchars = current_language->la_word_break_characters ();
+
+      breakchars = alloca (strlen (lang_breakchars) + 1);
+      strcpy (breakchars, lang_breakchars);
+      end = &breakchars[strlen (breakchars)];
+
+      s = strchr (breakchars, ':');
+      if (s != NULL)
+	{
+	  *s = *--end;
+	  *end = 0;
+	}
+      p = skip_quoted_chars (*argptr, NULL, breakchars);
     }
 
   /* Keep any template parameters.  */
--- a/gdb/testsuite/gdb.cp/pr11734-1.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-1.cc
@@ -20,11 +20,21 @@
 
 #include "pr11734.h"
 
+namespace A { void func () {} }
+
+using namespace A;
+
 int
 main ()
 {
   pr11734 *p = new pr11734;
   p->foo ();
+
+  func ();
+
+  pr11734::foo ();
+  pr11734::foo ((char *) "");
+  pr11734::foo (0);
+
   return 0;
 }
-
--- a/gdb/testsuite/gdb.cp/pr11734-2.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-2.cc
@@ -20,8 +20,11 @@
 
 #include "pr11734.h"
 
+namespace A {
+
 void
 pr11734::foo(void)
 {
 }
 
+}
--- a/gdb/testsuite/gdb.cp/pr11734-3.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-3.cc
@@ -20,8 +20,11 @@
 
 #include "pr11734.h"
 
+namespace A {
+
 void
 pr11734::foo (int a)
 {
 }
 
+}
--- a/gdb/testsuite/gdb.cp/pr11734-4.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-4.cc
@@ -20,8 +20,11 @@
 
 #include "pr11734.h"
 
+namespace A {
+
 void
 pr11734::foo (char *a)
 {
 }
 
+}
--- a/gdb/testsuite/gdb.cp/pr11734.exp
+++ b/gdb/testsuite/gdb.cp/pr11734.exp
@@ -34,6 +34,15 @@ if {![runto_main]} {
     continue
 }
 
+if [gdb_breakpoint "func"] {
+  pass "break func"
+}
+if [gdb_breakpoint "'func'"] {
+  pass "break 'func'"
+}
+
+gdb_test "ptype ${class}::l" "type = long"
+
 # An array holding the overload types for the method pr11734::foo.  The
 # first element is the overloaded method parameter.  The second element
 # is the expected source file number, e.g. "pr11734-?.cc".
--- a/gdb/testsuite/gdb.cp/pr11734.h
+++ b/gdb/testsuite/gdb.cp/pr11734.h
@@ -18,11 +18,15 @@
    Please email any bugs, comments, and/or additions to this file to:
    bug-gdb@gnu.org  */
 
+namespace A {
+
 class pr11734
 {
  public:
-  void foo ();
-  void foo (int);
-  void foo (char *);
+  static void foo ();
+  static void foo (int);
+  static void foo (char *);
+  long l;
 };
 
+}

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFA] c++/11734 revisited
  2010-12-14 20:03   ` Keith Seitz
  2011-01-24 18:15     ` Jan Kratochvil
@ 2011-02-06 22:04     ` Jan Kratochvil
  2011-02-06 22:45     ` [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273) Jan Kratochvil
                       ` (3 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-02-06 22:04 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

On Tue, 14 Dec 2010 21:02:48 +0100, Keith Seitz wrote:
...
>  static struct partial_symbol *
>  lookup_partial_symbol (struct partial_symtab *pst, const char *name,
> -		       int global, domain_enum domain)
> +		       int global, domain_enum domain, enum language language)
>  {
[...]
> +  search_name = psymtab_search_name (name, language);
> +  cleanup = make_cleanup (xfree, (void *) search_name);
[...]
> -	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
> +	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
> +				 search_name) >= 0)
[...]
>        while (top <= real_top
> -	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
> +	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
>  	{
>  	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
>  				     SYMBOL_DOMAIN (*top), domain))
> -	    return (*top);
> +	    {
> +	      do_cleanups (cleanup);
> +	      return (*top);
> +	    }
>  	  top++;
>  	}
>      }
> @@ -594,10 +657,14 @@ lookup_partial_symbol (struct partial_sy
>  	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
>  				     SYMBOL_DOMAIN (*psym), domain)
>  	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
                                                    ^^^^
Here should be also `search_name' I think.

> -	    return (*psym);
> +	    {
> +	      do_cleanups (cleanup);
> +	      return (*psym);
> +	    }


Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2010-12-14 20:03   ` Keith Seitz
  2011-01-24 18:15     ` Jan Kratochvil
  2011-02-06 22:04     ` Jan Kratochvil
@ 2011-02-06 22:45     ` Jan Kratochvil
  2011-02-08 21:42       ` Tom Tromey
  2011-02-10 21:45       ` Keith Seitz
  2011-02-06 22:46     ` [patch 1/3] revert physname part (b) [Re: [RFA] c++/11734 revisited] Jan Kratochvil
                       ` (2 subsequent siblings)
  5 siblings, 2 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-02-06 22:45 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

Hi Keith,

the patch
	42284fdf9d8cdb20c8e833bdbdb2b56977fea525
	http://sourceware.org/ml/gdb-cvs/2010-03/msg00082.html
	dwarf2_physname patchset:
	[RFA] dwarf2_physname FINAL
	http://sourceware.org/ml/gdb-patches/2010-03/msg00220.html

contained three parts:

(a) Drop DW_AT_linkage_name use.
    =>PR 12328 - the const/volatile qualifiers
    =>PR 11734 non-decode_compound - psymtabs no longer having parameter types
 * To be definitely kept in.

(b) switch from classname::any(any) linespec resolving by decode_variable
    to the classname-scope aware decode_compound.
    =>PR 11734 decode_compound - quoting fixup
    =>PR 12273 - decode_compound not aware of minimal symbols
 * Why to apply this part is this mail about.

(c) Some fixes of formerly unsupported linespecs (such as trailing ` const').
 * Partial port to the legacy GDB codebase.

I do not fully understand the reasons for part (b).  The old code is not nice
but IMO neither is the new code (already checked in by the physname patch) due to
the linespec.c caller.  Moreover the new code has shown its regressions.


If the code should be nice I tried archer-jankratochvil-linespec where
linespec is based on the expressions.  Noted by Daniel Jacobowitz before:
	http://sourceware.org/ml/gdb-patches/2009-11/msg00266.html
It still has some regressions but the most common C++ cases work there and
I find it doable with some more time (mostly if the error messages can be
changed).  The are problems with:
 * expression evaluator cares about the function value (=address) where the
   function symbol for linespec is already dropped.
 * linespec should have no side effects.  But the expression evaluator's
   EVAL_AVOID_SIDE_EFFECTS mode cares only about types, not about values.


This "revert" series:
[patch 1/3] is revert of the parts (b)+(c).
[patch 2/3] is application of the 11734 non-decode_compound part psymtabs fix.
[patch 3/3] is reapplication of (c).

Afterwards there remain these regresions:
+FAIL: gdb.cp/pr12273.exp: setting breakpoint at GDB<int>::operator ==
+FAILs in gdb.java/jmisc.exp and gdb.java/jprint.exp
 (IMHO gcj gdb.java/* should not be anything relevant with openjdk now.)

OTOH namespaces work better with these three "revert" patches while it does
not with the patches of yours under review.  With the testfiles

	http://sourceware.org/ml/gdb-patches/2011-01/msg00514.html
../gdb -nx gdb.cp/pr11734 -ex start -ex 'b pr11734::foo()'

Sure the namespaces could be fixed in your patchset, but also `operator =='
could be futher fixed up in these "revert" patches.


That is in general I would be either for futher not-nice fixing up the
pre-physname code or for the expression way like archer-jankratochvil-linespec
does.  Still at the moment your patchset gives the best user experience, just
it is a new untested code which does not make it nice anyway.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [patch 1/3] revert physname part (b)  [Re: [RFA] c++/11734 revisited]
  2010-12-14 20:03   ` Keith Seitz
                       ` (2 preceding siblings ...)
  2011-02-06 22:45     ` [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273) Jan Kratochvil
@ 2011-02-06 22:46     ` Jan Kratochvil
  2011-02-06 22:46     ` [patch 3/3] Various linespec fixups " Jan Kratochvil
  2011-02-06 22:46     ` [patch 2/3] Keith's psymtabs fix " Jan Kratochvil
  5 siblings, 0 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-02-06 22:46 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

Hi Keith,

[patch 1/3] is revert of the parts (b)+(c).


Jan


--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -40,7 +40,6 @@
 #include "interps.h"
 #include "mi/mi-cmds.h"
 #include "target.h"
-#include "arch-utils.h"
 
 /* We share this one with symtab.c, but it is not exported widely.  */
 
@@ -51,6 +50,8 @@ extern char *operator_chars (char *, char **);
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
+static void set_flags (char *arg, int *is_quoted, char **paren_pointer);
+
 static struct symtabs_and_lines decode_indirect (char **argptr);
 
 static char *locate_first_half (char **argptr, int *is_quote_enclosed);
@@ -632,37 +633,6 @@ decode_line_2 (struct symbol *sym_arr[], int nelts, int funfirstline,
   discard_cleanups (old_chain);
   return return_values;
 }
-
-/* A helper function for decode_line_1 and friends which skips P
-   past any method overload information at the beginning of P, e.g.,
-   "(const struct foo *)".
-
-   This function assumes that P has already been validated to contain
-   overload information, and it will assert if *P != '('.  */
-static char *
-find_method_overload_end (char *p)
-{
-  int depth = 0;
-
-  gdb_assert (*p == '(');
-
-  while (*p)
-    {
-      if (*p == '(')
-	++depth;
-      else if (*p == ')')
-	{
-	  if (--depth == 0)
-	    {
-	      ++p;
-	      break;
-	    }
-	}
-      ++p;
-    }
-
-  return p;
-}
 \f
 /* The parser of linespec itself.  */
 
@@ -724,6 +694,9 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
   struct symtab *file_symtab = NULL;
 
   char *copy;
+  /* This is NULL if there are no parens in *ARGPTR, or a pointer to
+     the closing parenthesis if there are parens.  */
+  char *paren_pointer;
   /* This says whether or not something in *ARGPTR is quoted with
      completer_quotes (i.e. with single quotes).  */
   int is_quoted;
@@ -748,9 +721,12 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
   if (**argptr == '*')
     return decode_indirect (argptr);
 
-  is_quoted = (*argptr
-	       && strchr (get_gdb_completer_quote_characters (),
-			  **argptr) != NULL);
+  /* Set various flags.  'paren_pointer' is important for overload
+     checking, where we allow things like:
+        (gdb) break c::f(int)
+  */
+
+  set_flags (*argptr, &is_quoted, &paren_pointer);
   if (is_quoted)
     end_quote = skip_quoted (*argptr);
 
@@ -768,7 +744,10 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
   /* Check if this is an Objective-C method (anything that starts with
      a '+' or '-' and a '[').  */
   if (is_objc_method_format (p))
-    is_objc_method = 1;
+    {
+      is_objc_method = 1;
+      paren_pointer  = NULL; /* Just a category name.  Ignore it.  */
+    }
 
   /* Check if the symbol could be an Objective-C selector.  */
 
@@ -794,33 +773,63 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 	
       if (p[0] == '.' || p[1] == ':')
 	{
-	  struct symtabs_and_lines values;
-
-	  if (is_quote_enclosed)
-	    ++saved_arg;
-	  values = decode_compound (argptr, funfirstline, canonical,
+	  if (paren_pointer == NULL)
+	    return decode_compound (argptr, funfirstline, canonical,
 				    saved_arg, p, not_found_ptr);
-	  if (is_quoted && **argptr == '\'')
-	    *argptr = *argptr + 1;
-	  return values;
+	  /* Otherwise, fall through to decode_variable below.  */
 	}
+      else
+	{
+	  /* No, the first part is a filename; set file_symtab to be that file's
+	     symtab.  Also, move argptr past the filename.  */
 
-      /* No, the first part is a filename; set file_symtab to be that file's
-	 symtab.  Also, move argptr past the filename.  */
+	  file_symtab = symtab_from_filename (argptr, p, is_quote_enclosed,
+					      not_found_ptr);
 
-      file_symtab = symtab_from_filename (argptr, p, is_quote_enclosed,
-					  not_found_ptr);
+	  /* Check for single quotes on the non-filename part.  */
+	  if (!is_quoted)
+	    {
+	      is_quoted = (**argptr
+			   && strchr (get_gdb_completer_quote_characters (),
+				      **argptr) != NULL);
+	      if (is_quoted)
+		end_quote = skip_quoted (*argptr);
+	    }
+	}
+    }
+#if 0
+  /* No one really seems to know why this was added. It certainly
+     breaks the command line, though, whenever the passed
+     name is of the form ClassName::Method. This bit of code
+     singles out the class name, and if funfirstline is set (for
+     example, you are setting a breakpoint at this function),
+     you get an error. This did not occur with earlier
+     verions, so I am ifdef'ing this out. 3/29/99 */
+  else
+    {
+      /* Check if what we have till now is a symbol name */
 
-      /* Check for single quotes on the non-filename part.  */
-      if (!is_quoted)
+      /* We may be looking at a template instantiation such
+         as "foo<int>".  Check here whether we know about it,
+         instead of falling through to the code below which
+         handles ordinary function names, because that code
+         doesn't like seeing '<' and '>' in a name -- the
+         skip_quoted call doesn't go past them.  So see if we
+         can figure it out right now. */
+
+      copy = (char *) alloca (p - *argptr + 1);
+      memcpy (copy, *argptr, p - *argptr);
+      copy[p - *argptr] = '\000';
+      sym = lookup_symbol (copy, 0, VAR_DOMAIN, 0);
+      if (sym)
 	{
-	  is_quoted = (**argptr
-		       && strchr (get_gdb_completer_quote_characters (),
-				  **argptr) != NULL);
-	  if (is_quoted)
-	    end_quote = skip_quoted (*argptr);
+	  *argptr = (*p == '\'') ? p + 1 : p;
+	  return symbol_found (funfirstline, canonical, copy, sym, NULL);
 	}
+      /* Otherwise fall out from here and go to file/line spec
+         processing, etc. */
     }
+#endif
 
   /* file_symtab is specified file's symtab, or 0 if no file specified.
      arg no longer contains the file name.  */
@@ -866,6 +875,10 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
       /* allow word separators in method names for Obj-C.  */
       p = skip_quoted_chars (*argptr, NULL, "");
     }
+  else if (paren_pointer != NULL)
+    {
+      p = paren_pointer + 1;
+    }
   else
     {
       p = skip_quoted (*argptr);
@@ -875,14 +888,6 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
   if (*p == '<')
     p = find_template_name_end (p);
 
-  /* Keep method overload information.  */
-  if (*p == '(')
-    p = find_method_overload_end (p);
-
-  /* Make sure we keep important kewords like "const".  */
-  if (strncmp (p, " const", 6) == 0)
-    p += 6;
-
   copy = (char *) alloca (p - *argptr + 1);
   memcpy (copy, *argptr, p - *argptr);
   copy[p - *argptr] = '\0';
@@ -940,9 +945,10 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
    function is passed ARGPTR as an argument, it modifies what ARGPTR
    points to; typically, it advances *ARGPTR past whatever substring
    it has just looked at.  (If it doesn't modify *ARGPTR, then the
-   function gets passed *ARGPTR instead, which is then called ARG.)
-   Also, functions that return a struct symtabs_and_lines may modify
-   CANONICAL, as in the description of decode_line_1.
+   function gets passed *ARGPTR instead, which is then called ARG: see
+   set_flags, for example.)  Also, functions that return a struct
+   symtabs_and_lines may modify CANONICAL, as in the description of
+   decode_line_1.
 
    If a function returns a struct symtabs_and_lines, then that struct
    will immediately make its way up the call chain to be returned by
@@ -969,6 +975,44 @@ initialize_defaults (struct symtab **default_symtab, int *default_line)
     }
 }
 
+static void
+set_flags (char *arg, int *is_quoted, char **paren_pointer)
+{
+  char *ii;
+  int has_if = 0;
+
+  /* 'has_if' is for the syntax:
+        (gdb) break foo if (a==b)
+  */
+  if ((ii = strstr (arg, " if ")) != NULL ||
+      (ii = strstr (arg, "\tif ")) != NULL ||
+      (ii = strstr (arg, " if\t")) != NULL ||
+      (ii = strstr (arg, "\tif\t")) != NULL ||
+      (ii = strstr (arg, " if(")) != NULL ||
+      (ii = strstr (arg, "\tif( ")) != NULL)
+    has_if = 1;
+  /* Temporarily zap out "if (condition)" to not confuse the
+     parenthesis-checking code below.  This is undone below. Do not
+     change ii!!  */
+  if (has_if)
+    {
+      *ii = '\0';
+    }
+
+  *is_quoted = (*arg
+		&& strchr (get_gdb_completer_quote_characters (),
+			   *arg) != NULL);
+
+  *paren_pointer = strchr (arg, '(');
+  if (*paren_pointer != NULL)
+    *paren_pointer = strrchr (*paren_pointer, ')');
+
+  /* Now that we're safely past the paren_pointer check, put back " if
+     (condition)" so outer layers can see it.  */
+  if (has_if)
+    *ii = ' ';
+}
+
 \f
 
 /* Decode arg of the form *PC.  */
@@ -1076,9 +1120,8 @@ locate_first_half (char **argptr, int *is_quote_enclosed)
       if (p[0] == '.' && strchr (p, ':') == NULL)
 	{
 	  /* Java qualified method.  Find the *last* '.', since the
-	     others are package qualifiers.  Stop at any open parenthesis
-	     which might provide overload information.  */
-	  for (p1 = p; *p1 && *p1 != '('; p1++)
+	     others are package qualifiers.  */
+	  for (p1 = p; *p1; p1++)
 	    {
 	      if (*p1 == '.')
 		p = p1;
@@ -1236,7 +1279,6 @@ decode_compound (char **argptr, int funfirstline, char ***canonical,
   char *copy;
   struct symbol *sym_class;
   struct type *t;
-  char *saved_java_argptr = NULL;
 
   /* First check for "global" namespace specification, of the form
      "::foo".  If found, skip over the colons and jump to normal
@@ -1285,8 +1327,7 @@ decode_compound (char **argptr, int funfirstline, char ***canonical,
       /* PASS2: p2->"::fun", p->":fun" */
 
       /* Move pointer ahead to next double-colon.  */
-      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\'')
-	     && (*p != '('))
+      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\''))
 	{
 	  if (current_language->la_language == language_cplus)
 	    p += cp_validate_operator (p);
@@ -1364,11 +1405,9 @@ decode_compound (char **argptr, int funfirstline, char ***canonical,
       else
 	{
 	  /* At this point argptr->"fun".  */
-	  char *a;
 
 	  p = *argptr;
-	  while (*p && *p != ' ' && *p != '\t' && *p != ',' && *p != ':'
-		 && *p != '(')
+	  while (*p && *p != ' ' && *p != '\t' && *p != ',' && *p != ':')
 	    p++;
 	  /* At this point p->"".  String ended.  */
 	  /* Nope, C++ operators could have spaces in them
@@ -1380,43 +1419,6 @@ decode_compound (char **argptr, int funfirstline, char ***canonical,
 	      /* The above loop has already swallowed "operator".  */
 	      p += cp_validate_operator (p - 8) - 8;
 	    }
-
-	  /* Keep any template parameters.  */
-	  if (*p == '<')
-	    p = find_template_name_end (p);
-
-	  /* Keep method overload information.  */
-	  a = strchr (p, '(');
-	  if (a != NULL)
-	    p = find_method_overload_end (a);
-
-	  /* Make sure we keep important kewords like "const".  */
-	  if (strncmp (p, " const", 6) == 0)
-	    p += 6;
-
-	  /* Java may append typenames,  so assume that if there is
-	     anything else left in *argptr, it must be a typename.  */
-	  if (*p && current_language->la_language == language_java)
-	    {
-	      struct type *type;
-
-	      p2 = p;
-	      while (*p2)
-		++p2;
-	      copy = (char *) alloca (p2 - p + 1);
-	      memcpy (copy, p, p2 - p);
-	      copy[p2 - p] = '\0';
-	      type = lookup_typename (current_language, get_current_arch (),
-				      copy, NULL, 1);
-	      if (type != NULL)
-		{
-		  /* Save the location of this just in case this
-		     method/type combination isn't actually defined.
-		     It will be checked later.  */
-		  saved_java_argptr = p;
-		  p = p2;
-		}
-	    }
 	}
 
       /* Allocate our own copy of the substring between argptr and
@@ -1445,27 +1447,9 @@ decode_compound (char **argptr, int funfirstline, char ***canonical,
 	 here, we return.  If not, and we are at the and of the string,
 	 we'll lookup the whole string in the symbol tables.  */
 
-      values = find_method (funfirstline, canonical, saved_arg,
-			    copy, t, sym_class, not_found_ptr);
-      if (saved_java_argptr != NULL && values.nelts == 1)
-	{
-	  /* The user specified a specific return type for a java method.
-	     Double-check that it really is the one the user specified.
-	     [This is a necessary evil because strcmp_iw_ordered stops
-	     comparisons too prematurely.]  */
-	  sym = find_pc_sect_function (values.sals[0].pc,
-				       values.sals[0].section);
-	  /* We just found a SAL, we had better be able to go backwards!  */
-	  gdb_assert (sym != NULL);
-	  if (strcmp_iw (SYMBOL_LINKAGE_NAME (sym), saved_arg) != 0)
-	    {
-	      xfree (values.sals);
-	      error (_("the class `%s' does not have "
-		       "any method instance named %s"),
-		     SYMBOL_PRINT_NAME (sym_class), copy);
-	    }
-	}
-      return values;
+      return find_method (funfirstline, canonical, saved_arg,
+			  copy, t, sym_class, not_found_ptr);
+
     } /* End if symbol found.  */
 
 
@@ -1593,43 +1577,8 @@ find_method (int funfirstline, char ***canonical, char *saved_arg,
     }
   if (i1 > 0)
     {
-      /* If we were given a specific overload instance, use that
-	 (or error if no matches were found).  Otherwise ask the user
-	 which one to use.  */
-      if (strchr (saved_arg, '(') != NULL)
-	{
-	  int i;
-	  char *name = saved_arg;
-	  char *canon = cp_canonicalize_string (name);
-	  struct cleanup *cleanup;
-
-	  if (canon != NULL)
-	    {
-	      name = canon;
-	      cleanup = make_cleanup (xfree, canon);
-	    }
-	  else
-	    cleanup = make_cleanup (null_cleanup, NULL);
-
-	  for (i = 0; i < i1; ++i)
-	    {
-	      if (strcmp_iw (name, SYMBOL_LINKAGE_NAME (sym_arr[i])) == 0)
-		{
-		  values.sals = (struct symtab_and_line *)
-		    xmalloc (sizeof (struct symtab_and_line));
-		  values.nelts = 1;
-		  values.sals[0] = find_function_start_sal (sym_arr[i],
-							    funfirstline);
-		  do_cleanups (cleanup);
-		  return values;
-		}
-	    }
-
-	  error (_("the class `%s' does not have "
-		   "any method instance named %s"),
-		 SYMBOL_PRINT_NAME (sym_class), copy);
-	}
-
+      /* There is more than one field with that name
+	 (overloaded).  Ask the user which one to use.  */
       return decode_line_2 (sym_arr, i1, funfirstline, canonical);
     }
   else

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [patch 2/3] Keith's psymtabs fix  [Re: [RFA] c++/11734 revisited]
  2010-12-14 20:03   ` Keith Seitz
                       ` (4 preceding siblings ...)
  2011-02-06 22:46     ` [patch 3/3] Various linespec fixups " Jan Kratochvil
@ 2011-02-06 22:46     ` Jan Kratochvil
  5 siblings, 0 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-02-06 22:46 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

Hi Keith,

[patch 2/3] is application of the 11734 non-decode_compound part psymtabs fix.

this is a part of the patch of yours which should be applied in any case.


Jan


--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -35,6 +35,7 @@
 #include "exceptions.h"
 #include "expression.h"
 #include "value.h"
+#include "language.h"
 
 #include "safe-ctype.h"
 
@@ -936,7 +937,8 @@ make_symbol_overload_list_qualified (const char *func_name)
   ALL_OBJFILES (objfile)
   {
     if (objfile->sf)
-      objfile->sf->qf->expand_symtabs_for_function (objfile, func_name);
+      objfile->sf->qf->expand_symtabs_for_function (objfile, func_name,
+						    language_cplus);
   }
 
   /* Search upwards from currently selected frame (so that we can
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2362,7 +2362,8 @@ dw2_lookup_symtab (struct objfile *objfile, const char *name,
 
 static struct symtab *
 dw2_lookup_symbol (struct objfile *objfile, int block_index,
-		   const char *name, domain_enum domain)
+		   const char *name, domain_enum domain,
+		   enum language language)
 {
   /* We do all the work in the pre_expand_symtabs_matching hook
      instead.  */
@@ -2438,7 +2439,8 @@ dw2_relocate (struct objfile *objfile, struct section_offsets *new_offsets,
 
 static void
 dw2_expand_symtabs_for_function (struct objfile *objfile,
-				 const char *func_name)
+				 const char *func_name,
+				 enum language language)
 {
   dw2_do_expand_symtabs_matching (objfile, func_name);
 }
@@ -2498,7 +2500,8 @@ dw2_expand_symtabs_with_filename (struct objfile *objfile,
 }
 
 static const char *
-dw2_find_symbol_file (struct objfile *objfile, const char *name)
+dw2_find_symbol_file (struct objfile *objfile, const char *name,
+		      enum language language)
 {
   struct dwarf2_per_cu_data *per_cu;
   offset_type *vec;
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -33,6 +33,8 @@
 #include "readline/readline.h"
 #include "gdb_regex.h"
 #include "dictionary.h"
+#include "language.h"
+#include "cp-support.h"
 
 #ifndef DEV_TTY
 #define DEV_TTY "/dev/tty"
@@ -55,7 +57,8 @@ static struct partial_symbol *match_partial_symbol (struct partial_symtab *,
 
 static struct partial_symbol *lookup_partial_symbol (struct partial_symtab *,
 						     const char *, int,
-						     domain_enum);
+						     domain_enum,
+						     enum language);
 
 static char *psymtab_to_fullname (struct partial_symtab *ps);
 
@@ -418,15 +421,35 @@ fixup_psymbol_section (struct partial_symbol *psym, struct objfile *objfile)
 static struct symtab *
 lookup_symbol_aux_psymtabs (struct objfile *objfile,
 			    int block_index, const char *name,
-			    const domain_enum domain)
+			    const domain_enum domain, enum language language)
 {
   struct partial_symtab *ps;
   const int psymtab_index = (block_index == GLOBAL_BLOCK ? 1 : 0);
 
   ALL_OBJFILE_PSYMTABS (objfile, ps)
   {
-    if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
+    if (!ps->readin
+	&& lookup_partial_symbol (ps, name, psymtab_index, domain, language))
+      {
+	struct symbol *sym;
+	struct symtab *stab = PSYMTAB_TO_SYMTAB (ps);
+	sym = NULL;
+
+	/* Some caution must be observed with overloaded functions
+	   and methods, since the psymtab will not contain any overload
+	   information (but NAME might contain it).  */
+	if (stab->primary)
+	  {
+	    struct blockvector *bv = BLOCKVECTOR (stab);
+	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
+	    sym = lookup_block_symbol (block, name, domain);
+	  }
+
+	if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
+	  return stab;
+
+	/* Keep looking through other psymtabs.  */
+      }
   }
 
   return NULL;
@@ -519,22 +542,58 @@ pre_expand_symtabs_matching_psymtabs (struct objfile *objfile,
   /* Nothing.  */
 }
 
+/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
+   not contain any method/function instance information (since this would
+   force reading type information while reading psymtabs).  Therefore,
+   if NAME contains overload information, it must be stripped before searching
+   psymtabs.
+
+   The caller is responsible for freeing the return result.  */
+
+static const char *
+psymtab_search_name (const char *name, enum language language)
+{
+  switch (language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+       if (strchr (name, '('))
+         {
+           char *ret = cp_remove_params (name);
+           if (ret)
+             return ret;
+         }
+      }
+
+    default:
+      break;
+    }
+
+  return xstrdup (name);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    Check the global symbols if GLOBAL, the static symbols if not.  */
 
 static struct partial_symbol *
 lookup_partial_symbol (struct partial_symtab *pst, const char *name,
-		       int global, domain_enum domain)
+		       int global, domain_enum domain, enum language language)
 {
   struct partial_symbol **start, **psym;
   struct partial_symbol **top, **real_top, **bottom, **center;
   int length = (global ? pst->n_global_syms : pst->n_static_syms);
   int do_linear_search = 1;
+  const char *search_name;
+  struct cleanup *cleanup;
 
   if (length == 0)
     {
       return (NULL);
     }
+
+  search_name = psymtab_search_name (name, language);
+  cleanup = make_cleanup (xfree, (void *) search_name);
   start = (global ?
 	   pst->objfile->global_psymbols.list + pst->globals_offset :
 	   pst->objfile->static_psymbols.list + pst->statics_offset);
@@ -563,7 +622,8 @@ lookup_partial_symbol (struct partial_symtab *pst, const char *name,
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
+				 search_name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -577,11 +637,14 @@ lookup_partial_symbol (struct partial_symtab *pst, const char *name,
 			_("failed internal consistency check"));
 
       while (top <= real_top
-	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
 				     SYMBOL_DOMAIN (*top), domain))
-	    return (*top);
+	    {
+	      do_cleanups (cleanup);
+	      return (*top);
+	    }
 	  top++;
 	}
     }
@@ -596,10 +659,14 @@ lookup_partial_symbol (struct partial_symtab *pst, const char *name,
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
 				     SYMBOL_DOMAIN (*psym), domain)
 	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
-	    return (*psym);
+	    {
+	      do_cleanups (cleanup);
+	      return (*psym);
+	    }
 	}
     }
 
+  do_cleanups (cleanup);
   return (NULL);
 }
 
@@ -911,7 +978,8 @@ dump_psymtabs_for_objfile (struct objfile *objfile)
    by matching FUNC_NAME.  Make sure we read that symbol table in.  */
 
 static void
-read_symtabs_for_function (struct objfile *objfile, const char *func_name)
+read_symtabs_for_function (struct objfile *objfile, const char *func_name,
+			   enum language language)
 {
   struct partial_symtab *ps;
 
@@ -920,9 +988,9 @@ read_symtabs_for_function (struct objfile *objfile, const char *func_name)
     if (ps->readin)
       continue;
 
-    if ((lookup_partial_symbol (ps, func_name, 1, VAR_DOMAIN)
+    if ((lookup_partial_symbol (ps, func_name, 1, VAR_DOMAIN, language)
 	 != NULL)
-	|| (lookup_partial_symbol (ps, func_name, 0, VAR_DOMAIN)
+	|| (lookup_partial_symbol (ps, func_name, 0, VAR_DOMAIN, language)
 	    != NULL))
       psymtab_to_symtab (ps);
   }
@@ -1042,13 +1110,14 @@ psymtab_to_fullname (struct partial_symtab *ps)
 }
 
 static const char *
-find_symbol_file_from_partial (struct objfile *objfile, const char *name)
+find_symbol_file_from_partial (struct objfile *objfile, const char *name,
+			       enum language language)
 {
   struct partial_symtab *pst;
 
   ALL_OBJFILE_PSYMTABS (objfile, pst)
     {
-      if (lookup_partial_symbol (pst, name, 1, VAR_DOMAIN))
+      if (lookup_partial_symbol (pst, name, 1, VAR_DOMAIN, language))
 	return pst->filename;
     }
   return NULL;
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -167,14 +167,15 @@ struct quick_symbol_functions
   /* Check to see if the symbol is defined in a "partial" symbol table
      of OBJFILE.  KIND should be either GLOBAL_BLOCK or STATIC_BLOCK,
      depending on whether we want to search global symbols or static
-     symbols.  NAME is the name of the symbol to look for.  DOMAIN
-     indicates what sort of symbol to search for.
+     symbols.  NAME (valid in LANGUAGE) is the name of the symbol to look for.
+     DOMAIN indicates what sort of symbol to search for.
 
      Returns the newly-expanded symbol table in which the symbol is
      defined, or NULL if no such symbol table exists.  */
   struct symtab *(*lookup_symbol) (struct objfile *objfile,
 				   int kind, const char *name,
-				   domain_enum domain);
+				   domain_enum domain,
+				   enum language language);
 
   /* This is called to expand symbol tables before looking up a
      symbol.  A backend can choose to implement this and then have its
@@ -200,10 +201,11 @@ struct quick_symbol_functions
 		    struct section_offsets *new_offsets,
 		    struct section_offsets *delta);
 
-  /* Find all the symbols in OBJFILE named FUNC_NAME, and ensure that
-     the corresponding symbol tables are loaded.  */
+  /* Find all the symbols in OBJFILE named FUNC_NAME (valid in LANGUAGE),
+     and ensure that the corresponding symbol tables are loaded.  */
   void (*expand_symtabs_for_function) (struct objfile *objfile,
-				       const char *func_name);
+				       const char *func_name,
+				       enum language language);
 
   /* Read all symbol tables associated with OBJFILE.  */
   void (*expand_all_symtabs) (struct objfile *objfile);
@@ -217,8 +219,10 @@ struct quick_symbol_functions
 					const char *filename);
 
   /* Return the file name of the file holding the symbol in OBJFILE
-     named NAME.  If no such symbol exists in OBJFILE, return NULL.  */
-  const char *(*find_symbol_file) (struct objfile *objfile, const char *name);
+     named NAME (valid in LANGUAGE).  If no such symbol exists in OBJFILE,
+     return NULL.  */
+  const char *(*find_symbol_file) (struct objfile *objfile, const char *name,
+				   enum language language);
 
   /* Find global or static symbols in all tables that are in NAMESPACE 
      and for which MATCH (symbol name, NAME) == 0, passing each to 
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1380,7 +1380,8 @@ lookup_symbol_aux_quick (struct objfile *objfile, int kind,
 
   if (!objfile->sf)
     return NULL;
-  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, domain);
+  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, domain,
+					   current_language->la_language);
   if (!symtab)
     return NULL;
 
@@ -1554,7 +1555,8 @@ basic_lookup_transparent_type_quick (struct objfile *objfile, int kind,
 
   if (!objfile->sf)
     return NULL;
-  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, STRUCT_DOMAIN);
+  symtab = objfile->sf->qf->lookup_symbol (objfile, kind, name, STRUCT_DOMAIN,
+					   current_language->la_language);
   if (!symtab)
     return NULL;
 
@@ -1686,7 +1688,8 @@ find_main_filename (void)
 
     if (!objfile->sf)
       continue;
-    result = objfile->sf->qf->find_symbol_file (objfile, name);
+    result = objfile->sf->qf->find_symbol_file (objfile, name,
+						current_language->la_language);
     if (result)
       return result;
   }

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [patch 3/3] Various linespec fixups  [Re: [RFA] c++/11734 revisited]
  2010-12-14 20:03   ` Keith Seitz
                       ` (3 preceding siblings ...)
  2011-02-06 22:46     ` [patch 1/3] revert physname part (b) [Re: [RFA] c++/11734 revisited] Jan Kratochvil
@ 2011-02-06 22:46     ` Jan Kratochvil
  2011-02-06 22:46     ` [patch 2/3] Keith's psymtabs fix " Jan Kratochvil
  5 siblings, 0 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-02-06 22:46 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Tom Tromey, gdb-patches

Hi Keith,

[patch 3/3] is reapplication of (c).

that is various linespec fixes which you did along in the original physname
patch.  THis set is not yet complete.


Jan


--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -40,6 +40,7 @@
 #include "interps.h"
 #include "mi/mi-cmds.h"
 #include "target.h"
+#include <ctype.h>
 
 /* We share this one with symtab.c, but it is not exported widely.  */
 
@@ -1007,6 +1008,17 @@ set_flags (char *arg, int *is_quoted, char **paren_pointer)
   if (*paren_pointer != NULL)
     *paren_pointer = strrchr (*paren_pointer, ')');
 
+  /* Make sure we keep important kewords like "const" */
+  if (*paren_pointer != NULL)
+    {
+      /* Skip trailing Java types.  */
+      while ((*paren_pointer)[1] && !isspace ((*paren_pointer)[1]))
+	(*paren_pointer)++;
+
+      if (strncmp (*paren_pointer + 1, " const", 6) == 0)
+	(*paren_pointer) += 6;
+    }
+
   /* Now that we're safely past the paren_pointer check, put back " if
      (condition)" so outer layers can see it.  */
   if (has_if)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-06 22:45     ` [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273) Jan Kratochvil
@ 2011-02-08 21:42       ` Tom Tromey
  2011-02-10 21:45       ` Keith Seitz
  1 sibling, 0 replies; 29+ messages in thread
From: Tom Tromey @ 2011-02-08 21:42 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Keith Seitz, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> If the code should be nice I tried archer-jankratochvil-linespec where
Jan> linespec is based on the expressions.  Noted by Daniel Jacobowitz before:
Jan> 	http://sourceware.org/ml/gdb-patches/2009-11/msg00266.html

I think this would be a good direction to head.

One difficulty is that, right now, linespecs for one language often work
while the current language is set to something else.  This doesn't work
for Java, I think, but I think that is the point of the Objective C
hooks in linespec.c.

I'm not sure this is important enough that it is worth the maintenance
headache we have with linespecs.

Jan> It still has some regressions but the most common C++ cases work
Jan> there and I find it doable with some more time (mostly if the error
Jan> messages can be changed).

As long as the messages make sense to the user, I think the wording is
negotiable.

Jan>  The are problems with:
Jan>  * expression evaluator cares about the function value (=address) where the
Jan>    function symbol for linespec is already dropped.

Jan>  * linespec should have no side effects.  But the expression evaluator's
Jan>    EVAL_AVOID_SIDE_EFFECTS mode cares only about types, not about values.

It would be useful in other situations to be able to disable side
effects but still evaluate an expression.  I think there was a bug
report requesting this for GUIs, but I am not sure.

It would be nice if we could limit the parser to just consider names.
Maybe instead of evaluating the expression we could examine it and
extract the information we need from the parsed form.  Or, just make a
new language entry point and make each language do this internally.

Tom

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-06 22:45     ` [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273) Jan Kratochvil
  2011-02-08 21:42       ` Tom Tromey
@ 2011-02-10 21:45       ` Keith Seitz
  2011-02-17 18:37         ` Keith Seitz
  1 sibling, 1 reply; 29+ messages in thread
From: Keith Seitz @ 2011-02-10 21:45 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches

I apologize to everyone, my Internet connectivity died on me last week, 
and I've only just had service restored this morning...

Honestly, I wasn't trying to ignore anyone. [Although I would love to be 
able to ignore linespec.c! :-)]

On 02/06/2011 02:45 PM, Jan Kratochvil wrote:

> I do not fully understand the reasons for part (b).  The old code is not nice
> but IMO neither is the new code (already checked in by the physname patch) due to
> the linespec.c caller.

One of the intents behind the patchset was to make the code behave the 
way it was documented. Forcing linespecs that would normally be handled 
by decode_compound into decode_variable based on the existence of 
overload information was (and still is) just plain wrong to me.

Mind you, that doesn't necessarily mean that decode_compound isn't evil 
or superfluous.

>  Moreover the new code has shown its regressions.

I'd like to make clear the meaning of the word "regression" in this 
context. Are we experiencing some fallout in linespecs because of this? 
Yes, we are. Are these bugs introduced by the patch? No. These are bugs 
that have existed for many years, but since the code did not do what the 
comments said it should do, these bugs lay hidden all this time inside 
decode_compound. And you have discovered another one (more on this 
later). And I found one more yesterday, too.

I did not set out to rewrite linespecs, just make them work with the 
least amount of invasion. I don't like rewriting tons of fragile, 
relatively undocumented code to fix a bug (even a non-trivial one) 
unless it is known/understood a priori that a redesign/rewrite is 
expected or wanted.

> If the code should be nice I tried archer-jankratochvil-linespec where
> linespec is based on the expressions.  Noted by Daniel Jacobowitz before:
> 	http://sourceware.org/ml/gdb-patches/2009-11/msg00266.html

I would love to see us allocate time to someone to finish the work on 
this approach. It would be a huge maintenance win -- even if it comes 
with its own set of problems or regressions. O:-) [That's the cost of 
progress IMO.]

> That is in general I would be either for futher not-nice fixing up the
> pre-physname code or for the expression way like archer-jankratochvil-linespec
> does.  Still at the moment your patchset gives the best user experience, just
> it is a new untested code which does not make it nice anyway.

I'm not entirely sure if I need to respond to your other messages on 
this topic -- I'm *way* out of context by now, but please ping me on IRC 
or email if there is something specific you'd like me to address.

Returning to the bug you discovered ("regression"), I do want to mention 
why decode_variable works and decode_compound does not. [Reminder: this 
is demonstrated by your namespace addition to the pr11734.exp test case.]

The answer is actually really simple: deocde_variable calls 
lookup_symbol with a valid block; decode_compound does not. Passing 
"get_selected_block (0)" to lookup_symbol [more or less] fixes the 
problems. [Follow-on patch to fix the fallout of that once I know what 
direction I'm being asked to go with this.]

I have expanded the original pr11734.exp test case even further by 
adding another namespace and class inside it, e.g., "A::B::MyClass", and 
this exposed another place where lookup_symbol was not getting a valid 
block to work with. Again, trivially fixed.

So, I'm not quite sure where I stand with this 11734/12273 (and other 
related bugs)... What do you want me to do?

Keith

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-10 21:45       ` Keith Seitz
@ 2011-02-17 18:37         ` Keith Seitz
  2011-02-18  3:24           ` Keith Seitz
  2011-02-21 11:41           ` Jan Kratochvil
  0 siblings, 2 replies; 29+ messages in thread
From: Keith Seitz @ 2011-02-17 18:37 UTC (permalink / raw)
  To: gdb-patches

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

On 02/10/2011 01:45 PM, Keith Seitz wrote:
> So, I'm not quite sure where I stand with this 11734/12273 (and other
> related bugs)... What do you want me to do?

In private IRC, Jan recommended that I post an update of my patches. I 
am including both here.

I believe I made all the changes Jan recommended, but I may have missed 
something. If so, please let me know, and I will fix immediately.

These patches are largely the same as the past two I posted for 11734 
and 12273, but there is, of necessity, a handful of noteworthy 
additions/changes worth mentioning here (perhaps if nothing more than to 
facilitate review).

- The test case for 11734 (now called ovsrch.exp) was modified to expose 
more bugs.
- Fixes for the above bugs (passing valid blocks to lookup_symbol in 
several places, reconstructing search name based on SYM_CLASS in 
find_method -- see code for detailed explanation)
- Unilateral removal of the single quote in decode_compound

These patches cause no regressions in the test suite on x86_64 linux, 
but that doesn't mean that they won't eventually uncover some other 
hidden bug(s).

Questions/comments/concerns?

Keith

ChangeLog
2011-02-17  Keith Seitz  <keiths@redhat.com>

         PR c++/12273
         * linespec.c (locate_first_half): Keep overload information, too.
         (decode_compound): Use a string to represent break characters
         to escape the loop.
         If P points to a break character, do not increment it.
         For C++ and Java, keep overload information and relevant keywords.
         If we cannot find a symbol, search the minimal symbols.

         PR c++/11734
         * linespec.c (decode_compound): Rename SAVED_ARG to
         THE_REAL_SAVED_ARG.
         Make a copy of THE_REAL_SAVED_ARG in SAVED_ARG and strip
         single-quotes.
         Pass a valid block to lookup_symbol.
         (lookup_prefix_sym): Likewise.
         (find_method): Construct search name based on SYM_CLASS instead
         of SAVED_ARG.
         * psymtab.c (lookup_partial_symbol): Add language parameter.
         (lookup_symbol_aux_psymtabs): Likewise.
         Don't assume that the psymtab we found was the right one. Search
         for the desired symbol in the symtab to be certain.
         (psymtab_search_name): New function.
         (lookup_partial_symbol): Use psymtab_search_name.
         Add language parameter.
         (read_symtabs_for_function): Add language parameter and pass to
         lookup_partial_symbol.
         (find_symbol_file_from_partial): Likewise.
         * symfile.h (struct quick_symbol_functions): Add language parameter
         to lookup_symbol, expand_symtabs_for_function, and 
find_symbol_file.
         * cp-support.c (make_symbol_overload_list): Update above API
         changes.
         * symtab.c (lookup_symbol_aux_quick): Pass the current language
         to the quick symbol functions.
         (basic_lookup_transparent_type_quick): Likewise.
         (find_main_filename): Likewise.
         * dwarf2_read.c (dw2_lookup_symbol): Add langauge parameter.
         (dw2_expand_symtabs_for_function): Likewise.
         (dw2_find_symbol_file): Likewise.

testsuite/ChangeLog
2011-02-17  Keith Seitz  <keiths@redhat.com>

	PR c++/12273
	* gdb.cp/cmpd-minsyms.exp: New test.
	* gdb.cp/cmpd-minsyms.cc: New file.

	PR c++/11734
	* gdb.cp/ovsrch.exp: New test.
	* gdb.cp/ovsrch.h: New file.
	* gdb.cp/ovsrch1.cc: New file.
	* gdb.cp/ovsrch2.cc: New file.
	* gdb.cp/ovsrch3.cc: New file.
	* gdb.cp/ovsrch4.cc: New file.

[-- Attachment #2: 11734-12273.patch --]
[-- Type: text/plain, Size: 24015 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.109
diff -u -p -r1.109 linespec.c
--- linespec.c	9 Jan 2011 03:08:57 -0000	1.109
+++ linespec.c	17 Feb 2011 17:45:12 -0000
@@ -1057,6 +1057,10 @@ locate_first_half (char **argptr, int *i
 	    error (_("malformed template specification in command"));
 	  p = temp_end;
 	}
+
+      if (p[0] == '(')
+	p = find_method_overload_end (p);
+
       /* Check for a colon and a plus or minus and a [ (which
          indicates an Objective-C method).  */
       if (is_objc_method_format (p))
@@ -1226,7 +1230,7 @@ decode_objc (char **argptr, int funfirst
 
 static struct symtabs_and_lines
 decode_compound (char **argptr, int funfirstline, char ***canonical,
-		 char *saved_arg, char *p, int *not_found_ptr)
+		 char *the_real_saved_arg, char *p, int *not_found_ptr)
 {
   struct symtabs_and_lines values;
   char *p2;
@@ -1237,6 +1241,23 @@ decode_compound (char **argptr, int funf
   struct symbol *sym_class;
   struct type *t;
   char *saved_java_argptr = NULL;
+  char *saved_arg;
+
+  /* If the user specified any single-quotes in the input, strip them.
+     They are superfluous.  */
+  saved_arg = alloca (strlen (the_real_saved_arg) + 1);
+  {
+    char *dst = saved_arg;
+    char *src = the_real_saved_arg;
+
+    while (*src != '\0')
+      {
+	if (*src != '\'')
+	  *dst++ = *src;
+	++src;
+      }
+    *dst = '\0';
+  }
 
   /* First check for "global" namespace specification, of the form
      "::foo".  If found, skip over the colons and jump to normal
@@ -1253,8 +1274,10 @@ decode_compound (char **argptr, int funf
         find_method.
 
      2) AAA::inA isn't the name of a class.  In that case, either the
-        user made a typo or AAA::inA is the name of a namespace.
-        Either way, we just look up AAA::inA::fun with lookup_symbol.
+        user made a typo, AAA::inA is the name of a namespace, or it is
+        the name of a minimal symbol.
+        We just look up AAA::inA::fun with lookup_symbol.  If that fails,
+        try lookup_minimal_symbol.
 
      Thus, our first task is to find everything before the last set of
      double-colons and figure out if it's the name of a class.  So we
@@ -1275,6 +1298,8 @@ decode_compound (char **argptr, int funf
 
   while (1)
     {
+      static char *break_characters = " \t\'(";
+
       /* Move pointer up to next possible class/namespace token.  */
 
       p = p2 + 1;	/* Restart with old value +1.  */
@@ -1285,8 +1310,7 @@ decode_compound (char **argptr, int funf
       /* PASS2: p2->"::fun", p->":fun" */
 
       /* Move pointer ahead to next double-colon.  */
-      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\'')
-	     && (*p != '('))
+      while (*p && strchr (break_characters, *p) == NULL)
 	{
 	  if (current_language->la_language == language_cplus)
 	    p += cp_validate_operator (p);
@@ -1310,9 +1334,12 @@ decode_compound (char **argptr, int funf
 	  else if ((p[0] == ':') && (p[1] == ':'))
 	    break;	/* Found double-colon.  */
 	  else
-	    /* PASS2: We'll keep getting here, until p->"", at which point
-	       we exit this loop.  */
-	    p++;
+	    {
+	      /* PASS2: We'll keep getting here, until P points to one of the
+		 break characters, at which point we exit this loop.  */
+	      if (strchr (break_characters, *p) == NULL)
+		p++;
+	    }
 	}
 
       if (*p != ':')
@@ -1321,7 +1348,7 @@ decode_compound (char **argptr, int funf
 			   unsuccessfully all the components of the
 			   string, and p->""(PASS2).  */
 
-      /* We get here if p points to ' ', '\t', '\'', "::" or ""(i.e
+      /* We get here if p points to one of the break characters or "" (i.e.,
 	 string ended).  */
       /* Save restart for next time around.  */
       p2 = p;
@@ -1472,6 +1499,18 @@ decode_compound (char **argptr, int funf
   /* We couldn't find a class, so we're in case 2 above.  We check the
      entire name as a symbol instead.  */
 
+  if (current_language->la_language == language_cplus
+      || current_language->la_language == language_java)
+    {
+      char *paren = strchr (p, '(');
+      if (paren != NULL)
+	p = find_method_overload_end (paren);
+
+      /* Make sure we keep important kewords like "const" */
+      if (strncmp (p, " const", 6) == 0)
+	p += 6;
+    }
+
   copy = (char *) alloca (p - saved_arg2 + 1);
   memcpy (copy, saved_arg2, p - saved_arg2);
   /* Note: if is_quoted should be true, we snuff out quote here
@@ -1481,15 +1520,24 @@ decode_compound (char **argptr, int funf
   *argptr = (*p == '\'') ? p + 1 : p;
 
   /* Look up entire name.  */
-  sym = lookup_symbol (copy, 0, VAR_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), VAR_DOMAIN, 0);
   if (sym)
     return symbol_found (funfirstline, canonical, copy, sym, NULL);
+  else
+    {
+      struct minimal_symbol *msym;
+
+      /* Couldn't find any interpretation as classes/namespaces.  As a last
+	 resort, try the minimal symbol tables.  */
+      msym = lookup_minimal_symbol (copy, NULL, NULL);
+      if (msym != NULL)
+	return minsym_found (funfirstline, msym);
+    }    
 
-  /* Couldn't find any interpretation as classes/namespaces, so give
-     up.  The quotes are important if copy is empty.  */
+  /* Couldn't find a minimal symbol, either, so give up.  */
   if (not_found_ptr)
     *not_found_ptr = 1;
-  cplusplus_error (saved_arg,
+  cplusplus_error (the_real_saved_arg,
 		   "Can't find member of namespace, "
 		   "class, struct, or union named \"%s\"\n",
 		   copy);
@@ -1528,7 +1576,7 @@ lookup_prefix_sym (char **argptr, char *
   /* At this point p1->"::inA::fun", p->"inA::fun" copy->"AAA",
      argptr->"inA::fun".  */
 
-  sym = lookup_symbol (copy, 0, STRUCT_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), STRUCT_DOMAIN, 0);
   if (sym == NULL)
     {
       /* Typedefs are in VAR_DOMAIN so the above symbol lookup will
@@ -1599,17 +1647,31 @@ find_method (int funfirstline, char ***c
       if (strchr (saved_arg, '(') != NULL)
 	{
 	  int i;
-	  char *name = saved_arg;
-	  char *canon = cp_canonicalize_string (name);
+	  char *name;
+	  char *canon;
 	  struct cleanup *cleanup;
 
+	  /* Construct the proper search name based on SYM_CLASS and COPY.
+	     SAVED_ARG may contain a valid name, but that name might not be
+	     what is actually stored in the symbol table.  For example,
+	     if SAVED_ARG (and SYM_CLASS) were found via an import
+	     ("using namespace" in C++), then the physname of
+	     SYM_CLASS ("A::myclass") may not be the same as SAVED_ARG
+	     ("myclass").  */
+	  name = xmalloc (strlen (SYMBOL_LINKAGE_NAME (sym_class))
+			  + 2 /* "::" */ + strlen (copy) + 1);
+	  strcpy (name, SYMBOL_LINKAGE_NAME (sym_class));
+	  strcat (name, "::");
+	  strcat (name, copy);
+	  canon = cp_canonicalize_string (name);
 	  if (canon != NULL)
 	    {
+	      xfree (name);
 	      name = canon;
 	      cleanup = make_cleanup (xfree, canon);
 	    }
 	  else
-	    cleanup = make_cleanup (null_cleanup, NULL);
+	    cleanup = make_cleanup (xfree, name);
 
 	  for (i = 0; i < i1; ++i)
 	    {
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.22
diff -u -p -r1.22 psymtab.c
--- psymtab.c	10 Jan 2011 20:38:50 -0000	1.22
+++ psymtab.c	17 Feb 2011 17:45:12 -0000
@@ -33,6 +33,8 @@
 #include "readline/readline.h"
 #include "gdb_regex.h"
 #include "dictionary.h"
+#include "language.h"
+#include "cp-support.h"
 
 #ifndef DEV_TTY
 #define DEV_TTY "/dev/tty"
@@ -426,7 +428,26 @@ lookup_symbol_aux_psymtabs (struct objfi
   ALL_OBJFILE_PSYMTABS (objfile, ps)
   {
     if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
+      {
+	struct symbol *sym = NULL;
+	struct symtab *stab = PSYMTAB_TO_SYMTAB (ps);
+
+	/* Some caution must be observed with overloaded functions
+	   and methods, since the psymtab will not contain any overload
+	   information (but NAME might contain it).  */
+	if (stab->primary)
+	  {
+	    struct blockvector *bv = BLOCKVECTOR (stab);
+	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
+
+	    sym = lookup_block_symbol (block, name, domain);
+	  }
+
+	if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
+	  return stab;
+
+	/* Keep looking through other psymtabs.  */
+      }
   }
 
   return NULL;
@@ -519,6 +540,39 @@ pre_expand_symtabs_matching_psymtabs (st
   /* Nothing.  */
 }
 
+/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
+   not contain any method/function instance information (since this would
+   force reading type information while reading psymtabs).  Therefore,
+   if NAME contains overload information, it must be stripped before searching
+   psymtabs.
+
+   The caller is responsible for freeing the return result.  */
+
+static char *
+psymtab_search_name (const char *name)
+{
+  switch (current_language->la_language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+       if (strchr (name, '('))
+         {
+           char *ret = cp_remove_params (name);
+
+           if (ret)
+             return ret;
+         }
+      }
+      break;
+
+    default:
+      break;
+    }
+
+  return xstrdup (name);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    Check the global symbols if GLOBAL, the static symbols if not.  */
 
@@ -530,11 +584,16 @@ lookup_partial_symbol (struct partial_sy
   struct partial_symbol **top, **real_top, **bottom, **center;
   int length = (global ? pst->n_global_syms : pst->n_static_syms);
   int do_linear_search = 1;
+  char *search_name;
+  struct cleanup *cleanup;
 
   if (length == 0)
     {
       return (NULL);
     }
+
+  search_name = psymtab_search_name (name);
+  cleanup = make_cleanup (xfree, search_name);
   start = (global ?
 	   pst->objfile->global_psymbols.list + pst->globals_offset :
 	   pst->objfile->static_psymbols.list + pst->statics_offset);
@@ -563,7 +622,8 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
+				 search_name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -577,11 +637,14 @@ lookup_partial_symbol (struct partial_sy
 			_("failed internal consistency check"));
 
       while (top <= real_top
-	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
 				     SYMBOL_DOMAIN (*top), domain))
-	    return (*top);
+	    {
+	      do_cleanups (cleanup);
+	      return (*top);
+	    }
 	  top++;
 	}
     }
@@ -595,11 +658,15 @@ lookup_partial_symbol (struct partial_sy
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
 				     SYMBOL_DOMAIN (*psym), domain)
-	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
-	    return (*psym);
+	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, search_name))
+	    {
+	      do_cleanups (cleanup);
+	      return (*psym);
+	    }
 	}
     }
 
+  do_cleanups (cleanup);
   return (NULL);
 }
 
Index: testsuite/gdb.cp/cmpd-minsyms.exp
===================================================================
RCS file: testsuite/gdb.cp/cmpd-minsyms.exp
diff -N testsuite/gdb.cp/cmpd-minsyms.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cmpd-minsyms.exp	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,50 @@
+# Copyright 2010 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+if {[skip_cplus_tests]} { continue }
+
+# Test for c++/12273
+set testfile "cmpd-minsyms"
+# Do NOT compile with debug flag.
+prepare_for_testing $testfile $testfile $testfile.cc {c++}
+
+gdb_test_no_output "set language c++"
+
+# A list of minimal symbol names to check.
+# Note that GDB<char>::even_harder<int>(char) is quoted and includes
+# the return type.  This is necessary because this is the demangled name
+# of the minimal symbol.
+set min_syms [list \
+		  "GDB<int>::operator ==" \
+		  "GDB<int>::operator==(GDB<int> const&)" \
+		  "GDB<char>::harder(char)" \
+		  "GDB<int>::harder(int)" \
+		  {"int GDB<char>::even_harder<int>(char)"} \
+		  "GDB<int>::simple()"]
+
+foreach sym $min_syms {
+    set tst "setting breakpoint at $sym"
+    if {[gdb_breakpoint $sym]} {
+	pass $tst
+    } else {
+	fail $tst
+    }
+}
+
+gdb_exit
Index: testsuite/gdb.cp/cmpd-minsyms.cc
===================================================================
RCS file: testsuite/gdb.cp/cmpd-minsyms.cc
diff -N testsuite/gdb.cp/cmpd-minsyms.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cmpd-minsyms.cc	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,37 @@
+/* This test case is part of GDB, the GNU debugger.
+
+   Copyright 2010 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/>.  */
+
+template <typename T>
+class GDB
+{
+ public:
+   static int simple (void) { return 0; }
+   static int harder (T a) { return 1; }
+   template <typename X>
+   static X even_harder (T a) { return static_cast<X> (a); }
+   int operator == (GDB const& other)
+   { return 1; }
+};
+
+int main(int argc, char **argv)
+{
+   GDB<int> a, b;
+   if (a == b)
+     return GDB<char>::harder('a') + GDB<int>::harder(3)
+	+ GDB<char>::even_harder<int> ('a');
+   return GDB<int>::simple ();
+}
Index: testsuite/gdb.cp/ovsrch.exp
===================================================================
RCS file: testsuite/gdb.cp/ovsrch.exp
diff -N testsuite/gdb.cp/ovsrch.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch.exp	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,103 @@
+# Copyright 2011 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+proc test_class {class} {
+
+    # An array holding the overload types for the methods A::outer::foo
+    # and A::B::inner::foo.  The first element is the overloaded method
+    # parameter.  The second element is the expected source file number,
+    # e.g. "ovsrch?.cc".
+    array set tests {
+	"char*"  4
+	"int"    3
+	"void"   2
+    }
+
+    # Test each overload instance twice: once quoted, once unquoted
+    foreach ovld [array names tests] {
+	set method "${class}::foo\($ovld\)"
+	set result "Breakpoint (\[0-9\]).*file .*/ovsrch$tests($ovld).*"
+	gdb_test "break $method" $result
+	gdb_test "break '$method'" $result
+    }
+}
+
+if { [skip_cplus_tests] } { continue }
+
+# Test for c++/11734
+set testfile "ovsrch"
+set binfile [file join $objdir $subdir $testfile]
+
+set srcfiles {}
+for {set i 1} {$i < 5} {incr i} {
+    lappend srcfiles [file join $srcdir $subdir $testfile$i.cc]
+}
+if  {[gdb_compile $srcfiles $binfile executable {debug c++}] != "" } {
+     untested $testfile.exp
+     return -1
+}
+
+if {[get_compiler_info $binfile "c++"]} {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+
+if {![runto_main]} {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# Break in A::stop_here and run tests.
+if {[gdb_breakpoint "stop_here"]} {
+    pass "break stop_here"
+} else {
+    fail "break stop_here"
+}
+
+if {[gdb_breakpoint "'stop_here'"]} {
+    pass "break 'stop_here'"
+} else {
+    fail "break 'stop_here'"
+}
+
+gdb_continue_to_breakpoint "stop_here"
+test_class outer
+
+# Break in A::B::stop_here_too and run tests.
+if {[gdb_breakpoint "B::stop_here_too"]} {
+    pass "break B::stop_here_too"
+} else {
+    fail "break B::stop_here_too"
+}
+
+if {[gdb_breakpoint "'B::stop_here_too'"]} {
+    pass "break 'B::stop_here_too'"
+} else {
+    fail "break 'B::stop_here_too'"
+}
+
+gdb_continue_to_breakpoint "stop_here_too"
+test_class inner
+
+gdb_exit
+return 0
Index: testsuite/gdb.cp/ovsrch.h
===================================================================
RCS file: testsuite/gdb.cp/ovsrch.h
diff -N testsuite/gdb.cp/ovsrch.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch.h	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+namespace A
+{
+  class outer
+  {
+  public:
+    void foo (void);
+    void foo (int);
+    void foo (char *);
+  };
+
+  namespace B
+  {
+    class inner
+    {
+    public:
+      void foo (void);
+      void foo (int);
+      void foo (char *);
+    };
+  }
+}
Index: testsuite/gdb.cp/ovsrch1.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch1.cc
diff -N testsuite/gdb.cp/ovsrch1.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch1.cc	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,41 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+namespace A
+{
+  void stop_here (void) { }
+
+  namespace B
+  {
+    void stop_here_too (void) { }
+  }
+}
+
+using namespace A;
+
+int
+main ()
+{
+  outer *p = new outer;
+  stop_here ();
+  B::stop_here_too ();
+  p->foo ();
+  return 0;
+}
+
Index: testsuite/gdb.cp/ovsrch2.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch2.cc
diff -N testsuite/gdb.cp/ovsrch2.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch2.cc	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (void)
+{
+}
+
+void
+A::B::inner::foo (void)
+{
+}
Index: testsuite/gdb.cp/ovsrch3.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch3.cc
diff -N testsuite/gdb.cp/ovsrch3.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch3.cc	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (int a)
+{
+}
+
+void
+A::B::inner::foo (int a)
+{
+}
Index: testsuite/gdb.cp/ovsrch4.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch4.cc
diff -N testsuite/gdb.cp/ovsrch4.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch4.cc	17 Feb 2011 17:45:12 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (char *a)
+{
+}
+
+void
+A::B::inner::foo (char *a)
+{
+}

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-17 18:37         ` Keith Seitz
@ 2011-02-18  3:24           ` Keith Seitz
  2011-02-21 11:41           ` Jan Kratochvil
  1 sibling, 0 replies; 29+ messages in thread
From: Keith Seitz @ 2011-02-18  3:24 UTC (permalink / raw)
  To: gdb-patches

And of course, no sooner do I submit this, than I see I forgot something...

On 02/17/2011 10:12 AM, Keith Seitz wrote:
> PR c++/11734
> * gdb.cp/ovsrch.exp: New test.

I have changed this to use prepare_for_testing:

@@ -500,21 +500,10 @@
  +
  +set srcfiles {}
  +for {set i 1} {$i < 5} {incr i} {
-+    lappend srcfiles [file join $srcdir $subdir $testfile$i.cc]
-+}
-+if  {[gdb_compile $srcfiles $binfile executable {debug c++}] != "" } {
-+     untested $testfile.exp
-+     return -1
++    lappend srcfiles $testfile$i.cc
  +}
  +
-+if {[get_compiler_info $binfile "c++"]} {
-+    return -1
-+}
-+
-+gdb_exit
-+gdb_start
-+gdb_reinitialize_dir $srcdir/$subdir
-+gdb_load $binfile
++prepare_for_testing $testfile $testfile $srcfiles {c++ debug}
  +
  +if {![runto_main]} {
  +    perror "couldn't run to breakpoint"

Keith

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-17 18:37         ` Keith Seitz
  2011-02-18  3:24           ` Keith Seitz
@ 2011-02-21 11:41           ` Jan Kratochvil
  2011-02-24 20:41             ` Keith Seitz
  2011-02-27 21:18             ` Jan Kratochvil
  1 sibling, 2 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-02-21 11:41 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Hi Keith,

On Thu, 17 Feb 2011 19:12:59 +0100, Keith Seitz wrote:
> These patches are largely the same as the past two I posted for
> 11734 and 12273, but there is, of necessity, a handful of noteworthy
> additions/changes worth mentioning here (perhaps if nothing more
> than to facilitate review).
> 
> - The test case for 11734 (now called ovsrch.exp) was modified to
> expose more bugs.
> - Fixes for the above bugs (passing valid blocks to lookup_symbol in
> several places, reconstructing search name based on SYM_CLASS in
> find_method -- see code for detailed explanation)
> - Unilateral removal of the single quote in decode_compound

- The new parameter `enum language language' was removed all over the code as
  it has been passed only `current_language->la_language' anyway.

mv pr12273.exp  cmpd-minsyms.exp
mv pr12273.cc   cmpd-minsyms.cc
mv pr11734.exp  ovsrch.exp
mv pr11734.h    ovsrch.h
mv pr11734-1.cc ovsrch1.cc
mv pr11734-2.cc ovsrch2.cc
mv pr11734-3.cc ovsrch3.cc
mv pr11734-4.cc ovsrch4.cc


> --- linespec.c	9 Jan 2011 03:08:57 -0000	1.109
> +++ linespec.c	17 Feb 2011 17:45:12 -0000
> @@ -1057,6 +1057,10 @@ locate_first_half (char **argptr, int *i
>  	    error (_("malformed template specification in command"));
>  	  p = temp_end;
>  	}
> +
> +      if (p[0] == '(')
> +	p = find_method_overload_end (p);

pre-physname:
GNU gdb (GDB) 7.1.50.20100309-cvs
(gdb) b A::outer::foo (int)
Breakpoint 1 at 0x4005a3: file ./gdb.cp/ovsrch3.cc, line 23.
(gdb) b A::outer::foo (void)
Breakpoint 2 at 0x40058c: file ./gdb.cp/ovsrch2.cc, line 23.
(gdb) b A::outer::foo (char *)
Breakpoint 3 at 0x4005c0: file ./gdb.cp/ovsrch4.cc, line 23.

patched:
GNU gdb (GDB) 7.2.50.20110220-cvs
(gdb) b A::outer::foo (int)
the class A::outer does not have any method named foo (int)

This is a regression.

There should be some whitespace skipping, as strcmp_iw_ordered does.


> +
>        /* Check for a colon and a plus or minus and a [ (which
>           indicates an Objective-C method).  */
>        if (is_objc_method_format (p))
> @@ -1226,7 +1230,7 @@ decode_objc (char **argptr, int funfirst
>  
>  static struct symtabs_and_lines
>  decode_compound (char **argptr, int funfirstline, char ***canonical,
> -		 char *saved_arg, char *p, int *not_found_ptr)
> +		 char *the_real_saved_arg, char *p, int *not_found_ptr)
>  {
>    struct symtabs_and_lines values;
>    char *p2;
> @@ -1237,6 +1241,23 @@ decode_compound (char **argptr, int funf
>    struct symbol *sym_class;
>    struct type *t;
>    char *saved_java_argptr = NULL;
> +  char *saved_arg;
> +
> +  /* If the user specified any single-quotes in the input, strip them.
> +     They are superfluous.  */
> +  saved_arg = alloca (strlen (the_real_saved_arg) + 1);
> +  {
> +    char *dst = saved_arg;
> +    char *src = the_real_saved_arg;
> +
> +    while (*src != '\0')
> +      {
> +	if (*src != '\'')

It assumes get_gdb_completer_quote_characters() is just "'" as currently is.
It cannot be changed now anyway and for future it should be rather removed
than extended.  It was already hardcoded into decode_compound before physname
patch.

Depending on your choice either to call get_gdb_completer_quote_characters() or
dropping get_gdb_completer_quote_characters() or maybe even a comment
referencing it would suffice.


BTW realized Ada has operator ' (seen in gdb.ada/*/*.adb) but it is applicable
only to types and variables so probably not an issue for linespec.  Googled:
	http://www.dei.isep.ipp.pt/~lpinho/ada95/linux_book/10.html#10.4


> +	  *dst++ = *src;
> +	++src;
> +      }
> +    *dst = '\0';
> +  }
>  
>    /* First check for "global" namespace specification, of the form
>       "::foo".  If found, skip over the colons and jump to normal
> @@ -1253,8 +1274,10 @@ decode_compound (char **argptr, int funf
>          find_method.
>  
>       2) AAA::inA isn't the name of a class.  In that case, either the
> -        user made a typo or AAA::inA is the name of a namespace.
> -        Either way, we just look up AAA::inA::fun with lookup_symbol.
> +        user made a typo, AAA::inA is the name of a namespace, or it is
> +        the name of a minimal symbol.
> +        We just look up AAA::inA::fun with lookup_symbol.  If that fails,
> +        try lookup_minimal_symbol.
>  
>       Thus, our first task is to find everything before the last set of
>       double-colons and figure out if it's the name of a class.  So we
> @@ -1275,6 +1298,8 @@ decode_compound (char **argptr, int funf
>  
>    while (1)
>      {
> +      static char *break_characters = " \t\'(";

Éxcessive backslash.

Some comment and/or removal wrt get_gdb_completer_quote_characters for '.
(OK, this ' hardcoded in decode_compound was present even before physname,
I can clean it up afterwards.)


> +
>        /* Move pointer up to next possible class/namespace token.  */
>  
>        p = p2 + 1;	/* Restart with old value +1.  */
> @@ -1285,8 +1310,7 @@ decode_compound (char **argptr, int funf
>        /* PASS2: p2->"::fun", p->":fun" */
>  
>        /* Move pointer ahead to next double-colon.  */
> -      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\'')
> -	     && (*p != '('))
> +      while (*p && strchr (break_characters, *p) == NULL)
>  	{
>  	  if (current_language->la_language == language_cplus)
>  	    p += cp_validate_operator (p);
> @@ -1310,9 +1334,12 @@ decode_compound (char **argptr, int funf
>  	  else if ((p[0] == ':') && (p[1] == ':'))
>  	    break;	/* Found double-colon.  */
>  	  else
> -	    /* PASS2: We'll keep getting here, until p->"", at which point
> -	       we exit this loop.  */
> -	    p++;
> +	    {
> +	      /* PASS2: We'll keep getting here, until P points to one of the
> +		 break characters, at which point we exit this loop.  */
> +	      if (strchr (break_characters, *p) == NULL)
> +		p++;

*p can == '\0' here, it works - but I would prefer for better readability:
	if (*p && strchr (break_characters, *p) == NULL)
	  p++;


> +	    }
>  	}
>  
>        if (*p != ':')
> @@ -1321,7 +1348,7 @@ decode_compound (char **argptr, int funf
>  			   unsuccessfully all the components of the
>  			   string, and p->""(PASS2).  */
>  
> -      /* We get here if p points to ' ', '\t', '\'', "::" or ""(i.e
> +      /* We get here if p points to one of the break characters or "" (i.e.,
>  	 string ended).  */
>        /* Save restart for next time around.  */
>        p2 = p;
> @@ -1472,6 +1499,18 @@ decode_compound (char **argptr, int funf
>    /* We couldn't find a class, so we're in case 2 above.  We check the
>       entire name as a symbol instead.  */
>  
> +  if (current_language->la_language == language_cplus
> +      || current_language->la_language == language_java)
> +    {
> +      char *paren = strchr (p, '(');
> +      if (paren != NULL)
> +	p = find_method_overload_end (paren);

00000000004004ea W _ZNK1C1mEv
00000000004004ea W C::m() const

pre-physname:
GNU gdb (GDB) 7.1.50.20100309-cvs
(gdb) b C::m if C::m()
Breakpoint 1 at 0x4004f2: file 3.C, line 4.
(gdb) b 'C::m' if C::m()
Breakpoint 1 at 0x4004f2: file 3.C, line 4.

patched:
GNU gdb (GDB) 7.2.50.20110220-cvs
(gdb) b C::m if C::m()
the class C does not have any method named m if C::m()
(gdb) b 'C::m' if C::m()
the class C does not have any method named m' if C::m()

This is a regression.

pre-physname had that IMHO-not-so-nice `has_if' protection.


> +
> +      /* Make sure we keep important kewords like "const" */
> +      if (strncmp (p, " const", 6) == 0)
> +	p += 6;

00000000004004ea W _ZNK1C1mEv
00000000004004ea W C::m() const

pre-physname:
GNU gdb (GDB) 7.1.50.20100309-cvs
(gdb) b 'C::m()  const'
Breakpoint 1 at 0x4004f2: file 3.C, line 4.

patched:
GNU gdb (GDB) 7.2.50.20110220-cvs
(gdb) b 'C::m()  const'
Junk at end of arguments.

This is a regression.

There should be some whitespace skipping, as strcmp_iw_ordered does.


> +    }
> +
>    copy = (char *) alloca (p - saved_arg2 + 1);
>    memcpy (copy, saved_arg2, p - saved_arg2);
>    /* Note: if is_quoted should be true, we snuff out quote here
> @@ -1481,15 +1520,24 @@ decode_compound (char **argptr, int funf
>    *argptr = (*p == '\'') ? p + 1 : p;
>  
>    /* Look up entire name.  */
> -  sym = lookup_symbol (copy, 0, VAR_DOMAIN, 0);
> +  sym = lookup_symbol (copy, get_selected_block (0), VAR_DOMAIN, 0);
>    if (sym)
>      return symbol_found (funfirstline, canonical, copy, sym, NULL);
> +  else
> +    {
> +      struct minimal_symbol *msym;
> +
> +      /* Couldn't find any interpretation as classes/namespaces.  As a last
> +	 resort, try the minimal symbol tables.  */
> +      msym = lookup_minimal_symbol (copy, NULL, NULL);
> +      if (msym != NULL)
> +	return minsym_found (funfirstline, msym);
> +    }    
>  
> -  /* Couldn't find any interpretation as classes/namespaces, so give
> -     up.  The quotes are important if copy is empty.  */
> +  /* Couldn't find a minimal symbol, either, so give up.  */
>    if (not_found_ptr)
>      *not_found_ptr = 1;
> -  cplusplus_error (saved_arg,
> +  cplusplus_error (the_real_saved_arg,
>  		   "Can't find member of namespace, "
>  		   "class, struct, or union named \"%s\"\n",
>  		   copy);
> @@ -1528,7 +1576,7 @@ lookup_prefix_sym (char **argptr, char *
>    /* At this point p1->"::inA::fun", p->"inA::fun" copy->"AAA",
>       argptr->"inA::fun".  */
>  
> -  sym = lookup_symbol (copy, 0, STRUCT_DOMAIN, 0);
> +  sym = lookup_symbol (copy, get_selected_block (0), STRUCT_DOMAIN, 0);
>    if (sym == NULL)
>      {
>        /* Typedefs are in VAR_DOMAIN so the above symbol lookup will
> @@ -1599,17 +1647,31 @@ find_method (int funfirstline, char ***c
>        if (strchr (saved_arg, '(') != NULL)
>  	{
>  	  int i;
> -	  char *name = saved_arg;
> -	  char *canon = cp_canonicalize_string (name);
> +	  char *name;
> +	  char *canon;
>  	  struct cleanup *cleanup;
>  
> +	  /* Construct the proper search name based on SYM_CLASS and COPY.
> +	     SAVED_ARG may contain a valid name, but that name might not be
> +	     what is actually stored in the symbol table.  For example,
> +	     if SAVED_ARG (and SYM_CLASS) were found via an import
> +	     ("using namespace" in C++), then the physname of
> +	     SYM_CLASS ("A::myclass") may not be the same as SAVED_ARG
> +	     ("myclass").  */
> +	  name = xmalloc (strlen (SYMBOL_LINKAGE_NAME (sym_class))
> +			  + 2 /* "::" */ + strlen (copy) + 1);
> +	  strcpy (name, SYMBOL_LINKAGE_NAME (sym_class));

Probably not meaningful for SYM_CLASS but isn't here appropriate rather
SYMBOL_NATURAL_NAME?  If SYMBOL_LINKAGE_NAME would be a mangled name for the
class fully qualified name it would not work.  But linkage name of a class
type does not make sense so the suggested change is rather for code
readability.


> +	  strcat (name, "::");
> +	  strcat (name, copy);
> +	  canon = cp_canonicalize_string (name);
>  	  if (canon != NULL)
>  	    {
> +	      xfree (name);
>  	      name = canon;
>  	      cleanup = make_cleanup (xfree, canon);
>  	    }
>  	  else
> -	    cleanup = make_cleanup (null_cleanup, NULL);
> +	    cleanup = make_cleanup (xfree, name);
>  
>  	  for (i = 0; i < i1; ++i)
>  	    {
> Index: psymtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/psymtab.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 psymtab.c
> --- psymtab.c	10 Jan 2011 20:38:50 -0000	1.22
> +++ psymtab.c	17 Feb 2011 17:45:12 -0000
> @@ -33,6 +33,8 @@
>  #include "readline/readline.h"
>  #include "gdb_regex.h"
>  #include "dictionary.h"
> +#include "language.h"
> +#include "cp-support.h"
>  
>  #ifndef DEV_TTY
>  #define DEV_TTY "/dev/tty"
> @@ -426,7 +428,26 @@ lookup_symbol_aux_psymtabs (struct objfi
>    ALL_OBJFILE_PSYMTABS (objfile, ps)
>    {
>      if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
> -      return PSYMTAB_TO_SYMTAB (ps);
> +      {
> +	struct symbol *sym = NULL;
> +	struct symtab *stab = PSYMTAB_TO_SYMTAB (ps);
> +
> +	/* Some caution must be observed with overloaded functions
> +	   and methods, since the psymtab will not contain any overload
> +	   information (but NAME might contain it).  */
> +	if (stab->primary)
> +	  {
> +	    struct blockvector *bv = BLOCKVECTOR (stab);
> +	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
> +
> +	    sym = lookup_block_symbol (block, name, domain);
> +	  }
> +
> +	if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
> +	  return stab;
> +
> +	/* Keep looking through other psymtabs.  */
> +      }
>    }
>  
>    return NULL;
> @@ -519,6 +540,39 @@ pre_expand_symtabs_matching_psymtabs (st
>    /* Nothing.  */
>  }
>  
> +/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
> +   not contain any method/function instance information (since this would
> +   force reading type information while reading psymtabs).  Therefore,
> +   if NAME contains overload information, it must be stripped before searching
> +   psymtabs.
> +
> +   The caller is responsible for freeing the return result.  */
> +
> +static char *
> +psymtab_search_name (const char *name)
> +{
> +  switch (current_language->la_language)
> +    {
> +    case language_cplus:
> +    case language_java:
> +      {
> +       if (strchr (name, '('))
> +         {
> +           char *ret = cp_remove_params (name);
> +
> +           if (ret)
> +             return ret;
> +         }
> +      }
> +      break;
> +
> +    default:
> +      break;
> +    }
> +
> +  return xstrdup (name);
> +}
> +
>  /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
>     Check the global symbols if GLOBAL, the static symbols if not.  */
>  
> @@ -530,11 +584,16 @@ lookup_partial_symbol (struct partial_sy
>    struct partial_symbol **top, **real_top, **bottom, **center;
>    int length = (global ? pst->n_global_syms : pst->n_static_syms);
>    int do_linear_search = 1;
> +  char *search_name;
> +  struct cleanup *cleanup;
>  
>    if (length == 0)
>      {
>        return (NULL);
>      }
> +
> +  search_name = psymtab_search_name (name);
> +  cleanup = make_cleanup (xfree, search_name);
>    start = (global ?
>  	   pst->objfile->global_psymbols.list + pst->globals_offset :
>  	   pst->objfile->static_psymbols.list + pst->statics_offset);
> @@ -563,7 +622,8 @@ lookup_partial_symbol (struct partial_sy
>  	    {
>  	      do_linear_search = 1;
>  	    }
> -	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
> +	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
> +				 search_name) >= 0)
>  	    {
>  	      top = center;
>  	    }
> @@ -577,11 +637,14 @@ lookup_partial_symbol (struct partial_sy
>  			_("failed internal consistency check"));
>  
>        while (top <= real_top
> -	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
> +	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
>  	{
>  	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
>  				     SYMBOL_DOMAIN (*top), domain))
> -	    return (*top);
> +	    {
> +	      do_cleanups (cleanup);
> +	      return (*top);
> +	    }
>  	  top++;
>  	}
>      }
> @@ -595,11 +658,15 @@ lookup_partial_symbol (struct partial_sy
>  	{
>  	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
>  				     SYMBOL_DOMAIN (*psym), domain)
> -	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
> -	    return (*psym);
> +	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, search_name))
> +	    {
> +	      do_cleanups (cleanup);
> +	      return (*psym);
> +	    }
>  	}
>      }
>  
> +  do_cleanups (cleanup);
>    return (NULL);
>  }
>  
> Index: testsuite/gdb.cp/cmpd-minsyms.exp
> ===================================================================
> RCS file: testsuite/gdb.cp/cmpd-minsyms.exp
> diff -N testsuite/gdb.cp/cmpd-minsyms.exp
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.cp/cmpd-minsyms.exp	17 Feb 2011 17:45:12 -0000
> @@ -0,0 +1,50 @@
> +# Copyright 2010 Free Software Foundation, Inc.

2011

> +#
> +# Contributed by Red Hat, originally written by Keith Seitz.
> +#
> +# 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/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +if {[skip_cplus_tests]} { continue }
> +
> +# Test for c++/12273
> +set testfile "cmpd-minsyms"
> +# Do NOT compile with debug flag.
> +prepare_for_testing $testfile $testfile $testfile.cc {c++}

if [prepare_for_testing ... ] {
    return -1
}

> +
> +gdb_test_no_output "set language c++"
> +
> +# A list of minimal symbol names to check.
> +# Note that GDB<char>::even_harder<int>(char) is quoted and includes
> +# the return type.  This is necessary because this is the demangled name
> +# of the minimal symbol.
> +set min_syms [list \
> +		  "GDB<int>::operator ==" \
> +		  "GDB<int>::operator==(GDB<int> const&)" \
> +		  "GDB<char>::harder(char)" \
> +		  "GDB<int>::harder(int)" \
> +		  {"int GDB<char>::even_harder<int>(char)"} \
> +		  "GDB<int>::simple()"]
> +
> +foreach sym $min_syms {
> +    set tst "setting breakpoint at $sym"
> +    if {[gdb_breakpoint $sym]} {
> +	pass $tst
> +    } else {
> +	fail $tst

[nitpick] gdb_breakpoint already always prints FAIL if it fails.

> +    }
> +}
> +
> +gdb_exit
> Index: testsuite/gdb.cp/cmpd-minsyms.cc
> ===================================================================
> RCS file: testsuite/gdb.cp/cmpd-minsyms.cc
> diff -N testsuite/gdb.cp/cmpd-minsyms.cc
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.cp/cmpd-minsyms.cc	17 Feb 2011 17:45:12 -0000
> @@ -0,0 +1,37 @@
> +/* This test case is part of GDB, the GNU debugger.
> +
> +   Copyright 2010 Free Software Foundation, Inc.

2011

> +
> +   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/>.  */
> +
> +template <typename T>
> +class GDB
> +{
> + public:
> +   static int simple (void) { return 0; }
> +   static int harder (T a) { return 1; }
> +   template <typename X>
> +   static X even_harder (T a) { return static_cast<X> (a); }
> +   int operator == (GDB const& other)
> +   { return 1; }
> +};
> +
> +int main(int argc, char **argv)
> +{
> +   GDB<int> a, b;
> +   if (a == b)
> +     return GDB<char>::harder('a') + GDB<int>::harder(3)
> +	+ GDB<char>::even_harder<int> ('a');
> +   return GDB<int>::simple ();
> +}
> Index: testsuite/gdb.cp/ovsrch.exp
> ===================================================================
> RCS file: testsuite/gdb.cp/ovsrch.exp
> diff -N testsuite/gdb.cp/ovsrch.exp
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.cp/ovsrch.exp	17 Feb 2011 17:45:12 -0000
> @@ -0,0 +1,103 @@
> +# Copyright 2011 Free Software Foundation, Inc.
> +#
> +# Contributed by Red Hat, originally written by Keith Seitz.
> +#
> +# 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/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +proc test_class {class} {
> +
> +    # An array holding the overload types for the methods A::outer::foo
> +    # and A::B::inner::foo.  The first element is the overloaded method
> +    # parameter.  The second element is the expected source file number,
> +    # e.g. "ovsrch?.cc".
> +    array set tests {
> +	"char*"  4
> +	"int"    3
> +	"void"   2
> +    }
> +
> +    # Test each overload instance twice: once quoted, once unquoted
> +    foreach ovld [array names tests] {
> +	set method "${class}::foo\($ovld\)"

Excessive backslashes.

> +	set result "Breakpoint (\[0-9\]).*file .*/ovsrch$tests($ovld).*"
> +	gdb_test "break $method" $result
> +	gdb_test "break '$method'" $result
> +    }
> +}
> +
> +if { [skip_cplus_tests] } { continue }
> +
> +# Test for c++/11734
> +set testfile "ovsrch"
> +set binfile [file join $objdir $subdir $testfile]
> +
> +set srcfiles {}
> +for {set i 1} {$i < 5} {incr i} {
> +    lappend srcfiles [file join $srcdir $subdir $testfile$i.cc]
> +}
> +if  {[gdb_compile $srcfiles $binfile executable {debug c++}] != "" } {
> +     untested $testfile.exp
> +     return -1
> +}
> +
> +if {[get_compiler_info $binfile "c++"]} {
> +    return -1
> +}
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load $binfile
> +
> +if {![runto_main]} {
> +    perror "couldn't run to breakpoint"
> +    continue
> +}
> +
> +# Break in A::stop_here and run tests.
> +if {[gdb_breakpoint "stop_here"]} {
> +    pass "break stop_here"
> +} else {
> +    fail "break stop_here"

[nitpick] gdb_breakpoint already always prints FAIL if it fails.
(and below)

> +}
> +
> +if {[gdb_breakpoint "'stop_here'"]} {
> +    pass "break 'stop_here'"
> +} else {
> +    fail "break 'stop_here'"
> +}
> +
> +gdb_continue_to_breakpoint "stop_here"
> +test_class outer
> +
> +# Break in A::B::stop_here_too and run tests.
> +if {[gdb_breakpoint "B::stop_here_too"]} {
> +    pass "break B::stop_here_too"
> +} else {
> +    fail "break B::stop_here_too"
> +}
> +
> +if {[gdb_breakpoint "'B::stop_here_too'"]} {
> +    pass "break 'B::stop_here_too'"
> +} else {
> +    fail "break 'B::stop_here_too'"
> +}
> +
> +gdb_continue_to_breakpoint "stop_here_too"
> +test_class inner
> +
> +gdb_exit
> +return 0

[...]

> Index: testsuite/gdb.cp/ovsrch4.cc
> ===================================================================
> RCS file: testsuite/gdb.cp/ovsrch4.cc
> diff -N testsuite/gdb.cp/ovsrch4.cc
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.cp/ovsrch4.cc	17 Feb 2011 17:45:12 -0000
> @@ -0,0 +1,28 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2011 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/>.  */
> +
> +#include "ovsrch.h"
> +
> +void
> +A::outer::foo (char *a)
> +{
> +}
> +
> +void
> +A::B::inner::foo (char *a)
> +{
> +}


On Thu, 17 Feb 2011 23:31:44 +0100, Keith Seitz wrote:
> And of course, no sooner do I submit this, than I see I forgot something...
> 
> On 02/17/2011 10:12 AM, Keith Seitz wrote:
> >PR c++/11734
> >* gdb.cp/ovsrch.exp: New test.
> 
> I have changed this to use prepare_for_testing:
> 
> @@ -500,21 +500,10 @@
>  +
>  +set srcfiles {}
>  +for {set i 1} {$i < 5} {incr i} {
> -+    lappend srcfiles [file join $srcdir $subdir $testfile$i.cc]
> -+}
> -+if  {[gdb_compile $srcfiles $binfile executable {debug c++}] != "" } {
> -+     untested $testfile.exp
> -+     return -1
> ++    lappend srcfiles $testfile$i.cc
>  +}
>  +
> -+if {[get_compiler_info $binfile "c++"]} {
> -+    return -1
> -+}
> -+
> -+gdb_exit
> -+gdb_start
> -+gdb_reinitialize_dir $srcdir/$subdir
> -+gdb_load $binfile
> ++prepare_for_testing $testfile $testfile $srcfiles {c++ debug}

if [prepare_for_testing ... ] {
    return -1
}


>  +
>  +if {![runto_main]} {
>  +    perror "couldn't run to breakpoint"


Sorry for the review delay.


Thanks for all the linespec work,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-21 11:41           ` Jan Kratochvil
@ 2011-02-24 20:41             ` Keith Seitz
  2011-02-27 21:18             ` Jan Kratochvil
  1 sibling, 0 replies; 29+ messages in thread
From: Keith Seitz @ 2011-02-24 20:41 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

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

On 02/21/2011 03:06 AM, Jan Kratochvil wrote:

> pre-physname:
> GNU gdb (GDB) 7.1.50.20100309-cvs
> (gdb) b A::outer::foo (int)
> Breakpoint 1 at 0x4005a3: file ./gdb.cp/ovsrch3.cc, line 23.
> (gdb) b A::outer::foo (void)
> Breakpoint 2 at 0x40058c: file ./gdb.cp/ovsrch2.cc, line 23.
> (gdb) b A::outer::foo (char *)
> Breakpoint 3 at 0x4005c0: file ./gdb.cp/ovsrch4.cc, line 23.
>
> patched:
> GNU gdb (GDB) 7.2.50.20110220-cvs
> (gdb) b A::outer::foo (int)
> the class A::outer does not have any method named foo (int)

This is caused by one of the consequences of dwarf2_physname: we need to 
have all user input canonicalized before calling lookup_symbol[*]. This 
is a problem I have wondered about since the dwarf2_physname patchset 
was originally committed, and now we have a concrete example of how to 
trigger this.

IIRC, the only functional changes (excluding requested changes) to the 
patchset are:

- canonicalization of user input in find_methods (linespec.c)
- add tests to ovsrch.exp et al to cover the regressions you've identified

> It assumes get_gdb_completer_quote_characters() is just "'" as currently is.
> It cannot be changed now anyway and for future it should be rather removed
> than extended.  It was already hardcoded into decode_compound before physname
> patch.

Yes, that was sloppy on my part. I apologize. I have reintroduced the 
use of get_gdb_completer_quote_characters. IMO there's something to be 
said for consistency. One day, we'll want to identify the places where 
this assumption is made, and using get_gdb_completer_quote_characters is 
undoubtedly going to be the key to doing it.

> Éxcessive backslash.

Fixed all of these.

> Some comment and/or removal wrt get_gdb_completer_quote_characters for '.
> (OK, this ' hardcoded in decode_compound was present even before physname,
> I can clean it up afterwards.)

If I have understood you correctly, I've removed the quote character and 
tested independently for it using get_gdb_completer_quote_characters.

> *p can == '\0' here, it works - but I would prefer for better readability:

Fixed.

> 00000000004004ea W _ZNK1C1mEv
> 00000000004004ea W C::m() const
>
> pre-physname:
> GNU gdb (GDB) 7.1.50.20100309-cvs
> (gdb) b C::m if C::m()
> Breakpoint 1 at 0x4004f2: file 3.C, line 4.
> (gdb) b 'C::m' if C::m()
> Breakpoint 1 at 0x4004f2: file 3.C, line 4.
>
> patched:
> GNU gdb (GDB) 7.2.50.20110220-cvs
> (gdb) b C::m if C::m()
> the class C does not have any method named m if C::m()
> (gdb) b 'C::m' if C::m()
> the class C does not have any method named m' if C::m()

This is also fixed, and I have added a test for this to ovsrch.exp.

> 00000000004004ea W _ZNK1C1mEv
> 00000000004004ea W C::m() const
>
> pre-physname:
> GNU gdb (GDB) 7.1.50.20100309-cvs
> (gdb) b 'C::m()  const'
> Breakpoint 1 at 0x4004f2: file 3.C, line 4.
>
> patched:
> GNU gdb (GDB) 7.2.50.20110220-cvs
> (gdb) b 'C::m()  const'
> Junk at end of arguments.
>
> This is a regression.

This is also a consequence of (the lack of) canonicalization. I've 
modified ovsrch.exp et al to test for this.

> Probably not meaningful for SYM_CLASS but isn't here appropriate rather
> SYMBOL_NATURAL_NAME?  If SYMBOL_LINKAGE_NAME would be a mangled name for the
> class fully qualified name it would not work.  But linkage name of a class
> type does not make sense so the suggested change is rather for code
> readability.

Changed.

>> +# Copyright 2010 Free Software Foundation, Inc.
>
> 2011

Fixed.

>
> if [prepare_for_testing ... ] {
>      return -1
> }

Fixed.

> [nitpick] gdb_breakpoint already always prints FAIL if it fails.

Grr. Deep down, I knew that to be the case. For some reason, it looked 
odd, so I added the fail clause. Fixed.

>> +   Copyright 2010 Free Software Foundation, Inc.
>
> 2011

Fixed.

>> +	set method "${class}::foo\($ovld\)"
>
> Excessive backslashes.

Fixed.


> [nitpick] gdb_breakpoint already always prints FAIL if it fails.
> (and below)

Fixed.

> if [prepare_for_testing ... ] {
>      return -1
> }

Fixed.

> Sorry for the review delay.

It's a big patchset. It's not like I have nothing else to do! ;-)

I'm not sure whether it would be easier to diff the old and new 
patchsets or just attach the whole thing. But I'm erring on the side of 
"too much information."

Keith

ChangeLog
2011-02-24  Keith Seitz  <keiths@redhat.com>

	* linespec.c (find_methods): Canonicalize NAME before looking
	up the symbol.

	PR c++/12273
	* linespec.c (locate_first_half): Keep overload information, too.
	(decode_compound): Use a string to represent break characters
	to escape the loop.
	If P points to a break character, do not increment it.
	For C++ and Java, keep overload information and relevant keywords.
	If we cannot find a symbol, search the minimal symbols.

	PR c++/11734
	* linespec.c (decode_compound): Rename SAVED_ARG to
	THE_REAL_SAVED_ARG.
	Make a copy of THE_REAL_SAVED_ARG in SAVED_ARG and strip
	single-quotes.
	Pass a valid block to lookup_symbol.
	(lookup_prefix_sym): Likewise.
	(find_method): Construct search name based on SYM_CLASS instead
	of SAVED_ARG.
	* psymtab.c (lookup_partial_symbol): Add language parameter.
	(lookup_symbol_aux_psymtabs): Likewise.
	Don't assume that the psymtab we found was the right one. Search
	for the desired symbol in the symtab to be certain.
	(psymtab_search_name): New function.
	(lookup_partial_symbol): Use psymtab_search_name.
	Add language parameter.
	(read_symtabs_for_function): Add language parameter and pass to
	lookup_partial_symbol.
	(find_symbol_file_from_partial): Likewise.
	* symfile.h (struct quick_symbol_functions): Add language parameter
	to lookup_symbol, expand_symtabs_for_function, and find_symbol_file.
	* cp-support.c (make_symbol_overload_list): Update above API
	changes.
	* symtab.c (lookup_symbol_aux_quick): Pass the current language
	to the quick symbol functions.
	(basic_lookup_transparent_type_quick): Likewise.
	(find_main_filename): Likewise.
	* dwarf2_read.c (dw2_lookup_symbol): Add langauge parameter.
	(dw2_expand_symtabs_for_function): Likewise.
	(dw2_find_symbol_file): Likewise.

testsuite/ChangeLog
[unchanged from last time]

Keith

[-- Attachment #2: 11734-12273-3.patch --]
[-- Type: text/plain, Size: 25161 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.109
diff -u -p -r1.109 linespec.c
--- linespec.c	9 Jan 2011 03:08:57 -0000	1.109
+++ linespec.c	24 Feb 2011 20:36:35 -0000
@@ -213,6 +213,19 @@ find_methods (struct type *t, char *name
   int i1 = 0;
   int ibase;
   char *class_name = type_name_no_tag (t);
+  struct cleanup *cleanup;
+  char *canon;
+
+  /* NAME is typed by the user: it needs to be canonicalized before
+     passing to lookup_symbol.  */
+  canon = cp_canonicalize_string (name);
+  if (canon != NULL)
+    {
+      name = canon;
+      cleanup = make_cleanup (xfree, name);
+    }
+  else
+    cleanup = make_cleanup (null_cleanup, NULL);
 
   /* Ignore this class if it doesn't have a name.  This is ugly, but
      unless we figure out how to get the physname without the name of
@@ -275,6 +288,7 @@ find_methods (struct type *t, char *name
       i1 += find_methods (TYPE_BASECLASS (t, ibase), name,
 			  language, sym_arr + i1);
 
+  do_cleanups (cleanup);
   return i1;
 }
 
@@ -1057,6 +1071,10 @@ locate_first_half (char **argptr, int *i
 	    error (_("malformed template specification in command"));
 	  p = temp_end;
 	}
+
+      if (p[0] == '(')
+	p = find_method_overload_end (p);
+
       /* Check for a colon and a plus or minus and a [ (which
          indicates an Objective-C method).  */
       if (is_objc_method_format (p))
@@ -1226,7 +1244,7 @@ decode_objc (char **argptr, int funfirst
 
 static struct symtabs_and_lines
 decode_compound (char **argptr, int funfirstline, char ***canonical,
-		 char *saved_arg, char *p, int *not_found_ptr)
+		 char *the_real_saved_arg, char *p, int *not_found_ptr)
 {
   struct symtabs_and_lines values;
   char *p2;
@@ -1237,6 +1255,23 @@ decode_compound (char **argptr, int funf
   struct symbol *sym_class;
   struct type *t;
   char *saved_java_argptr = NULL;
+  char *saved_arg;
+
+  /* If the user specified any completer quote characters in the input,
+     strip them.  They are superfluous.  */
+  saved_arg = alloca (strlen (the_real_saved_arg) + 1);
+  {
+    char *dst = saved_arg;
+    char *src = the_real_saved_arg;
+    char *quotes = get_gdb_completer_quote_characters ();
+    while (*src != '\0')
+      {
+	if (strchr (quotes, *src) == NULL)
+	  *dst++ = *src;
+	++src;
+      }
+    *dst = '\0';
+  }
 
   /* First check for "global" namespace specification, of the form
      "::foo".  If found, skip over the colons and jump to normal
@@ -1253,8 +1288,10 @@ decode_compound (char **argptr, int funf
         find_method.
 
      2) AAA::inA isn't the name of a class.  In that case, either the
-        user made a typo or AAA::inA is the name of a namespace.
-        Either way, we just look up AAA::inA::fun with lookup_symbol.
+        user made a typo, AAA::inA is the name of a namespace, or it is
+        the name of a minimal symbol.
+        We just look up AAA::inA::fun with lookup_symbol.  If that fails,
+        try lookup_minimal_symbol.
 
      Thus, our first task is to find everything before the last set of
      double-colons and figure out if it's the name of a class.  So we
@@ -1275,6 +1312,8 @@ decode_compound (char **argptr, int funf
 
   while (1)
     {
+      static char *break_characters = " \t(";
+
       /* Move pointer up to next possible class/namespace token.  */
 
       p = p2 + 1;	/* Restart with old value +1.  */
@@ -1285,8 +1324,9 @@ decode_compound (char **argptr, int funf
       /* PASS2: p2->"::fun", p->":fun" */
 
       /* Move pointer ahead to next double-colon.  */
-      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\'')
-	     && (*p != '('))
+      while (*p
+	     && strchr (break_characters, *p) == NULL
+	     && strchr (get_gdb_completer_quote_characters (), *p) == NULL)
 	{
 	  if (current_language->la_language == language_cplus)
 	    p += cp_validate_operator (p);
@@ -1310,9 +1350,12 @@ decode_compound (char **argptr, int funf
 	  else if ((p[0] == ':') && (p[1] == ':'))
 	    break;	/* Found double-colon.  */
 	  else
-	    /* PASS2: We'll keep getting here, until p->"", at which point
-	       we exit this loop.  */
-	    p++;
+	    {
+	      /* PASS2: We'll keep getting here, until P points to one of the
+		 break characters, at which point we exit this loop.  */
+	      if (*p && strchr (break_characters, *p) == NULL)
+		p++;
+	    }
 	}
 
       if (*p != ':')
@@ -1321,7 +1364,7 @@ decode_compound (char **argptr, int funf
 			   unsuccessfully all the components of the
 			   string, and p->""(PASS2).  */
 
-      /* We get here if p points to ' ', '\t', '\'', "::" or ""(i.e
+      /* We get here if p points to one of the break characters or "" (i.e.,
 	 string ended).  */
       /* Save restart for next time around.  */
       p2 = p;
@@ -1472,6 +1515,18 @@ decode_compound (char **argptr, int funf
   /* We couldn't find a class, so we're in case 2 above.  We check the
      entire name as a symbol instead.  */
 
+  if (current_language->la_language == language_cplus
+      || current_language->la_language == language_java)
+    {
+      char *paren = strchr (p, '(');
+      if (paren != NULL)
+	p = find_method_overload_end (paren);
+
+      /* Make sure we keep important kewords like "const" */
+      if (strncmp (p, " const", 6) == 0)
+	p += 6;
+    }
+
   copy = (char *) alloca (p - saved_arg2 + 1);
   memcpy (copy, saved_arg2, p - saved_arg2);
   /* Note: if is_quoted should be true, we snuff out quote here
@@ -1481,15 +1536,24 @@ decode_compound (char **argptr, int funf
   *argptr = (*p == '\'') ? p + 1 : p;
 
   /* Look up entire name.  */
-  sym = lookup_symbol (copy, 0, VAR_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), VAR_DOMAIN, 0);
   if (sym)
     return symbol_found (funfirstline, canonical, copy, sym, NULL);
+  else
+    {
+      struct minimal_symbol *msym;
 
-  /* Couldn't find any interpretation as classes/namespaces, so give
-     up.  The quotes are important if copy is empty.  */
+      /* Couldn't find any interpretation as classes/namespaces.  As a last
+	 resort, try the minimal symbol tables.  */
+      msym = lookup_minimal_symbol (copy, NULL, NULL);
+      if (msym != NULL)
+	return minsym_found (funfirstline, msym);
+    }    
+
+  /* Couldn't find a minimal symbol, either, so give up.  */
   if (not_found_ptr)
     *not_found_ptr = 1;
-  cplusplus_error (saved_arg,
+  cplusplus_error (the_real_saved_arg,
 		   "Can't find member of namespace, "
 		   "class, struct, or union named \"%s\"\n",
 		   copy);
@@ -1528,7 +1592,7 @@ lookup_prefix_sym (char **argptr, char *
   /* At this point p1->"::inA::fun", p->"inA::fun" copy->"AAA",
      argptr->"inA::fun".  */
 
-  sym = lookup_symbol (copy, 0, STRUCT_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), STRUCT_DOMAIN, 0);
   if (sym == NULL)
     {
       /* Typedefs are in VAR_DOMAIN so the above symbol lookup will
@@ -1599,17 +1663,29 @@ find_method (int funfirstline, char ***c
       if (strchr (saved_arg, '(') != NULL)
 	{
 	  int i;
-	  char *name = saved_arg;
-	  char *canon = cp_canonicalize_string (name);
+	  char *name;
+	  char *canon;
 	  struct cleanup *cleanup;
 
+	  /* Construct the proper search name based on SYM_CLASS and COPY.
+	     SAVED_ARG may contain a valid name, but that name might not be
+	     what is actually stored in the symbol table.  For example,
+	     if SAVED_ARG (and SYM_CLASS) were found via an import
+	     ("using namespace" in C++), then the physname of
+	     SYM_CLASS ("A::myclass") may not be the same as SAVED_ARG
+	     ("myclass").  */
+	  name = xmalloc (strlen (SYMBOL_NATURAL_NAME (sym_class))
+			  + 2 /* "::" */ + strlen (copy) + 1);
+	  strcpy (name, SYMBOL_NATURAL_NAME (sym_class));
+	  strcat (name, "::");
+	  strcat (name, copy);
+	  canon = cp_canonicalize_string (name);
 	  if (canon != NULL)
 	    {
+	      xfree (name);
 	      name = canon;
-	      cleanup = make_cleanup (xfree, canon);
 	    }
-	  else
-	    cleanup = make_cleanup (null_cleanup, NULL);
+	  cleanup = make_cleanup (xfree, name);
 
 	  for (i = 0; i < i1; ++i)
 	    {
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.22
diff -u -p -r1.22 psymtab.c
--- psymtab.c	10 Jan 2011 20:38:50 -0000	1.22
+++ psymtab.c	24 Feb 2011 20:36:35 -0000
@@ -33,6 +33,8 @@
 #include "readline/readline.h"
 #include "gdb_regex.h"
 #include "dictionary.h"
+#include "language.h"
+#include "cp-support.h"
 
 #ifndef DEV_TTY
 #define DEV_TTY "/dev/tty"
@@ -426,7 +428,26 @@ lookup_symbol_aux_psymtabs (struct objfi
   ALL_OBJFILE_PSYMTABS (objfile, ps)
   {
     if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
+      {
+	struct symbol *sym = NULL;
+	struct symtab *stab = PSYMTAB_TO_SYMTAB (ps);
+
+	/* Some caution must be observed with overloaded functions
+	   and methods, since the psymtab will not contain any overload
+	   information (but NAME might contain it).  */
+	if (stab->primary)
+	  {
+	    struct blockvector *bv = BLOCKVECTOR (stab);
+	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
+
+	    sym = lookup_block_symbol (block, name, domain);
+	  }
+
+	if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
+	  return stab;
+
+	/* Keep looking through other psymtabs.  */
+      }
   }
 
   return NULL;
@@ -519,6 +540,39 @@ pre_expand_symtabs_matching_psymtabs (st
   /* Nothing.  */
 }
 
+/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
+   not contain any method/function instance information (since this would
+   force reading type information while reading psymtabs).  Therefore,
+   if NAME contains overload information, it must be stripped before searching
+   psymtabs.
+
+   The caller is responsible for freeing the return result.  */
+
+static char *
+psymtab_search_name (const char *name)
+{
+  switch (current_language->la_language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+       if (strchr (name, '('))
+         {
+           char *ret = cp_remove_params (name);
+
+           if (ret)
+             return ret;
+         }
+      }
+      break;
+
+    default:
+      break;
+    }
+
+  return xstrdup (name);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    Check the global symbols if GLOBAL, the static symbols if not.  */
 
@@ -530,11 +584,16 @@ lookup_partial_symbol (struct partial_sy
   struct partial_symbol **top, **real_top, **bottom, **center;
   int length = (global ? pst->n_global_syms : pst->n_static_syms);
   int do_linear_search = 1;
+  char *search_name;
+  struct cleanup *cleanup;
 
   if (length == 0)
     {
       return (NULL);
     }
+
+  search_name = psymtab_search_name (name);
+  cleanup = make_cleanup (xfree, search_name);
   start = (global ?
 	   pst->objfile->global_psymbols.list + pst->globals_offset :
 	   pst->objfile->static_psymbols.list + pst->statics_offset);
@@ -563,7 +622,8 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
+				 search_name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -577,11 +637,14 @@ lookup_partial_symbol (struct partial_sy
 			_("failed internal consistency check"));
 
       while (top <= real_top
-	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
 				     SYMBOL_DOMAIN (*top), domain))
-	    return (*top);
+	    {
+	      do_cleanups (cleanup);
+	      return (*top);
+	    }
 	  top++;
 	}
     }
@@ -595,11 +658,15 @@ lookup_partial_symbol (struct partial_sy
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
 				     SYMBOL_DOMAIN (*psym), domain)
-	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
-	    return (*psym);
+	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, search_name))
+	    {
+	      do_cleanups (cleanup);
+	      return (*psym);
+	    }
 	}
     }
 
+  do_cleanups (cleanup);
   return (NULL);
 }
 
Index: testsuite/gdb.cp/cmpd-minsyms.cc
===================================================================
RCS file: testsuite/gdb.cp/cmpd-minsyms.cc
diff -N testsuite/gdb.cp/cmpd-minsyms.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cmpd-minsyms.cc	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,37 @@
+/* This test case is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+template <typename T>
+class GDB
+{
+ public:
+   static int simple (void) { return 0; }
+   static int harder (T a) { return 1; }
+   template <typename X>
+   static X even_harder (T a) { return static_cast<X> (a); }
+   int operator == (GDB const& other)
+   { return 1; }
+};
+
+int main(int argc, char **argv)
+{
+   GDB<int> a, b;
+   if (a == b)
+     return GDB<char>::harder('a') + GDB<int>::harder(3)
+	+ GDB<char>::even_harder<int> ('a');
+   return GDB<int>::simple ();
+}
Index: testsuite/gdb.cp/cmpd-minsyms.exp
===================================================================
RCS file: testsuite/gdb.cp/cmpd-minsyms.exp
diff -N testsuite/gdb.cp/cmpd-minsyms.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cmpd-minsyms.exp	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,50 @@
+# Copyright 2011 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+if {[skip_cplus_tests]} { continue }
+
+# Test for c++/12273
+set testfile "cmpd-minsyms"
+# Do NOT compile with debug flag.
+if {[prepare_for_testing $testfile $testfile $testfile.cc {c++}]} {
+    return -1
+}
+
+gdb_test_no_output "set language c++"
+
+# A list of minimal symbol names to check.
+# Note that GDB<char>::even_harder<int>(char) is quoted and includes
+# the return type.  This is necessary because this is the demangled name
+# of the minimal symbol.
+set min_syms [list \
+		  "GDB<int>::operator ==" \
+		  "GDB<int>::operator==(GDB<int> const&)" \
+		  "GDB<char>::harder(char)" \
+		  "GDB<int>::harder(int)" \
+		  {"int GDB<char>::even_harder<int>(char)"} \
+		  "GDB<int>::simple()"]
+
+foreach sym $min_syms {
+    set tst "setting breakpoint at $sym"
+    if {[gdb_breakpoint $sym]} {
+	pass $tst
+    }
+}
+
+gdb_exit
Index: testsuite/gdb.cp/ovsrch.exp
===================================================================
RCS file: testsuite/gdb.cp/ovsrch.exp
diff -N testsuite/gdb.cp/ovsrch.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch.exp	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,96 @@
+# Copyright 2011 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+proc test_class {class} {
+
+    # An array holding the overload types for the methods A::outer::foo
+    # and A::B::inner::foo.  The first element is the overloaded method
+    # parameter.  The second element is the expected source file number,
+    # e.g. "ovsrch?.cc".
+    array set tests {
+	"char*"  4
+	"int"    3
+	"void"   2
+    }
+
+    # Test each overload instance twice: once quoted, once unquoted
+    set conditional1 "if (a == 3)"
+    set conditional2 "if (A::outer::func ())"
+    foreach ovld [array names tests] {
+	set method "${class}::foo ($ovld) const"
+	set result "Breakpoint (\[0-9\]).*file .*/ovsrch$tests($ovld).*"
+	gdb_test "break $method" $result
+	gdb_test "break '$method'" $result
+
+	# Also test with a conditional tacked onto the end.
+	if {[string compare $ovld "void"] != 0} {
+	    gdb_test "break $method $conditional1" $result
+	    gdb_test "break '$method' $conditional1" $result
+	    gdb_test "break $method $conditional2" $result
+	    gdb_test "break '$method' $conditional2" $result
+	}
+    }
+}
+
+if { [skip_cplus_tests] } { continue }
+
+# Test for c++/11734
+set testfile "ovsrch"
+set binfile [file join $objdir $subdir $testfile]
+
+set srcfiles {}
+for {set i 1} {$i < 5} {incr i} {
+    lappend srcfiles $testfile$i.cc
+}
+
+if {[prepare_for_testing $testfile $testfile $srcfiles {c++ debug}]} {
+    return -1
+}
+
+if {![runto_main]} {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# Break in A::stop_here and run tests.
+if {[gdb_breakpoint "stop_here"]} {
+    pass "break stop_here"
+}
+
+if {[gdb_breakpoint "'stop_here'"]} {
+    pass "break 'stop_here'"
+}
+
+gdb_continue_to_breakpoint "stop_here"
+test_class outer
+
+# Break in A::B::stop_here_too and run tests.
+if {[gdb_breakpoint "B::stop_here_too"]} {
+    pass "break B::stop_here_too"
+}
+
+if {[gdb_breakpoint "'B::stop_here_too'"]} {
+    pass "break 'B::stop_here_too'"
+}
+
+gdb_continue_to_breakpoint "stop_here_too"
+test_class inner
+
+gdb_exit
+return 0
Index: testsuite/gdb.cp/ovsrch.h
===================================================================
RCS file: testsuite/gdb.cp/ovsrch.h
diff -N testsuite/gdb.cp/ovsrch.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch.h	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+namespace A
+{
+  class outer
+  {
+  public:
+    void foo (void) const;
+    void foo (int) const;
+    void foo (char *) const;
+    bool func (void) { return true; }
+  };
+
+  namespace B
+  {
+    class inner
+    {
+    public:
+      void foo (void) const;
+      void foo (int) const;
+      void foo (char *) const;
+    };
+  }
+}
Index: testsuite/gdb.cp/ovsrch1.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch1.cc
diff -N testsuite/gdb.cp/ovsrch1.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch1.cc	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,41 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+namespace A
+{
+  void stop_here (void) { }
+
+  namespace B
+  {
+    void stop_here_too (void) { }
+  }
+}
+
+using namespace A;
+
+int
+main ()
+{
+  outer *p = new outer;
+  stop_here ();
+  B::stop_here_too ();
+  p->foo ();
+  return 0;
+}
+
Index: testsuite/gdb.cp/ovsrch2.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch2.cc
diff -N testsuite/gdb.cp/ovsrch2.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch2.cc	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (void) const
+{
+}
+
+void
+A::B::inner::foo (void) const
+{
+}
Index: testsuite/gdb.cp/ovsrch3.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch3.cc
diff -N testsuite/gdb.cp/ovsrch3.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch3.cc	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (int a) const
+{
+}
+
+void
+A::B::inner::foo (int a) const
+{
+}
Index: testsuite/gdb.cp/ovsrch4.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch4.cc
diff -N testsuite/gdb.cp/ovsrch4.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch4.cc	24 Feb 2011 20:36:35 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (char *a) const
+{
+}
+
+void
+A::B::inner::foo (char *a) const
+{
+}

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-21 11:41           ` Jan Kratochvil
  2011-02-24 20:41             ` Keith Seitz
@ 2011-02-27 21:18             ` Jan Kratochvil
  2011-03-01 22:00               ` Keith Seitz
  1 sibling, 1 reply; 29+ messages in thread
From: Jan Kratochvil @ 2011-02-27 21:18 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

On Thu, 24 Feb 2011 21:40:36 +0100, Keith Seitz wrote:
> On 02/21/2011 03:06 AM, Jan Kratochvil wrote:
> >pre-physname:
> >GNU gdb (GDB) 7.1.50.20100309-cvs
> >(gdb) b A::outer::foo (int)
> >Breakpoint 1 at 0x4005a3: file ./gdb.cp/ovsrch3.cc, line 23.
> >
> >patched:
> >GNU gdb (GDB) 7.2.50.20110220-cvs
> >(gdb) b A::outer::foo (int)
> >the class A::outer does not have any method named foo (int)
> 
> This is caused by one of the consequences of dwarf2_physname: we
> need to have all user input canonicalized before calling
> lookup_symbol[*]. This is a problem I have wondered about since the
> dwarf2_physname patchset was originally committed, and now we have a
> concrete example of how to trigger this.

This part is now fixed, thanks.


> If I have understood you correctly, I've removed the quote character
> and tested independently for it using
> get_gdb_completer_quote_characters.

Yes, I find this change as one of the valid possibilites.


> >00000000004004ea W _ZNK1C1mEv
> >00000000004004ea W C::m() const
> >
> >pre-physname:
> >GNU gdb (GDB) 7.1.50.20100309-cvs
> >(gdb) b C::m if C::m()
> >Breakpoint 1 at 0x4004f2: file 3.C, line 4.
> >(gdb) b 'C::m' if C::m()
> >Breakpoint 1 at 0x4004f2: file 3.C, line 4.
> >
> >patched:
> >GNU gdb (GDB) 7.2.50.20110220-cvs
> >(gdb) b C::m if C::m()
> >the class C does not have any method named m if C::m()
> >(gdb) b 'C::m' if C::m()
> >the class C does not have any method named m' if C::m()
> 
> This is also fixed,

This is still not fixed.  I quoted there the line:

> > +      char *paren = strchr (p, '(');

The problem is it skips anything in between, incl. the " if " keyword.
(There are two cases of this quoted GDB line of code.)

Maybe some - simplified
	while (isspace (*p)) p++;
	if (*p == '(')
would be enough?  I did not try.

> and I have added a test for this to ovsrch.exp.

The testcase still does not use a bare method name without any parentheses to
reproduce this problem.

Such a reproducer turning PASS->FAIL from pre-physname is:
echo 'class C { public: void m() const {} } c; int main () { c.m(); }' | gcc -x c++ - -Wall -g; ../gdb -nx ./a.out -ex 'b C::m if C::m()' -ex q

It is currently more difficult to see the regression on the gdb.cp/ovsrch.exp
testcase with pre-physname GDB as gdb.cp/ovsrch.exp uses namespaces now which
were not supported by pre-physname GDB.  Contrary to it this minimal testcase
above is compatible with pre-physname GDB
(that is 42284fdf9d8cdb20c8e833bdbdb2b56977fea525^)


> >00000000004004ea W _ZNK1C1mEv
> >00000000004004ea W C::m() const
> >
> >pre-physname:
> >GNU gdb (GDB) 7.1.50.20100309-cvs
> >(gdb) b 'C::m()  const'
> >Breakpoint 1 at 0x4004f2: file 3.C, line 4.
> >
> >patched:
> >GNU gdb (GDB) 7.2.50.20110220-cvs
> >(gdb) b 'C::m()  const'
> >Junk at end of arguments.
> >
> >This is a regression.
> 
> This is also a consequence of (the lack of) canonicalization. I've
> modified ovsrch.exp et al to test for this.

This is also not fixed.  Reproducible by modifying the testcase:
-	set method "${class}::foo ($ovld) const"
+	set method "${class}::foo ($ovld)  const"

The problem is the quoted code:

> > +      /* Make sure we keep important kewords like "const" */
> > +      if (strncmp (p, " const", 6) == 0)
> > +	p += 6;

is executed before find_methods is called at all.  Therefore find_methods will
get the "const" suffix already stripped.

Maybe some - simplified
	while (isspace (*p)) p++;
	if (strncmp (p, "const", 5) == 0)
would be enough?


Also I am suspicious on not checking that !isalnum (p[strlen ("const")]) - that
there is a word break and isn't there something like:
	(gdb) break method(int) constvariablename

But I think if there is any string afterwards the caller will abort on it
anyway so it is not a problem is part of it gets incorrectly interpreted as
the "const" keyword.


> I'm not sure whether it would be easier to diff the old and new
> patchsets or just attach the whole thing. But I'm erring on the side
> of "too much information."

I do not mind either way as I commit it first into GIT to operate it.
When we already talk about it it would be most convenient to format patches
by `git format-patch' so that one can use `git am'.  It would also better
match my battalion of regression testing scripts expecting -p1 patches.


> +    set conditional1 "if (a == 3)"
> +    set conditional2 "if (A::outer::func ())"
> +    foreach ovld [array names tests] {
> +	set method "${class}::foo ($ovld) const"

Here isn't tested:
	set method "${class}::foo"
which with patched GDB will:
	(gdb) break outer::foo
	Breakpoint 4 at 0x4005c0: file ./gdb.cp/ovsrch4.cc, line 23.
	PASS: gdb.cp/ovsrch.exp: break outer::foo
but with patched GDB it will also:
	(gdb) break outer::foo if (a == 3)
	the class A::outer does not have any method named foo if (a == 3)
	Hint: try 'outer::foo if (a == 3)<TAB> or 'outer::foo if (a == 3)<ESC-?>
	(Note leading single quote.)
	Make breakpoint pending on future shared library load? (y or [n]) n
	FAIL: gdb.cp/ovsrch.exp: break outer::foo if (a == 3) (got interactive prompt)



Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-02-27 21:18             ` Jan Kratochvil
@ 2011-03-01 22:00               ` Keith Seitz
  2011-03-14  7:52                 ` Jan Kratochvil
  0 siblings, 1 reply; 29+ messages in thread
From: Keith Seitz @ 2011-03-01 22:00 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

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

On 02/27/2011 01:16 PM, Jan Kratochvil wrote:
> On Thu, 24 Feb 2011 21:40:36 +0100, Keith Seitz wrote:
>
> This is still not fixed.  I quoted there the line:
>
>>> +      char *paren = strchr (p, '(');

My bad. I misread your test case. I *think* I've added an appropriate 
test and fixed this for good. Or at least I hope I did. :-D


To summarize the changes in this version:

- Added some more tests based on your input
- Added a new function to collect all this naming stuff that
   started appearing in decode_line_1 and decode_compound.
- Rewrote this naming stuff to remove a bunch of bad/unintended
   assumptions (strchr ('('), whitespace)

I'm sure that some cleanup could be done to this, but as far as I am 
concerned, since this is supposedly blocking the next release, I am not 
in favor of even bigger changes right now, cleanups or rewrites. It's 
just too risky at the moment.

> Maybe some - simplified
> 	while (isspace (*p)) p++;
> 	if (*p == '(')
> would be enough?  I did not try.

Yeah, I added something like that.

> This is also not fixed.  Reproducible by modifying the testcase:
> -	set method "${class}::foo ($ovld) const"
> +	set method "${class}::foo ($ovld)  const"
>
> The problem is the quoted code:
>
>>> +      /* Make sure we keep important kewords like "const" */
>>> +      if (strncmp (p, " const", 6) == 0)
>>> +	p += 6;

I've changed all this. This should now be fixed.

> Also I am suspicious on not checking that !isalnum (p[strlen ("const")]) - that
> there is a word break and isn't there something like:
> 	(gdb) break method(int) constvariablename

I've implemented a check for this case.

> Here isn't tested:
> 	set method "${class}::foo"

I've added this.

I've attached the next revision of this mega-linespec-fixing patch.
Keith

ChangeLog
2011-03-01  Keith Seitz  <keiths@redhat.com>

	* linespec.c (find_methods): Canonicalize NAME before looking
	up the symbol.
	(name_end): New function.
	(is_overloaded): New function.
	(keep_name_info): New function.
	(decode_line_1): Use keep_name_info.
	(decode_compound): Likewise.

	PR c++/12273
	* linespec.c (locate_first_half): Keep overload information, too.
	(decode_compound): Use a string to represent break characters
	to escape the loop.
	If P points to a break character, do not increment it.
	For C++ and Java, keep overload information and relevant keywords.
	If we cannot find a symbol, search the minimal symbols.

	PR c++/11734
	* linespec.c (decode_compound): Rename SAVED_ARG to
	THE_REAL_SAVED_ARG.
	Make a copy of THE_REAL_SAVED_ARG in SAVED_ARG and strip
	single-quotes.
	Pass a valid block to lookup_symbol.
	(lookup_prefix_sym): Likewise.
	(find_method): Construct search name based on SYM_CLASS instead
	of SAVED_ARG.
	* psymtab.c (lookup_partial_symbol): Add language parameter.
	(lookup_symbol_aux_psymtabs): Likewise.
	Don't assume that the psymtab we found was the right one. Search
	for the desired symbol in the symtab to be certain.
	(psymtab_search_name): New function.
	(lookup_partial_symbol): Use psymtab_search_name.
	Add language parameter.
	(read_symtabs_for_function): Add language parameter and pass to
	lookup_partial_symbol.
	(find_symbol_file_from_partial): Likewise.
	* symfile.h (struct quick_symbol_functions): Add language parameter
	to lookup_symbol, expand_symtabs_for_function, and find_symbol_file.
	* cp-support.c (make_symbol_overload_list): Update above API
	changes.
	* symtab.c (lookup_symbol_aux_quick): Pass the current language
	to the quick symbol functions.
	(basic_lookup_transparent_type_quick): Likewise.
	(find_main_filename): Likewise.
	* dwarf2_read.c (dw2_lookup_symbol): Add langauge parameter.
	(dw2_expand_symtabs_for_function): Likewise.
	(dw2_find_symbol_file): Likewise.

testsuite/ChangeLog
2011-03-01  Keith Seitz  <keiths@redhat.com>

	PR c++/12273
	* gdb.cp/cmpd-minsyms.exp: New test.
	* gdb.cp/cmpd-minsyms.cc: New file.

	PR c++/11734
	* gdb.cp/ovsrch.exp: New test.
	* gdb.cp/ovsrch.h: New file.
	* gdb.cp/ovsrch1.cc: New file.
	* gdb.cp/ovsrch2.cc: New file.
	* gdb.cp/ovsrch3.cc: New file.
	* gdb.cp/ovsrch4.cc: New file.

[-- Attachment #2: big-linespec.patch --]
[-- Type: text/plain, Size: 29011 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.110
diff -u -p -r1.110 linespec.c
--- linespec.c	1 Mar 2011 00:26:14 -0000	1.110
+++ linespec.c	1 Mar 2011 21:38:20 -0000
@@ -41,6 +41,7 @@
 #include "mi/mi-cmds.h"
 #include "target.h"
 #include "arch-utils.h"
+#include <ctype.h>
 
 /* We share this one with symtab.c, but it is not exported widely.  */
 
@@ -213,6 +214,19 @@ find_methods (struct type *t, char *name
   int i1 = 0;
   int ibase;
   char *class_name = type_name_no_tag (t);
+  struct cleanup *cleanup;
+  char *canon;
+
+  /* NAME is typed by the user: it needs to be canonicalized before
+     passing to lookup_symbol.  */
+  canon = cp_canonicalize_string (name);
+  if (canon != NULL)
+    {
+      name = canon;
+      cleanup = make_cleanup (xfree, name);
+    }
+  else
+    cleanup = make_cleanup (null_cleanup, NULL);
 
   /* Ignore this class if it doesn't have a name.  This is ugly, but
      unless we figure out how to get the physname without the name of
@@ -275,6 +289,7 @@ find_methods (struct type *t, char *name
       i1 += find_methods (TYPE_BASECLASS (t, ibase), name,
 			  language, sym_arr + i1);
 
+  do_cleanups (cleanup);
   return i1;
 }
 
@@ -663,6 +678,96 @@ find_method_overload_end (char *p)
 
   return p;
 }
+
+/* A helper function for decerning whether the string P contains
+   overload information.  */
+
+static int
+is_overloaded (char *p)
+{
+  /* Locate a parenthesis in P.  */
+  char *paren = strchr (p, '(');
+
+  if (paren != NULL)
+    {
+      /* Locate a (possible) "if" clause.  */
+      char *has_if = strstr (p, "if");
+
+      /* Double-check that the "if" was not part of some sort of name,
+	 by checking the characters before and after the "if".  */
+      if (has_if > p && (isalnum (*(has_if - 1)) || isalnum (*(has_if + 2))))
+	has_if = NULL;
+
+      /* If the "if" was seen after the parenthesis, P points to
+	 an overloaded method.  */
+      if (has_if > paren || has_if == NULL)
+	return 1;
+    }
+
+  /* Otherwise, P does not point to an overloaded method.  */
+  return 0;
+}
+
+
+/* Does P point to a sequence of characters which implies the end
+   of a name?  Terminals include "if" and "thread" clauses. */
+
+static int
+name_end (char *p)
+{
+  while (isspace (*p))
+    ++p;
+  if (*p == 'i' && *(p + 1) == 'f'
+      && (isspace (*(p + 2)) || *(p + 2) == '\0' || *(p + 2) == '('))
+    return 1;
+
+  if (strncmp (p, "thread", 6) == 0
+      && ((isspace (*p + 6)) || *(p + 6) == '\0'))
+    return 1;
+
+  return 0;
+}
+
+/* Keep important information used when looking up a name.  This includes
+   template parameters, overload information, and important keywords.  */
+
+static char *
+keep_name_info (char *ptr)
+{
+  char *p = ptr;
+
+  /* Keep any template parameters.  */
+  if (name_end (ptr))
+    goto done;
+
+  while (isspace (*p))
+    ++p;
+  if (*p == '<')
+    ptr = p = find_template_name_end (ptr);
+
+  if (name_end (ptr))
+    goto done;
+
+  /* Keep method overload information.  */
+  if (is_overloaded (p))
+    ptr = p = find_method_overload_end (p);
+
+  if (name_end (ptr))
+    goto done;
+
+  /* Keep important keywords.  */  
+  while (isspace (*p))
+    ++p;
+  if (strncmp (p, "const", 5) == 0
+      && (isspace (*(p + 5)) || *(p + 5) == '\0' || *(p + 5) == '\''))
+    ptr = p = p + 5;
+
+ done:
+  while (isspace (*(ptr - 1)))
+    --ptr;
+  return ptr;
+}
+
 \f
 /* The parser of linespec itself.  */
 
@@ -871,17 +976,8 @@ decode_line_1 (char **argptr, int funfir
       p = skip_quoted (*argptr);
     }
 
-  /* Keep any template parameters.  */
-  if (*p == '<')
-    p = find_template_name_end (p);
-
-  /* Keep method overload information.  */
-  if (*p == '(')
-    p = find_method_overload_end (p);
-
-  /* Make sure we keep important kewords like "const".  */
-  if (strncmp (p, " const", 6) == 0)
-    p += 6;
+  /* Keep any important naming information.  */
+  p = keep_name_info (p);
 
   copy = (char *) alloca (p - *argptr + 1);
   memcpy (copy, *argptr, p - *argptr);
@@ -1057,6 +1153,10 @@ locate_first_half (char **argptr, int *i
 	    error (_("malformed template specification in command"));
 	  p = temp_end;
 	}
+
+      if (p[0] == '(')
+	p = find_method_overload_end (p);
+
       /* Check for a colon and a plus or minus and a [ (which
          indicates an Objective-C method).  */
       if (is_objc_method_format (p))
@@ -1224,7 +1324,7 @@ decode_objc (char **argptr, int funfirst
 
 static struct symtabs_and_lines
 decode_compound (char **argptr, int funfirstline, char ***canonical,
-		 char *saved_arg, char *p, int *not_found_ptr)
+		 char *the_real_saved_arg, char *p, int *not_found_ptr)
 {
   struct symtabs_and_lines values;
   char *p2;
@@ -1235,6 +1335,23 @@ decode_compound (char **argptr, int funf
   struct symbol *sym_class;
   struct type *t;
   char *saved_java_argptr = NULL;
+  char *saved_arg;
+
+  /* If the user specified any completer quote characters in the input,
+     strip them.  They are superfluous.  */
+  saved_arg = alloca (strlen (the_real_saved_arg) + 1);
+  {
+    char *dst = saved_arg;
+    char *src = the_real_saved_arg;
+    char *quotes = get_gdb_completer_quote_characters ();
+    while (*src != '\0')
+      {
+	if (strchr (quotes, *src) == NULL)
+	  *dst++ = *src;
+	++src;
+      }
+    *dst = '\0';
+  }
 
   /* First check for "global" namespace specification, of the form
      "::foo".  If found, skip over the colons and jump to normal
@@ -1251,8 +1368,10 @@ decode_compound (char **argptr, int funf
         find_method.
 
      2) AAA::inA isn't the name of a class.  In that case, either the
-        user made a typo or AAA::inA is the name of a namespace.
-        Either way, we just look up AAA::inA::fun with lookup_symbol.
+        user made a typo, AAA::inA is the name of a namespace, or it is
+        the name of a minimal symbol.
+        We just look up AAA::inA::fun with lookup_symbol.  If that fails,
+        try lookup_minimal_symbol.
 
      Thus, our first task is to find everything before the last set of
      double-colons and figure out if it's the name of a class.  So we
@@ -1273,6 +1392,8 @@ decode_compound (char **argptr, int funf
 
   while (1)
     {
+      static char *break_characters = " \t(";
+
       /* Move pointer up to next possible class/namespace token.  */
 
       p = p2 + 1;	/* Restart with old value +1.  */
@@ -1283,8 +1404,9 @@ decode_compound (char **argptr, int funf
       /* PASS2: p2->"::fun", p->":fun" */
 
       /* Move pointer ahead to next double-colon.  */
-      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\'')
-	     && (*p != '('))
+      while (*p
+	     && strchr (break_characters, *p) == NULL
+	     && strchr (get_gdb_completer_quote_characters (), *p) == NULL)
 	{
 	  if (current_language->la_language == language_cplus)
 	    p += cp_validate_operator (p);
@@ -1308,9 +1430,12 @@ decode_compound (char **argptr, int funf
 	  else if ((p[0] == ':') && (p[1] == ':'))
 	    break;	/* Found double-colon.  */
 	  else
-	    /* PASS2: We'll keep getting here, until p->"", at which point
-	       we exit this loop.  */
-	    p++;
+	    {
+	      /* PASS2: We'll keep getting here, until P points to one of the
+		 break characters, at which point we exit this loop.  */
+	      if (*p && strchr (break_characters, *p) == NULL)
+		p++;
+	    }
 	}
 
       if (*p != ':')
@@ -1319,7 +1444,7 @@ decode_compound (char **argptr, int funf
 			   unsuccessfully all the components of the
 			   string, and p->""(PASS2).  */
 
-      /* We get here if p points to ' ', '\t', '\'', "::" or ""(i.e
+      /* We get here if p points to one of the break characters or "" (i.e.,
 	 string ended).  */
       /* Save restart for next time around.  */
       p2 = p;
@@ -1379,18 +1504,8 @@ decode_compound (char **argptr, int funf
 	      p += cp_validate_operator (p - 8) - 8;
 	    }
 
-	  /* Keep any template parameters.  */
-	  if (*p == '<')
-	    p = find_template_name_end (p);
-
-	  /* Keep method overload information.  */
-	  a = strchr (p, '(');
-	  if (a != NULL)
-	    p = find_method_overload_end (a);
-
-	  /* Make sure we keep important kewords like "const".  */
-	  if (strncmp (p, " const", 6) == 0)
-	    p += 6;
+	  /* Keep any important naming information.  */
+	  p = keep_name_info (p);
 
 	  /* Java may append typenames,  so assume that if there is
 	     anything else left in *argptr, it must be a typename.  */
@@ -1470,6 +1585,10 @@ decode_compound (char **argptr, int funf
   /* We couldn't find a class, so we're in case 2 above.  We check the
      entire name as a symbol instead.  */
 
+  if (current_language->la_language == language_cplus
+      || current_language->la_language == language_java)
+      p = keep_name_info (p);
+
   copy = (char *) alloca (p - saved_arg2 + 1);
   memcpy (copy, saved_arg2, p - saved_arg2);
   /* Note: if is_quoted should be true, we snuff out quote here
@@ -1479,15 +1598,24 @@ decode_compound (char **argptr, int funf
   *argptr = (*p == '\'') ? p + 1 : p;
 
   /* Look up entire name.  */
-  sym = lookup_symbol (copy, 0, VAR_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), VAR_DOMAIN, 0);
   if (sym)
     return symbol_found (funfirstline, canonical, copy, sym, NULL);
+  else
+    {
+      struct minimal_symbol *msym;
+
+      /* Couldn't find any interpretation as classes/namespaces.  As a last
+	 resort, try the minimal symbol tables.  */
+      msym = lookup_minimal_symbol (copy, NULL, NULL);
+      if (msym != NULL)
+	return minsym_found (funfirstline, msym);
+    }    
 
-  /* Couldn't find any interpretation as classes/namespaces, so give
-     up.  The quotes are important if copy is empty.  */
+  /* Couldn't find a minimal symbol, either, so give up.  */
   if (not_found_ptr)
     *not_found_ptr = 1;
-  cplusplus_error (saved_arg,
+  cplusplus_error (the_real_saved_arg,
 		   "Can't find member of namespace, "
 		   "class, struct, or union named \"%s\"\n",
 		   copy);
@@ -1526,7 +1654,7 @@ lookup_prefix_sym (char **argptr, char *
   /* At this point p1->"::inA::fun", p->"inA::fun" copy->"AAA",
      argptr->"inA::fun".  */
 
-  sym = lookup_symbol (copy, 0, STRUCT_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), STRUCT_DOMAIN, 0);
   if (sym == NULL)
     {
       /* Typedefs are in VAR_DOMAIN so the above symbol lookup will
@@ -1594,20 +1722,32 @@ find_method (int funfirstline, char ***c
       /* If we were given a specific overload instance, use that
 	 (or error if no matches were found).  Otherwise ask the user
 	 which one to use.  */
-      if (strchr (saved_arg, '(') != NULL)
+      if (is_overloaded (saved_arg))
 	{
 	  int i;
-	  char *name = saved_arg;
-	  char *canon = cp_canonicalize_string (name);
+	  char *name;
+	  char *canon;
 	  struct cleanup *cleanup;
 
+	  /* Construct the proper search name based on SYM_CLASS and COPY.
+	     SAVED_ARG may contain a valid name, but that name might not be
+	     what is actually stored in the symbol table.  For example,
+	     if SAVED_ARG (and SYM_CLASS) were found via an import
+	     ("using namespace" in C++), then the physname of
+	     SYM_CLASS ("A::myclass") may not be the same as SAVED_ARG
+	     ("myclass").  */
+	  name = xmalloc (strlen (SYMBOL_NATURAL_NAME (sym_class))
+			  + 2 /* "::" */ + strlen (copy) + 1);
+	  strcpy (name, SYMBOL_NATURAL_NAME (sym_class));
+	  strcat (name, "::");
+	  strcat (name, copy);
+	  canon = cp_canonicalize_string (name);
 	  if (canon != NULL)
 	    {
+	      xfree (name);
 	      name = canon;
-	      cleanup = make_cleanup (xfree, canon);
 	    }
-	  else
-	    cleanup = make_cleanup (null_cleanup, NULL);
+	  cleanup = make_cleanup (xfree, name);
 
 	  for (i = 0; i < i1; ++i)
 	    {
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.23
diff -u -p -r1.23 psymtab.c
--- psymtab.c	28 Feb 2011 18:16:59 -0000	1.23
+++ psymtab.c	1 Mar 2011 21:38:20 -0000
@@ -33,6 +33,8 @@
 #include "readline/readline.h"
 #include "gdb_regex.h"
 #include "dictionary.h"
+#include "language.h"
+#include "cp-support.h"
 
 #ifndef DEV_TTY
 #define DEV_TTY "/dev/tty"
@@ -426,7 +428,26 @@ lookup_symbol_aux_psymtabs (struct objfi
   ALL_OBJFILE_PSYMTABS (objfile, ps)
   {
     if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
+      {
+	struct symbol *sym = NULL;
+	struct symtab *stab = PSYMTAB_TO_SYMTAB (ps);
+
+	/* Some caution must be observed with overloaded functions
+	   and methods, since the psymtab will not contain any overload
+	   information (but NAME might contain it).  */
+	if (stab->primary)
+	  {
+	    struct blockvector *bv = BLOCKVECTOR (stab);
+	    struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);
+
+	    sym = lookup_block_symbol (block, name, domain);
+	  }
+
+	if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
+	  return stab;
+
+	/* Keep looking through other psymtabs.  */
+      }
   }
 
   return NULL;
@@ -519,6 +540,39 @@ pre_expand_symtabs_matching_psymtabs (st
   /* Nothing.  */
 }
 
+/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
+   not contain any method/function instance information (since this would
+   force reading type information while reading psymtabs).  Therefore,
+   if NAME contains overload information, it must be stripped before searching
+   psymtabs.
+
+   The caller is responsible for freeing the return result.  */
+
+static char *
+psymtab_search_name (const char *name)
+{
+  switch (current_language->la_language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+       if (strchr (name, '('))
+         {
+           char *ret = cp_remove_params (name);
+
+           if (ret)
+             return ret;
+         }
+      }
+      break;
+
+    default:
+      break;
+    }
+
+  return xstrdup (name);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    Check the global symbols if GLOBAL, the static symbols if not.  */
 
@@ -530,11 +584,16 @@ lookup_partial_symbol (struct partial_sy
   struct partial_symbol **top, **real_top, **bottom, **center;
   int length = (global ? pst->n_global_syms : pst->n_static_syms);
   int do_linear_search = 1;
+  char *search_name;
+  struct cleanup *cleanup;
 
   if (length == 0)
     {
       return (NULL);
     }
+
+  search_name = psymtab_search_name (name);
+  cleanup = make_cleanup (xfree, search_name);
   start = (global ?
 	   pst->objfile->global_psymbols.list + pst->globals_offset :
 	   pst->objfile->static_psymbols.list + pst->statics_offset);
@@ -563,7 +622,8 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center),
+				 search_name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -577,11 +637,14 @@ lookup_partial_symbol (struct partial_sy
 			_("failed internal consistency check"));
 
       while (top <= real_top
-	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+	     && SYMBOL_MATCHES_SEARCH_NAME (*top, search_name))
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
 				     SYMBOL_DOMAIN (*top), domain))
-	    return (*top);
+	    {
+	      do_cleanups (cleanup);
+	      return (*top);
+	    }
 	  top++;
 	}
     }
@@ -595,11 +658,15 @@ lookup_partial_symbol (struct partial_sy
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym),
 				     SYMBOL_DOMAIN (*psym), domain)
-	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
-	    return (*psym);
+	      && SYMBOL_MATCHES_SEARCH_NAME (*psym, search_name))
+	    {
+	      do_cleanups (cleanup);
+	      return (*psym);
+	    }
 	}
     }
 
+  do_cleanups (cleanup);
   return (NULL);
 }
 
Index: testsuite/gdb.cp/cmpd-minsyms.cc
===================================================================
RCS file: testsuite/gdb.cp/cmpd-minsyms.cc
diff -N testsuite/gdb.cp/cmpd-minsyms.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cmpd-minsyms.cc	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,37 @@
+/* This test case is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+template <typename T>
+class GDB
+{
+ public:
+   static int simple (void) { return 0; }
+   static int harder (T a) { return 1; }
+   template <typename X>
+   static X even_harder (T a) { return static_cast<X> (a); }
+   int operator == (GDB const& other)
+   { return 1; }
+};
+
+int main(int argc, char **argv)
+{
+   GDB<int> a, b;
+   if (a == b)
+     return GDB<char>::harder('a') + GDB<int>::harder(3)
+	+ GDB<char>::even_harder<int> ('a');
+   return GDB<int>::simple ();
+}
Index: testsuite/gdb.cp/cmpd-minsyms.exp
===================================================================
RCS file: testsuite/gdb.cp/cmpd-minsyms.exp
diff -N testsuite/gdb.cp/cmpd-minsyms.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cmpd-minsyms.exp	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,50 @@
+# Copyright 2011 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+if {[skip_cplus_tests]} { continue }
+
+# Test for c++/12273
+set testfile "cmpd-minsyms"
+# Do NOT compile with debug flag.
+if {[prepare_for_testing $testfile $testfile $testfile.cc {c++}]} {
+    return -1
+}
+
+gdb_test_no_output "set language c++"
+
+# A list of minimal symbol names to check.
+# Note that GDB<char>::even_harder<int>(char) is quoted and includes
+# the return type.  This is necessary because this is the demangled name
+# of the minimal symbol.
+set min_syms [list \
+		  "GDB<int>::operator ==" \
+		  "GDB<int>::operator==(GDB<int> const&)" \
+		  "GDB<char>::harder(char)" \
+		  "GDB<int>::harder(int)" \
+		  {"int GDB<char>::even_harder<int>(char)"} \
+		  "GDB<int>::simple()"]
+
+foreach sym $min_syms {
+    set tst "setting breakpoint at $sym"
+    if {[gdb_breakpoint $sym]} {
+	pass $tst
+    }
+}
+
+gdb_exit
Index: testsuite/gdb.cp/ovsrch.exp
===================================================================
RCS file: testsuite/gdb.cp/ovsrch.exp
diff -N testsuite/gdb.cp/ovsrch.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch.exp	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,100 @@
+# Copyright 2011 Free Software Foundation, Inc.
+#
+# Contributed by Red Hat, originally written by Keith Seitz.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+proc test_class {class} {
+
+    # An array holding the overload types for the methods A::outer::foo
+    # and A::B::inner::foo.  The first element is the overloaded method
+    # parameter.  The second element is the expected source file number,
+    # e.g. "ovsrch?.cc".
+    array set tests {
+	"char*"  4
+	"int"    3
+	"void"   2
+    }
+
+    # Test each overload instance twice: once quoted, once unquoted
+    set conditional1 "if (a == 3)"
+    set conditional2 "if (A::outer::func ())"
+    foreach ovld [array names tests] {
+	set method "${class}::foo  ($ovld)  const"
+	set result "Breakpoint (\[0-9\]).*file .*/ovsrch$tests($ovld).*"
+	gdb_test "break $method" $result
+	gdb_test "break '$method'" $result
+
+	# Also test with a conditional tacked onto the end.
+	if {[string compare $ovld "void"] != 0} {
+	    gdb_test "break $method $conditional1" $result
+	    gdb_test "break '$method' $conditional1" $result
+	    gdb_test "break $method $conditional2" $result
+	    gdb_test "break '$method' $conditional2" $result
+	}
+    }
+
+    # Test whether open parentheses are correctly identified as overload
+    # information or conditional.
+    gdb_test "break ${class}::foo if (a == 3)" "Breakpoint (\[0-9\]).*"
+}
+
+if { [skip_cplus_tests] } { continue }
+
+# Test for c++/11734
+set testfile "ovsrch"
+set binfile [file join $objdir $subdir $testfile]
+
+set srcfiles {}
+for {set i 1} {$i < 5} {incr i} {
+    lappend srcfiles $testfile$i.cc
+}
+
+if {[prepare_for_testing $testfile $testfile $srcfiles {c++ debug}]} {
+    return -1
+}
+
+if {![runto_main]} {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# Break in A::stop_here and run tests.
+if {[gdb_breakpoint "stop_here"]} {
+    pass "break stop_here"
+}
+
+if {[gdb_breakpoint "'stop_here'"]} {
+    pass "break 'stop_here'"
+}
+
+gdb_continue_to_breakpoint "stop_here"
+test_class outer
+
+# Break in A::B::stop_here_too and run tests.
+if {[gdb_breakpoint "B::stop_here_too"]} {
+    pass "break B::stop_here_too"
+}
+
+if {[gdb_breakpoint "'B::stop_here_too'"]} {
+    pass "break 'B::stop_here_too'"
+}
+
+gdb_continue_to_breakpoint "stop_here_too"
+test_class inner
+
+gdb_exit
+return 0
Index: testsuite/gdb.cp/ovsrch.h
===================================================================
RCS file: testsuite/gdb.cp/ovsrch.h
diff -N testsuite/gdb.cp/ovsrch.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch.h	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+namespace A
+{
+  class outer
+  {
+  public:
+    void foo (void) const;
+    void foo (int) const;
+    void foo (char *) const;
+    bool func (void) { return true; }
+  };
+
+  namespace B
+  {
+    class inner
+    {
+    public:
+      void foo (void) const;
+      void foo (int) const;
+      void foo (char *) const;
+    };
+  }
+}
Index: testsuite/gdb.cp/ovsrch1.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch1.cc
diff -N testsuite/gdb.cp/ovsrch1.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch1.cc	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,41 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+namespace A
+{
+  void stop_here (void) { }
+
+  namespace B
+  {
+    void stop_here_too (void) { }
+  }
+}
+
+using namespace A;
+
+int
+main ()
+{
+  outer *p = new outer;
+  stop_here ();
+  B::stop_here_too ();
+  p->foo ();
+  return 0;
+}
+
Index: testsuite/gdb.cp/ovsrch2.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch2.cc
diff -N testsuite/gdb.cp/ovsrch2.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch2.cc	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (void) const
+{
+}
+
+void
+A::B::inner::foo (void) const
+{
+}
Index: testsuite/gdb.cp/ovsrch3.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch3.cc
diff -N testsuite/gdb.cp/ovsrch3.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch3.cc	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (int a) const
+{
+}
+
+void
+A::B::inner::foo (int a) const
+{
+}
Index: testsuite/gdb.cp/ovsrch4.cc
===================================================================
RCS file: testsuite/gdb.cp/ovsrch4.cc
diff -N testsuite/gdb.cp/ovsrch4.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ovsrch4.cc	1 Mar 2011 21:38:20 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+#include "ovsrch.h"
+
+void
+A::outer::foo (char *a) const
+{
+}
+
+void
+A::B::inner::foo (char *a) const
+{
+}

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-01 22:00               ` Keith Seitz
@ 2011-03-14  7:52                 ` Jan Kratochvil
  2011-03-15 19:03                   ` Keith Seitz
  0 siblings, 1 reply; 29+ messages in thread
From: Jan Kratochvil @ 2011-03-14  7:52 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

On Tue, 01 Mar 2011 23:00:20 +0100, Keith Seitz wrote:
[...]
> --- linespec.c	1 Mar 2011 00:26:14 -0000	1.110
> +++ linespec.c	1 Mar 2011 21:38:20 -0000
[...]
> @@ -663,6 +678,96 @@ find_method_overload_end (char *p)
>  
>    return p;
>  }
> +
> +/* A helper function for decerning whether the string P contains
> +   overload information.  */
> +
> +static int
> +is_overloaded (char *p)
> +{
> +  /* Locate a parenthesis in P.  */
> +  char *paren = strchr (p, '(');
> +
> +  if (paren != NULL)
> +    {
> +      /* Locate a (possible) "if" clause.  */
> +      char *has_if = strstr (p, "if");
> +
> +      /* Double-check that the "if" was not part of some sort of name,
> +	 by checking the characters before and after the "if".  */
> +      if (has_if > p && (isalnum (*(has_if - 1)) || isalnum (*(has_if + 2))))

Although I wrote `isalnum' it is a wrong character class, for example
isalnum ('_') == 0 but `_' is a valid identifier character.

Rather than examining is_overloaded in detail suggesting to rather drop this
whole function.  Please see its callers.


> +	has_if = NULL;
> +
> +      /* If the "if" was seen after the parenthesis, P points to
> +	 an overloaded method.  */
> +      if (has_if > paren || has_if == NULL)
> +	return 1;
> +    }
> +
> +  /* Otherwise, P does not point to an overloaded method.  */
> +  return 0;
> +}
> +
> +
> +/* Does P point to a sequence of characters which implies the end
> +   of a name?  Terminals include "if" and "thread" clauses. */
> +
> +static int
> +name_end (char *p)
> +{
> +  while (isspace (*p))
> +    ++p;
> +  if (*p == 'i' && *(p + 1) == 'f'
> +      && (isspace (*(p + 2)) || *(p + 2) == '\0' || *(p + 2) == '('))

wrt '(' - it did not work in pre-physname GDB as a whitespace is required by
find_condition_and_thread.  But it was present there in `set_flags' so I agree
it may be safer to keep it here.

Just a statement, no request for any change.


> +    return 1;
> +
> +  if (strncmp (p, "thread", 6) == 0
> +      && ((isspace (*p + 6)) || *(p + 6) == '\0'))

All these cases should be written like p[6] both for the IMO better
readability and for making it less fragile against bugs like `(*p + 6)'.


> +    return 1;

There may be also "task" catching but that is used by Ada and it already
worked before without such exception so it is probably OK.

Just a statement, no request for any change.


> +
> +  return 0;
> +}
> +
> +/* Keep important information used when looking up a name.  This includes
> +   template parameters, overload information, and important keywords.  */
> +
> +static char *
> +keep_name_info (char *ptr)
> +{
> +  char *p = ptr;
> +
> +  /* Keep any template parameters.  */
> +  if (name_end (ptr))
> +    goto done;
> +
> +  while (isspace (*p))
> +    ++p;
> +  if (*p == '<')
> +    ptr = p = find_template_name_end (ptr);
> +
> +  if (name_end (ptr))
> +    goto done;
> +
> +  /* Keep method overload information.  */
> +  if (is_overloaded (p))

It seems to me here could be sufficient instead of is_overloaded just:
  if (*p == '(')

Or do you have a countercase where it would not work?


> +    ptr = p = find_method_overload_end (p);
> +
> +  if (name_end (ptr))
> +    goto done;
> +
> +  /* Keep important keywords.  */  
> +  while (isspace (*p))
> +    ++p;
> +  if (strncmp (p, "const", 5) == 0
> +      && (isspace (*(p + 5)) || *(p + 5) == '\0' || *(p + 5) == '\''))

*(p + 5) == '\''
->
strchr (get_gdb_completer_quote_characters (), p[5]) != NULL


> +    ptr = p = p + 5;
> +
> + done:
> +  while (isspace (*(ptr - 1)))
> +    --ptr;

Underrun of the strings you reported as present even in pre-physname GDB so
I have just filed it as:
	decode_linespec_1 vagrind errors on ""
	http://sourceware.org/bugzilla/show_bug.cgi?id=12535


> +  return ptr;
> +}
> +
>  \f
>  /* The parser of linespec itself.  */
>  
[...]
> @@ -1470,6 +1585,10 @@ decode_compound (char **argptr, int funf
>    /* We couldn't find a class, so we're in case 2 above.  We check the
>       entire name as a symbol instead.  */
>  
> +  if (current_language->la_language == language_cplus
> +      || current_language->la_language == language_java)
> +      p = keep_name_info (p);

Wrong indentation.


> +
>    copy = (char *) alloca (p - saved_arg2 + 1);
>    memcpy (copy, saved_arg2, p - saved_arg2);
>    /* Note: if is_quoted should be true, we snuff out quote here
[...]
> @@ -1594,20 +1722,32 @@ find_method (int funfirstline, char ***c
>        /* If we were given a specific overload instance, use that
>  	 (or error if no matches were found).  Otherwise ask the user
>  	 which one to use.  */
> -      if (strchr (saved_arg, '(') != NULL)
> +      if (is_overloaded (saved_arg))

It seems to me here could be sufficient instead of is_overloaded just:
  if (strchr (copy, '(') != NULL)

Or do you have a countercase where it would not work?


>  	{
>  	  int i;
> -	  char *name = saved_arg;
> -	  char *canon = cp_canonicalize_string (name);
> +	  char *name;
> +	  char *canon;
>  	  struct cleanup *cleanup;
>  
> +	  /* Construct the proper search name based on SYM_CLASS and COPY.
> +	     SAVED_ARG may contain a valid name, but that name might not be
> +	     what is actually stored in the symbol table.  For example,
> +	     if SAVED_ARG (and SYM_CLASS) were found via an import
> +	     ("using namespace" in C++), then the physname of
> +	     SYM_CLASS ("A::myclass") may not be the same as SAVED_ARG
> +	     ("myclass").  */
> +	  name = xmalloc (strlen (SYMBOL_NATURAL_NAME (sym_class))
> +			  + 2 /* "::" */ + strlen (copy) + 1);
> +	  strcpy (name, SYMBOL_NATURAL_NAME (sym_class));
> +	  strcat (name, "::");
> +	  strcat (name, copy);
> +	  canon = cp_canonicalize_string (name);
>  	  if (canon != NULL)
>  	    {
> +	      xfree (name);
>  	      name = canon;
> -	      cleanup = make_cleanup (xfree, canon);
>  	    }
> -	  else
> -	    cleanup = make_cleanup (null_cleanup, NULL);
> +	  cleanup = make_cleanup (xfree, name);
>  
>  	  for (i = 0; i < i1; ++i)
>  	    {
[...]


It is approved with these changes if you agree with them.  I do not expect
anyone else is going to futher review it.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-14  7:52                 ` Jan Kratochvil
@ 2011-03-15 19:03                   ` Keith Seitz
  2011-03-16  8:28                     ` Jan Kratochvil
  0 siblings, 1 reply; 29+ messages in thread
From: Keith Seitz @ 2011-03-15 19:03 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

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

Hi, Jan, thank you for taking the time to look at this.  Believe me when 
I say, I feel for ya!

On 03/13/2011 03:28 PM, Jan Kratochvil wrote:
> Although I wrote `isalnum' it is a wrong character class, for example
> isalnum ('_') == 0 but `_' is a valid identifier character.

I have a cleanup that I've been working on which changed all this, but 
you've figured out before I have submitted my cleanup.  I have done 
exactly as you suggest.

> All these cases should be written like p[6] both for the IMO better
> readability and for making it less fragile against bugs like `(*p + 6)'.

Done.

> There may be also "task" catching but that is used by Ada and it already
> worked before without such exception so it is probably OK.

I looked in the manual and the various gdb command help strings, but I 
could not find this.  Maybe I didn't check Ada-specific commands?  In 
any case, this whole linespec thing seems overtly fragile to begin with. 
  It is a very poor abstraction for what is essentially a 
language-dependent task (of identifying names).

But it is trivial enough to add to this function, for whatever that's worth.

> It seems to me here could be sufficient instead of is_overloaded just:
>    if (*p == '(')
>
> Or do you have a countercase where it would not work?

Nope. That was one of the cleanups that I'd arrived at last week, too.

> *(p + 5) == '\''
> ->
> strchr (get_gdb_completer_quote_characters (), p[5]) != NULL

Done.

> Underrun of the strings you reported as present even in pre-physname GDB so
> I have just filed it as:
> 	decode_linespec_1 vagrind errors on ""
> 	http://sourceware.org/bugzilla/show_bug.cgi?id=12535

Indeed! I've changed this to check that ptr > start (where start is ptr 
at entry to the function).

>> +  if (current_language->la_language == language_cplus
>> +      || current_language->la_language == language_java)
>> +      p = keep_name_info (p);
>
> Wrong indentation.

Fixed.

> It seems to me here could be sufficient instead of is_overloaded just:
>    if (strchr (copy, '(') != NULL)
>
> Or do you have a countercase where it would not work?

No countercase.  I agree that strchr should be sufficient here when 
comparing the variable copy instead of real_saved_arg.  Thank you for 
pointing that out -- I missed that.

> It is approved with these changes if you agree with them.  I do not expect
> anyone else is going to futher review it.

I'll await one final "OK" from you before committing, just in case you 
seen anything additional on which you'd like to comment.

Thanks again for looking at this.  FWIW, I've attached only the linespec 
patch which includes the requested changes.

Keith

[-- Attachment #2: linespec.c.patch --]
[-- Type: text/plain, Size: 11072 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.113
diff -u -p -r1.113 linespec.c
--- linespec.c	4 Mar 2011 20:07:22 -0000	1.113
+++ linespec.c	15 Mar 2011 18:40:18 -0000
@@ -41,6 +41,7 @@
 #include "mi/mi-cmds.h"
 #include "target.h"
 #include "arch-utils.h"
+#include <ctype.h>
 
 /* We share this one with symtab.c, but it is not exported widely.  */
 
@@ -213,6 +214,19 @@ find_methods (struct type *t, char *name
   int i1 = 0;
   int ibase;
   char *class_name = type_name_no_tag (t);
+  struct cleanup *cleanup;
+  char *canon;
+
+  /* NAME is typed by the user: it needs to be canonicalized before
+     passing to lookup_symbol.  */
+  canon = cp_canonicalize_string (name);
+  if (canon != NULL)
+    {
+      name = canon;
+      cleanup = make_cleanup (xfree, name);
+    }
+  else
+    cleanup = make_cleanup (null_cleanup, NULL);
 
   /* Ignore this class if it doesn't have a name.  This is ugly, but
      unless we figure out how to get the physname without the name of
@@ -275,6 +289,7 @@ find_methods (struct type *t, char *name
       i1 += find_methods (TYPE_BASECLASS (t, ibase), name,
 			  language, sym_arr + i1);
 
+  do_cleanups (cleanup);
   return i1;
 }
 
@@ -663,6 +678,68 @@ find_method_overload_end (char *p)
 
   return p;
 }
+
+/* Does P point to a sequence of characters which implies the end
+   of a name?  Terminals include "if" and "thread" clauses. */
+
+static int
+name_end (char *p)
+{
+  while (isspace (*p))
+    ++p;
+  if (*p == 'i' && p[1] == 'f'
+      && (isspace (p[2]) || p[2] == '\0' || p[2] == '('))
+    return 1;
+
+  if (strncmp (p, "thread", 6) == 0
+      && (isspace (p[6]) || p[6] == '\0'))
+    return 1;
+
+  return 0;
+}
+
+/* Keep important information used when looking up a name.  This includes
+   template parameters, overload information, and important keywords.  */
+
+static char *
+keep_name_info (char *ptr)
+{
+  char *p = ptr;
+  char *start = ptr;
+
+  /* Keep any template parameters.  */
+  if (name_end (ptr))
+    goto done;
+
+  while (isspace (*p))
+    ++p;
+  if (*p == '<')
+    ptr = p = find_template_name_end (ptr);
+
+  if (name_end (ptr))
+    goto done;
+
+  /* Keep method overload information.  */
+  if (*p == '(')
+    ptr = p = find_method_overload_end (p);
+
+  if (name_end (ptr))
+    goto done;
+
+  /* Keep important keywords.  */  
+  while (isspace (*p))
+    ++p;
+  if (strncmp (p, "const", 5) == 0
+      && (isspace (p[5]) || p[5] == '\0'
+	  || strchr (get_gdb_completer_quote_characters (), p[5]) != NULL))
+    ptr = p = p + 5;
+
+ done:
+  while (ptr > start && isspace (*(ptr - 1)))
+    --ptr;
+  return ptr;
+}
+
 \f
 /* The parser of linespec itself.  */
 
@@ -871,17 +948,8 @@ decode_line_1 (char **argptr, int funfir
       p = skip_quoted (*argptr);
     }
 
-  /* Keep any template parameters.  */
-  if (*p == '<')
-    p = find_template_name_end (p);
-
-  /* Keep method overload information.  */
-  if (*p == '(')
-    p = find_method_overload_end (p);
-
-  /* Make sure we keep important kewords like "const".  */
-  if (strncmp (p, " const", 6) == 0)
-    p += 6;
+  /* Keep any important naming information.  */
+  p = keep_name_info (p);
 
   copy = (char *) alloca (p - *argptr + 1);
   memcpy (copy, *argptr, p - *argptr);
@@ -1057,6 +1125,10 @@ locate_first_half (char **argptr, int *i
 	    error (_("malformed template specification in command"));
 	  p = temp_end;
 	}
+
+      if (p[0] == '(')
+	p = find_method_overload_end (p);
+
       /* Check for a colon and a plus or minus and a [ (which
          indicates an Objective-C method).  */
       if (is_objc_method_format (p))
@@ -1224,7 +1296,7 @@ decode_objc (char **argptr, int funfirst
 
 static struct symtabs_and_lines
 decode_compound (char **argptr, int funfirstline, char ***canonical,
-		 char *saved_arg, char *p, int *not_found_ptr)
+		 char *the_real_saved_arg, char *p, int *not_found_ptr)
 {
   struct symtabs_and_lines values;
   char *p2;
@@ -1235,6 +1307,23 @@ decode_compound (char **argptr, int funf
   struct symbol *sym_class;
   struct type *t;
   char *saved_java_argptr = NULL;
+  char *saved_arg;
+
+  /* If the user specified any completer quote characters in the input,
+     strip them.  They are superfluous.  */
+  saved_arg = alloca (strlen (the_real_saved_arg) + 1);
+  {
+    char *dst = saved_arg;
+    char *src = the_real_saved_arg;
+    char *quotes = get_gdb_completer_quote_characters ();
+    while (*src != '\0')
+      {
+	if (strchr (quotes, *src) == NULL)
+	  *dst++ = *src;
+	++src;
+      }
+    *dst = '\0';
+  }
 
   /* First check for "global" namespace specification, of the form
      "::foo".  If found, skip over the colons and jump to normal
@@ -1251,8 +1340,10 @@ decode_compound (char **argptr, int funf
         find_method.
 
      2) AAA::inA isn't the name of a class.  In that case, either the
-        user made a typo or AAA::inA is the name of a namespace.
-        Either way, we just look up AAA::inA::fun with lookup_symbol.
+        user made a typo, AAA::inA is the name of a namespace, or it is
+        the name of a minimal symbol.
+        We just look up AAA::inA::fun with lookup_symbol.  If that fails,
+        try lookup_minimal_symbol.
 
      Thus, our first task is to find everything before the last set of
      double-colons and figure out if it's the name of a class.  So we
@@ -1273,6 +1364,8 @@ decode_compound (char **argptr, int funf
 
   while (1)
     {
+      static char *break_characters = " \t(";
+
       /* Move pointer up to next possible class/namespace token.  */
 
       p = p2 + 1;	/* Restart with old value +1.  */
@@ -1283,8 +1376,9 @@ decode_compound (char **argptr, int funf
       /* PASS2: p2->"::fun", p->":fun" */
 
       /* Move pointer ahead to next double-colon.  */
-      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\'')
-	     && (*p != '('))
+      while (*p
+	     && strchr (break_characters, *p) == NULL
+	     && strchr (get_gdb_completer_quote_characters (), *p) == NULL)
 	{
 	  if (current_language->la_language == language_cplus)
 	    p += cp_validate_operator (p);
@@ -1308,9 +1402,12 @@ decode_compound (char **argptr, int funf
 	  else if ((p[0] == ':') && (p[1] == ':'))
 	    break;	/* Found double-colon.  */
 	  else
-	    /* PASS2: We'll keep getting here, until p->"", at which point
-	       we exit this loop.  */
-	    p++;
+	    {
+	      /* PASS2: We'll keep getting here, until P points to one of the
+		 break characters, at which point we exit this loop.  */
+	      if (*p && strchr (break_characters, *p) == NULL)
+		p++;
+	    }
 	}
 
       if (*p != ':')
@@ -1319,7 +1416,7 @@ decode_compound (char **argptr, int funf
 			   unsuccessfully all the components of the
 			   string, and p->""(PASS2).  */
 
-      /* We get here if p points to ' ', '\t', '\'', "::" or ""(i.e
+      /* We get here if p points to one of the break characters or "" (i.e.,
 	 string ended).  */
       /* Save restart for next time around.  */
       p2 = p;
@@ -1379,18 +1476,8 @@ decode_compound (char **argptr, int funf
 	      p += cp_validate_operator (p - 8) - 8;
 	    }
 
-	  /* Keep any template parameters.  */
-	  if (*p == '<')
-	    p = find_template_name_end (p);
-
-	  /* Keep method overload information.  */
-	  a = strchr (p, '(');
-	  if (a != NULL)
-	    p = find_method_overload_end (a);
-
-	  /* Make sure we keep important kewords like "const".  */
-	  if (strncmp (p, " const", 6) == 0)
-	    p += 6;
+	  /* Keep any important naming information.  */
+	  p = keep_name_info (p);
 
 	  /* Java may append typenames,  so assume that if there is
 	     anything else left in *argptr, it must be a typename.  */
@@ -1470,6 +1557,10 @@ decode_compound (char **argptr, int funf
   /* We couldn't find a class, so we're in case 2 above.  We check the
      entire name as a symbol instead.  */
 
+  if (current_language->la_language == language_cplus
+      || current_language->la_language == language_java)
+    p = keep_name_info (p);
+
   copy = (char *) alloca (p - saved_arg2 + 1);
   memcpy (copy, saved_arg2, p - saved_arg2);
   /* Note: if is_quoted should be true, we snuff out quote here
@@ -1479,15 +1570,24 @@ decode_compound (char **argptr, int funf
   *argptr = (*p == '\'') ? p + 1 : p;
 
   /* Look up entire name.  */
-  sym = lookup_symbol (copy, 0, VAR_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), VAR_DOMAIN, 0);
   if (sym)
     return symbol_found (funfirstline, canonical, copy, sym, NULL);
+  else
+    {
+      struct minimal_symbol *msym;
+
+      /* Couldn't find any interpretation as classes/namespaces.  As a last
+	 resort, try the minimal symbol tables.  */
+      msym = lookup_minimal_symbol (copy, NULL, NULL);
+      if (msym != NULL)
+	return minsym_found (funfirstline, msym);
+    }    
 
-  /* Couldn't find any interpretation as classes/namespaces, so give
-     up.  The quotes are important if copy is empty.  */
+  /* Couldn't find a minimal symbol, either, so give up.  */
   if (not_found_ptr)
     *not_found_ptr = 1;
-  cplusplus_error (saved_arg,
+  cplusplus_error (the_real_saved_arg,
 		   "Can't find member of namespace, "
 		   "class, struct, or union named \"%s\"\n",
 		   copy);
@@ -1526,7 +1626,7 @@ lookup_prefix_sym (char **argptr, char *
   /* At this point p1->"::inA::fun", p->"inA::fun" copy->"AAA",
      argptr->"inA::fun".  */
 
-  sym = lookup_symbol (copy, 0, STRUCT_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_selected_block (0), STRUCT_DOMAIN, 0);
   if (sym == NULL)
     {
       /* Typedefs are in VAR_DOMAIN so the above symbol lookup will
@@ -1594,20 +1694,32 @@ find_method (int funfirstline, char ***c
       /* If we were given a specific overload instance, use that
 	 (or error if no matches were found).  Otherwise ask the user
 	 which one to use.  */
-      if (strchr (saved_arg, '(') != NULL)
+      if (strchr (copy, '('))
 	{
 	  int i;
-	  char *name = saved_arg;
-	  char *canon = cp_canonicalize_string (name);
+	  char *name;
+	  char *canon;
 	  struct cleanup *cleanup;
 
+	  /* Construct the proper search name based on SYM_CLASS and COPY.
+	     SAVED_ARG may contain a valid name, but that name might not be
+	     what is actually stored in the symbol table.  For example,
+	     if SAVED_ARG (and SYM_CLASS) were found via an import
+	     ("using namespace" in C++), then the physname of
+	     SYM_CLASS ("A::myclass") may not be the same as SAVED_ARG
+	     ("myclass").  */
+	  name = xmalloc (strlen (SYMBOL_NATURAL_NAME (sym_class))
+			  + 2 /* "::" */ + strlen (copy) + 1);
+	  strcpy (name, SYMBOL_NATURAL_NAME (sym_class));
+	  strcat (name, "::");
+	  strcat (name, copy);
+	  canon = cp_canonicalize_string (name);
 	  if (canon != NULL)
 	    {
+	      xfree (name);
 	      name = canon;
-	      cleanup = make_cleanup (xfree, canon);
 	    }
-	  else
-	    cleanup = make_cleanup (null_cleanup, NULL);
+	  cleanup = make_cleanup (xfree, name);
 
 	  for (i = 0; i < i1; ++i)
 	    {

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-15 19:03                   ` Keith Seitz
@ 2011-03-16  8:28                     ` Jan Kratochvil
  2011-03-16 13:58                       ` Tom Tromey
  2011-03-16 23:20                       ` Keith Seitz
  0 siblings, 2 replies; 29+ messages in thread
From: Jan Kratochvil @ 2011-03-16  8:28 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Hi Keith,

On Tue, 15 Mar 2011 19:48:09 +0100, Keith Seitz wrote:
> Hi, Jan, thank you for taking the time to look at this.  Believe me
> when I say, I feel for ya!

fortunately it seems this part of linespec is done.


> Thanks again for looking at this.  FWIW, I've attached only the
> linespec patch which includes the requested changes.

I guess there should have been a ChangeLog for FSF patch reviews compliance
but I am fine with it as is.


> +static char *
> +keep_name_info (char *ptr)
> +{
> +  char *p = ptr;
> +  char *start = ptr;
> +
> +  /* Keep any template parameters.  */
> +  if (name_end (ptr))
> +    goto done;
> +
> +  while (isspace (*p))
> +    ++p;
> +  if (*p == '<')
> +    ptr = p = find_template_name_end (ptr);
> +
> +  if (name_end (ptr))
> +    goto done;
> +
> +  /* Keep method overload information.  */
> +  if (*p == '(')
> +    ptr = p = find_method_overload_end (p);
> +
> +  if (name_end (ptr))
> +    goto done;
> +
> +  /* Keep important keywords.  */  
> +  while (isspace (*p))
> +    ++p;
> +  if (strncmp (p, "const", 5) == 0
> +      && (isspace (p[5]) || p[5] == '\0'
> +	  || strchr (get_gdb_completer_quote_characters (), p[5]) != NULL))
> +    ptr = p = p + 5;
> +
> + done:
> +  while (ptr > start && isspace (*(ptr - 1)))
                                    ptr[-1]
> +    --ptr;
> +  return ptr;
> +}

I see now the goto is in fact not any win here.

static const char *
remove_tail_spaces (const char *start, const char *s)
{
  while (s > start && isspace (s[-1]))
    s--;

  return s;
}
...
  if (name_end (ptr))
    return remove_tail_spaces (start, ptr);

So asking for this change if I haven't missed anything and a check-in.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-16  8:28                     ` Jan Kratochvil
@ 2011-03-16 13:58                       ` Tom Tromey
  2011-03-16 23:20                       ` Keith Seitz
  1 sibling, 0 replies; 29+ messages in thread
From: Tom Tromey @ 2011-03-16 13:58 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Keith Seitz, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> static const char *
Jan> remove_tail_spaces (const char *start, const char *s)
Jan> {
Jan>   while (s > start && isspace (s[-1]))
Jan>     s--;
Jan>   return s;
Jan> }

Seems like a candidate for cli-utils.c.

Tom

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-16  8:28                     ` Jan Kratochvil
  2011-03-16 13:58                       ` Tom Tromey
@ 2011-03-16 23:20                       ` Keith Seitz
  2011-03-17  3:19                         ` Joel Brobecker
  1 sibling, 1 reply; 29+ messages in thread
From: Keith Seitz @ 2011-03-16 23:20 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On 03/15/2011 11:59 PM, Jan Kratochvil wrote:
> So asking for this change if I haven't missed anything and a check-in.

I've changed this, updated the ChangeLog, added a 
"remove_trailing_whitespace" function to cli/cli-utils.c as Tom 
suggested, and checked this all in.

Thank you very much for the reviews.  I know how difficult this bit of 
code is to sift through.

Keith

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-16 23:20                       ` Keith Seitz
@ 2011-03-17  3:19                         ` Joel Brobecker
  2011-03-17  9:11                           ` Jan Kratochvil
  0 siblings, 1 reply; 29+ messages in thread
From: Joel Brobecker @ 2011-03-17  3:19 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Jan Kratochvil, gdb-patches

> >So asking for this change if I haven't missed anything and a check-in.
> 
> I've changed this, updated the ChangeLog, added a
> "remove_trailing_whitespace" function to cli/cli-utils.c as Tom
> suggested, and checked this all in.

Does it mean that we should be good to go for the 7.2.1 release soon?
I don't think that the patches have been checked in the 7.2 branch,
but these PRs have been identified as critical for the .1 release.
It seems that finding these fixes have been a bit long and treacherous,
so we may want to think twice about putting it in 7.2 (?); or wait
a certain amount of time to observer the change in HEAD before
porting to the branch...

-- 
Joel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-17  3:19                         ` Joel Brobecker
@ 2011-03-17  9:11                           ` Jan Kratochvil
  2011-03-17 13:21                             ` Joel Brobecker
  0 siblings, 1 reply; 29+ messages in thread
From: Jan Kratochvil @ 2011-03-17  9:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Keith Seitz, gdb-patches

On Thu, 17 Mar 2011 00:20:46 +0100, Joel Brobecker wrote:
> Does it mean that we should be good to go for the 7.2.1 release soon?

There is yet
	Re: [RFA] c++/12506
	http://sourceware.org/ml/gdb-patches/2011-03/msg00742.html

which is also a physname regression and the fix is not anything large IMO.


> It seems that finding these fixes have been a bit long and treacherous,
> so we may want to think twice about putting it in 7.2 (?);

as 7.3 should be released soon with all the delays on 7.2.x I am no longer
sure how much valid is the 7.2.x release now.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)
  2011-03-17  9:11                           ` Jan Kratochvil
@ 2011-03-17 13:21                             ` Joel Brobecker
  0 siblings, 0 replies; 29+ messages in thread
From: Joel Brobecker @ 2011-03-17 13:21 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Keith Seitz, gdb-patches

> > Does it mean that we should be good to go for the 7.2.1 release soon?
> 
> There is yet
> 	Re: [RFA] c++/12506
> 	http://sourceware.org/ml/gdb-patches/2011-03/msg00742.html
> 
> which is also a physname regression and the fix is not anything large IMO.

Thanks for pointing this one out. I've added it to both 7.2 & 7.3
checklists.

> > It seems that finding these fixes have been a bit long and treacherous,
> > so we may want to think twice about putting it in 7.2 (?);
> 
> as 7.3 should be released soon with all the delays on 7.2.x I am no longer
> sure how much valid is the 7.2.x release now.

I'm with you on the relative usefulness of producing a 7.2.1, and
I'm not sure that I would use it myself or not.  So I'm happy to
drop this release. But it's only about an hour of my time, so I am
also just as happy making it, if it's useful to someone.

For 7.3, we still have a few extra items to take care of:
  * Merge your work on the ifunc branch
  * Pedro's <unavailable> value support
  * NEWS & Manual checks (easy and probably fast)

Once this is done, we can branch, and produce our first RC, but it's
then at least another 2 weeks before we produce the first release.
So, I'm going to venture a guess that we're still, say, 4 weeks away
from release.

-- 
Joel

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2011-03-17 12:41 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-09  0:50 [RFA] c++/11734 revisited Keith Seitz
2010-12-09  4:02 ` Eli Zaretskii
2010-12-09 21:45 ` Tom Tromey
2010-12-09 21:52   ` Jan Kratochvil
2010-12-10 15:21     ` Keith Seitz
2010-12-14 20:03   ` Keith Seitz
2011-01-24 18:15     ` Jan Kratochvil
2011-01-26 23:14       ` Jan Kratochvil
2011-02-06 22:04     ` Jan Kratochvil
2011-02-06 22:45     ` [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273) Jan Kratochvil
2011-02-08 21:42       ` Tom Tromey
2011-02-10 21:45       ` Keith Seitz
2011-02-17 18:37         ` Keith Seitz
2011-02-18  3:24           ` Keith Seitz
2011-02-21 11:41           ` Jan Kratochvil
2011-02-24 20:41             ` Keith Seitz
2011-02-27 21:18             ` Jan Kratochvil
2011-03-01 22:00               ` Keith Seitz
2011-03-14  7:52                 ` Jan Kratochvil
2011-03-15 19:03                   ` Keith Seitz
2011-03-16  8:28                     ` Jan Kratochvil
2011-03-16 13:58                       ` Tom Tromey
2011-03-16 23:20                       ` Keith Seitz
2011-03-17  3:19                         ` Joel Brobecker
2011-03-17  9:11                           ` Jan Kratochvil
2011-03-17 13:21                             ` Joel Brobecker
2011-02-06 22:46     ` [patch 1/3] revert physname part (b) [Re: [RFA] c++/11734 revisited] Jan Kratochvil
2011-02-06 22:46     ` [patch 3/3] Various linespec fixups " Jan Kratochvil
2011-02-06 22:46     ` [patch 2/3] Keith's psymtabs fix " Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).