[5/8] Insert breakpoint even when the raw breakpoint is found

Message ID 1457088276-1170-6-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi March 4, 2016, 10:44 a.m. UTC
  When GDBserver inserts a breakpoint, it looks for raw breakpoint, if
the raw breakpoint is found, increase its refcount, and return.  This
doesn't work when it steps over a breakpoint using software single
step and the underneath instruction of breakpoint is branch to self.

When stepping over a breakpoint on ADDR using software single step,
GDBserver uninsert the breakpoint, so the corresponding raw breakpoint
RAW's 'inserted' flag is zero.  Then, GDBserver insert single step
breakpoint at the same address ADDR because the instruction is branch
to self, the same raw brekapoint RAW is found, and increase the
refcount.  However, the raw breakpoint is not inserted, and the
program won't stop.

gdb/gdbserver:

2016-03-04  Yao Qi  <yao.qi@linaro.org>

	* mem-break.c (set_raw_breakpoint_at): Insert raw breakpoint
	when its refcount is increased.
---
 gdb/gdbserver/mem-break.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
  

Comments

Pedro Alves March 11, 2016, 11:53 a.m. UTC | #1
On 03/04/2016 10:44 AM, Yao Qi wrote:
> When GDBserver inserts a breakpoint, it looks for raw breakpoint, if
> the raw breakpoint is found, increase its refcount, and return.  This
> doesn't work when it steps over a breakpoint using software single
> step and the underneath instruction of breakpoint is branch to self.
> 
> When stepping over a breakpoint on ADDR using software single step,
> GDBserver uninsert the breakpoint, so the corresponding raw breakpoint
> RAW's 'inserted' flag is zero.  Then, GDBserver insert single step
> breakpoint at the same address ADDR because the instruction is branch
> to self, the same raw brekapoint RAW is found, and increase the
> refcount.  However, the raw breakpoint is not inserted, and the
> program won't stop.
> 
> gdb/gdbserver:
> 
> 2016-03-04  Yao Qi  <yao.qi@linaro.org>
> 
> 	* mem-break.c (set_raw_breakpoint_at): Insert raw breakpoint
> 	when its refcount is increased.
> ---
>   gdb/gdbserver/mem-break.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
> index b06f8e9..73c5e8a 100644
> --- a/gdb/gdbserver/mem-break.c
> +++ b/gdb/gdbserver/mem-break.c
> @@ -411,7 +411,22 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
>     if (bp != NULL)
>       {
>         bp->refcount++;
> -      return bp;
> +
> +      if (!bp->inserted)
> +	{
> +	  *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);
> +	      free (bp);

If this raw breapoint already existed, and we just bumped the
refcount, then this free leaves those other breakpoints with
a stale pointer.

But I'd like to defer this patch until we reach a conclusion on the
previous one on the gdb side.

> +	      return NULL;
> +	    }
> +	  bp->inserted = 1;
> +	}
> +	return bp;
>       }
>   
>     bp = XCNEW (struct raw_breakpoint);
>
  

Patch

diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index b06f8e9..73c5e8a 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -411,7 +411,22 @@  set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
   if (bp != NULL)
     {
       bp->refcount++;
-      return bp;
+
+      if (!bp->inserted)
+	{
+	  *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);
+	      free (bp);
+	      return NULL;
+	    }
+	  bp->inserted = 1;
+	}
+	return bp;
     }
 
   bp = XCNEW (struct raw_breakpoint);