Memory corruption for host double format different from target double format

Memory corruption for host double format different from target double format

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 7945 bytes --]

Hi!

Thanks to the recent memory checking instrastructure (well, it's been
some weeks already...), a memory corruption issue has been uncovered for
configurations where the host double format is not equal to target double
format.  I have seen this for SH, but don't believe it really is specific
to SH, it's just that the very most of all GDB targets have host double
format match the target double format.

As I'm not sufficiently experienced with GDB's expressions and type
system, I'd like some help here.

    $ install/bin/*-gdb -q -ex 'file [...]/gdb.cp/misc' -ex 'show architecture' -ex 'print (bool)17.93'
    Reading symbols from [...]/gdb.cp/misc...done.
    The target architecture is set automatically (currently sh2a-or-sh3e)
    $1 = true
    memory clobbered past end of allocated block
    Aborted

sh2a-or-sh3e configures for a 32-bit double format, as opposed to the
"normal" 64-bit double format (which also is the x86_64 host's double
format).

sh-tdep.c:sh_gdbarch_init:

        case bfd_mach_sh2a_or_sh3e:
          /* doubles on sh2e and sh3e are actually 4 byte.  */
          set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);

First and foremost -- is my understanding correct that given this
configuration the expression »(bool) 17.93« then indeed is to be
evaluated in a 32-bit double format, and not in the host's 64-bit double
format?

