Re: [PATCH] Handle bitfields inside inner structs for internalvars

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Saturday, February 09 2013, Jan Kratochvil wrote:

> On Sat, 09 Feb 2013 05:52:32 +0100, Sergio Durigan Junior wrote:
>> On Friday, February 08 2013, Jan Kratochvil wrote:
>> >> +	if (value_bitsize (toval))
>> >> +	  offset += value_offset (value_parent (toval));
>> >
>> > value_address rather tests for value_parent existence; although value_bitsize
>> > is right as value_parent is currently not used elsewhere.
>> >
>> > 	if (value_parent (toval))
>> 
>> Do you think it's clearer to use `value_parent' here instead of
>> `value_bitsize'?
>
> Choose any way but therefore put there a comment that value_parent is non-NULL
> iff value_bitsize is non-zero.

Ok, thanks, I committed the patch below.

> Otherwise I was curious - what to do if value_parent exists but TOVAL is not
> a bitfield?  Isn't it a forgotten case?  (It is not but...)

According to comments in gdb/value.{c,h}, value_parent is only used iff
the we are dealing with bitfields, so I guess this case is covered
(otherwise it is a bug).  Was it a rhetorical question?

Checked-in:
        http://sourceware.org/ml/gdb-cvs/2013-02/msg00071.html

Thanks,

-- 
Sergio

2013-02-11  Sergio Durigan Junior  <sergiodj@redhat.com>

	* valops.c (value_assign): Handling bitfield offset in
	`lval_internalvar_component' case.

2013-02-11  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.base/bitfields.c (struct internalvartest): New declaration.
	* gdb.base/bitfields.exp (bitfield_internalvar): New function.

---
 gdb/testsuite/gdb.base/bitfields.c   |   16 ++++++++++++++++
 gdb/testsuite/gdb.base/bitfields.exp |   26 ++++++++++++++++++++++++++
 gdb/valops.c                         |   26 +++++++++++++++++++++-----
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.base/bitfields.c b/gdb/testsuite/gdb.base/bitfields.c
index ed1634c..3a6b76f 100644
--- a/gdb/testsuite/gdb.base/bitfields.c
+++ b/gdb/testsuite/gdb.base/bitfields.c
@@ -23,6 +23,22 @@ struct fields
   signed char	sc    ;
 } flags;
 
+struct internalvartest
+{
+  unsigned int a : 1;
+  struct
+    {
+      unsigned int b : 1;
+      struct
+	{
+	  unsigned int c : 1;
+	  signed int   d : 1;
+	} deep;
+      signed int   e : 1;
+    } inner;
+  signed int   f : 1;
+} dummy_internalvartest;
+
 void break1 ()
 {
 }
diff --git a/gdb/testsuite/gdb.base/bitfields.exp b/gdb/testsuite/gdb.base/bitfields.exp
index 9095736..82f7b10 100644
--- a/gdb/testsuite/gdb.base/bitfields.exp
+++ b/gdb/testsuite/gdb.base/bitfields.exp
@@ -245,6 +245,31 @@ proc bitfield_at_offset {} {
     gdb_test "print container.two.u3" ".* = 3"
 }
 
+proc bitfield_internalvar {} {
+    global gdb_prompt
+
+    # First, we create an internal var holding an instance of
+    # the struct (zeroed out).
+    gdb_test "set \$myvar = (struct internalvartest) \{0\}" "" \
+      "set internal var"
+
+    # Now, we set the proper bits.
+    gdb_test_no_output "set \$myvar.a = 0"
+    gdb_test_no_output "set \$myvar.inner.b = 1"
+    gdb_test_no_output "set \$myvar.inner.deep.c = 0"
+    gdb_test_no_output "set \$myvar.inner.deep.d = -1"
+    gdb_test_no_output "set \$myvar.inner.e = 1"
+    gdb_test_no_output "set \$myvar.f = 1"
+
+    # Here comes the true testing.
+    gdb_test "print \$myvar.a" "\\$\[0-9\]\+ = 0"
+    gdb_test "print \$myvar.inner.b" "\\$\[0-9\]\+ = 1"
+    gdb_test "print \$myvar.inner.deep.c" "\\$\[0-9\]\+ = 0"
+    gdb_test "print \$myvar.inner.deep.d" "\\$\[0-9\]\+ = -1"
+    gdb_test "print \$myvar.inner.e" "\\$\[0-9\]\+ = -1"
+    gdb_test "print \$myvar.f" "\\$\[0-9\]\+ = -1"
+}
+
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
@@ -256,3 +281,4 @@ bitfield_containment
 bitfield_unsignedness
 bitfield_signedness
 bitfield_at_offset
+bitfield_internalvar
diff --git a/gdb/valops.c b/gdb/valops.c
index 2132f3e..93c09d8 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1233,11 +1233,27 @@ value_assign (struct value *toval, struct value *fromval)
 				   VALUE_INTERNALVAR (toval));
 
     case lval_internalvar_component:
-      set_internalvar_component (VALUE_INTERNALVAR (toval),
-				 value_offset (toval),
-				 value_bitpos (toval),
-				 value_bitsize (toval),
-				 fromval);
+      {
+	int offset = value_offset (toval);
+
+	/* Are we dealing with a bitfield?
+
+	   It is important to mention that `value_parent (toval)' is
+	   non-NULL iff `value_bitsize (toval)' is non-zero.  */
+	if (value_bitsize (toval))
+	  {
+	    /* VALUE_INTERNALVAR below refers to the parent value, while
+	       the offset is relative to this parent value.  */
+	    gdb_assert (value_parent (value_parent (toval)) == NULL);
+	    offset += value_offset (value_parent (toval));
+	  }
+
+	set_internalvar_component (VALUE_INTERNALVAR (toval),
+				   offset,
+				   value_bitpos (toval),
+				   value_bitsize (toval),
+				   fromval);
+      }
       break;
 
     case lval_memory:
-- 
1.7.7.6