Re: [PATCH, v3] Expect SI_KERNEL or TRAP_BRKPT si_code values for MIPS breakpoint traps

[Luis Machado <lgustavo at codesourcery dot com>]

On 02/24/2016 09:55 AM, Pedro Alves wrote:
> On 02/23/2016 02:00 AM, Luis Machado wrote:
> > On 02/22/2016 07:41 PM, Pedro Alves wrote:
> > > On 02/22/2016 10:20 PM, Luis Machado wrote:
> > >
> > > > diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
> > > > index ba58717..72b32b1 100644
> > > > --- a/gdb/nat/linux-ptrace.h
> > > > +++ b/gdb/nat/linux-ptrace.h
> > > > @@ -140,11 +140,16 @@ struct buffer;
> > > >       in SPU code on a Cell/B.E.  However, SI_KERNEL is never seen
> > > >       on a SIGTRAP for any other reason.
> > > >
> > > > +   The MIPS kernel uses the default si_code of SI_KERNEL for software
> > > > +   breakpoints, hardware watchpoints and SIGTRAP's in general.
> > >
> > > If we get this for both software breakpoints and hardware watchpoints,
> > > then it seems to me that this change still leaves watchpoints broken,
> > > as I can't see how check_stopped_by_watchpoint is reached, in either
> > > gdb/linux-nat.c or gdbserver/linux-low.c.
> >
> > With or without this specific breakpoint fix, hardware watchpoints are
> > already broken for MIPS AFAICS.
>
> Yep, hence my "still".  ;-)
>
> > MIPS' kernel has never set si_code to anything other than SI_KERNEL.
> > When the change to expect TRAP_HWBKPT was committed, then hardware
> > watchpoints stopped working properly for MIPS.
> >
> > Now all hardware watchpoints produce are breakpoint-like traps that get
> > silently ignored by GDB as "delayed software breakpoint trap". Neither
> > GDB nor GDBserver can tell when a hardware watchpoint really happened by
> > using si_code information.
> >
> > But this seems to be a different issue and i think it should be handle
> > separately.
>
> I think it's all the same, and that we'll need to change
> linux-nat.c:save_sigtrap and the equivalent bits in gdbserver/linux-low.c.
>
> So if we pick the table I added for x86, and fill it in
> for MIPS, this is what we get:
>
>     | what                                     | si_code        |
>     |------------------------------------------+----------------|
>     | software breakpoints                     | SI_KERNEL      |
>     | single-steps                             | N/A            |
>     | single-stepping a syscall                | N/A            |
>     | user sent SIGTRAP                        | 0              |
>     | exec SIGTRAP (when no PTRACE_EVENT_EXEC) | doesn't matter |
>     | hardware breakpoints                     | N/A            |
>     | hardware watchpoints                     | SI_KERNEL      |

Should i add that table explicitly or will v4 with the most meaningful 
bits of that table cut it? It seems we will eventually move from 
SI_KERNEL to something more appropriate for MIPS.

> So the only distinction we need to make is between software
> breakpoints SI_KERNEL, and hardware watchpoints SI_KERNEL.
>
> And we can accurately do that with
> mips_linux_stopped_by_watchpoint / mips_stopped_by_watchpoint.
>
> Is my understanding correct?

Yes. We need to check the MIPS debug registers unconditionally and not 
just when a meaningful si_code of TRAP_HWBKPT shows up.

Did you want that in the same patch or in a separate one? I was 
attempting to address the software breakpoint issue for now, but it 
seems i ended up with 2 problems now. :-)