[binutils-gdb] Fix crash in value_print_array_elements

[binutils-gdb] Fix crash in value_print_array_elements

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e379f6521a9fbc65de8cd90a950e28d0ca522ae3

commit e379f6521a9fbc65de8cd90a950e28d0ca522ae3
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Sep 27 12:53:25 2022 -0600

    Fix crash in value_print_array_elements
    
    A user noticed that gdb would crash when printing a packed array after
    doing "set lang c".  Packed arrays don't exist in C, but it's
    occasionally useful to print things in C mode when working in a non-C
    language -- this lets you see under the hood a little bit.
    
    The bug here is that generic value printing does not handle packed
    arrays at all.  This patch fixes the bug by introducing a new function
    to extract a value from a bit offset and width.
    
    The new function includes a hack to avoid problems with some existing
    test cases when using -fgnat-encodings=all.  Cleaning up this code
    looked difficult, and since "all" is effectively deprecated, I thought
    it made sense to simply work around the problems.

Diff:
---
 gdb/testsuite/gdb.ada/packed_array.exp | 13 ++++++
 gdb/valprint.c                         | 20 ++++++---
 gdb/value.c                            | 79 ++++++++++++++++++++++++++++++++++
 gdb/value.h                            | 21 +++++++++
 4 files changed, 126 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.ada/packed_array.exp b/gdb/testsuite/gdb.ada/packed_array.exp
