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

[Simon Marchi <simon dot marchi at ericsson dot com>]

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.  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;
>  }

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.

Thanks,

Simon