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

[Yao Qi]

Hi,
I find a program will receive segv fault when I set a regular tracepoint
and a fast tracepoint at the same address, start tracing and resume program.

gdbserver has taken care of this situation in many places of the code,
when uninserting breakpoint or fast tracepoint, write_inferior_memory is
called to take care of layering breakpoints on top of fast tracepoints.
 However, it is not right to me.  Here is an example to illustrate this
problem.

Supposing I set a regular tracepoint and a fast tracepoint on 0x080484fc,

    0x080484fc <+3>:     e8 f3 ff ff ff  call   0x80484f4 <func>

During insertion, trap insn (for regular tracepoint) and jmp insn (for
fast tracepoint) are inserted, and gdbserver takes care of them to make
sure trap insn is *always* inserted on top of jmp insn.  In my example,
gdbserver will first insert a jmp insn (0xe9XXXXXXXX), save original
insn in jump shadow (0xe8f3ffffff), and then insert trap insn on top of
jump insn.  So, gdbserver writes 0xcc to 0x080484fc and save the first
byte (0xe9) in bp->old_data.  Memory content on 0x080484fc is
0xccXXXXXXXX.  Note that bp->old_data saves the first byte of jmp insn
rather than original insn.

When program hits trap insn, gdbserver will step-over trap insn.
gdbserver will uninsert trap and uninsert fast tracepoint jumps (see
linux-low.c:start_step_over).  In uninsert_raw_breakpoint, bp->old_data
will be written back to bp->pc, in my example, 0xe9 is written back to
0x080484fc.  check_mem_write will be called, and check the overlap of
memory to be write and breakpoint/tracepoint insn memory range.  In my
example, there is an overlap with fast tracepoint jump insn and trap
tracepoint insn.  So, 1) 0xe9 is written to the first byte of jump
shadow and shadow becomes 0xe9f3ffffff, which is clobbered, 2) insn on
0x080484fc becomes 0xe9XXXXXXXX (right jump insn).  Later, when
uninserting fast tracepoint jumps, jump shadow will be written back, so
0xe9f3ffffff is written back, it is a wrong insn.

Generally, when writing to memory, we need check_mem_write to take care
of layering breakpoint and tracepoint, however, when writing memory
during breakpoint/tracepoint uninsertion, we don't have to.  Because 1)
trap breakpoint is *always* inserted on top of fast tracepoint jump, 2)
trap insn is not longer than jmp insn.  During uninsertion, we don't
have to worry about layering, and uninsert trap breakpoint and fast
tracepoint jump one by one with simple *the_target->write_memory.

Test case attached is about testing setting breakpoint and different
kinds of tracepoint at the same address.  In current trunk, there are 3
fails on x86_64-linux,

  FAIL: gdb.trace/trace-break.exp: 1 ftrace on: continue to end
  FAIL: gdb.trace/trace-break.exp: 2 trace ftrace on: continue to end
  FAIL: gdb.trace/trace-break.exp: 2 trace ftrace off: continue to end

I also run trace-break.exp on commit
de642393026eee797efdd1355c1913f8054dab64, which is the first revision
for fast tracepoint.

commit de642393026eee797efdd1355c1913f8054dab64
Author: Pedro Alves <pedro@codesourcery.com>
Date:   Tue Jun 1 13:20:49 2010 +0000

    gdb/gdbserver/
    2010-06-01  Pedro Alves  <pedro@codesourcery.com>
            Stan Shebs  <stan@codesourcery.com>

[...]
        * mem-break.c (set_raw_breakpoint_at): Use read_inferior_memory.
        (struct fast_tracepoint_jump): New.
        (fast_tracepoint_jump_insn): New.
        (fast_tracepoint_jump_shadow): New.
        (find_fast_tracepoint_jump_at): New.
        (fast_tracepoint_jump_here): New.
        (delete_fast_tracepoint_jump): New.
        (set_fast_tracepoint_jump): New.
        (uninsert_fast_tracepoint_jumps_at): New.
        (reinsert_fast_tracepoint_jumps_at): New.
        (set_breakpoint_at): Use write_inferior_memory.
        (uninsert_raw_breakpoint): Use write_inferior_memory.
        (check_mem_read): Mask out fast tracepoint jumps.
        (check_mem_write): Mask out fast tracepoint jumps.
[...]

  FAIL: gdb.trace/trace-break.exp: 1 ftrace on: continue to end
  FAIL: gdb.trace/trace-break.exp: 2 trace ftrace on: continue to end
  FAIL: gdb.trace/trace-break.exp: 3 ftrace on: continue to end
  FAIL: gdb.trace/trace-break.exp: 2 trace ftrace off: continue to end
  FAIL: gdb.trace/trace-break.exp: 3 ftrace off: continue to end

Test case fails as well, so these fails are not regression.

Regression tested on x86_64-linux, no regression and fails in
gdb.trace/trace-break.exp are fixed.  OK for mainline?

-- 
Yao (齐尧)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-new-testcase-trace-break.patch
Type: text/x-patch
Size: 9815 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20111027/1b6d35ec/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-don-t-call-write_inferior_memory.patch
Type: text/x-patch
Size: 3160 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20111027/1b6d35ec/attachment-0001.bin>