Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [commit] Fix bitfield errors - PR 10565
@ 2009-08-28 19:55 Daniel Jacobowitz
  2009-09-27 21:48 ` [rfc] Fix bitfield regressions on 64-bit big-endian targets Ulrich Weigand
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Jacobowitz @ 2009-08-28 19:55 UTC (permalink / raw)
  To: gdb-patches

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


^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-09-29  0:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-28 19:55 [commit] Fix bitfield errors - PR 10565 Daniel Jacobowitz
2009-09-27 21:48 ` [rfc] Fix bitfield regressions on 64-bit big-endian targets Ulrich Weigand
2009-09-27 22:28   ` Daniel Jacobowitz
2009-09-28  9:20     ` Ulrich Weigand
2009-09-28 16:33       ` Daniel Jacobowitz
2009-09-28 18:07       ` Joel Brobecker
2009-09-28 18:32         ` Daniel Jacobowitz
2009-09-29  0:44           ` Ulrich Weigand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox