[rfc] Improve handling of failing remove_breakpoint calls

[rfc] Improve handling of failing remove_breakpoint calls

[Ulrich Weigand]

Hello,

one more breakpoint-related issue arose from the Cell debugger:  in some
cases, GDB is unable to remove a breakpoint from a main SPU stand-alone
exectuable if the corresponding SPU context was already removed.

This causes remove_breakpoint to return failure.  Now for the most part,
this doesn't really have any effect (there are a few callers that under
certain circumstances issue a warning or error, but those do not apply
in the Cell case).

However, there is one big problem: the loops across all breakpoint locations
in remove_breakpoints and similar functions will themselves terminate as
soon as any of the remove_breakpoint calls fail.  They will not even attempt
to remove any of the *other* locations on the chain -- even if those could
be removed without problem.

This has quite weird effects if certain internal breakpoints (e.g. solib
event breakpoints) are not removed when GDB assumes they were, causing
GDB to loop endlessly in some cases.

It seems to me there is really no point for this behaviour: those routines
should simply attempt to remove *all* breakpoint locations, and simply
return a summary status reporting where there all could be removed or
whether there was an error with any of them.

The following patch implements this idea.

Tested on powerpc64-linux with no regressions.   Any comments?
I'm planning to commit this within a week or so.

Bye,
Ulrich


ChangeLog:

	* breakpoint.c (remove_breakpoints): If removing one breakpoint
	location fails, still continue to remove other locations.
	(remove_hw_watchpoints): Likewise.
	(detach_breakpoints): Likewise.


Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c
+++ src/gdb/breakpoint.c
@@ -1409,36 +1409,28 @@ int
 remove_breakpoints (void)
 {
   struct bp_location *b;
-  int val;
+  int val = 0;
 
   ALL_BP_LOCATIONS (b)
   {
     if (b->inserted)
-      {
-	val = remove_breakpoint (b, mark_uninserted);
-	if (val != 0)
-	  return val;
-      }
+      val |= remove_breakpoint (b, mark_uninserted);
   }
-  return 0;
+  return val;
 }
 
 int
 remove_hw_watchpoints (void)
 {
   struct bp_location *b;
-  int val;
+  int val = 0;
 
   ALL_BP_LOCATIONS (b)
   {
     if (b->inserted && b->loc_type == bp_loc_hardware_watchpoint)
-      {
-	val = remove_breakpoint (b, mark_uninserted);
-	if (val != 0)
-	  return val;
-      }
+      val |= remove_breakpoint (b, mark_uninserted);
   }
-  return 0;
+  return val;
 }
 
 int
@@ -1663,7 +1655,7 @@ int
 detach_breakpoints (int pid)
 {
   struct bp_location *b;
-  int val;
+  int val = 0;
   struct cleanup *old_chain = save_inferior_ptid ();
 
   if (pid == PIDGET (inferior_ptid))
@@ -1674,17 +1666,10 @@ detach_breakpoints (int pid)
   ALL_BP_LOCATIONS (b)
   {
     if (b->inserted)
-      {
-	val = remove_breakpoint (b, mark_inserted);
-	if (val != 0)
-	  {
-	    do_cleanups (old_chain);
-	    return val;
-	  }
-      }
+      val |= remove_breakpoint (b, mark_inserted);
   }
   do_cleanups (old_chain);
-  return 0;
+  return val;
 }
 
 static int
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Improve handling of failing remove_breakpoint calls

[Pedro Alves]

On Wednesday 22 July 2009 18:02:15, Ulrich Weigand wrote:
> The following patch implements this idea.

It makes sense to me.

-- 
Pedro Alves

Re: [rfc] Improve handling of failing remove_breakpoint calls

[Ulrich Weigand]

Pedro Alves wrote:
> On Wednesday 22 July 2009 18:02:15, Ulrich Weigand wrote:
> > The following patch implements this idea.
> 
> It makes sense to me.

Thanks for the review!  I've checked this in as well.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com