This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [4/9] Unify init_type and arch_type interface and helpers


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]