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

[Pedro Alves <palves at redhat dot com>]

On 05/28/2013 05:20 AM, Yao Qi wrote:
> This patch is to reset the GDB local trace state in ctf_open and
> tfile_open, say, when opening trace file.  This patch fixes the
> fails mentioned in patch 1/2.
> 
> The whole series are tested on x86-linux with both native and
> gdbserver.

Sounds like the problem is more widespread than just the tfile/ctf
targets:

shell 1                                        shell 2
                                               [spawn gdbserver]
[connect to gdbserver, collect a tracepoint, then]
(gdb) tfind
Found trace frame 0, tracepoint 2
2729      char **next_arg = &argv[1];

                                               [kill gdbserver]
[force some RSP traffic with flushregs]
(gdb) flushregs
Register cache flushed.
Remote connection closed
                                               [spawn another gdbserver]
(gdb) tar rem :9999
Remote debugging using :9999
warning: could not set remote traceframe
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib64/ld-2.15.so.debug...done.
done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
Failed to read a valid object file image from memory.
0x00007ffff7dde530 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) tstatus
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.
Looking at trace frame 0, tracepoint 2.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Target returns error code '01'.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
(gdb)

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.


[
 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)

 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...
]

> 
> gdb:
> 
> 2013-05-28  Yao Qi  <yao@codesourcery.com>
> 
> 	* ctf.c (ctf_open): Call trace_reset_local_state.
> 	* tracepoint.c (start_tracing): Move some code to ...
> 	(trace_reset_local_state): ... here.  New.
> 	(tfile_open): Call trace_reset_local_state.
> 	* tracepoint.h (trace_reset_local_state): Declare.
> ---
>  gdb/ctf.c        |    2 ++
>  gdb/tracepoint.c |   17 +++++++++++++----
>  gdb/tracepoint.h |    1 +
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/ctf.c b/gdb/ctf.c
> index 13df089..c99d26d 100644
> --- a/gdb/ctf.c
> +++ b/gdb/ctf.c
> @@ -1201,6 +1201,8 @@ ctf_open (char *dirname, int from_tty)
>  
>    merge_uploaded_trace_state_variables (&uploaded_tsvs);
>    merge_uploaded_tracepoints (&uploaded_tps);
> +
> +  trace_reset_local_state ();
>  }
>  
>  /* This is the implementation of target_ops method to_close.  Destroy
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index ba59ab0..998483e 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -1720,6 +1720,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)
> @@ -1842,11 +1852,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.
> @@ -4282,6 +4289,8 @@ tfile_open (char *filename, int from_tty)
>    merge_uploaded_trace_state_variables (&uploaded_tsvs);
>  
>    merge_uploaded_tracepoints (&uploaded_tps);
> +
> +  trace_reset_local_state ();
>  }
>  
>  /* Interpret the given line from the definitions part of the trace
> diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
> index 18762ca..7a60ef2 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);
> 


-- 
Pedro Alves