[PATCH v3 1/3] gdbsupport: add array_view copy function

[PATCH v3 1/3] gdbsupport: add array_view copy function

[Simon Marchi]

New in v3:

 - Fix stale comment
 - Change order of parameters
 - Don't copy if src == dest

An assertion was recently added to array_view::operator[] to ensure we
don't do out of bounds accesses.  However, when the array_view is copied
to or from using memcpy, it bypasses that safety.

To address this, add a `copy` free function that copies data from an
array view to another, ensuring that the destination and source array
views have the same size.  When copying to or from parts of an
array_view, we are expected to use gdb::array_view::slice, which does
its own bounds check.  With all that, any copy operation that goes out
of bounds should be caught by an assertion at runtime.

copy is implemented using std::copy and std::copy_backward, which, at
least on libstdc++, appears to pick memmove when copying trivial data.
So in the end there shouldn't be much difference vs using a bare memcpy,
as we do right now.  When copying non-trivial data, std::copy and
std::copy_backward assigns each element in a loop.

To properly support overlapping ranges, we must use std::copy or
std::copy_backward, depending on whether the destination is before the
source or vice-versa.  std::copy and std::copy_backward don't support
copying exactly overlapping ranges (where the source range is equal to
the destination range).  But in this case, no copy is needed anyway, so
we do nothing.

The order of parameters of the new copy function is based on std::copy
and std::copy_backward, where the source comes before the destination.

Change a few randomly selected spots to use the new function, to show
how it can be used.

Add a test for the new function, testing both with arrays of a trivial
type (int) and of a non-trivial type (foo).  Test non-overlapping
ranges as well as three kinds of overlapping ranges: source before dest,
dest before source, and dest == source.

Change-Id: Ibeaca04e0028410fd44ce82f72e60058d6230a03
---
 gdb/ada-lang.c                       |  17 ++---
 gdb/dwarf2/expr.c                    |   4 +-
 gdb/unittests/array-view-selftests.c | 104 +++++++++++++++++++++++++++
 gdb/valarith.c                       |  49 +++++++------
 gdb/valops.c                         |  35 ++++-----
 gdb/value.c                          |  22 +++---
 gdbsupport/array-view.h              |  15 ++++
 7 files changed, 184 insertions(+), 62 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index b84e10fd029f..c6cefe9d8e89 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -2586,9 +2586,7 @@ ada_value_assign (struct value *toval, struct value *fromval)
       write_memory_with_notification (to_addr, buffer, len);
 
       val = value_copy (toval);
-      memcpy (value_contents_raw (val).data (),
-	      value_contents (fromval).data (),
-	      TYPE_LENGTH (type));
+      copy (value_contents (fromval), value_contents_raw (val));
       deprecated_set_value_type (val, type);
 
       return val;
@@ -4184,9 +4182,7 @@ ada_convert_actual (struct value *actual, struct type *formal_type0)
 
 	      actual_type = ada_check_typedef (value_type (actual));
 	      val = allocate_value (actual_type);
-	      memcpy ((char *) value_contents_raw (val).data (),
-		      (char *) value_contents (actual).data (),
-		      TYPE_LENGTH (actual_type));
+	      copy (value_contents (actual), value_contents_raw (val));
 	      actual = ensure_lval (val);
 	    }
 	  result = value_addr (actual);
@@ -8898,7 +8894,6 @@ ada_promote_array_of_integrals (struct type *type, struct value *val)
 {
   struct type *elt_type = TYPE_TARGET_TYPE (type);
   LONGEST lo, hi;
-  struct value *res;
   LONGEST i;
 
   /* Verify that both val and type are arrays of scalars, and
@@ -8914,16 +8909,16 @@ ada_promote_array_of_integrals (struct type *type, struct value *val)
   if (!get_array_bounds (type, &lo, &hi))
     error (_("unable to determine array bounds"));
 
-  res = allocate_value (type);
+  value *res = allocate_value (type);
+  gdb::array_view<gdb_byte> res_contents = value_contents_writeable (res);
 
   /* Promote each array element.  */
   for (i = 0; i < hi - lo + 1; i++)
     {
       struct value *elt = value_cast (elt_type, value_subscript (val, lo + i));
+      int elt_len = TYPE_LENGTH (elt_type);
 
-      memcpy ((value_contents_writeable (res).data ()
-	       + (i * TYPE_LENGTH (elt_type))),
-	      value_contents_all (elt).data (), TYPE_LENGTH (elt_type));
+      copy (value_contents_all (elt), res_contents.slice (elt_len * i, elt_len));
     }
 
   return res;
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 652161955d52..592dbe19d562 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -1037,8 +1037,8 @@ dwarf_expr_context::fetch_result (struct type *type, struct type *subobj_type,
 	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG)
 	      subobj_offset += n - max;
 
-	    memcpy (value_contents_raw (retval).data (),
-		    value_contents_all (val).data () + subobj_offset, len);
+	    copy (value_contents_all (val).slice (subobj_offset, len),
+		  value_contents_raw (retval));
 	  }
 	  break;
 
diff --git a/gdb/unittests/array-view-selftests.c b/gdb/unittests/array-view-selftests.c
index 9df48db39129..998bd3a57812 100644
--- a/gdb/unittests/array-view-selftests.c
+++ b/gdb/unittests/array-view-selftests.c
@@ -546,6 +546,108 @@ run_tests ()
   }
 }
 
