This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Rely on beneath to do partial xfer from tfile target


On 03/22/2013 05:49 PM, Yao Qi wrote:
> I agree that <unavailable> is the best choice.  The "non-const-
> not-collected data" should be <unavailable> also, right?  If so, I'll
> post a patch to change "Cannot access memory at address" to
> "<unavailable>".

In valops.c:read_value_memory, we have

      if (get_traceframe_number () < 0
	  || !traceframe_available_memory (&available_memory, memaddr, length))
	{
	}
      else
	{
	  /* Fallback to reading from read-only sections.  */
	}

The code is correct to remote target, however it is not perfect to
tfile target when the current traceframe is not selected
(get_traceframe_number returns -1).  In this case, GDB should fall back
to the 'else' branch to read from read-only sections, so the fix could
be 'don't check the return value of get_traceframe_number and let the
target decide where to read data from (live inferior or read-only
sections)'.

We don't check get_traceframe_number here, means target_ops method
'to_traceframe_info' will be called on many targets, so we change its
default to 'return_zero' instead of 'tcomplain'.

Fortunately, we make use of an inconsistency of 'to_traceframe_info'
of remote target and tfile target.  That is, when current traceframe
number is -1, remote_traceframe_info returns NULL and
tfile_traceframe_info return a empty traceframe_info pointer.  In this
way, the usage of 'to_traceframe_info' is extended, so I add some
comments to it.

The rationale of this patch is to teach 'to_traceframe_info' returns
something different so that GDB is able to tell the differences of
targets when current traceframe is not selected.  The other caller to
traceframe_available_memory is guarded by 'get_traceframe_number () !=
-1', so changes here don't affect it.  Regression tested on
x86_64-linux.  Is it OK?

-- 
Yao (éå)

gdb:

2013-04-01  Yao Qi  <yao@codesourcery.com>

	* target.c (update_current_target): Change the default action
	of 'to_traceframe_info' from tcomplain to return_zero.
	* target.h (struct target_ops) <to_traceframe_info>: Add more
	comments.
	* valops.c (read_value_memory): Call
	traceframe_available_memory unconditionally.

gdb/testsuite:

2013-04-01  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/read-memory.exp (test_from_remote): Update test.
	(teset_from_exec): Likewise.
---
 gdb/target.c                            |    2 +-
 gdb/target.h                            |   15 ++++++++++++---
 gdb/testsuite/gdb.trace/read-memory.exp |   10 ++++------
 gdb/valops.c                            |    3 +--
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index 9193c97..97da6b1 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -948,7 +948,7 @@ update_current_target (void)
 	    tcomplain);
   de_fault (to_traceframe_info,
 	    (struct traceframe_info * (*) (void))
-	    tcomplain);
+	    return_zero);
   de_fault (to_supports_evaluation_of_breakpoint_conditions,
 	    (int (*) (void))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index ee3fbff..072117e 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -850,9 +850,18 @@ struct target_ops
       (const char *id);
 
     /* Return a traceframe info object describing the current
-       traceframe's contents.  This method should not cache data;
-       higher layers take care of caching, invalidating, and
-       re-fetching when necessary.  */
+       traceframe's contents.  If the target doesn't support
+       traceframe info, return NULL.  If the current traceframe is not
+       selected (the current traceframe number is -1), the target can
+       choose to return either NULL or an empty traceframe info.  If
+       NULL is returned, for example in remote target, GDB will read
+       from the live inferior.  If an empty traceframe info is
+       returned, for example in tfile target, which means the
+       traceframe info is available, but the requested memory is not
+       available in it.  GDB will try to see if the requested memory
+       is available in the read-only sections.  This method should not
+       cache data; higher layers take care of caching, invalidating,
+       and re-fetching when necessary.  */
     struct traceframe_info *(*to_traceframe_info) (void);
 
     /* Ask the target to use or not to use agent according to USE.  Return 1
diff --git a/gdb/testsuite/gdb.trace/read-memory.exp b/gdb/testsuite/gdb.trace/read-memory.exp
index e7c0af3..21b0e7e 100644
--- a/gdb/testsuite/gdb.trace/read-memory.exp
+++ b/gdb/testsuite/gdb.trace/read-memory.exp
@@ -104,9 +104,8 @@ proc test_from_remote { target } {
 	}
 
 	with_test_prefix "/wo setting traceframe" {
-	    gdb_test "print testglob" "Cannot access memory at address.*"
-	    gdb_test "print testglob_not_collected" \
-		"Cannot access memory at address.*"
+	    gdb_test "print testglob" " = <unavailable>"
+	    gdb_test "print testglob_not_collected" " = <unavailable>"
 	    gdb_test "print constglob" " = 10000"
 	    gdb_test "print constglob_not_collected" " = 100"
 	}
@@ -141,9 +140,8 @@ proc teset_from_exec { target } {
 	"change to ${target} target"
 
     with_test_prefix "exec to ${target} /wo setting traceframe" {
-	gdb_test "print testglob" "Cannot access memory at address.*"
-	gdb_test "print testglob_not_collected" \
-	    "Cannot access memory at address.*"
+	gdb_test "print testglob" " = <unavailable>"
+	gdb_test "print testglob_not_collected" " = <unavailable>"
 	gdb_test "print constglob" " = 10000"
 	gdb_test "print constglob_not_collected" " = 100"
     }
diff --git a/gdb/valops.c b/gdb/valops.c
index 93c09d8..80b9995 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1117,8 +1117,7 @@ read_value_memory (struct value *val, int embedded_offset,
     {
       VEC(mem_range_s) *available_memory;
 
-      if (get_traceframe_number () < 0
-	  || !traceframe_available_memory (&available_memory, memaddr, length))
+      if (!traceframe_available_memory (&available_memory, memaddr, length))
 	{
 	  if (stack)
 	    read_stack (memaddr, buffer, length);
-- 
1.7.7.6


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]