Sign-extend non-bit-fields in unpack_bits_as_long

unpack_bits_as_long is documented as sign-extending its result when
the type is signed.  However, it was only doing sign-extension in the
case where the field was a bitfield -- that is, not when the "bitsize"
parameter was 0, indicating the size should be taken from the type.

Also, unpack_bits_as_long was incorrectly computing the shift for
big-endian architectures for the non-bitfield case.

This patch fixes these bugs in a straightforward way.  A new selftest
is included.

2018-02-26  Tom Tromey  <tom@tromey.com>

	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
	unittests/unpack-selftests.c.
	* unittests/unpack-selftests.c: New file.
	* value.c (unpack_bits_as_long): Fix bugs in non-bitfield cases.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e7dd890ac9b9fd8cfc4dda834a8fbd8fb1a960c9..0d01668e329dc2ddb9e61fec5c6e4f37723a7b7f 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-02-26  Tom Tromey  <tom@tromey.com>
+
+       * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
+       unittests/unpack-selftests.c.
+       * unittests/unpack-selftests.c: New file.
+       * value.c (unpack_bits_as_long): Fix bugs in non-bitfield cases.
+
 2018-02-26  Yao Qi  <yao.qi@linaro.org>
 
        * dwarf2read.c (struct partial_die_info) <read>: New method.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3a105d75c681e8d03e8174512c1c03c3f56a5ce7..1c58b9270d63b3bc30ebd61ae725149b2e5e760a 100644 (file)
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -427,6 +427,7 @@ SUBDIR_UNITTESTS_SRCS = \
        unittests/scoped_fd-selftests.c \
        unittests/scoped_mmap-selftests.c \
        unittests/scoped_restore-selftests.c \
+       unittests/unpack-selftests.c \
        unittests/xml-utils-selftests.c
 
 SUBDIR_UNITTESTS_OBS = $(patsubst %.c,%.o,$(SUBDIR_UNITTESTS_SRCS))

diff --git a/gdb/unittests/unpack-selftests.c b/gdb/unittests/unpack-selftests.c
new file mode 100644 (file)
index 0000000..89f3e09
--- /dev/null
+++ b/gdb/unittests/unpack-selftests.c
@@ -0,0 +1,61 @@
+/* Self tests for unpack_field_as_long
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "selftest.h"
+#include "selftest-arch.h"
+#include "value.h"
+#include "gdbtypes.h"
+#include "arch-utils.h"
+
+namespace selftests {
+namespace unpack {
+
+static void
+unpack_field_as_long_tests (struct gdbarch *arch)
+{
+  gdb_byte buffer[8];
+  const struct builtin_type *bt = builtin_type (arch);
+  struct type *struct_type = arch_composite_type (arch, "<<selftest>>",
+                                                 TYPE_CODE_STRUCT);
+
+  append_composite_type_field (struct_type, "field0", bt->builtin_int8);
+  append_composite_type_field_aligned (struct_type, "field1",
+                                      bt->builtin_uint32, 4);
+
+  memset (buffer, 0, sizeof (buffer));
+  buffer[0] = 255;
+  if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG)
+    buffer[7] = 23;
+  else
+    buffer[4] = 23;
+
+  SELF_CHECK (unpack_field_as_long (struct_type, buffer, 0) == -1);
+  SELF_CHECK (unpack_field_as_long (struct_type, buffer, 1) == 23);
+}
+
+}
+}
+
+void
+_initialize_unpack_selftests ()
+{
+  selftests::register_test_foreach_arch
+    ("unpack_field_as_long", selftests::unpack::unpack_field_as_long_tests);
+}

diff --git a/gdb/value.c b/gdb/value.c
index 9cd9a2fcc72717d32f94e5be3854c879fb694211..110d16dcf5b17bc6359874e85379bd7a0353bc79 100644 (file)
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3214,7 +3214,8 @@ value_fn_field (struct value **arg1p, struct fn_field *f,
 
 /* Unpack a bitfield of the specified FIELD_TYPE, from the object at
    VALADDR, and store the result in *RESULT.
-   The bitfield starts at BITPOS bits and contains BITSIZE bits.
+   The bitfield starts at BITPOS bits and contains BITSIZE bits; if
+   BITSIZE is zero, then the length is taken from FIELD_TYPE.
 
    Extracting bits depends on endianness of the machine.  Compute the
    number of least significant bits to discard.  For big endian machines,
@@ -3244,7 +3245,10 @@ unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr,
   if (bitsize)
     bytes_read = ((bitpos % 8) + bitsize + 7) / 8;
   else
-    bytes_read = TYPE_LENGTH (field_type);
+    {
+      bytes_read = TYPE_LENGTH (field_type);
+      bitsize = 8 * bytes_read;
+    }
 
   read_offset = bitpos / 8;
 
@@ -3262,7 +3266,7 @@ unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr,
   /* If the field does not entirely fill a LONGEST, then zero the sign bits.
      If the field is signed, and is negative, then sign extend.  */
 
-  if ((bitsize > 0) && (bitsize < 8 * (int) sizeof (val)))
+  if (bitsize < 8 * (int) sizeof (val))
     {
       valmask = (((ULONGEST) 1) << bitsize) - 1;
       val &= valmask;