Re: [PATCH] MIPS bit field failures in gdb.base/store.exp

[Luis Machado <lgustavo@codesourcery.com>]

Hi,

On 09/26/2014 12:16 PM, Pedro Alves wrote:
> On 09/25/2014 08:31 PM, Luis Machado wrote:
>
>> ping! Any ideas on different approaches suitable for this problem or is
>> the proposed fix ok (with either passing a value struct or a bit size)?
>
> Sorry, it's not easy to have a quick opinion without thinking this
> through...
>
> So, in value_assign, the case in question, we see:
>
> 	gdbarch = get_frame_arch (frame);
> 	if (gdbarch_convert_register_p (gdbarch, VALUE_REGNUM (toval), type))
> 	  {
> 	    /* If TOVAL is a special machine register requiring
> 	       conversion of program values to a special raw
> 	       format.  */
> 	    gdbarch_value_to_register (gdbarch, frame,
> 				       VALUE_REGNUM (toval), type,
> 				       value_contents (fromval));
> 	  }
>
> Notice how gdbarch_value_to_register takes the fromval's contents
> as a buffer, only, and isn't passed down anything that would make it
> possible to find out whether it's writing to a bitfield, so that
> the implementation could do a read-modify-write itself and
> write to the proper bitfield offset.
>
> So, it seems to me that until we find an arch that needs to handle
> bitfields especially (I'm having trouble imagining why that
> would be necessary), we should just change value_assign's
> lval_register handling from:
>
> 	if (gdbarch_convert_register_p (gdbarch, VALUE_REGNUM (toval), type))
> 	  {	
>               gdbarch_value_to_register ();
> 	  }
> 	else
> 	  {
> 	    if (value_bitsize (toval))
>                 {
>                     // read-modify-write
>                 }
>              else
> 	       {
> 		   put_frame_register_bytes ();
>                 }
>            }
>
> to:
>
>         if (value_bitsize (toval))
>            {
>                // read-modify-write
>            }
> 	else
> 	  {
>               if (gdbarch_convert_register_p (gdbarch, VALUE_REGNUM (toval), type))
> 	       {
>                    gdbarch_value_to_register ();
> 	       }
>              else
> 	       {
> 		  put_frame_register_bytes ();
>                 }
>            }

Though a bit less generic, that also seems to be a reasonable solution 
for now, and it fixes the failures i saw for MIPS. Out of the top of my 
head i also don't recall a target that handles bit fields in a special 
way. Should i go with this patch for the next submission or do you want 
to author it?

Thanks,
Luis