Our friend Valgrind is able to confirm this memory corruption issue, and
-- as so often -- gives a clue where to begin looking:

    $ valgrind -v -- install/bin/*-gdb -q -ex 'file [...]/gdb.cp/misc' -ex 'print (bool)17.93'
    [...]
    ==6509== Invalid write of size 1
    ==6509==    at 0x47FB974: memcpy (mc_replace_strmem.c:497)
    ==6509==    by 0x824D487: floatformat_from_doublest (doublest.c:768)
    ==6509==    by 0x824D78D: store_typed_floating (doublest.c:881)
    ==6509==    by 0x81023DA: value_from_double (value.c:3170)
    ==6509==    by 0x810424F: evaluate_subexp_standard (eval.c:846)
    ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
    ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
    ==6509==    by 0x810A157: evaluate_subexp_standard (eval.c:2713)
    ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
    ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
    ==6509==    by 0x8102B02: evaluate_expression (eval.c:148)
    ==6509==    by 0x811EE31: print_command_1 (printcmd.c:966)
    ==6509==  Address 0x8e83bbc is 0 bytes after a block of size 4 alloc'd
    ==6509==    at 0x47F925F: calloc (vg_replace_malloc.c:467)
    ==6509==    by 0x82725B4: xcalloc (common-utils.c:90)
    ==6509==    by 0x82725EA: xzalloc (common-utils.c:100)
    ==6509==    by 0x80FE7AB: allocate_value_contents (value.c:695)
    ==6509==    by 0x80FE7D4: allocate_value (value.c:705)
    ==6509==    by 0x8102383: value_from_double (value.c:3164)
    ==6509==    by 0x810424F: evaluate_subexp_standard (eval.c:846)
    ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
    ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
    ==6509==    by 0x810A157: evaluate_subexp_standard (eval.c:2713)
    ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
    ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
    [...]

Here is what is happening, in value.c:value_from_double:

    struct value *
    value_from_double (struct type *type, DOUBLEST num)
    {
      struct value *val = allocate_value (type);
      struct type *base_type = check_typedef (type);
      enum type_code code = TYPE_CODE (base_type);
    
      if (code == TYPE_CODE_FLT)
        {
          store_typed_floating (value_contents_raw (val), base_type, num);
    [...]

    Breakpoint 1, value_from_double (type=0x852c388, num=17.93) at [...]/gdb/value.c:3164
    3164      struct value *val = allocate_value (type);
    (gdb) print *type
    $2 = {pointer_type = 0x0, reference_type = 0x0, chain = 0x852c388, instance_flags = 0, length = 4, main_type = 0x852c3c0}
    (gdb) print *type->main_type
    $3 = {code = TYPE_CODE_FLT, flag_unsigned = 0, flag_nosign = 0, flag_stub = 0, flag_target_stub = 0, flag_static = 0, flag_prototyped = 0, flag_incomplete = 0, flag_varargs = 0, flag_vector = 0, flag_stub_supported = 0, 
      flag_gnu_ifunc = 0, flag_fixed_instance = 0, flag_objfile_owned = 0, flag_declared_class = 0, flag_flag_enum = 0, type_specific_field = TYPE_SPECIFIC_NONE, nfields = 0, vptr_fieldno = -1, name = 0x852c408 "double", tag_name = 0x0, 
      owner = {objfile = 0x8512f80, gdbarch = 0x8512f80}, target_type = 0x0, flds_bnds = {fields = 0x0, bounds = 0x0}, vptr_basetype = 0x0, type_specific = {cplus_stuff = 0x8476d8c, gnat_stuff = 0x8476d8c, floatformat = 0x8476d8c, 
        func_stuff = 0x8476d8c}}
    (gdb) print type->main_type->type_specific->floatformat[1]
    $36 = (const struct floatformat *) 0x8461f40
    (gdb) print host_float_format 
    $31 = (const struct floatformat *) 0x8461e80
    (gdb) print host_double_format 
    $35 = (const struct floatformat *) 0x8461f40

Here we can already see that, TYPE's floatformat is host_double_format,
which is 64-bit double format.  However, in my understanding, TYPE is
meant to be a 32-bit double type, having a 32-bit double format.

Stepping further, allocate_value, allocate_value_contents:

    allocate_value (type=0x852c388) at [...]/gdb/value.c:705
    705       allocate_value_contents (val);
    (gdb) 
    allocate_value_contents (val=0x8522508) at [...]/gdb/value.c:694
    694       if (!val->contents)
    (gdb) 
    695         val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
    (gdb) 
    xzalloc (size=4) at [...]/gdb/common/common-utils.c:100
    100       return xcalloc (1, size);

So indeed we allocate 32 bits.

Back in value_from_double, on to doublest.c:store_typed_floating:

    void
    store_typed_floating (void *addr, const struct type *type, DOUBLEST val)
    {
      const struct floatformat *fmt = floatformat_from_type (type);
    [...]
      floatformat_from_doublest (fmt, &val, addr);
    }

    store_typed_floating (addr=0x850a570, type=0x852c388, val=17.93) at [...]/gdb/doublest.c:859
    859       const struct floatformat *fmt = floatformat_from_type (type);
    [...]
    881       floatformat_from_doublest (fmt, &val, addr);
    (gdb) s
    floatformat_from_doublest (fmt=0x8461f40, in=0xffffbfd8, out=0x850a570) at [...]/gdb/doublest.c:757

Here we indeed see floatformat fmt 0x8461f40 being passed, which is
host_double_format, the 64-bit double format (see Bearkpoint 1 above).
In the following, things break:

    (gdb) s
    758       if (fmt == host_float_format)
    (gdb) 
    764       else if (fmt == host_double_format)
    (gdb) 
    766           double val = *in;
    (gdb) 
    768           memcpy (out, &val, sizeof (val));

We have host_double_format, »sizeof val« is 64 bits, but we only
allocated 32 bits, thus memcpy corrupts the memory.

Looking at doublest:floatformat_from_type:

    const struct floatformat *
    floatformat_from_type (const struct type *type)
    {
      struct gdbarch *gdbarch = get_type_arch (type);
    
      gdb_assert (TYPE_CODE (type) == TYPE_CODE_FLT);
      if (TYPE_FLOATFORMAT (type) != NULL)
        return TYPE_FLOATFORMAT (type)[gdbarch_byte_order (gdbarch)];
      else
        return floatformat_from_length (gdbarch, TYPE_LENGTH (type));
    }

In our case, »TYPE_FLOATFORMAT (type)[gdbarch_byte_order (gdbarch)]« is
host_double_format (see Breakpoint 1 above).  If I instead make that take
the floatformat_from_length route, it correctly returns the 32-bit
host_float_type, and everything works as expected.

So, my understanding is that »TYPE_FLOATFORMAT (type)« is set
incorrectly -- but where and why is this happening?


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: Memory corruption for host double format different from target double format

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 11156 bytes --]

Hi!

On Thu, 09 Aug 2012 20:18:27 +0200, I wrote:
> Thanks to the recent memory checking instrastructure (well, it's been
> some weeks already...), a memory corruption issue has been uncovered for
> configurations where the host double format is not equal to target double
> format.  I have seen this for SH, but don't believe it really is specific
> to SH, it's just that the very most of all GDB targets have host double
> format match the target double format.
> 
> As I'm not sufficiently experienced with GDB's expressions and type
> system, I'd like some help here.
> 
>     $ install/bin/*-gdb -q -ex 'file [...]/gdb.cp/misc' -ex 'show architecture' -ex 'print (bool)17.93'
>     Reading symbols from [...]/gdb.cp/misc...done.
>     The target architecture is set automatically (currently sh2a-or-sh3e)
>     $1 = true
>     memory clobbered past end of allocated block
>     Aborted
> 
> sh2a-or-sh3e configures for a 32-bit double format, as opposed to the
> "normal" 64-bit double format (which also is the x86_64 host's double
> format).
> 
> sh-tdep.c:sh_gdbarch_init:
> 
>         case bfd_mach_sh2a_or_sh3e:
>           /* doubles on sh2e and sh3e are actually 4 byte.  */
>           set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);

Turns out it -- somewhat -- is an issue in sh-tdep.c.  With the following
patch, everything is fine.

Index: sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.246
diff -u -p -r1.246 sh-tdep.c
--- sh-tdep.c	22 Jul 2012 16:52:41 -0000	1.246
+++ sh-tdep.c	10 Aug 2012 09:15:04 -0000
@@ -2299,6 +2299,7 @@ sh_gdbarch_init (struct gdbarch_info inf
     case bfd_mach_sh2e:
       /* doubles on sh2e and sh3e are actually 4 byte.  */
       set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
+      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
 
       set_gdbarch_register_name (gdbarch, sh_sh2e_register_name);
       set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);
