[unavailable values part 1, 05/17] move traceframe memory reading fallback to read-only sections to GDB side

[Pedro Alves <pedro at codesourcery dot com>]

This moves the fall-back to read from read-only sections
to GDB side.  This completely emulates QTro on GDB's end,
making the target side implementation unnecessary.  

That is, it makes GDB itself fall back to reading read-only
memory from the live target (forcing a temporary switch out
of tfind mode), if a given memory address we want to read
has not been collected.
This makes the set of read-only sections always match 100% that
used by the new read_value_memory function from patch 4, which
was what make me want to fix this.  Having GDB do the fallback
is more correct than having the target do it using the
readonly section list GDB gave the target at "tstart" time with
the QTro packet, because in the presence of dynamically loaded
modules, the read-only sections the target knows about (as
given by GDB earlier with QTro) may not match the current
read-only sections of the live target, at the time the user is
inspecting a traceframe.

This implementation also does _not_ assume a single transfer
can _not_ cover half-trace + half-live memory, unlike gdbserver's.

Note that the switching out and back of tfind mode is
lazy in the patch below.  The common code only switches
"traceframe_number", and both the remote and the
tfile targets make sure the target end's current traceframe
matches common GDB's, at appropriate times.  This avoids many
unnecessary switching between tfind mode off and on when, for
example, disassembling, where you have several reads from live
memory in a row.  We already use the same technique to avoid
a switch of the current thread in common code immediately
causing a switch of the remote current thread
(remote.c:set_general_thread).

With this patch, if the target supports the new
qXfer:traceframe-info:read packet, GDB will never even
attempt reading memory that has not been collected in the
traceframe, so effectively, the QTro handling code in the remote
end is never exercised.  This allows deprecating the QTro packet, but
at the same time, keep sending it to (older) stubs (that will still
need it), and allows stubs to continue implementing the packet if
they want their tracepoint support to work with earlier GDBs.

-- 
Pedro Alves

2011-02-07  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* target.c (target_read_live_memory): New function.
	(memory_xfer_live_readonly_partial): New.
	(memory_xfer_partial): If reading from a traceframe, fallback to
	reading unavailable read-only memory from read-only regions of
	live target memory.
	* tracepoint.c (disconnect_tracing): Adjust.
	(set_current_traceframe): New, factored out from
	set_traceframe_number.
	(set_traceframe_number): Reimplement to only change the traceframe
	number on the GDB side.
	(do_restore_current_traceframe_cleanup): Adjust.
	(make_cleanup_restore_traceframe_number): New.
	(cur_traceframe_number): New global.
	(tfile_open): Set cur_traceframe_number to no traceframe.
	(set_tfile_traceframe): New function.
	(tfile_trace_find): If looking up a traceframe using any method
	other than by number, make sure the current tfile traceframe
	matches gdb's current traceframe.  Update the current tfile
	traceframe if the lookup succeeded.
	(tfile_fetch_registers, tfile_xfer_partial)
	(tfile_get_trace_state_variable_value): Make sure the remote
	traceframe matches gdb's current traceframe.
	* remote.c (remote_traceframe_number): New global.
	(remote_open_1): Set it to -1.
	(set_remote_traceframe): New function.
	(remote_fetch_registers, remote_store_registers)
	(remote_xfer_memory, remote_xfer_partial)
	(remote_get_trace_state_variable_value): Make sure the remote
	traceframe matches gdb's current traceframe.
	(remote_trace_find): If looking up a traceframe using any method
	other than by number, make sure the current remote traceframe
	matches gdb's current traceframe.  Update the current remote
	traceframe if the lookup succeeded.
	* infrun.c (proceed): Adjust.
	* tracepoint.h (set_current_traceframe): Declare.
	(get_traceframe_number, set_traceframe_number): Add describing
	comments.

---
 gdb/infrun.c     |    2 
 gdb/remote.c     |   43 ++++++++++++++++++
 gdb/target.c     |  129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/tracepoint.c |   70 ++++++++++++++++++++++++-----
 gdb/tracepoint.h |   19 +++++++-
 5 files changed, 247 insertions(+), 16 deletions(-)

Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c	2011-02-07 11:14:48.146706001 +0000
+++ src/gdb/target.c	2011-02-07 11:15:14.186706002 +0000
@@ -1271,6 +1271,82 @@ target_section_by_addr (struct target_op
   return NULL;
 }
 