+template <typename T>
+void
+run_copy_test ()
+{
+  /* Test non-overlapping copy.  */
+  {
+    const std::vector<T> src_v = {1, 2, 3, 4};
+    std::vector<T> dest_v (4, -1);
+
+    SELF_CHECK (dest_v != src_v);
+    copy (gdb::array_view<const T> (src_v), gdb::array_view<T> (dest_v));
+    SELF_CHECK (dest_v == src_v);
+  }
+
+  /* Test overlapping copy, where the source is before the destination.  */
+  {
+    std::vector<T> vec = {1, 2, 3, 4, 5, 6, 7, 8};
+    gdb::array_view<T> v = vec;
+
+    copy (v.slice (1, 4),
+	  v.slice (2, 4));
+
+    std::vector<T> expected = {1, 2, 2, 3, 4, 5, 7, 8};
+    SELF_CHECK (vec == expected);
+  }
+
+  /* Test overlapping copy, where the source is after the destination.  */
+  {
+    std::vector<T> vec = {1, 2, 3, 4, 5, 6, 7, 8};
+    gdb::array_view<T> v = vec;
+
+    copy (v.slice (2, 4),
+	  v.slice (1, 4));
+
+    std::vector<T> expected = {1, 3, 4, 5, 6, 6, 7, 8};
+    SELF_CHECK (vec == expected);
+  }
+
+  /* Test overlapping copy, where the source is the same as the destination.  */
+  {
+    std::vector<T> vec = {1, 2, 3, 4, 5, 6, 7, 8};
+    gdb::array_view<T> v = vec;
+
+    copy (v.slice (2, 4),
+	  v.slice (2, 4));
+
+    std::vector<T> expected = {1, 2, 3, 4, 5, 6, 7, 8};
+    SELF_CHECK (vec == expected);
+  }
+}
+
+/* Class with a non-trivial copy assignment operator, used to test the
+   array_view copy function.  */
+struct foo
+{
+  /* Can be implicitly constructed from an int, such that we can use the same
+     templated test function to test against array_view<int> and
+     array_view<foo>.  */
+  foo (int n)
+    : n (n)
+  {}
+
+  /* Needed to avoid -Wdeprecated-copy-with-user-provided-copy error with
+     Clang.  */
+  foo (const foo &other) = default;
+
+  void operator= (const foo &other)
+  {
+    this->n = other.n;
+    this->n_assign_op_called++;
+  }
+
+  bool operator==(const foo &other) const
+  {
+    return this->n == other.n;
+  }
+
+  int n;
+
+  /* Number of times the assignment operator has been called.  */
+  static int n_assign_op_called;
+};
+
+int foo::n_assign_op_called = 0;
+
+/* Test the array_view copy free function.  */
+
+static void
+run_copy_tests ()
+{
+  /* Test with a trivial type.  */
+  run_copy_test<int> ();
+
+  /* Test with a non-trivial type.  */
+  foo::n_assign_op_called = 0;
+  run_copy_test<foo> ();
+
+  /* Make sure that for the non-trivial type foo, the assignment operator was
+     called an amount of times that makes sense.  */
+  SELF_CHECK (foo::n_assign_op_called == 12);
+}
+
 } /* namespace array_view_tests */
 } /* namespace selftests */
 
@@ -555,4 +657,6 @@ _initialize_array_view_selftests ()
 {
   selftests::register_test ("array_view",
 			    selftests::array_view_tests::run_tests);
+  selftests::register_test ("array_view-copy",
+			    selftests::array_view_tests::run_copy_tests);
 }
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 140ef448137e..496570762799 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -1531,7 +1531,7 @@ value_vector_widen (struct value *scalar_value, struct type *vector_type)
 {
   /* Widen the scalar to a vector.  */
   struct type *eltype, *scalar_type;
-  struct value *val, *elval;
+  struct value *elval;
   LONGEST low_bound, high_bound;
   int i;
 
@@ -1554,11 +1554,14 @@ value_vector_widen (struct value *scalar_value, struct type *vector_type)
       && !value_equal (elval, scalar_value))
     error (_("conversion of scalar to vector involves truncation"));
 
-  val = allocate_value (vector_type);
+  value *val = allocate_value (vector_type);
+  gdb::array_view<gdb_byte> val_contents = value_contents_writeable (val);
+  int elt_len = TYPE_LENGTH (eltype);
+
   for (i = 0; i < high_bound - low_bound + 1; i++)
     /* Duplicate the contents of elval into the destination vector.  */
-    memcpy (value_contents_writeable (val).data () + (i * TYPE_LENGTH (eltype)),
-	    value_contents_all (elval).data (), TYPE_LENGTH (eltype));
+    copy (value_contents_all (elval),
+	  val_contents.slice (i * elt_len, elt_len));
 
   return val;
 }
@@ -1569,7 +1572,6 @@ value_vector_widen (struct value *scalar_value, struct type *vector_type)
 static struct value *
 vector_binop (struct value *val1, struct value *val2, enum exp_opcode op)
 {
-  struct value *val, *tmp, *mark;
   struct type *type1, *type2, *eltype1, *eltype2;
   int t1_is_vec, t2_is_vec, elsize, i;
   LONGEST low_bound1, high_bound1, low_bound2, high_bound2;
@@ -1599,15 +1601,15 @@ vector_binop (struct value *val1, struct value *val2, enum exp_opcode op)
       || low_bound1 != low_bound2 || high_bound1 != high_bound2)
     error (_("Cannot perform operation on vectors with different types"));
 
-  val = allocate_value (type1);
-  mark = value_mark ();
+  value *val = allocate_value (type1);
+  gdb::array_view<gdb_byte> val_contents = value_contents_writeable (val);
+  value *mark = value_mark ();
   for (i = 0; i < high_bound1 - low_bound1 + 1; i++)
     {
-      tmp = value_binop (value_subscript (val1, i),
-			 value_subscript (val2, i), op);
-      memcpy (value_contents_writeable (val).data () + i * elsize,
-	      value_contents_all (tmp).data (),
-	      elsize);
+      value *tmp = value_binop (value_subscript (val1, i),
+				value_subscript (val2, i), op);
+      copy (value_contents_all (tmp),
+	    val_contents.slice (i * elsize, elsize));
      }
   value_free_to_mark (mark);
 
@@ -1888,7 +1890,7 @@ value_neg (struct value *arg1)
     return value_binop (value_zero (type, not_lval), arg1, BINOP_SUB);
   else if (type->code () == TYPE_CODE_ARRAY && type->is_vector ())
     {
-      struct value *tmp, *val = allocate_value (type);
+      struct value *val = allocate_value (type);
       struct type *eltype = check_typedef (TYPE_TARGET_TYPE (type));
       int i;
       LONGEST low_bound, high_bound;
@@ -1896,12 +1898,14 @@ value_neg (struct value *arg1)
       if (!get_array_bounds (type, &low_bound, &high_bound))
 	error (_("Could not determine the vector bounds"));
 
+      gdb::array_view<gdb_byte> val_contents = value_contents_writeable (val);
+      int elt_len = TYPE_LENGTH (eltype);
+
       for (i = 0; i < high_bound - low_bound + 1; i++)
 	{
-	  tmp = value_neg (value_subscript (arg1, i));
-	  memcpy ((value_contents_writeable (val).data ()
-		   + i * TYPE_LENGTH (eltype)),
-		  value_contents_all (tmp).data (), TYPE_LENGTH (eltype));
+	  value *tmp = value_neg (value_subscript (arg1, i));
+	  copy (value_contents_all (tmp),
+		val_contents.slice (i * elt_len, elt_len));
 	}
       return val;
     }