@@ -2344,6 +2345,7 @@ sh_gdbarch_init (struct gdbarch_info inf
     case bfd_mach_sh2a_or_sh3e:
       /* doubles on sh2e and sh3e are actually 4 byte.  */
       set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
+      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
 
       set_gdbarch_register_name (gdbarch, sh_sh3e_register_name);
       set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);

The testresults speak for themselves:

                    === gdb Summary ===
     
    -# of expected passes           14953
    -# of unexpected failures       154
    +# of expected passes           15760
    +# of unexpected failures       100
     # of unexpected successes      2
     # of expected failures         42
     # of known failures            59
    -# of unresolved testcases      797
     # of untested testcases        45
     # of unsupported tests         172

However, it seems there are other targets where a similar patching would
be needed.  And, to begin with, why does GDB allow for letting this
mismatch happen?  Quoting my yesterday's analysis:

> First and foremost -- is my understanding correct that given this
> configuration the expression »(bool) 17.93« then indeed is to be
> evaluated in a 32-bit double format, and not in the host's 64-bit double
> format?
> 
> Our friend Valgrind is able to confirm this memory corruption issue, and
> -- as so often -- gives a clue where to begin looking:
> 
>     $ valgrind -v -- install/bin/*-gdb -q -ex 'file [...]/gdb.cp/misc' -ex 'print (bool)17.93'
>     [...]
>     ==6509== Invalid write of size 1
>     ==6509==    at 0x47FB974: memcpy (mc_replace_strmem.c:497)
>     ==6509==    by 0x824D487: floatformat_from_doublest (doublest.c:768)
>     ==6509==    by 0x824D78D: store_typed_floating (doublest.c:881)
>     ==6509==    by 0x81023DA: value_from_double (value.c:3170)
>     ==6509==    by 0x810424F: evaluate_subexp_standard (eval.c:846)
>     ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
>     ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
>     ==6509==    by 0x810A157: evaluate_subexp_standard (eval.c:2713)
>     ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
>     ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
>     ==6509==    by 0x8102B02: evaluate_expression (eval.c:148)
>     ==6509==    by 0x811EE31: print_command_1 (printcmd.c:966)
>     ==6509==  Address 0x8e83bbc is 0 bytes after a block of size 4 alloc'd
>     ==6509==    at 0x47F925F: calloc (vg_replace_malloc.c:467)
>     ==6509==    by 0x82725B4: xcalloc (common-utils.c:90)
>     ==6509==    by 0x82725EA: xzalloc (common-utils.c:100)
>     ==6509==    by 0x80FE7AB: allocate_value_contents (value.c:695)
>     ==6509==    by 0x80FE7D4: allocate_value (value.c:705)
>     ==6509==    by 0x8102383: value_from_double (value.c:3164)
>     ==6509==    by 0x810424F: evaluate_subexp_standard (eval.c:846)
>     ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
>     ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
>     ==6509==    by 0x810A157: evaluate_subexp_standard (eval.c:2713)
>     ==6509==    by 0x82020E3: evaluate_subexp_c (c-lang.c:719)
>     ==6509==    by 0x8102975: evaluate_subexp (eval.c:73)
>     [...]
> 
> Here is what is happening, in value.c:value_from_double:
> 
>     struct value *
>     value_from_double (struct type *type, DOUBLEST num)
>     {
>       struct value *val = allocate_value (type);
>       struct type *base_type = check_typedef (type);
>       enum type_code code = TYPE_CODE (base_type);
>     
>       if (code == TYPE_CODE_FLT)
>         {
>           store_typed_floating (value_contents_raw (val), base_type, num);
>     [...]
> 
>     Breakpoint 1, value_from_double (type=0x852c388, num=17.93) at [...]/gdb/value.c:3164
>     3164      struct value *val = allocate_value (type);
>     (gdb) print *type
>     $2 = {pointer_type = 0x0, reference_type = 0x0, chain = 0x852c388, instance_flags = 0, length = 4, main_type = 0x852c3c0}
>     (gdb) print *type->main_type
>     $3 = {code = TYPE_CODE_FLT, flag_unsigned = 0, flag_nosign = 0, flag_stub = 0, flag_target_stub = 0, flag_static = 0, flag_prototyped = 0, flag_incomplete = 0, flag_varargs = 0, flag_vector = 0, flag_stub_supported = 0, 
>       flag_gnu_ifunc = 0, flag_fixed_instance = 0, flag_objfile_owned = 0, flag_declared_class = 0, flag_flag_enum = 0, type_specific_field = TYPE_SPECIFIC_NONE, nfields = 0, vptr_fieldno = -1, name = 0x852c408 "double", tag_name = 0x0, 
>       owner = {objfile = 0x8512f80, gdbarch = 0x8512f80}, target_type = 0x0, flds_bnds = {fields = 0x0, bounds = 0x0}, vptr_basetype = 0x0, type_specific = {cplus_stuff = 0x8476d8c, gnat_stuff = 0x8476d8c, floatformat = 0x8476d8c, 
>         func_stuff = 0x8476d8c}}
>     (gdb) print type->main_type->type_specific->floatformat[1]
>     $36 = (const struct floatformat *) 0x8461f40
>     (gdb) print host_float_format 
>     $31 = (const struct floatformat *) 0x8461e80
>     (gdb) print host_double_format 
>     $35 = (const struct floatformat *) 0x8461f40
> 
> Here we can already see that, TYPE's floatformat is host_double_format,
> which is 64-bit double format.  However, in my understanding, TYPE is
> meant to be a 32-bit double type, having a 32-bit double format.
> 
> Stepping further, allocate_value, allocate_value_contents:
> 
>     allocate_value (type=0x852c388) at [...]/gdb/value.c:705
>     705       allocate_value_contents (val);
>     (gdb) 
>     allocate_value_contents (val=0x8522508) at [...]/gdb/value.c:694
>     694       if (!val->contents)
>     (gdb) 
>     695         val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
>     (gdb) 
>     xzalloc (size=4) at [...]/gdb/common/common-utils.c:100
>     100       return xcalloc (1, size);
> 
> So indeed we allocate 32 bits.
> 
> Back in value_from_double, on to doublest.c:store_typed_floating:
> 
>     void
>     store_typed_floating (void *addr, const struct type *type, DOUBLEST val)
>     {
>       const struct floatformat *fmt = floatformat_from_type (type);
>     [...]
>       floatformat_from_doublest (fmt, &val, addr);
>     }
> 
>     store_typed_floating (addr=0x850a570, type=0x852c388, val=17.93) at [...]/gdb/doublest.c:859
>     859       const struct floatformat *fmt = floatformat_from_type (type);
>     [...]
>     881       floatformat_from_doublest (fmt, &val, addr);
>     (gdb) s
>     floatformat_from_doublest (fmt=0x8461f40, in=0xffffbfd8, out=0x850a570) at [...]/gdb/doublest.c:757
> 
> Here we indeed see floatformat fmt 0x8461f40 being passed, which is
> host_double_format, the 64-bit double format (see Bearkpoint 1 above).
> In the following, things break:
> 
>     (gdb) s
>     758       if (fmt == host_float_format)
>     (gdb) 
>     764       else if (fmt == host_double_format)
>     (gdb) 
>     766           double val = *in;
>     (gdb) 
>     768           memcpy (out, &val, sizeof (val));
> 
> We have host_double_format, »sizeof val« is 64 bits, but we only
> allocated 32 bits, thus memcpy corrupts the memory.
> 
> Looking at doublest:floatformat_from_type:
> 
>     const struct floatformat *
>     floatformat_from_type (const struct type *type)
>     {
>       struct gdbarch *gdbarch = get_type_arch (type);
>     
>       gdb_assert (TYPE_CODE (type) == TYPE_CODE_FLT);
>       if (TYPE_FLOATFORMAT (type) != NULL)
>         return TYPE_FLOATFORMAT (type)[gdbarch_byte_order (gdbarch)];
>       else
>         return floatformat_from_length (gdbarch, TYPE_LENGTH (type));
>     }
> 
> In our case, »TYPE_FLOATFORMAT (type)[gdbarch_byte_order (gdbarch)]« is
> host_double_format (see Breakpoint 1 above).  If I instead make that take
> the floatformat_from_length route, it correctly returns the 32-bit
> host_float_type, and everything works as expected.
> 
> So, my understanding is that »TYPE_FLOATFORMAT (type)« is set
> incorrectly -- but where and why is this happening?

It is here:

gdbarch.c:verify_gdbarch:

    [...]
      /* Skip verify of float_bit, invalid_p == 0 */
      if (gdbarch->float_format == 0)
        gdbarch->float_format = floatformats_ieee_single;
      /* Skip verify of double_bit, invalid_p == 0 */
      if (gdbarch->double_format == 0)
        gdbarch->double_format = floatformats_ieee_double;
      /* Skip verify of long_double_bit, invalid_p == 0 */
      if (gdbarch->long_double_format == 0)
        gdbarch->long_double_format = floatformats_ieee_double;
    [...]