+/* Read memory from the live target, even if currently inspecting a
+   traceframe.  The return is the same as that of target_read.  */
+
+static LONGEST
+target_read_live_memory (enum target_object object,
+			 ULONGEST memaddr, gdb_byte *myaddr, LONGEST len)
+{
+  int ret;
+  struct cleanup *cleanup;
+
+  /* Switch momentarily out of tfind mode so to access live memory.
+     Note that this must not clear global state, such as the frame
+     cache, which must still remain valid for the previous traceframe.
+     We may be _building_ the frame cache at this point.  */
+  cleanup = make_cleanup_restore_traceframe_number ();
+  set_traceframe_number (-1);
+
+  ret = target_read (current_target.beneath, object, NULL,
+		     myaddr, memaddr, len);
+
+  do_cleanups (cleanup);
+  return ret;
+}
+
+/* Using the set of read-only target sections of OPS, read live
+   read-only memory.  Note that the actual reads start from the
+   top-most target again.  */
+
+static LONGEST
+memory_xfer_live_readonly_partial (struct target_ops *ops,
+				   enum target_object object,
+				   gdb_byte *readbuf, ULONGEST memaddr,
+				   LONGEST len)
+{
+  struct target_section *secp;
+  struct target_section_table *table;
+
+  secp = target_section_by_addr (ops, memaddr);
+  if (secp != NULL
+      && (bfd_get_section_flags (secp->bfd, secp->the_bfd_section)
+	  & SEC_READONLY))
+    {
+      struct target_section *p;
+      ULONGEST memend = memaddr + len;
+
+      table = target_get_section_table (ops);
+
+      for (p = table->sections; p < table->sections_end; p++)
+	{
+	  if (memaddr >= p->addr)
+	    {
+	      if (memend <= p->endaddr)
+		{
+		  /* Entire transfer is within this section.  */
+		  return target_read_live_memory (object, memaddr,
+						  readbuf, len);
+		}
+	      else if (memaddr >= p->endaddr)
+		{
+		  /* This section ends before the transfer starts.  */
+		  continue;
+		}
+	      else
+		{
+		  /* This section overlaps the transfer.  Just do half.  */
+		  len = p->endaddr - memaddr;
+		  return target_read_live_memory (object, memaddr,
+						  readbuf, len);
+		}
+	    }
+	}
+    }
+
+  return 0;
+}
+
 /* Perform a partial memory transfer.
    For docs see target.h, to_xfer_partial.  */
 
@@ -1329,6 +1405,59 @@ memory_xfer_partial (struct target_ops *
 	}
     }
 
+  /* If reading unavailable memory in the context of traceframes, and
+     this address falls within a read-only section, fallback to
+     reading from live memory.  */
+  if (readbuf != NULL && get_traceframe_number () != -1)
+    {
+      VEC(mem_range_s) *available;
+
+      /* If we fail to get the set of available memory, then the
+	 target does not support querying traceframe info, and so we
+	 attempt reading from the traceframe anyway (assuming the
+	 target implements the old QTro packet then).  */
+      if (traceframe_available_memory (&available, memaddr, len))
+	{
+	  struct cleanup *old_chain;
+
+	  old_chain = make_cleanup (VEC_cleanup(mem_range_s), &available);
+
+	  if (VEC_empty (mem_range_s, available)
+	      || VEC_index (mem_range_s, available, 0)->start != memaddr)
+	    {
+	      /* Don't read into the traceframe's available
+		 memory.  */
+	      if (!VEC_empty (mem_range_s, available))
+		{
+		  LONGEST oldlen = len;
+
+		  len = VEC_index (mem_range_s, available, 0)->start - memaddr;
+		  gdb_assert (len <= oldlen);
+		}
+
+	      do_cleanups (old_chain);
+
+	      /* This goes through the topmost target again.  */
+	      res = memory_xfer_live_readonly_partial (ops, object,
+						       readbuf, memaddr, len);
+	      if (res > 0)
+		return res;
+
+	      /* No use trying further, we know some memory starting
+		 at MEMADDR isn't available.  */
+	      return -1;
+	    }
+
+	  /* Don't try to read more than how much is available, in
+	     case the target implements the deprecated QTro packet to
+	     cater for older GDBs (the target's knowledge of read-only
+	     sections may be outdated by now).  */
+	  len = VEC_index (mem_range_s, available, 0)->length;
+
+	  do_cleanups (old_chain);
+	}
+    }
+
   /* Try GDB's internal data cache.  */
   region = lookup_mem_region (memaddr);
   /* region->hi == 0 means there's no upper bound.  */