@@ -1931,7 +1935,6 @@ value_complement (struct value *arg1)
     val = value_from_longest (type, ~value_as_long (arg1));
   else if (type->code () == TYPE_CODE_ARRAY && type->is_vector ())
     {
-      struct value *tmp;
       struct type *eltype = check_typedef (TYPE_TARGET_TYPE (type));
       int i;
       LONGEST low_bound, high_bound;
@@ -1940,12 +1943,14 @@ value_complement (struct value *arg1)
 	error (_("Could not determine the vector bounds"));
 
       val = allocate_value (type);
+      gdb::array_view<gdb_byte> val_contents = value_contents_writeable (val);
+      int elt_len = TYPE_LENGTH (eltype);
+
       for (i = 0; i < high_bound - low_bound + 1; i++)
 	{
-	  tmp = value_complement (value_subscript (arg1, i));
-	  memcpy ((value_contents_writeable (val).data ()
-		   + i * TYPE_LENGTH (eltype)),
-		  value_contents_all (tmp).data (), TYPE_LENGTH (eltype));
+	  value *tmp = value_complement (value_subscript (arg1, i));
+	  copy (value_contents_all (tmp),
+		val_contents.slice (i * elt_len, elt_len));
 	}
     }
   else if (type->code () == TYPE_CODE_COMPLEX)
diff --git a/gdb/valops.c b/gdb/valops.c
index c552e828a947..ca71c128de9c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -954,18 +954,19 @@ value_one (struct type *type)
       struct type *eltype = check_typedef (TYPE_TARGET_TYPE (type1));
       int i;
       LONGEST low_bound, high_bound;
-      struct value *tmp;
 
       if (!get_array_bounds (type1, &low_bound, &high_bound))
 	error (_("Could not determine the vector bounds"));
 
       val = allocate_value (type);
+      gdb::array_view<gdb_byte> val_contents = value_contents_writeable (val);
+      int elt_len = TYPE_LENGTH (eltype);
+
       for (i = 0; i < high_bound - low_bound + 1; i++)
 	{
-	  tmp = value_one (eltype);
-	  memcpy ((value_contents_writeable (val).data ()
-		   + i * TYPE_LENGTH (eltype)),
-		  value_contents_all (tmp).data (), TYPE_LENGTH (eltype));
+	  value *tmp = value_one (eltype);
+	  copy (value_contents_all (tmp),
+		val_contents.slice (i * elt_len, elt_len));
 	}
     }
   else
@@ -1342,8 +1343,7 @@ value_assign (struct value *toval, struct value *fromval)
      implies the returned value is not lazy, even if TOVAL was.  */
   val = value_copy (toval);
   set_value_lazy (val, 0);
-  memcpy (value_contents_raw (val).data (), value_contents (fromval).data (),
-	  TYPE_LENGTH (type));
+  copy (value_contents (fromval), value_contents_raw (val));
 
   /* We copy over the enclosing type and pointed-to offset from FROMVAL
      in the case of pointer types.  For object types, the enclosing type
@@ -4058,10 +4058,13 @@ value_literal_complex (struct value *arg1,
   arg1 = value_cast (real_type, arg1);
   arg2 = value_cast (real_type, arg2);
 
-  memcpy (value_contents_raw (val).data (),
-	  value_contents (arg1).data (), TYPE_LENGTH (real_type));
-  memcpy (value_contents_raw (val).data () + TYPE_LENGTH (real_type),
-	  value_contents (arg2).data (), TYPE_LENGTH (real_type));
+  int len = TYPE_LENGTH (real_type);
+
+  copy (value_contents (arg1),
+	value_contents_raw (val).slice (0, len));
+  copy (value_contents (arg2),
+	value_contents_raw (val).slice (len, len));
+
   return val;
 }
 
@@ -4102,12 +4105,12 @@ cast_into_complex (struct type *type, struct value *val)
       struct type *val_real_type = TYPE_TARGET_TYPE (value_type (val));
       struct value *re_val = allocate_value (val_real_type);
       struct value *im_val = allocate_value (val_real_type);
+      int len = TYPE_LENGTH (val_real_type);
 
-      memcpy (value_contents_raw (re_val).data (),
-	      value_contents (val).data (), TYPE_LENGTH (val_real_type));
-      memcpy (value_contents_raw (im_val).data (),
-	      value_contents (val).data () + TYPE_LENGTH (val_real_type),
-	      TYPE_LENGTH (val_real_type));
+      copy (value_contents (val).slice (0, len),
+	    value_contents_raw (re_val));
+      copy (value_contents (val).slice (len, len),
+	    value_contents_raw (im_val));
 
       return value_literal_complex (re_val, im_val, type);
     }
diff --git a/gdb/value.c b/gdb/value.c
index 7649b029f91b..4e66329e82c5 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1344,9 +1344,13 @@ value_contents_copy_raw (struct value *dst, LONGEST dst_offset,
 					     TARGET_CHAR_BIT * length));
 
   /* Copy the data.  */
-  memcpy (value_contents_all_raw (dst).data () + dst_offset * unit_size,
-	  value_contents_all_raw (src).data () + src_offset * unit_size,
-	  length * unit_size);
+  gdb::array_view<gdb_byte> dst_contents
+    = value_contents_all_raw (dst).slice (dst_offset * unit_size,
+					  length * unit_size);
+  gdb::array_view<const gdb_byte> src_contents
+    = value_contents_all_raw (src).slice (src_offset * unit_size,
+					  length * unit_size);
+  copy (src_contents, dst_contents);
 
   /* Copy the meta-data, adjusted.  */
   src_bit_offset = src_offset * unit_size * HOST_CHAR_BIT;
