[PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats

  * Tom Tromey <tromey@adacore.com> [2019-05-21 12:06:39 -0400]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> >> This new version moves the use of floatformats_ia64_quad out of
> >> f-lang.c, which I think is an improvement.  With this done I plan to
> >> push this patch shortly unless anyone complains.
> 
> Andrew> This has now been pushed.
> 
> This caused a crash while testing gdb master using the internal AdaCore
> test suite.
> 
> In particular what happened is that the ARM target (in fact many
> targets) uses default_floatformat_for_type, returns NULL.  Then, if
> build_fortran_types is called, it will crash gdb in arch_float_type:
> 
>     (top) bt
>     #0  0x0000000000b4e3b1 in arch_float_type (gdbarch=0x32ef480, bit=128, name=0x1617675 "real*16", floatformats=0x0)
>         at ../../binutils-gdb/gdb/gdbtypes.c:5169
>     #1  0x0000000000b14b11 in build_fortran_types (gdbarch=0x32ef480) at ../../binutils-gdb/gdb/f-lang.c:735
>     #2  0x0000000000b40d64 in gdbarch_data (gdbarch=0x32ef480, data=0x2d84bc0) at ../../binutils-gdb/gdb/gdbarch.c:5269
>     #3  0x0000000000b14bb5 in builtin_f_type (gdbarch=0x32ef480) at ../../binutils-gdb/gdb/f-lang.c:755
>     #4  0x0000000000b1378e in f_language_arch_info (gdbarch=0x32ef480, lai=0x33155e8) at ../../binutils-gdb/gdb/f-lang.c:171
>     #5  0x0000000000be010d in language_gdbarch_post_init (gdbarch=0x32ef480) at ../../binutils-gdb/gdb/language.c:978
>     #6  0x0000000000b40d64 in gdbarch_data (gdbarch=0x32ef480, data=0x31a43d0) at ../../binutils-gdb/gdb/gdbarch.c:5269
>     #7  0x0000000000be0140 in language_string_char_type (la=0x15a20e0 <ada_language_defn>, gdbarch=0x32ef480)
>         at ../../binutils-gdb/gdb/language.c:990
>     #8  0x00000000008ac86c in type_char (par_state=0x7fffffffd400) at ../../binutils-gdb/gdb/ada-exp.y:1453
>     #9  0x00000000008a7d85 in ada_yylex () at ../../binutils-gdb/gdb/ada-lex.l:151
> 
> In this case it happens via the Ada parser, but it can happen in many
> ways, I think.

Sorry for breaking this.

> 
> I'm testing the appended but I'm not really sure this is the best
> approach.
> 
> Tom
> 
> diff --git a/gdb/f-lang.c b/gdb/f-lang.c
> index 097e8906d1f..5c8eeab6f6b 100644
> --- a/gdb/f-lang.c
> +++ b/gdb/f-lang.c
> @@ -731,8 +731,12 @@ build_fortran_types (struct gdbarch *gdbarch)
>      = arch_float_type (gdbarch, gdbarch_double_bit (gdbarch),
>  		       "real*8", gdbarch_double_format (gdbarch));
>    auto fmt = gdbarch_floatformat_for_type (gdbarch, "real(kind=16)", 128);
> -  builtin_f_type->builtin_real_s16
> -    = arch_float_type (gdbarch, 128, "real*16", fmt);
> +  if (fmt != nullptr)
> +    builtin_f_type->builtin_real_s16
> +      = arch_float_type (gdbarch, 128, "real*16", fmt);
> +  else
> +    builtin_f_type->builtin_real_s16
> +      = arch_type (gdbarch, TYPE_CODE_ERROR, 128, "real*16");

Maybe we should switch back to the original code in the else block,
which would just make use of long double format?

Something like:


What do you think?

Again, sorry for the breakage.

Thanks,
Andrew
  

Andrew> Sorry for breaking this.

Thanks!  Don't feel too bad, though, I think the occasional bug like
this is going to creep in.

Andrew> Or maybe we should be even more restrictive, and only use long double
Andrew> format if its the correct length, like:

This seems to make the most sense to me, but I don't actually know
Fortran, so I'm just guessing.

thanks,
Tom
  

* Tom Tromey <tromey@adacore.com> [2019-05-21 13:05:40 -0400]:

> Andrew> Sorry for breaking this.
> 
> Thanks!  Don't feel too bad, though, I think the occasional bug like
> this is going to creep in.
> 
> Andrew> Or maybe we should be even more restrictive, and only use long double
> Andrew> format if its the correct length, like:
> 
> This seems to make the most sense to me, but I don't actually know
> Fortran, so I'm just guessing.

Honestly, I don't know the specifics here either.  I suspect the real
answer is that different targets and maybe even different Fortran
compilers could use different formats for 16-byte floats.

The intention with my change was to allow each target to find
something suitable (clearly that didn't quite work).  I think falling
back to long double means we're no worse off than we were.

If long double turns out to be wrong for other targets they can always
implement gdbarch_floatformat_for_type like i386 does.

Would you like me to push a fix, or do you want to do it?

Thanks,
Andrew
  

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Would you like me to push a fix, or do you want to do it?

Go for it.

Tom