Index: src/gdb/tracepoint.c
===================================================================
--- src.orig/gdb/tracepoint.c	2011-02-07 11:14:54.026705999 +0000
+++ src/gdb/tracepoint.c	2011-02-07 11:15:14.186706002 +0000
@@ -1927,7 +1927,7 @@ disconnect_tracing (int from_tty)
      confusing upon reconnection.  Just use these calls instead of
      full tfind_1 behavior because we're in the middle of detaching,
      and there's no point to updating current stack frame etc.  */
-  set_traceframe_number (-1);
+  set_current_traceframe (-1);
   set_traceframe_context (NULL);
 }
 
@@ -2931,20 +2931,11 @@ get_traceframe_number (void)
   return traceframe_number;
 }
 
-/* Make the traceframe NUM be the current trace frame.  Does nothing
-   if NUM is already current.  */
-
 void
-set_traceframe_number (int num)
+set_current_traceframe (int num)
 {
   int newnum;
 
-  if (traceframe_number == num)
-    {
-      /* Nothing to do.  */
-      return;
-    }
-
   newnum = target_trace_find (tfind_number, num, 0, 0, NULL);
 
   if (newnum != num)
@@ -2959,6 +2950,15 @@ set_traceframe_number (int num)
   clear_traceframe_info ();
 }
 
+/* Make the traceframe NUM be the current trace frame, and do nothing
+   more.  */
+
+void
+set_traceframe_number (int num)
+{
+  traceframe_number = num;
+}
+
 /* A cleanup used when switching away and back from tfind mode.  */
 
 struct current_traceframe_cleanup
@@ -2972,7 +2972,7 @@ do_restore_current_traceframe_cleanup (v
 {
   struct current_traceframe_cleanup *old = arg;
 
-  set_traceframe_number (old->traceframe_number);
+  set_current_traceframe (old->traceframe_number);
 }
 
 static void
@@ -2995,6 +2995,12 @@ make_cleanup_restore_current_traceframe
 			    restore_current_traceframe_cleanup_dtor);
 }
 
+struct cleanup *
+make_cleanup_restore_traceframe_number (void)
+{
+  return make_cleanup_restore_integer (&traceframe_number);
+}
+
 /* Given a number and address, return an uploaded tracepoint with that
    number, creating if necessary.  */
 
@@ -3247,6 +3253,7 @@ char *trace_filename;
 int trace_fd = -1;
 off_t trace_frames_offset;
 off_t cur_offset;
+int cur_traceframe_number;
 int cur_data_size;
 int trace_regblock_size;
 
