[RFA] Setting long long bitfields

[RFA] Setting long long bitfields

[Paul Hilfinger]

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.

OK?

Paul Hilfinger

2004-10-31  Paul N. Hilfinger  <Hilfinger@gnat.com>

	* 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);
 }
 \f
 /* 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;
+}

Re: [RFA] Setting long long bitfields

[Andrew Cagney]

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  <Hilfinger@gnat.com>
> 
> 	* 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);
>  }
>  \f
>  /* 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;
> +}
> 

Re: [RFA] Setting long long bitfields

[Andreas Schwab]

Andrew Cagney <cagney@gnu.org> writes:

> One possible tweak, several of us have an aversion to "?:", it would be
> nice if it wasn't there :-)

Like this perhaps:

+  ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFA] Setting long long bitfields

[Paul Hilfinger]

 > > One possible tweak, several of us have an aversion to "?:", it would be
 > > nice if it wasn't there :-)
 > 
 > Like this perhaps:
 > 
 > +  ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);

Andreas,

I had considered exactly that line, but unfortunately ran across the
following really irritating provision in the C standard:

    "If the value of the right operand is negative or is 
     *greater than or equal to* the width of the promoted left
     operand, the behavior is undefined."

[I know why the provision is there, of course: many machines treat a
shift of wordsize bits as 0, because they mask off bits to the left,
but it's STILL irritating.]

Now, I presume we will never encounter a bitfield size of 8 * sizeof
(ULONGEST), but the original code apparently tried to bullet-proof
against this possibility, so I just went along.

Paul

Re: [RFA] Setting long long bitfields

[Andreas Schwab]

Paul Hilfinger <hilfingr@EECS.Berkeley.EDU> writes:

>  > > One possible tweak, several of us have an aversion to "?:", it would be
>  > > nice if it wasn't there :-)
>  > 
>  > Like this perhaps:
>  > 
>  > +  ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);
>
> Andreas,
>
> I had considered exactly that line, but unfortunately ran across the
> following really irritating provision in the C standard:
>
>     "If the value of the right operand is negative or is 
>      *greater than or equal to* the width of the promoted left
>      operand, the behavior is undefined."

That cannot happen, because bitsize will never be zero.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFA] Setting long long bitfields

[Paul Hilfinger]

I have committed the revised version of this patch, below.

Paul Hilfinger

2004-11-01  Paul N. Hilfinger  <Hilfinger@gnat.com>

	* values.c (modify_field): Correct handling of bit-fields that
	don't fit in 32 bits.  Use unsigned operations throughout and 
	simplify the code a bit.  Document preconditions.

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	1 Nov 2004 10:42:29 -0000
@@ -1070,45 +1070,42 @@ unpack_field_as_long (struct type *type,
 /* Modify the value of a bitfield.  ADDR points to a block of memory in
    target byte order; the bitfield starts in the byte pointed to.  FIELDVAL
    is the desired value of the field, in host byte order.  BITPOS and BITSIZE
-   indicate which bits (in target bit order) comprise the bitfield.  */
+   indicate which bits (in target bit order) comprise the bitfield.  
+   Requires 0 < BITSIZE <= lbits, 0 <= BITPOS+BITSIZE <= lbits, and
+   0 <= BITPOS, where lbits is the size of a LONGEST in bits.  */
 
 void
 modify_field (char *addr, LONGEST fieldval, int bitpos, int bitsize)
 {
-  LONGEST oword;
+  ULONGEST oword;
+  ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);
 
   /* 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);
 }
 \f
 /* Convert C numbers into newly allocated values */

Re: [RFA] Setting long long bitfields

[Paul N. Hilfinger]

> How can bitsize ever become zero?  Note that the original code contains a
> shift by bitsize - 1, which is undefined as well in this case.

Actually, Ada programs, unlike C, can generate fields of width 0.  I see,
however, that (somewhat accidentally) that case is handled as a 0-byte
ordinary byte-aligned field (where the alignment doesn't matter, of course,
since 0 bytes of data are modified).  Very well, I will adopt Andreas'
suggestion, and also document the precondition in the comment.  

Thanks.

Paul