Re: [PATCH] Expect SI_KERNEL si_code for a MIPS software breakpoint trap

From: Pedro Alves <palves@redhat.com>
To: "Maciej W. Rozycki" <macro@imgtec.com>,
	Luis Machado <lgustavo@codesourcery.com>
Cc: "Maciej W. Rozycki" <macro@linux-mips.org>,
	gdb-patches@sourceware.org, linux-mips@linux-mips.org
Subject: Re: [PATCH] Expect SI_KERNEL si_code for a MIPS software breakpoint trap
Date: Tue, 16 Feb 2016 00:30:02 +0000	[thread overview]
Message-ID: <56C26D8A.9070401@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1602152315540.15885@tp.orcam.me.uk>

On 02/15/2016 11:50 PM, Maciej W. Rozycki wrote:

>  FWIW, I maintain it's GDB that should be handling it.  What if TRAP_BRKPT 
> is reported for a breakpoint that has not been set by GDB in the first 
> place and is still there in code?  I take it either GDB or gdbserver, as 
> applicable, will just sit there looping indefinitely in an attempt to 
> discard the event while executing the breakpoint instruction over and over 
> again.

Nope.

> There's nothing stopping the user from having a MIPS `BREAK 5' 
> instruction or say INT3 for the x86 target already present in the 
> executable being debugged.

GDB only ignores the TRAP_BRKPT event if there's no "BREAK 5" instruction
hardcoded in the binary.  If there is, then the program is stopped and
a SIGTRAP is reported to the user.

>  What I think GDB ought to be doing here is caching addresses for recently 
> removed breakpoints and discarding spurious hits in that cache.

That does not work in general.  The most problematic archs are those that
leave the PC pointing after the breakpoint instruction, such as x86.
See more below.

> It may 
> actually be worth verifying, before discarding such a hit, that there is 
> no breakpoint instruction there anymore in target memory too -- a clever 
> user might have set a breakpoint on a breakpoint instruction already there 
> in code!

Yep, GDB does that already, and we have tests that cover this.
See gdb.base/bp-permanent.exp.

> 
>  It seems to me it'll be enough if the cache is only retained over a 
> single resumption step, per thread of course, as it does not appear to me 
> that the kernel might queue more than one software breakpoint signal at 
> once.

That wouldn't work, as a new thread GDB doesn't know about yet may report
a stop for the PC where a breakpoint used to be, and then you don't
know whether you need to adjust its PC.

Remembering whether a breakpoint was inserted was what GDB used to
do, and it was because it was problematic that "swbreak" was
invented.  See:

 https://sourceware.org/ml/gdb-patches/2015-02/msg00726.html

Particularly:

 https://sourceware.org/ml/gdb-patches/2015-02/msg00730.html

This was a previous attempt that tried to preserve moribund
locations, but was still not sufficient:
 https://sourceware.org/ml/gdb-patches/2014-10/msg00781.html

I'm really hoping that at some point all archs implement
TRAP_BRKPT and the moribund locations heuristic can be removed
from gdb.

Thanks,
Pedro Alves