@@ -3338,6 +3345,8 @@ tfile_open (char *filename, int from_tty
   ts->disconnected_tracing = 0;
   ts->circular_buffer = 0;
 
+  cur_traceframe_number = -1;
+
   /* Read through a section of newline-terminated lines that
      define things like tracepoints.  */
   i = 0;
@@ -3752,6 +3761,28 @@ tfile_get_traceframe_address (off_t tfra
   return addr;
 }
 
+/* Make tfile's selected traceframe match GDB's selected
+   traceframe.  */
+
+static void
+set_tfile_traceframe (void)
+{
+  int newnum;
+
+  if (cur_traceframe_number == get_traceframe_number ())
+    return;
+
+  /* Avoid recursion, tfile_trace_find calls us again.  */
+  cur_traceframe_number = get_traceframe_number ();
+
+  newnum = target_trace_find (tfind_number,
+			      get_traceframe_number (), 0, 0, NULL);
+
+  /* Should not happen.  If it does, all bets are off.  */
+  if (newnum != get_traceframe_number ())
+    warning (_("could not set tfile's traceframe"));
+}
+
 /* Given a type of search and some parameters, scan the collection of
    traceframes in the file looking for a match.  When found, return
    both the traceframe and tracepoint number, otherwise -1 for
@@ -3768,6 +3799,12 @@ tfile_trace_find (enum trace_find_type t
   off_t offset, tframe_offset;
   ULONGEST tfaddr;
 
+  /* Lookups other than by absolute frame number depend on the current
+     trace selected, so make sure it is correct on the tfile end
+     first.  */
+  if (type != tfind_number)
+    set_tfile_traceframe ();
+
   lseek (trace_fd, trace_frames_offset, SEEK_SET);
   offset = trace_frames_offset;
   while (1)
@@ -3820,6 +3857,7 @@ tfile_trace_find (enum trace_find_type t
 	    *tpp = tpnum;
 	  cur_offset = offset;
 	  cur_data_size = data_size;
+	  cur_traceframe_number = tfnum;
 	  return tfnum;
 	}
       /* Skip past the traceframe's data.  */
@@ -3936,6 +3974,8 @@ tfile_fetch_registers (struct target_ops
   if (!trace_regblock_size)
     return;
 
+  set_tfile_traceframe ();
+
   regs = alloca (trace_regblock_size);
 
   if (traceframe_find_block_type ('R', 0) >= 0)
@@ -4019,7 +4059,9 @@ tfile_xfer_partial (struct target_ops *o
   if (readbuf == NULL)
     error (_("tfile_xfer_partial: trace file is read-only"));
 
-  if (traceframe_number != -1)
+  set_tfile_traceframe ();
+
+ if (traceframe_number != -1)
     {
       int pos = 0;
 
@@ -4102,6 +4144,8 @@ tfile_get_trace_state_variable_value (in
 {
   int pos;
 
+  set_tfile_traceframe ();
+
   pos = 0;
   while ((pos = traceframe_find_block_type ('V', pos)) >= 0)
     {
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2011-02-07 11:14:48.166706003 +0000
+++ src/gdb/remote.c	2011-02-07 11:15:14.196706003 +0000
@@ -1353,6 +1353,10 @@ static ptid_t any_thread_ptid;
 static ptid_t general_thread;
 static ptid_t continue_thread;
 
+/* This the traceframe which we last selected on the remote system.
+   It will be -1 if no traceframe is selected.  */
+static int remote_traceframe_number = -1;
+
 /* Find out if the stub attached to PID (and hence GDB should offer to
    detach instead of killing it when bailing out).  */
 
@@ -4002,6 +4006,7 @@ remote_open_1 (char *name, int from_tty,
 
   general_thread = not_sent_ptid;
   continue_thread = not_sent_ptid;
+  remote_traceframe_number = -1;
 
   /* Probe for ability to use "ThreadInfo" query, as required.  */
   use_threadinfo_query = 1;
@@ -5770,6 +5775,28 @@ fetch_registers_using_g (struct regcache
   process_g_packet (regcache);
 }
 
+/* Make the remote selected traceframe match GDB's selected
+   traceframe.  */
+
+static void
+set_remote_traceframe (void)
+{
+  int newnum;
+
+  if (remote_traceframe_number == get_traceframe_number ())
+    return;
+
+  /* Avoid recursion, remote_trace_find calls us again.  */
+  remote_traceframe_number = get_traceframe_number ();
+
+  newnum = target_trace_find (tfind_number,
+			      get_traceframe_number (), 0, 0, NULL);
+
+  /* Should not happen.  If it does, all bets are off.  */
+  if (newnum != get_traceframe_number ())
+    warning (_("could not set remote traceframe"));
+}
+
 static void
 remote_fetch_registers (struct target_ops *ops,
 			struct regcache *regcache, int regnum)
@@ -5777,6 +5804,7 @@ remote_fetch_registers (struct target_op
   struct remote_arch_state *rsa = get_remote_arch_state ();
   int i;
 
+  set_remote_traceframe ();
   set_general_thread (inferior_ptid);
 
   if (regnum >= 0)
@@ -5934,6 +5962,7 @@ remote_store_registers (struct target_op
   struct remote_arch_state *rsa = get_remote_arch_state ();
   int i;
 
+  set_remote_traceframe ();
   set_general_thread (inferior_ptid);
 
   if (regnum >= 0)
@@ -6502,6 +6531,7 @@ remote_xfer_memory (CORE_ADDR mem_addr,
 {
   int res;
 
+  set_remote_traceframe ();
   set_general_thread (inferior_ptid);
 
   if (should_write)
@@ -8084,6 +8114,7 @@ remote_xfer_partial (struct target_ops *
   char *p2;
   char query_type;
 
+  set_remote_traceframe ();
   set_general_thread (inferior_ptid);
 
   rs = get_remote_state ();
@@ -9972,6 +10003,12 @@ remote_trace_find (enum trace_find_type
   char *p, *reply;
   int target_frameno = -1, target_tracept = -1;
 
+  /* Lookups other than by absolute frame number depend on the current
+     trace selected, so make sure it is correct on the remote end
+     first.  */
+  if (type != tfind_number)
+    set_remote_traceframe ();
+
   p = rs->buf;
   strcpy (p, "QTFrame:");
   p = strchr (p, '\0');
@@ -10009,6 +10046,8 @@ remote_trace_find (enum trace_find_type
 	target_frameno = (int) strtol (p, &reply, 16);
 	if (reply == p)
 	  error (_("Unable to parse trace frame number"));
+	/* Don't update our remote traceframe number cache on failure
+	   to select a remote traceframe.  */
 	if (target_frameno == -1)
 	  return -1;
 	break;
@@ -10029,6 +10068,8 @@ remote_trace_find (enum trace_find_type
       }
   if (tpp)
     *tpp = target_tracept;
+
+  remote_traceframe_number = target_frameno;
   return target_frameno;
 }
 
@@ -10039,6 +10080,8 @@ remote_get_trace_state_variable_value (i
   char *reply;
   ULONGEST uval;
 
+  set_remote_traceframe ();
+
   sprintf (rs->buf, "qTV:%x", tsvnum);
   putpkt (rs->buf);
   reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2011-02-02 18:58:36.277899003 +0000
+++ src/gdb/infrun.c	2011-02-07 11:15:14.206706005 +0000
@@ -2001,7 +2001,7 @@ proceed (CORE_ADDR addr, enum target_sig
   if (non_stop)
     {
       make_cleanup_restore_current_traceframe ();
-      set_traceframe_number (-1);
+      set_current_traceframe (-1);
     }
 
   if (non_stop)
Index: src/gdb/tracepoint.h
===================================================================
--- src.orig/gdb/tracepoint.h	2011-02-07 11:14:54.026705999 +0000
+++ src/gdb/tracepoint.h	2011-02-07 11:15:14.206706005 +0000
@@ -192,9 +192,24 @@ extern void release_static_tracepoint_ma
 extern void (*deprecated_trace_find_hook) (char *arg, int from_tty);
 extern void (*deprecated_trace_start_stop_hook) (int start, int from_tty);
 
-int get_traceframe_number (void);
-void set_traceframe_number (int);
+/* Returns the current traceframe number.  */
+extern int get_traceframe_number (void);
+
+/* Make the traceframe NUM be the current GDB trace frame number, and
+   do nothing more.  In particular, this does not flush the
+   register/frame caches or notify the target about the trace frame
+   change, so that is can be used when we need to momentarily access
+   live memory.  Targets lazily switch their current traceframe to
+   match GDB's traceframe number, at the appropriate times.  */
+extern void set_traceframe_number (int);
+
+/* Make the traceframe NUM be the current trace frame, all the way to
+   the target, and flushes all global state (register/frame caches,
+   etc.).  */
+extern void set_current_traceframe (int num);
+
 struct cleanup *make_cleanup_restore_current_traceframe (void);
+struct cleanup *make_cleanup_restore_traceframe_number (void);
 
 void free_actions (struct breakpoint *);
 extern void validate_actionline (char **, struct breakpoint *);