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]

[unavailable values part 1, 13/17] value unavailable-ness propagation


Accesses to fields and baseclasses of non-collected non-lazy
values are misbehaving.  Ex., with:

 struct small_struct
 {
   int member;
 };

 struct small_struct_b : public small_struct
 {
 };

 small_struct g_smallstruct;
 small_struct_b g_smallstruct_b;

And a tracepoint that collects nothing, notice:

 (gdb) print g_smallstruct
 $31 = {member = <unavailable>}
 (gdb) p $.member
 $32 = 0

And this:

 (gdb) p g_smallstruct_b
 $34 = {<small_struct> = {member = <unavailable>}, <No data fields>}
 (gdb) p (small_struct) $
 $35 = {member = 0}

In both cases, "member = 0" is wrong.  It should be <unavailable>.



This is caused by the fact that there are places that
create a value, and then copies the original value's contents
using a straight memcpy, which forgets to copy the unavailable-ness
from the original value to the new value.

So I've added a couple of functions, value_contents_copy and
value_contents_copy_raw, that know to copy the contents and the 
metadata about the contents.  They differ in a similar way
value_contents and value_contents_raw differ.

Then, I've audited calls to memcpy in core value handling
files (valops.c, value.c, valarith.c), and found a few issues
more:

If g_int is a non-collected int, printing it shows:

 (gdb) print g_int
 $1 = <unavailable>

but, notice this happens:

 (gdb) print { 1, g_int, 3 }
 value is not available

while the correct would be:

 (gdb) print { 1, g_int, 3 }
 $1 = {1, <unavailable>, 3}

Assuming the above issue is fixed, this happens:

 (gdb) print $[1]
 $1 = 0

while the correct is:

 (gdb) print $[1]
 $1 = <unavailable>


The patch below fixes all this.  I've also added testcases
covering all the cases touched by the patch, except for
value_slice, which is only used by the Ada support.

-- 
Pedro Alves

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

	gdb/
	* value.h (value_contents_copy, value_contents_copy_raw): Declare.
	* value.c (value_contents_copy_raw, value_contents_copy): New
	functions.
	(value_primitive_field): Use value_contents_copy_raw instead of
	memcpy.
	* valops.c (value_fetch_lazy): Use value_contents_copy instead of
	memcpy.
	(value_array, value_slice): Ditto.
	* valarith.c (value_subscripted_rvalue): Use
	value_contents_copy_raw instead of memcpy.

	gdb/testsuite/
	* gdb.trace/unavailable.exp (gdb_collect_globals_test): Add new
	tests for building arrays from unavailable values, subscripting
	non-memory rvalue unvailable arrays, and accessing fields or
	baseclasses of non-lazy unavailable values,
	* gdb.trace/unavailable.cc (small_struct, small_struct_b): New
	struct types.
	(g_smallstruct, g_smallstruct_b): New globals.

---
 gdb/testsuite/gdb.trace/unavailable.cc  |   11 ++++
 gdb/testsuite/gdb.trace/unavailable.exp |   25 +++++++++++
 gdb/valarith.c                          |    5 +-
 gdb/valops.c                            |   31 ++++++-------
 gdb/value.c                             |   72 ++++++++++++++++++++++++++++----
 gdb/value.h                             |    6 ++
 6 files changed, 124 insertions(+), 26 deletions(-)

Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h	2011-02-07 13:17:33.696706002 +0000
+++ src/gdb/value.h	2011-02-07 13:18:21.426706000 +0000
@@ -492,6 +492,12 @@ extern struct value *read_var_value (str
 extern struct value *allocate_value (struct type *type);
 extern struct value *allocate_value_lazy (struct type *type);
 extern void allocate_value_contents (struct value *value);
+extern void value_contents_copy (struct value *dst, int dst_offset,
+				 struct value *src, int src_offset,
+				 int length);
+extern void value_contents_copy_raw (struct value *dst, int dst_offset,
+				     struct value *src, int src_offset,
+				     int length);
 
 extern struct value *allocate_repeat_value (struct type *type, int count);
 
Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c	2011-02-07 13:17:33.696706002 +0000
+++ src/gdb/value.c	2011-02-07 13:18:21.426706000 +0000
@@ -701,6 +701,64 @@ value_contents_all (struct value *value)
   return result;
 }
 
