[patch] Fix a regression by me on breakpoint-cond-infcall

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

GDB has now a regression since:
	Re: [patch] Performance optimize large bp_location count
	http://sourceware.org/ml/gdb-patches/2009-10/msg00632.html
	=
	2009-10-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
	Performance optimize large bp_location count.

on breakpoints with conditions calling inferior.

Bringing the code back to the state before my acceleration patch.

The code before already assumed no breakpoints or their bp_locations can
change across the inferior call which should be true - trying to do some:
	break a if b()
	break b
	command 1
	delete 2
	end
or similar cannot work as inside "if b()" evaluation no breakpoints can be
added or removed.

update_global_location_list also does not removed/add `struct bp_location's
themselves but only pointers to them in the bp_location array.  As the new
iteration no longer uses the bp_location array it is no longer a problem.

Original problem was found by and fixed differently by Phil Muldoon.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.


Thanks,
Jan


gdb/
2009-12-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* breakpoint.c (bpstat_stop_status): Iterate using ALL_BREAKPOINTS and
	the B->LOC list.  Remove gdb_assert on B.  Change bp_hardware_watchpoint
	continue to break.  Remove variable update_locations.  Remove HIT_COUNT
	increment protection by an ENABLE_STATE check.  Inline the delayed
	update_global_location_list call.

gdb/testsuite/
2009-12-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.base/condbreak.exp: Put breakpoint on marker3 and marker4.
	(bp_location13, bp_location14, bp_location17, bp_location18)
	(marker3_proto, marker4_proto): New variables.
	(breakpoint info): Update output.
	(run until breakpoint at marker3, run until breakpoint at marker4): New
	tests.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3549,80 +3549,81 @@ bpstat_stop_status (struct address_space *aspace,
   /* Pointer to the last thing in the chain currently.  */
   bpstat bs = root_bs;
   int ix;
-  int need_remove_insert, update_locations = 0;
+  int need_remove_insert;
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
-  {
-    b = bl->owner;
-    gdb_assert (b);
-    if (!breakpoint_enabled (b) && b->enable_state != bp_permanent)
-      continue;
+  /* ALL_BP_LOCATIONS iteration would break across
+     update_global_location_list possibly executed by
+     bpstat_check_breakpoint_conditions's inferior call.  */
 
-    /* For hardware watchpoints, we look only at the first location.
-       The watchpoint_check function will work on entire expression,
-       not the individual locations.  For read watchopints, the
-       watchpoints_triggered function have checked all locations
-       already.  */
-    if (b->type == bp_hardware_watchpoint && bl != b->loc)
-      continue;
+  ALL_BREAKPOINTS (b)
+    {
+      if (!breakpoint_enabled (b) && b->enable_state != bp_permanent)
+	continue;
 
-    if (!bpstat_check_location (bl, aspace, bp_addr))
-      continue;
+      for (bl = b->loc; bl != NULL; bl = bl->next)
+	{
+	  /* For hardware watchpoints, we look only at the first location.
+	     The watchpoint_check function will work on entire expression,
+	     not the individual locations.  For read watchopints, the
+	     watchpoints_triggered function have checked all locations
+	     already.  */
+	  if (b->type == bp_hardware_watchpoint && bl != b->loc)
+	    break;
 
-    /* Come here if it's a watchpoint, or if the break address matches */
+	  if (!bpstat_check_location (bl, aspace, bp_addr))
+	    continue;
 
-    bs = bpstat_alloc (bl, bs);	/* Alloc a bpstat to explain stop */
+	  /* Come here if it's a watchpoint, or if the break address matches */
 
-    /* Assume we stop.  Should we find watchpoint that is not actually
-       triggered, or if condition of breakpoint is false, we'll reset
-       'stop' to 0.  */
-    bs->stop = 1;
-    bs->print = 1;
+	  bs = bpstat_alloc (bl, bs);	/* Alloc a bpstat to explain stop */
 
-    bpstat_check_watchpoint (bs);
-    if (!bs->stop)
-      continue;
+	  /* Assume we stop.  Should we find watchpoint that is not actually
+	     triggered, or if condition of breakpoint is false, we'll reset
+	     'stop' to 0.  */
+	  bs->stop = 1;
+	  bs->print = 1;
 
-    if (b->type == bp_thread_event || b->type == bp_overlay_event
-	|| b->type == bp_longjmp_master)
-      /* We do not stop for these.  */
-      bs->stop = 0;
-    else
-      bpstat_check_breakpoint_conditions (bs, ptid);
-  
-    if (bs->stop)
-      {
-	if (b->enable_state != bp_disabled)
-	  ++(b->hit_count);
+	  bpstat_check_watchpoint (bs);
+	  if (!bs->stop)
+	    continue;
 
-	/* We will stop here */
-	if (b->disposition == disp_disable)
-	  {
-	    if (b->enable_state != bp_permanent)
-	      b->enable_state = bp_disabled;
-	    update_locations = 1;
-	  }
-	if (b->silent)
-	  bs->print = 0;
-	bs->commands = b->commands;
-	if (bs->commands
-	    && (strcmp ("silent", bs->commands->line) == 0
-		|| (xdb_commands && strcmp ("Q", bs->commands->line) == 0)))
-	  {
-	    bs->commands = bs->commands->next;
-	    bs->print = 0;
-	  }
-	bs->commands = copy_command_lines (bs->commands);
-      }
+	  if (b->type == bp_thread_event || b->type == bp_overlay_event
+	      || b->type == bp_longjmp_master)
+	    /* We do not stop for these.  */
+	    bs->stop = 0;
+	  else
+	    bpstat_check_breakpoint_conditions (bs, ptid);
+	
+	  if (bs->stop)
+	    {
+	      ++(b->hit_count);
 
-    /* Print nothing for this entry if we dont stop or if we dont print.  */
-    if (bs->stop == 0 || bs->print == 0)
-      bs->print_it = print_it_noop;
-  }
+	      /* We will stop here */
+	      if (b->disposition == disp_disable)
+		{
+		  if (b->enable_state != bp_permanent)
+		    b->enable_state = bp_disabled;
+		  update_global_location_list (0);
+		}
+	      if (b->silent)
+		bs->print = 0;
+	      bs->commands = b->commands;
+	      if (bs->commands
+		  && (strcmp ("silent", bs->commands->line) == 0
+		      || (xdb_commands && strcmp ("Q",
+						  bs->commands->line) == 0)))
+		{
+		  bs->commands = bs->commands->next;
+		  bs->print = 0;
+		}
+	      bs->commands = copy_command_lines (bs->commands);
+	    }
 
-  /* Delay this call which would break the ALL_BP_LOCATIONS iteration above.  */
-  if (update_locations)
-    update_global_location_list (0);
+	  /* Print nothing for this entry if we dont stop or dont print.  */
+	  if (bs->stop == 0 || bs->print == 0)
+	    bs->print_it = print_it_noop;
+	}
+    }
 
   for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
     {
--- a/gdb/testsuite/gdb.base/condbreak.exp
+++ b/gdb/testsuite/gdb.base/condbreak.exp
@@ -68,8 +68,12 @@ set bp_location1  [gdb_get_line_number "set breakpoint 1 here"]
 set bp_location6  [gdb_get_line_number "set breakpoint 6 here"]
 set bp_location8  [gdb_get_line_number "set breakpoint 8 here" $srcfile1]
 set bp_location9  [gdb_get_line_number "set breakpoint 9 here" $srcfile1]
+set bp_location13 [gdb_get_line_number "set breakpoint 13 here" $srcfile1]
+set bp_location14 [gdb_get_line_number "set breakpoint 14 here" $srcfile1]
 set bp_location15 [gdb_get_line_number "set breakpoint 15 here" $srcfile1]
 set bp_location16 [gdb_get_line_number "set breakpoint 16 here" $srcfile1]
+set bp_location17 [gdb_get_line_number "set breakpoint 17 here" $srcfile1]
+set bp_location18 [gdb_get_line_number "set breakpoint 18 here" $srcfile1]
 
 #
 # test break at function
@@ -110,15 +114,29 @@ gdb_test "break marker2 if (a==43)" \
     "Breakpoint.*at.* file .*$srcfile1, line.*"
 
 #
+# Check break involving inferior function call.
+# Ensure there is at least one additional breakpoint with higher VMA.
+#
+gdb_test "break marker3 if (multi_line_if_conditional(1,1,1)==0)" \
+    "Breakpoint.*at.* file .*$srcfile1, line.*"
+gdb_test "break marker4" \
+    "Breakpoint.*at.* file .*$srcfile1, line.*"
+
+#
 # check to see what breakpoints are set
 #
 
 if {$hp_aCC_compiler} {
     set marker1_proto "\\(void\\)"
     set marker2_proto "\\(int\\)"
+    # Not checked.
+    set marker3_proto "\\(char \\*, char \\*\\)"
+    set marker4_proto "\\(long\\)"
 } else {
     set marker1_proto ""
     set marker2_proto ""
+    set marker3_proto ""
+    set marker4_proto ""
 }
 
 gdb_test "info break" \
@@ -129,7 +147,10 @@ gdb_test "info break" \
 \[0-9\]+\[\t \]+breakpoint     keep y.* in main at .*$srcfile:$bp_location1.*
 \[\t \]+stop only if \\(1==1\\).*
 \[0-9\]+\[\t \]+breakpoint     keep y.* in marker2$marker2_proto at .*$srcfile1:($bp_location8|$bp_location9).*
-\[\t \]+stop only if \\(a==43\\).*" \
+\[\t \]+stop only if \\(a==43\\).*
+\[0-9\]+\[\t \]+breakpoint     keep y.* in marker3$marker3_proto at .*$srcfile1:($bp_location17|$bp_location18).*
+\[\t \]+stop only if \\(multi_line_if_conditional\\(1,1,1\\)==0\\).*
+\[0-9\]+\[\t \]+breakpoint     keep y.* in marker4$marker4_proto at .*$srcfile1:($bp_location13|$bp_location14).*" \
     "breakpoint info"
 
 
@@ -247,3 +268,23 @@ gdb_test "break *main if (1==1) th 999" \
     "Unknown thread 999\\."
 gdb_test "break *main if (1==1) ta 999" \
     "Unknown task 999\\."
+
+set test "run until breakpoint at marker3"
+gdb_test_multiple "continue" $test {
+    -re "Continuing\\..*Breakpoint \[0-9\]+, marker3 \\(a=$hex \"stack\", b=$hex \"trace\"\\) at .*$srcfile1:($bp_location17|$bp_location18).*($bp_location17|$bp_location18)\[\t \]+.*$gdb_prompt $" {
+	pass $test
+    }
+    -re "Continuing\\..*Breakpoint \[0-9\]+, $hex in marker3 \\(a=$hex \"stack\", b=$hex \"trace\"\\) at .*$srcfile1:($bp_location17|$bp_location18).*($bp_location17|$bp_location18)\[\t \]+.*$gdb_prompt $" {
+	xfail $test
+    }
+}
+
+set test "run until breakpoint at marker4"
+gdb_test_multiple "continue" $test {
+    -re "Continuing\\..*Breakpoint \[0-9\]+, marker4 \\(d=177601976\\) at .*$srcfile1:($bp_location13|$bp_location14).*($bp_location13|$bp_location14)\[\t \]+.*$gdb_prompt $" {
+	pass $test
+    }
+    -re "Continuing\\..*Breakpoint \[0-9\]+, $hex in marker4 \\(d=177601976\\) at .*$srcfile1:($bp_location13|$bp_location14).*($bp_location13|$bp_location14)\[\t \]+.*$gdb_prompt $" {
+	xfail $test
+    }
+}