@@ -1721,13 +1725,11 @@ value_copy (struct value *arg)
   val->stack = arg->stack;
   val->is_zero = arg->is_zero;
   val->initialized = arg->initialized;
+
   if (!value_lazy (val))
-    {
-      memcpy (value_contents_all_raw (val).data (),
-	      value_contents_all_raw (arg).data (),
-	      TYPE_LENGTH (value_enclosing_type (arg)));
+    copy (value_contents_all_raw (arg),
+	  value_contents_all_raw (val));
 
-    }
   val->unavailable = arg->unavailable;
   val->optimized_out = arg->optimized_out;
   val->parent = arg->parent;
@@ -1772,9 +1774,7 @@ value_non_lval (struct value *arg)
       struct type *enc_type = value_enclosing_type (arg);
       struct value *val = allocate_value (enc_type);
 
-      memcpy (value_contents_all_raw (val).data (),
-	      value_contents_all (arg).data (),
-	      TYPE_LENGTH (enc_type));
+      copy (value_contents_all (arg), value_contents_all_raw (val));
       val->type = arg->type;
       set_value_embedded_offset (val, value_embedded_offset (arg));
       set_value_pointed_to_offset (val, value_pointed_to_offset (arg));
diff --git a/gdbsupport/array-view.h b/gdbsupport/array-view.h
index edf66559e2d9..2590bc96061c 100644
--- a/gdbsupport/array-view.h
+++ b/gdbsupport/array-view.h
@@ -19,6 +19,7 @@
 #define COMMON_ARRAY_VIEW_H
 
 #include "traits.h"
+#include <algorithm>
 #include <type_traits>
 
 /* An array_view is an abstraction that provides a non-owning view
@@ -206,6 +207,20 @@ class array_view
   size_type m_size;
 };
 
+/* Copy the contents referenced by the array view SRC to the array view DEST.
+
+   The two array views must have the same length.  */
+
+template <typename T, typename U>
+void copy (gdb::array_view<U> src, gdb::array_view<T> dest)
+{
+  gdb_assert (dest.size () == src.size ());
+  if (dest.data () < src.data ())
+    std::copy (src.begin (), src.end (), dest.begin ());
+  else if (dest.data () > src.data ())
+    std::copy_backward (src.begin (), src.end (), dest.end ());
+}
+
 /* Compare LHS and RHS for (deep) equality.  That is, whether LHS and
    RHS have the same sizes, and whether each pair of elements of LHS
    and RHS at the same position compares equal.  */
-- 
2.33.1

[PATCH v3 2/3] gdb: make extract_integer take an array_view

[Simon Marchi]

I think it would make sense for extract_integer, extract_signed_integer
and extract_unsigned_integer to take an array_view.  This way, when we
extract an integer, we can validate that we don't overflow the buffer
passed by the caller (e.g. ask to extract a 4-byte integer but pass a
2-byte buffer).

 - Change extract_integer to take an array_view
 - Add overloads of extract_signed_integer and extract_unsigned_integer
   that take array_views.  Keep the existing versions so we don't
   need to change all callers, but make them call the array_view
   versions.

This shortens some places like:

  result = extract_unsigned_integer (value_contents (result_val).data (),
				     TYPE_LENGTH (value_type (result_val)),
				     byte_order);

into

  result = extract_unsigned_integer (value_contents (result_val), byte_order);

value_contents returns an array view that is of length
`TYPE_LENGTH (value_type (result_val))` already, so the length is
implicitly communicated through the array view.

Change-Id: Ic1c1f98c88d5c17a8486393af316f982604d6c95
---
 gdb/amd64-linux-tdep.c              |  2 +-
 gdb/defs.h                          | 23 ++++++++++++++++---
 gdb/dwarf2/expr.c                   |  7 ++----
 gdb/fbsd-tdep.c                     |  6 ++---
 gdb/findvar.c                       | 35 ++++++++++++++---------------
 gdb/frame.c                         |  4 +---
 gdb/frv-tdep.c                      |  2 +-
 gdb/hppa-bsd-tdep.c                 |  2 +-
 gdb/hppa-linux-tdep.c               |  2 +-
 gdb/i386-linux-tdep.c               |  2 +-
 gdb/ia64-tdep.c                     |  4 ++--
 gdb/regcache.c                      | 21 +++++++----------
 gdb/unittests/gmp-utils-selftests.c |  2 +-
 gdb/valops.c                        |  3 +--
 14 files changed, 60 insertions(+), 55 deletions(-)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 817a197ceaae..3fe3d3949328 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -237,7 +237,7 @@ amd64_linux_get_syscall_number (struct gdbarch *gdbarch,
      is stored at %rax register.  */
   regcache->cooked_read (AMD64_LINUX_ORIG_RAX_REGNUM, buf);
 
-  ret = extract_signed_integer (buf, 8, byte_order);
+  ret = extract_signed_integer (buf, byte_order);
 
   return ret;
 }
diff --git a/gdb/defs.h b/gdb/defs.h
index f7e09eca9db1..3b6a0e63905e 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -63,6 +63,7 @@
 
 #include "gdbsupport/host-defs.h"
 #include "gdbsupport/enum-flags.h"
+#include "gdbsupport/array-view.h"
 
 /* Scope types enumerator.  List the types of scopes the compiler will
    accept.  */
@@ -500,20 +501,36 @@ enum symbol_needs_kind
 /* In findvar.c.  */
 
 template<typename T, typename = RequireLongest<T>>
