[patch, gdbserver] Uninsert bpkt when regular and fast tracepoint are set at the same address

Yao Qi yao@codesourcery.com
Sat Oct 29 12:40:00 GMT 2011


On 10/27/2011 11:59 PM, Pedro Alves wrote:
>> > Note that bp->old_data saves the first byte of jmp insn
>> > rather than original insn.
> _This_ is the bug.  I must have introduced it as a last minute change
> before pushing fast tracepoints upstream, because I'm sure this used
> to work, proof being all the comments in the code about it.  :-/
> 
> The idea is that the shadow of all breakpoints and fast tracepoint
> jumps always has the contents of the memory as if no breakpoint
> of fast tracepoint was inserted.
> 

Usually, shadow of both breakpoints and fast tracepoint jumps is a copy
of *original* insn.  That is absolutely correct.

> The problem is that check_mem_write does:
> 
>  - loop over fast tracepoint jumps, updating their shadow buffers,
>    _and_ clobbers the input buffer.
>  - loop over raw breakpoints, updating their shadow buffers, but
>    at this point, the input buffer is already overwritten with
>    fast tracepoint jumps...
> 
> The fix is to split updating the shadow buffers from stapling fast
> tracepoint jumps and breakpoints on top of the input buffer.
> 

Your fix looks right to me, except one concern on performance.  Your
patch doubles the time on iterating over fast tracepoint jump list and
raw breakpoint list.  It is not a big deal when the number of fast
tracepoint and breakpoint is small.  I did an experiment on showing how
much time step-over breakpoint may cost as the number of tracepoint goes
up.  In my experiment, I set thousands (from 1*1296 to 6*1296) of
tracepoints at some different unreachable places to make sure the
breakpoint list in gdbserver is long enough, and set four breakpoints in
program.  GDB will step-over these breakpoint until the end of program,
and I'll calculate the time spent.  The result shows there is no notable
slowdown with your patch applied, it is good, and I am over-worried :)

Another thing I want you to help me to understand is what is wrong with
my patch?  In my patch, the interpretation of shadow is different from
yours.  In my patch, when raw breakpoint and fast tracepoint jump is set
at the same address, shadow of breakpoint is a copy of jump insn.  It
makes easier in uninsert breakpoint, because check_mem_write is not
needed, and we can simply write memory.  The only problem I can think of
is about removing fast tracepoint first, and leaving breakpoint still
there.  Except this problem, is there any more problem?

> While at it, I did several changes to the new tests.  Thanks a lot
> for writing them BTW.  They're awesome.  I wish I had done so before...
> I'll point out the main issues below.
> 

The tests after your modifications looks better.  Please check them in
with your patch.  Thanks for reviewing them.

-- 
Yao (齐尧)



More information about the Gdb-patches mailing list