From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20647 invoked by alias); 31 Oct 2004 18:13:24 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 20633 invoked from network); 31 Oct 2004 18:13:19 -0000 Received: from unknown (HELO localhost.redhat.com) (24.42.65.225) by sourceware.org with SMTP; 31 Oct 2004 18:13:19 -0000 Received: from [127.0.0.1] (localhost.localdomain [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 923CA129D8C; Sun, 31 Oct 2004 13:13:00 -0500 (EST) Message-ID: <41852B2B.6030305@gnu.org> Date: Sun, 31 Oct 2004 18:13:00 -0000 From: Andrew Cagney User-Agent: Mozilla Thunderbird 0.8 (X11/20041020) MIME-Version: 1.0 To: Paul Hilfinger Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] Setting long long bitfields References: <20041031090847.2E303F2985@nile.gnat.com> In-Reply-To: <20041031090847.2E303F2985@nile.gnat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-10/txt/msg00545.txt.bz2 Paul Hilfinger wrote: > The patch below fixes a small problem with assignments in GDB to > long long bitfields (which, while not a standard part of C are provided > by GCC). The existing code computes the correct mask for zeroing out > the old value, but uses int computations in the code that truncates > the field value and tests whether it fits. Bit-fields that don't fit > in an int are rare in C code, but they do crop up more in Ada, which > allows considerably more aggressive packing. I have included a draft of > a new set of testsuite files (in C) that tickle the problem. Looks under patch, nope no testcase, looking forward to it :-) Yes, ok. One possible tweak, several of us have an aversion to "?:", it would be nice if it wasn't there :-) Andrew > Paul Hilfinger > > 2004-10-31 Paul N. Hilfinger > > * values.c (modify_field): Correct handling of bit-fields that > don't fit in 32 bits. Use unsigned operations consistently and > simplify the code a bit. > > Index: gdb/values.c > =================================================================== > RCS file: /cvs/src/src/gdb/values.c,v > retrieving revision 1.71 > diff -u -p -r1.71 values.c > --- gdb/values.c 28 Jul 2004 02:46:24 -0000 1.71 > +++ gdb/values.c 31 Oct 2004 08:26:17 -0000 > @@ -1075,40 +1075,36 @@ unpack_field_as_long (struct type *type, > void > modify_field (char *addr, LONGEST fieldval, int bitpos, int bitsize) > { > - LONGEST oword; > + ULONGEST oword; > + ULONGEST mask = (bitsize == 8 * (int) sizeof (ULONGEST) > + ? ~(ULONGEST) 0 : ((ULONGEST) 1 << bitsize) - 1); > > /* If a negative fieldval fits in the field in question, chop > off the sign extension bits. */ > - if (bitsize < (8 * (int) sizeof (fieldval)) > - && (~fieldval & ~((1 << (bitsize - 1)) - 1)) == 0) > - fieldval = fieldval & ((1 << bitsize) - 1); > + if ((~fieldval & ~(mask >> 1)) == 0) > + fieldval &= mask; > > /* Warn if value is too big to fit in the field in question. */ > - if (bitsize < (8 * (int) sizeof (fieldval)) > - && 0 != (fieldval & ~((1 << bitsize) - 1))) > + if (0 != (fieldval & ~mask)) > { > /* FIXME: would like to include fieldval in the message, but > we don't have a sprintf_longest. */ > warning ("Value does not fit in %d bits.", bitsize); > > /* Truncate it, otherwise adjoining fields may be corrupted. */ > - fieldval = fieldval & ((1 << bitsize) - 1); > + fieldval &= mask; > } > > - oword = extract_signed_integer (addr, sizeof oword); > + oword = extract_unsigned_integer (addr, sizeof oword); > > /* Shifting for bit field depends on endianness of the target machine. */ > if (BITS_BIG_ENDIAN) > bitpos = sizeof (oword) * 8 - bitpos - bitsize; > > - /* Mask out old value, while avoiding shifts >= size of oword */ > - if (bitsize < 8 * (int) sizeof (oword)) > - oword &= ~(((((ULONGEST) 1) << bitsize) - 1) << bitpos); > - else > - oword &= ~((~(ULONGEST) 0) << bitpos); > + oword &= ~(mask << bitpos); > oword |= fieldval << bitpos; > > - store_signed_integer (addr, sizeof oword, oword); > + store_unsigned_integer (addr, sizeof oword, oword); > } > > /* Convert C numbers into newly allocated values */ > Index: current-public.116/gdb/testsuite/gdb.base/bitfields2.exp > --- current-public.116/gdb/testsuite/gdb.base/bitfields2.exp Sun, 31 Oct 2004 01:11:22 -0700 hilfingr () > +++ merge.238(w)/gdb/testsuite/gdb.base/bitfields2.exp Sun, 31 Oct 2004 00:51:17 -0700 hilfingr (GdbPub/q/c/36_bitfields2 1.1 644) > @@ -0,0 +1,325 @@ > +# Copyright 1992, 1994, 1995, 1997, 2004 Free Software Foundation, Inc. > + > +# 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 2 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, write to the Free Software > +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. > + > +# Please email any bugs, comments, and/or additions to this file to: > +# bug-gdb@prep.ai.mit.edu > + > +# This file was adapted from bitfields.exp by Paul Hilfinger > +# (Hilfinger@gnat.com) > + > +# > +# Tests for bit-fields that do not fit in type (unsigned) int, but do fit > +# in type (unsigned) long long. We perform essentially the same tests as > +# in bitfields.c, which considers only bit-fields that are <= 9 bits long. > +# > + > +if $tracelevel then { > + strace $tracelevel > +} > + > +set prms_id 0 > +set bug_id 0 > + > +set testfile "bitfields2" > +set srcfile ${testfile}.c > +set binfile ${objdir}/${subdir}/${testfile} > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { > + gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail." > +} > + > +set no_signed 0 > + > +# > +# Test bitfield locating and uniqueness. > +# For each member, set that member to 1 and verify that the member (and only > +# that member) is 1, then reset it back to 0. > +# > + > +proc bitfield_uniqueness {} { > + global decimal > + global hex > + global gdb_prompt > + global srcfile > + > + if { ! [runto break1] } { > + gdb_suppress_tests; > + } > + > + if [gdb_test "print flags" ".*u1 = 0, u2 = 0, u3 = 0, s1 = 1, s2 = 0, s3 = 0.*" "bitfield uniqueness (s1)"] { > + gdb_suppress_tests; > + } > + if [gdb_test "cont" "Break.*break1 \\(\\) at .*$srcfile:$decimal.*" "continuing to break1 #1"] { > + gdb_suppress_tests; > + } > + if [gdb_test "print flags" ".*u1 = 1, u2 = 0, u3 = 0, s1 = 0, s2 = 0, s3 = 0.*" "bitfield uniqueness (u1)"] { > + gdb_suppress_tests; > + } > + if [gdb_test "cont" "Break.*break1 \\(\\) at .*$srcfile:$decimal.*" "continuing to break1 #2"] { > + gdb_suppress_tests; > + } > + if [gdb_test "print flags" ".*u1 = 0, u2 = 0, u3 = 0, s1 = 0, s2 = 1, s3 = 0.*" "bitfield uniqueness (s2)"] { > + gdb_suppress_tests; > + } > + if [gdb_test "cont" "Break.*break1 \\(\\) at .*$srcfile:$decimal.*" "continuing to break1 #3"] { > + gdb_suppress_tests; > + } > + if [gdb_test "print flags" ".*u1 = 0, u2 = 1, u3 = 0, s1 = 0, s2 = 0, s3 = 0.*" "bitfield uniqueness (u2)"] { > + gdb_suppress_tests; > + } > + if [gdb_test "cont" "Break.*break1 \\(\\) at .*$srcfile:$decimal.*" "continuing to break1 #4"] { > + gdb_suppress_tests; > + } > + if [gdb_test "print flags" ".*u1 = 0, u2 = 0, u3 = 0, s1 = 0, s2 = 0, s3 = 1.*" "bitfield uniqueness (s3)"] { > + gdb_suppress_tests; > + } > + if [gdb_test "cont" "Break.*break1 \\(\\) at .*$srcfile:$decimal.*" "continuing to break1 #5"] { > + gdb_suppress_tests; > + } > + if [gdb_test "print flags" ".*u1 = 0, u2 = 0, u3 = 1, s1 = 0, s2 = 0, s3 = 0.*" "bitfield uniqueness (u3)"] { > + gdb_suppress_tests > + } > + gdb_stop_suppressing_tests; > +} > + > + > +# > +# Test bitfield containment. > +# Fill alternating fields with all 1's and verify that none of the bits > +# "bleed over" to the other fields. > +# > + > +proc bitfield_containment {} { > + global decimal > + global hex > + global gdb_prompt > + global srcfile > + > + delete_breakpoints > + > + if { ![runto break2] } { > + gdb_suppress_tests > + } > + > + # If program is compiled with Sun CC, signed fields print out as their > + # actual sizes; if compiled with gcc, they print out as 0xffffffff. > + if [gdb_test "print/x flags" "= {u1 = 0x7fff, u2 = 0x0, u3 = 0xffff, s1 = 0x0, s2 = 0x(1ffffffff|f*), s3 = 0x0}" "bitfield containment #1"] { > + gdb_suppress_tests > + } > + > + if [gdb_test "cont" "Break.*break2 \\(\\) at .*$srcfile:$decimal.*" "continuing to break2"] { > + gdb_suppress_tests > + } > + > + if [gdb_test "print/x flags" "= {u1 = 0x0, u2 = 0x1ffffffff, u3 = 0x0, s1 = 0x(7fff|f*), s2 = 0x0, s3 = 0xf*}" "bitfield containment #2"] { > + gdb_suppress_tests > + } > + gdb_stop_suppressing_tests; > +} > + > +# Test unsigned bitfields for unsignedness and range. > +# Fill the unsigned fields with the maximum positive value and verify that > +# the values are printed correctly. > + > +proc bitfield_unsignedness {} { > + global decimal > + global hex > + global gdb_prompt > + global srcfile > + > + delete_breakpoints > + > + if { ![runto break3] } { > + gdb_suppress_tests > + } > + > + if [gdb_test "print flags" ".*u1 = 32767, u2 = 8589934591, u3 = 65535, s1 = 0, s2 = 0, s3 = 0.*" "unsigned bitfield ranges"] { > + gdb_suppress_tests > + } > + gdb_stop_suppressing_tests; > +} > + > +# > +# Test signed bitfields for signedness and range. > +# Fill the signed fields with the maximum positive value, then the maximally > +# negative value, then -1, and verify in each case that the values are > +# printed correctly. > +# > + > +proc bitfield_signedness {} { > + global decimal > + global hex > + global gdb_prompt > + global srcfile > + global no_signed > + > + delete_breakpoints > + > + if { ! [runto break4] } { > + gdb_suppress_tests > + } > + > + if [gdb_test "print flags" "= {.*u1 = 0, u2 = 0, u3 = 0, s1 = 16383, s2 = 4294967295, s3 = 32767.*}" "signed bitfields, max positive values"] { > + gdb_suppress_tests > + } > + > + if [gdb_test "cont" "Break.*break4 \\(\\) at .*$srcfile:$decimal.*" "continuing to break4 #1"] { > + gdb_suppress_tests > + } > + > + # Determine if the target has signed bitfields so we can xfail the > + # the signed bitfield tests if it doesn't. > + set no_signed 1 > + send_gdb "print i\n" > + gdb_expect { > + -re ".* = -32768.*$gdb_prompt $" { > + set no_signed 0 > + pass "determining signed-ness of bitfields" > + } > + -re ".* = 32768.*$gdb_prompt $" { > + pass "determining signed-ness of bitfields" > + setup_xfail "*-*-*" > + } > + -re ".*$gdb_prompt $" { > + fail "determining signed-ness of bitfields" > + gdb_suppress_tests > + } > + default { > + fail "determining signed-ness of bitfields" ; > + gdb_suppress_tests; > + } > + } > + > + if [gdb_test "print flags" "u1 = 0, u2 = 0, u3 = 0, s1 = -16384, s2 = -4294967296, s3 = -32768.*" "signed bitfields, max negative values"] { > + gdb_suppress_tests > + } > + > + if [gdb_test "cont" "Break.*break4 \\(\\) at .*$srcfile:$decimal.*" "continuing to break4 #2"] { > + gdb_suppress_tests > + } > + > + if [gdb_test "print flags" "u1 = 0, u2 = 0, u3 = 0, s1 = -1, s2 = -1, s3 = -1.*" "signed bitfields with -1"] { > + gdb_suppress_tests > + } > + # Hmmmm??? > + gdb_stop_suppressing_tests; > +} > + > + > +# Test setting of long long bit fields from within GDB. > + > +proc bitfield_set {} { > + global decimal > + global hex > + global gdb_prompt > + global srcfile > + global no_signed > + > + delete_breakpoints > + > + if { ! [runto break5] } { > + gdb_suppress_tests > + } > + > + set big_set_failed 0 > + set test "set long long unsigned bitfield" > + gdb_test_multiple "print flags.u2 = 0x100000000" $test { > + -re "warning: Value does not fit.*$gdb_prompt $" { > + fail "$test" > + gdb_suppress_tests > + } > + -re "= 4294967296.*$gdb_prompt $" { > + pass "$test" > + } > + } > + > + set test "set long long signed bitfield positive" > + gdb_test_multiple "print flags.s2 = 0x80000000" $test { > + -re "warning: Value does not fit.*$gdb_prompt $" { > + fail "$test" > + gdb_suppress_tests > + } > + -re "= 2147483648.*$gdb_prompt $" { > + pass "$test" > + } > + } > + > + if [gdb_test "print flags" "u1 = 0, u2 = 4294967296, u3 = 0, s1 = 0, s2 = 2147483648, s3 = 0.*" "long long bitfield values after set"] { > + gdb_suppress_tests > + } > + > + if $no_signed then { > + setup_xfail "*-*-*" > + } > + set test "set long long signed bitfield negative" > + gdb_test_multiple "print flags.s2 = -1" $test { > + -re "warning: Value does not fit.*$gdb_prompt $" { > + fail "$test" > + gdb_suppress_tests > + } > + -re "= -1.*$gdb_prompt $" { > + pass "$test" > + } > + } > + > + if $no_signed then { > + setup_xfail "*-*-*" > + } > + if [gdb_test "print flags" "u1 = 0, u2 = 4294967296, u3 = 0, s1 = 0, s2 = -1, s3 = 0.*" "long long bitfield values after set negative"] { > + gdb_suppress_tests > + } > + gdb_stop_suppressing_tests; > +} > + > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir > +gdb_load ${binfile} > + > +send_gdb "set print sevenbit-strings\n" ; gdb_expect -re "$gdb_prompt $" > + > +bitfield_uniqueness > +if [istarget "mips-idt-*"] then { > + # Restart because IDT/SIM runs out of file descriptors. > + gdb_exit > + gdb_start > + gdb_reinitialize_dir $srcdir/$subdir > + gdb_load ${binfile} > +} > +bitfield_containment > +if [istarget "mips-idt-*"] then { > + # Restart because IDT/SIM runs out of file descriptors. > + gdb_exit > + gdb_start > + gdb_reinitialize_dir $srcdir/$subdir > + gdb_load ${binfile} > +} > +bitfield_unsignedness > +if [istarget "mips-idt-*"] then { > + # Restart because IDT/SIM runs out of file descriptors. > + gdb_exit > + gdb_start > + gdb_reinitialize_dir $srcdir/$subdir > + gdb_load ${binfile} > +} > +bitfield_signedness > +if [istarget "mips-idt-*"] then { > + # Restart because IDT/SIM runs out of file descriptors. > + gdb_exit > + gdb_start > + gdb_reinitialize_dir $srcdir/$subdir > + gdb_load ${binfile} > +} > +bitfield_set > + > Index: current-public.116/gdb/testsuite/gdb.base/bitfields2.c > --- current-public.116/gdb/testsuite/gdb.base/bitfields2.c Sun, 31 Oct 2004 01:11:22 -0700 hilfingr () > +++ merge.238(w)/gdb/testsuite/gdb.base/bitfields2.c Sun, 31 Oct 2004 00:51:17 -0700 hilfingr (GdbPub/q/c/37_bitfields2 1.1 644) > @@ -0,0 +1,162 @@ > +/* Test program to test bit field operations on bit fields of large > + integer types. */ > + > +/* This file is expected to fail to compile if the type long long int > + is not supported, but in that case it is irrelevant. */ > + > +#if !defined(__STDC__) && !defined(__cplusplus) > +#define signed /**/ > +#endif > + > +struct fields > +{ > + unsigned long long u1 : 15; > + unsigned long long u2 : 33; > + unsigned long long u3 : 16; > + signed long long s1 : 15; > + signed long long s2 : 33; > + signed long long s3 : 16; > +} flags; > + > +void break1 () > +{ > +} > + > +void break2 () > +{ > +} > + > +void break3 () > +{ > +} > + > +void break4 () > +{ > +} > + > +void break5 () > +{ > +} > + > +void break6 () > +{ > +} > + > +void break7 () > +{ > +} > + > +void break8 () > +{ > +} > + > +void break9 () > +{ > +} > + > +void break10 () > +{ > +} > + > +/* This is used by bitfields.exp to determine if the target understands > + signed bitfields. */ > +int i; > + > +int main () > +{ > + /* For each member, set that member to 1, allow gdb to verify that the > + member (and only that member) is 1, and then reset it back to 0. */ > + > +#ifdef usestubs > + set_debug_traps(); > + breakpoint(); > +#endif > + flags.s1 = 1; > + break1 (); > + flags.s1 = 0; > + > + flags.u1 = 1; > + break1 (); > + flags.u1 = 0; > + > + flags.s2 = 1; > + break1 (); > + flags.s2 = 0; > + > + flags.u2 = 1; > + break1 (); > + flags.u2 = 0; > + > + flags.s3 = 1; > + break1 (); > + flags.s3 = 0; > + > + flags.u3 = 1; > + break1 (); > + flags.u3 = 0; > + > + /* Fill alternating fields with all 1's and verify that none of the bits > + "bleed over" to the other fields. */ > + > + flags.u1 = 0x7FFF; > + flags.u3 = 0xFFFF; > + flags.s2 = -1LL; > + break2 (); > + flags.u1 = 0; > + flags.u3 = 0; > + flags.s2 = 0; > + > + flags.u2 = 0x1FFFFFFFFLL; > + flags.s1 = -1; > + flags.s3 = -1; > + break2 (); > + > + flags.u2 = 0; > + flags.s1 = 0; > + flags.s3 = 0; > + > + /* Fill the unsigned fields with the maximum positive value and verify > + that the values are printed correctly. */ > + > + flags.u1 = 0x7FFF; > + flags.u2 = 0x1FFFFFFFFLL; > + flags.u3 = 0xFFFF; > + break3 (); > + flags.u1 = 0; > + flags.u2 = 0; > + flags.u3 = 0; > + > + /* Fill the signed fields with the maximum positive value, then the maximally > + negative value, then -1, and verify in each case that the values are > + printed correctly. */ > + > + /* Maximum positive values */ > + flags.s1 = 0x3FFF; > + flags.s2 = 0xFFFFFFFFLL; > + flags.s3 = 0x7FFF; > + break4 (); > + > + /* Maximally negative values */ > + flags.s1 = -0x4000; > + flags.s2 = -0x100000000LL; > + flags.s3 = -0x8000; > + > + /* Extract bitfield value so that bitfield.exp can check if the target > + understands signed bitfields. */ > + i = flags.s3; > + break4 (); > + > + /* -1 */ > + flags.s1 = -1; > + flags.s2 = -1; > + flags.s3 = -1; > + break4 (); > + > + flags.s1 = 0; > + flags.s2 = 0; > + flags.s3 = 0; > + > + break5 (); > + > + return 0; > +} >