[RFA] "Error in re-setting breakpoint," c++/12750

[RFA] "Error in re-setting breakpoint," c++/12750

[Keith Seitz]

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

Hi,

This bug is a regression caused by some breakpoint canonicalization 
churn that committed a while ago. Basically:

(gdb) break xxx::fun1
Breakpoint 1 at 0xblahblah: file main.cc, line blah.
(gdb) run
Starting program: a.out
Error in re-setting breakpoint 1: Can't find member of namespace, class, 
  struct, or union named "main.cc:xxx::fun1"

As you can see, the first time decode_line_1 is called (with 
"xxx::fun1"), parsing the linespec succeeds. The breakpoint code stores 
a canonicalized version of this string ("main.cc:xxx::fun1"), and 
decode_line_1 is unable to properly parse this.

The attached patch is an attempt to correct this. It essentially makes 
two changes: 1) look up the file symtab first; 2) pass this file symtab 
to any function which might call lookup_symbol[_*], and use this symtab 
to determine the appropriate block to search.

The test case included here will require the patch for symtab/12704. I 
ran across this bug while working on that bug, so I've included a test 
for that specific case, too.

Tested on the buildbot.

Keith

	PR c++/12750
	* linespec.c (get_search_block): New function.
	(find_methods): Add FILE_SYMTATB parameter and use it and
	get_search_block to pass an appropriate block to
	lookup_symbol_in_namespace.
	(decode_line_1): Record if *ARGPTR is single-quote enclosed.
	Check if *ARGPTR starts with a filename first.
	If it does, call locate_first_half again to locate the next
	"first half" of the linespec.
	Pass FILE_SYMTATB to decode_objc and decode_compound.
	Swallow the trailing single-quote if IS_SQUOTE_ENCLOSED.
	(locate_first_half): Stop on the first colon seen.
	(decode_compound): Add FILE_SYMTAB parameter.
	Pass FILE_SYMTAB to lookup_prefix_sym and find_method.
	(lookup_prefix_sym): Add FILE_SYMTAB parameter and use
	get_search_block with lookup_symbol.
	(find_method): Add FILE_SYMTAB parameter and pass it to
	find_methods.
	(decode_objc): Use get_search_block.

testsuite/ChangeLog

2010-05-18  Keith Seitz  <keiths@redhat.com>

	PR c++/12750
	* gdb.cp/static-method.cc: New file.
	* gdb.cp/static-method.exp: New file.

[-- Attachment #2: 12750.patch --]
[-- Type: text/plain, Size: 15866 bytes --]

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 4658e2d..b33f599 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -66,17 +66,20 @@ static struct symtabs_and_lines decode_objc (char **argptr,
 static struct symtabs_and_lines decode_compound (char **argptr,
 						 int funfirstline,
 						 struct linespec_result *canonical,
+						 struct symtab *file_symtab,
 						 char *saved_arg,
 						 char *p);
 
-static struct symbol *lookup_prefix_sym (char **argptr, char *p);
+static struct symbol *lookup_prefix_sym (char **argptr, char *p,
+					 struct symtab *);
 
 static struct symtabs_and_lines find_method (int funfirstline,
 					     struct linespec_result *canonical,
 					     char *saved_arg,
 					     char *copy,
 					     struct type *t,
-					     struct symbol *sym_class);
+					     struct symbol *sym_class,
+					     struct symtab *);
 
 static void cplusplus_error (const char *name, const char *fmt, ...)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 3);
@@ -84,7 +87,7 @@ static void cplusplus_error (const char *name, const char *fmt, ...)
 static int total_number_of_methods (struct type *type);
 
 static int find_methods (struct type *, char *,
-			 enum language, struct symbol **);
+			 enum language, struct symbol **, struct symtab *);
 
 static int add_matching_methods (int method_counter, struct type *t,
 				 enum language language,
@@ -203,6 +206,30 @@ total_number_of_methods (struct type *type)
   return count;
 }
 