-T extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order);
+T extract_integer (gdb::array_view<const gdb_byte>, enum bfd_endian byte_order);
+
+static inline LONGEST
+extract_signed_integer (gdb::array_view<const gdb_byte> buf,
+			enum bfd_endian byte_order)
+{
+  return extract_integer<LONGEST> (buf, byte_order);
+}
 
 static inline LONGEST
 extract_signed_integer (const gdb_byte *addr, int len,
 			enum bfd_endian byte_order)
 {
-  return extract_integer<LONGEST> (addr, len, byte_order);
+  return extract_signed_integer (gdb::array_view<const gdb_byte> (addr, len),
+				 byte_order);
+}
+
+static inline ULONGEST
+extract_unsigned_integer (gdb::array_view<const gdb_byte> buf,
+			  enum bfd_endian byte_order)
+{
+  return extract_integer<ULONGEST> (buf, byte_order);
 }
 
 static inline ULONGEST
 extract_unsigned_integer (const gdb_byte *addr, int len,
 			  enum bfd_endian byte_order)
 {
-  return extract_integer<ULONGEST> (addr, len, byte_order);
+  return extract_unsigned_integer (gdb::array_view<const gdb_byte> (addr, len),
+				   byte_order);
 }
 
 extern int extract_long_unsigned_integer (const gdb_byte *, int,
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 592dbe19d562..b267785ef51a 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -577,8 +577,7 @@ indirect_pieced_value (value *value)
      encode address spaces and other things in CORE_ADDR.  */
   bfd_endian byte_order = gdbarch_byte_order (get_frame_arch (frame));
   LONGEST byte_offset
-    = extract_signed_integer (value_contents (value).data (),
-			      TYPE_LENGTH (type), byte_order);
+    = extract_signed_integer (value_contents (value), byte_order);
   byte_offset += piece->v.ptr.offset;
 
   return indirect_synthetic_pointer (piece->v.ptr.die_sect_off,
@@ -1157,9 +1156,7 @@ dwarf_expr_context::fetch_address (int n)
   ULONGEST result;
 
   dwarf_require_integral (value_type (result_val));
-  result = extract_unsigned_integer (value_contents (result_val).data (),
-				     TYPE_LENGTH (value_type (result_val)),
-				     byte_order);
+  result = extract_unsigned_integer (value_contents (result_val), byte_order);
 
   /* For most architectures, calling extract_unsigned_integer() alone
      is sufficient for extracting an address.  However, some
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 07cd844c818f..4da7798544b9 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -611,7 +611,7 @@ fbsd_core_xfer_siginfo (struct gdbarch *gdbarch, gdb_byte *readbuf,
 				 LWPINFO_OFFSET + LWPINFO_PL_FLAGS, 4))
     return -1;
 
-  int pl_flags = extract_signed_integer (buf, 4, gdbarch_byte_order (gdbarch));
+  int pl_flags = extract_signed_integer (buf, gdbarch_byte_order (gdbarch));
   if (!(pl_flags & PL_FLAG_SI))
     return -1;
 
@@ -1933,7 +1933,7 @@ fbsd_read_integer_by_name (struct gdbarch *gdbarch, const char *name)
   if (target_read_memory (BMSYMBOL_VALUE_ADDRESS (ms), buf, sizeof buf) != 0)
     error (_("Unable to read value of '%s'"), name);
 
-  return extract_signed_integer (buf, sizeof buf, gdbarch_byte_order (gdbarch));
+  return extract_signed_integer (buf, gdbarch_byte_order (gdbarch));
 }
 
 /* Lookup offsets of fields in the runtime linker's 'Obj_Entry'
@@ -2004,7 +2004,7 @@ fbsd_get_tls_index (struct gdbarch *gdbarch, CORE_ADDR lm_addr)
     throw_error (TLS_GENERIC_ERROR,
 		 _("Cannot find thread-local variables on this target"));
 
-  return extract_signed_integer (buf, sizeof buf, gdbarch_byte_order (gdbarch));
+  return extract_signed_integer (buf, gdbarch_byte_order (gdbarch));
 }
 
 /* See fbsd-tdep.h.  */
diff --git a/gdb/findvar.c b/gdb/findvar.c
index f7e632809d07..a0031d2dadd9 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -48,14 +48,11 @@ you lose
 
 template<typename T, typename>
 T
-extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order)
+extract_integer (gdb::array_view<const gdb_byte> buf, enum bfd_endian byte_order)
 {
   typename std::make_unsigned<T>::type retval = 0;
-  const unsigned char *p;
-  const unsigned char *startaddr = addr;
-  const unsigned char *endaddr = startaddr + len;
 
-  if (len > (int) sizeof (T))
+  if (buf.size () > (int) sizeof (T))
     error (_("\
 That operation is not available on integers of more than %d bytes."),
 	   (int) sizeof (T));
@@ -64,36 +61,38 @@ That operation is not available on integers of more than %d bytes."),
      the least significant.  */
   if (byte_order == BFD_ENDIAN_BIG)
     {
-      p = startaddr;
+      size_t i = 0;
+
       if (std::is_signed<T>::value)
 	{
 	  /* Do the sign extension once at the start.  */
-	  retval = ((LONGEST) * p ^ 0x80) - 0x80;
-	  ++p;
+	  retval = ((LONGEST) buf[i] ^ 0x80) - 0x80;
+	  ++i;
 	}
-      for (; p < endaddr; ++p)
-	retval = (retval << 8) | *p;
+      for (; i < buf.size (); ++i)
+	retval = (retval << 8) | buf[i];
     }
   else
     {
-      p = endaddr - 1;
+      ssize_t i = buf.size () - 1;
+
       if (std::is_signed<T>::value)
 	{
 	  /* Do the sign extension once at the start.  */
-	  retval = ((LONGEST) * p ^ 0x80) - 0x80;
-	  --p;
+	  retval = ((LONGEST) buf[i] ^ 0x80) - 0x80;
+	  --i;
 	}
-      for (; p >= startaddr; --p)
-	retval = (retval << 8) | *p;
+      for (; i >= 0; --i)
+	retval = (retval << 8) | buf[i];
     }
   return retval;
 }
 
 /* Explicit instantiations.  */
-template LONGEST extract_integer<LONGEST> (const gdb_byte *addr, int len,
+template LONGEST extract_integer<LONGEST> (gdb::array_view<const gdb_byte> buf,
 					   enum bfd_endian byte_order);
-template ULONGEST extract_integer<ULONGEST> (const gdb_byte *addr, int len,
-					     enum bfd_endian byte_order);
+template ULONGEST extract_integer<ULONGEST>
+  (gdb::array_view<const gdb_byte> buf, enum bfd_endian byte_order);
 
 /* Sometimes a long long unsigned integer can be extracted as a
    LONGEST value.  This is done so that we can print these values
diff --git a/gdb/frame.c b/gdb/frame.c
index 2a899fc494f2..7944d1edef8d 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1288,7 +1288,6 @@ frame_unwind_register_signed (frame_info *next_frame, int regnum)
 {
   struct gdbarch *gdbarch = frame_unwind_arch (next_frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  int size = register_size (gdbarch, regnum);
   struct value *value = frame_unwind_register_value (next_frame, regnum);
 
   gdb_assert (value != NULL);
@@ -1304,8 +1303,7 @@ frame_unwind_register_signed (frame_info *next_frame, int regnum)
 		   _("Register %d is not available"), regnum);
     }
 
-  LONGEST r = extract_signed_integer (value_contents_all (value).data (), size,
-				      byte_order);
+  LONGEST r = extract_signed_integer (value_contents_all (value), byte_order);
 
   release_value (value);
   return r;
diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
index 09274d54a0e5..3e2729f6c918 100644
--- a/gdb/frv-tdep.c
+++ b/gdb/frv-tdep.c
@@ -596,7 +596,7 @@ frv_analyze_prologue (struct gdbarch *gdbarch, CORE_ADDR pc,
 
       if (target_read_memory (pc, buf, sizeof buf) != 0)
 	break;
-      op = extract_signed_integer (buf, sizeof buf, byte_order);
+      op = extract_signed_integer (buf, byte_order);
 
       next_pc = pc + 4;
 
diff --git a/gdb/hppa-bsd-tdep.c b/gdb/hppa-bsd-tdep.c
index f4567b48f826..9021414a460e 100644
--- a/gdb/hppa-bsd-tdep.c
+++ b/gdb/hppa-bsd-tdep.c
@@ -75,7 +75,7 @@ hppabsd_find_global_pointer (struct gdbarch *gdbarch, struct value *function)
 	      if (target_read_memory (addr, buf, sizeof buf) != 0)
 		break;
 
-	      tag = extract_signed_integer (buf, sizeof buf, byte_order);
+	      tag = extract_signed_integer (buf, byte_order);
 	      if (tag == DT_PLTGOT)
 		{
 		  CORE_ADDR pltgot;
diff --git a/gdb/hppa-linux-tdep.c b/gdb/hppa-linux-tdep.c
index 1dd6993ab09b..6bb8580aa62e 100644
--- a/gdb/hppa-linux-tdep.c
+++ b/gdb/hppa-linux-tdep.c
@@ -384,7 +384,7 @@ hppa_linux_find_global_pointer (struct gdbarch *gdbarch,
 	      status = target_read_memory (addr, buf, sizeof (buf));
 	      if (status != 0)
 		break;
-	      tag = extract_signed_integer (buf, sizeof (buf), byte_order);
+	      tag = extract_signed_integer (buf, byte_order);
 
 	      if (tag == DT_PLTGOT)
 		{
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 898b73f632c4..7c6274589c9b 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -549,7 +549,7 @@ i386_linux_get_syscall_number_from_regcache (struct regcache *regcache)
      is stored at %eax register.  */
   regcache->cooked_read (I386_LINUX_ORIG_EAX_REGNUM, buf);
 
-  ret = extract_signed_integer (buf, 4, byte_order);
+  ret = extract_signed_integer (buf, byte_order);
 
   return ret;
 }
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 829909dab186..f1af7cb0fec8 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -3450,7 +3450,7 @@ ia64_find_global_pointer_from_dynamic_section (struct gdbarch *gdbarch,
 	      status = target_read_memory (addr, buf, sizeof (buf));
 	      if (status != 0)
 		break;
-	      tag = extract_signed_integer (buf, sizeof (buf), byte_order);
+	      tag = extract_signed_integer (buf, byte_order);
 
 	      if (tag == DT_PLTGOT)
 		{
@@ -3531,7 +3531,7 @@ find_extant_func_descr (struct gdbarch *gdbarch, CORE_ADDR faddr)
 	      status = target_read_memory (addr, buf, sizeof (buf));
 	      if (status != 0)
 		break;
-	      faddr2 = extract_signed_integer (buf, sizeof (buf), byte_order);
+	      faddr2 = extract_signed_integer (buf, byte_order);
 
 	      if (faddr == faddr2)
 		return addr;
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 8457284c12a1..b3ecba2fbe62 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -620,15 +620,12 @@ template<typename T, typename>
 enum register_status
 readable_regcache::raw_read (int regnum, T *val)
 {
-  gdb_byte *buf;
-  enum register_status status;
-
   assert_regnum (regnum);
-  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
-  status = raw_read (regnum, buf);
+  size_t len = m_descr->sizeof_register[regnum];
+  gdb_byte *buf = (gdb_byte *) alloca (len);
+  register_status status = raw_read (regnum, buf);
   if (status == REG_VALID)
-    *val = extract_integer<T> (buf,
-			       m_descr->sizeof_register[regnum],
+    *val = extract_integer<T> ({buf, len},
 			       gdbarch_byte_order (m_descr->gdbarch));
   else
     *val = 0;
@@ -772,14 +769,12 @@ template<typename T, typename>
 enum register_status
 readable_regcache::cooked_read (int regnum, T *val)
 {
-  enum register_status status;
-  gdb_byte *buf;
-
   gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
-  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
-  status = cooked_read (regnum, buf);
+  size_t len = m_descr->sizeof_register[regnum];
+  gdb_byte *buf = (gdb_byte *) alloca (len);
+  register_status status = cooked_read (regnum, buf);
   if (status == REG_VALID)
-    *val = extract_integer<T> (buf, m_descr->sizeof_register[regnum],
+    *val = extract_integer<T> ({buf, len},
 			       gdbarch_byte_order (m_descr->gdbarch));
   else
     *val = 0;
diff --git a/gdb/unittests/gmp-utils-selftests.c b/gdb/unittests/gmp-utils-selftests.c
index f40666e4cd56..e48bb39166e7 100644
--- a/gdb/unittests/gmp-utils-selftests.c
+++ b/gdb/unittests/gmp-utils-selftests.c
@@ -299,7 +299,7 @@ write_and_extract (T val, size_t buf_len, enum bfd_endian byte_order)
   gdb_byte *buf = (gdb_byte *) alloca (buf_len);
   v.write ({buf, buf_len}, byte_order, !std::is_signed<T>::value);
 
-  return extract_integer<T> (buf, buf_len, byte_order);
+  return extract_integer<T> ({buf, buf_len}, byte_order);
 }
 
 /* Test the gdb_mpz::write method over a reasonable range of values.
diff --git a/gdb/valops.c b/gdb/valops.c
index ca71c128de9c..779ca93edd7d 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -587,8 +587,7 @@ value_cast (struct type *type, struct value *arg2)
 	 bits.  */
       if (code2 == TYPE_CODE_PTR)
 	longest = extract_unsigned_integer
-		    (value_contents (arg2).data (), TYPE_LENGTH (type2),
-		     type_byte_order (type2));
+		    (value_contents (arg2), type_byte_order (type2));
       else
 	longest = value_as_long (arg2);
       return value_from_longest (to_type, convert_to_boolean ?
-- 
2.33.1

[PATCH v3 3/3] gdb: trivial changes to use array_view

[Simon Marchi]

Change a few relatively obvious spots using value contents to propagate
the use array_view a bit more.

Change-Id: I5338a60986f06d5969fec803d04f8423c9288a15
---
 gdb/frame.c     |  2 +-
 gdb/rust-lang.c | 10 ++++------
 gdb/valarith.c  |  9 +++------
 gdb/valops.c    | 28 ++++++++++------------------
 gdb/value.c     |  2 +-
 5 files changed, 19 insertions(+), 32 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index 7944d1edef8d..896d80d87bf1 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1261,7 +1261,7 @@ frame_unwind_register_value (frame_info *next_frame, int regnum)
 	  else
 	    {
 	      int i;
-	      const gdb_byte *buf = value_contents (value).data ();
+	      gdb::array_view<const gdb_byte> buf = value_contents (value);
 
 	      fprintf_unfiltered (&debug_file, " bytes=");
 	      fprintf_unfiltered (&debug_file, "[");
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index e5a404187bef..9ab8fbe30c0e 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -1317,9 +1317,8 @@ eval_op_rust_struct_anon (struct type *expect_type, struct expression *exp,
 
       if (rust_enum_p (type))
 	{
-	  gdb::array_view<const gdb_byte> view (value_contents (lhs).data (),
-						TYPE_LENGTH (type));
-	  type = resolve_dynamic_type (type, view, value_address (lhs));
+	  type = resolve_dynamic_type (type, value_contents (lhs),
+				       value_address (lhs));
 
 	  if (rust_empty_enum_p (type))
 	    error (_("Cannot access field %d of empty enum %s"),
@@ -1380,9 +1379,8 @@ eval_op_rust_structop (struct type *expect_type, struct expression *exp,
   struct type *type = value_type (lhs);
   if (type->code () == TYPE_CODE_STRUCT && rust_enum_p (type))
     {
-      gdb::array_view<const gdb_byte> view (value_contents (lhs).data (),
-					    TYPE_LENGTH (type));
-      type = resolve_dynamic_type (type, view, value_address (lhs));
+      type = resolve_dynamic_type (type, value_contents (lhs),
+				   value_address (lhs));
 
       if (rust_empty_enum_p (type))
 	error (_("Cannot access field %s of empty enum %s"),
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 496570762799..e37ef48aeb91 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -927,12 +927,10 @@ fixed_point_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
 	  type2 = type1;
 	}
 
-      v1.read_fixed_point (gdb::make_array_view (value_contents (arg1).data (),
-						 TYPE_LENGTH (type1)),
+      v1.read_fixed_point (value_contents (arg1),
 			   type_byte_order (type1), type1->is_unsigned (),
 			   type1->fixed_point_scaling_factor ());
-      v2.read_fixed_point (gdb::make_array_view (value_contents (arg2).data (),
-						 TYPE_LENGTH (type2)),
+      v2.read_fixed_point (value_contents (arg2),
 			   type_byte_order (type2), type2->is_unsigned (),
 			   type2->fixed_point_scaling_factor ());
     }
@@ -942,8 +940,7 @@ fixed_point_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
       value *fp_val = allocate_value (type1);
 
       fp.write_fixed_point
-	(gdb::make_array_view (value_contents_raw (fp_val).data (),
-			       TYPE_LENGTH (type1)),
+	(value_contents_raw (fp_val),
 	 type_byte_order (type1),
 	 type1->is_unsigned (),
 	 type1->fixed_point_scaling_factor ());
diff --git a/gdb/valops.c b/gdb/valops.c
index 779ca93edd7d..e0214c5d2f48 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -351,9 +351,8 @@ value_to_gdb_mpq (struct value *value)
 		  || is_fixed_point_type (type));
 
       gdb_mpz vz;
-      vz.read (gdb::make_array_view (value_contents (value).data (),
-				     TYPE_LENGTH (type)),
-	       type_byte_order (type), type->is_unsigned ());
+      vz.read (value_contents (value), type_byte_order (type),
+	       type->is_unsigned ());
       mpq_set_z (result.val, vz.val);
 
       if (is_fixed_point_type (type))
@@ -392,8 +391,7 @@ value_cast_to_fixed_point (struct type *to_type, struct value *from_val)
   /* Finally, create the result value, and pack the unscaled value
      in it.  */
   struct value *result = allocate_value (to_type);
-  unscaled.write (gdb::make_array_view (value_contents_raw (result).data (),
-					TYPE_LENGTH (to_type)),
+  unscaled.write (value_contents_raw (result),
 		  type_byte_order (to_type),
 		  to_type->is_unsigned ());
 
@@ -554,11 +552,10 @@ value_cast (struct type *type, struct value *arg2)
 	{
 	  gdb_mpq fp_val;
 
-	  fp_val.read_fixed_point
-	    (gdb::make_array_view (value_contents (arg2).data (),
-				   TYPE_LENGTH (type2)),
-	     type_byte_order (type2), type2->is_unsigned (),
-	     type2->fixed_point_scaling_factor ());
+	  fp_val.read_fixed_point (value_contents (arg2),
+				   type_byte_order (type2),
+				   type2->is_unsigned (),
+				   type2->fixed_point_scaling_factor ());
 
 	  struct value *v = allocate_value (to_type);
 	  target_float_from_host_double (value_contents_raw (v).data (),
@@ -1255,14 +1252,9 @@ value_assign (struct value *toval, struct value *fromval)
 					   value_contents (fromval).data ());
 	      }
 	    else
-	      {
-		gdb::array_view<const gdb_byte> contents
-		  = gdb::make_array_view (value_contents (fromval).data (),
-					  TYPE_LENGTH (type));
-		put_frame_register_bytes (frame, value_reg,
-					  value_offset (toval),
-					  contents);
-	      }
+	      put_frame_register_bytes (frame, value_reg,
+					value_offset (toval),
+					value_contents (fromval));
 	  }
 
 	gdb::observers::register_changed.notify (frame, value_reg);
diff --git a/gdb/value.c b/gdb/value.c
index 4e66329e82c5..7d939ab11dbf 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -4011,7 +4011,7 @@ value_fetch_lazy_register (struct value *val)
       else
 	{
 	  int i;
-	  const gdb_byte *buf = value_contents (new_val).data ();
+	  gdb::array_view<const gdb_byte> buf = value_contents (new_val);
 
 	  if (VALUE_LVAL (new_val) == lval_register)
 	    fprintf_unfiltered (&debug_file, " register=%d",
-- 
2.33.1

Re: [PATCH v3 1/3] gdbsupport: add array_view copy function

[Pedro Alves]

On 2021-12-01 16:49, Simon Marchi via Gdb-patches wrote:
> New in v3:
> 
>  - Fix stale comment
>  - Change order of parameters
>  - Don't copy if src == dest

Thanks.  This version LGTM.  Small/trivial remark below.

> +/* Copy the contents referenced by the array view SRC to the array view DEST.
> +
> +   The two array views must have the same length.  */
> +
> +template <typename T, typename U>
> +void copy (gdb::array_view<U> src, gdb::array_view<T> dest)

Swap U/T to match the template parameters declaration order?

Re: [PATCH v3 2/3] gdb: make extract_integer take an array_view

[Pedro Alves]

On 2021-12-01 16:49, Simon Marchi via Gdb-patches wrote:
> I think it would make sense for extract_integer, extract_signed_integer
> and extract_unsigned_integer to take an array_view.  This way, when we
> extract an integer, we can validate that we don't overflow the buffer
> passed by the caller (e.g. ask to extract a 4-byte integer but pass a
> 2-byte buffer).
> 
>  - Change extract_integer to take an array_view
>  - Add overloads of extract_signed_integer and extract_unsigned_integer
>    that take array_views.  Keep the existing versions so we don't
>    need to change all callers, but make them call the array_view
>    versions.
> 
> This shortens some places like:
> 
>   result = extract_unsigned_integer (value_contents (result_val).data (),
> 				     TYPE_LENGTH (value_type (result_val)),
> 				     byte_order);
> 
> into
> 
>   result = extract_unsigned_integer (value_contents (result_val), byte_order);
> 
> value_contents returns an array view that is of length
> `TYPE_LENGTH (value_type (result_val))` already, so the length is
> implicitly communicated through the array view.

LGTM.

Re: [PATCH v3 3/3] gdb: trivial changes to use array_view

[Pedro Alves]

On 2021-12-01 16:49, Simon Marchi via Gdb-patches wrote:
> Change a few relatively obvious spots using value contents to propagate
> the use array_view a bit more.

OK.

Re: [PATCH v3 1/3] gdbsupport: add array_view copy function

[Simon Marchi]

On 2021-12-03 08:57, Pedro Alves wrote:
> On 2021-12-01 16:49, Simon Marchi via Gdb-patches wrote:
>> New in v3:
>>
>>  - Fix stale comment
>>  - Change order of parameters
>>  - Don't copy if src == dest
> 
> Thanks.  This version LGTM.  Small/trivial remark below.
> 
>> +/* Copy the contents referenced by the array view SRC to the array view DEST.
>> +
>> +   The two array views must have the same length.  */
>> +
>> +template <typename T, typename U>
>> +void copy (gdb::array_view<U> src, gdb::array_view<T> dest)
> 
> Swap U/T to match the template parameters declaration order?
> 

Done, thanks.

Simon

Re: [PATCH v3 3/3] gdb: trivial changes to use array_view

[Simon Marchi]

On 2021-12-03 09:10, Pedro Alves wrote:
> On 2021-12-01 16:49, Simon Marchi via Gdb-patches wrote:
>> Change a few relatively obvious spots using value contents to propagate
>> the use array_view a bit more.
> 
> OK.
> 

Thanks, all pushed.

Simon

Re: [PATCH v3 3/3] gdb: trivial changes to use array_view

[Simon Marchi]

On 2021-12-03 16:44, Simon Marchi via Gdb-patches wrote:
> 
> 
> On 2021-12-03 09:10, Pedro Alves wrote:
>> On 2021-12-01 16:49, Simon Marchi via Gdb-patches wrote:
>>> Change a few relatively obvious spots using value contents to propagate
>>> the use array_view a bit more.
>>
>> OK.
>>
> 
> Thanks, all pushed.
> 
> Simon
> 

I had totally forgotten that the last patch causes an internal error
when running an Ada test case.  I did mention it here before:

https://sourceware.org/pipermail/gdb-patches/2021-November/183252.html

I pushed the following patch to revert just the change that causes this
internal error.

From fb2a515fd07be61a9deda0ebfe9507f8000d93cc Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Fri, 3 Dec 2021 20:39:57 -0500
Subject: [PATCH] gdb: revert one array_view copy change in ada-lang.c

Commit 4bce7cdaf481 ("gdbsupport: add array_view copy function") caused
an internal error when running gdb.ada/packed_array_assign.exp:

    print pra(1) := pr^M
    /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/array-view.h:217: internal-error: copy: Assertion `dest.size () == src.size ()' failed.^M

I am not sure what's the root cause of this, whether it is a GDB bug
exposed by using the array_view copy function or not.  Back out the
change that triggers the internal error for now, while we investigate
it.

Change-Id: I055ab14143e4cfd3ca7ce8f4855c6c3c05db52a7
---
 gdb/ada-lang.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index c6cefe9d8e89..f8ba05b42765 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -2586,7 +2586,9 @@ ada_value_assign (struct value *toval, struct value *fromval)
       write_memory_with_notification (to_addr, buffer, len);
 
       val = value_copy (toval);
-      copy (value_contents (fromval), value_contents_raw (val));
+      memcpy (value_contents_raw (val).data (),
+	      value_contents (fromval).data (),
+	      TYPE_LENGTH (type));
       deprecated_set_value_type (val, type);
 
       return val;
-- 
2.33.1