index df34f31348a..e73298ec84c 100644
--- a/gdb/testsuite/gdb.ada/packed_array.exp
+++ b/gdb/testsuite/gdb.ada/packed_array.exp
@@ -63,4 +63,17 @@ foreach_with_prefix scenario {all minimal} {
 	[string_to_regexp " = ($line, $line, $line, $line)"]
     gdb_test "print o2_var" \
 	[string_to_regexp " = ($line, $line, $line, $line)"]
+
+    # This is a regression test for a crash with
+    # -fgnat-encodings=minimal, and with 'all' the output is unusual,
+    # so restrict the test.
+    if {$scenario == "minimal"} {
+	set line "{true, false, true, false, true}"
+
+	gdb_test "set lang c" \
+	    "Warning: the current language does not match this frame."
+	gdb_test "print o2_var" \
+	    [string_to_regexp " = {$line, $line, $line, $line}"] \
+	    "print o2_var in c mode"
+    }
 }
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 685f8900628..25db57ea794 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1927,7 +1927,6 @@ value_print_array_elements (struct value *val, struct ui_file *stream,
   unsigned int things_printed = 0;
   unsigned len;
   struct type *elttype, *index_type;
-  unsigned eltlen;
   /* Position of the array element we are examining to see
      whether it is repeated.  */
   unsigned int rep1;
@@ -1938,7 +1937,9 @@ value_print_array_elements (struct value *val, struct ui_file *stream,
   struct type *type = check_typedef (value_type (val));
 
   elttype = type->target_type ();
-  eltlen = type_length_units (check_typedef (elttype));
+  unsigned bit_stride = type->bit_stride ();
+  if (bit_stride == 0)
+    bit_stride = 8 * check_typedef (elttype)->length ();
   index_type = type->index_type ();
   if (index_type->code () == TYPE_CODE_RANGE)
     index_type = index_type->target_type ();
@@ -1987,23 +1988,28 @@ value_print_array_elements (struct value *val, struct ui_file *stream,
       maybe_print_array_index (index_type, i + low_bound,
 			       stream, options);
 
+      struct value *element = value_from_component_bitsize (val, elttype,
+							    bit_stride * i,
+							    bit_stride);
       rep1 = i + 1;
       reps = 1;
       /* Only check for reps if repeat_count_threshold is not set to
 	 UINT_MAX (unlimited).  */
       if (options->repeat_count_threshold < UINT_MAX)
 	{
-	  while (rep1 < len
-		 && value_contents_eq (val, i * eltlen,
-				       val, rep1 * eltlen,
-				       eltlen))
+	  while (rep1 < len)
 	    {
+	      struct value *rep_elt
+		= value_from_component_bitsize (val, elttype,
+						rep1 * bit_stride,
+						bit_stride);
+	      if (!value_contents_eq (element, rep_elt))
+		break;
 	      ++reps;
 	      ++rep1;
 	    }
 	}
 
-      struct value *element = value_from_component (val, elttype, eltlen * i);
       common_val_print (element, stream, recurse + 1, options,
 			current_language);
 
diff --git a/gdb/value.c b/gdb/value.c
index 47028630259..74af654c451 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -916,6 +916,17 @@ value_contents_eq (const struct value *val1, LONGEST offset1,
 				 length * TARGET_CHAR_BIT);
 }
 
+/* See value.h.  */
+
+bool
+value_contents_eq (const struct value *val1, const struct value *val2)
+{
+  ULONGEST len1 = check_typedef (value_enclosing_type (val1))->length ();
+  ULONGEST len2 = check_typedef (value_enclosing_type (val2))->length ();
+  if (len1 != len2)
+    return false;
+  return value_contents_eq (val1, 0, val2, 0, len1);
+}
 
 /* The value-history records all the values printed by print commands
    during this session.  */
@@ -1368,6 +1379,43 @@ value_contents_copy_raw (struct value *dst, LONGEST dst_offset,
 			      bit_length);
 }
 
+/* A helper for value_from_component_bitsize that copies bits from SRC
+   to DEST.  */
+
+static void
+value_contents_copy_raw_bitwise (struct value *dst, LONGEST dst_bit_offset,
+				 struct value *src, LONGEST src_bit_offset,
+				 LONGEST bit_length)
+{
+  /* 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);
+
+  /* The overwritten DST range gets unavailability ORed in, not
+     replaced.  Make sure to remember to implement replacing if it
+     turns out actually necessary.  */
+  LONGEST dst_offset = dst_bit_offset / TARGET_CHAR_BIT;
+  LONGEST length = bit_length / TARGET_CHAR_BIT;
+  gdb_assert (value_bytes_available (dst, dst_offset, length));
+  gdb_assert (!value_bits_any_optimized_out (dst, dst_bit_offset,
+					     bit_length));
+
+  /* Copy the data.  */
+  gdb::array_view<gdb_byte> dst_contents = value_contents_all_raw (dst);
+  gdb::array_view<const gdb_byte> src_contents = value_contents_all_raw (src);
+  copy_bitwise (dst_contents.data (), dst_bit_offset,
+		src_contents.data (), src_bit_offset,
+		bit_length,
+		type_byte_order (value_type (src)) == BFD_ENDIAN_BIG);
+
+  /* Copy the meta-data.  */
+  value_ranges_copy_adjusted (dst, dst_bit_offset,
+			      src, src_bit_offset,
+			      bit_length);
+}
+
 /* Copy LENGTH bytes of SRC value's (all) contents
    (value_contents_all) starting at SRC_OFFSET byte, into DST value's
    (all) contents, starting at DST_OFFSET.  If unavailable contents
@@ -3774,6 +3822,37 @@ value_from_component (struct value *whole, struct type *type, LONGEST offset)
   return v;
 }
 
+/* See value.h.  */
+
+struct value *
+value_from_component_bitsize (struct value *whole, struct type *type,
+			      LONGEST bit_offset, LONGEST bit_length)
+{
+  gdb_assert (!value_lazy (whole));
+
+  /* Preserve lvalue-ness if possible.  This is needed to avoid
+     array-printing failures (including crashes) when printing Ada
+     arrays in programs compiled with -fgnat-encodings=all.  */
+  if ((bit_offset % TARGET_CHAR_BIT) == 0
+      && (bit_length % TARGET_CHAR_BIT) == 0
+      && bit_length == TARGET_CHAR_BIT * type->length ())
+    return value_from_component (whole, type, bit_offset / TARGET_CHAR_BIT);
+
+  struct value *v = allocate_value (type);
+
+  LONGEST dst_offset = TARGET_CHAR_BIT * value_embedded_offset (v);
+  if (is_scalar_type (type) && type_byte_order (type) == BFD_ENDIAN_BIG)
+    dst_offset += TARGET_CHAR_BIT * type->length () - bit_length;
+
+  value_contents_copy_raw_bitwise (v, dst_offset,
+				   whole,
+				   TARGET_CHAR_BIT
+				   * value_embedded_offset (whole)
+				   + bit_offset,
+				   bit_length);
+  return v;
+}
+
 struct value *
 coerce_ref_if_computed (const struct value *arg)
 {
diff --git a/gdb/value.h b/gdb/value.h
index d4b4f95a9c5..2d148ce13d3 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -599,6 +599,12 @@ extern bool value_contents_eq (const struct value *val1, LONGEST offset1,
 			       const struct value *val2, LONGEST offset2,
 			       LONGEST length);
 
+/* An overload of value_contents_eq that compares the entirety of both
+   values.  */
+
+extern bool value_contents_eq (const struct value *val1,
+			       const struct value *val2);
+
 /* Read LENGTH addressable memory units starting at MEMADDR into BUFFER,
    which is (or will be copied to) VAL's contents buffer offset by
    BIT_OFFSET bits.  Marks value contents ranges as unavailable if
@@ -687,6 +693,21 @@ extern struct value *value_from_history_ref (const char *, const char **);
 extern struct value *value_from_component (struct value *, struct type *,
 					   LONGEST);
 
+
+/* Create a new value by extracting it from WHOLE.  TYPE is the type
+   of the new value.  BIT_OFFSET and BIT_LENGTH describe the offset
+   and field width of the value to extract from WHOLE -- BIT_LENGTH
+   may differ from TYPE's length in the case where WHOLE's type is
+   packed.
+
+   When the value does come from a non-byte-aligned offset or field
+   width, it will be marked non_lval.  */
+
+extern struct value *value_from_component_bitsize (struct value *whole,
+						   struct type *type,
+						   LONGEST bit_offset,
+						   LONGEST bit_length);
+
 extern struct value *value_at (struct type *type, CORE_ADDR addr);
 extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);