+/* Returns the block to be used for symbol searches for the given SYMTAB,
+   which may be NULL.  */
+
+static struct block *
+get_search_block (struct symtab *symtab)
+{
+  struct block *block;
+
+  if (symtab != NULL)
+    block = BLOCKVECTOR_BLOCK (BLOCKVECTOR (symtab), STATIC_BLOCK);
+  else
+    {
+      enum language save_language;
+
+      /* get_selected_block can change the current language when there is
+	 no selected frame yet.  */
+      save_language = current_language->la_language;
+      block = get_selected_block (0);
+      set_language (save_language);
+    }
+
+  return block;
+}
+
 /* Recursive helper function for decode_line_1.
    Look for methods named NAME in type T.
    Return number of matches.
@@ -212,7 +239,7 @@ total_number_of_methods (struct type *type)
 
 static int
 find_methods (struct type *t, char *name, enum language language,
-	      struct symbol **sym_arr)
+	      struct symbol **sym_arr, struct symtab *file_symtab)
 {
   int i1 = 0;
   int ibase;
@@ -235,7 +262,7 @@ find_methods (struct type *t, char *name, enum language language,
      unless we figure out how to get the physname without the name of
      the class, then the loop can't do any good.  */
   if (class_name
-      && (lookup_symbol_in_language (class_name, (struct block *) NULL,
+      && (lookup_symbol_in_language (class_name, get_search_block (file_symtab),
 			 STRUCT_DOMAIN, language, (int *) NULL)))
     {
       int method_counter;
@@ -290,7 +317,7 @@ find_methods (struct type *t, char *name, enum language language,
   if (i1 == 0)
     for (ibase = 0; ibase < TYPE_N_BASECLASSES (t); ibase++)
       i1 += find_methods (TYPE_BASECLASS (t, ibase), name,
-			  language, sym_arr + i1);
+			  language, sym_arr + i1, file_symtab);
 
   do_cleanups (cleanup);
   return i1;
@@ -806,6 +833,8 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
   char *saved_arg = *argptr;
   /* If IS_QUOTED, the end of the quoted bit.  */
   char *end_quote = NULL;
+  /* Is *ARGPTR enclosed in single quotes?  */
+  int is_squote_enclosed = 0;
   /* The "first half" of the linespec.  */
   char *first_half;
 
@@ -829,7 +858,11 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 		       **argptr) != NULL);
 
   if (is_quoted)
-    end_quote = skip_quoted (*argptr);
+    {
+      end_quote = skip_quoted (*argptr);
+      if (*end_quote == '\0')
+	is_squote_enclosed = 1;
+    }
 
   /* Check to see if it's a multipart linespec (with colons or
      periods).  */
@@ -842,6 +875,26 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   first_half = p = locate_first_half (argptr, &is_quote_enclosed);
 
+  /* First things first: if ARGPTR starts with a filename, get its
+     symtab and strip the filename from ARGPTR.  */
+  TRY_CATCH (file_exception, RETURN_MASK_ERROR)
+    {
+      file_symtab = symtab_from_filename (argptr, p, is_quote_enclosed);
+    }
+
+  if (file_exception.reason >= 0)
+    {
+      /* Check for single quotes on the non-filename part.  */
+      is_quoted = (**argptr
+		   && strchr (get_gdb_completer_quote_characters (),
+			      **argptr) != NULL);
+      if (is_quoted)
+	end_quote = skip_quoted (*argptr);
+
+      /* Locate the next "half" of the linespec.  */
+      first_half = p = locate_first_half (argptr, &is_quote_enclosed);
+    }
+
   /* Check if this is an Objective-C method (anything that starts with
      a '+' or '-' and a '[').  */
   if (is_objc_method_format (p))
@@ -852,7 +905,7 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
   {
     struct symtabs_and_lines values;
 
-    values = decode_objc (argptr, funfirstline, NULL,
+    values = decode_objc (argptr, funfirstline, file_symtab,
 			  canonical, saved_arg);
     if (values.sals != NULL)
       return values;
@@ -876,20 +929,14 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 	  if (is_quote_enclosed)
 	    ++saved_arg;
 	  values = decode_compound (argptr, funfirstline, canonical,
-				    saved_arg, p);
-	  if (is_quoted && **argptr == '\'')
+				    file_symtab, saved_arg, p);
+	  if ((is_quoted || is_squote_enclosed) && **argptr == '\'')
 	    *argptr = *argptr + 1;
 	  return values;
 	}
 
-      /* No, the first part is a filename; set file_symtab to be that file's
-	 symtab.  Also, move argptr past the filename.  */
-
-      TRY_CATCH (file_exception, RETURN_MASK_ERROR)
-	{
-	  file_symtab = symtab_from_filename (argptr, p, is_quote_enclosed);
-	}
-      /* If that failed, maybe we have `function:label'.  */
+      /* If there was an exception looking up a specified filename earlier,
+	 then check whether we were really given `function:label'.   */
       if (file_exception.reason < 0)
 	{
 	  function_symbol = find_function_symbol (argptr, p, is_quote_enclosed);
@@ -946,7 +993,7 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
   if (**argptr == '$')		/* May be a convenience variable.  */
     /* One or two $ chars possible.  */
     p = skip_quoted (*argptr + (((*argptr)[1] == '$') ? 2 : 1));
-  else if (is_quoted)
+  else if (is_quoted || is_squote_enclosed)
     {
       p = end_quote;
       if (p[-1] != '\'')
@@ -976,7 +1023,7 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
       copy[p - *argptr - 1] = '\0';
       copy++;
     }
-  else if (is_quoted)
+  else if (is_quoted || is_squote_enclosed)
     copy[p - *argptr - 1] = '\0';
   while (*p == ' ' || *p == '\t')
     p++;
@@ -1152,13 +1199,11 @@ locate_first_half (char **argptr, int *is_quote_enclosed)
 	  break;
 	}
       /* Check for the end of the first half of the linespec.  End of
-         line, a tab, a double colon or the last single colon, or a
-         space.  But if enclosed in double quotes we do not break on
-         enclosed spaces.  */
+         line, a tab, a colon or a space.  But if enclosed in double
+	 quotes we do not break on enclosed spaces.  */
       if (!*p
 	  || p[0] == '\t'
-	  || ((p[0] == ':')
-	      && ((p[1] == ':') || (strchr (p + 1, ':') == NULL)))
+	  || (p[0] == ':')
 	  || ((p[0] == ' ') && !*is_quote_enclosed))
 	break;
       if (p[0] == '.' && strchr (p, ':') == NULL)
@@ -1217,20 +1262,8 @@ decode_objc (char **argptr, int funfirstline, struct symtab *file_symtab,
   values.sals = NULL;
   values.nelts = 0;
 
-  if (file_symtab != NULL)
-    block = BLOCKVECTOR_BLOCK (BLOCKVECTOR (file_symtab), STATIC_BLOCK);
-  else
-    {
-      enum language save_language;
-
-      /* get_selected_block can change the current language when there is
-	 no selected frame yet.  */
-      save_language = current_language->la_language;
-      block = get_selected_block (0);
-      set_language (save_language);
-    }
-
-  find_imps (file_symtab, block, *argptr, NULL, &i1, &i2); 
+  find_imps (file_symtab, get_search_block (file_symtab), *argptr,
+	     NULL, &i1, &i2); 
     
   if (i1 > 0)
     {
@@ -1312,7 +1345,7 @@ decode_objc (char **argptr, int funfirstline, struct symtab *file_symtab,
 
 static struct symtabs_and_lines
 decode_compound (char **argptr, int funfirstline,
-		 struct linespec_result *canonical,
+		 struct linespec_result *canonical, struct symtab *file_symtab,
 		 char *the_real_saved_arg, char *p)
 {
   struct symtabs_and_lines values;
@@ -1460,7 +1493,7 @@ decode_compound (char **argptr, int funfirstline,
   /* Before the call, argptr->"AAA::inA::fun",
      p->"", p2->"::fun".  After the call: argptr->"fun", p, p2
      unchanged.  */
-  sym_class = lookup_prefix_sym (argptr, p2);
+  sym_class = lookup_prefix_sym (argptr, p2, file_symtab);
 
   /* If sym_class has been found, and if "AAA::inA" is a class, then
      we're in case 1 above.  So we look up "fun" as a method of that
@@ -1555,7 +1588,7 @@ decode_compound (char **argptr, int funfirstline,
 	 we'll lookup the whole string in the symbol tables.  */
 
       values = find_method (funfirstline, canonical, saved_arg,
-			    copy, t, sym_class);
+			    copy, t, sym_class, file_symtab);
       if (saved_java_argptr != NULL && values.nelts == 1)
 	{
 	  /* The user specified a specific return type for a java method.
@@ -1625,7 +1658,7 @@ decode_compound (char **argptr, int funfirstline,
    example, say ARGPTR is "AAA::inA::fun" and P is "::inA::fun".  */
 
 static struct symbol *
-lookup_prefix_sym (char **argptr, char *p)
+lookup_prefix_sym (char **argptr, char *p, struct symtab *file_symtab)
 {
   char *p1;
   char *copy;
@@ -1648,7 +1681,7 @@ lookup_prefix_sym (char **argptr, char *p)
   /* At this point p1->"::inA::fun", p->"inA::fun" copy->"AAA",
      argptr->"inA::fun".  */
 
-  sym = lookup_symbol (copy, get_selected_block (0), STRUCT_DOMAIN, 0);
+  sym = lookup_symbol (copy, get_search_block (file_symtab), STRUCT_DOMAIN, 0);
   if (sym == NULL)
     {
       /* Typedefs are in VAR_DOMAIN so the above symbol lookup will
@@ -1678,7 +1711,8 @@ lookup_prefix_sym (char **argptr, char *p)
 static struct symtabs_and_lines
 find_method (int funfirstline, struct linespec_result *canonical,
 	     char *saved_arg,
-	     char *copy, struct type *t, struct symbol *sym_class)
+	     char *copy, struct type *t, struct symbol *sym_class,
+	     struct symtab *file_symtab)
 {
   struct symtabs_and_lines values;
   struct symbol *sym = NULL;
@@ -1689,7 +1723,8 @@ find_method (int funfirstline, struct linespec_result *canonical,
   /* Find all methods with a matching name, and put them in
      sym_arr.  */
 
-  i1 = find_methods (t, copy, SYMBOL_LANGUAGE (sym_class), sym_arr);
+  i1 = find_methods (t, copy, SYMBOL_LANGUAGE (sym_class), sym_arr,
+		     file_symtab);
 
   if (i1 == 1)
     {
@@ -2080,11 +2115,7 @@ decode_variable (char *copy, int funfirstline,
   struct symbol *sym;
   struct minimal_symbol *msymbol;
 
-  sym = lookup_symbol (copy,
-		       (file_symtab
-			? BLOCKVECTOR_BLOCK (BLOCKVECTOR (file_symtab),
-					     STATIC_BLOCK)
-			: get_selected_block (0)),
+  sym = lookup_symbol (copy, get_search_block (file_symtab),
 		       VAR_DOMAIN, 0);
 
   if (sym != NULL)
diff --git a/gdb/testsuite/gdb.cp/static-method.cc b/gdb/testsuite/gdb.cp/static-method.cc
new file mode 100644
index 0000000..5d8e0b9
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/static-method.cc
@@ -0,0 +1,46 @@
+/* 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 xxx
+{
+  namespace
+  {
+    static int func (void) { return 0; } // xxx::func
+    class A
+    {
+    public:
+      static int func (void) { return 0; } // xxx::A::func
+    };
+  }
+}
+
+int
+test_function (void)
+{
+  return xxx::func () + xxx::A::func ();
+}
+
+int
+main (void)
+{
+  int i, x;
+
+  for (i = 0; i < 1000; ++i)
+    x += test_function ();
+
+  return x;
+}
diff --git a/gdb/testsuite/gdb.cp/static-method.exp b/gdb/testsuite/gdb.cp/static-method.exp
new file mode 100644
index 0000000..9867c2c
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/static-method.exp
@@ -0,0 +1,82 @@
+# 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.
+
+# A helper proc which sets a breakpoint at FUNC and attempts to
+# run to the breakpoint.
+proc test_breakpoint {func result} {
+    set DEC {[0-9]}
+
+    # Return to the top of the test function every time.
+    delete_breakpoints
+    if {![gdb_breakpoint test_function]} {
+        fail "set test_function breakpoint for $func"
+    } elseif {[gdb_test "continue" \
+		   "Continuing.\r\n\r\nBreakpoint $DEC+,.*test_function.*" \
+		   ""] != 0} {
+        fail "continue to test_function for $func"
+    } else {
+        gdb_breakpoint "$func"
+        gdb_test "continue" \
+            "Continuing.\r\n\r\nBreakpoint $DEC+,.*$result.*" \
+            "continue to $func"
+    }
+}
+
+if {[skip_cplus_tests]} { continue }
+
+# Tests for c++/12750
+set testfile "static-method"
+set srcfile $testfile.cc
+
+if {[prepare_for_testing $testfile $testfile $srcfile {c++ debug}]} {
+    return -1
+}
+
+if {![runto_main]} {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+set ans {(anonymous namespace)}
+set methods {}
+lappend methods "xxx::${ans}::func"
+lappend methods "xxx::${ans}::A::func"
+
+gdb_test_no_output "set listsize 1" ""
+
+foreach test $methods {
+    # The result we expect is the source code name of the symbol,
+    # i.e., without "(anonymous namespace)".
+    regsub -all [string_to_regexp "${ans}::"] $test "" expected
+    set result ".*// [string_to_regexp $expected]"
+
+    # Test whether the function/method can be "list"ed
+    # with the filename pre-pended.
+    gdb_test "list ${srcfile}:$test" $result
+    gdb_test "list '${srcfile}:$test'" $result
+    gdb_test "list '${srcfile}':$test'" $result
+    gdb_test "list ${srcfile}:'$test'" $result
+
+    # Test setting and hitting a breakoint at the function/method.
+    test_breakpoint $test $expected
+    test_breakpoint "'$test'" $expected
+}
+
+gdb_exit
+return 0

Re: [RFA] "Error in re-setting breakpoint," c++/12750

[Tom Tromey]

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

Keith> 	PR c++/12750
Keith> 	* linespec.c (get_search_block): New function.
Keith> 	(find_methods): Add FILE_SYMTATB parameter and use it and
Keith> 	get_search_block to pass an appropriate block to
Keith> 	lookup_symbol_in_namespace.
Keith> 	(decode_line_1): Record if *ARGPTR is single-quote enclosed.
Keith> 	Check if *ARGPTR starts with a filename first.
Keith> 	If it does, call locate_first_half again to locate the next
Keith> 	"first half" of the linespec.
Keith> 	Pass FILE_SYMTATB to decode_objc and decode_compound.
Keith> 	Swallow the trailing single-quote if IS_SQUOTE_ENCLOSED.
Keith> 	(locate_first_half): Stop on the first colon seen.
Keith> 	(decode_compound): Add FILE_SYMTAB parameter.
Keith> 	Pass FILE_SYMTAB to lookup_prefix_sym and find_method.
Keith> 	(lookup_prefix_sym): Add FILE_SYMTAB parameter and use
Keith> 	get_search_block with lookup_symbol.
Keith> 	(find_method): Add FILE_SYMTAB parameter and pass it to
Keith> 	find_methods.
Keith> 	(decode_objc): Use get_search_block.

Keith> 2010-05-18  Keith Seitz  <keiths@redhat.com>
Keith> 	PR c++/12750
Keith> 	* gdb.cp/static-method.cc: New file.
Keith> 	* gdb.cp/static-method.exp: New file.

Ok.

Tom

Re: [RFA] "Error in re-setting breakpoint," c++/12750

[Keith Seitz]

On 05/31/2011 01:58 PM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz<keiths@redhat.com>  writes:
>
> Keith>  	PR c++/12750
> Keith>  	* linespec.c (get_search_block): New function.
[snip]
> Keith>  2010-05-18  Keith Seitz<keiths@redhat.com>
> Keith>  	PR c++/12750
> Keith>  	* gdb.cp/static-method.cc: New file.
> Keith>  	* gdb.cp/static-method.exp: New file.
>
> Ok.

Committed. Thank you for reviewing this patch.

Keith

regression: gdb.objc/basicclass.exp [Re: [RFA] "Error in re-setting breakpoint," c++/12750]

[Jan Kratochvil]

On Wed, 01 Jun 2011 00:16:05 +0200, Keith Seitz wrote:
> On 05/31/2011 01:58 PM, Tom Tromey wrote:
> >>>>>>"Keith" == Keith Seitz<keiths@redhat.com>  writes:
> >
> >Keith>  	PR c++/12750
> >Keith>  	* linespec.c (get_search_block): New function.
> [snip]
> >Keith>  2010-05-18  Keith Seitz<keiths@redhat.com>
> >Keith>  	PR c++/12750
> >Keith>  	* gdb.cp/static-method.cc: New file.
> >Keith>  	* gdb.cp/static-method.exp: New file.
> >
> >Ok.
> 
> Committed. Thank you for reviewing this patch.

-PASS: gdb.objc/basicclass.exp: resetting breakpoints when rerunning
-PASS: gdb.objc/basicclass.exp: continue until method breakpoint
-PASS: gdb.objc/basicclass.exp: print an ivar of self
-PASS: gdb.objc/basicclass.exp: print self
-PASS: gdb.objc/basicclass.exp: print contents of self
+FAIL: gdb.objc/basicclass.exp: resetting breakpoints when rerunning
+FAIL: gdb.objc/basicclass.exp: continue until method breakpoint (GDB internal error)
+FAIL: gdb.objc/basicclass.exp: print an ivar of self
+FAIL: gdb.objc/basicclass.exp: print self
+FAIL: gdb.objc/basicclass.exp: print contents of self

835cf7647bf08d33e2c7286b45658cc58704b363 is the first bad commit
commit 835cf7647bf08d33e2c7286b45658cc58704b363
Author: Keith Seitz <keiths@redhat.com>
Date:   Tue May 31 22:13:51 2011 +0000

	PR c++/12750
	* linespec.c (get_search_block): New function.
	(find_methods): Add FILE_SYMTATB parameter and use it and
	get_search_block to pass an appropriate block to
	lookup_symbol_in_namespace.
	(decode_line_1): Record if *ARGPTR is single-quote enclosed.
	Check if *ARGPTR starts with a filename first.
	If it does, call locate_first_half again to locate the next
	"first half" of the linespec.
	Pass FILE_SYMTATB to decode_objc and decode_compound.
	Swallow the trailing single-quote if IS_SQUOTE_ENCLOSED.
	(locate_first_half): Stop on the first colon seen.
	(decode_compound): Add FILE_SYMTAB parameter.
	Pass FILE_SYMTAB to lookup_prefix_sym and find_method.
	(lookup_prefix_sym): Add FILE_SYMTAB parameter and use
	get_search_block with lookup_symbol.
	(find_method): Add FILE_SYMTAB parameter and pass it to
	find_methods.
	(decode_objc): Use get_search_block.

on {x86_64,x86_64-m32,i686}-fedora{13,14,15,rawhide}-linux-gnu (all the tested
platforms).


Regards,
Jan

Re: regression: gdb.objc/basicclass.exp [Re: [RFA] "Error in re-setting breakpoint," c++/12750]

[Keith Seitz]

On 06/01/2011 12:07 AM, Jan Kratochvil wrote:
> -PASS: gdb.objc/basicclass.exp: resetting breakpoints when rerunning
> -PASS: gdb.objc/basicclass.exp: continue until method breakpoint
> -PASS: gdb.objc/basicclass.exp: print an ivar of self
> -PASS: gdb.objc/basicclass.exp: print self
> -PASS: gdb.objc/basicclass.exp: print contents of self
> +FAIL: gdb.objc/basicclass.exp: resetting breakpoints when rerunning
> +FAIL: gdb.objc/basicclass.exp: continue until method breakpoint (GDB internal error)
> +FAIL: gdb.objc/basicclass.exp: print an ivar of self
> +FAIL: gdb.objc/basicclass.exp: print self
> +FAIL: gdb.objc/basicclass.exp: print contents of self

Oddly, using my Fedora 13 compiler (gcc version 4.4.5 20101112 (Red Hat 
4.4.5-2) (GCC)), I don't see any regressions.

So I built gcc from git (gcc version 4.7.0 20110601 (experimental) 
(GCC)), and that shows the regressions (assertion failures).

I'll get right on it.

Keith

Re: regression: gdb.objc/basicclass.exp [Re: [RFA] "Error in re-setting breakpoint," c++/12750]

[Keith Seitz]

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

On 06/01/2011 12:07 AM, Jan Kratochvil wrote:
> -PASS: gdb.objc/basicclass.exp: resetting breakpoints when rerunning
> -PASS: gdb.objc/basicclass.exp: continue until method breakpoint
> -PASS: gdb.objc/basicclass.exp: print an ivar of self
> -PASS: gdb.objc/basicclass.exp: print self
> -PASS: gdb.objc/basicclass.exp: print contents of self
> +FAIL: gdb.objc/basicclass.exp: resetting breakpoints when rerunning
> +FAIL: gdb.objc/basicclass.exp: continue until method breakpoint (GDB internal error)
> +FAIL: gdb.objc/basicclass.exp: print an ivar of self
> +FAIL: gdb.objc/basicclass.exp: print self
> +FAIL: gdb.objc/basicclass.exp: print contents of self

This is caused by a thinko in objc-lang.c:find_methods. Amongst its many 
duties, that function counts the number of ObjC symbols seen in an 
objfile, storing this result in the objfile. It is then checked every 
time find_methods is called.

However, the loop over the minimal symbols has a few continuations in 
it, e.g., to check if the symbol is in the desired block and other 
things; if not, it continues the loop with the next minimal symbol. 
Unfortunately, that bypasses the incrementing of the objfile's symbol 
counter.

The method not being in the desired block does not make it an invalid 
ObjC method for the objfile. It must still be counted for the later 
assertion.

Before the patch for c++/12750, decode_objc would always get NULL for 
file_symtab (so why does it even have that parameter?), and the check 
for the block would never occur.

This is all way more verbiage than the simple patch to fix it: increment 
the counter before shorting the loop.

Keith

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

	* objc-lang.c (find_methods): Increment objfile_csym earlier.


[-- Attachment #2: objc-regression.patch --]
[-- Type: text/plain, Size: 684 bytes --]

diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index dfa5388..592b52e 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1221,6 +1221,8 @@ find_methods (struct symtab *symtab, char type,
 	  pc = gdbarch_convert_from_func_ptr_addr (gdbarch, pc,
 						   &current_target);
 
+	  objfile_csym++;
+
 	  if (symtab)
 	    if (pc < BLOCK_START (block) || pc >= BLOCK_END (block))
 	      /* Not in the specified symtab.  */
@@ -1237,8 +1239,6 @@ find_methods (struct symtab *symtab, char type,
 	  if (parse_method (tmp, &ntype, &nclass,
 			    &ncategory, &nselector) == NULL)
 	    continue;
-      
-	  objfile_csym++;
 
 	  if ((type != '\0') && (ntype != type))
 	    continue;

Re: regression: gdb.objc/basicclass.exp

[Jan Kratochvil]

On Thu, 02 Jun 2011 00:22:16 +0200, Keith Seitz wrote:
> This is all way more verbiage than the simple patch to fix it:
> increment the counter before shorting the loop.

since:
commit 1a9561ecd22cbc983e3c981630bcb8565aa8e095
2009-05-13  Paul Pluzhnikov  <ppluzhnikov@google.com>
	* objc-lang.c (objc_objfile_data): New variable.
	(find_methods): Skip objfiles without Obj-C methods.
	(_initialize_objc_lang): New function.

I agree, please check it in.

The only file having objc methods on my box is
/usr/lib/debug/usr/lib64/libobjc.so.3.0.0.debug and parse_method does not fail
on any of its symbols so it does not even make the performance worse (on this
file).


Thanks,
Jan

Re: regression: gdb.objc/basicclass.exp

[Keith Seitz]

On 06/02/2011 02:29 AM, Jan Kratochvil wrote:

> I agree, please check it in.

I've committed this. Thank you for reviewing it.

Keith