[commit] Fix bitfield errors - PR 10565

[Daniel Jacobowitz <drow at false dot org>]

This patch fixes two problems which showed up as crashes on some
Windows hosts, and H.J. reported a nice testcase for wrong output on
Linux hosts.

value_offset for a bitfield is an offset to be added into the parent's
contents.  So including the parent's offset is incorrect; the attached
test shows that we were reading the wrong location.  With large enough
offsets, we could wander off to another page of memory and fault.
This problem was introduced by my patch.

The other problem was pre-existing.  unpack_bits_as_long reads from
valaddr + bitpos / 8, but it reads a ULONGEST - even if we only want a
byte.  We never used the extra bits, but they were outside of the
buffer and could lead to faults.

Tested on x86_64-linux, checked in.

-- 
Daniel Jacobowitz
CodeSourcery

2009-08-28  Daniel Jacobowitz  <dan@codesourcery.com>

	PR gdb/10565

	* value.c (value_primitive_field): Do not save value_offset for
	bitfields.
	(unpack_bits_as_long): Do not read an entire ULONGEST.

2009-08-28  Daniel Jacobowitz  <dan@codesourcery.com>

	PR gdb/10565

	* gdb.base/bitfields.c (struct container, container): New.
	(main): Initialize it and call break5.
	* gdb.base/bitfields.exp (bitfield_at_offset): New test.

Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.94
diff -u -p -r1.94 value.c
--- value.c	27 Aug 2009 23:37:35 -0000	1.94
+++ value.c	28 Aug 2009 18:29:31 -0000
@@ -1878,7 +1878,7 @@ value_primitive_field (struct value *arg
 	v->bitpos = bitpos % container_bitsize;
       else
 	v->bitpos = bitpos % 8;
-      v->offset = value_offset (arg1) + value_embedded_offset (arg1)
+      v->offset = value_embedded_offset (arg1)
 	+ (bitpos - v->bitpos) / 8;
       v->parent = arg1;
       value_incref (v->parent);
@@ -2031,15 +2031,23 @@ unpack_bits_as_long (struct type *field_
   ULONGEST val;
   ULONGEST valmask;
   int lsbcount;
+  int bytes_read;
 
-  val = extract_unsigned_integer (valaddr + bitpos / 8,
-				  sizeof (val), byte_order);
+  /* Read the minimum number of bytes required; there may not be
+     enough bytes to read an entire ULONGEST.  */
   CHECK_TYPEDEF (field_type);
+  if (bitsize)
+    bytes_read = ((bitpos % 8) + bitsize + 7) / 8;
+  else
+    bytes_read = TYPE_LENGTH (field_type);
+
+  val = extract_unsigned_integer (valaddr + bitpos / 8,
+				  bytes_read, byte_order);
 
   /* Extract bits.  See comment above. */
 
   if (gdbarch_bits_big_endian (get_type_arch (field_type)))
-    lsbcount = (sizeof val * 8 - bitpos % 8 - bitsize);
+    lsbcount = (bytes_read * 8 - bitpos % 8 - bitsize);
   else
     lsbcount = (bitpos % 8);
   val >>= lsbcount;
Index: testsuite/gdb.base/bitfields.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/bitfields.c,v
retrieving revision 1.2
diff -u -p -r1.2 bitfields.c
--- testsuite/gdb.base/bitfields.c	30 Jan 2002 22:38:56 -0000	1.2
+++ testsuite/gdb.base/bitfields.c	28 Aug 2009 18:29:31 -0000
@@ -63,6 +63,12 @@ void break10 ()
 {
 }
 
+struct container
+{
+  struct fields one;
+  struct fields two;
+} container;
+
 /* This is used by bitfields.exp to determine if the target understands
    signed bitfields.  */
 int i;
@@ -190,5 +196,10 @@ int main ()
   flags.s3 = 0;
   flags.s9 = 0;
 
+  /* Bitfields at a non-zero offset in a containing structure.  */
+  container.one.u3 = 5;
+  container.two.u3 = 3;
+  break5 ();
+
   return 0;
 }
Index: testsuite/gdb.base/bitfields.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/bitfields.exp,v
retrieving revision 1.8
diff -u -p -r1.8 bitfields.exp
--- testsuite/gdb.base/bitfields.exp	3 Jan 2009 05:58:03 -0000	1.8
+++ testsuite/gdb.base/bitfields.exp	28 Aug 2009 18:29:31 -0000
@@ -233,6 +233,26 @@ proc bitfield_signedness {} {
     gdb_stop_suppressing_tests;
 }
 
+# Test bitfields at non-zero offsets in a struct.
+
+proc bitfield_at_offset {} {
+    global decimal
+    global hex
+    global gdb_prompt
+    global srcfile
+
+    gdb_breakpoint break5
+    if [gdb_test "cont" "Break.*break5 \\(\\) at .*$srcfile:$decimal.*" "continuing to break5"] {
+	return
+    }
+
+    set one ".*uc = 0 .*, s1 = 0, u1 = 0, s2 = 0, u2 = 0, s3 = 0, u3 = 5, s9 = 0, u9 = 0, sc = 0.*"
+    set two ".*uc = 0 .*, s1 = 0, u1 = 0, s2 = 0, u2 = 0, s3 = 0, u3 = 3, s9 = 0, u9 = 0, sc = 0.*"
+    gdb_test "print container" "$one$two" "distinct bitfields in container"
+    gdb_test "print container.one.u3" ".* = 5"
+    gdb_test "print container.two.u3" ".* = 3"
+}
+
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
@@ -264,3 +284,5 @@ if [istarget "mips-idt-*"] then {
     gdb_load ${binfile}
 }
 bitfield_signedness
+
+bitfield_at_offset