Commit Message
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.
---
gdb/dwarf2loc.c | 200 ++++++++---------------------
gdb/testsuite/gdb.dwarf2/nonvar-access.exp | 33 ++++-
2 files changed, 84 insertions(+), 149 deletions(-)
Comments
On 11/14/2016 09:02 AM, Andreas Arnez wrote:
> 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.
> ---
> gdb/dwarf2loc.c | 200 ++++++++---------------------
> gdb/testsuite/gdb.dwarf2/nonvar-access.exp | 33 ++++-
> 2 files changed, 84 insertions(+), 149 deletions(-)
>
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 93aec1f..3a241a8 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -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;
Are you sure you will always have non-zero source_offset and dest_offset
when explicitly dividing them by 8? If i were to feed (or GDB, in some
erroneous state) invalid data to the function, this would likely crash?
There are other cases of explicit / operations.
> }
>
> - gdb_assert ((datum & ~mask) == 0);
> -
> - *dest = (*dest & ~mask) | datum;
> -}
> -
> -/* 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;
> + /* 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;
Maybe it's just me, but having constructs like the above don't help much
performance-wise and make the code slightly less readable. Should we
expand this further? There are multiple occurrences of this.
Also, should we harden the method to prevent dereferencing NULL source
or dest?
> + buf <<= dest_offset;
> + buf |= *dest & ((1 << dest_offset) - 1);
>
> - /* Reduce everything to byte-size pieces. */
> - dest += dest_offset_bits / 8;
> - dest_offset_bits %= 8;
> - source += source_offset_bits / 8;
> - source_offset_bits %= 8;
> + /* NBITS: bits yet to be written; AVAIL: BUF's fill level. */
> + nbits += dest_offset;
> + avail = dest_offset + 8 - source_offset;
>
> - 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;
> }
> }
>
> diff --git a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
> index 21532a6..8910f6d 100644
> --- a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
> +++ b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
> @@ -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>"
>
Otherwise it looks good to me.
On Mon, Nov 14 2016, Luis Machado wrote:
> On 11/14/2016 09:02 AM, Andreas Arnez wrote:
[...]
>> + dest += dest_offset / 8;
>> + dest_offset %= 8;
>> + source += source_offset / 8;
>> + source_offset %= 8;
>
> Are you sure you will always have non-zero source_offset and dest_offset
> when explicitly dividing them by 8? If i were to feed (or GDB, in some
> erroneous state) invalid data to the function, this would likely crash?
>
> There are other cases of explicit / operations.
No, copy_bitwise should work fine with source_offset and dest_offset set
to zero. Where do you think it would crash?
[...]
>> + /* 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;
>
> Maybe it's just me, but having constructs like the above don't help much
> performance-wise and make the code slightly less readable. Should we
> expand this further? There are multiple occurrences of this.
Well, I've tried a few different ways and found this approach actually
the easiest to read, for my taste. For instance, it makes the multiple
occurrences easy to recognize -- as you pointed out ;-)
Of course, if people feel that this post-decrement/increment pattern
really hurts readability, I can provide a more "stretched" form instead.
> Also, should we harden the method to prevent dereferencing NULL source or
> dest?
I wouldn't consider that necessary, but I'm open for other opinions.
[...]
> Otherwise it looks good to me.
Thanks!
--
Andreas
On 11/14/2016 11:54 AM, Andreas Arnez wrote:
> On Mon, Nov 14 2016, Luis Machado wrote:
>
>> On 11/14/2016 09:02 AM, Andreas Arnez wrote:
>
> [...]
>
>>> + dest += dest_offset / 8;
>>> + dest_offset %= 8;
>>> + source += source_offset / 8;
>>> + source_offset %= 8;
>>
>> Are you sure you will always have non-zero source_offset and dest_offset
>> when explicitly dividing them by 8? If i were to feed (or GDB, in some
>> erroneous state) invalid data to the function, this would likely crash?
>>
>> There are other cases of explicit / operations.
>
> No, copy_bitwise should work fine with source_offset and dest_offset set
> to zero. Where do you think it would crash?
>
Well, obviously i wasn't fully awake. Nevermind this.
> [...]
>
>>> + /* 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;
>>
>> Maybe it's just me, but having constructs like the above don't help much
>> performance-wise and make the code slightly less readable. Should we
>> expand this further? There are multiple occurrences of this.
>
> Well, I've tried a few different ways and found this approach actually
> the easiest to read, for my taste. For instance, it makes the multiple
> occurrences easy to recognize -- as you pointed out ;-)
>
> Of course, if people feel that this post-decrement/increment pattern
> really hurts readability, I can provide a more "stretched" form instead.
>
No strong opinions there. Just a suggestion.
On Mon, Nov 14 2016, Andreas Arnez wrote:
[...]
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 93aec1f..3a241a8 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
[...]
For reference, since the patch itself might be a bit clumsy to read,
here's the resulting function:
/* 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
copy_bitwise (gdb_byte *dest, ULONGEST dest_offset,
const gdb_byte *source, ULONGEST source_offset,
ULONGEST nbits, int bits_big_endian)
{
unsigned int buf, avail;
if (nbits == 0)
return;
if (bits_big_endian)
{
/* 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
{
dest += dest_offset / 8;
dest_offset %= 8;
source += source_offset / 8;
source_offset %= 8;
}
/* 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);
/* NBITS: bits yet to be written; AVAIL: BUF's fill level. */
nbits += dest_offset;
avail = dest_offset + 8 - source_offset;
/* Flush 8 bits from BUF, if appropriate. */
if (nbits >= 8 && avail >= 8)
{
*(bits_big_endian ? dest-- : dest++) = buf;
buf >>= 8;
avail -= 8;
nbits -= 8;
}
/* Copy the middle part. */
if (nbits >= 8)
{
size_t len = nbits / 8;
while (len--)
{
buf |= *(bits_big_endian ? source-- : source++) << avail;
*(bits_big_endian ? dest-- : dest++) = buf;
buf >>= 8;
}
nbits %= 8;
}
/* Write the last byte. */
if (nbits)
{
if (avail < nbits)
buf |= *source << avail;
buf &= (1 << nbits) - 1;
*dest = (*dest & (~0 << nbits)) | buf;
}
}
On 11/14/2016 03:02 PM, Andreas Arnez wrote:
> 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.
Looks like the sort of function that should be possible to
cover all sorts of inputs/outputs with unit tests. Aligned, misaligned,
big/little endian, etc., that sort of thing. That'd help
a lot with ensuring rewrites behave as intended. Would you feel like
including some?
The unit testing framework is trivial to use, since it's baked inside
gdb. All you need to do is write a function that throws some corner case
inputs at copy_bitwise, checking expected outputs and calling SELF_CHECK.
You register that function in the selftests frameworks with "register_self_test".
Then to trigger the tests, just do: gdb -ex "maintenance selftest".
testsuite/gdb.gdb/unittest.exp does that too.
Thanks,
Pedro Alves
@@ -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);
-
- *dest = (*dest & ~mask) | datum;
-}
-
-/* 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;
+ /* 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);
- /* Reduce everything to byte-size pieces. */
- dest += dest_offset_bits / 8;
- dest_offset_bits %= 8;
- source += source_offset_bits / 8;
- source_offset_bits %= 8;
+ /* NBITS: bits yet to be written; AVAIL: BUF's fill level. */
+ nbits += dest_offset;
+ avail = dest_offset + 8 - source_offset;
- 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;
}
}
@@ -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>"