Re: [PATCH 2/2] Reset trace local state when opening trace file

[Yao Qi <yao at codesourcery dot com>]

On 06/04/2013 07:40 PM, Pedro Alves wrote:
> I was going to suggest fixing somewhere along target_preopen/target_pre_inferior,
> but then the stale trace state may even interfere with the exec target,
> (what we get at the top of the stack when we drop from remote/tfile/ctf).
> 
> So, instead, how about we fix this in remote_close/tfile_close/ctf_close ?
> 
> Haven't tried it, but it should fix the above case as well.
> 

Pedro,
I can reproduce it in your example, and in the new patch, I call
trace_reset_local_state in remote_close/tfile_close/ctf_close.  The
problem is fixed.

....
(gdb) flushregs 
Register cache flushed.
Remote connection closed
(gdb) target remote :1234
Remote debugging using :1234
Reading symbols from /lib/ld-linux.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib/ld-linux.so.2
0x4ce4c2b0 in _start () from /lib/ld-linux.so.2
(gdb) set debug remote 1
(gdb) tstatus 
Sending packet: $qTStatus#49...Packet received: T0;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:000000000;stoptime:000000000;username::;notes::
No trace has been run on the target.
Collected 0 trace frames.
Trace buffer has 5242880 bytes of 5242880 bytes free (0% full).
Trace will stop if GDB disconnects.
Not looking at any trace frame.
Sending packet: $qTP:2:80483ca#86...Packet received: E01
Target returns error code '01'.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The message "warning: could not set remote traceframe" and "Looking
at trace frame 0, tracepoint 2" disappeared, but "Target returns error
code '01'" is still there.  It caused by gdb sending 'qTP' packet, and
it is not related to this issue.  The patch below is regress tested on
x86_64 again.

> 
> [
>   The problem should exist as well if we instead switch to the native
>   Linux target just after inspecting a trace frame, but it's masked
>   out by another unrelated problem...:
> 
>   (gdb) flushregs
>   Register cache flushed.
>   Remote connection closed
>   (gdb) start
>   ../../src/gdb/thread.c:72: internal-error: inferior_thread: Assertion `tp' failed.
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   Quit this debugging session? (y or n)
> 

'info threads' also trigger an assertion failure too.

>   The issue with this one is that regcache_raw_read restores inferior_ptid even
>   when the target is gone.  ooks like we'll have that problem with every
>   save_inferior_ptid cleanup...
> ]

I agree with your analysis, but have no idea how to fix it.  Probably
'getting rid of inferior_ptid' is a right fix.

-- 
Yao (éå)

gdb:

2013-06-05  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c (start_tracing): Move code to ...
	(trace_reset_local_state): ... here.  New.
	(tfile_close): Call trace_reset_local_state.
	* ctf.c (ctf_close): Likewise.
	* remote.c (remote_close): Likewise.
	* tracepoint.h (trace_reset_local_state): Declare.
---
 gdb/ctf.c        |    2 ++
 gdb/remote.c     |    2 ++
 gdb/tracepoint.c |   17 +++++++++++++----
 gdb/tracepoint.h |    1 +
 4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/gdb/ctf.c b/gdb/ctf.c
index 13df089..278f950 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1212,6 +1212,8 @@ ctf_close (void)
   ctf_destroy ();
   xfree (trace_dirname);
   trace_dirname = NULL;
+
+  trace_reset_local_state ();
 }
 
 /* This is the implementation of target_ops method to_files_info.
diff --git a/gdb/remote.c b/gdb/remote.c
index d8854ae..5da7eb9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3050,6 +3050,8 @@ remote_close (void)
     delete_async_event_handler (&remote_async_inferior_event_token);
 
   remote_notif_unregister_async_event_handler ();
+
+  trace_reset_local_state ();
 }
 
 /* Query the remote side for the text, data and bss offsets.  */
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index d6ff051..0a715ef 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1700,6 +1700,16 @@ process_tracepoint_on_disconnect (void)
 	       " GDB is disconnected\n"));
 }
 
+/* Reset local state of tracing.  */
+
+void
+trace_reset_local_state (void)
+{
+  set_traceframe_num (-1);
+  set_tracepoint_num (-1);
+  set_traceframe_context (NULL);
+  clear_traceframe_info ();
+}
 
 void
 start_tracing (char *notes)
@@ -1822,11 +1832,8 @@ start_tracing (char *notes)
   target_trace_start ();
 
   /* Reset our local state.  */
-  set_traceframe_num (-1);
-  set_tracepoint_num (-1);
-  set_traceframe_context (NULL);
+  trace_reset_local_state ();
   current_trace_status()->running = 1;
-  clear_traceframe_info ();
 }
 
 /* The tstart command requests the target to start a new trace run.
@@ -4657,6 +4664,8 @@ tfile_close (void)
   trace_fd = -1;
   xfree (trace_filename);
   trace_filename = NULL;
+
+  trace_reset_local_state ();
 }
 
 static void
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index d7ebc16..3b09ca8 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -389,6 +389,7 @@ extern void merge_uploaded_trace_state_variables (struct uploaded_tsv **utsvp);
 
 extern void query_if_trace_running (int from_tty);
 extern void disconnect_tracing (void);
+extern void trace_reset_local_state (void);
 
 extern void start_tracing (char *notes);
 extern void stop_tracing (char *notes);
-- 
1.7.7.6