gdb: change functions returning value contents to use gdb::array_view

The bug fixed by this [1] patch was caused by an out-of-bounds access to
a value's content.  The code gets the value's content (just a pointer)
and then indexes it with a non-sensical index.

This made me think of changing functions that return value contents to
return array_views instead of a plain pointer.  This has the advantage
that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view
are checked, making bugs more apparent / easier to find.

This patch changes the return types of these functions, and updates
callers to call .data() on the result, meaning it's not changing
anything in practice.  Additional work will be needed (which can be done
little by little) to make callers propagate the use of array_view and
reap the benefits.

[1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html

Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3

1 files changed, 45 insertions, 35 deletions

diff --git a/gdb/value.c b/gdb/value.c
index bb2adae..42ce804 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1146,21 +1146,25 @@ set_value_parent (struct value *value, struct value *parent)
   value->parent = value_ref_ptr::new_reference (parent);
 }
 
-gdb_byte *
+gdb::array_view<gdb_byte>
 value_contents_raw (struct value *value)
 {
   struct gdbarch *arch = get_value_arch (value);
   int unit_size = gdbarch_addressable_memory_unit_size (arch);
 
   allocate_value_contents (value);
-  return value->contents.get () + value->embedded_offset * unit_size;
+
+  ULONGEST length = TYPE_LENGTH (value_type (value));
+  return {value->contents.get () + value->embedded_offset * unit_size, length};
 }
 
-gdb_byte *
+gdb::array_view<gdb_byte>
 value_contents_all_raw (struct value *value)
 {
   allocate_value_contents (value);
-  return value->contents.get ();
+
+  ULONGEST length = TYPE_LENGTH (value_type (value));
+  return {value->contents.get (), length};
 }
 
 struct type *
@@ -1238,25 +1242,29 @@ require_available (const struct value *value)
     throw_error (NOT_AVAILABLE_ERROR, _("value is not available"));
 }
 
-const gdb_byte *
+gdb::array_view<const gdb_byte>
 value_contents_for_printing (struct value *value)
 {
   if (value->lazy)
     value_fetch_lazy (value);
-  return value->contents.get ();
+
+  ULONGEST length = TYPE_LENGTH (value_type (value));
+  return {value->contents.get (), length};
 }
 
-const gdb_byte *
+gdb::array_view<const gdb_byte>
 value_contents_for_printing_const (const struct value *value)
 {
   gdb_assert (!value->lazy);
-  return value->contents.get ();
+
+  ULONGEST length = TYPE_LENGTH (value_type (value));
+  return {value->contents.get (), length};
 }
 
-const gdb_byte *
+gdb::array_view<const gdb_byte>
 value_contents_all (struct value *value)
 {
-  const gdb_byte *result = value_contents_for_printing (value);
+  gdb::array_view<const gdb_byte> result = value_contents_for_printing (value);
   require_not_optimized_out (value);
   require_available (value);
   return result;
@@ -1334,8 +1342,8 @@ value_contents_copy_raw (struct value *dst, LONGEST dst_offset,
 					     TARGET_CHAR_BIT * length));
 
   /* Copy the data.  */
-  memcpy (value_contents_all_raw (dst) + dst_offset * unit_size,
-	  value_contents_all_raw (src) + src_offset * unit_size,
+  memcpy (value_contents_all_raw (dst).data () + dst_offset * unit_size,
+	  value_contents_all_raw (src).data () + src_offset * unit_size,
 	  length * unit_size);
 
   /* Copy the meta-data, adjusted.  */
@@ -1392,16 +1400,16 @@ set_value_stack (struct value *value, int val)
   value->stack = val;
 }
 
-const gdb_byte *
+gdb::array_view<const gdb_byte>
 value_contents (struct value *value)
 {
-  const gdb_byte *result = value_contents_writeable (value);
+  gdb::array_view<const gdb_byte> result = value_contents_writeable (value);
   require_not_optimized_out (value);
   require_available (value);
   return result;
 }
 