That is, if set_gdbarch_double_format has not been called, it will
default to floatformats_ieee_double -- even though set_gdbarch_double_bit
may have been called setting it unequal to the 64-bit double format.
Hmm, and gdbarch.c:verify_gdbarch has the following comment on top of it:
»Ensure that all values in a GDBARCH are reasonable.«  ;-)


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: Memory corruption for host double format different from target double format

[Yao Qi]

On Friday, August 10, 2012 11:32:53 AM Thomas Schwinge wrote:
> gdbarch.c:verify_gdbarch:
> 
>     [...]
>       /* Skip verify of float_bit, invalid_p == 0 */
>       if (gdbarch->float_format == 0)
>         gdbarch->float_format = floatformats_ieee_single;
>       /* Skip verify of double_bit, invalid_p == 0 */
>       if (gdbarch->double_format == 0)
>         gdbarch->double_format = floatformats_ieee_double;
>       /* Skip verify of long_double_bit, invalid_p == 0 */
>       if (gdbarch->long_double_format == 0)
>         gdbarch->long_double_format = floatformats_ieee_double;
>     [...]
> 
> That is, if set_gdbarch_double_format has not been called, it will
> default to floatformats_ieee_double -- even though set_gdbarch_double_bit
> may have been called setting it unequal to the 64-bit double format.
> Hmm, and gdbarch.c:verify_gdbarch has the following comment on top of it:
> »Ensure that all values in a GDBARCH are reasonable.«  ;-)

