This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 6/9] Fix handling of DWARF register pieces on big-endian targets
On 2017-04-07 13:38, Andreas Arnez wrote:
For big-endian targets the logic in read/write_pieced_value tries to
take
a register piece from the LSB end. This requires offsets and sizes to
be
adjusted accordingly, and that's where the current implementation has
some
issues:
* The formulas for recalculating the bit- and byte-offsets into the
register are wrong. They just happen to yield correct results if
everything is byte-aligned and the piece's last byte belongs to the
given value.
* After recalculating the bit offset into the register, the number of
bytes to be copied from the register is not recalculated. Of course
this does not matter if everything (particularly the piece size) is
byte-aligned.
These issues are fixed. The size calculation is performed with a new
helper function bits_to_bytes(). For simplification, the variable
buffer_size is dropped.
gdb/ChangeLog:
* dwarf2loc.c (bits_to_bytes): New function.
(read_pieced_value): Fix offset calculations for register pieces
on big-endian targets.
(write_pieced_value): Likewise.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/var-access.exp: Add test for non-byte-aligned
register pieces.
---
gdb/dwarf2loc.c | 74
+++++++++++++++++----------------
gdb/testsuite/gdb.dwarf2/var-access.exp | 32 ++++++++++++++
2 files changed, 70 insertions(+), 36 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 1f89a08..d13c0c5 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1751,6 +1751,15 @@ copy_bitwise_tests (void)
#endif /* GDB_SELF_TEST */
+/* Return the number of bytes overlapping a contiguous chunk of N_BITS
+ bits whose first bit is located at bit offset START. */
+
+static size_t
+bits_to_bytes (ULONGEST start, ULONGEST n_bits)
+{
+ return (start % 8 + n_bits + 7) / 8;
+}
+
static void
read_pieced_value (struct value *v)
{
@@ -1761,7 +1770,6 @@ read_pieced_value (struct value *v)
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (v);
size_t type_len;
- size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
= gdbarch_bits_big_endian (get_type_arch (value_type (v)));
@@ -1805,13 +1813,9 @@ read_pieced_value (struct value *v)
if (this_size_bits > type_len - offset)
this_size_bits = type_len - offset;
- this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
+ this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+ buffer.reserve (this_size);
source_offset = source_offset_bits / 8;
- if (buffer_size < this_size)
- {
- buffer_size = this_size;
- buffer.reserve (buffer_size);
- }
The removal of buffer_size and this "if" (and equivalent in
write_pieced_value) seems to be unrelated to the problem you are fixing,
but rather a simplification that could be done anyway. If that's the
case, you should bundle it with the similar changes from the previous
patch, in a patch that only does some refactoring but not behavior
changes.
intermediate_buffer = buffer.data ();
/* Copy from the source to DEST_BUFFER. */
@@ -1822,22 +1826,22 @@ read_pieced_value (struct value *v)
struct frame_info *frame = frame_find_by_id (c->frame_id);
struct gdbarch *arch = get_frame_arch (frame);
int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
+ ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
int optim, unavail;
- LONGEST reg_offset = source_offset;
if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && this_size < register_size (arch, gdb_regnum))
+ && p->size < reg_bits)
{
/* Big-endian, and we want less than full size. */
- reg_offset = register_size (arch, gdb_regnum) - this_size;
- /* We want the lower-order THIS_SIZE_BITS of the bytes
- we extract from the register. */
- source_offset_bits += 8 * this_size - this_size_bits;
+ source_offset_bits += reg_bits - p->size;
}
+ this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+ buffer.reserve (this_size);
- if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
- this_size, buffer.data (),
- &optim, &unavail))
+ if (!get_frame_register_bytes (frame, gdb_regnum,
+ source_offset_bits / 8,
+ this_size, buffer.data (),
+ &optim, &unavail))
{
/* Just so garbage doesn't ever shine through. */
memset (buffer.data (), 0, this_size);
@@ -1847,9 +1851,8 @@ read_pieced_value (struct value *v)
if (unavail)
mark_value_bits_unavailable (v, offset, this_size_bits);
}
-
copy_bitwise (contents, dest_offset_bits,
- intermediate_buffer, source_offset_bits % 8,
+ buffer.data (), source_offset_bits % 8,
this_size_bits, bits_big_endian);
}
break;
@@ -1927,7 +1930,6 @@ write_pieced_value (struct value *to, struct
value *from)
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (to);
size_t type_len;
- size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
= gdbarch_bits_big_endian (get_type_arch (value_type (to)));
@@ -1972,7 +1974,7 @@ write_pieced_value (struct value *to, struct
value *from)
if (this_size_bits > type_len - offset)
this_size_bits = type_len - offset;
- this_size = (this_size_bits + dest_offset_bits % 8 + 7) / 8;
+ this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
source_offset = source_offset_bits / 8;
dest_offset = dest_offset_bits / 8;
if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0
@@ -1983,11 +1985,7 @@ write_pieced_value (struct value *to, struct
value *from)
}
else
{
- if (buffer_size < this_size)
- {
- buffer_size = this_size;
- buffer.reserve (buffer_size);
- }
+ buffer.reserve (this_size);
source_buffer = buffer.data ();
need_bitwise = 1;
}
@@ -1999,20 +1997,24 @@ write_pieced_value (struct value *to, struct
value *from)
struct frame_info *frame = frame_find_by_id (c->frame_id);
struct gdbarch *arch = get_frame_arch (frame);
int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
- int reg_offset = dest_offset;
+ ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && this_size <= register_size (arch, gdb_regnum))
+ && p->size <= reg_bits)
{
/* Big-endian, and we want less than full size. */
- reg_offset = register_size (arch, gdb_regnum) - this_size;
+ dest_offset_bits += reg_bits - p->size;
}
+ this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
+ buffer.reserve (this_size);
- if (need_bitwise)
+ if (dest_offset_bits % 8 != 0 || this_size_bits % 8 != 0)
I thought the "need_bitwise" variable name helped to understand. To
compensate, could you add a comment explaining the "why" of that if?
{
+ /* Need some bits from original register value. */
int optim, unavail;
- if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
+ if (!get_frame_register_bytes (frame, gdb_regnum,
+ dest_offset_bits / 8,
this_size, buffer.data (),
&optim, &unavail))
{
@@ -2027,14 +2029,14 @@ write_pieced_value (struct value *to, struct
value *from)
"bitfield; containing word "
"is unavailable"));
}
- copy_bitwise (buffer.data (), dest_offset_bits,
- contents, source_offset_bits,
- this_size_bits,
- bits_big_endian);
}
- put_frame_register_bytes (frame, gdb_regnum, reg_offset,
- this_size, source_buffer);
+ copy_bitwise (buffer.data (), dest_offset_bits % 8,
+ contents, source_offset_bits,
+ this_size_bits, bits_big_endian);
+ put_frame_register_bytes (frame, gdb_regnum,
+ dest_offset_bits / 8,
+ this_size, buffer.data ());
}
break;
case DWARF_VALUE_MEMORY:
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp
b/gdb/testsuite/gdb.dwarf2/var-access.exp
index c6abc87..4787dfb 100644
--- a/gdb/testsuite/gdb.dwarf2/var-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -192,6 +192,18 @@ Dwarf::assemble $asm_file {
piece 1
} SPECIAL_expr}
}
+ # Register pieces for bitfield access.
+ DW_TAG_variable {
+ {name "t2"}
+ {type :$struct_t_label}
+ {location {
+ piece 4
+ regx [lindex $dwarf_regnum 0]
+ piece 3
+ regx [lindex $dwarf_regnum 1]
+ piece 1
+ } SPECIAL_expr}
+ }
}
}
}
@@ -206,6 +218,15 @@ if ![runto_main] {
return -1
}
+# Determine endianness.
+set endian "little"
+gdb_test_multiple "show endian" "determine endianness" {
+ -re ".* (big|little) endian.*$gdb_prompt $" {
+ set endian $expect_out(1,string)
+ pass "endianness: $endian"
+ }
+}
"pass" should be passed the test name, which should be stable as much as
possible. The typical way to do it would be:
# Determine endianness.
set endian "little"
set test "determine endianness"
gdb_test_multiple "show endian" $test {
-re ".* (big|little) endian.*$gdb_prompt $" {
set endian $expect_out(1,string)
pass $test
}
}
+
# Byte-aligned memory pieces.
gdb_test "print/d s1" " = \\{a = 2, b = 3, c = 0, d = 1\\}" \
"s1 == re-ordered buf"
@@ -244,3 +265,14 @@ gdb_test_no_output "set var t1.y = 1234"
gdb_test "print t1" " = \\{u = <optimized out>, x = -7, y = 1234, z =
340\\}" \
"verify t1"
+switch $endian { big {set val 0x7ffc} little {set val 0x3ffe00} }
I had to draw this in order to understand, so it might be useful to
others.
For little endian (assuming the register is 32 bits):
3 f f e 0 0
xxxx xxxx 0011 1111 1111 1110 0000 0000
^^ ^^^^ ^^^^
| ^^ ^^^^ ^^^^ ^^^ ` x
| `- y
`- part of z
For big endian (assuming the register is 32 bits):
0 0 7 f f c
xxxx xxxx 0000 0000 0111 1111 1111 1100
^^^^ ^^^^ ^ ^^
`- x ^^^ ^^^^ ^^^^ ^^ `- part of z
`- y
Not having worked with a big endian machine before, I'm still confused
in how bits are aligned in the register and which bit means what.
+gdb_test_no_output "set var \$[lindex $regname 0] = $val" \
+ "init t2, first piece"
+gdb_test_no_output "set var \$[lindex $regname 1] = 0" \
+ "init t2, second piece"
+gdb_test "print/d t2" " = \\{u = <optimized out>, x = 0, y = -1, z =
0\\}" \
+ "initialized t2 from regs"
+gdb_test_no_output "set var t2.y = 2641"
+gdb_test_no_output "set var t2.z = -400"
+gdb_test_no_output "set var t2.x = 200"
+gdb_test "print t2.x + t2.y + t2.z" " = 2441"
Thanks,
Simon