This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] MIPS bit field failures in gdb.base/store.exp
- From: Pedro Alves <alves dot ped at gmail dot com>
- To: lgustavo at codesourcery dot com, "'gdb-patches at sourceware dot org'" <gdb-patches at sourceware dot org>
- Date: Mon, 29 Sep 2014 14:30:19 +0100
- Subject: Re: [PATCH] MIPS bit field failures in gdb.base/store.exp
- Authentication-results: sourceware.org; auth=none
- References: <5413534F dot 7000705 at codesourcery dot com> <541C63DF dot 8090206 at redhat dot com> <541C6A43 dot 2000200 at codesourcery dot com> <54246DAE dot 6080208 at codesourcery dot com> <5425835B dot 609 at redhat dot com> <5429523F dot 3000706 at codesourcery dot com>
On 09/29/2014 01:36 PM, Luis Machado wrote:
> 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.
The proper solution for an arch that needs to treat bitfields differently
might well be to do without gdbarch_convert_register_p and change
gdbarch_value_to_register's parameters to
'gdbarch_value_to_register(gdbarch, toval, fromval)', and rename it
to gdbarch_register_assign while at it, as it's only called by value_assign.
Like:
if (gdbarch_register_assign_p (gdbarch))
{
gdbarch_register_assign (gdbarch, toval, fromval);
}
else
{
// default fallback
}
(Or install the fallback code as fallback gdbarch_register_assign
implementation and then just call gdbarch_register_assign directly.)
Seems unnecessary to do until we find a user that wants to treat
bitfields differently though. Or viewed another way, we're discussing
what that "default fallback" code should look like. :-)
> 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
Yes, please.
> or do you want to author it?
Nope.
Thanks,
Pedro Alves