[binutils-gdb] Sign-extend non-bit-fields in unpack_bits_as_long

[Tom Tromey <tromey at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=15ce8941e7d2807a3396a6874c528b24c387660a

commit 15ce8941e7d2807a3396a6874c528b24c387660a
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Feb 21 10:36:55 2018 -0700

    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:
---
 gdb/ChangeLog                    |  7 +++++
 gdb/Makefile.in                  |  1 +
 gdb/unittests/unpack-selftests.c | 61 ++++++++++++++++++++++++++++++++++++++++
 gdb/value.c                      | 10 +++++--
 4 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e7dd890..0d01668 100644
--- 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 3a105d7..1c58b92 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 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 9cd9a2f..110d16d 100644
--- 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;