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

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Wed, 06 Feb 2013 21:39:10 +0100, Sergio Durigan Junior wrote:
> gdb/
> 2013-02-06  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* valops.c (value_assign): Handling bitfield offset in
> 	`lval_internalvar_component' case.
> 
> gdb/testsuite/
> 2013-02-06  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* gdb.base/bitfields.c (struct internalvartest): New declaration.
> 	* gdb.base/bitfields.exp (bitfield_internalvar): New function.

OK for check-in with the bits below.


> --- 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\}" "" \

Backslashes are excessive here, parentheses are not special characters for the
TCL interpretation and this string is not regexp:
       gdb_test "set \$myvar = (struct internalvartest) \{0\}" "" \


> +      "set internal var"
> +
> +    # Now, we set the proper bits.
[...]
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -1233,11 +1233,18 @@ 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);
> +
> +	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))
	  {
	    /* VALUE_INTERNALVAR below corresponds to the parent value while
	       offset is relative to the parent value.  */
	    gdb_assert (value_parent (value_parent (toval)) == NULL);
	    offset += value_offset (value_parent (toval));
	  }

And it was not so clear to me on the first look so I have added this comment
if you are OK with it.


> +
> +	set_internalvar_component (VALUE_INTERNALVAR (toval),
> +				   offset,
> +				   value_bitpos (toval),
> +				   value_bitsize (toval),
> +				   fromval);
> +      }
>        break;
>  


Thanks,
Jan