Looks like some checking like this is missing?

  gdbarch->float_format->totalsize <= gdbarch->float_bit
  gdbarch->double_format->totalsize <= gdbarch->double_bit

-- 
Yao (齐尧)

Re: Memory corruption for host double format different from target double format

[Ulrich Weigand]

Yao Qi wrote:
> On Friday, August 10, 2012 11:32:53 AM Thomas Schwinge wrote:
> > That is, if set_gdbarch_double_format has not been called, it will
> > default to floatformats_ieee_double -- even though set_gdbarch_double_bit
> > may have been called setting it unequal to the 64-bit double format.
> > Hmm, and gdbarch.c:verify_gdbarch has the following comment on top of it:
> > Ensure that all values in a GDBARCH are reasonable.  ;-)
> 
> Looks like some checking like this is missing?
> 
>   gdbarch->float_format->totalsize <= gdbarch->float_bit
>   gdbarch->double_format->totalsize <= gdbarch->double_bit

In fact, I'd prefer to make gdbarch_double_format etc. *mandatory*
and gdbarch_double_bit etc. optional, defaulting to the format size.
(Currently, _bit is mandatory and _format is optional.)

This would mean that nearly all calls to set_gdbarch_double_bit
could go away, with the exception of special cases like "long double"
on i386 ...

[ I guess we could also hunt down and remove the final few places
that still create a TYPE_CODE_FLT with no format set; then the
floatflormat_from_length routine could go away completely. ]

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: Memory corruption for host double format different from target double format

[Mark Kettenis]

> Date: Fri, 10 Aug 2012 14:56:46 +0200 (CEST)
> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> 
> Yao Qi wrote:
> > On Friday, August 10, 2012 11:32:53 AM Thomas Schwinge wrote:
> > > That is, if set_gdbarch_double_format has not been called, it will
> > > default to floatformats_ieee_double -- even though set_gdbarch_double_bit
> > > may have been called setting it unequal to the 64-bit double format.
> > > Hmm, and gdbarch.c:verify_gdbarch has the following comment on top of it:
> > > Ensure that all values in a GDBARCH are reasonable.  ;-)
> > 
> > Looks like some checking like this is missing?
> > 
> >   gdbarch->float_format->totalsize <= gdbarch->float_bit
> >   gdbarch->double_format->totalsize <= gdbarch->double_bit
> 
> In fact, I'd prefer to make gdbarch_double_format etc. *mandatory*
> and gdbarch_double_bit etc. optional, defaulting to the format size.
> (Currently, _bit is mandatory and _format is optional.)
> 
> This would mean that nearly all calls to set_gdbarch_double_bit
> could go away, with the exception of special cases like "long double"
> on i386 ...

Initializing _bit based on _format by default makes sense, but I don't
think this is easy to implement given the way how the gdbarch.c code
is generated.

Making _format mandatory doesn't make sense to me though.  I'd say
that ieee_single and ieee_double are perfectly reasonable defaults for
float_format and double_format.

> [ I guess we could also hunt down and remove the final few places
> that still create a TYPE_CODE_FLT with no format set; then the
> floatflormat_from_length routine could go away completely. ]

I don't think that's possible.  Many (all?) debug formats only
encode the size of floating-point variables.

Re: Memory corruption for host double format different from target double format

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 4687 bytes --]

Hi!

