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

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 --]

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

[Mark Kettenis]

> From: Thomas Schwinge <thomas@codesourcery.com>
> Date: Fri, 7 Sep 2012 10:20:09 +0200
> 
> Hi!
> 
> Ping.

Looks correct to me.  Guess enough time has passed to give people an
opportunity to object.  So go ahead with this.

> 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>
> > > > >=20
> > > > > 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 w=
> ill
> > > > > > > default to floatformats_ieee_double -- even though set_gdbarch_=
> double_bit
> > > > > > > may have been called setting it unequal to the 64-bit double fo=
> rmat.
> > > > > > > Hmm, and gdbarch.c:verify_gdbarch has the following comment on =
> top of it:
> > > > > > > Ensure that all values in a GDBARCH are reasonable.  ;-)
> > > > > >=20
> > > > > > Looks like some checking like this is missing?
> > > > > >=20
> > > > > >   gdbarch->float_format->totalsize <=3D gdbarch->float_bit
> > > > > >   gdbarch->double_format->totalsize <=3D gdbarch->double_bit
> > > > >=20
> > > > > 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.)
> > > > >=20
> > > > > This would mean that nearly all calls to set_gdbarch_double_bit
> > > > > could go away, with the exception of special cases like "long doubl=
> e"
> > > > > on i386 ...
> > > >=20
> > > > 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.
> > > >=20
> > > > 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.
> > >=20
> > > Is there a reasonable way for at least detecting the mismatch that I
> > > happened to observe for SH?
> > >=20
> > >=20
> > > 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.
> >=20
> > Stephane Carrez' email address <stcarrez@nerim.fr> (as listed in
> > gdb/MAINTAINERS) bounces saying =C2=BBunknown user=C2=AB, but I found ano=
> ther 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).
> >=20
> > 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?
> >=20
> > And what about the h8300 bits?
> >=20
> > > 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.
> > >=20
> > > 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, str=
> uct 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);
> > >=20=20
> > >    set_gdbarch_believe_pcc_promotion (gdbarch, 1);
> > >=20=20
> > > 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);
> > >=20=20
> > >        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);
> > >=20=20
> > >        set_gdbarch_register_name (gdbarch, sh_sh3e_register_name);
> > >        set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);
> 
> 
> Gr=C3=BC=C3=9Fe,
>  Thomas
> 
> --=-=-=
> Content-Type: application/pgp-signature
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> 
> iQEcBAEBAgAGBQJQSa45AAoJENuKOtuXzphJFvkH/jXx0W5f7wdvu5NXfCWxnwTD
> 8BypvAD0GYXry13yQNLWPCvlEvADa2lW3cS1hbTVw30VLZXtonGrQxQMz7J1lRHE
> 4vzamt9Rl5oDKySXDG87LYuXKPyEM07cq6hvRlrS/6Q0LpvT+JjLFnxysYnsHZUv
> guNLfXi6kC5G7hTq9Iklb9zh2wGjA592+UReiDmrWykRgi3ynDxeWk0drw1Drfx3
> ksr0kIk+8IzVAtLruRE+hOi3aMlyWiGoOR/2+MqnBiwketpx+oHpDvRjJnXEpvdt
> 00m/LuPbmahq1BTr3eBopeklT+DrcooKTmOwK/tZnjiaUD/YOqEZS7s04GcHybM=
> =Xqnz
> -----END PGP SIGNATURE-----
> --=-=-=--
>