From 22347e554cd7ba2a0bf36dc81ebfcbe2e2fd74af Mon Sep 17 00:00:00 2001 From: Andreas Arnez Date: Thu, 24 Nov 2016 17:48:03 +0100 Subject: [PATCH] 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. --- gdb/ChangeLog | 7 + gdb/dwarf2loc.c | 200 ++++++--------------- gdb/testsuite/ChangeLog | 5 + gdb/testsuite/gdb.dwarf2/nonvar-access.exp | 33 +++- 4 files changed, 96 insertions(+), 149 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 22f02cccd50..aa2fe417db8 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2016-11-24 Andreas Arnez + + * 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 PR gdb/12616 diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 44dceda26ea..b238133463e 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; } - 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; } } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index e9dc36f8576..81470915c1b 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2016-11-24 Andreas Arnez + + * gdb.dwarf2/nonvar-access.exp: Add a test for accessing + non-byte-aligned bit fields. + 2016-11-24 Andreas Arnez PR gdb/12616 diff --git a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp index 21532a60f2b..8910f6d7304 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" " = " gdb_test "print undef_s.a" " = " -- 2.30.2