RE: [Patch] Cannot set pending bp if condition set explicitly

[Marc Khouzam <marc.khouzam@ericsson.com>]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com] 
> Sent: Wednesday, July 25, 2012 11:28 AM
> To: Marc Khouzam
> Cc: 'gdb-patches@sourceware.org'
> Subject: Re: [Patch] Cannot set pending bp if condition set explicitly
> 
> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
> 
> Marc> If I set a condition explicitly on a pending breakpoint (using
> Marc> the 'condition' command), the breakpoint fails to install.
> 
> Marc> +      /* For a pending breakoint, the condition is not 
> parsed yet */
> 
> Comment should end with a period and two spaces.

Fixed, along with "breakoint" typo.

> Marc> +      if (b->loc == NULL || b->loc->shlib_disabled)
> 
> I think this ought to use all_locations_are_pending.
> But, I am not 100% sure of this.

I took the original line from print_one_exception_catchpoint()
when it figures out to print <PENDING>.

But I changed it to use all_locations_are_pending() in the below 
patch, and it also solves the problem I was seeing.

> A test case for this would be quite nice to have.

I modified gdb.base/pending.exp to test this case. The test fails
before the patch and passes after.  You'll note that I removed the 
two tests tha were disabling bp 7 and 5.  There were no bp 7 or 5 in
the original test, and my changes added bp 5 which needed to stay 
enabled.

How does it look for HEAD and 7_5 (not the tests
for 7_5 I gather)?

Thanks

Marc

2012-07-25  Marc Khouzam  <marc.khouzam@ericsson.com>

	* breakpoint.c: Add declaration of all_locations_are_pending.
	(set_breakpoint_condition): For pending breakpoints, mark 
	condition as not parsed.

2012-07-25  Marc Khouzam  <marc.khouzam@ericsson.com>

	* gdb.base/pendshr.c: Extra line to set a new breakpoint.
	* gdb.base/pending.exp: Set an enabled pending breakpoint
	with an explicit condition.


### Eclipse Workspace Patch 1.0
#P src
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.695
diff -u -r1.695 breakpoint.c
--- gdb/breakpoint.c    24 Jul 2012 17:37:56 -0000      1.695
+++ gdb/breakpoint.c    25 Jul 2012 20:15:20 -0000
@@ -230,6 +230,8 @@
 
 static void tcatch_command (char *arg, int from_tty);
 
+static int all_locations_are_pending (struct bp_location *loc);
+
 static void detach_single_step_breakpoints (void);
 
 static int single_step_breakpoint_inserted_here_p (struct address_space *,
@@ -951,7 +953,12 @@
       /* I don't know if it matters whether this is the string the user
         typed in or the decompiled expression.  */
       b->cond_string = xstrdup (arg);
-      b->condition_not_parsed = 0;
+
+      /* For a pending breakpoint, the condition is not parsed yet.  */
+      if (all_locations_are_pending (b->loc))
+       b->condition_not_parsed = 1;
+      else
+       b->condition_not_parsed = 0;
 
       if (is_watchpoint (b))
        {
Index: gdb/testsuite/gdb.base/pending.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/pending.exp,v
retrieving revision 1.26
diff -u -r1.26 pending.exp
--- gdb/testsuite/gdb.base/pending.exp  21 Jun 2012 20:46:22 -0000      1.26
+++ gdb/testsuite/gdb.base/pending.exp  25 Jul 2012 20:15:20 -0000
@@ -209,6 +209,32 @@
 "multiple pending breakpoints 2"
 
 #
+# Try a pending break with an explicit condition which is enabled at startup
+#
+
+set bp5_loc [gdb_get_line_number "y++" ${libfile}.c]
+gdb_test_multiple "break pendshr.c:$bp5_loc" "Set pending breakpoint 4" {
+     -re ".*Make breakpoint pending.*y or \\\[n\\\]. $" {
+           gdb_test "y" "Breakpoint.*pendshr.c:$bp5_loc.*pending." \
+               "Set pending breakpoint 5"
+     }
+}
+
+gdb_test_no_output "condition 5 k == 1"
+
+gdb_test "info break" \
+    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+breakpoint     keep n.*PENDING.*pendfunc1.*
+\[\t \]+stop only if k == 1.*
+\[\t \]+print k.*
+\[0-9\]+\[\t \]+breakpoint     keep y.* in main at .*$srcfile:$mainline.*
+\[0-9\]+\[\t \]+breakpoint     keep y.*PENDING.*pendshr.c:$bp2_loc if x > 3.*
+\[0-9\]+\[\t \]+breakpoint     keep y.*PENDING.*pendshr.c:$bp3_loc.*ignore next 2 hits.*
+\[0-9\]+\[\t \]+breakpoint     keep y.*PENDING.*pendshr.c:$bp5_loc.*
+\[\t \]+stop only if k == 1.*" \
+"multiple pending breakpoints 3"
+
+#
 # Run to main which should resolve a pending breakpoint
 #
 
@@ -218,6 +244,24 @@
 "running to main"
 
 #
+# Verify that all pending breakpoints have resolved.
+#
+gdb_test "info break" \
+    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+breakpoint     keep n.* in pendfunc1 at .*
+\[\t \]+stop only if k == 1.*
+\[\t \]+print k.*
+\[0-9\]+\[\t \]+breakpoint     keep y.* in main at .*$srcfile:$mainline.*
+\[\t \]+breakpoint already hit 1 time.*
+\[0-9\]+\[\t \]+breakpoint     keep y.* in pendfunc1 at .*pendshr.c:$bp2_loc.*
+\[\t \]+stop only if x > 3.*
+\[0-9\]+\[\t \]+breakpoint     keep y.* in pendfunc1 at .*pendshr.c:$bp3_loc.*
+\[\t \]+ignore next 2 hits.*
+\[0-9\]+\[\t \]+breakpoint     keep y.* in pendfunc1 at .*pendshr.c:$bp5_loc.*
+\[\t \]+stop only if k == 1.*" \
+"multiple pending breakpoints 4"
+
+#
 # Re-enable the first pending breakpoint which should resolve
 #
 
@@ -237,19 +281,14 @@
 \[$\]1 = 1." \
 "continue to resolved breakpoint 1"
 
-#
-# Disable the other two breakpoints, and continue to the one with
-# the ignore count.  Make sure you hit it the third time, x should
-# be 3 then.
-#
-
-gdb_test "disable 7" "" "Disable other breakpoints"
-gdb_test "disable 5" "" "Disable other breakpoints"
-
 gdb_test "continue" \
         ".*Breakpoint.*pendfunc1.*\\\(x=3\\\) at.*pendshr.c:$bp3_loc.*printf.*;" \
 "continue to resolved breakpoint 3"
 
+gdb_test "continue" \
+        ".*Breakpoint.*pendfunc1.*\\\(x=3\\\) at.*pendshr.c:$bp5_loc.*y\\+\\+;" \
+"continue to resolved breakpoint 5"
+
 delete_breakpoints
 
 gdb_breakpoint "main"
Index: gdb/testsuite/gdb.base/pendshr.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/pendshr.c,v
retrieving revision 1.10
diff -u -r1.10 pendshr.c
--- gdb/testsuite/gdb.base/pendshr.c    4 Jan 2012 08:17:46 -0000       1.10
+++ gdb/testsuite/gdb.base/pendshr.c    25 Jul 2012 20:15:20 -0000
@@ -21,6 +21,7 @@
 {
   int y = x + 4;
   printf ("in pendfunc1, x is %d\n", x);
+  y++;
 }
 
 void pendfunc (int x)