On Fri, 10 Aug 2012 16:31:47 +0200, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > Date: Fri, 10 Aug 2012 14:56:46 +0200 (CEST)
> > From: "Ulrich Weigand" <uweigand@de.ibm.com>
> > 
> > Yao Qi wrote:
> > > On Friday, August 10, 2012 11:32:53 AM Thomas Schwinge wrote:
> > > > That is, if set_gdbarch_double_format has not been called, it will
> > > > default to floatformats_ieee_double -- even though set_gdbarch_double_bit
> > > > may have been called setting it unequal to the 64-bit double format.
> > > > Hmm, and gdbarch.c:verify_gdbarch has the following comment on top of it:
> > > > Ensure that all values in a GDBARCH are reasonable.  ;-)
> > > 
> > > Looks like some checking like this is missing?
> > > 
> > >   gdbarch->float_format->totalsize <= gdbarch->float_bit
> > >   gdbarch->double_format->totalsize <= gdbarch->double_bit
> > 
> > In fact, I'd prefer to make gdbarch_double_format etc. *mandatory*
> > and gdbarch_double_bit etc. optional, defaulting to the format size.
> > (Currently, _bit is mandatory and _format is optional.)
> > 
> > This would mean that nearly all calls to set_gdbarch_double_bit
> > could go away, with the exception of special cases like "long double"
> > on i386 ...
> 
> Initializing _bit based on _format by default makes sense, but I don't
> think this is easy to implement given the way how the gdbarch.c code
> is generated.
> 
> Making _format mandatory doesn't make sense to me though.  I'd say
> that ieee_single and ieee_double are perfectly reasonable defaults for
> float_format and double_format.

Is there a reasonable way for at least detecting the mismatch that I
happened to observe for SH?


Other than that, OK to check in the following?  I have only tested the SH
bits; no maintainer listed for h8300, Stephane CCed for m68hc11.

gdb/
	* h8300-tdep.c (h8300_gdbarch_init): Invoke
	set_gdbarch_double_format and set_gdbarch_long_double_format.
	* m68hc11-tdep.c (m68hc11_gdbarch_init): Invoke
	set_gdbarch_double_format.
	* sh-tdep.c (sh_gdbarch_init): Likewise.

diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 7fc4daa..bcb769e 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -1351,7 +1351,9 @@ h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_long_bit (gdbarch, 4 * TARGET_CHAR_BIT);
   set_gdbarch_long_long_bit (gdbarch, 8 * TARGET_CHAR_BIT);
   set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
+  set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
   set_gdbarch_long_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
+  set_gdbarch_long_double_format (gdbarch, floatformats_ieee_single);
 
   set_gdbarch_believe_pcc_promotion (gdbarch, 1);
 
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index 79629ef..cd32459 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -1498,7 +1498,16 @@ m68hc11_gdbarch_init (struct gdbarch_info info,
   set_gdbarch_short_bit (gdbarch, 16);
   set_gdbarch_int_bit (gdbarch, elf_flags & E_M68HC11_I32 ? 32 : 16);
   set_gdbarch_float_bit (gdbarch, 32);
-  set_gdbarch_double_bit (gdbarch, elf_flags & E_M68HC11_F64 ? 64 : 32);
+  if (elf_flags & E_M68HC11_F64)
+    {
+      set_gdbarch_double_bit (gdbarch, 64);
+      set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
+    }
+  else
+    {
+      set_gdbarch_double_bit (gdbarch, 32);
+      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
+    }
   set_gdbarch_long_double_bit (gdbarch, 64);
   set_gdbarch_long_bit (gdbarch, 32);
   set_gdbarch_ptr_bit (gdbarch, 16);
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index 1ede13a..caf940d 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -2299,6 +2299,7 @@ sh_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     case bfd_mach_sh2e:
       /* doubles on sh2e and sh3e are actually 4 byte.  */
       set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
+      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
 
       set_gdbarch_register_name (gdbarch, sh_sh2e_register_name);
       set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);
@@ -2344,6 +2345,7 @@ sh_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     case bfd_mach_sh2a_or_sh3e:
       /* doubles on sh2e and sh3e are actually 4 byte.  */
       set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
+      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
 
       set_gdbarch_register_name (gdbarch, sh_sh3e_register_name);
       set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: Memory corruption for host double format different from target double format

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 5424 bytes --]

Hi!

On Wed, 29 Aug 2012 17:36:54 +0200, I wrote:
> On Fri, 10 Aug 2012 16:31:47 +0200, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > Date: Fri, 10 Aug 2012 14:56:46 +0200 (CEST)
> > > From: "Ulrich Weigand" <uweigand@de.ibm.com>
> > > 
> > > Yao Qi wrote:
> > > > On Friday, August 10, 2012 11:32:53 AM Thomas Schwinge wrote:
> > > > > That is, if set_gdbarch_double_format has not been called, it will
> > > > > default to floatformats_ieee_double -- even though set_gdbarch_double_bit
> > > > > may have been called setting it unequal to the 64-bit double format.
> > > > > Hmm, and gdbarch.c:verify_gdbarch has the following comment on top of it:
> > > > > Ensure that all values in a GDBARCH are reasonable.  ;-)
> > > > 
> > > > Looks like some checking like this is missing?
> > > > 
> > > >   gdbarch->float_format->totalsize <= gdbarch->float_bit
> > > >   gdbarch->double_format->totalsize <= gdbarch->double_bit
> > > 
> > > In fact, I'd prefer to make gdbarch_double_format etc. *mandatory*
> > > and gdbarch_double_bit etc. optional, defaulting to the format size.
> > > (Currently, _bit is mandatory and _format is optional.)
> > > 
> > > This would mean that nearly all calls to set_gdbarch_double_bit
> > > could go away, with the exception of special cases like "long double"
> > > on i386 ...
> > 
> > Initializing _bit based on _format by default makes sense, but I don't
> > think this is easy to implement given the way how the gdbarch.c code
> > is generated.
> > 
> > Making _format mandatory doesn't make sense to me though.  I'd say
> > that ieee_single and ieee_double are perfectly reasonable defaults for
> > float_format and double_format.
> 
> Is there a reasonable way for at least detecting the mismatch that I
> happened to observe for SH?
> 
> 
> Other than that, OK to check in the following?  I have only tested the SH
> bits; no maintainer listed for h8300, Stephane CCed for m68hc11.

