* [PATCH 1/2] Fix bug in Ada packed array handling
2022-10-06 20:06 [PATCH 0/2] Fix two bugs with packed array printing Tom Tromey
@ 2022-10-06 20:06 ` Tom Tromey
2022-10-06 20:06 ` [PATCH 2/2] Fix crash in value_print_array_elements Tom Tromey
2022-10-21 15:40 ` [PATCH 0/2] Fix two bugs with packed array printing Tom Tromey
2 siblings, 0 replies; 4+ messages in thread
From: Tom Tromey @ 2022-10-06 20:06 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
A user found a bug where an array of packed arrays was printed
incorrectly. The bug here is that the packed array has a bit stride,
but the outer array does not -- and should not. However,
update_static_array_size does not distinguish between an array of
packed arrays and a multi-dimensional packed array, and for the
latter, only the innermost array will end up with a stride.
This patch fixes the problem by adding a flag to indicate whether a
given array type is a constituent of a multi-dimensional array.
---
gdb/dwarf2/read.c | 4 ++++
gdb/gdbtypes.c | 1 +
gdb/gdbtypes.h | 21 +++++++++++++++++++++
gdb/testsuite/gdb.ada/packed_array.exp | 6 ++++++
gdb/testsuite/gdb.ada/packed_array/pa.adb | 14 ++++++++++++++
5 files changed, 46 insertions(+)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 78f4cc1f60d..116fc59821a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -15695,6 +15695,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
{
type = create_array_type_with_stride (NULL, type, range_types[i++],
byte_stride_prop, bit_stride);
+ type->set_is_multi_dimensional (true);
bit_stride = 0;
byte_stride_prop = nullptr;
}
@@ -15706,11 +15707,14 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
{
type = create_array_type_with_stride (NULL, type, range_types[ndim],
byte_stride_prop, bit_stride);
+ type->set_is_multi_dimensional (true);
bit_stride = 0;
byte_stride_prop = nullptr;
}
}
+ /* Clear the flag on the outermost array type. */
+ type->set_is_multi_dimensional (false);
gdb_assert (type != element_type);
/* Understand Dwarf2 support for vector types (like they occur on
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 643bb0a14a3..9ddb1f81cc1 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1319,6 +1319,7 @@ update_static_array_size (struct type *type)
wrong size when trying to find elements of the outer
array. */
if (element_type->code () == TYPE_CODE_ARRAY
+ && (stride != 0 || element_type->is_multi_dimensional ())
&& element_type->length () != 0
&& TYPE_FIELD_BITSIZE (element_type, 0) != 0
&& get_array_bounds (element_type, &low_bound, &high_bound)
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 94d4b6684fb..f92b3894a1b 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -926,6 +926,15 @@ struct main_type
unsigned int m_flag_flag_enum : 1;
+ /* * For TYPE_CODE_ARRAY, this is true if this type is part of a
+ multi-dimensional array. Multi-dimensional arrays are
+ represented internally as arrays of arrays, and this flag lets
+ gdb distinguish between multiple dimensions and an ordinary array
+ of arrays. The flag is set on each inner dimension, but not the
+ outermost dimension. */
+
+ unsigned int m_multi_dimensional : 1;
+
/* * A discriminant telling us which field of the type_specific
union is being used for this type, if any. */
@@ -1349,6 +1358,18 @@ struct type
this->main_type->m_flag_flag_enum = is_flag_enum;
}
+ /* True if this array type is part of a multi-dimensional array. */
+
+ bool is_multi_dimensional () const
+ {
+ return this->main_type->m_multi_dimensional;
+ }
+
+ void set_is_multi_dimensional (bool value)
+ {
+ this->main_type->m_multi_dimensional = value;
+ }
+
/* * Assuming that THIS is a TYPE_CODE_FIXED_POINT, return a reference
to this type's fixed_point_info. */
diff --git a/gdb/testsuite/gdb.ada/packed_array.exp b/gdb/testsuite/gdb.ada/packed_array.exp
index eb857f7e564..df34f31348a 100644
--- a/gdb/testsuite/gdb.ada/packed_array.exp
+++ b/gdb/testsuite/gdb.ada/packed_array.exp
@@ -57,4 +57,10 @@ foreach_with_prefix scenario {all minimal} {
xfail $test
}
}
+
+ set line "(4 => true, false, true, false, true)"
+ gdb_test "print o_var" \
+ [string_to_regexp " = ($line, $line, $line, $line)"]
+ gdb_test "print o2_var" \
+ [string_to_regexp " = ($line, $line, $line, $line)"]
}
diff --git a/gdb/testsuite/gdb.ada/packed_array/pa.adb b/gdb/testsuite/gdb.ada/packed_array/pa.adb
index 2955f986b00..bb6a2acd388 100644
--- a/gdb/testsuite/gdb.ada/packed_array/pa.adb
+++ b/gdb/testsuite/gdb.ada/packed_array/pa.adb
@@ -27,6 +27,20 @@ procedure PA is
U_Var : Unconstrained_Packed_Array (1 .. Ident (6));
+ -- Note that this array is not packed.
+ type Outer_Array is array (1 .. 4) of Packed_Array;
+ O_Var : Outer_Array := ((true, false, true, false, true),
+ (true, false, true, false, true),
+ (true, false, true, false, true),
+ (true, false, true, false, true));
+
+ type Outer_Array2 is array (1 .. 4) of Packed_Array;
+ pragma pack (Outer_Array2);
+ O2_Var : Outer_Array2 := ((true, false, true, false, true),
+ (true, false, true, false, true),
+ (true, false, true, false, true),
+ (true, false, true, false, true));
+
begin
Var := (True, False, True, False, True);
--
2.34.3
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] Fix crash in value_print_array_elements
2022-10-06 20:06 [PATCH 0/2] Fix two bugs with packed array printing Tom Tromey
2022-10-06 20:06 ` [PATCH 1/2] Fix bug in Ada packed array handling Tom Tromey
@ 2022-10-06 20:06 ` Tom Tromey
2022-10-21 15:40 ` [PATCH 0/2] Fix two bugs with packed array printing Tom Tromey
2 siblings, 0 replies; 4+ messages in thread
From: Tom Tromey @ 2022-10-06 20:06 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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.
---
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 91a59419c4e..266a76dbdc4 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1928,7 +1928,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;
@@ -1939,7 +1938,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 ();
@@ -1988,23 +1989,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 8ed941f3749..53b35af5620 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 52752df1f4c..57a7724388b 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);
--
2.34.3
^ permalink raw reply [flat|nested] 4+ messages in thread