This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [4/9] Unify init_type and arch_type interface and helpers
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: simon dot marchi at ericsson dot com (Simon Marchi)
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 17 Jan 2017 15:16:33 +0100 (CET)
- Subject: Re: [4/9] Unify init_type and arch_type interface and helpers
- Authentication-results: sourceware.org; auth=none
Simon Marchi wrote:
> On 16-08-25 11:06 AM, Ulrich Weigand wrote:
> > +static int
> > +verify_floatformat (int bit, const struct floatformat **floatformats)
> > +{
> > + if (bit == -1)
> > + {
> > + gdb_assert (floatformats != NULL);
> > + gdb_assert (floatformats[0] != NULL && floatformats[1] != NULL);
> > + bit = floatformats[0]->totalsize;
> > + }
> > + gdb_assert (bit >= 0);
> > +
> > + if (floatformats != NULL)
> > + {
> > + size_t len = bit / TARGET_CHAR_BIT;
> > +
> > + gdb_assert (len >= floatformat_totalsize_bytes (floatformats[0]));
> > + gdb_assert (len >= floatformat_totalsize_bytes (floatformats[1]));
>
> Hi Ulrich,
>
> I am in the process of syncing with master our port of GDB for which
> HOST_CHAR_BIT (8) != TARGET_CHAR_BIT (16). I believe that the piece of code
> compares target bytes and host bytes (which is not good), whereas the old code
> was comparing host bytes with host bytes.
Everything involved here should be target bytes (and target types). There
should not have been any change from the old code either:
> Here's the old one:
>
> > - if (floatformats != NULL)
> > - {
> > - size_t len = TYPE_LENGTH (t);
> > -
> > - gdb_assert (len >= floatformat_totalsize_bytes (floatformats[0]));
> > - gdb_assert (len >= floatformat_totalsize_bytes (floatformats[1]));
> > - }
> > -
> > return t;
> > }
since the type "t" was just created via
t = arch_type (gdbarch, TYPE_CODE_FLT, bit / TARGET_CHAR_BIT, name);
so TYPE_LENGTH (t) should always equal bit / TARGET_CHAR_BIT.
> The new code triggers the assert, because len is 2 target bytes and
> floatformat_totalsize_bytes is 4 host bytes. If it's indeed an error, I can
> make a patch for it, but I wanted to check with you first.
Why would floatformat_totalsize_bytes be host bytes? I think this is
*intended* to be target bytes, as the floatformat in general is intended
to describe the *target* floating-point format. I do see that doublest.c
does indeed make the assumption that a character has 8 bits, but that is
something that will need to be fixed if you want to support targets with
a different byte size; I don't see how this is related to this particular
patch of mine ...
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com