Re: [RFA/7.8] user breakpoint not inserted if software-single-step at same location

[Pedro Alves <palves@redhat.com>]

On 05/29/2014 09:11 PM, Joel Brobecker wrote:
> Hello,
> 
> with the following code...
> 
>     12    Nested;   -- break #1
>     13    return I; -- break #2
>     14  end;
> 
> (line 12 is a call to function Nested)
> 
> ... we have noticed the following errorneous behavior on ppc-aix,

"erroneous"

> This patch does make one assumption, however, which is the fact
> that user breakpoints get inserted before software single-step
> breakpoints. I think this is a fair assumption, as software
> single-step breakpoints get created as part of the target-step
> operation (resume with step=1), which logically can only happen
> after we've inserted all breakpoints.

Async/background execution breaks that assumption though.

E.g., say you do:

 (gdb) b *0xsame_addr_as_sss_bkpt
 (gdb) s &
 (gdb) del

That "del" runs while the target is single-stepping.  And it
might just delete the breakpoint that was at the same address
as a single-step breakpoint, before the single-step breakpoint
traps or gdb collects the trap.

If you try that on native GNU/Linux, it'll fail because
then GDB can't poke at memory while the program is running.
You can still trigger it with using two threads:

 (gdb) b *0xsame_addr_as_sss_bkpt
 (gdb) set scheduler-locking on
 (gdb) thread 1
 (gdb) s &
 (gdb) thread 2  // this thread is stopped, so we can poke at memory.
 (gdb) del

That's why I thought it'd be easier to do this in the "remove" path.

We can still bypass actually inserting the sss breakpoint in
the "insert" path if there's already another breakpoint there,
but we'll need to create/clone the location and its shadow buffer,
and then still handle the issue in the "remove" path.  I'm now
thinking that indeed we should implement that optimization, but not
for efficiency (this being a corner case, it's doubtful it
matters), but to cater for remote targets that might not handle
duplicate Z0 packets well.  They're supposed to be handled in
an idempotent manner, but even GDBserver had related issues
recently.

Oh, _but_!  If the target supports breakpoint evaluating
breakpoint conditions itself (target_supports_evaluation_of_breakpoint_conditions),
such as GDBserver, then we _need_ to send the duplicate
software single-step Z0 breakpoint, in case the non-sss breakpoint
that is already inserted at the same address was conditional, otherwise
the target is only going to report the breakpoint hit if the
non-sss breakpoint's condition evaluates true, while we need the
sss breakpoint to be unconditional.  (In this case we know for
sure that the target knows what to do with the duplicate Z0
packets, it's a requirement of the feature.)

In sum, in the "insert" path:

 - if !target_supports_evaluation_of_breakpoint_conditions; then
     optimize out the sss breakpoint if there's already a
     non-sss breakpoint inserted at the same address.
   else
     make sure to resend/reinsert the breakpoint sss breakpoint,
     even if there's already a non-sss in place, in case that other
     breakpoint was conditional.
   fi

And in the "remove" path:

 - if there's still a non-sss breakpoint inserted at the
   same address, then don't actually remove the breakpoint
   off of the target, just wipe it from gdb's list.

> Also, due to the nature of the regression (compared to 7.7), I think
> we should wait for a resolution of this issue before creating the
> gdb 7.8 release branch.

I agree.

Thanks,
-- 
Pedro Alves