Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis Machado <lgustavo@codesourcery.com>
To: Pedro Alves <palves@redhat.com>,
	"'gdb-patches@sourceware.org'"	<gdb-patches@sourceware.org>
Subject: Re: [PATCH] MIPS bit field failures in gdb.base/store.exp
Date: Fri, 19 Sep 2014 17:39:00 -0000	[thread overview]
Message-ID: <541C6A43.2000200@codesourcery.com> (raw)
In-Reply-To: <541C63DF.8090206@redhat.com>

Hi,

On 09/19/2014 02:11 PM, Pedro Alves wrote:
> Hi Luis,
>
> On 09/12/2014 09:10 PM, Luis Machado wrote:
>
>> Now, GDB knows how to do bit field assignment properly, but MIPS is one
>> of those architectures that uses a hook for the register-to-value
>> conversion. Although we can properly tell when the type being passed is
>> a structure or union, we cannot tell when it is a bit field, because the
>> bit field data lives in a value structure. Such data only lives in a
>> "type" structure when the parent structure is being referenced, thus you
>> can collect them from the flds_bnds members.
>>
>
>> A bit field type structure looks pretty much the same as any other
>> primitive type like int or char, so we can't distinguish them. Forcing
>> more fields into the type structure wouldn't help much, because the type
>> structs are shared.
>
> If we can't do that, then ...
>
>> It feels to me GDB's type system is a bit dated and needs to be more
>> precise about what it is describing, but for now i just want to fix a
>> pending bug.
>
> ... this leaves me wondering about what you're thinking we'd
> do differently if we had infinite time?
>

That is a dangerous question! It could create infinite work. :-)

In a quick response, I'd move that data out of the value struct and put 
it in the type struct, but that would require additional changes to 
prevent sharing the base types that have bit field data in them.

For a longer answer, i think for everyday normal use GDB's current type 
mechanism works OK, but if you go beyond that and start doing alternate 
named address spaces, different kinds of pointers and strange type 
castings you start to reach the limits of the assumptions GDB makes (or 
has made in the past). There is definitely room for improvement in the 
type system.

The difficulty to change this is because the type mechanism is a bit 
entangled with the symbol machinery and the DWARF reader, with potential 
for breakage.

I haven't dealt much with C++ types, but from what I've dealt with, it 
seems we have some difficulties in there too. Again, it works, but could 
be better.

Anyway, i think this has potential for a bigger and interesting discussion.

>>
>> The most elegant solution i could find without having to touch a number
>> of other type-related data structures is making the
>> gdbarch_convert_register_p predicate accept a value structure instead of
>> a type structure. That way we can properly tell when a bit field is
>> being manipulated in registers.
>>
>> There is still a little problem though. We don't always have a
>> meaningful value struct to pass to this predicate, like both ocurrences
>> of it in findvar.c. In those cases i went for a dummy value.
>>
>> In the end, it is functional but a bit ugly. Unless folks have a better
>> suggestion, is this ok?
>
> Well, why not pass down value_bitsize() (an integer) instead of
> the whole value?
>

I thought about doing that, but seems weird to have a function parameter 
just for the bit field information. I'm not sure if we're not missing 
other bits of information that may be useful for register/value 
conversion, that's why i went with passing the value structure. It holds 
everything anyway, not just the bit field data.

Passing the bit size is OK with me though.

>> I did tests with x86, mips32 be/le and mips64 be/le. No regressions found.
>>
>> The lack of bit field data in the type struct also affects other
>> functions that rely on type descriptions, though there may not be
>> explicit failures due to those yet.
>
> That's a bit vague.  :-)  Got pointers?
>

I meant the functions that have to act on the type that is passed as a 
parameter rather than the value itself, like the following:

gdbarch_register_to_value
gdbarch_value_to_register
gdbarch_value_from_register

The dummy call hook already acts on the value structs of the parameters, 
so it doesn't sound totally wrong to want to pass the value struct in 
this case too.

Luis


  reply	other threads:[~2014-09-19 17:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-12 20:11 Luis Machado
2014-09-19 16:45 ` Luis Machado
2014-09-19 17:12 ` Pedro Alves
2014-09-19 17:39   ` Luis Machado [this message]
2014-09-25 19:32     ` Luis Machado
2014-09-26 15:45       ` Pedro Alves
2014-09-29 12:36         ` Luis Machado
2014-09-29 13:30           ` Pedro Alves
2014-09-29 17:35             ` Luis Machado
2014-09-30 11:00               ` Pedro Alves
2014-10-03 11:23                 ` Luis Machado

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=541C6A43.2000200@codesourcery.com \
    --to=lgustavo@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox