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

Pedro Alves
2016-04-12T09:41:50Z

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