-gdb_byte *
+gdb::array_view<gdb_byte>
 value_contents_writeable (struct value *value)
 {
   if (value->lazy)
@@ -1713,7 +1721,8 @@ value_copy (struct value *arg)
   val->initialized = arg->initialized;
   if (!value_lazy (val))
     {
-      memcpy (value_contents_all_raw (val), value_contents_all_raw (arg),
+      memcpy (value_contents_all_raw (val).data (),
+	      value_contents_all_raw (arg).data (),
 	      TYPE_LENGTH (value_enclosing_type (arg)));
 
     }
@@ -1761,7 +1770,8 @@ 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), value_contents_all (arg),
+      memcpy (value_contents_all_raw (val).data (),
+	      value_contents_all (arg).data (),
 	      TYPE_LENGTH (enc_type));
       val->type = arg->type;
       set_value_embedded_offset (val, value_embedded_offset (arg));
@@ -1778,7 +1788,7 @@ value_force_lval (struct value *v, CORE_ADDR addr)
 {
   gdb_assert (VALUE_LVAL (v) == not_lval);
 
-  write_memory (addr, value_contents_raw (v), TYPE_LENGTH (value_type (v)));
+  write_memory (addr, value_contents_raw (v).data (), TYPE_LENGTH (value_type (v)));
   v->lval = lval_memory;
   v->location.address = addr;
 }
@@ -2303,7 +2313,7 @@ set_internalvar_component (struct internalvar *var,
   switch (var->kind)
     {
     case INTERNALVAR_VALUE:
-      addr = value_contents_writeable (var->u.value);
+      addr = value_contents_writeable (var->u.value).data ();
       arch = get_value_arch (var->u.value);
       unit_size = gdbarch_addressable_memory_unit_size (arch);
 
@@ -2311,7 +2321,7 @@ set_internalvar_component (struct internalvar *var,
 	modify_field (value_type (var->u.value), addr + offset,
 		      value_as_long (newval), bitpos, bitsize);
       else
-	memcpy (addr + offset * unit_size, value_contents (newval),
+	memcpy (addr + offset * unit_size, value_contents (newval).data (),
 		TYPE_LENGTH (value_type (newval)));
       break;
 
@@ -2695,7 +2705,7 @@ value_as_long (struct value *val)
      in disassemble_command).  It also dereferences references, which
      I suspect is the most logical thing to do.  */
   val = coerce_array (val);
-  return unpack_long (value_type (val), value_contents (val));
+  return unpack_long (value_type (val), value_contents (val).data ());
 }
 
 /* Extract a value as a C pointer.  Does not deallocate the value.
@@ -2798,9 +2808,9 @@ value_as_address (struct value *val)
   if (!value_type (val)->is_pointer_or_reference ()
       && gdbarch_integer_to_address_p (gdbarch))
     return gdbarch_integer_to_address (gdbarch, value_type (val),
-				       value_contents (val));
+				       value_contents (val).data ());
 
-  return unpack_long (value_type (val), value_contents (val));
+  return unpack_long (value_type (val), value_contents (val).data ());
 #endif
 }
 
@@ -2924,7 +2934,7 @@ is_floating_value (struct value *val)
 
   if (is_floating_type (type))
     {
-      if (!target_float_is_valid (value_contents (val), type))
+      if (!target_float_is_valid (value_contents (val).data (), type))
 	error (_("Invalid floating value found in program."));
       return true;
     }
@@ -3066,7 +3076,7 @@ value_primitive_field (struct value *arg1, LONGEST offset,
 	 for references to ordinary fields of unavailable values.  */
       if (BASETYPE_VIA_VIRTUAL (arg_type, fieldno))
 	boffset = baseclass_offset (arg_type, fieldno,
-				    value_contents (arg1),
+				    value_contents (arg1).data (),
 				    value_embedded_offset (arg1),
 				    value_address (arg1),
 				    arg1);
@@ -3325,7 +3335,7 @@ unpack_value_bitfield (struct value *dest_val,
 
       num = unpack_bits_as_long (field_type, valaddr + embedded_offset,
 				 bitpos, bitsize);
-      store_signed_integer (value_contents_raw (dest_val),
+      store_signed_integer (value_contents_raw (dest_val).data (),
 			    TYPE_LENGTH (field_type), byte_order, num);
     }
 
@@ -3532,7 +3542,7 @@ value_from_longest (struct type *type, LONGEST num)
 {
   struct value *val = allocate_value (type);
 
-  pack_long (value_contents_raw (val), type, num);
+  pack_long (value_contents_raw (val).data (), type, num);
   return val;
 }
 
@@ -3544,7 +3554,7 @@ value_from_ulongest (struct type *type, ULONGEST num)
 {
   struct value *val = allocate_value (type);
 
-  pack_unsigned_long (value_contents_raw (val), type, num);
+  pack_unsigned_long (value_contents_raw (val).data (), type, num);
 
   return val;
 }
@@ -3558,7 +3568,7 @@ value_from_pointer (struct type *type, CORE_ADDR addr)
 {
   struct value *val = allocate_value (type);
 
-  store_typed_address (value_contents_raw (val),
+  store_typed_address (value_contents_raw (val).data (),
 		       check_typedef (type), addr);
   return val;
 }
@@ -3572,7 +3582,7 @@ value_from_host_double (struct type *type, double d)
 {
   struct value *value = allocate_value (type);
   gdb_assert (type->code () == TYPE_CODE_FLT);
-  target_float_from_host_double (value_contents_raw (value),
+  target_float_from_host_double (value_contents_raw (value).data (),
 				 value_type (value), d);
   return value;
 }
@@ -3638,7 +3648,7 @@ value_from_contents (struct type *type, const gdb_byte *contents)
   struct value *result;
 
   result = allocate_value (type);
-  memcpy (value_contents_raw (result), contents, TYPE_LENGTH (type));
+  memcpy (value_contents_raw (result).data (), contents, TYPE_LENGTH (type));
   return result;
 }
 
@@ -3790,7 +3800,7 @@ coerce_ref (struct value *arg)
   enc_type = check_typedef (value_enclosing_type (arg));
   enc_type = TYPE_TARGET_TYPE (enc_type);
 
-  CORE_ADDR addr = unpack_pointer (value_type (arg), value_contents (arg));
+  CORE_ADDR addr = unpack_pointer (value_type (arg), value_contents (arg).data ());
   retval = value_at_lazy (enc_type, addr);
   enc_type = value_type (retval);
   return readjust_indirect_value_type (retval, enc_type, value_type_arg_tmp,
@@ -3887,7 +3897,7 @@ value_fetch_lazy_bitfield (struct value *val)
     value_fetch_lazy (parent);
 
   unpack_value_bitfield (val, value_bitpos (val), value_bitsize (val),
-			 value_contents_for_printing (parent),
+			 value_contents_for_printing (parent).data (),
 			 value_offset (val), parent);
 }
 
@@ -3903,7 +3913,7 @@ value_fetch_lazy_memory (struct value *val)
 
   if (TYPE_LENGTH (type))
       read_value_memory (val, 0, value_stack (val),
-			 addr, value_contents_all_raw (val),
+			 addr, value_contents_all_raw (val).data (),
 			 type_length_units (type));
 }
 
@@ -3999,7 +4009,7 @@ value_fetch_lazy_register (struct value *val)
       else
 	{
 	  int i;
-	  const gdb_byte *buf = value_contents (new_val);
+	  const gdb_byte *buf = value_contents (new_val).data ();
 
 	  if (VALUE_LVAL (new_val) == lval_register)
 	    fprintf_unfiltered (&debug_file, " register=%d",