[RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long

[Tom Tromey <tom@tromey.com>]

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-22  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.
---
 gdb/ChangeLog                    |  7 +++++
 gdb/Makefile.in                  |  1 +
 gdb/unittests/unpack-selftests.c | 61 ++++++++++++++++++++++++++++++++++++++++
 gdb/value.c                      |  7 +++--
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 gdb/unittests/unpack-selftests.c

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1644ac364f..734bbdf832 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-02-22  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-21  John Baldwin  <jhb@FreeBSD.org>
 
 	* arch/aarch64.c: Include "common-defs.h".
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3a105d75c6..1c58b9270d 100644
--- 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
index 0000000000..89f3e09c4e
--- /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 9cd9a2fcc7..0d85fe3774 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3244,7 +3244,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 +3265,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;
-- 
2.13.6