* [PATCH] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats @ 2019-05-04 12:17 Andrew Burgess 2019-05-16 16:01 ` [PATCHv2] " Andrew Burgess 0 siblings, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2019-05-04 12:17 UTC (permalink / raw) To: gdb-patches; +Cc: Richard Bunt, Sergio Durigan Junior, Andrew Burgess This patch addresses some regressions that Sergio identified in this mail: https://sourceware.org/ml/gdb-patches/2019-05/msg00060.html It turns out these regressions are all related to bug PR gdb/18644 which I've resolved in this patch. I'm posting this as a new thread rather than a reply for more visibility. Thanks, Andrew --- PR gdb/18644 is caused by GDB using the wrong floating point format for gfortran's 16-byte floating point type, including when the 16-byte float is used as the component of a 32-byte complex type. This commit addresses the issue in two places, first in i386-tdep.c there already exists some code to force the use of floatformats_ia64_quad for specific named types. This code is extended to include the type names that gfortran uses for its 16-byte floats. Second, the builtin 16-byte float type (in f-lang.c) is changed to use floatformats_ia64_quad instead of the long double type from gdbarch. On some targets (for example i386) long double is not 16-bytes, and so is inappropriate. This patch was tested on X86-64/GNU-Linux using '--target_board=unix' and '--target_board=unix/-m32', and resolves all of the known failures associated with PR gdb/18644. I've also added the test case from the original bug report. gdb/ChangeLog: PR gdb/18644: * f-lang.c (build_fortran_types): Use floatformats_ia64_quad for 16-byte floats. * i386-tdep.c (i386_floatformat_for_type): Use floatformats_ia64_quad for the 16-byte floating point component within a fortran 32-byte complex number. gdb/testsuite/ChangeLog: PR gdb/18644 * gdb.fortran/complex.exp: Remove setup_kfail calls. * gdb.fortran/printing-types.exp: Add new test. * gdb.fortran/printing-types.f90: Add 16-byte real variable for testing. * gdb.fortran/type-kinds.exp (test_cast_1_to_type_kind): Remove setup_kfail call. --- gdb/ChangeLog | 9 +++++++++ gdb/f-lang.c | 3 +-- gdb/i386-tdep.c | 4 +++- gdb/testsuite/ChangeLog | 10 ++++++++++ gdb/testsuite/gdb.fortran/complex.exp | 2 -- gdb/testsuite/gdb.fortran/printing-types.exp | 1 + gdb/testsuite/gdb.fortran/printing-types.f90 | 2 ++ gdb/testsuite/gdb.fortran/type-kinds.exp | 6 ------ 8 files changed, 26 insertions(+), 11 deletions(-) diff --git a/gdb/f-lang.c b/gdb/f-lang.c index 9da6fdb3e1c..d7b322f759d 100644 --- a/gdb/f-lang.c +++ b/gdb/f-lang.c @@ -728,8 +728,7 @@ build_fortran_types (struct gdbarch *gdbarch) = arch_float_type (gdbarch, gdbarch_double_bit (gdbarch), "real*8", gdbarch_double_format (gdbarch)); builtin_f_type->builtin_real_s16 - = arch_float_type (gdbarch, gdbarch_long_double_bit (gdbarch), - "real*16", gdbarch_long_double_format (gdbarch)); + = arch_float_type (gdbarch, 128, "real*16", floatformats_ia64_quad); builtin_f_type->builtin_complex_s8 = arch_complex_type (gdbarch, "complex*8", diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 54d9dd873b8..66fc6d679e2 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -8158,7 +8158,9 @@ i386_floatformat_for_type (struct gdbarch *gdbarch, if (len == 128 && name) if (strcmp (name, "__float128") == 0 || strcmp (name, "_Float128") == 0 - || strcmp (name, "complex _Float128") == 0) + || strcmp (name, "complex _Float128") == 0 + || strcmp (name, "complex(kind=16)") == 0 + || strcmp (name, "real(kind=16)") == 0) return floatformats_ia64_quad; return default_floatformat_for_type (gdbarch, name, len); diff --git a/gdb/testsuite/gdb.fortran/complex.exp b/gdb/testsuite/gdb.fortran/complex.exp index 136f1c4df79..94ac53afc70 100644 --- a/gdb/testsuite/gdb.fortran/complex.exp +++ b/gdb/testsuite/gdb.fortran/complex.exp @@ -33,7 +33,6 @@ gdb_test "print c4" " = \\(1000,-50\\)" gdb_test "print c8" " = \\(321,-22\\)" gdb_test "print dc" " = \\(321,-22\\)" -setup_kfail gdb/18644 "*-*-*" gdb_test "print c16" " = \\(-874,19\\)" gdb_test "whatis c" "type = complex\\(kind=4\\)" @@ -53,7 +52,6 @@ gdb_test "print \$_creal (dc)" " = 321" gdb_test "whatis \$" " = real\\*8" gdb_test "whatis c16" "type = complex\\(kind=16\\)" -setup_kfail gdb/18644 "*-*-*" gdb_test "print \$_creal (c16)" " = -874" gdb_test "whatis \$" " = real\\*16" diff --git a/gdb/testsuite/gdb.fortran/printing-types.exp b/gdb/testsuite/gdb.fortran/printing-types.exp index 2f6be4ec249..6394e45f4c3 100644 --- a/gdb/testsuite/gdb.fortran/printing-types.exp +++ b/gdb/testsuite/gdb.fortran/printing-types.exp @@ -33,3 +33,4 @@ gdb_test "print oneByte" " = 1" gdb_test "print twobytes" " = 2" gdb_test "print chvalue" " = \'a\'" gdb_test "print logvalue" " = \.TRUE\." +gdb_test "print rVal" " = 2000" diff --git a/gdb/testsuite/gdb.fortran/printing-types.f90 b/gdb/testsuite/gdb.fortran/printing-types.f90 index b4ff928604f..36b63532c8e 100644 --- a/gdb/testsuite/gdb.fortran/printing-types.f90 +++ b/gdb/testsuite/gdb.fortran/printing-types.f90 @@ -18,10 +18,12 @@ program prog integer(2) :: twoBytes character :: chValue logical(1) :: logValue + real(kind=16) :: rVal oneByte = 1 twoBytes = 2 chValue = 'a' logValue = .true. + rVal = 2000 write(*,*) s end diff --git a/gdb/testsuite/gdb.fortran/type-kinds.exp b/gdb/testsuite/gdb.fortran/type-kinds.exp index 1ae15b96f1a..9d19a9ceb39 100644 --- a/gdb/testsuite/gdb.fortran/type-kinds.exp +++ b/gdb/testsuite/gdb.fortran/type-kinds.exp @@ -27,12 +27,6 @@ if { [skip_fortran_tests] } { continue } proc test_cast_1_to_type_kind {base_type type_kind cast_result size_result} { set type_string "$base_type (kind=$type_kind)" gdb_test "p (($type_string) 1)" " = $cast_result" - - if {($base_type == "real" || $base_type == "complex") - && $type_kind == 16} { - setup_kfail gdb/18644 "*-*-*" - } - gdb_test "p sizeof (($type_string) 1)" " = $size_result" } -- 2.14.5 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-04 12:17 [PATCH] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats Andrew Burgess @ 2019-05-16 16:01 ` Andrew Burgess 2019-05-18 8:54 ` Andrew Burgess 0 siblings, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2019-05-16 16:01 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess 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. Thanks, Andrew --- PR gdb/18644 is caused by GDB using the wrong floating point format for gfortran's 16-byte floating point type, including when the 16-byte float is used as the component of a 32-byte complex type. This commit addresses the issue in two places, first in i386-tdep.c, there is already some code to force the use of floatformats_ia64_quad for specific named types, this is extended to include the type names that gfortran uses for its 16-byte floats. Second, the builtin 16-byte float type (in f-lang.c) is changed so it no longer uses gdbarch_long_double_format. On i386 this type is not 16-bytes, but is smaller, this is not what gfortran is expecting. Instead we now use gdbarch_floatformat_for_type and ask for a 16-byte (128 bit) type using the common gfortran type name. This is then spotted in i386-tdep.c (thanks to the first change above) and we again get floatformats_ia64_quad returned. This patch was tested on X86-64/GNU-Linux using '--target_board=unix' and '--target_board=unix/-m32', and resolves all of the known failures associated with PR gdb/18644. I've also added the test case from the original bug report. gdb/ChangeLog: PR gdb/18644: * f-lang.c (build_fortran_types): Use floatformats_ia64_quad for 16-byte floats. * i386-tdep.c (i386_floatformat_for_type): Use floatformats_ia64_quad for the 16-byte floating point component within a fortran 32-byte complex number. gdb/testsuite/ChangeLog: PR gdb/18644 * gdb.fortran/complex.exp: Remove setup_kfail calls. * gdb.fortran/printing-types.exp: Add new test. * gdb.fortran/printing-types.f90: Add 16-byte real variable for testing. * gdb.fortran/type-kinds.exp (test_cast_1_to_type_kind): Remove setup_kfail call. --- gdb/ChangeLog | 9 +++++++++ gdb/f-lang.c | 4 ++-- gdb/i386-tdep.c | 4 +++- gdb/testsuite/ChangeLog | 10 ++++++++++ gdb/testsuite/gdb.fortran/complex.exp | 2 -- gdb/testsuite/gdb.fortran/printing-types.exp | 1 + gdb/testsuite/gdb.fortran/printing-types.f90 | 2 ++ gdb/testsuite/gdb.fortran/type-kinds.exp | 6 ------ 8 files changed, 27 insertions(+), 11 deletions(-) diff --git a/gdb/f-lang.c b/gdb/f-lang.c index 9da6fdb3e1c..5855c68b38c 100644 --- a/gdb/f-lang.c +++ b/gdb/f-lang.c @@ -727,9 +727,9 @@ build_fortran_types (struct gdbarch *gdbarch) builtin_f_type->builtin_real_s8 = 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, gdbarch_long_double_bit (gdbarch), - "real*16", gdbarch_long_double_format (gdbarch)); + = arch_float_type (gdbarch, 128, "real*16", fmt); builtin_f_type->builtin_complex_s8 = arch_complex_type (gdbarch, "complex*8", diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 54d9dd873b8..66fc6d679e2 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -8158,7 +8158,9 @@ i386_floatformat_for_type (struct gdbarch *gdbarch, if (len == 128 && name) if (strcmp (name, "__float128") == 0 || strcmp (name, "_Float128") == 0 - || strcmp (name, "complex _Float128") == 0) + || strcmp (name, "complex _Float128") == 0 + || strcmp (name, "complex(kind=16)") == 0 + || strcmp (name, "real(kind=16)") == 0) return floatformats_ia64_quad; return default_floatformat_for_type (gdbarch, name, len); diff --git a/gdb/testsuite/gdb.fortran/complex.exp b/gdb/testsuite/gdb.fortran/complex.exp index 136f1c4df79..94ac53afc70 100644 --- a/gdb/testsuite/gdb.fortran/complex.exp +++ b/gdb/testsuite/gdb.fortran/complex.exp @@ -33,7 +33,6 @@ gdb_test "print c4" " = \\(1000,-50\\)" gdb_test "print c8" " = \\(321,-22\\)" gdb_test "print dc" " = \\(321,-22\\)" -setup_kfail gdb/18644 "*-*-*" gdb_test "print c16" " = \\(-874,19\\)" gdb_test "whatis c" "type = complex\\(kind=4\\)" @@ -53,7 +52,6 @@ gdb_test "print \$_creal (dc)" " = 321" gdb_test "whatis \$" " = real\\*8" gdb_test "whatis c16" "type = complex\\(kind=16\\)" -setup_kfail gdb/18644 "*-*-*" gdb_test "print \$_creal (c16)" " = -874" gdb_test "whatis \$" " = real\\*16" diff --git a/gdb/testsuite/gdb.fortran/printing-types.exp b/gdb/testsuite/gdb.fortran/printing-types.exp index 2f6be4ec249..6394e45f4c3 100644 --- a/gdb/testsuite/gdb.fortran/printing-types.exp +++ b/gdb/testsuite/gdb.fortran/printing-types.exp @@ -33,3 +33,4 @@ gdb_test "print oneByte" " = 1" gdb_test "print twobytes" " = 2" gdb_test "print chvalue" " = \'a\'" gdb_test "print logvalue" " = \.TRUE\." +gdb_test "print rVal" " = 2000" diff --git a/gdb/testsuite/gdb.fortran/printing-types.f90 b/gdb/testsuite/gdb.fortran/printing-types.f90 index b4ff928604f..36b63532c8e 100644 --- a/gdb/testsuite/gdb.fortran/printing-types.f90 +++ b/gdb/testsuite/gdb.fortran/printing-types.f90 @@ -18,10 +18,12 @@ program prog integer(2) :: twoBytes character :: chValue logical(1) :: logValue + real(kind=16) :: rVal oneByte = 1 twoBytes = 2 chValue = 'a' logValue = .true. + rVal = 2000 write(*,*) s end diff --git a/gdb/testsuite/gdb.fortran/type-kinds.exp b/gdb/testsuite/gdb.fortran/type-kinds.exp index 1ae15b96f1a..9d19a9ceb39 100644 --- a/gdb/testsuite/gdb.fortran/type-kinds.exp +++ b/gdb/testsuite/gdb.fortran/type-kinds.exp @@ -27,12 +27,6 @@ if { [skip_fortran_tests] } { continue } proc test_cast_1_to_type_kind {base_type type_kind cast_result size_result} { set type_string "$base_type (kind=$type_kind)" gdb_test "p (($type_string) 1)" " = $cast_result" - - if {($base_type == "real" || $base_type == "complex") - && $type_kind == 16} { - setup_kfail gdb/18644 "*-*-*" - } - gdb_test "p sizeof (($type_string) 1)" " = $size_result" } -- 2.14.5 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-16 16:01 ` [PATCHv2] " Andrew Burgess @ 2019-05-18 8:54 ` Andrew Burgess 2019-05-21 16:06 ` Tom Tromey 0 siblings, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2019-05-18 8:54 UTC (permalink / raw) To: gdb-patches * Andrew Burgess <andrew.burgess@embecosm.com> [2019-05-16 17:00:58 +0100]: > 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. This has now been pushed. Thanks, Andrew > > Thanks, > Andrew > > > --- > > PR gdb/18644 is caused by GDB using the wrong floating point format > for gfortran's 16-byte floating point type, including when the 16-byte > float is used as the component of a 32-byte complex type. > > This commit addresses the issue in two places, first in i386-tdep.c, > there is already some code to force the use of floatformats_ia64_quad > for specific named types, this is extended to include the type names > that gfortran uses for its 16-byte floats. > > Second, the builtin 16-byte float type (in f-lang.c) is changed so it > no longer uses gdbarch_long_double_format. On i386 this type is not > 16-bytes, but is smaller, this is not what gfortran is expecting. > Instead we now use gdbarch_floatformat_for_type and ask for a > 16-byte (128 bit) type using the common gfortran type name. This is > then spotted in i386-tdep.c (thanks to the first change above) and we > again get floatformats_ia64_quad returned. > > This patch was tested on X86-64/GNU-Linux using '--target_board=unix' > and '--target_board=unix/-m32', and resolves all of the known failures > associated with PR gdb/18644. I've also added the test case from the > original bug report. > > gdb/ChangeLog: > > PR gdb/18644: > * f-lang.c (build_fortran_types): Use floatformats_ia64_quad for > 16-byte floats. > * i386-tdep.c (i386_floatformat_for_type): Use > floatformats_ia64_quad for the 16-byte floating point component > within a fortran 32-byte complex number. > > gdb/testsuite/ChangeLog: > > PR gdb/18644 > * gdb.fortran/complex.exp: Remove setup_kfail calls. > * gdb.fortran/printing-types.exp: Add new test. > * gdb.fortran/printing-types.f90: Add 16-byte real variable for > testing. > * gdb.fortran/type-kinds.exp (test_cast_1_to_type_kind): Remove > setup_kfail call. > --- > gdb/ChangeLog | 9 +++++++++ > gdb/f-lang.c | 4 ++-- > gdb/i386-tdep.c | 4 +++- > gdb/testsuite/ChangeLog | 10 ++++++++++ > gdb/testsuite/gdb.fortran/complex.exp | 2 -- > gdb/testsuite/gdb.fortran/printing-types.exp | 1 + > gdb/testsuite/gdb.fortran/printing-types.f90 | 2 ++ > gdb/testsuite/gdb.fortran/type-kinds.exp | 6 ------ > 8 files changed, 27 insertions(+), 11 deletions(-) > > diff --git a/gdb/f-lang.c b/gdb/f-lang.c > index 9da6fdb3e1c..5855c68b38c 100644 > --- a/gdb/f-lang.c > +++ b/gdb/f-lang.c > @@ -727,9 +727,9 @@ build_fortran_types (struct gdbarch *gdbarch) > builtin_f_type->builtin_real_s8 > = 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, gdbarch_long_double_bit (gdbarch), > - "real*16", gdbarch_long_double_format (gdbarch)); > + = arch_float_type (gdbarch, 128, "real*16", fmt); > > builtin_f_type->builtin_complex_s8 > = arch_complex_type (gdbarch, "complex*8", > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index 54d9dd873b8..66fc6d679e2 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -8158,7 +8158,9 @@ i386_floatformat_for_type (struct gdbarch *gdbarch, > if (len == 128 && name) > if (strcmp (name, "__float128") == 0 > || strcmp (name, "_Float128") == 0 > - || strcmp (name, "complex _Float128") == 0) > + || strcmp (name, "complex _Float128") == 0 > + || strcmp (name, "complex(kind=16)") == 0 > + || strcmp (name, "real(kind=16)") == 0) > return floatformats_ia64_quad; > > return default_floatformat_for_type (gdbarch, name, len); > diff --git a/gdb/testsuite/gdb.fortran/complex.exp b/gdb/testsuite/gdb.fortran/complex.exp > index 136f1c4df79..94ac53afc70 100644 > --- a/gdb/testsuite/gdb.fortran/complex.exp > +++ b/gdb/testsuite/gdb.fortran/complex.exp > @@ -33,7 +33,6 @@ gdb_test "print c4" " = \\(1000,-50\\)" > gdb_test "print c8" " = \\(321,-22\\)" > gdb_test "print dc" " = \\(321,-22\\)" > > -setup_kfail gdb/18644 "*-*-*" > gdb_test "print c16" " = \\(-874,19\\)" > > gdb_test "whatis c" "type = complex\\(kind=4\\)" > @@ -53,7 +52,6 @@ gdb_test "print \$_creal (dc)" " = 321" > gdb_test "whatis \$" " = real\\*8" > > gdb_test "whatis c16" "type = complex\\(kind=16\\)" > -setup_kfail gdb/18644 "*-*-*" > gdb_test "print \$_creal (c16)" " = -874" > gdb_test "whatis \$" " = real\\*16" > > diff --git a/gdb/testsuite/gdb.fortran/printing-types.exp b/gdb/testsuite/gdb.fortran/printing-types.exp > index 2f6be4ec249..6394e45f4c3 100644 > --- a/gdb/testsuite/gdb.fortran/printing-types.exp > +++ b/gdb/testsuite/gdb.fortran/printing-types.exp > @@ -33,3 +33,4 @@ gdb_test "print oneByte" " = 1" > gdb_test "print twobytes" " = 2" > gdb_test "print chvalue" " = \'a\'" > gdb_test "print logvalue" " = \.TRUE\." > +gdb_test "print rVal" " = 2000" > diff --git a/gdb/testsuite/gdb.fortran/printing-types.f90 b/gdb/testsuite/gdb.fortran/printing-types.f90 > index b4ff928604f..36b63532c8e 100644 > --- a/gdb/testsuite/gdb.fortran/printing-types.f90 > +++ b/gdb/testsuite/gdb.fortran/printing-types.f90 > @@ -18,10 +18,12 @@ program prog > integer(2) :: twoBytes > character :: chValue > logical(1) :: logValue > + real(kind=16) :: rVal > > oneByte = 1 > twoBytes = 2 > chValue = 'a' > logValue = .true. > + rVal = 2000 > write(*,*) s > end > diff --git a/gdb/testsuite/gdb.fortran/type-kinds.exp b/gdb/testsuite/gdb.fortran/type-kinds.exp > index 1ae15b96f1a..9d19a9ceb39 100644 > --- a/gdb/testsuite/gdb.fortran/type-kinds.exp > +++ b/gdb/testsuite/gdb.fortran/type-kinds.exp > @@ -27,12 +27,6 @@ if { [skip_fortran_tests] } { continue } > proc test_cast_1_to_type_kind {base_type type_kind cast_result size_result} { > set type_string "$base_type (kind=$type_kind)" > gdb_test "p (($type_string) 1)" " = $cast_result" > - > - if {($base_type == "real" || $base_type == "complex") > - && $type_kind == 16} { > - setup_kfail gdb/18644 "*-*-*" > - } > - > gdb_test "p sizeof (($type_string) 1)" " = $size_result" > } > > -- > 2.14.5 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-18 8:54 ` Andrew Burgess @ 2019-05-21 16:06 ` Tom Tromey 2019-05-21 16:53 ` Andrew Burgess 0 siblings, 1 reply; 10+ messages in thread From: Tom Tromey @ 2019-05-21 16:06 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "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. 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"); builtin_f_type->builtin_complex_s8 = arch_complex_type (gdbarch, "complex*8", ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-21 16:06 ` Tom Tromey @ 2019-05-21 16:53 ` Andrew Burgess 2019-05-21 17:05 ` Tom Tromey 0 siblings, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2019-05-21 16:53 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches * 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: diff --git a/gdb/f-lang.c b/gdb/f-lang.c index 5855c68b38c..b0c57878fec 100644 --- a/gdb/f-lang.c +++ b/gdb/f-lang.c @@ -728,8 +728,13 @@ 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_float_type (gdbarch, gdbarch_long_double_bit (gdbarch), + "real*16", gdbarch_long_double_format (gdbarch)); builtin_f_type->builtin_complex_s8 = arch_complex_type (gdbarch, "complex*8", Or maybe we should be even more restrictive, and only use long double format if its the correct length, like: diff --git a/gdb/f-lang.c b/gdb/f-lang.c index 5855c68b38c..e612eeda7f7 100644 --- a/gdb/f-lang.c +++ b/gdb/f-lang.c @@ -728,8 +728,16 @@ 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 if (gdbarch_long_double_bit (gdbarch) == 128) + builtin_f_type->builtin_real_s16 + = arch_float_type (gdbarch, gdbarch_long_double_bit (gdbarch), + "real*16", gdbarch_long_double_format (gdbarch)); + else + builtin_f_type->builtin_real_s16 + = arch_type (gdbarch, TYPE_CODE_ERROR, 128, "real*16"); builtin_f_type->builtin_complex_s8 = arch_complex_type (gdbarch, "complex*8", What do you think? Again, sorry for the breakage. Thanks, Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-21 16:53 ` Andrew Burgess @ 2019-05-21 17:05 ` Tom Tromey 2019-05-21 20:10 ` Andrew Burgess 0 siblings, 1 reply; 10+ messages in thread From: Tom Tromey @ 2019-05-21 17:05 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-21 17:05 ` Tom Tromey @ 2019-05-21 20:10 ` Andrew Burgess 2019-05-21 20:18 ` Tom Tromey 0 siblings, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2019-05-21 20:10 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches * 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-21 20:10 ` Andrew Burgess @ 2019-05-21 20:18 ` Tom Tromey 2019-05-21 22:56 ` Andrew Burgess 0 siblings, 1 reply; 10+ messages in thread From: Tom Tromey @ 2019-05-21 20:18 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-21 20:18 ` Tom Tromey @ 2019-05-21 22:56 ` Andrew Burgess 2019-05-22 16:56 ` Tom Tromey 0 siblings, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2019-05-21 22:56 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches * Tom Tromey <tromey@adacore.com> [2019-05-21 16:18:18 -0400]: > >>>>> "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. I pushed the below. Thanks, Andrew --- commit dc42e902cc54af2b7e7b54a1171d562f867342d5 Author: Andrew Burgess <andrew.burgess@embecosm.com> Date: Tue May 21 22:14:05 2019 +0100 gdb/fortran: Handle gdbarch_floatformat_for_type returning nullptr In this commit: commit 34d11c682fd96c7dbe3ebd6cd9033e65d51ec7a3 Date: Fri May 3 15:23:55 2019 +0100 gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats GDB was changed such that the Fortran's 16-byte float format was obtained by calling gdbarch_floatformat_for_type instead of just using gdbarch_long_double_format as it was before. The problem with this default_floatformat_for_type can return NULL in some cases, and the code introduced in 34d11c682f didn't consider this. This commit introduces several alternative strategies for finding a suitable 16-byte floating point type. First GDB calls gdbarch_floatformat_for_type (this was what 34d11c682f added), if this returns null GDB will use gdbarch_long_double_format if it is the correct size (this was the format used before 34d11c682f). Finally, if neither of the above provides a suitable type then GDB will create a new dummy type. This final dummy type is unlikely to provide an correct debug experience as far as examining the 16-byte floats, but it should prevent GDB crashing. gdb/ChangeLog: PR gdb/18644: * f-lang.c (build_fortran_types): Handle the case where gdbarch_floatformat_for_type returns a nullptr. diff --git a/gdb/f-lang.c b/gdb/f-lang.c index 5855c68b38c..e612eeda7f7 100644 --- a/gdb/f-lang.c +++ b/gdb/f-lang.c @@ -728,8 +728,16 @@ 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 if (gdbarch_long_double_bit (gdbarch) == 128) + builtin_f_type->builtin_real_s16 + = arch_float_type (gdbarch, gdbarch_long_double_bit (gdbarch), + "real*16", gdbarch_long_double_format (gdbarch)); + else + builtin_f_type->builtin_real_s16 + = arch_type (gdbarch, TYPE_CODE_ERROR, 128, "real*16"); builtin_f_type->builtin_complex_s8 = arch_complex_type (gdbarch, "complex*8", ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats 2019-05-21 22:56 ` Andrew Burgess @ 2019-05-22 16:56 ` Tom Tromey 0 siblings, 0 replies; 10+ messages in thread From: Tom Tromey @ 2019-05-22 16:56 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches >> Go for it. Andrew> I pushed the below. Thank you very much! Tom ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-22 16:56 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-04 12:17 [PATCH] gdb/fortran: Use floatformats_ia64_quad for fortran 16-byte floats Andrew Burgess 2019-05-16 16:01 ` [PATCHv2] " Andrew Burgess 2019-05-18 8:54 ` Andrew Burgess 2019-05-21 16:06 ` Tom Tromey 2019-05-21 16:53 ` Andrew Burgess 2019-05-21 17:05 ` Tom Tromey 2019-05-21 20:10 ` Andrew Burgess 2019-05-21 20:18 ` Tom Tromey 2019-05-21 22:56 ` Andrew Burgess 2019-05-22 16:56 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).