Stephane Carrez' email address <stcarrez@nerim.fr> (as listed in
gdb/MAINTAINERS) bounces saying »unknown user«, but I found another one
in the GCC context -- Stephane, is this you?  If yes, please update the
three occurences of your old email address in gdb/MAINTAINERS (and
possibly other files, too).

Kevin, I'm also adding you to the CC list, as you've been helpful with SH
issues before -- should you be listed as a maintainer for SH?

And what about the h8300 bits?

> gdb/
> 	* h8300-tdep.c (h8300_gdbarch_init): Invoke
> 	set_gdbarch_double_format and set_gdbarch_long_double_format.
> 	* m68hc11-tdep.c (m68hc11_gdbarch_init): Invoke
> 	set_gdbarch_double_format.
> 	* sh-tdep.c (sh_gdbarch_init): Likewise.
> 
> diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
> index 7fc4daa..bcb769e 100644
> --- a/gdb/h8300-tdep.c
> +++ b/gdb/h8300-tdep.c
> @@ -1351,7 +1351,9 @@ h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    set_gdbarch_long_bit (gdbarch, 4 * TARGET_CHAR_BIT);
>    set_gdbarch_long_long_bit (gdbarch, 8 * TARGET_CHAR_BIT);
>    set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> +  set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
>    set_gdbarch_long_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> +  set_gdbarch_long_double_format (gdbarch, floatformats_ieee_single);
>  
>    set_gdbarch_believe_pcc_promotion (gdbarch, 1);
>  
> diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
> index 79629ef..cd32459 100644
> --- a/gdb/m68hc11-tdep.c
> +++ b/gdb/m68hc11-tdep.c
> @@ -1498,7 +1498,16 @@ m68hc11_gdbarch_init (struct gdbarch_info info,
>    set_gdbarch_short_bit (gdbarch, 16);
>    set_gdbarch_int_bit (gdbarch, elf_flags & E_M68HC11_I32 ? 32 : 16);
>    set_gdbarch_float_bit (gdbarch, 32);
> -  set_gdbarch_double_bit (gdbarch, elf_flags & E_M68HC11_F64 ? 64 : 32);
> +  if (elf_flags & E_M68HC11_F64)
> +    {
> +      set_gdbarch_double_bit (gdbarch, 64);
> +      set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
> +    }
> +  else
> +    {
> +      set_gdbarch_double_bit (gdbarch, 32);
> +      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
> +    }
>    set_gdbarch_long_double_bit (gdbarch, 64);
>    set_gdbarch_long_bit (gdbarch, 32);
>    set_gdbarch_ptr_bit (gdbarch, 16);
> diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
> index 1ede13a..caf940d 100644
> --- a/gdb/sh-tdep.c
> +++ b/gdb/sh-tdep.c
> @@ -2299,6 +2299,7 @@ sh_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>      case bfd_mach_sh2e:
>        /* doubles on sh2e and sh3e are actually 4 byte.  */
>        set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> +      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
>  
>        set_gdbarch_register_name (gdbarch, sh_sh2e_register_name);
>        set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);
> @@ -2344,6 +2345,7 @@ sh_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>      case bfd_mach_sh2a_or_sh3e:
>        /* doubles on sh2e and sh3e are actually 4 byte.  */
>        set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> +      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
>  
>        set_gdbarch_register_name (gdbarch, sh_sh3e_register_name);
>        set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: Memory corruption for host double format different from target double format

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 5711 bytes --]

Hi!

Ping.


