Fix copy_bitwise()

When the user writes or reads a variable whose location is described
with DWARF pieces (DW_OP_piece or DW_OP_bit_piece), GDB's helper
function copy_bitwise is invoked for each piece.  The implementation of
this function has a bug that may result in a corrupted copy, depending
on alignment and bit size.  (Full-byte copies are not affected.)

This rewrites copy_bitwise, replacing its algorithm by a fixed version,
and adding an appropriate test case.  Without the fix the new test case
fails, e.g.:

  print def_t
  $2 = {a = 0, b = 4177919}
  (gdb) FAIL: gdb.dwarf2/nonvar-access.exp: print def_t

Written in binary, the wrong result above looks like this:

  01111111011111111111111

Which means that two zero bits have sneaked into the copy of the
original all-one bit pattern.  The test uses this simple all-one value
in order to avoid another GDB bug that causes the DWARF piece of a
DW_OP_stack_value to be taken from the wrong end on big-endian
architectures.

gdb/ChangeLog:

	* dwarf2loc.c (extract_bits_primitive): Remove.
	(extract_bits): Remove.
	(copy_bitwise): Rewrite.  Fixes a possible corruption that may
	occur for non-byte-aligned copies.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/nonvar-access.exp: Add a test for accessing
	non-byte-aligned bit fields.
This commit is contained in:
Andreas Arnez 2016-11-24 17:48:03 +01:00
parent da5b30da2d
commit 22347e554c
4 changed files with 96 additions and 149 deletions

View file

@ -1,3 +1,10 @@
2016-11-24 Andreas Arnez <arnez@linux.vnet.ibm.com>
* dwarf2loc.c (extract_bits_primitive): Remove.
(extract_bits): Remove.
(copy_bitwise): Rewrite. Fixes a possible corruption that may
occur for non-byte-aligned copies.
2016-11-24 Andreas Arnez <arnez@linux.vnet.ibm.com>
PR gdb/12616

View file

