Re: [PATCH 4/7] Insert breakpoint even when the raw breakpoint is found

[Pedro Alves <palves@redhat.com>]

On 04/12/2016 10:03 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Can we only increase the refcount if inserting succeeds?  gdbserver can
>> use gdb exceptions nowadays, and even though lots of current gdbserver
>> code isn't exception safe, it'd be nice to start considering that.
> 
> One thing I can think of is to properly decrease refcount if exception
> is thrown.  Anything else?

Nothing else, that's all I meant.

Actually, now that I look closer, I think we could merge the new
code with the code that handles inserting a new raw breakpoint just below.

For example:

diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index b06f8e9..419db9e 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -390,6 +390,7 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
 {
   struct process_info *proc = current_process ();
   struct raw_breakpoint *bp;
+  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
 
   if (type == raw_bkpt_type_sw || type == raw_bkpt_type_hw)
     {
@@ -408,32 +409,39 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
   else
     bp = find_raw_breakpoint_at (where, type, kind);
 
-  if (bp != NULL)
+  if (bp == NULL)
     {
-      bp->refcount++;
-      return bp;
+      bp = XCNEW (struct raw_breakpoint);
+      bp->pc = where;
+      bp->kind = kind;
+      bp->raw_type = type;
+      make_cleanup (xfree, bp);
     }
 
-  bp = XCNEW (struct raw_breakpoint);
-  bp->pc = where;
-  bp->kind = kind;
-  bp->refcount = 1;
-  bp->raw_type = type;
-
-  *err = the_target->insert_point (bp->raw_type, bp->pc, bp->kind, bp);
-  if (*err != 0)
+  if (!bp->inserted)
     {
-      if (debug_threads)
-	debug_printf ("Failed to insert breakpoint at 0x%s (%d).\n",
-		      paddress (where), *err);
-      free (bp);
-      return NULL;
+      *err = the_target->insert_point (bp->raw_type, bp->pc, bp->kind, bp);
+      if (*err != 0)
+	{
+	  if (debug_threads)
+	    debug_printf ("Failed to insert breakpoint at 0x%s (%d).\n",
+			  paddress (where), *err);
+
+	  do_cleanups (old_chain);
+	  return NULL;
+	}
+
+      bp->inserted = 1;
     }
 
-  bp->inserted = 1;
-  /* Link the breakpoint in.  */
-  bp->next = proc->raw_breakpoints;
-  proc->raw_breakpoints = bp;
+  discard_cleanups (old_chain);
+
+  /* Link the breakpoint in, if this is the first reference.  */
+  if (++bp->refcount == 1)
+    {
+      bp->next = proc->raw_breakpoints;
+      proc->raw_breakpoints = bp;
+    }
   return bp;
 }
 
-- 
2.5.5