On Thu, 30 Aug 2012 17:37:45 +0200, I wrote:
> On Wed, 29 Aug 2012 17:36:54 +0200, I wrote:
> > On Fri, 10 Aug 2012 16:31:47 +0200, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > > Date: Fri, 10 Aug 2012 14:56:46 +0200 (CEST)
> > > > From: "Ulrich Weigand" <uweigand@de.ibm.com>
> > > > 
> > > > Yao Qi wrote:
> > > > > On Friday, August 10, 2012 11:32:53 AM Thomas Schwinge wrote:
> > > > > > That is, if set_gdbarch_double_format has not been called, it will
> > > > > > default to floatformats_ieee_double -- even though set_gdbarch_double_bit
> > > > > > may have been called setting it unequal to the 64-bit double format.
> > > > > > Hmm, and gdbarch.c:verify_gdbarch has the following comment on top of it:
> > > > > > Ensure that all values in a GDBARCH are reasonable.  ;-)
> > > > > 
> > > > > Looks like some checking like this is missing?
> > > > > 
> > > > >   gdbarch->float_format->totalsize <= gdbarch->float_bit
> > > > >   gdbarch->double_format->totalsize <= gdbarch->double_bit
> > > > 
> > > > In fact, I'd prefer to make gdbarch_double_format etc. *mandatory*
> > > > and gdbarch_double_bit etc. optional, defaulting to the format size.
> > > > (Currently, _bit is mandatory and _format is optional.)
> > > > 
> > > > This would mean that nearly all calls to set_gdbarch_double_bit
> > > > could go away, with the exception of special cases like "long double"
> > > > on i386 ...
> > > 
> > > Initializing _bit based on _format by default makes sense, but I don't
> > > think this is easy to implement given the way how the gdbarch.c code
> > > is generated.
> > > 
> > > Making _format mandatory doesn't make sense to me though.  I'd say
> > > that ieee_single and ieee_double are perfectly reasonable defaults for
> > > float_format and double_format.
> > 
> > Is there a reasonable way for at least detecting the mismatch that I
> > happened to observe for SH?
> > 
> > 
> > Other than that, OK to check in the following?  I have only tested the SH
> > bits; no maintainer listed for h8300, Stephane CCed for m68hc11.
> 
> Stephane Carrez' email address <stcarrez@nerim.fr> (as listed in
> gdb/MAINTAINERS) bounces saying »unknown user«, but I found another one
> in the GCC context -- Stephane, is this you?  If yes, please update the
> three occurences of your old email address in gdb/MAINTAINERS (and
> possibly other files, too).
> 
> Kevin, I'm also adding you to the CC list, as you've been helpful with SH
> issues before -- should you be listed as a maintainer for SH?
> 
> And what about the h8300 bits?
> 
> > gdb/
> > 	* h8300-tdep.c (h8300_gdbarch_init): Invoke
> > 	set_gdbarch_double_format and set_gdbarch_long_double_format.
> > 	* m68hc11-tdep.c (m68hc11_gdbarch_init): Invoke
> > 	set_gdbarch_double_format.
> > 	* sh-tdep.c (sh_gdbarch_init): Likewise.
> > 
> > diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
> > index 7fc4daa..bcb769e 100644
> > --- a/gdb/h8300-tdep.c
> > +++ b/gdb/h8300-tdep.c
> > @@ -1351,7 +1351,9 @@ h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> >    set_gdbarch_long_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> >    set_gdbarch_long_long_bit (gdbarch, 8 * TARGET_CHAR_BIT);
> >    set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > +  set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
> >    set_gdbarch_long_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > +  set_gdbarch_long_double_format (gdbarch, floatformats_ieee_single);
> >  
> >    set_gdbarch_believe_pcc_promotion (gdbarch, 1);
> >  
> > diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
> > index 79629ef..cd32459 100644
> > --- a/gdb/m68hc11-tdep.c
> > +++ b/gdb/m68hc11-tdep.c
> > @@ -1498,7 +1498,16 @@ m68hc11_gdbarch_init (struct gdbarch_info info,
> >    set_gdbarch_short_bit (gdbarch, 16);
> >    set_gdbarch_int_bit (gdbarch, elf_flags & E_M68HC11_I32 ? 32 : 16);
> >    set_gdbarch_float_bit (gdbarch, 32);
> > -  set_gdbarch_double_bit (gdbarch, elf_flags & E_M68HC11_F64 ? 64 : 32);
> > +  if (elf_flags & E_M68HC11_F64)
> > +    {
> > +      set_gdbarch_double_bit (gdbarch, 64);
> > +      set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
> > +    }
> > +  else
> > +    {
> > +      set_gdbarch_double_bit (gdbarch, 32);
> > +      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
> > +    }
> >    set_gdbarch_long_double_bit (gdbarch, 64);
> >    set_gdbarch_long_bit (gdbarch, 32);
> >    set_gdbarch_ptr_bit (gdbarch, 16);
> > diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
> > index 1ede13a..caf940d 100644
> > --- a/gdb/sh-tdep.c
> > +++ b/gdb/sh-tdep.c
> > @@ -2299,6 +2299,7 @@ sh_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> >      case bfd_mach_sh2e:
> >        /* doubles on sh2e and sh3e are actually 4 byte.  */
> >        set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > +      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
> >  
> >        set_gdbarch_register_name (gdbarch, sh_sh2e_register_name);
> >        set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);
> > @@ -2344,6 +2345,7 @@ sh_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> >      case bfd_mach_sh2a_or_sh3e:
> >        /* doubles on sh2e and sh3e are actually 4 byte.  */
> >        set_gdbarch_double_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > +      set_gdbarch_double_format (gdbarch, floatformats_ieee_single);
> >  
> >        set_gdbarch_register_name (gdbarch, sh_sh3e_register_name);
> >        set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]