From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14380 invoked by alias); 28 Aug 2009 18:49:55 -0000 Received: (qmail 14368 invoked by uid 22791); 28 Aug 2009 18:49:53 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_93 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 28 Aug 2009 18:49:48 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id B948D7C09A for ; Fri, 28 Aug 2009 18:49:45 +0000 (GMT) Received: from caradoc.them.org (209.195.188.212.nauticom.net [209.195.188.212]) by nan.false.org (Postfix) with ESMTP id 84C6A108C6 for ; Fri, 28 Aug 2009 18:49:45 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1Mh6W6-0001dE-9N for gdb-patches@sourceware.org; Fri, 28 Aug 2009 14:49:42 -0400 Date: Fri, 28 Aug 2009 19:55:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: [commit] Fix bitfield errors - PR 10565 Message-ID: <20090828184942.GA5711@caradoc.them.org> Mail-Followup-To: gdb-patches@sourceware.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-08/txt/msg00522.txt.bz2 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 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 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