On 03/07/2012 01:03 AM, Pedro Alves wrote:
> Hmm, what does that error on "continue" mean?  With just your patch, I see in gdb.log:
> 
> continue
> Continuing.
> Target returns error code '01'.
> 
> That's another generic error that could be made to return "E." instead.

This problem is what I planed to fix in next step.

> 
> So I enabled remote debug, and we see:
> 
> continue
> Continuing.
> Sending packet: $QPassSignals:#f3...Packet received: OK
> Sending packet: $vCont;s:p3e0d.3e0d#e8...Packet received: T0506:80d6ffffff7f0000;07:58d6ffffff7f0000;10:6a07400000000000;thread:p3e0d.3e0d;core:3;
> Sending packet: $qTStatus#49...Packet received: T1;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:001331049107617717;stoptime:00000
> 0000;username::;notes::
> Sending packet: $qTMinFTPILen#3b...Packet received: 5
> Sending packet: $m7ffff7bf05c6,1#96...Packet received: e8
> Sending packet: $m7ffff7bf05c7,4#9a...Packet received: f1ffffff
> Sending packet: $QTDP:4:00007ffff7bf05c6:E:0:0:F5#75...Packet received: E.duplicate
> Target returns error code '01'.
> 
> So we're failing in QTDP handling, as expected.  But where exactly?  It's not when
> trying to install the fast tracepoint to target memory.  It's here:
> 
> --- c/gdb/gdbserver/tracepoint.c
> +++ w/gdb/gdbserver/tracepoint.c
> @@ -2307,7 +2307,8 @@ cmd_qtdp (char *own_buf)
>           trace_debug ("Tracepoint error: tracepoint %d"
>                        " at 0x%s already exists",
>                        (int) num, paddress (addr));
> -         write_enn (own_buf);
> +         sprintf (own_buf, "E.duplicate");
> +         //write_enn (own_buf);
>           return;
>         }
> 
> continue
> Continuing.
> Sending packet: $QPassSignals:#f3...Packet received: OK
> Sending packet: $vCont;s:p3e0d.3e0d#e8...Packet received: T0506:80d6ffffff7f0000;07:58d6ffffff7f0000;10:6a07400000000000;thread:p3e0d.3e0d;core:3;
> Sending packet: $qTStatus#49...Packet received: T1;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:001331049107617717;stoptime:00000
> 0000;username::;notes::
> Sending packet: $qTMinFTPILen#3b...Packet received: 5
> Sending packet: $m7ffff7bf05c6,1#96...Packet received: e8
> Sending packet: $m7ffff7bf05c7,4#9a...Packet received: f1ffffff
> Sending packet: $QTDP:4:00007ffff7bf05c6:E:0:0:F5#75...Packet received: E.duplicate
> Target returns error code '.duplicate'.
> 
> So something doesn't look exactly right.  The target knows about this fast
> tracepoint already, but it isn't really installed yet (so we have 3 levels of "inserted",
> not just 2: never downloaded; downloaded; downloaded and installed).  Maybe GDB

Yes, there are 3 levels so far.  Do you think target should keep fast
tracepoints that "downloaded but not installed yet"?  I am inclined to
teach target to remove downloaded fast tracepoints if they are not
installed successfully.  Does it sounds good?  The QTDP is like a
transaction, returns OK when both download and installation is
successful.  If installation failed, roll back to cleanup downloaded
fast tracepoint.

> should have disabled the tracepoint on "tstart", or deleted it from the target, or...
> Anyway, that'll need some further thinking.  In the mean time, I've made gdbserver

Agreed.  GDB tracepoint/breakpoint module should exposes some interfaces
for other modules, such as target, to delete or disable breakpoint
locations when needed.

> send a clearer error back to GDB, and we end up with:
> 
> Target returns error code '.Tracepoint 2 at 0x7ffff67c2579 already exists'.

It is OK for me to let remote target to reports a duplicated tracepoint.

In this version, some changes compared with last one,

  - move `tracepoint already exist' patch out, which can be a separate one,
  - remove downloaded tracepoint in target when failed to install,

-- 
Yao (齐尧)