+/* Copy LENGTH bytes of SRC value's contents starting at SRC_OFFSET,
+   into DST value's contents, starting at DST_OFFSET.  If unavailable
+   contents are being copied from SRC, the corresponding DST contents
+   are marked unavailable accordingly.  Neither DST nor SRC may be
+   lazy values.  */
+
+void
+value_contents_copy_raw (struct value *dst, int dst_offset,
+			 struct value *src, int src_offset, int length)
+{
+  range_s *r;
+  int i;
+
+  /* A lazy DST would make that this copy operation useless, since as
+     soon as DST's contents were un-lazied (by a later value_contents
+     call, say), the contents would be overwritten.  A lazy SRC would
+     mean we'd be copying garbage.  */
+  gdb_assert (!dst->lazy && !src->lazy);
+
+  /* Copy the data.  */
+  memcpy (value_contents_all_raw (dst) + dst_offset,
+	  value_contents_all_raw (src) + src_offset,
+	  length);
+
+  /* Copy the meta-data, adjusted.  */
+  for (i = 0; VEC_iterate (range_s, src->unavailable, i, r); i++)
+    {
+      ULONGEST h, l;
+
+      l = max (r->offset, src_offset);
+      h = min (r->offset + r->length, src_offset + length);
+
+      if (l < h)
+	mark_value_bytes_unavailable (dst,
+				      dst_offset + (l - src_offset),
+				      h - l);
+    }
+}
+
+/* Copy LENGTH bytes of SRC value's contents starting at SRC_OFFSET
+   byte, into DST value's contents, starting at DST_OFFSET.  If
+   unavailable contents are being copied from SRC, the corresponding
+   DST contents are marked unavailable accordingly.  DST must not be
+   lazy.  If SRC is lazy, it will be fetched now.  If SRC is not valid
+   (is optimized out), an error is thrown.  */
+
+void
+value_contents_copy (struct value *dst, int dst_offset,
+		     struct value *src, int src_offset, int length)
+{
+  require_not_optimized_out (src);
+
+  if (src->lazy)
+    value_fetch_lazy (src);
+
+  value_contents_copy_raw (dst, dst_offset, src, src_offset, length);
+}
+
 int
 value_lazy (struct value *value)
 {
@@ -2274,8 +2332,8 @@ value_primitive_field (struct value *arg
   else if (fieldno < TYPE_N_BASECLASSES (arg_type))
     {
       /* This field is actually a base subobject, so preserve the
-         entire object's contents for later references to virtual
-         bases, etc.  */
+	 entire object's contents for later references to virtual
+	 bases, etc.  */
 
       /* Lazy register values with offsets are not supported.  */
       if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
@@ -2286,8 +2344,8 @@ value_primitive_field (struct value *arg
       else
 	{
 	  v = allocate_value (value_enclosing_type (arg1));
-	  memcpy (value_contents_all_raw (v), value_contents_all_raw (arg1),
-		  TYPE_LENGTH (value_enclosing_type (arg1)));
+	  value_contents_copy_raw (v, 0, arg1, 0,
+				   TYPE_LENGTH (value_enclosing_type (arg1)));
 	}
       v->type = type;
       v->offset = value_offset (arg1);
@@ -2308,9 +2366,9 @@ value_primitive_field (struct value *arg
       else
 	{
 	  v = allocate_value (type);
-	  memcpy (value_contents_raw (v),
-		  value_contents_raw (arg1) + offset,
-		  TYPE_LENGTH (type));
+	  value_contents_copy_raw (v, value_embedded_offset (v),
+				   arg1, value_embedded_offset (arg1) + offset,
+				   TYPE_LENGTH (type));
 	}
       v->offset = (value_offset (arg1) + offset
 		   + value_embedded_offset (arg1));
Index: src/gdb/valops.c
===================================================================
--- src.orig/gdb/valops.c	2011-02-07 13:17:37.996706003 +0000
+++ src/gdb/valops.c	2011-02-07 13:18:21.426706000 +0000
@@ -1044,12 +1044,16 @@ value_fetch_lazy (struct value *val)
       if (value_lazy (new_val))
 	value_fetch_lazy (new_val);
 
-      /* If the register was not saved, mark it unavailable.  */
+      /* If the register was not saved, mark it optimized out.  */
       if (value_optimized_out (new_val))
 	set_value_optimized_out (val, 1);
       else
-	memcpy (value_contents_raw (val), value_contents (new_val),
-		TYPE_LENGTH (type));
+	{
+	  set_value_lazy (val, 0);
+	  value_contents_copy (val, value_embedded_offset (val),
+			       new_val, value_embedded_offset (new_val),
+			       TYPE_LENGTH (type));
+	}
 
       if (frame_debug)
 	{
@@ -1765,9 +1769,8 @@ value_ind (struct value *arg1)
   return 0;			/* For lint -- never reached.  */
 }
 
-/* Create a value for an array by allocating space in GDB, copying
-   copying the data into that space, and then setting up an array
-   value.
+/* Create a value for an array by allocating space in GDB, copying the
+   data into that space, and then setting up an array value.
 
    The array bounds are set from LOWBOUND and HIGHBOUND, and the array
    is populated from the values passed in ELEMVEC.
@@ -1809,11 +1812,8 @@ value_array (int lowbound, int highbound
     {
       val = allocate_value (arraytype);
       for (idx = 0; idx < nelem; idx++)
-	{
-	  memcpy (value_contents_all_raw (val) + (idx * typelength),
-		  value_contents_all (elemvec[idx]),
-		  typelength);
-	}
+	value_contents_copy (val, idx * typelength, elemvec[idx], 0,
+			     typelength);
       return val;
     }
 
@@ -1822,9 +1822,7 @@ value_array (int lowbound, int highbound
 
   val = allocate_value (arraytype);
   for (idx = 0; idx < nelem; idx++)
-    memcpy (value_contents_writeable (val) + (idx * typelength),
-	    value_contents_all (elemvec[idx]),
-	    typelength);
+    value_contents_copy (val, idx * typelength, elemvec[idx], 0, typelength);
   return val;
 }
 
@@ -3711,9 +3709,8 @@ value_slice (struct value *array, int lo
       else
 	{
 	  slice = allocate_value (slice_type);
-	  memcpy (value_contents_writeable (slice),
-		  value_contents (array) + offset,
-		  TYPE_LENGTH (slice_type));
+	  value_contents_copy (slice, 0, array, offset,
+			       TYPE_LENGTH (slice_type));
 	}
 
       set_value_component_location (slice, array);
Index: src/gdb/valarith.c
===================================================================
--- src.orig/gdb/valarith.c	2011-02-07 11:19:38.396706000 +0000
+++ src/gdb/valarith.c	2011-02-07 13:18:21.426706000 +0000
@@ -210,8 +210,9 @@ value_subscripted_rvalue (struct value *
   else
     {
       v = allocate_value (elt_type);
-      memcpy (value_contents_writeable (v),
-	      value_contents (array) + elt_offs, elt_size);
+      value_contents_copy (v, value_embedded_offset (v),
+			   array, value_embedded_offset (array) + elt_offs,
+			   elt_size);
     }
 
   set_value_component_location (v, array);
Index: src/gdb/testsuite/gdb.trace/unavailable.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/unavailable.exp	2011-02-07 13:17:39.086706006 +0000
+++ src/gdb/testsuite/gdb.trace/unavailable.exp	2011-02-07 13:18:21.426706000 +0000
@@ -214,6 +214,31 @@ proc gdb_collect_globals_test { } {
 
     gdb_test "p \$__" " = <unavailable>" "last examined value was <unavailable>"
 
+    # This tests that building the array does not require accessing
+    # g_int's contents.
+    gdb_test "print { 1, g_int, 3 }" \
+	" = \\{1, <unavailable>, 3\\}" \
+	"build array from unavailable value"
+
+    # Note, depends on previous test.
+    gdb_test "print \$\[1\]" \
+	" = <unavailable>" \
+	"subscript a non-memory rvalue array, accessing an unvailable element"
+
+    # Access a field of a non-lazy value, making sure the
+    # unavailable-ness is propagated.  History values are easy
+    # non-lazy values, so use those.  The first test just sets up for
+    # the second.
+    gdb_test "print g_smallstruct" " = \\{member = <unavailable>\\}"
+    gdb_test "print \$.member" " = <unavailable>"
+
+    # Cast to baseclass, checking the unavailable-ness is propagated.
+    gdb_test "print (small_struct) g_smallstruct_b" " = \\{member = <unavailable>\\}"
+
+    # Same cast, but starting from a non-lazy, value.
+    gdb_test "print g_smallstruct_b" " = \\{<small_struct> = \\{member = <unavailable>\\}, <No data fields>\\}"
+    gdb_test "print (small_struct) \$" " = \\{member = <unavailable>\\}"
+
     gdb_test "tfind none" \
 	"#0  end .*" \
 	"cease trace debugging"
Index: src/gdb/testsuite/gdb.trace/unavailable.cc
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/unavailable.cc	2011-02-07 13:17:39.076706003 +0000
+++ src/gdb/testsuite/gdb.trace/unavailable.cc	2011-02-07 13:18:21.436706001 +0000
@@ -30,6 +30,15 @@ typedef struct TEST_STRUCT {
   double memberd;
 } test_struct;
 
+struct small_struct
+{
+  int member;
+};
+
+struct small_struct_b : public small_struct
+{
+};
+
 typedef int test_array [4];
 
 /* Global variables to be collected.  */
@@ -41,6 +50,8 @@ double       globald;
 test_struct  globalstruct;
 test_struct *globalp;
 int          globalarr[16];
+small_struct g_smallstruct;
+small_struct_b g_smallstruct_b;
 
 /* Strings.  */
 


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