gdb.objc/objcdecode.exp test error..

gdb.objc/objcdecode.exp test error..

[Matt Rice]

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

the test is testing the original behaviour which is now 'set
multiple-symbols ask'
rather than the new default behaviour of 'set multiple-symbols all'

this returns the test back to its previous kfail state,
i should probably come up with another test for multiple-symbols all,
so neither failure gets forgotten, but i'll have to learn more about
dejagnu before i'm able to do that.

[-- Attachment #2: foo.diff --]
[-- Type: application/octet-stream, Size: 943 bytes --]

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 50e61ce..7445b87 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2009-03-05  Matt Rice  <ratmice@gmail.com>
+
+	* gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
+	to ask.
+
 2009-03-05  Paul Pluzhnikov  <ppluzhnikov@google.com>
 
 	* solib-display.exp: New file.
diff --git a/gdb/testsuite/gdb.objc/objcdecode.exp b/gdb/testsuite/gdb.objc/objcdecode.exp
index b751fb9..5e83623 100644
--- a/gdb/testsuite/gdb.objc/objcdecode.exp
+++ b/gdb/testsuite/gdb.objc/objcdecode.exp
@@ -60,6 +60,7 @@ do_objc_tests
 # Break on multiply defined method (PR objc/1236)
 #
 set name "break on multiply defined method"
+gdb_test "set multiple-symbols ask" ""
 gdb_test_multiple "break multipleDef" $name \
 {
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] -.Decode multipleDef. at .*\r\n\\\[3\\\] multipleDef at .*\r\n> $" {

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

Matt,

(interesting email address :-)

As far as I can tell, the copyright assignment papers you filed with
the FSF only covers GNUSTEP. We can take this change in as a "tiny
change", but it would be better if you had a proper copyright assignment
at least for GDB (I suggest to assign any past and future changes
to any GNU software and be done with it, but I understand if you prefer
to choose a more limited list of projects). Once you are covered for
GDB, let us know and we can add you to the list of contributors that
have write-after-approval access to the GDB repository.

> i should probably come up with another test for multiple-symbols all,
> so neither failure gets forgotten, but i'll have to learn more about
> dejagnu before i'm able to do that.

That would be very nice if you could add these extra tests.
It makes sense to test the behaviour in the "ask" case, but we really
ought to try to test the default case as well.

It shouldn't be very hard: Try it by hand, and then add a gdb_test
with the expected output.  The expected output regular expression
can be a bit hairy because you have to escape backslashes, but
otherwise, it's fairly simple to program by copy/paste.

> +2009-03-05  Matt Rice  <ratmice@gmail.com>
> +
> +	* gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
> +	to ask.

In the meantime, this looks reasonable to me. I can check it in
for you if you'd like.

-- 
Joel

Re: gdb.objc/objcdecode.exp test error..

[Pedro Alves]

Joel Wrote:
> Matt Rice wrote:
> > i should probably come up with another test for multiple-symbols all,
> > so neither failure gets forgotten, but i'll have to learn more about
> > dejagnu before i'm able to do that.

> That would be very nice if you could add these extra tests.
> It makes sense to test the behaviour in the "ask" case, but we really
> ought to try to test the default case as well.

Indeed.  This test internal-errors for me currently, but the patch masks
it.  I don't see any internal error mentioned in the PRs which
are linked from the kfails?  It looks like a different bug.

 http://sourceware.org/bugzilla/show_bug.cgi?id=8341
 http://sourceware.org/bugzilla/show_bug.cgi?id=8343

(gdb) PASS: gdb.objc/objcdecode.exp: break on multiply defined method
run
Starting program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.objc/objcdecode
../../src/gdb/breakpoint.c:7451: internal-error: breakpoint_re_set_one: Assertion `sals.nelts == 1' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.

-- 
Pedro Alves

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

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

Joel Wrote:
>but it would be better if you had a proper copyright assignment
>at least for GDB (I suggest to assign any past and future changes
>to any GNU software and be done with it, but I understand if you prefer
>to choose a more limited list of projects).

Thanks, I wasn't aware I could assign to all gnu projects,
I've started the paperwork, i'll let you know when its gone through.

>In the meantime, this looks reasonable to me. I can check it in
>for you if you'd like.

it doesn't matter much to me, we might as well just wait and get the
new tests in.

On Fri, Mar 6, 2009 at 11:13 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> Joel Wrote:
>> Matt Rice wrote:
>> > i should probably come up with another test for multiple-symbols all,
>> > so neither failure gets forgotten, but i'll have to learn more about
>> > dejagnu before i'm able to do that.
>
>> That would be very nice if you could add these extra tests.
>> It makes sense to test the behaviour in the "ask" case, but we really
>> ought to try to test the default case as well.
>
> Indeed.  This test internal-errors for me currently, but the patch masks
> it.  I don't see any internal error mentioned in the PRs which
> are linked from the kfails?  It looks like a different bug.
>
>  http://sourceware.org/bugzilla/show_bug.cgi?id=8341
>  http://sourceware.org/bugzilla/show_bug.cgi?id=8343
>
> (gdb) PASS: gdb.objc/objcdecode.exp: break on multiply defined method
> run
> Starting program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.objc/objcdecode
> ../../src/gdb/breakpoint.c:7451: internal-error: breakpoint_re_set_one: Assertion `sals.nelts == 1' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
>

yes this is a different albeit somewhat related bug, (same cause,
different effect),
it doesn't get this far when using 'ask'.

I went ahead and tried to do some new tests, when trying to clean those up
to use gdb_continue_to_breakpoint, I discovered that it works
correctly after we've hit main,
so i added some tests for that since it appears to be touching
different code paths.

below is some more information, on further reducing the test,
I wasn't sure if or how I should incorporate into the test, as i'm not
sure about the reliability of hard coding breakpoint numbers nor a way
to delete a breakpoint by function name,

it probably doesn't matter its already pretty much reduced.

let me know if you guys have any issues with the tests or ideas on
doing things cleaner,
still pretty new to dejagnu and its mostly just modifications of the
original 'ask' test.

Use the "delete" command to delete unwanted breakpoints.
(gdb) info b
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x000000000040072c in -[Decode multipleDef]
                                               at
../.././gdb/testsuite/gdb.objc/objcdecode.m:14
2       breakpoint     keep y   0x0000000000400753 in multipleDef
                                               at
../.././gdb/testsuite/gdb.objc/objcdecode.m:28
(gdb) delete 2
(gdb) r
Starting program: /home/ratmice/git/gdb/gdb/testsuite/gdb.objc/objcdecode
function multipleDef

Breakpoint 1, -[Decode multipleDef] (self=0x604f20, _cmd=0x601450)
    at ../.././gdb/testsuite/gdb.objc/objcdecode.m:14
14	  printf("method multipleDef\n");
(gdb) c
Continuing.
method multipleDef

Program exited normally.
(gdb) delete 1
(gdb) break multipleDef
Breakpoint 3 at 0x40072c: file
../.././gdb/testsuite/gdb.objc/objcdecode.m, line 14.
Breakpoint 4 at 0x400753: file
../.././gdb/testsuite/gdb.objc/objcdecode.m, line 28.
warning: Multiple breakpoints were set.
Use the "delete" command to delete unwanted breakpoints.
(gdb) delete 3
(gdb) r
Starting program: /home/ratmice/git/gdb/gdb/testsuite/gdb.objc/objcdecode
breakpoint.c:7450: internal-error: breakpoint_re_set_one: Assertion
`sals.nelts == 1' failed.

so it only fails on the multipleDef function, not the multipleDef method.

[-- Attachment #2: foo.diff --]
[-- Type: application/octet-stream, Size: 3084 bytes --]

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 50e61ce..4617c4d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2009-03-05  Matt Rice  <ratmice@gmail.com>
+
+	* gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
+	to ask for the original test. Add tests for the multiple symbols
+	found behaviour of all.
+
 2009-03-05  Paul Pluzhnikov  <ppluzhnikov@google.com>
 
 	* solib-display.exp: New file.
diff --git a/gdb/testsuite/gdb.objc/objcdecode.exp b/gdb/testsuite/gdb.objc/objcdecode.exp
index b751fb9..1b77aa6 100644
--- a/gdb/testsuite/gdb.objc/objcdecode.exp
+++ b/gdb/testsuite/gdb.objc/objcdecode.exp
@@ -59,7 +59,8 @@ do_objc_tests
 #
 # Break on multiply defined method (PR objc/1236)
 #
-set name "break on multiply defined method"
+set name "break on multiply defined method using multiple-symbols ask"
+gdb_test "set multiple-symbols ask" ""
 gdb_test_multiple "break multipleDef" $name \
 {
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] -.Decode multipleDef. at .*\r\n\\\[3\\\] multipleDef at .*\r\n> $" {
@@ -70,7 +71,7 @@ gdb_test_multiple "break multipleDef" $name \
     -re ".*$gdb_prompt $"   { kfail "gdb/1236" $name }
 }
 
-set name "continue after break on multiply defined symbol"
+set name "run after setting breakpoints on multiply defined symbol"
 gdb_run_cmd
 gdb_test_multiple "" $name \
 {
@@ -84,3 +85,53 @@ gdb_test_multiple "" $name \
 	# It would be difficult to do any more tests after this.
     }
 }
+
+do_objc_tests
+if ![runto_main] { fail "Can't run to main" }
+
+set name "break on multiply defined symbol with multiple-symbols all after main"
+gdb_test "set multiple-symbols all after main" ""
+gdb_test_multiple "break multipleDef" $name \
+{
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+  -re ".*$gdb_prompt $"   { fail $name }
+}
+
+set name "can hit multiply defined breakpoint on function after main"
+gdb_continue_to_breakpoint "function multipleDef"
+
+set name "can hit multiply defined breakpoint on objc method after main"
+gdb_continue_to_breakpoint "method multipleDef"
+gdb_exit
+
+do_objc_tests
+
+set name "break on multiply defined symbol with multiple-symbols all before main"
+gdb_test_multiple "break multipleDef" $name \
+{
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+  -re ".*$gdb_prompt $"   { fail $name }
+}
+
+set name "can hit multiply defined breakpoint on function before main"
+gdb_run_cmd
+gdb_test_multiple "" $name \
+{
+   -re "Breakpoint \[0-9\]+, multipleDef \\\(\\\) at .*\r\n$gdb_prompt $" {
+	pass $name
+    }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+
+set name "can hit multiply defined breakpoint on function before main"
+gdb_test_multiple "continue" $name \
+{
+   -re "Breakpoint \[0-9\]+, -\[Decode multipleDef\] at .*\r\n$gdb_prompt $" {
+	pass $name
+    }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+
+gdb_exit
+return 0
+

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

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

On Sat, Mar 7, 2009 at 4:07 AM, Matt Rice <ratmice@gmail.com> wrote:

> I went ahead and tried to do some new tests

sorry for replying to myself, after trying to fix the bug,
i found the tests still had failures because of some bad regular
expressions in the test.

attached are new tests, and patches for the bug,
the patches are mutually exclusive they both do the same thing pretty much.
what Daniel said to try here.

http://sourceware.org/ml/gdb-patches/2008-03/msg00306.html

after fixing these bugs it fixes the testcases in question, but.... a
number of other testcases though either cause random passes of
previous failures or failures of previous passes on my machine

i have tested that decode_objc only quotes symbols which match
objective-c methods,
so I don't see how this affects non-objc tests

I think these are related to some issues i've been seeing while trying
to debug gdb with gdb.
After running the inferior in the bottom gdb, the top gdb (the one
running the debugger) gets

warning: Unexpected waitpid result 00117f when waiting for vfork-done
linux-nat.c:1999: internal-error: unknown ptrace event 5
A problem internal to GDB has been detected,
further debugging may prove unreliable.

this is an intermittent error with an unpatched gdb, i can work around
it by doing the same stuff (setting breakpoints running the inferior
in different orders), anyhow I would be grateful if someone could run
these through the testsuite on a system which works.

cheers

[-- Attachment #2: decode_line_2.diff --]
[-- Type: application/octet-stream, Size: 3841 bytes --]

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 6579d42..b43d249 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -105,7 +105,7 @@ static char *find_toplevel_char (char *s, char c);
 static int is_objc_method_format (const char *s);
 
 static struct symtabs_and_lines decode_line_2 (struct symbol *[],
-					       int, int, char ***);
+					       int, int, char ***, int);
 
 static struct symtab *symtab_from_filename (char **argptr,
 					    char *p, int is_quote_enclosed,
@@ -477,11 +477,13 @@ is_objc_method_format (const char *s)
 /* Given a list of NELTS symbols in SYM_ARR, return a list of lines to
    operate on (ask user if necessary).
    If CANONICAL is non-NULL return a corresponding array of mangled names
-   as canonical line specs there.  */
+   as canonical line specs there.  If should_quote_canonical is non-zero
+   it will wrap the canonical in single quotes.
+*/
 
 static struct symtabs_and_lines
 decode_line_2 (struct symbol *sym_arr[], int nelts, int funfirstline,
-	       char ***canonical)
+	       char ***canonical, int should_quote_canonical)
 {
   struct symtabs_and_lines values, return_values;
   char *args, *arg1;
@@ -587,6 +589,16 @@ See set/show multiple-symbol."));
 		  if (canonical_arr[i] == NULL)
 		    {
 		      symname = SYMBOL_LINKAGE_NAME (sym_arr[i]);
+
+		      if (should_quote_canonical)
+			{
+			  char *qsymname;
+			  qsymname = alloca(strlen(symname) + sizeof("''\0"));
+			  strcpy(qsymname, "'");
+			  strcat(qsymname, symname);
+			  strcat(qsymname, "'");
+			  symname = qsymname;
+			}
 		      canonical_arr[i] = savestring (symname, strlen (symname));
 		    }
 		}
@@ -609,9 +621,27 @@ See set/show multiple-symbol."));
 	    {
 	      if (canonical_arr)
 		{
+		  char *the_symname;
+
 		  symname = SYMBOL_LINKAGE_NAME (sym_arr[num]);
+
+		  if (should_quote_canonical)
+		    {
+		      char *qsymname;
+
+		      qsymname = alloca(strlen(symname) + sizeof("''\0"));
+		      strcpy(qsymname, "'");
+		      strcat(qsymname, symname);
+		      strcat(qsymname, "'");
+		      the_symname = qsymname;
+		    }
+		  else
+		    the_symname = symname;
+
+		  // why cleanup? SYMBOL_LINKAGE_NAME doesn't allocate
+		  // anything, so this appears to free one of our args?
 		  make_cleanup (xfree, symname);
-		  canonical_arr[i] = savestring (symname, strlen (symname));
+		  canonical_arr[i] = savestring (the_symname, strlen (the_symname));
 		}
 	      return_values.sals[i++] = values.sals[num];
 	      values.sals[num].pc = 0;
@@ -740,14 +770,14 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
     }
 
   /* Check if the symbol could be an Objective-C selector.  */
-
-  {
-    struct symtabs_and_lines values;
-    values = decode_objc (argptr, funfirstline, NULL,
+  if (!is_quoted)
+    {
+      struct symtabs_and_lines values;
+      values = decode_objc (argptr, funfirstline, NULL,
 			  canonical, saved_arg);
-    if (values.sals != NULL)
-      return values;
-  }
+      if (values.sals != NULL)
+        return values;
+    }
 
   /* Does it look like there actually were two parts?  */
 
@@ -1182,7 +1212,7 @@ decode_objc (char **argptr, int funfirstline, struct symtab *file_symtab,
   if (i1 > 1)
     {
       /* More than one match. The user must choose one or more.  */
-      return decode_line_2 (sym_arr, i2, funfirstline, canonical);
+      return decode_line_2 (sym_arr, i2, funfirstline, canonical, 1);
     }
 
   return values;
@@ -1467,7 +1497,7 @@ find_method (int funfirstline, char ***canonical, char *saved_arg,
     {
       /* 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);
+      return decode_line_2 (sym_arr, i1, funfirstline, canonical, 0);
     }
   else
     {

[-- Attachment #3: decode_objc.diff --]
[-- Type: application/octet-stream, Size: 1504 bytes --]

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 6579d42..2fa030e 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -740,14 +740,14 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
     }
 
   /* Check if the symbol could be an Objective-C selector.  */
-
-  {
-    struct symtabs_and_lines values;
-    values = decode_objc (argptr, funfirstline, NULL,
-			  canonical, saved_arg);
-    if (values.sals != NULL)
-      return values;
-  }
+  if (!is_quoted)
+    {
+      struct symtabs_and_lines values;
+      values = decode_objc (argptr, funfirstline, NULL,
+			    canonical, saved_arg);
+      if (values.sals != NULL)
+        return values;
+    }
 
   /* Does it look like there actually were two parts?  */
 
@@ -1181,8 +1181,24 @@ decode_objc (char **argptr, int funfirstline, struct symtab *file_symtab,
 
   if (i1 > 1)
     {
+      int i;
+
       /* More than one match. The user must choose one or more.  */
-      return decode_line_2 (sym_arr, i2, funfirstline, canonical);
+      values = decode_line_2 (sym_arr, i2, funfirstline, canonical);
+
+      if (!canonical)
+        return values;
+
+      for (i = 0; i < values.nelts; i++)
+        {
+	  char **canonical_arr = *canonical;
+	  char *canonical_name = canonical_arr[i];
+	  char *fq_name = xmalloc(strlen(canonical_name) + sizeof("''\0"));
+
+	  sprintf(fq_name, "'%s'", canonical_name);
+	  canonical_arr[i] = fq_name;
+	  xfree(canonical_name);
+        }
     }
 
   return values;

[-- Attachment #4: tests.diff --]
[-- Type: application/octet-stream, Size: 3106 bytes --]

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 50e61ce..4617c4d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2009-03-05  Matt Rice  <ratmice@gmail.com>
+
+	* gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
+	to ask for the original test. Add tests for the multiple symbols
+	found behaviour of all.
+
 2009-03-05  Paul Pluzhnikov  <ppluzhnikov@google.com>
 
 	* solib-display.exp: New file.
diff --git a/gdb/testsuite/gdb.objc/objcdecode.exp b/gdb/testsuite/gdb.objc/objcdecode.exp
index b751fb9..b2a2d97 100644
--- a/gdb/testsuite/gdb.objc/objcdecode.exp
+++ b/gdb/testsuite/gdb.objc/objcdecode.exp
@@ -59,7 +59,8 @@ do_objc_tests
 #
 # Break on multiply defined method (PR objc/1236)
 #
-set name "break on multiply defined method"
+set name "break on multiply defined method using multiple-symbols ask"
+gdb_test "set multiple-symbols ask" ""
 gdb_test_multiple "break multipleDef" $name \
 {
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] -.Decode multipleDef. at .*\r\n\\\[3\\\] multipleDef at .*\r\n> $" {
@@ -70,7 +71,7 @@ gdb_test_multiple "break multipleDef" $name \
     -re ".*$gdb_prompt $"   { kfail "gdb/1236" $name }
 }
 
-set name "continue after break on multiply defined symbol"
+set name "run after setting breakpoints on multiply defined symbol"
 gdb_run_cmd
 gdb_test_multiple "" $name \
 {
@@ -84,3 +85,54 @@ gdb_test_multiple "" $name \
 	# It would be difficult to do any more tests after this.
     }
 }
+
+do_objc_tests
+if ![runto_main] { fail "Can't run to main" }
+
+set name "break on multiply defined symbol with multiple-symbols all after main"
+gdb_test "set multiple-symbols all after main" ""
+gdb_test_multiple "break multipleDef" $name \
+{
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+  -re ".*$gdb_prompt $"   { fail $name }
+}
+
+set name "can hit multiply defined breakpoint on function after main"
+gdb_continue_to_breakpoint "function multipleDef"
+
+set name "can hit multiply defined breakpoint on objc method after main"
+gdb_continue_to_breakpoint "method multipleDef"
+gdb_exit
+
+do_objc_tests
+
+set name "break on multiply defined symbol with multiple-symbols all before main"
+gdb_test_multiple "break multipleDef" $name \
+{
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+  -re ".*$gdb_prompt $"   { kfail $name }
+}
+
+set name "can hit multiply defined breakpoint on function before main"
+gdb_run_cmd
+gdb_test_multiple "" $name \
+{
+   -re "Breakpoint \[0-9\]+, multipleDef \\\(\\\) at .*\r\n$gdb_prompt $" {
+	pass $name
+    }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+
+set name "can hit multiply defined breakpoint on method before main"
+gdb_test_multiple "continue" $name \
+{
+   -re "Breakpoint \[0-9\]+, -\\\[Decode multipleDef\\\] \\\(.*\\\) at .*\r\n$gdb_prompt $" {
+       pass $name
+    }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+
+
+gdb_exit
+return 0
+

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

On Sun, Mar 8, 2009 at 7:16 AM, Matt Rice <ratmice@gmail.com> wrote:
> On Sat, Mar 7, 2009 at 4:07 AM, Matt Rice <ratmice@gmail.com> wrote:
>
>> I went ahead and tried to do some new tests
>
> sorry for replying to myself, after trying to fix the bug,
> i found the tests still had failures because of some bad regular
> expressions in the test.
>
> attached are new tests, and patches for the bug,
> the patches are mutually exclusive they both do the same thing pretty much.
> what Daniel said to try here.
>
> http://sourceware.org/ml/gdb-patches/2008-03/msg00306.html

well hmm these only partially fix the issue, still trying to figure
out how to compile a shared library
in the testsuite, but if the symbol is a c function, and doesn't match
a objc method,
until a shared library is loaded we get the assertion failure because
they are left unquoted unless they
match a known objective-c symbol, so we'd have to quote all things
which could potentially match objective-c methods,
this increases the scope quite a bit not sure we want to go this direction.....

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

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

On Sun, Mar 8, 2009 at 7:10 PM, Matt Rice <ratmice@gmail.com> wrote:
>
> well hmm these only partially fix the issue, still trying to figure
> out how to compile a shared library
> in the testsuite,

Alright so, i've finally gotten around to figuring that out.
Apologies for my lack of brevity.
Attached are testsuite patches which test this, and an additional way
to provoke the issue
I've currently decided to just focus on the assertion failure which is
evident in the current testsuite
and further explored in my tests, and also asked about by Pierre Muller in [1].

The assert was added at [2], this changed the handling in
breakpoint_re_set_one of the return value of decode_line_1 from being
iterated over to asserting that there is only a single value returned.

to quote the gdb manual at [3]:
It is also possible to specify just a method name:

     break create

end quote.

basically the effect of this is that it is possible to create an
ambiguous expression[5]
which is extrapolated into more ambiguous expressions,

e.g. 'create' becomes 'create'
and also '-[Foo create]'
and both 'create' and '-[Foo create]' can also become '-[Foo(bar) create]'
and it is possible for all 3 to exist at the same time.

As Vladimir mentioned on irc this is somewhat non-conventional, e.g.

The c++ language support doesn't do anything like it, e.g.
break create will not expand to Foo::create(void)
and it seems that we have similar behaviour to this already available in rbreak,
but none the less, the behaviour is documented and works _until_ it
hits ambiguity.
e.g. if 'break create' were to match only '-[Bar create]' and '-[Baz
create]', the 2 resulting breakpoints would not be ambiguous nor
trigger the assertion.

Regardless of the above there is a language feature of Objective-C
with which it is possible to also trigger this assertion,  The feature
is called categories, and they are a way in which to add
methods, or override methods,  This situation seems much more akin to
the situation in which breakpoint_re_set_one was intended to handle.

These are tested via the -bar methods in the added .m files.

after loading the Category in libobjcdecode3.so via dlopen, [obj bar];
will call a new implementation, but the symbol was not overridden via
normal c-style linkage, the symbols are unique, so both method
implementations exist, only the objects dispatch table has been
updated, therefore decode_line_1 returns 2 symbols matching the 'bar'
method.  With the criteria that whichever implementation was loaded
last is the one which is used.

because it is rather common for people to cache function pointers to
methods in order to avoid the overhead involved with the dispatch
table lookups, it is a uncommon but possible source of problems to
have a stale cached implementation, so its a good thing we can set a
breakpoint on all implementations of a specific method..

the patch for [5] at [6], will sort of kinda of fix part of these
issues, i've attached a rehashing of it
it will avoid the assertion failures, but it will also fail to
recognize when a category has been loaded and thus fail the "bar
category implementation" tests, so i suppose its better than the
current behaviour.

While the bug listed, mentions going another way about solving said
bug report, it is my opinion that the current assertion failures
cannot be fixed in such a way, only by either removing the assertion,
or avoiding the ambiguity,  though we could always keep the assertion
and split it into 2 stages or something, an ambiguous stage and a
non-ambiguous stage with the assertion.

but there is some weirdness about re-enabling the iteration, for
instance it seems it would cause them to share conditions, i'm not
sure this is wanted in the method/function case, but it probably is in
the method/category method case.

[1]: http://sourceware.org/ml/gdb-patches/2008-03/msg00473.html
[2]: http://sourceware.org/ml/gdb-patches/2007-09/msg00306.html
[3]: info -f gdb --node='Method Names in Commands'
[4]: info -f gdb --node='Ambiguous Expressions'
[5]: http://sourceware.org/bugzilla/show_bug.cgi?id=8343
[6]: http://sourceware.org/bugzilla/show_bug.cgi?id=8535

P.S. the new tests are somewhat duplicated by the original tests I had
done, and the ones from Adam Fedor. I'm guessing that the requirements
of the new ones will probably work on a more limited set of platforms
i suppose, due to the dlopen requirements.  Let me know if you want me
to roll these all into a single test, or if i should keep the old ones
around.

And with regard to 8343, this seems as good a place as any to discuss it,
given that we have up to 2 layers of ambiguity, maybe we need in the
breakpoint structure to differentiate between breakpoints which were
set by the user, and breakpoints which were derived from breakpoints
which are set by the user.

Then we can only ask the user about symbols which can be derived from
user-specified breakpoints, i'm not sure if this would require a
different return value from decode_line_1 or if this should be handled
in the 'break_command_really'.
that way we could avoid asking about '-[Foo(bar) create]' twice when
re_setting both 'create' and '-[Foo create]'. (when using
multiple-symbols ask), also seems like something that could be used
for something akin to a 'pending rbreak'.

P.P.S: these aren't 7.0 regressions they've been around since at least
6.8 in 7.0 we can ignore the assertions specifying 'N', and go about
our debugging where before we would get stuck in a loop with PR #8343
before we got to the assertions.

2009-09-10  Matt Rice  <ratmice@gmail.com>

        * lib/gdb.exp (gdb_continue_to_breakpoint): Fail on internal-error
        instead of falling back to timeout.
        * gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
        to ask for the original test. Add tests for the multiple symbols
        found behaviour of all.
        * gdb.objc/ambiguous.exp: New tests for ObjC ambiguous expressions
        across shared libraries.
        * gdb.objc/ambiguous.m: Ditto.
        * gdb.objc/objcdecode2.m: Ditto.
        * gdb.objc/objcdecode2.h: Ditto.
        * gdb.objc/objcdecode3.m: Ditto.

[-- Attachment #2: 0001--lib-gdb.exp-gdb_continue_to_breakpoint.patch --]
[-- Type: application/octet-stream, Size: 11242 bytes --]

From 62edd95e58fdd240c6fa85a12a4d3fbc7b1acb00 Mon Sep 17 00:00:00 2001
From: Matt Rice <ratmice@gmail.com>
Date: Fri, 11 Sep 2009 01:34:47 -0700
Subject: [PATCH 1/2]         * lib/gdb.exp (gdb_continue_to_breakpoint): Fail on internal-error
         instead of falling back to timeout.
         * gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
         to ask for the original test. Add tests for the multiple symbols
         found behaviour of all.
         * gdb.objc/ambiguous.exp: New tests for ObjC ambiguous expressions
         across shared libraries.
         * gdb.objc/ambiguous.m: Ditto.
         * gdb.objc/objcdecode2.m: Ditto.
         * gdb.objc/objcdecode2.h: Ditto.
         * gdb.objc/objcdecode3.m: Ditto.

---
 gdb/testsuite/gdb.objc/ambiguous.exp  |  149 +++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.objc/ambiguous.m    |   35 ++++++++
 gdb/testsuite/gdb.objc/objcdecode.exp |   55 ++++++++++++-
 gdb/testsuite/gdb.objc/objcdecode2.h  |    9 ++
 gdb/testsuite/gdb.objc/objcdecode2.m  |   20 +++++
 gdb/testsuite/gdb.objc/objcdecode3.m  |   19 ++++
 gdb/testsuite/lib/gdb.exp             |    5 +
 7 files changed, 290 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.objc/ambiguous.exp
 create mode 100644 gdb/testsuite/gdb.objc/ambiguous.m
 create mode 100644 gdb/testsuite/gdb.objc/objcdecode2.h
 create mode 100644 gdb/testsuite/gdb.objc/objcdecode2.m
 create mode 100644 gdb/testsuite/gdb.objc/objcdecode3.m

diff --git a/gdb/testsuite/gdb.objc/ambiguous.exp b/gdb/testsuite/gdb.objc/ambiguous.exp
new file mode 100644
index 0000000..8c1c0d2
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/ambiguous.exp
@@ -0,0 +1,149 @@
+# Copyright 2009 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/>.
+
+# This file tests breakpoints on ambiguous expressions in C and Obj-C symbols
+# And their affects across shared libraries.
+
+# This file was written by Matt Rice (ratmice@gmail.com)
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+proc do_objc_tests {} {
+    global prms_id
+    global bug_id
+    global subdir
+    global objdir
+    global srcdir
+    global binfile
+    global gdb_prompt
+
+    set prms_id 0
+    set bug_id 0
+
+    # Start with a fresh gdb.
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load $binfile
+
+}
+
+set testfile "objcdecode2"
+set srcfile ${testfile}.m
+set binfile ${objdir}/${subdir}/lib${testfile}.so
+set lib_flags [list debug additional_flags=[list -g -fPIC -I${srcdir}/${subdir}] ]
+
+if {[gdb_compile_shlib "${srcdir}/${subdir}/${srcfile}" "${binfile}" $lib_flags] != "" } {
+  return -1
+}
+
+set testfile "objcdecode3"
+set srcfile ${testfile}.m
+set binfile ${objdir}/${subdir}/lib${testfile}.so
+set decode3_binfile $binfile
+
+if {[gdb_compile_shlib "${srcdir}/${subdir}/${srcfile}" "${binfile}" $lib_flags] != "" } {
+  return -1
+}
+
+
+set testfile "ambiguous"
+set srcfile ${testfile}.m
+set binfile ${objdir}/${subdir}/${testfile}
+set ambig_opts [list debug additional_flags=[list -g
+ -DSHLIB_NAME=\"$decode3_binfile\"
+ -L${objdir}/${subdir} 
+ -Wl,-rpath,${objdir}/${subdir} 
+ -ldl -lobjcdecode2 
+ -I${srcdir}/${subdir}] ]
+
+if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $ambig_opts] != "" } {
+  return -1
+}
+
+do_objc_tests
+
+set name "ambiguous expression after shlib load"
+gdb_test_multiple "break foo" $name \
+{
+    -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+
+gdb_run_cmd
+gdb_test_multiple "" $name \
+{
+    -re "Breakpoint \[0-9\]+, foo \\\(\\\) at .*\r\n$gdb_prompt $" {
+        pass $name
+    }
+    -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] -.Decode foo. at .*\r\n\\\[3\\\] foo at .*\r\n> $" {
+        send_gdb "0\n"
+        kfail "gdb/1238" $name
+        # gdb is in a bad state here.
+        # It would be difficult to do any more tests after this.
+    }
+}
+
+gdb_test_multiple "break foo" $name \
+{
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+  -re ".*$gdb_prompt $"   { kfail $name }
+}
+gdb_continue_to_breakpoint "foo class implementation"
+gdb_exit
+
+do_objc_tests
+
+send_gdb "set breakpoint pending on\n"
+
+set name "setting pending breakpoint"
+gdb_test_multiple "break -\[DecodeShlib bar\]" $name {
+   -re ".*$gdb_prompt $" { pass $name }
+}
+
+set name "multiply defined method via category implementation"
+gdb_test_multiple "run" $name \
+{
+    -re "Breakpoint \[0-9\]+, -\\\[DecodeShlib bar\\\] \\\(.*\\\) at .*\r\n$gdb_prompt $" {
+        pass $name
+    }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+gdb_continue_to_breakpoint "bar category implementation"
+
+gdb_exit
+
+# i'm not sure why this reacts differently than the above tests
+# using pending breakpoints but it does: therefore i've added it, even though they appear quite similar.
+do_objc_tests
+
+if ![runto_main] {
+  fail "unable to run to main"
+}
+
+gdb_test_multiple "break bar" $name {
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" {
+	 pass $name
+   }
+  -re ".*$gdb_prompt $"   { fail $name }
+}
+
+gdb_continue_to_breakpoint "bar class implementation"
+gdb_continue_to_breakpoint "bar category implementation"
+
+return 0
diff --git a/gdb/testsuite/gdb.objc/ambiguous.m b/gdb/testsuite/gdb.objc/ambiguous.m
new file mode 100644
index 0000000..fb59919
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/ambiguous.m
@@ -0,0 +1,35 @@
+#include <objcdecode2.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+void foo()
+{
+  printf("function implementation\n");
+}
+
+int main()
+{
+  void *handle;
+  id obj = [DecodeShlib new];
+
+  foo();
+  [obj foo];
+
+  [obj bar]; /* class implementation */
+
+  handle = dlopen(SHLIB_NAME, RTLD_LAZY);
+  if (!handle)
+    fprintf(stderr, "error calling dlopen: %s\n", dlerror());
+
+  [obj foo]; /* category implementation */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.objc/objcdecode.exp b/gdb/testsuite/gdb.objc/objcdecode.exp
index b751fb9..556aacc 100644
--- a/gdb/testsuite/gdb.objc/objcdecode.exp
+++ b/gdb/testsuite/gdb.objc/objcdecode.exp
@@ -59,7 +59,8 @@ do_objc_tests
 #
 # Break on multiply defined method (PR objc/1236)
 #
-set name "break on multiply defined method"
+set name "break on multiply defined method using multiple-symbols ask"
+gdb_test "set multiple-symbols ask" ""
 gdb_test_multiple "break multipleDef" $name \
 {
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] -.Decode multipleDef. at .*\r\n\\\[3\\\] multipleDef at .*\r\n> $" {
@@ -70,7 +71,7 @@ gdb_test_multiple "break multipleDef" $name \
     -re ".*$gdb_prompt $"   { kfail "gdb/1236" $name }
 }
 
-set name "continue after break on multiply defined symbol"
+set name "run after setting breakpoints on multiply defined symbol"
 gdb_run_cmd
 gdb_test_multiple "" $name \
 {
@@ -84,3 +85,53 @@ gdb_test_multiple "" $name \
 	# It would be difficult to do any more tests after this.
     }
 }
+
+do_objc_tests
+if ![runto_main] { fail "Can't run to main" }
+
+set name "break on multiply defined symbol with multiple-symbols all after main"
+gdb_test "set multiple-symbols all after main" ""
+gdb_test_multiple "break multipleDef" $name \
+{
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+  -re ".*$gdb_prompt $"   { fail $name }
+}
+
+set name "can hit multiply defined breakpoint on function after main"
+gdb_continue_to_breakpoint "function multipleDef"
+
+set name "can hit multiply defined breakpoint on objc method after main"
+gdb_continue_to_breakpoint "method multipleDef"
+gdb_exit
+
+do_objc_tests
+
+set name "break on multiply defined symbol with multiple-symbols all before main"
+gdb_test_multiple "break multipleDef" $name \
+{
+  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
+  -re ".*$gdb_prompt $"   { kfail $name }
+}
+
+set name "can hit multiply defined breakpoint on function before main"
+gdb_run_cmd
+gdb_test_multiple "" $name \
+{
+   -re "Breakpoint \[0-9\]+, multipleDef \\\(\\\) at .*\r\n$gdb_prompt $" {
+	pass $name
+    }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+
+set name "can hit multiply defined breakpoint on method before main"
+gdb_test_multiple "continue" $name \
+{
+   -re "Breakpoint \[0-9\]+, -\\\[Decode multipleDef\\\] \\\(.*\\\) at .*\r\n$gdb_prompt $" {
+       pass $name
+    }
+   -re ".*$gdb_prompt $" { fail $name }
+}
+
+gdb_exit
+return 0
+
diff --git a/gdb/testsuite/gdb.objc/objcdecode2.h b/gdb/testsuite/gdb.objc/objcdecode2.h
new file mode 100644
index 0000000..d9c3bb8
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/objcdecode2.h
@@ -0,0 +1,9 @@
+#include <objc/Object.h>
+
+@interface DecodeShlib: Object
+{
+}
+- foo;
+- bar;
+- (const char *) myDescription;
+@end
diff --git a/gdb/testsuite/gdb.objc/objcdecode2.m b/gdb/testsuite/gdb.objc/objcdecode2.m
new file mode 100644
index 0000000..4860c54
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/objcdecode2.m
@@ -0,0 +1,20 @@
+#include <objcdecode2.h>
+#include <stdio.h>
+
+@implementation DecodeShlib
+
+- foo 
+{
+  printf("foo class implementation\n");
+}
+
+- bar 
+{
+  printf("bar class implementation\n");
+}
+
+- (const char *) myDescription
+{
+  return "DecodeShlib gdb test object";
+}
+@end
diff --git a/gdb/testsuite/gdb.objc/objcdecode3.m b/gdb/testsuite/gdb.objc/objcdecode3.m
new file mode 100644
index 0000000..97f6e4f
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/objcdecode3.m
@@ -0,0 +1,19 @@
+#include <objcdecode2.h>
+#include <stdio.h>
+
+@interface DecodeShlib(CategoryInShlib)
+@end
+
+@implementation DecodeShlib(CategoryInShlib)
+
+- foo 
+{
+  printf("foo category implementation\n");
+}
+
+- bar 
+{
+  printf("bar category implementation\n");
+}
+
+@end
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0c93a73..4f33486 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -474,6 +474,11 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
 	-re ".*$gdb_prompt $" {
 	    fail $full_name
 	}
+        -re ".*A problem internal to GDB has been detected" {
+             fail "$full_name (GDB internal error)"
+             gdb_internal_error_resync
+	}
+
 	timeout { 
 	    fail "$full_name (timeout)"
 	}
-- 
1.6.2.5


[-- Attachment #3: 0002-Add-enable_ambiguous-argument-to-decode_line_1.patch --]
[-- Type: application/octet-stream, Size: 6088 bytes --]

From 5a8bbadf3dc9bf701545c3592911351932810ece Mon Sep 17 00:00:00 2001
From: Matt Rice <ratmice@gmail.com>
Date: Fri, 11 Sep 2009 01:54:50 -0700
Subject: [PATCH 2/2] Add enable_ambiguous argument to decode_line_1.

---
 gdb/breakpoint.c   |   14 +++++++-------
 gdb/cli/cli-cmds.c |    8 ++++----
 gdb/linespec.c     |   17 +++++++++--------
 gdb/linespec.h     |    3 ++-
 gdb/symtab.c       |    2 +-
 gdb/tracepoint.c   |    2 +-
 6 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9f50872..d242d43 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5793,10 +5793,10 @@ parse_breakpoint_sals (char **address,
  		  && ((*address)[1] != '['))))
 	*sals = decode_line_1 (address, 1, default_breakpoint_symtab,
 			       default_breakpoint_line, addr_string, 
-			       not_found_ptr);
+			       not_found_ptr, 1);
       else
 	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0,
-		               addr_string, not_found_ptr);
+		               addr_string, not_found_ptr, 1);
     }
   /* For any SAL that didn't have a canonical string, fill one in. */
   if (sals->nelts > 0 && *addr_string == NULL)
@@ -6697,10 +6697,10 @@ until_break_command (char *arg, int from_tty, int anywhere)
 
   if (default_breakpoint_valid)
     sals = decode_line_1 (&arg, 1, default_breakpoint_symtab,
-			  default_breakpoint_line, (char ***) NULL, NULL);
+			  default_breakpoint_line, (char ***) NULL, NULL, 1);
   else
     sals = decode_line_1 (&arg, 1, (struct symtab *) NULL, 
-			  0, (char ***) NULL, NULL);
+			  0, (char ***) NULL, NULL, 1);
 
   if (sals.nelts != 1)
     error (_("Couldn't get information on specified line."));
@@ -7903,7 +7903,7 @@ breakpoint_re_set_one (void *bint)
       TRY_CATCH (e, RETURN_MASK_ERROR)
 	{
 	  sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL,
-				not_found_ptr);
+				not_found_ptr, 0);
 	}
       if (e.reason < 0)
 	{
@@ -8427,10 +8427,10 @@ decode_line_spec_1 (char *string, int funfirstline)
     sals = decode_line_1 (&string, funfirstline,
 			  default_breakpoint_symtab,
 			  default_breakpoint_line,
-			  (char ***) NULL, NULL);
+			  (char ***) NULL, NULL, 1);
   else
     sals = decode_line_1 (&string, funfirstline,
-			  (struct symtab *) NULL, 0, (char ***) NULL, NULL);
+			  (struct symtab *) NULL, 0, (char ***) NULL, NULL, 1);
   if (*string)
     error (_("Junk at end of line specification: %s"), string);
   return sals;
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index ce7c2a6..1f03305 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -650,7 +650,7 @@ edit_command (char *arg, int from_tty)
       /* Now should only be one argument -- decode it in SAL.  */
 
       arg1 = arg;
-      sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
+      sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0, 1);
 
       if (! sals.nelts)
 	{
@@ -777,7 +777,7 @@ list_command (char *arg, int from_tty)
     dummy_beg = 1;
   else
     {
-      sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
+      sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0, 1);
 
       if (!sals.nelts)
 	return;			/*  C++  */
@@ -810,9 +810,9 @@ list_command (char *arg, int from_tty)
       else
 	{
 	  if (dummy_beg)
-	    sals_end = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
+	    sals_end = decode_line_1 (&arg1, 0, 0, 0, 0, 0, 1);
 	  else
-	    sals_end = decode_line_1 (&arg1, 0, sal.symtab, sal.line, 0, 0);
+	    sals_end = decode_line_1 (&arg1, 0, sal.symtab, sal.line, 0, 0, 1);
 	  if (sals_end.nelts == 0)
 	    return;
 	  if (sals_end.nelts > 1)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 3e943a1..7cb6858 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -679,7 +679,7 @@ See set/show multiple-symbol."));
 
 struct symtabs_and_lines
 decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
-	       int default_line, char ***canonical, int *not_found_ptr)
+	       int default_line, char ***canonical, int *not_found_ptr, int enable_ambiguity)
 {
   char *p;
   char *q;
@@ -738,13 +738,14 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   /* Check if the symbol could be an Objective-C selector.  */
 
-  {
-    struct symtabs_and_lines values;
-    values = decode_objc (argptr, funfirstline, NULL,
-			  canonical, saved_arg);
-    if (values.sals != NULL)
-      return values;
-  }
+  if (enable_ambiguity)
+    {
+       struct symtabs_and_lines values;
+       values = decode_objc (argptr, funfirstline, NULL,
+			     canonical, saved_arg);
+       if (values.sals != NULL)
+         return values;
+    }
 
   /* Does it look like there actually were two parts?  */
 
diff --git a/gdb/linespec.h b/gdb/linespec.h
index 8ac6ea7..d80aee9 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -22,6 +22,7 @@ struct symtab;
 extern struct symtabs_and_lines
 	decode_line_1 (char **argptr, int funfirstline,
 		       struct symtab *default_symtab, int default_line,
-		       char ***canonical, int *not_found_ptr);
+		       char ***canonical, int *not_found_ptr,
+		       int enable_ambiguity);
 
 #endif /* defined (LINESPEC_H) */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8d9d72c..dae40d5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4471,7 +4471,7 @@ decode_line_spec (char *string, int funfirstline)
 
   sals = decode_line_1 (&string, funfirstline,
 			cursal.symtab, cursal.line,
-			(char ***) NULL, NULL);
+			(char ***) NULL, NULL, 1);
 
   if (*string)
     error (_("Junk at end of line specification: %s"), string);
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 0d439ef..c7b1d2d 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1852,7 +1852,7 @@ scope_info (char *args, int from_tty)
   if (args == 0 || *args == 0)
     error (_("requires an argument (function, line or *addr) to define a scope"));
 
-  sals = decode_line_1 (&args, 1, NULL, 0, &canonical, NULL);
+  sals = decode_line_1 (&args, 1, NULL, 0, &canonical, NULL, 1);
   if (sals.nelts == 0)
     return;		/* presumably decode_line_1 has already warned */
 
-- 
1.6.2.5

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

Matt and I had a pretty long discussion on IRC about this, and I think
that the core of the problem is that we don't know how to handle
breakpoints on homonyms just yet. I wonder how things works with C++.
One day, I'll take the time to learn how GDB handles this language...

Anyway, one thing that makes it really really bad is the fact that

    (gdb) break create

can result in numerous breakpoints being set if create just happens
to be the name any ObjC method. And as it turns out, the NSThread
class has a method called "main"...

I really think that allowing the above shortcut is a mis-feature
that we should consider removing. We could possibly think about
introducing another syntax meaning all "create" methods in all
classes, if it really is needed by ObjC developers.

Another, more compromising approach, might be to start searching
ObjC method only when we did not find a symbol with that name.
In other words, do a pure C/C++ match first, and if nothing found,
then try objective-C.  We could conditionalize that to the current
language being objective-c, but I have the feeling that this is
contrary to the spirit of the current code.

Anyway, my short-term proposal is to start by either getting rid
of that feature, or else to reduce it's scope.

This would not help handling the case of breakpoint expressions
leading to more than one location.  For this, once we have determined
all possible matches, we need to be able to store their location in
a way that uniquely identifies them.  Otherwise, we wouldn't be
able to "re_set" each one of them when running the program, or when
a new shared-library is loaded.

This is an issue that's actually not limited to ObjC. We discussed
this earlier for Pascal and Ada.  We don't have a perfect solution,
but we think that this is going to be better than what we do now
(or maybe it's just me :-P).

URLs if this thread:
http://www.sourceware.org/ml/gdb-patches/2008-03/msg00473.html
http://www.sourceware.org/ml/gdb-patches/2008-09/msg00379.html

I would like us to make sure we don't lose the good work done
writing testcases, however, so I will review the latest version,
and hopefully we can commit them soon, even if it means KFAILing
some of the tests until we work on fixing them.

-- 
Joel

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

On Wed, Sep 23, 2009 at 5:53 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> Matt and I had a pretty long discussion on IRC about this, and I think
> that the core of the problem is that we don't know how to handle
> breakpoints on homonyms just yet. I wonder how things works with C++.
> One day, I'll take the time to learn how GDB handles this language...

well, I know overloaded methods are handled by encoding the type

e.g. foo(char), foo(int)
so those are canonical.

> Anyway, one thing that makes it really really bad is the fact that
>
>    (gdb) break create
>
> can result in numerous breakpoints being set if create just happens
> to be the name any ObjC method. And as it turns out, the NSThread
> class has a method called "main"...
>
> I really think that allowing the above shortcut is a mis-feature
> that we should consider removing. We could possibly think about
> introducing another syntax meaning all "create" methods in all
> classes, if it really is needed by ObjC developers.

I agree, I've sent an email to the discuss-gnustep asking for feedback...

the thread is here:
http://thread.gmane.org/gmane.comp.lib.gnustep.general/33219

> Another, more compromising approach, might be to start searching
> ObjC method only when we did not find a symbol with that name.
> In other words, do a pure C/C++ match first, and if nothing found,
> then try objective-C.

I see this little gem in objc-lang.c, so this apparently used to be
the case, not sure why it changed

 * Note that it is possible for a normal (c-style) function to have
 * the same name as an objective c selector.  To prevent the selector
 * from eclipsing the function, we allow the caller (decode_line_1) to
 * search for such a function first, and if it finds one, pass it in
 * to us.  We will then integrate it into the list.  We also search
 * for one here, among the minsyms.


>  We could conditionalize that to the current
> language being objective-c, but I have the feeling that this is
> contrary to the spirit of the current code.
>
> Anyway, my short-term proposal is to start by either getting rid
> of that feature, or else to reduce it's scope.
>
> This would not help handling the case of breakpoint expressions
> leading to more than one location.  For this, once we have determined
> all possible matches, we need to be able to store their location in
> a way that uniquely identifies them.  Otherwise, we wouldn't be
> able to "re_set" each one of them when running the program, or when
> a new shared-library is loaded.

It just occured to me that we could canonicalize these homonyms
using '-[Foo() bar]' to mean method not in a category, and
'-[Foo(categoryName) bar]', that also means extending decode_objc to
accept -[Foo() bar] syntax I'm not sure if it will currently accept
it.

> This is an issue that's actually not limited to ObjC. We discussed
> this earlier for Pascal and Ada.  We don't have a perfect solution,
> but we think that this is going to be better than what we do now
> (or maybe it's just me :-P).
>
> URLs if this thread:
> http://www.sourceware.org/ml/gdb-patches/2008-03/msg00473.html
> http://www.sourceware.org/ml/gdb-patches/2008-09/msg00379.html
>
> I would like us to make sure we don't lose the good work done
> writing testcases, however, so I will review the latest version,
> and hopefully we can commit them soon, even if it means KFAILing
> some of the tests until we work on fixing them.

I'll get working on patches for these short-term things in the morning.

Re: gdb.objc/objcdecode.exp test error..

[Tom Tromey]

>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:

Matt> I agree, I've sent an email to the discuss-gnustep asking for
Matt> feedback...
Matt> the thread is here:
Matt> http://thread.gmane.org/gmane.comp.lib.gnustep.general/33219

That thread pointed out this unreviewed patch:

http://sourceware.org/ml/gdb/2009-03/msg00099.html

I don't know whether this one is still relevant... Kai, is it?

Tom

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

On Thu, Sep 24, 2009 at 11:22 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:
>
> Matt> I agree, I've sent an email to the discuss-gnustep asking for
> Matt> feedback...
> Matt> the thread is here:
> Matt> http://thread.gmane.org/gmane.comp.lib.gnustep.general/33219
>
> That thread pointed out this unreviewed patch:
>
> http://sourceware.org/ml/gdb/2009-03/msg00099.html
>
> I don't know whether this one is still relevant... Kai, is it?
>

Yeah, it appears to be the same assertion which my tests are hitting,

this one goes about it by attempting to allow breakpoint_re_set to
return ambiguous results, and doesn't look like it will work without
debug symbols

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

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

On Thu, Sep 24, 2009 at 1:24 AM, Matt Rice <ratmice@gmail.com> wrote:
> On Wed, Sep 23, 2009 at 5:53 PM, Joel Brobecker <brobecker@adacore.com> wrote:

>> This would not help handling the case of breakpoint expressions
>> leading to more than one location.  For this, once we have determined
>> all possible matches, we need to be able to store their location in
>> a way that uniquely identifies them.  Otherwise, we wouldn't be
>> able to "re_set" each one of them when running the program, or when
>> a new shared-library is loaded.
>
> It just occured to me that we could canonicalize these homonyms
> using '-[Foo() bar]' to mean method not in a category, and
> '-[Foo(categoryName) bar]', that also means extending decode_objc to
> accept -[Foo() bar] syntax I'm not sure if it will currently accept
> it.


ok, so this patch disambiguates the homonyms,

given a method with 2 implementations:
-[Foo bar] and -[Foo(aCategoryName) bar]

going 'break -[Foo bar]' will possibly create up to 2 breakpoints
-[Foo() bar], and -[Foo(aCategoryname) bar]

if we get better homonym support we can still refer to them both as a
single deal, using [Foo bar] syntax, but we can also refer to each
method individually.

I'll get working on the other part now, I see that the ada code uses
obsavestring in this, but i'm really not quite sure what its doing and
if it is possible to do this that in this fashion yet.

2009-09-24  Matt Rice  <ratmice@gmail.com>

        * symtab.c (symbol_natural_name): Call objc_decode_symbol.
        * objc-lang.c (objc_decode_symbol): New function.

[-- Attachment #2: category.diff --]
[-- Type: application/octet-stream, Size: 3916 bytes --]

diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 0e4fb71..b94c0af 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -244,22 +244,20 @@ objc_demangle (const char *mangled, int options)
 	  xfree(demangled);	/* not mangled name */
 	  return NULL;
 	}
-      if (cp[1] == '_') {	/* easy case: no category name     */
-	*cp++ = ' ';		/* replace two '_' with one ' '    */
-	strcpy(cp, mangled + (cp - demangled) + 2);
-      }
-      else {
-	*cp++ = '(';		/* less easy case: category name */
-	cp = strchr(cp, '_');
-	if (!cp)
-	  {
-	    xfree(demangled);	/* not mangled name */
-	    return NULL;
-	  }
-	*cp++ = ')';
-	*cp++ = ' ';		/* overwriting 1st char of method name...  */
-	strcpy(cp, mangled + (cp - demangled));	/* get it back */
-      }
+
+      /* We leave the parentheses when lacking a category name to
+         disambiguate when there is multiple versions of a method.
+         So that break [Foo bar] can return canonical results.  */ 
+      *cp++ = '(';		/* less easy case: category name */
+      cp = strchr(cp, '_');
+      if (!cp)
+	{
+	  xfree(demangled);	/* not mangled name */
+	  return NULL;
+	}
+      *cp++ = ')';
+      *cp++ = ' ';		/* overwriting 1st char of method name...  */
+      strcpy(cp, mangled + (cp - demangled));	/* get it back */
 
       while (*cp && *cp == '_')
 	cp++;			/* skip any initial underbars in method name */
@@ -1702,11 +1700,9 @@ find_implementation_from_class (struct gdbarch *gdbarch,
 	      struct objc_method meth_str;
 	      read_objc_methlist_method (gdbarch, mlist, i, &meth_str);
 
-#if 0
 	      fprintf (stderr, 
 		       "checking method 0x%lx against selector 0x%lx\n", 
 		       meth_str.name, sel);
-#endif
 
 	      if (meth_str.name == sel) 
 		/* FIXME: hppa arch was doing a pointer dereference
@@ -1841,3 +1837,50 @@ _initialize_objc_lang (void)
 {
   objc_objfile_data = register_objfile_data ();
 }
+
+char *
+objc_decode_symbol (const struct general_symbol_info *gsymbol)
+{
+  char ntype = '\0';
+  char *nclass = NULL;
+  char *ncategory = NULL;
+  char *nselector = NULL;
+
+  char **resultp =
+    (char **) &gsymbol->language_specific.cplus_specific.demangled_name;
+
+  if (*resultp == NULL)
+    {
+ 
+      parse_method (gsymbol->name, &ntype, &nclass, &ncategory, &nselector);
+
+      if (ncategory == NULL)
+	ncategory = "\0";
+      if (ntype && nclass && ncategory && nselector) { 
+      size_t newlen;
+      struct objfile *objf;
+
+      newlen = sizeof(ntype) + strlen(nclass) + strlen (ncategory)
+			  + strlen(nselector) + sizeof("[]()");
+
+      *resultp = xmalloc(newlen);
+      sprintf(*resultp, "%c[%s(%s) %s]", ntype, nclass, ncategory, nselector);
+
+//      objf = gsymbol->obj_section->objfile;
+//      *resultp = obsavestring (*resultp, newlen, &objf->objfile_obstack);
+      return *resultp;
+      }
+#if 0
+      if (*resultp == NULL)
+        {
+          char **slot = (char **) htab_find_slot (decoded_names_store,
+                                                  decoded, INSERT);
+          if (*slot == NULL)
+            *slot = xstrdup (decoded);
+
+          *resultp = *slot;
+	}
+#endif
+    }
+  return gsymbol->name;
+}
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8d9d72c..f893155 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -632,9 +632,14 @@ symbol_natural_name (const struct general_symbol_info *gsymbol)
     {
     case language_cplus:
     case language_java:
+      if (gsymbol->language_specific.cplus_specific.demangled_name != NULL)
+	return gsymbol->language_specific.cplus_specific.demangled_name;
+      break;
     case language_objc:
       if (gsymbol->language_specific.cplus_specific.demangled_name != NULL)
 	return gsymbol->language_specific.cplus_specific.demangled_name;
+      else
+	return objc_decode_symbol (gsymbol);
       break;
     case language_ada:
       if (gsymbol->language_specific.cplus_specific.demangled_name != NULL)

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

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

On Thu, Sep 24, 2009 at 3:28 PM, Matt Rice <ratmice@gmail.com> wrote:
> On Thu, Sep 24, 2009 at 1:24 AM, Matt Rice <ratmice@gmail.com> wrote:
>> On Wed, Sep 23, 2009 at 5:53 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>
>>> This would not help handling the case of breakpoint expressions
>>> leading to more than one location.  For this, once we have determined
>>> all possible matches, we need to be able to store their location in
>>> a way that uniquely identifies them.  Otherwise, we wouldn't be
>>> able to "re_set" each one of them when running the program, or when
>>> a new shared-library is loaded.
>>
>> It just occured to me that we could canonicalize these homonyms
>> using '-[Foo() bar]' to mean method not in a category, and
>> '-[Foo(categoryName) bar]', that also means extending decode_objc to
>> accept -[Foo() bar] syntax I'm not sure if it will currently accept
>> it.
>
>
> ok, so this patch disambiguates the homonyms,
>
> given a method with 2 implementations:
> -[Foo bar] and -[Foo(aCategoryName) bar]
>
> going 'break -[Foo bar]' will possibly create up to 2 breakpoints
> -[Foo() bar], and -[Foo(aCategoryname) bar]
>
> if we get better homonym support we can still refer to them both as a
> single deal, using [Foo bar] syntax, but we can also refer to each
> method individually.
>
> I'll get working on the other part now, I see that the ada code uses
> obsavestring in this, but i'm really not quite sure what its doing and
> if it is possible to do this that in this fashion yet.
>

Doh, missing headers, lots of cruft, and missing changelog entries...
itchy trigger finger, sorry.


2009-09-24  Matt Rice  <ratmice@gmail.com>

        * symtab.c (symbol_natural_name): Call objc_decode_symbol.
        * objc-lang.c (objc_decode_symbol): New function.
        (demangle_obj): Leave category parentheses even if there is no
        category name.  Fix for gnu coding standards.
        * objc-lang.h: (objc_decode_symbol): Declare.

[-- Attachment #2: category.diff --]
[-- Type: application/octet-stream, Size: 4284 bytes --]

diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 0e4fb71..fdd0a8c 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -225,7 +225,7 @@ objc_demangle (const char *mangled, int options)
      (mangled[1] == 'i' || mangled[1] == 'c') &&
       mangled[2] == '_')
     {
-      cp = demangled = xmalloc(strlen(mangled) + 2);
+      cp = demangled = xmalloc (strlen (mangled) + 2);
 
       if (mangled[1] == 'i')
 	*cp++ = '-';		/* for instance method */
@@ -233,33 +233,31 @@ objc_demangle (const char *mangled, int options)
 	*cp++ = '+';		/* for class    method */
 
       *cp++ = '[';		/* opening left brace  */
-      strcpy(cp, mangled+3);	/* tack on the rest of the mangled name */
+      strcpy (cp, mangled+3);	/* tack on the rest of the mangled name */
 
       while (*cp && *cp == '_')
 	cp++;			/* skip any initial underbars in class name */
 
-      cp = strchr(cp, '_');
+      cp = strchr (cp, '_');
       if (!cp)	                /* find first non-initial underbar */
 	{
-	  xfree(demangled);	/* not mangled name */
+	  xfree (demangled);	/* not mangled name */
 	  return NULL;
 	}
-      if (cp[1] == '_') {	/* easy case: no category name     */
-	*cp++ = ' ';		/* replace two '_' with one ' '    */
-	strcpy(cp, mangled + (cp - demangled) + 2);
-      }
-      else {
-	*cp++ = '(';		/* less easy case: category name */
-	cp = strchr(cp, '_');
-	if (!cp)
-	  {
-	    xfree(demangled);	/* not mangled name */
-	    return NULL;
-	  }
-	*cp++ = ')';
-	*cp++ = ' ';		/* overwriting 1st char of method name...  */
-	strcpy(cp, mangled + (cp - demangled));	/* get it back */
-      }
+
+      /* We leave the parentheses when lacking a category name to
+         disambiguate when there is multiple versions of a method.
+         So that break [Foo bar] can return canonical results.  */ 
+      *cp++ = '(';		/* less easy case: category name */
+      cp = strchr (cp, '_');
+      if (!cp)
+	{
+	  xfree (demangled);	/* not mangled name */
+	  return NULL;
+	}
+      *cp++ = ')';
+      *cp++ = ' ';		/* overwriting 1st char of method name...  */
+      strcpy (cp, mangled + (cp - demangled));	/* get it back */
 
       while (*cp && *cp == '_')
 	cp++;			/* skip any initial underbars in method name */
@@ -1841,3 +1839,40 @@ _initialize_objc_lang (void)
 {
   objc_objfile_data = register_objfile_data ();
 }
+
+char *
+objc_decode_symbol (const struct general_symbol_info *gsymbol)
+{
+  char ntype = '\0';
+  char *nclass = NULL;
+  char *ncategory = NULL;
+  char *nselector = NULL;
+
+  char **resultp =
+    (char **) &gsymbol->language_specific.cplus_specific.demangled_name;
+
+  if (*resultp == NULL)
+    {
+ 
+      parse_method (gsymbol->name, &ntype, &nclass, &ncategory, &nselector);
+
+      if (ncategory == NULL)
+	ncategory = "\0";
+      if (ntype && nclass && ncategory && nselector)
+	{ 
+	  size_t newlen;
+	  struct objfile *objf;
+
+	  newlen = sizeof (ntype) + strlen (nclass) + strlen (ncategory)
+			  + strlen (nselector) + sizeof ("[]()");
+
+	  *resultp = xmalloc(newlen);
+	  sprintf(*resultp, "%c[%s(%s) %s]", ntype, nclass, ncategory,
+					     nselector);
+
+	  return *resultp;
+        }
+    }
+
+  return gsymbol->name;
+}
diff --git a/gdb/objc-lang.h b/gdb/objc-lang.h
index 0d23467..e5ae036 100644
--- a/gdb/objc-lang.h
+++ b/gdb/objc-lang.h
@@ -58,5 +58,7 @@ extern int end_msglist (void);
 
 struct symbol *lookup_struct_typedef (char *name, struct block *block,
 				      int noerr);
+char *
+objc_decode_symbol (const struct general_symbol_info *gsymbol);
 
 #endif
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8d9d72c..f893155 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -632,9 +632,14 @@ symbol_natural_name (const struct general_symbol_info *gsymbol)
     {
     case language_cplus:
     case language_java:
+      if (gsymbol->language_specific.cplus_specific.demangled_name != NULL)
+	return gsymbol->language_specific.cplus_specific.demangled_name;
+      break;
     case language_objc:
       if (gsymbol->language_specific.cplus_specific.demangled_name != NULL)
 	return gsymbol->language_specific.cplus_specific.demangled_name;
+      else
+	return objc_decode_symbol (gsymbol);
       break;
     case language_ada:
       if (gsymbol->language_specific.cplus_specific.demangled_name != NULL)

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

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

On Wed, Sep 23, 2009 at 5:53 PM, Joel Brobecker <brobecker@adacore.com> wrote:
<snip>

>    (gdb) break create
>
> can result in numerous breakpoints being set if create just happens
> to be the name any ObjC method. And as it turns out, the NSThread
> class has a method called "main"...
>
> I really think that allowing the above shortcut is a mis-feature
> that we should consider removing. We could possibly think about
> introducing another syntax meaning all "create" methods in all
> classes, if it really is needed by ObjC developers.

here is a patch which does this....

the previous patches i'd sent (category.diff) i keep finding failures
that aren't covered by my tests.

so i'll keep working on that and getting more complete test coverage,
I will have to update the tests that I had previously sent

because they tested the various failure modes of this feature mostly,
if we remove it, there are lots of invalid tests in there that need to
be tweaked.

2009-09-24  Matt Rice  <ratmice@gmail.com>

     * linespec.c: (is_objc_method_format): Allow '[' .... ']' formatted
     objc methods.
    (decode_line_1): Don't call decode_objc unless its an objc
    method format.

[-- Attachment #2: disable-misfeature.patch --]
[-- Type: application/octet-stream, Size: 1860 bytes --]

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 52149f0..d53886f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11085,15 +11085,6 @@ enter:
 list +[NSText initialize]
 @end smallexample
 
-In the current version of @value{GDBN}, the plus or minus sign is
-required.  In future versions of @value{GDBN}, the plus or minus
-sign will be optional, but you can use it to narrow the search.  It
-is also possible to specify just a method name:
-
-@smallexample
-break create
-@end smallexample
-
 You must specify the complete method name, including any colons.  If
 your program's source files contain more than one @code{create} method,
 you'll be presented with a numbered list of classes that implement that
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 70e27f7..f4dc4a7 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -466,6 +466,8 @@ is_objc_method_format (const char *s)
   if ((s[0] == ':') && (strchr ("+-", s[1]) != NULL) 
       && (s[2] == '[') && strchr(s, ']'))
     return 1;
+  else if (s[0] == '[' && strchr(s, ']'))
+    return 1; 
   /* Handle arguments that are just SYMBOL.  */
   else if ((strchr ("+-", s[0]) != NULL) && (s[1] == '[') && strchr(s, ']'))
     return 1;
@@ -739,13 +741,14 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   /* Check if the symbol could be an Objective-C selector.  */
 
-  {
-    struct symtabs_and_lines values;
-    values = decode_objc (argptr, funfirstline, NULL,
-			  canonical, saved_arg);
-    if (values.sals != NULL)
-      return values;
-  }
+  if (is_objc_method)
+    {
+      struct symtabs_and_lines values;
+      values = decode_objc (argptr, funfirstline, NULL,
+			    canonical, saved_arg);
+      if (values.sals != NULL)
+        return values;
+    }
 
   /* Does it look like there actually were two parts?  */

Re: gdb.objc/objcdecode.exp test error..

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

[...]

Joel> I really think that allowing the above shortcut is a mis-feature
Joel> that we should consider removing. We could possibly think about
Joel> introducing another syntax meaning all "create" methods in all
Joel> classes, if it really is needed by ObjC developers.

Joel> Another, more compromising approach, might be to start searching
Joel> ObjC method only when we did not find a symbol with that name.
Joel> In other words, do a pure C/C++ match first, and if nothing found,
Joel> then try objective-C.  We could conditionalize that to the current
Joel> language being objective-c, but I have the feeling that this is
Joel> contrary to the spirit of the current code.

I tend to think that the results should depend a bit on the current
language.

If the current language is C, I think we should require full
qualification of the name.  So, "break foo" would just find a function
named "foo", but "break x::y" would look at various C++ functions.

However, we could give languages a method that could short-circuit this
search a bit.  That way something like "break func" in C++ could
consider the current context and do C++-style name lookup.  I think this
would yield a natural result for C++ programmers.

If we did this, then "break create" in C mode would only find the C
function of that name, but the ObjC code would be free to make
"break create" do whatever searching ObjC programmers deem reasonable.

Whether other languages accept all the forms accepted by C is open to
debate.  I think C must be a special case because it is the default
language, and we want users to be able to start gdb and set a breakpoint
without an intervening "set language".

What do you think?

Tom

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

> I tend to think that the results should depend a bit on the current
> language.

I agree.

> If we did this, then "break create" in C mode would only find the C
> function of that name, but the ObjC code would be free to make
> "break create" do whatever searching ObjC programmers deem reasonable.

Just being the devil's advocate:

The issue with this approach, I suspect, is that ObjC developers where
used to be able to do ObjC debugging while in using the "c" language.
It seems to be an excessive convenience at first sight, but I understand
a bit the need for it, because I would imagine that a typical ObjC
program is a mix of C and ObjC, not pure ObjC. So if you switch
languages because you switch frames, sometimes "break foo" will "work"
(when the current language is "objc", and sometimes it won't (when
the language is c/c++, etc).  The definition of "work" is currently
meant to match the current ObjC developer expectation.

I've already said that I think that the current feature is wrong and
should be removed.  But perhaps we can find a compromise where "break
foo" now only matches "foo" when in C language, and extend the C
language support to provide a way of specifying all foo ObjC methods.

More generally speaking, I think it's OK to provide convenience
access to non-C language features, but it should not be at the expense
of the C language support.

-- 
Joel

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

Matt,

> attached are new tests, and patches for the bug,

I'm only going to look at the testcase patch for now, since it appears
that the code patches are not ready yet (they fix your issues, but
at the price of breaking other things, right?). We can work on the
code patches separately (but, if you don't mind, summarize again what
the issues are all about, because i'm having a tough time going through
the scattered emails).

2009-03-05  Matt Rice  <ratmice@gmail.com>

	* gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
	to ask for the original test. Add tests for the multiple symbols
	found behaviour of all.

> +do_objc_tests

I know it's not your fault, but it's definitely a misleading name for
a routine that simple restarts the debugger... Would you mind renaming
it to something more meaningful? I noticed also that a couple of global
declarations are not referenced from that routine, and so can be
removed: objdir and gdb_prompt.

Thanks! :)

> +gdb_test "set multiple-symbols all after main" ""

Uh oh, looks like you merged the command being sent to GDB with
the description of the test :)

> +gdb_test_multiple "break multipleDef" $name \
> +{
> +  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
> +  -re ".*$gdb_prompt $"   { fail $name }
> +}

The following can be simplified into a simple call to gdb_test, no?
Also, please consider adding a call to setup_kfail if we expect
the test to fail with the current sources.  This is true of most if not
all the tests that are you adding.

> +gdb_exit
> +do_objc_tests

The call to gdb_exit before do_objc_tests is unnecessary.

> +gdb_exit
> +return 0

I don't think that this is necessary. All scripts do a little bit of
cleanup before starting the meat of the testcase, so let's not do double
pollution...

-- 
Joel

Re: gdb.objc/objcdecode.exp test error..

[Matt Rice]

On Wed, Sep 23, 2009 at 4:13 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> Matt,
>
>> attached are new tests, and patches for the bug,
>
> I'm only going to look at the testcase patch for now, since it appears
> that the code patches are not ready yet (they fix your issues, but
> at the price of breaking other things, right?). We can work on the
> code patches separately (but, if you don't mind, summarize again what
> the issues are all about, because i'm having a tough time going through
> the scattered emails).

Thats fine

> 2009-03-05  Matt Rice  <ratmice@gmail.com>
>
>        * gdb.objc/objcdecode.exp: Set the multiple symbols found behaviour
>        to ask for the original test. Add tests for the multiple symbols
>        found behaviour of all.
>
>> +do_objc_tests
>
> I know it's not your fault, but it's definitely a misleading name for
> a routine that simple restarts the debugger... Would you mind renaming
> it to something more meaningful? I noticed also that a couple of global
> declarations are not referenced from that routine, and so can be
> removed: objdir and gdb_prompt.
>
> Thanks! :)

np, this is also duplicated in the 2 .exp files,
i should figure out if i can somehow include it from the gdb.objc?

>> +gdb_test "set multiple-symbols all after main" ""
>
> Uh oh, looks like you merged the command being sent to GDB with
> the description of the test :)

oops,

>> +gdb_test_multiple "break multipleDef" $name \
>> +{
>> +  -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
>> +  -re ".*$gdb_prompt $"   { fail $name }
>> +}
>
> The following can be simplified into a simple call to gdb_test, no?
> Also, please consider adding a call to setup_kfail if we expect
> the test to fail with the current sources.  This is true of most if not
> all the tests that are you adding.
>

yeah, I tried to fix all of these in the patch,
there are a couple gdb_test_multiple's left that looks like they need to be.

I also added the setup_kfails, adjusted the existing kfails to use the
bugzilla #.
I will try to remember to go through grep and look for more in testsuite/

well, theres a lot of tests that pass that just involve various setup
for the tests which fail, i think ideally, these would not be tests,
but e.g. if i just do send_gdb
I seem to run into lots of timing issues, i just kfailed the ones that do fail.

ChangeLog is in the git patch header.

> The call to gdb_exit before do_objc_tests is unnecessary.

k

>> +gdb_exit
>> +return 0
>
> I don't think that this is necessary. All scripts do a little bit of
> cleanup before starting the meat of the testcase, so let's not do double
> pollution...

k

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

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

For the record, here is the patch that Matt sent me privately when
he realized he forgot to attach it.

-- 
Joel

[-- Attachment #2: 0001-New-objc-tests.patch --]
[-- Type: text/x-diff, Size: 10207 bytes --]

From ad407eb015bbdde409cbcdf98f116b9ea949644a Mon Sep 17 00:00:00 2001
From: Matt Rice <ratmice@gmail.com>
Date: Wed, 23 Sep 2009 22:48:41 -0700
Subject: [PATCH] New objc tests.

2009-09-23  Matt Rice  <ratmice@gmail.com>

	* gdb.objc/ambiguous.exp: New objc tests for 8343 with shared libs.
	* gdb.objc/ambiguous.m: New objc file.
	* gdb.objc/objcdecode.exp: Additional objc tests for 8343.
	* gdb.objc/objcdecode2.m: New objc file.
	* gdb.objc/objcdecode3.m: New objc file.
	* lib/gdb.exp (gdb_continue_to_breakpoint): Handle internal error.
---
 gdb/testsuite/gdb.objc/ambiguous.exp  |  119 +++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.objc/ambiguous.m    |   35 ++++++++++
 gdb/testsuite/gdb.objc/objcdecode.exp |   43 ++++++++++--
 gdb/testsuite/gdb.objc/objcdecode2.m  |   20 ++++++
 gdb/testsuite/gdb.objc/objcdecode3.m  |   19 +++++
 gdb/testsuite/lib/gdb.exp             |    5 ++
 6 files changed, 233 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.objc/ambiguous.exp
 create mode 100644 gdb/testsuite/gdb.objc/ambiguous.m
 create mode 100644 gdb/testsuite/gdb.objc/objcdecode2.m
 create mode 100644 gdb/testsuite/gdb.objc/objcdecode3.m

diff --git a/gdb/testsuite/gdb.objc/ambiguous.exp b/gdb/testsuite/gdb.objc/ambiguous.exp
new file mode 100644
index 0000000..aefc7f7
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/ambiguous.exp
@@ -0,0 +1,119 @@
+# Copyright 2009 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/>.
+
+# This file tests breakpoints on ambiguous expressions in C and Obj-C symbols
+# And their affects across shared libraries.
+
+# This file was written by Matt Rice (ratmice@gmail.com)
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+proc gdb_reinitialize {} {
+    global prms_id
+    global bug_id
+    global subdir
+    global srcdir
+    global binfile
+
+    set prms_id 0
+    set bug_id 0
+
+    # Start with a fresh gdb.
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load $binfile
+
+}
+
+set testfile "objcdecode2"
+set srcfile ${testfile}.m
+set binfile ${objdir}/${subdir}/lib${testfile}.so
+set lib_flags [list debug additional_flags=[list -g -fPIC -I${srcdir}/${subdir}] ]
+
+if {[gdb_compile_shlib "${srcdir}/${subdir}/${srcfile}" "${binfile}" $lib_flags] != "" } {
+  return -1
+}
+
+set testfile "objcdecode3"
+set srcfile ${testfile}.m
+set binfile ${objdir}/${subdir}/lib${testfile}.so
+set decode3_binfile $binfile
+
+if {[gdb_compile_shlib "${srcdir}/${subdir}/${srcfile}" "${binfile}" $lib_flags] != "" } {
+  return -1
+}
+
+
+set testfile "ambiguous"
+set srcfile ${testfile}.m
+set binfile ${objdir}/${subdir}/${testfile}
+set ambig_opts [list debug additional_flags=[list -g \
+ -DSHLIB_NAME=\"$decode3_binfile\" \
+ -L${objdir}/${subdir} \
+ -Wl,-rpath,${objdir}/${subdir} \
+ -ldl -lobjcdecode2 \
+ -I${srcdir}/${subdir}] ]
+
+if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${ambig_opts}] != "" } {
+  return -1
+}
+
+gdb_reinitialize
+
+set name "setup ambiguous expression"
+gdb_test "break foo" "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*" $name
+
+gdb_run_cmd
+set name "loading shlib with breakpoint on ambiguous expr"
+setup_kfail "*-*-*" gdb/8343
+gdb_test "" "Breakpoint \[0-9\]+, foo \\\(\\\) at .*" $name
+
+set name "breakpoint on ambiguous objc method after shlib" 
+gdb_test "break foo" "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*" $name
+
+gdb_continue_to_breakpoint "foo class implementation"
+
+gdb_reinitialize
+
+gdb_test "set breakpoint pending on" "" 
+gdb_test "break -\[DecodeShlib bar\]" ".*not defined.*"
+
+gdb_test "run" \
+"Breakpoint \[0-9\]+, -\\\[DecodeShlib bar\\\] \\\(.*\\\) at .*" \
+"break on bar class implementation"
+
+setup_kfail "*-*-*" gdb/8343
+gdb_continue_to_breakpoint "bar category implementation"
+
+# i'm not sure why this reacts differently than the above tests
+# using pending breakpoints but it does: therefore i've added it
+# even though they appear quite similar.
+gdb_reinitialize
+
+if ![runto_main] {
+  fail "unable to run to main"
+}
+
+gdb_test "break bar" "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*" $name
+
+gdb_continue_to_breakpoint "bar class implementation"
+setup_kfail "*-*-*" gdb/8343
+gdb_continue_to_breakpoint "bar category implementation"
+
+return 0
diff --git a/gdb/testsuite/gdb.objc/ambiguous.m b/gdb/testsuite/gdb.objc/ambiguous.m
new file mode 100644
index 0000000..fb59919
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/ambiguous.m
@@ -0,0 +1,35 @@
+#include <objcdecode2.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+void foo()
+{
+  printf("function implementation\n");
+}
+
+int main()
+{
+  void *handle;
+  id obj = [DecodeShlib new];
+
+  foo();
+  [obj foo];
+
+  [obj bar]; /* class implementation */
+
+  handle = dlopen(SHLIB_NAME, RTLD_LAZY);
+  if (!handle)
+    fprintf(stderr, "error calling dlopen: %s\n", dlerror());
+
+  [obj foo]; /* category implementation */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.objc/objcdecode.exp b/gdb/testsuite/gdb.objc/objcdecode.exp
index b751fb9..317f050 100644
--- a/gdb/testsuite/gdb.objc/objcdecode.exp
+++ b/gdb/testsuite/gdb.objc/objcdecode.exp
@@ -33,14 +33,12 @@ if {[gdb_compile_objc "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [
   return -1
 }
 
-proc do_objc_tests {} {
+proc gdb_reinitialize {} {
     global prms_id
     global bug_id
     global subdir
-    global objdir
     global srcdir
     global binfile
-    global gdb_prompt
 
     set prms_id 0
     set bug_id 0
@@ -54,12 +52,13 @@ proc do_objc_tests {} {
 
 }
 
-do_objc_tests
+gdb_reinitialize
 
 #
 # Break on multiply defined method (PR objc/1236)
 #
-set name "break on multiply defined method"
+set name "break on multiply defined method using multiple-symbols ask"
+gdb_test "set multiple-symbols ask" ""
 gdb_test_multiple "break multipleDef" $name \
 {
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] -.Decode multipleDef. at .*\r\n\\\[3\\\] multipleDef at .*\r\n> $" {
@@ -67,10 +66,10 @@ gdb_test_multiple "break multipleDef" $name \
 	exp_continue
     }
     -re "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\n$gdb_prompt $" { pass $name }
-    -re ".*$gdb_prompt $"   { kfail "gdb/1236" $name }
+    -re ".*$gdb_prompt $"   { kfail "gdb/8343" $name }
 }
 
-set name "continue after break on multiply defined symbol"
+set name "run after setting breakpoints on multiply defined symbol"
 gdb_run_cmd
 gdb_test_multiple "" $name \
 {
@@ -79,8 +78,36 @@ gdb_test_multiple "" $name \
     }
     -re "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] -.Decode multipleDef. at .*\r\n\\\[3\\\] multipleDef at .*\r\n> $" {
 	send_gdb "0\n"
-	kfail "gdb/1238" $name
+	kfail "gdb/8343" $name
 	# gdb is in a bad state here.
 	# It would be difficult to do any more tests after this.
     }
 }
+
+gdb_reinitialize
+if ![runto_main] { fail "Can't run to main" }
+
+gdb_test "set multiple-symbols all" ".*" "setup multiple-symbols all"
+gdb_test "break multipleDef" \
+"Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*" \
+"break on multiply defined symbol with multiple-symbols all after main"
+
+set name "can hit multiply defined breakpoint on function after main"
+gdb_continue_to_breakpoint "function multipleDef"
+
+set name "can hit multiply defined breakpoint on objc method after main"
+gdb_continue_to_breakpoint "method multipleDef"
+
+gdb_reinitialize
+
+gdb_test "break multipleDef" \
+"Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*\r\nBreakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*" \
+"break on multiply defined symbol with multiple-symbols all before main"
+
+setup_kfail "*-*-*" gdb/8343
+gdb_run_cmd
+gdb_test "" \
+"Breakpoint \[0-9\]+, multipleDef \\\(\\\) at .*" \
+"can hit multiply defined breakpoint on function before main"
+
+gdb_continue_to_breakpoint "can hit multiply defined breakpoint on method before main"
diff --git a/gdb/testsuite/gdb.objc/objcdecode2.m b/gdb/testsuite/gdb.objc/objcdecode2.m
new file mode 100644
index 0000000..4860c54
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/objcdecode2.m
@@ -0,0 +1,20 @@
+#include <objcdecode2.h>
+#include <stdio.h>
+
+@implementation DecodeShlib
+
+- foo 
+{
+  printf("foo class implementation\n");
+}
+
+- bar 
+{
+  printf("bar class implementation\n");
+}
+
+- (const char *) myDescription
+{
+  return "DecodeShlib gdb test object";
+}
+@end
diff --git a/gdb/testsuite/gdb.objc/objcdecode3.m b/gdb/testsuite/gdb.objc/objcdecode3.m
new file mode 100644
index 0000000..97f6e4f
--- /dev/null
+++ b/gdb/testsuite/gdb.objc/objcdecode3.m
@@ -0,0 +1,19 @@
+#include <objcdecode2.h>
+#include <stdio.h>
+
+@interface DecodeShlib(CategoryInShlib)
+@end
+
+@implementation DecodeShlib(CategoryInShlib)
+
+- foo 
+{
+  printf("foo category implementation\n");
+}
+
+- bar 
+{
+  printf("bar category implementation\n");
+}
+
+@end
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0c93a73..4f33486 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -474,6 +474,11 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
 	-re ".*$gdb_prompt $" {
 	    fail $full_name
 	}
+        -re ".*A problem internal to GDB has been detected" {
+             fail "$full_name (GDB internal error)"
+             gdb_internal_error_resync
+	}
+
 	timeout { 
 	    fail "$full_name (timeout)"
 	}
-- 
1.6.2.5

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

> 2009-09-23  Matt Rice  <ratmice@gmail.com>
> 
> 	* gdb.objc/ambiguous.exp: New objc tests for 8343 with shared libs.
> 	* gdb.objc/ambiguous.m: New objc file.
>       * gdb.objc/objcdecode2.m: New objc file.
>       * gdb.objc/objcdecode3.m: New objc file.

I'm going to treat this testcase independently from the rest as well...
The objc files are missing a copyright header. Can you add one, please?

> set lib_flags [list debug additional_flags=[list -g -fPIC -I${srcdir}/${subdir}] ]

The additional flags that you are adding are a little suspicious.
gdb_compile_shlib is already supposed to handle that, I think.
For use the -g is covered by the "debug" flag. -fPIC seems to be
already taken care of through the addition of the -fpic flag, which
is very similar (apparently, if I understand the GCC doc correctly,
-fPIC just tries to avoid system limitations and -fpic doesn't).
I'm also surprised that the -I[...] option is needed at all.
We have C files in gdb.base that #include .h files, and yet we don't
seem to need to explicitly provide a -I flag. See macscp.exp for one
example of that, where macscp1.c includes macscp3.h.  The problem with
providing the -I switch is that it's specific to GCC and may not work
with other compilers.

> set binfile ${objdir}/${subdir}/${testfile}
> set ambig_opts [list debug additional_flags=[list -g \
>  -DSHLIB_NAME=\"$decode3_binfile\" \
>  -L${objdir}/${subdir} \
>  -Wl,-rpath,${objdir}/${subdir} \
>  -ldl -lobjcdecode2 \
>  -I${srcdir}/${subdir}] ]

Same here. -g should not be necessary. Neither should be the -I.
Neither should be the various -L/-Wl options... Or is there something
special for SOs with ObjC code?

> set name "setup ambiguous expression"
> gdb_test "break foo" "Breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*" $name

When using gdb_test, it's fine to use a variable, but it is not
necessary.  The use of a variable with gdb_test_multiple is just
because it avoids duplicating the test name.

> gdb_test "run" \
> "Breakpoint \[0-9\]+, -\\\[DecodeShlib bar\\\] \\\(.*\\\) at .*" \
> "break on bar class implementation"

You should use "gdb_run_cmd" in this case. Otherwise, it would not work
in a cross environement for instance.

> # i'm not sure why this reacts differently than the above tests

Just a nit: I should be capitalized. But thank you very much for adding
a comment explaining why you've added the test.  These are really helpful.

> return 0

This is unnecessary.

-- 
Joel

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

> 	* gdb.objc/objcdecode.exp: Additional objc tests for 8343.

And finally, review of the changes for this testcase.

> +gdb_test "set multiple-symbols all" ".*" "setup multiple-symbols all"

The .* is unnecessary, gdb_test already does that for you.  But harmless.
Still, I'd remove it, though, since we expect no output at all.

> +set name "can hit multiply defined breakpoint on function after main"
> +gdb_continue_to_breakpoint "function multipleDef"
> +set name "can hit multiply defined breakpoint on objc method after main"
> +gdb_continue_to_breakpoint "method multipleDef"

name seems unusued in these two cases? Or did you mean to use it in
the call to gdb_continue_to_breakpoint?

> +setup_kfail "*-*-*" gdb/8343
> +gdb_run_cmd
> +gdb_test "" \
> +"Breakpoint \[0-9\]+, multipleDef \\\(\\\) at .*" \
> +"can hit multiply defined breakpoint on function before main"

I would put the setup_kfail after the gdb_run_cmd, if you don't mind.
That way, it's clear that the kfail is for the gdb_test, not the
gdb_run_cmd.

Pre-approved with the changes requested.

A big thank you for providing the testcases. I'm sure they will help
lots in the future when we actually tackle this issue.

-- 
Joel

Re: gdb.objc/objcdecode.exp test error..

[Joel Brobecker]

> np, this is also duplicated in the 2 .exp files,
> i should figure out if i can somehow include it from the gdb.objc?

I'd say nah, let's keep it kiss for now.

> well, theres a lot of tests that pass that just involve various setup
> for the tests which fail, i think ideally, these would not be tests,
> but e.g. if i just do send_gdb I seem to run into lots of timing
> issues, i just kfailed the ones that do fail.

It's fine for them to be tests, even if they are very simple minded
and are not directly related to the issue we're trying to test. They
are still part of the scenario, and if something goes wrong, then we
get a FAIL. send_gdb should never ever be used in a testcase except
under peculiar conditions that gdb_test_multiple does not handle
already.  The send_gdb thing is not all that evil, it's just that
people tend to follow it up with an expect_gdb, which is usually
incomplete, and always a maintenance issue.

We can handle the change to lib/gdb.exp independently of the rest.

@@ -474,6 +474,11 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
 	-re ".*$gdb_prompt $" {
 	    fail $full_name
 	}
+        -re ".*A problem internal to GDB has been detected" {
+             fail "$full_name (GDB internal error)"
+             gdb_internal_error_resync
+	}
+
 	timeout { 
 	    fail "$full_name (timeout)"
 	}

This is a perfect example of why send_gdb/expect_gdb is usually
a bad idea. This should be replaced by gdb_test_multiple. The whole
implementation can then be replaced by:

    gdb_test_mutiple "continue" $full_name {
        -re "Breakpoint .* (at|in) $location_pattern\r\n$gdb_prompt $" {
            pass $full_name
        }
    }

It could also possibly be replaced by a simple call to gdb_test, but
I think that gdb_test has a side-effect of appending a wildcard match
at the end of the given regexp. So it would transform $location_pattern
into $location_pattern followed by ".*".

-- 
Joel