Re: [pushed 1/2] PR gdb/18002: Fix reinsert of a permanent breakpoints

[Pedro Alves <palves@redhat.com>]

On 03/06/2015 02:31 PM, Yao Qi wrote:

> Your fix looks right to me, although I am testing a different one, in
> which bp_location_has_shadow returns false if bl->permanent is true.
> Anyway, Thanks for fixing this bug, Pedro.

Yeah, I also considered that, though we'd likely end up needing more
special casing in other places: the setting of shadow_len before the
read and the shadow_contents afterwards looked so bogus that I
thought that even if we fixed this some other way, we should still
do what I had done.  And then I was seeing more fundamental brokenness
with permanent breakpoints and stopped at that point.  The other issues
I identified were:

- If the user manually changes memory at the permanent breakpoint
address, we should really write to the shadow buffer, not to
memory, and clear the permanent bp flag.  We don't do the latter.
Likewise, if the user writes an int3 manually where a breakpoint is
already set, we should mark the breakpoint as permanent.  I suspect
that filling the shadow buffer (with a software breakpoint) immediately
when we detect the program breakpoint (bp_loc_is_permanent's caller)
would make things simpler, considering targets where the breakpoint
is longer than one byte, and writes to only parts of the breakpoint.

- I also considered completely getting rid of the ->permanent
flag, and then in places where we need to know whether we stopped
at one (to step over it), we would check if the shadow contains
a software breakpoint.  The case that gave me pause was hardware
breakpoints on top of a permanent bp, which don't have a shadow.
Or we could even always check if there's a program breakpoint at
PC that we should skip by manually advancing the PC, even if there's
no user breakpoint on top...  The trick will be to try to avoid
the extra memory read.  But maybe either that doesn't matter in
practice, given that we should limit that for when the program had
stopped for a SIGTRAP, and, SIGTRAP isn't set to "pass", and the PC
it still the PC the thread had when it stopped.  Hmm...

Thanks,
Pedro Alves