@ -1491,175 +1491,79 @@ allocate_piece_closure (struct dwarf2_per_cu_data *per_cu,
return c;
}
/* The lowest-level function to extract bits from a byte buffer.
SOURCE is the buffer. It is updated if we read to the end of a
byte.
SOURCE_OFFSET_BITS is the offset of the first bit to read. It is
updated to reflect the number of bits actually read.
NBITS is the number of bits we want to read. It is updated to
reflect the number of bits actually read. This function may read
fewer bits.
BITS_BIG_ENDIAN is taken directly from gdbarch.
This function returns the extracted bits. */
static unsigned int
extract_bits_primitive (const gdb_byte **source,
unsigned int *source_offset_bits,
int *nbits, int bits_big_endian)
{
unsigned int avail, mask, datum;
gdb_assert (*source_offset_bits < 8);
avail = 8 - *source_offset_bits;
if (avail > *nbits)
avail = *nbits;
mask = (1 << avail) - 1;
datum = **source;
if (bits_big_endian)
datum >>= 8 - (*source_offset_bits + *nbits);
else
datum >>= *source_offset_bits;
datum &= mask;
*nbits -= avail;
*source_offset_bits += avail;
if (*source_offset_bits >= 8)
{
*source_offset_bits -= 8;
++*source;
}
return datum;
}
/* Extract some bits from a source buffer and move forward in the
buffer.
SOURCE is the source buffer. It is updated as bytes are read.
SOURCE_OFFSET_BITS is the offset into SOURCE. It is updated as
bits are read.
NBITS is the number of bits to read.
BITS_BIG_ENDIAN is taken directly from gdbarch.
This function returns the bits that were read. */
static unsigned int
extract_bits (const gdb_byte **source, unsigned int *source_offset_bits,
int nbits, int bits_big_endian)
{
unsigned int datum;
gdb_assert (nbits > 0 && nbits <= 8);
datum = extract_bits_primitive (source, source_offset_bits, &nbits,
bits_big_endian);
if (nbits > 0)
{
unsigned int more;
more = extract_bits_primitive (source, source_offset_bits, &nbits,
bits_big_endian);
if (bits_big_endian)
datum <<= nbits;
else
more <<= nbits;
datum |= more;
}
return datum;
}
/* Write some bits into a buffer and move forward in the buffer.
DATUM is the bits to write. The low-order bits of DATUM are used.
DEST is the destination buffer. It is updated as bytes are
written.
DEST_OFFSET_BITS is the bit offset in DEST at which writing is
done.
NBITS is the number of valid bits in DATUM.
BITS_BIG_ENDIAN is taken directly from gdbarch. */
/* Copy NBITS bits from SOURCE to DEST starting at the given bit
offsets. Use the bit order as specified by BITS_BIG_ENDIAN.
Source and destination buffers must not overlap. */
static void
insert_bits (unsigned int datum,
gdb_byte *dest, unsigned int dest_offset_bits,
int nbits, int bits_big_endian)
copy_bitwise (gdb_byte *dest, ULONGEST dest_offset,
const gdb_byte *source, ULONGEST source_offset,
ULONGEST nbits, int bits_big_endian)
{
unsigned int mask;
unsigned int buf, avail;
gdb_assert (dest_offset_bits + nbits <= 8);
if (nbits == 0)
return;
mask = (1 << nbits) - 1;
if (bits_big_endian)
{
datum <<= 8 - (dest_offset_bits + nbits);
mask <<= 8 - (dest_offset_bits + nbits);
/* Start from the end, then work backwards. */
dest_offset += nbits - 1;
dest += dest_offset / 8;
dest_offset = 7 - dest_offset % 8;
source_offset += nbits - 1;
source += source_offset / 8;
source_offset = 7 - source_offset % 8;
}
else
{
datum <<= dest_offset_bits;
mask <<= dest_offset_bits;
dest += dest_offset / 8;
dest_offset %= 8;
source += source_offset / 8;
source_offset %= 8;
}
gdb_assert ((datum & ~mask) == 0);
/* Fill BUF with DEST_OFFSET bits from the destination and 8 -
SOURCE_OFFSET bits from the source. */
buf = *(bits_big_endian ? source-- : source++) >> source_offset;
buf <<= dest_offset;
buf |= *dest & ((1 << dest_offset) - 1);
*dest = (*dest & ~mask) | datum;
}
/* NBITS: bits yet to be written; AVAIL: BUF's fill level. */
nbits += dest_offset;
avail = dest_offset + 8 - source_offset;
/* Copy bits from a source to a destination.
DEST is where the bits should be written.
DEST_OFFSET_BITS is the bit offset into DEST.
SOURCE is the source of bits.
SOURCE_OFFSET_BITS is the bit offset into SOURCE.
BIT_COUNT is the number of bits to copy.
BITS_BIG_ENDIAN is taken directly from gdbarch. */
static void
copy_bitwise (gdb_byte *dest, unsigned int dest_offset_bits,
const gdb_byte *source, unsigned int source_offset_bits,
unsigned int bit_count,
int bits_big_endian)
{
unsigned int dest_avail;
int datum;
/* Reduce everything to byte-size pieces. */
dest += dest_offset_bits / 8;
dest_offset_bits %= 8;
source += source_offset_bits / 8;
source_offset_bits %= 8;
dest_avail = 8 - dest_offset_bits % 8;
/* See if we can fill the first destination byte. */
if (dest_avail < bit_count)
/* Flush 8 bits from BUF, if appropriate. */
if (nbits >= 8 && avail >= 8)
{
datum = extract_bits (&source, &source_offset_bits, dest_avail,
bits_big_endian);
insert_bits (datum, dest, dest_offset_bits, dest_avail, bits_big_endian);
++dest;
dest_offset_bits = 0;
bit_count -= dest_avail;
*(bits_big_endian ? dest-- : dest++) = buf;
buf >>= 8;
avail -= 8;
nbits -= 8;
}
/* Now, either DEST_OFFSET_BITS is byte-aligned, or we have fewer
than 8 bits remaining. */
gdb_assert (dest_offset_bits % 8 == 0 || bit_count < 8);
for (; bit_count >= 8; bit_count -= 8)
/* Copy the middle part. */
if (nbits >= 8)
{
datum = extract_bits (&source, &source_offset_bits, 8, bits_big_endian);
*dest++ = (gdb_byte) datum;
size_t len = nbits / 8;
while (len--)
{
buf |= *(bits_big_endian ? source-- : source++) << avail;
*(bits_big_endian ? dest-- : dest++) = buf;
buf >>= 8;
}
nbits %= 8;
}
/* Finally, we may have a few leftover bits. */
gdb_assert (bit_count <= 8 - dest_offset_bits % 8);
if (bit_count > 0)
/* Write the last byte. */
if (nbits)
{
datum = extract_bits (&source, &source_offset_bits, bit_count,
bits_big_endian);
insert_bits (datum, dest, dest_offset_bits, bit_count, bits_big_endian);
if (avail < nbits)
buf |= *source << avail;
buf &= (1 << nbits) - 1;
*dest = (*dest & (~0 << nbits)) | buf;
}
}

View file

@ -1,3 +1,8 @@
2016-11-24 Andreas Arnez <arnez@linux.vnet.ibm.com>
* gdb.dwarf2/nonvar-access.exp: Add a test for accessing
non-byte-aligned bit fields.
2016-11-24 Andreas Arnez <arnez@linux.vnet.ibm.com>
PR gdb/12616

View file

@ -32,7 +32,7 @@ Dwarf::assemble $asm_file {
{DW_AT_name main.c}
} {
declare_labels int_type_label short_type_label
declare_labels struct_s_label
declare_labels struct_s_label struct_t_label
int_type_label: base_type {
{name "int"}
@ -58,6 +58,24 @@ Dwarf::assemble $asm_file {
}
}
struct_t_label: structure_type {
{name t}
{byte_size 4 DW_FORM_sdata}
} {
member {
{name a}
{type :$int_type_label}
{data_member_location 0 DW_FORM_udata}
{bit_size 9 DW_FORM_udata}
}
member {
{name b}
{type :$int_type_label}
{data_bit_offset 9 DW_FORM_udata}
{bit_size 23 DW_FORM_udata}
}
}
DW_TAG_subprogram {
{name main}
{DW_AT_external 1 flag}
@ -84,6 +102,18 @@ Dwarf::assemble $asm_file {
bit_piece 24 0
} SPECIAL_expr}
}
DW_TAG_variable {
{name def_t}
{type :$struct_t_label}
{location {
const1u 0
stack_value
bit_piece 9 0
const1s -1
stack_value
bit_piece 23 0
} SPECIAL_expr}
}
}
}
}
@ -99,5 +129,6 @@ if ![runto_main] {
}
gdb_test "print def_s" " = \\{a = 0, b = -1\\}"
gdb_test "print def_t" " = \\{a = 0, b = -1\\}"
gdb_test "print undef_int" " = <optimized out>"
gdb_test "print undef_s.a" " = <optimized out>"