public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
@ 2024-05-27  3:29 Tom de Vries
  2024-05-27  3:29 ` [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail " Tom de Vries
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Tom de Vries @ 2024-05-27  3:29 UTC (permalink / raw)
  To: gdb-patches

When running test-case gdb.base/complex-parts.exp on arm-linux, I get:
...
(gdb) p $_cimag (z3)^M
$6 = 6.5^M
(gdb) PASS: gdb.base/complex-parts.exp: long double imaginary: p $_cimag (z3)
ptype $^M
type = double^M
(gdb) FAIL: gdb.base/complex-parts.exp: long double imaginary: ptype $
...

Given that z3 is a complex long double, the test-case expects the type of the
imaginary part of z3 to be long double, but it's double instead.

This is due to the fact that the dwarf info doesn't specify an explicit target
type:
...
    <5b>   DW_AT_name        : z3
    <60>   DW_AT_type        : <0xa4>
  ...
 <1><a4>: Abbrev Number: 2 (DW_TAG_base_type)
    <a5>   DW_AT_byte_size   : 16
    <a6>   DW_AT_encoding    : 3        (complex float)
    <a7>   DW_AT_name        : complex long double
...
and consequently we're guessing in dwarf2_init_complex_target_type based on
the size:
...
	case 64:
	  tt = builtin_type (gdbarch)->builtin_double;
	  break;
	case 96: /* The x86-32 ABI specifies 96-bit long double.  */
	case 128:
	  tt = builtin_type (gdbarch)->builtin_long_double;
	  break;
...

For arm-linux, complex long double is 16 bytes, so the target type is assumed
to be 8 bytes, which is handled by the "case 64", which gets us double
instead of long double.

Fix this by searching for "long" in the name_hint parameter, and using long
double instead.

Tested on arm-linux.
---
 gdb/dwarf2/read.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4818da58acb..498c0464f97 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -15116,7 +15116,15 @@ dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
 	  tt = builtin_type (gdbarch)->builtin_float;
 	  break;
 	case 64:
-	  tt = builtin_type (gdbarch)->builtin_double;
+	  /* If both "double" and "long double" are 8 bytes, choose "double"
+	     for "complex double" and "long double" for "complex long
+	     double".  */
+	  if (builtin_type (gdbarch)->builtin_long_double->length () == 8
+	      && name_hint != nullptr && *name_hint != '\0'
+	      && strstr (name_hint, "long") != nullptr)
+	    tt = builtin_type (gdbarch)->builtin_long_double;
+	  else
+	    tt = builtin_type (gdbarch)->builtin_double;
 	  break;
 	case 96:	/* The x86-32 ABI specifies 96-bit long double.  */
 	case 128:

base-commit: bdc10cded85aa8995e80394099c9e542b6172979
-- 
2.35.3


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail on arm
  2024-05-27  3:29 [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm Tom de Vries
@ 2024-05-27  3:29 ` Tom de Vries
  2024-06-11  8:51   ` Tom de Vries
  2024-05-27  3:29 ` [PATCH 3/3] [gdb/testsuite] Fix gdb.fortran/array-bounds.exp " Tom de Vries
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2024-05-27  3:29 UTC (permalink / raw)
  To: gdb-patches

When running test-case gdb.fortran/intrinsics.exp on arm-linux, I get:
...
(gdb) p cmplx (4,4,16)^M
/home/linux/gdb/src/gdb/f-lang.c:1002: internal-error: eval_op_f_cmplx: \
  Assertion `kind_arg->code () == TYPE_CODE_COMPLEX' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
----- Backtrace -----^M
FAIL: gdb.fortran/intrinsics.exp: p cmplx (4,4,16) (GDB internal error)
...

The problem is that 16-byte floats are unsupported:
...
$ gfortran test.f90
test.f90:2:17:

    2 |     REAL(kind=16) :: foo = 1
      |                 1
Error: Kind 16 not supported for type REAL at (1)
...
and consequently we end up with a builtin_real_s16 and builtin_complex_s16 with
code TYPE_CODE_ERROR.

Fix this by bailing out asap when encountering such a type.

Without this patch we're able to do the rather useless:
...
(gdb) ptype real*16
type = real*16
(gdb) ptype real_16
type = real*16
...
but with this patch we get:
...
(gdb) ptype real*16
unsupported kind 16 for type real*4
(gdb) ptype real_16
unsupported type real*16
...

Tested on arm-linux.

PR fortran/30537
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30537
---
 gdb/f-exp.y                              | 39 +++++++++++++++++-------
 gdb/testsuite/gdb.fortran/intrinsics.exp |  8 +++--
 gdb/testsuite/gdb.fortran/type-kinds.exp | 22 ++++++++++---
 gdb/testsuite/gdb.fortran/types.exp      | 19 +++++++++++-
 4 files changed, 70 insertions(+), 18 deletions(-)

diff --git a/gdb/f-exp.y b/gdb/f-exp.y
index bdf9c32a81b..259f274d341 100644
--- a/gdb/f-exp.y
+++ b/gdb/f-exp.y
@@ -754,7 +754,11 @@ typebase  /* Implements (approximately): (type-qualifier)* type-specifier */
 	|       REAL_S8_KEYWORD
 			{ $$ = parse_f_type (pstate)->builtin_real_s8; }
 	|	REAL_S16_KEYWORD
-			{ $$ = parse_f_type (pstate)->builtin_real_s16; }
+			{ $$ = parse_f_type (pstate)->builtin_real_s16;
+			  if ($$->code () == TYPE_CODE_ERROR)
+			    error (_("unsupported type %s"),
+				   TYPE_SAFE_NAME ($$));
+			}
 	|	COMPLEX_KEYWORD
 			{ $$ = parse_f_type (pstate)->builtin_complex; }
 	|	COMPLEX_S4_KEYWORD
@@ -762,7 +766,11 @@ typebase  /* Implements (approximately): (type-qualifier)* type-specifier */
 	|	COMPLEX_S8_KEYWORD
 			{ $$ = parse_f_type (pstate)->builtin_complex_s8; }
 	|	COMPLEX_S16_KEYWORD 
-			{ $$ = parse_f_type (pstate)->builtin_complex_s16; }
+			{ $$ = parse_f_type (pstate)->builtin_complex_s16;
+			  if ($$->code () == TYPE_CODE_ERROR)
+			    error (_("unsupported type %s"),
+				   TYPE_SAFE_NAME ($$));
+			}
 	|	SINGLE PRECISION
 			{ $$ = parse_f_type (pstate)->builtin_real;}
 	|	DOUBLE PRECISION
@@ -1156,12 +1164,9 @@ push_kind_type (LONGEST val, struct type *type)
   type_stack->push (tp_kind);
 }
 
-/* Called when a type has a '(kind=N)' modifier after it, for example
-   'character(kind=1)'.  The BASETYPE is the type described by 'character'
-   in our example, and KIND is the integer '1'.  This function returns a
-   new type that represents the basetype of a specific kind.  */
+/* Helper function for convert_to_kind_type.  */
 static struct type *
-convert_to_kind_type (struct type *basetype, int kind)
+convert_to_kind_type_1 (struct type *basetype, int kind)
 {
   if (basetype == parse_f_type (pstate)->builtin_character)
     {
@@ -1211,13 +1216,25 @@ convert_to_kind_type (struct type *basetype, int kind)
 	return parse_f_type (pstate)->builtin_integer_s8;
     }
 
-  error (_("unsupported kind %d for type %s"),
-	 kind, TYPE_SAFE_NAME (basetype));
-
-  /* Should never get here.  */
   return nullptr;
 }
 
+/* Called when a type has a '(kind=N)' modifier after it, for example
+   'character(kind=1)'.  The BASETYPE is the type described by 'character'
+   in our example, and KIND is the integer '1'.  This function returns a
+   new type that represents the basetype of a specific kind.  */
+static struct type *
+convert_to_kind_type (struct type *basetype, int kind)
+{
+  struct type *res = convert_to_kind_type_1 (basetype, kind);
+
+  if (res == nullptr || res->code () == TYPE_CODE_ERROR)
+    error (_("unsupported kind %d for type %s"),
+	   kind, TYPE_SAFE_NAME (basetype));
+
+  return res;
+}
+
 struct f_token
 {
   /* The string to match against.  */
diff --git a/gdb/testsuite/gdb.fortran/intrinsics.exp b/gdb/testsuite/gdb.fortran/intrinsics.exp
index 60c79f956dc..060bb53db97 100644
--- a/gdb/testsuite/gdb.fortran/intrinsics.exp
+++ b/gdb/testsuite/gdb.fortran/intrinsics.exp
@@ -112,10 +112,14 @@ gdb_test "ptype cmplx (4,4)" "= complex\\*4"
 gdb_test "p cmplx (-14,-4)" "= \\(-14,-4\\)"
 gdb_test "p cmplx (4,4,4)" "\\(4,4\\)"
 gdb_test "p cmplx (4,4,8)" "\\(4,4\\)"
-gdb_test "p cmplx (4,4,16)" "\\(4,4\\)"
+set re_unsupported_kind_16 \
+    [string_to_regexp "unsupported kind 16 for type complex*4"]
+gdb_test "p cmplx (4,4,16)" \
+    ([string_to_regexp " = (4,4)"]|$re_unsupported_kind_16)
 gdb_test "ptype cmplx (4,4,4)" "= complex\\*4"
 gdb_test "ptype cmplx (4,4,8)" "= complex\\*8"
-gdb_test "ptype cmplx (4,4,16)" "= complex\\*16"
+gdb_test "ptype cmplx (4,4,16)" \
+    ([string_to_regexp " = complex*16"]|$re_unsupported_kind_16)
 
 gdb_test "p cmplx (4,4,1)" "unsupported kind 1 for type complex\\*4"
 gdb_test "p cmplx (4,4,-1)" "unsupported kind -1 for type complex\\*4"
diff --git a/gdb/testsuite/gdb.fortran/type-kinds.exp b/gdb/testsuite/gdb.fortran/type-kinds.exp
index ab5f19f97a4..a6f2aa4e870 100644
--- a/gdb/testsuite/gdb.fortran/type-kinds.exp
+++ b/gdb/testsuite/gdb.fortran/type-kinds.exp
@@ -43,12 +43,20 @@ proc test_basic_parsing_of_type_kinds {} {
     test_cast_1_to_type_kind "complex" "" "\\(1,0\\)" "8"
     test_cast_1_to_type_kind "complex" "4" "\\(1,0\\)" "8"
     test_cast_1_to_type_kind "complex" "8" "\\(1,0\\)" "16"
-    test_cast_1_to_type_kind "complex" "16" "\\(1,0\\)" "32"
+    set re_unsupported_kind \
+	[string_to_regexp "unsupported kind 16 for type complex*4"]
+    test_cast_1_to_type_kind "complex" "16" \
+	[string_to_regexp (1,0)]|$re_unsupported_kind \
+	32|$re_unsupported_kind
 
     test_cast_1_to_type_kind "real" "" "1" "4"
     test_cast_1_to_type_kind "real" "4" "1" "4"
     test_cast_1_to_type_kind "real" "8" "1" "8"
-    test_cast_1_to_type_kind "real" "16" "1" "16"
+    set re_unsupported_kind \
+	[string_to_regexp "unsupported kind 16 for type real*4"]
+    test_cast_1_to_type_kind "real" "16" \
+	1|$re_unsupported_kind \
+	16|$re_unsupported_kind
 
     test_cast_1_to_type_kind "logical" "" "\\.TRUE\\." "4"
     test_cast_1_to_type_kind "logical" "1" "\\.TRUE\\." "1"
@@ -83,11 +91,17 @@ proc test_old_star_type_sizes {} {
 
     gdb_test "p ((complex*4) 1)" " = \\(1,0\\)"
     gdb_test "p ((complex*8) 1)" " = \\(1,0\\)"
-    gdb_test "p ((complex*16) 1)" " = \\(1,0\\)"
+    set re_unsupported_kind \
+	[string_to_regexp "unsupported kind 16 for type complex*4"]
+    gdb_test "p ((complex*16) 1)" \
+	[string_to_regexp " = (1,0)"]|$re_unsupported_kind
 
     gdb_test "p ((real*4) 1)" " = 1"
     gdb_test "p ((real*8) 1)" " = 1"
-    gdb_test "p ((real*16) 1)" " = 1"
+    set re_unsupported_kind \
+	[string_to_regexp "unsupported kind 16 for type real*4"]
+    gdb_test "p ((real*16) 1)" \
+	"( = 1|$re_unsupported_kind)"
 
     gdb_test "p ((logical*1) 1)" " = \\.TRUE\\."
     gdb_test "p ((logical*4) 1)" " = \\.TRUE\\."
diff --git a/gdb/testsuite/gdb.fortran/types.exp b/gdb/testsuite/gdb.fortran/types.exp
index 83b109869e6..edbf5abee97 100644
--- a/gdb/testsuite/gdb.fortran/types.exp
+++ b/gdb/testsuite/gdb.fortran/types.exp
@@ -94,7 +94,24 @@ proc test_primitive_types_known {} {
 	# While TYPE_KIND is allowed as input, GDB will always return the
 	# Fortran notation TYPE*KIND
 	regsub -all "_" $type "\*" type_res
-	gdb_test "ptype $type" [string_to_regexp "type = $type_res"]
+	set re [string_to_regexp "type = $type_res"]
+	switch $type {
+	    real*16 - complex*16 {
+		regexp {^[^*_]*} $type base
+		set re_unsupported \
+		    [string_to_regexp \
+			 "unsupported kind 16 for type $base*4"]
+		set re ($re|$re_unsupported)
+	    }
+	    real_16 - complex_16 {
+		set re_unsupported \
+		    [string_to_regexp \
+			 "unsupported type $type_res"]
+		set re ($re|$re_unsupported)
+	    }
+	}
+
+	gdb_test "ptype $type" $re
     }
 }
 
-- 
2.35.3


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] [gdb/testsuite] Fix gdb.fortran/array-bounds.exp on arm
  2024-05-27  3:29 [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm Tom de Vries
  2024-05-27  3:29 ` [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail " Tom de Vries
@ 2024-05-27  3:29 ` Tom de Vries
  2024-06-07  6:13   ` Tom de Vries
  2024-05-28 15:11 ` [PATCH 1/3] [gdb/exp] Fix target type of complex long double " Tom Tromey
  2024-06-03 14:52 ` Luis Machado
  3 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2024-05-27  3:29 UTC (permalink / raw)
  To: gdb-patches

When running test-case gdb.fortran/array-bounds.exp on arm-linux, we run into:
...
(gdb) print &foo^M
$1 = (PTR TO -> ( real(kind=4) (0:1) )) 0xfffef008^M
(gdb) FAIL: gdb.fortran/array-bounds.exp: print &foo
print &bar^M
$2 = (PTR TO -> ( real(kind=4) (-1:0) )) 0xfffef010^M
(gdb) FAIL: gdb.fortran/array-bounds.exp: print &bar
...

This is due to gcc PR debug/54934.

The test-case contains a kfail for this, which is only activated for
x86_64/i386.

Fix this by enabling the kfail for all ilp32 targets.

Also:
- change the kfail into an xfail, because gdb is not at fault here, and
- limit the xfail to the gfortran compiler.

Tested on arm-linux.
---
 gdb/testsuite/gdb.fortran/array-bounds.exp | 45 +++++++++++++++-------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/gdb/testsuite/gdb.fortran/array-bounds.exp b/gdb/testsuite/gdb.fortran/array-bounds.exp
index f09eda9d646..46ee614a37f 100644
--- a/gdb/testsuite/gdb.fortran/array-bounds.exp
+++ b/gdb/testsuite/gdb.fortran/array-bounds.exp
@@ -31,21 +31,38 @@ if {![fortran_runto_main]} {
     return
 }
 
-# Convenience proc to setup for KFAIL
-proc kfail_if {exp bugid triplet} {
-    if {$exp} {
-	setup_kfail $bugid $triplet
+# GCC outputs incorrect range debug info for -m32, gcc PR debug/54934.
+set expect_xfail \
+    [expr \
+	 [test_compiler_info {gfortran-*} f90] \
+	 && [is_ilp32_target]]
+
+set re_ok [string_to_regexp (4294967296:4294967297)]
+set re_xfail [string_to_regexp (0:1)]
+gdb_test_multiple "print &foo" "" {
+    -re -wrap $re_ok.* {
+	pass $gdb_test_name
+    }
+    -re -wrap $re_xfail.* {
+	if { $expect_xfail } {
+	    xfail $gdb_test_name
+	} else {
+	    fail $gdb_test_name
+	}
     }
 }
 
-# GCC outputs incorrect range debug info for -m32.
-set expect_fail false
-if {[is_ilp32_target] && ([istarget "i\[34567\]86-*-linux*"]
-			  || [istarget "x86_64-*-linux*"])} {
-    set expect_fail true
+set re_ok [string_to_regexp (-4294967297:-4294967296)]
+set re_xfail [string_to_regexp (-1:0)]
+gdb_test_multiple "print &bar" "" {
+    -re -wrap $re_ok.* {
+	pass $gdb_test_name
+    }
+    -re -wrap $re_xfail.* {
+	if { $expect_xfail } {
+	    xfail $gdb_test_name
+	} else {
+	    fail $gdb_test_name
+	}
+    }
 }
-
-kfail_if $expect_fail "gcc/54934" "*-*-*"
-gdb_test "print &foo" {.*\(4294967296:4294967297\).*}
-kfail_if $expect_fail "gcc/54934" "*-*-*"
-gdb_test "print &bar" {.*\(-4294967297:-4294967296\).*}
-- 
2.35.3


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
  2024-05-27  3:29 [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm Tom de Vries
  2024-05-27  3:29 ` [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail " Tom de Vries
  2024-05-27  3:29 ` [PATCH 3/3] [gdb/testsuite] Fix gdb.fortran/array-bounds.exp " Tom de Vries
@ 2024-05-28 15:11 ` Tom Tromey
  2024-05-29 11:44   ` Tom de Vries
  2024-06-03 14:52 ` Luis Machado
  3 siblings, 1 reply; 13+ messages in thread
From: Tom Tromey @ 2024-05-28 15:11 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> For arm-linux, complex long double is 16 bytes, so the target type is assumed
Tom> to be 8 bytes, which is handled by the "case 64", which gets us double
Tom> instead of long double.

Tom> Fix this by searching for "long" in the name_hint parameter, and using long
Tom> double instead.

Can you file a compiler bug report about this and then put the link in a
comment?

Does this same problem occur for scalars of type double -vs- long double?
If so then maybe gdb should just accept that this target is kind of
broken in this way.

Tom> +	  /* If both "double" and "long double" are 8 bytes, choose "double"
Tom> +	     for "complex double" and "long double" for "complex long
Tom> +	     double".  */
Tom> +	  if (builtin_type (gdbarch)->builtin_long_double->length () == 8
Tom> +	      && name_hint != nullptr && *name_hint != '\0'

I don't think the \0 part of this check is needed.

Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
  2024-05-28 15:11 ` [PATCH 1/3] [gdb/exp] Fix target type of complex long double " Tom Tromey
@ 2024-05-29 11:44   ` Tom de Vries
  2024-06-03 14:37     ` Tom Tromey
                       ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Tom de Vries @ 2024-05-29 11:44 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 5/28/24 17:11, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> For arm-linux, complex long double is 16 bytes, so the target type is assumed
> Tom> to be 8 bytes, which is handled by the "case 64", which gets us double
> Tom> instead of long double.
> 
> Tom> Fix this by searching for "long" in the name_hint parameter, and using long
> Tom> double instead.
> 
> Can you file a compiler bug report about this and then put the link in a
> comment?
> 

Done ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115272 ).

> Does this same problem occur for scalars of type double -vs- long double?

For a complex DW_TAG_base_type, there's a need to figure out the subtype.

For scalars there's not, so I'm not sure how to interpret "same problem" 
here.

> If so then maybe gdb should just accept that this target is kind of
> broken in this way.
> 
> Tom> +	  /* If both "double" and "long double" are 8 bytes, choose "double"
> Tom> +	     for "complex double" and "long double" for "complex long
> Tom> +	     double".  */
> Tom> +	  if (builtin_type (gdbarch)->builtin_long_double->length () == 8
> Tom> +	      && name_hint != nullptr && *name_hint != '\0'
> 
> I don't think the \0 part of this check is needed.

Ah, I see, it's the needle is empty string case that is special, and 
name_hint is the haystack argument.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
  2024-05-29 11:44   ` Tom de Vries
@ 2024-06-03 14:37     ` Tom Tromey
  2024-06-07 13:36     ` Tom de Vries
  2024-06-19 15:42     ` Tom de Vries
  2 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2024-06-03 14:37 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Tom Tromey, gdb-patches

>> Does this same problem occur for scalars of type double -vs- long double?

Tom> For a complex DW_TAG_base_type, there's a need to figure out the subtype.

Tom> For scalars there's not, so I'm not sure how to interpret "same
Tom> problem" here.

Yeah, never mind, I forgot that the complex type doesn't mention the
base type.

Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
  2024-05-27  3:29 [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm Tom de Vries
                   ` (2 preceding siblings ...)
  2024-05-28 15:11 ` [PATCH 1/3] [gdb/exp] Fix target type of complex long double " Tom Tromey
@ 2024-06-03 14:52 ` Luis Machado
  2024-06-03 15:04   ` Tom de Vries
  3 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2024-06-03 14:52 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 5/27/24 04:29, Tom de Vries wrote:
> When running test-case gdb.base/complex-parts.exp on arm-linux, I get:
> ...
> (gdb) p $_cimag (z3)^M
> $6 = 6.5^M
> (gdb) PASS: gdb.base/complex-parts.exp: long double imaginary: p $_cimag (z3)
> ptype $^M
> type = double^M
> (gdb) FAIL: gdb.base/complex-parts.exp: long double imaginary: ptype $
> ...
> 
> Given that z3 is a complex long double, the test-case expects the type of the
> imaginary part of z3 to be long double, but it's double instead.
> 
> This is due to the fact that the dwarf info doesn't specify an explicit target
> type:
> ...
>     <5b>   DW_AT_name        : z3
>     <60>   DW_AT_type        : <0xa4>
>   ...
>  <1><a4>: Abbrev Number: 2 (DW_TAG_base_type)
>     <a5>   DW_AT_byte_size   : 16
>     <a6>   DW_AT_encoding    : 3        (complex float)
>     <a7>   DW_AT_name        : complex long double
> ...
> and consequently we're guessing in dwarf2_init_complex_target_type based on
> the size:
> ...
> 	case 64:
> 	  tt = builtin_type (gdbarch)->builtin_double;
> 	  break;
> 	case 96: /* The x86-32 ABI specifies 96-bit long double.  */
> 	case 128:
> 	  tt = builtin_type (gdbarch)->builtin_long_double;
> 	  break;
> ...
> 
> For arm-linux, complex long double is 16 bytes, so the target type is assumed
> to be 8 bytes, which is handled by the "case 64", which gets us double
> instead of long double.
> 
> Fix this by searching for "long" in the name_hint parameter, and using long
> double instead.
> 
> Tested on arm-linux.
> ---
>  gdb/dwarf2/read.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 4818da58acb..498c0464f97 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -15116,7 +15116,15 @@ dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
>  	  tt = builtin_type (gdbarch)->builtin_float;
>  	  break;
>  	case 64:
> -	  tt = builtin_type (gdbarch)->builtin_double;
> +	  /* If both "double" and "long double" are 8 bytes, choose "double"
> +	     for "complex double" and "long double" for "complex long
> +	     double".  */
> +	  if (builtin_type (gdbarch)->builtin_long_double->length () == 8
> +	      && name_hint != nullptr && *name_hint != '\0'
> +	      && strstr (name_hint, "long") != nullptr)
> +	    tt = builtin_type (gdbarch)->builtin_long_double;
> +	  else
> +	    tt = builtin_type (gdbarch)->builtin_double;
>  	  break;
>  	case 96:	/* The x86-32 ABI specifies 96-bit long double.  */
>  	case 128:
> 
> base-commit: bdc10cded85aa8995e80394099c9e542b6172979

I suppose once the compiler potentially fixes this, we'd still like to have a workaround
for the broken DWARF info for broken compilers?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
  2024-06-03 14:52 ` Luis Machado
@ 2024-06-03 15:04   ` Tom de Vries
  0 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2024-06-03 15:04 UTC (permalink / raw)
  To: Luis Machado, gdb-patches

On 6/3/24 16:52, Luis Machado wrote:
> On 5/27/24 04:29, Tom de Vries wrote:
>> When running test-case gdb.base/complex-parts.exp on arm-linux, I get:
>> ...
>> (gdb) p $_cimag (z3)^M
>> $6 = 6.5^M
>> (gdb) PASS: gdb.base/complex-parts.exp: long double imaginary: p $_cimag (z3)
>> ptype $^M
>> type = double^M
>> (gdb) FAIL: gdb.base/complex-parts.exp: long double imaginary: ptype $
>> ...
>>
>> Given that z3 is a complex long double, the test-case expects the type of the
>> imaginary part of z3 to be long double, but it's double instead.
>>
>> This is due to the fact that the dwarf info doesn't specify an explicit target
>> type:
>> ...
>>      <5b>   DW_AT_name        : z3
>>      <60>   DW_AT_type        : <0xa4>
>>    ...
>>   <1><a4>: Abbrev Number: 2 (DW_TAG_base_type)
>>      <a5>   DW_AT_byte_size   : 16
>>      <a6>   DW_AT_encoding    : 3        (complex float)
>>      <a7>   DW_AT_name        : complex long double
>> ...
>> and consequently we're guessing in dwarf2_init_complex_target_type based on
>> the size:
>> ...
>> 	case 64:
>> 	  tt = builtin_type (gdbarch)->builtin_double;
>> 	  break;
>> 	case 96: /* The x86-32 ABI specifies 96-bit long double.  */
>> 	case 128:
>> 	  tt = builtin_type (gdbarch)->builtin_long_double;
>> 	  break;
>> ...
>>
>> For arm-linux, complex long double is 16 bytes, so the target type is assumed
>> to be 8 bytes, which is handled by the "case 64", which gets us double
>> instead of long double.
>>
>> Fix this by searching for "long" in the name_hint parameter, and using long
>> double instead.
>>
>> Tested on arm-linux.
>> ---
>>   gdb/dwarf2/read.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 4818da58acb..498c0464f97 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -15116,7 +15116,15 @@ dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
>>   	  tt = builtin_type (gdbarch)->builtin_float;
>>   	  break;
>>   	case 64:
>> -	  tt = builtin_type (gdbarch)->builtin_double;
>> +	  /* If both "double" and "long double" are 8 bytes, choose "double"
>> +	     for "complex double" and "long double" for "complex long
>> +	     double".  */
>> +	  if (builtin_type (gdbarch)->builtin_long_double->length () == 8
>> +	      && name_hint != nullptr && *name_hint != '\0'
>> +	      && strstr (name_hint, "long") != nullptr)
>> +	    tt = builtin_type (gdbarch)->builtin_long_double;
>> +	  else
>> +	    tt = builtin_type (gdbarch)->builtin_double;
>>   	  break;
>>   	case 96:	/* The x86-32 ABI specifies 96-bit long double.  */
>>   	case 128:
>>
>> base-commit: bdc10cded85aa8995e80394099c9e542b6172979
> 
> I suppose once the compiler potentially fixes this, we'd still like to have a workaround
> for the broken DWARF info for broken compilers?

I'd say yes.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] [gdb/testsuite] Fix gdb.fortran/array-bounds.exp on arm
  2024-05-27  3:29 ` [PATCH 3/3] [gdb/testsuite] Fix gdb.fortran/array-bounds.exp " Tom de Vries
@ 2024-06-07  6:13   ` Tom de Vries
  0 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2024-06-07  6:13 UTC (permalink / raw)
  To: gdb-patches

On 5/27/24 05:29, Tom de Vries wrote:
> When running test-case gdb.fortran/array-bounds.exp on arm-linux, we run into:
> ...
> (gdb) print &foo^M
> $1 = (PTR TO -> ( real(kind=4) (0:1) )) 0xfffef008^M
> (gdb) FAIL: gdb.fortran/array-bounds.exp: print &foo
> print &bar^M
> $2 = (PTR TO -> ( real(kind=4) (-1:0) )) 0xfffef010^M
> (gdb) FAIL: gdb.fortran/array-bounds.exp: print &bar
> ...
> 
> This is due to gcc PR debug/54934.
> 
> The test-case contains a kfail for this, which is only activated for
> x86_64/i386.
> 
> Fix this by enabling the kfail for all ilp32 targets.
> 
> Also:
> - change the kfail into an xfail, because gdb is not at fault here, and
> - limit the xfail to the gfortran compiler.
> 
> Tested on arm-linux.


Pushed.

Thanks,
- Tom

> ---
>   gdb/testsuite/gdb.fortran/array-bounds.exp | 45 +++++++++++++++-------
>   1 file changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.fortran/array-bounds.exp b/gdb/testsuite/gdb.fortran/array-bounds.exp
> index f09eda9d646..46ee614a37f 100644
> --- a/gdb/testsuite/gdb.fortran/array-bounds.exp
> +++ b/gdb/testsuite/gdb.fortran/array-bounds.exp
> @@ -31,21 +31,38 @@ if {![fortran_runto_main]} {
>       return
>   }
>   
> -# Convenience proc to setup for KFAIL
> -proc kfail_if {exp bugid triplet} {
> -    if {$exp} {
> -	setup_kfail $bugid $triplet
> +# GCC outputs incorrect range debug info for -m32, gcc PR debug/54934.
> +set expect_xfail \
> +    [expr \
> +	 [test_compiler_info {gfortran-*} f90] \
> +	 && [is_ilp32_target]]
> +
> +set re_ok [string_to_regexp (4294967296:4294967297)]
> +set re_xfail [string_to_regexp (0:1)]
> +gdb_test_multiple "print &foo" "" {
> +    -re -wrap $re_ok.* {
> +	pass $gdb_test_name
> +    }
> +    -re -wrap $re_xfail.* {
> +	if { $expect_xfail } {
> +	    xfail $gdb_test_name
> +	} else {
> +	    fail $gdb_test_name
> +	}
>       }
>   }
>   
> -# GCC outputs incorrect range debug info for -m32.
> -set expect_fail false
> -if {[is_ilp32_target] && ([istarget "i\[34567\]86-*-linux*"]
> -			  || [istarget "x86_64-*-linux*"])} {
> -    set expect_fail true
> +set re_ok [string_to_regexp (-4294967297:-4294967296)]
> +set re_xfail [string_to_regexp (-1:0)]
> +gdb_test_multiple "print &bar" "" {
> +    -re -wrap $re_ok.* {
> +	pass $gdb_test_name
> +    }
> +    -re -wrap $re_xfail.* {
> +	if { $expect_xfail } {
> +	    xfail $gdb_test_name
> +	} else {
> +	    fail $gdb_test_name
> +	}
> +    }
>   }
> -
> -kfail_if $expect_fail "gcc/54934" "*-*-*"
> -gdb_test "print &foo" {.*\(4294967296:4294967297\).*}
> -kfail_if $expect_fail "gcc/54934" "*-*-*"
> -gdb_test "print &bar" {.*\(-4294967297:-4294967296\).*}


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
  2024-05-29 11:44   ` Tom de Vries
  2024-06-03 14:37     ` Tom Tromey
@ 2024-06-07 13:36     ` Tom de Vries
  2024-06-19 15:42     ` Tom de Vries
  2 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2024-06-07 13:36 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 5/29/24 13:44, Tom de Vries wrote:
> On 5/28/24 17:11, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> For arm-linux, complex long double is 16 bytes, so the target 
>> type is assumed
>> Tom> to be 8 bytes, which is handled by the "case 64", which gets us 
>> double
>> Tom> instead of long double.
>>
>> Tom> Fix this by searching for "long" in the name_hint parameter, and 
>> using long
>> Tom> double instead.
>>
>> Can you file a compiler bug report about this and then put the link in a
>> comment?
>>
> 
> Done ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115272 ).
> 

>> Tom> +      /* If both "double" and "long double" are 8 bytes, choose 
>> "double"
>> Tom> +         for "complex double" and "long double" for "complex long
>> Tom> +         double".  */
>> Tom> +      if (builtin_type (gdbarch)->builtin_long_double->length () 
>> == 8
>> Tom> +          && name_hint != nullptr && *name_hint != '\0'
>>
>> I don't think the \0 part of this check is needed.
> 
> Ah, I see, it's the needle is empty string case that is special, and 
> name_hint is the haystack argument.

I've fixed this in a v2 ( 
https://sourceware.org/pipermail/gdb-patches/2024-June/209753.html ), 
and made the comment a bit shorter.

I've also added a dwarf assembly test-case.

Furthermore, I've updated the commit message to make it clear that the 
missing explicit target type is standard-conformant (I'm not sure 
whether I realized that already when I sent v1).

Thanks,
- Tom



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail on arm
  2024-05-27  3:29 ` [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail " Tom de Vries
@ 2024-06-11  8:51   ` Tom de Vries
  2024-06-13  9:14     ` Tom de Vries
  0 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2024-06-11  8:51 UTC (permalink / raw)
  To: gdb-patches

On 5/27/24 05:29, Tom de Vries wrote:
> When running test-case gdb.fortran/intrinsics.exp on arm-linux, I get:
> ...
> (gdb) p cmplx (4,4,16)^M
> /home/linux/gdb/src/gdb/f-lang.c:1002: internal-error: eval_op_f_cmplx: \
>    Assertion `kind_arg->code () == TYPE_CODE_COMPLEX' failed.^M
> A problem internal to GDB has been detected,^M
> further debugging may prove unreliable.^M
> ----- Backtrace -----^M
> FAIL: gdb.fortran/intrinsics.exp: p cmplx (4,4,16) (GDB internal error)
> ...
> 
> The problem is that 16-byte floats are unsupported:
> ...
> $ gfortran test.f90
> test.f90:2:17:
> 
>      2 |     REAL(kind=16) :: foo = 1
>        |                 1
> Error: Kind 16 not supported for type REAL at (1)
> ...
> and consequently we end up with a builtin_real_s16 and builtin_complex_s16 with
> code TYPE_CODE_ERROR.
> 
> Fix this by bailing out asap when encountering such a type.
> 
> Without this patch we're able to do the rather useless:
> ...
> (gdb) ptype real*16
> type = real*16
> (gdb) ptype real_16
> type = real*16
> ...
> but with this patch we get:
> ...
> (gdb) ptype real*16
> unsupported kind 16 for type real*4
> (gdb) ptype real_16
> unsupported type real*16
> ...
> 

FWIW, a minimal approach would fix things like this:
...
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 58f35bf0f3f..236d092e142 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -998,6 +998,8 @@ value *
  eval_op_f_cmplx (type *expect_type, expression *exp, noside noside,
  		 exp_opcode opcode, value *arg1, value *arg2, type *kind_arg)
  {
+  if (kind_arg->code () == TYPE_CODE_ERROR)
+    error (_("unsupported kind"));
    gdb_assert (kind_arg->code () == TYPE_CODE_COMPLEX);
    if (arg1->type ()->code () == TYPE_CODE_COMPLEX
        || arg2->type ()->code () == TYPE_CODE_COMPLEX)
...
but my fear is that this kind of fix is required in many other places, 
and we're just not hitting those in the test suite.

The approach I've chose is more intrusive, but does try to catch as many 
cases as possible by bailing out asap.

Thanks,
- Tom

> Tested on arm-linux.
> 
> PR fortran/30537
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30537
> ---
>   gdb/f-exp.y                              | 39 +++++++++++++++++-------
>   gdb/testsuite/gdb.fortran/intrinsics.exp |  8 +++--
>   gdb/testsuite/gdb.fortran/type-kinds.exp | 22 ++++++++++---
>   gdb/testsuite/gdb.fortran/types.exp      | 19 +++++++++++-
>   4 files changed, 70 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/f-exp.y b/gdb/f-exp.y
> index bdf9c32a81b..259f274d341 100644
> --- a/gdb/f-exp.y
> +++ b/gdb/f-exp.y
> @@ -754,7 +754,11 @@ typebase  /* Implements (approximately): (type-qualifier)* type-specifier */
>   	|       REAL_S8_KEYWORD
>   			{ $$ = parse_f_type (pstate)->builtin_real_s8; }
>   	|	REAL_S16_KEYWORD
> -			{ $$ = parse_f_type (pstate)->builtin_real_s16; }
> +			{ $$ = parse_f_type (pstate)->builtin_real_s16;
> +			  if ($$->code () == TYPE_CODE_ERROR)
> +			    error (_("unsupported type %s"),
> +				   TYPE_SAFE_NAME ($$));
> +			}
>   	|	COMPLEX_KEYWORD
>   			{ $$ = parse_f_type (pstate)->builtin_complex; }
>   	|	COMPLEX_S4_KEYWORD
> @@ -762,7 +766,11 @@ typebase  /* Implements (approximately): (type-qualifier)* type-specifier */
>   	|	COMPLEX_S8_KEYWORD
>   			{ $$ = parse_f_type (pstate)->builtin_complex_s8; }
>   	|	COMPLEX_S16_KEYWORD
> -			{ $$ = parse_f_type (pstate)->builtin_complex_s16; }
> +			{ $$ = parse_f_type (pstate)->builtin_complex_s16;
> +			  if ($$->code () == TYPE_CODE_ERROR)
> +			    error (_("unsupported type %s"),
> +				   TYPE_SAFE_NAME ($$));
> +			}
>   	|	SINGLE PRECISION
>   			{ $$ = parse_f_type (pstate)->builtin_real;}
>   	|	DOUBLE PRECISION
> @@ -1156,12 +1164,9 @@ push_kind_type (LONGEST val, struct type *type)
>     type_stack->push (tp_kind);
>   }
>   
> -/* Called when a type has a '(kind=N)' modifier after it, for example
> -   'character(kind=1)'.  The BASETYPE is the type described by 'character'
> -   in our example, and KIND is the integer '1'.  This function returns a
> -   new type that represents the basetype of a specific kind.  */
> +/* Helper function for convert_to_kind_type.  */
>   static struct type *
> -convert_to_kind_type (struct type *basetype, int kind)
> +convert_to_kind_type_1 (struct type *basetype, int kind)
>   {
>     if (basetype == parse_f_type (pstate)->builtin_character)
>       {
> @@ -1211,13 +1216,25 @@ convert_to_kind_type (struct type *basetype, int kind)
>   	return parse_f_type (pstate)->builtin_integer_s8;
>       }
>   
> -  error (_("unsupported kind %d for type %s"),
> -	 kind, TYPE_SAFE_NAME (basetype));
> -
> -  /* Should never get here.  */
>     return nullptr;
>   }
>   
> +/* Called when a type has a '(kind=N)' modifier after it, for example
> +   'character(kind=1)'.  The BASETYPE is the type described by 'character'
> +   in our example, and KIND is the integer '1'.  This function returns a
> +   new type that represents the basetype of a specific kind.  */
> +static struct type *
> +convert_to_kind_type (struct type *basetype, int kind)
> +{
> +  struct type *res = convert_to_kind_type_1 (basetype, kind);
> +
> +  if (res == nullptr || res->code () == TYPE_CODE_ERROR)
> +    error (_("unsupported kind %d for type %s"),
> +	   kind, TYPE_SAFE_NAME (basetype));
> +
> +  return res;
> +}
> +
>   struct f_token
>   {
>     /* The string to match against.  */
> diff --git a/gdb/testsuite/gdb.fortran/intrinsics.exp b/gdb/testsuite/gdb.fortran/intrinsics.exp
> index 60c79f956dc..060bb53db97 100644
> --- a/gdb/testsuite/gdb.fortran/intrinsics.exp
> +++ b/gdb/testsuite/gdb.fortran/intrinsics.exp
> @@ -112,10 +112,14 @@ gdb_test "ptype cmplx (4,4)" "= complex\\*4"
>   gdb_test "p cmplx (-14,-4)" "= \\(-14,-4\\)"
>   gdb_test "p cmplx (4,4,4)" "\\(4,4\\)"
>   gdb_test "p cmplx (4,4,8)" "\\(4,4\\)"
> -gdb_test "p cmplx (4,4,16)" "\\(4,4\\)"
> +set re_unsupported_kind_16 \
> +    [string_to_regexp "unsupported kind 16 for type complex*4"]
> +gdb_test "p cmplx (4,4,16)" \
> +    ([string_to_regexp " = (4,4)"]|$re_unsupported_kind_16)
>   gdb_test "ptype cmplx (4,4,4)" "= complex\\*4"
>   gdb_test "ptype cmplx (4,4,8)" "= complex\\*8"
> -gdb_test "ptype cmplx (4,4,16)" "= complex\\*16"
> +gdb_test "ptype cmplx (4,4,16)" \
> +    ([string_to_regexp " = complex*16"]|$re_unsupported_kind_16)
>   
>   gdb_test "p cmplx (4,4,1)" "unsupported kind 1 for type complex\\*4"
>   gdb_test "p cmplx (4,4,-1)" "unsupported kind -1 for type complex\\*4"
> diff --git a/gdb/testsuite/gdb.fortran/type-kinds.exp b/gdb/testsuite/gdb.fortran/type-kinds.exp
> index ab5f19f97a4..a6f2aa4e870 100644
> --- a/gdb/testsuite/gdb.fortran/type-kinds.exp
> +++ b/gdb/testsuite/gdb.fortran/type-kinds.exp
> @@ -43,12 +43,20 @@ proc test_basic_parsing_of_type_kinds {} {
>       test_cast_1_to_type_kind "complex" "" "\\(1,0\\)" "8"
>       test_cast_1_to_type_kind "complex" "4" "\\(1,0\\)" "8"
>       test_cast_1_to_type_kind "complex" "8" "\\(1,0\\)" "16"
> -    test_cast_1_to_type_kind "complex" "16" "\\(1,0\\)" "32"
> +    set re_unsupported_kind \
> +	[string_to_regexp "unsupported kind 16 for type complex*4"]
> +    test_cast_1_to_type_kind "complex" "16" \
> +	[string_to_regexp (1,0)]|$re_unsupported_kind \
> +	32|$re_unsupported_kind
>   
>       test_cast_1_to_type_kind "real" "" "1" "4"
>       test_cast_1_to_type_kind "real" "4" "1" "4"
>       test_cast_1_to_type_kind "real" "8" "1" "8"
> -    test_cast_1_to_type_kind "real" "16" "1" "16"
> +    set re_unsupported_kind \
> +	[string_to_regexp "unsupported kind 16 for type real*4"]
> +    test_cast_1_to_type_kind "real" "16" \
> +	1|$re_unsupported_kind \
> +	16|$re_unsupported_kind
>   
>       test_cast_1_to_type_kind "logical" "" "\\.TRUE\\." "4"
>       test_cast_1_to_type_kind "logical" "1" "\\.TRUE\\." "1"
> @@ -83,11 +91,17 @@ proc test_old_star_type_sizes {} {
>   
>       gdb_test "p ((complex*4) 1)" " = \\(1,0\\)"
>       gdb_test "p ((complex*8) 1)" " = \\(1,0\\)"
> -    gdb_test "p ((complex*16) 1)" " = \\(1,0\\)"
> +    set re_unsupported_kind \
> +	[string_to_regexp "unsupported kind 16 for type complex*4"]
> +    gdb_test "p ((complex*16) 1)" \
> +	[string_to_regexp " = (1,0)"]|$re_unsupported_kind
>   
>       gdb_test "p ((real*4) 1)" " = 1"
>       gdb_test "p ((real*8) 1)" " = 1"
> -    gdb_test "p ((real*16) 1)" " = 1"
> +    set re_unsupported_kind \
> +	[string_to_regexp "unsupported kind 16 for type real*4"]
> +    gdb_test "p ((real*16) 1)" \
> +	"( = 1|$re_unsupported_kind)"
>   
>       gdb_test "p ((logical*1) 1)" " = \\.TRUE\\."
>       gdb_test "p ((logical*4) 1)" " = \\.TRUE\\."
> diff --git a/gdb/testsuite/gdb.fortran/types.exp b/gdb/testsuite/gdb.fortran/types.exp
> index 83b109869e6..edbf5abee97 100644
> --- a/gdb/testsuite/gdb.fortran/types.exp
> +++ b/gdb/testsuite/gdb.fortran/types.exp
> @@ -94,7 +94,24 @@ proc test_primitive_types_known {} {
>   	# While TYPE_KIND is allowed as input, GDB will always return the
>   	# Fortran notation TYPE*KIND
>   	regsub -all "_" $type "\*" type_res
> -	gdb_test "ptype $type" [string_to_regexp "type = $type_res"]
> +	set re [string_to_regexp "type = $type_res"]
> +	switch $type {
> +	    real*16 - complex*16 {
> +		regexp {^[^*_]*} $type base
> +		set re_unsupported \
> +		    [string_to_regexp \
> +			 "unsupported kind 16 for type $base*4"]
> +		set re ($re|$re_unsupported)
> +	    }
> +	    real_16 - complex_16 {
> +		set re_unsupported \
> +		    [string_to_regexp \
> +			 "unsupported type $type_res"]
> +		set re ($re|$re_unsupported)
> +	    }
> +	}
> +
> +	gdb_test "ptype $type" $re
>       }
>   }
>   


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail on arm
  2024-06-11  8:51   ` Tom de Vries
@ 2024-06-13  9:14     ` Tom de Vries
  0 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2024-06-13  9:14 UTC (permalink / raw)
  To: gdb-patches, Andrew Burgess

On 6/11/24 10:51, Tom de Vries wrote:
> On 5/27/24 05:29, Tom de Vries wrote:
>> When running test-case gdb.fortran/intrinsics.exp on arm-linux, I get:
>> ...
>> (gdb) p cmplx (4,4,16)^M
>> /home/linux/gdb/src/gdb/f-lang.c:1002: internal-error: eval_op_f_cmplx: \
>>    Assertion `kind_arg->code () == TYPE_CODE_COMPLEX' failed.^M
>> A problem internal to GDB has been detected,^M
>> further debugging may prove unreliable.^M
>> ----- Backtrace -----^M
>> FAIL: gdb.fortran/intrinsics.exp: p cmplx (4,4,16) (GDB internal error)
>> ...
>>
>> The problem is that 16-byte floats are unsupported:
>> ...
>> $ gfortran test.f90
>> test.f90:2:17:
>>
>>      2 |     REAL(kind=16) :: foo = 1
>>        |                 1
>> Error: Kind 16 not supported for type REAL at (1)
>> ...
>> and consequently we end up with a builtin_real_s16 and 
>> builtin_complex_s16 with
>> code TYPE_CODE_ERROR.
>>
>> Fix this by bailing out asap when encountering such a type.
>>
>> Without this patch we're able to do the rather useless:
>> ...
>> (gdb) ptype real*16
>> type = real*16
>> (gdb) ptype real_16
>> type = real*16
>> ...
>> but with this patch we get:
>> ...
>> (gdb) ptype real*16
>> unsupported kind 16 for type real*4
>> (gdb) ptype real_16
>> unsupported type real*16
>> ...
>>
> 
> FWIW, a minimal approach would fix things like this:
> ...
> diff --git a/gdb/f-lang.c b/gdb/f-lang.c
> index 58f35bf0f3f..236d092e142 100644
> --- a/gdb/f-lang.c
> +++ b/gdb/f-lang.c
> @@ -998,6 +998,8 @@ value *
>   eval_op_f_cmplx (type *expect_type, expression *exp, noside noside,
>            exp_opcode opcode, value *arg1, value *arg2, type *kind_arg)
>   {
> +  if (kind_arg->code () == TYPE_CODE_ERROR)
> +    error (_("unsupported kind"));
>     gdb_assert (kind_arg->code () == TYPE_CODE_COMPLEX);
>     if (arg1->type ()->code () == TYPE_CODE_COMPLEX
>         || arg2->type ()->code () == TYPE_CODE_COMPLEX)
> ...
> but my fear is that this kind of fix is required in many other places, 
> and we're just not hitting those in the test suite.
> 
> The approach I've chose is more intrusive, but does try to catch as many 
> cases as possible by bailing out asap.
> 

Andrew,

I saw that you introduced the dummy type with TYPE_CODE_ERROR in commit 
dc42e902cc5 ("gdb/fortran: Handle gdbarch_floatformat_for_type returning 
nullptr"), so I was hoping that you would have an opinion on which 
approach to take.

Thanks,
- Tom

>> Tested on arm-linux.
>>
>> PR fortran/30537
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30537
>> ---
>>   gdb/f-exp.y                              | 39 +++++++++++++++++-------
>>   gdb/testsuite/gdb.fortran/intrinsics.exp |  8 +++--
>>   gdb/testsuite/gdb.fortran/type-kinds.exp | 22 ++++++++++---
>>   gdb/testsuite/gdb.fortran/types.exp      | 19 +++++++++++-
>>   4 files changed, 70 insertions(+), 18 deletions(-)
>>
>> diff --git a/gdb/f-exp.y b/gdb/f-exp.y
>> index bdf9c32a81b..259f274d341 100644
>> --- a/gdb/f-exp.y
>> +++ b/gdb/f-exp.y
>> @@ -754,7 +754,11 @@ typebase  /* Implements (approximately): 
>> (type-qualifier)* type-specifier */
>>       |       REAL_S8_KEYWORD
>>               { $$ = parse_f_type (pstate)->builtin_real_s8; }
>>       |    REAL_S16_KEYWORD
>> -            { $$ = parse_f_type (pstate)->builtin_real_s16; }
>> +            { $$ = parse_f_type (pstate)->builtin_real_s16;
>> +              if ($$->code () == TYPE_CODE_ERROR)
>> +                error (_("unsupported type %s"),
>> +                   TYPE_SAFE_NAME ($$));
>> +            }
>>       |    COMPLEX_KEYWORD
>>               { $$ = parse_f_type (pstate)->builtin_complex; }
>>       |    COMPLEX_S4_KEYWORD
>> @@ -762,7 +766,11 @@ typebase  /* Implements (approximately): 
>> (type-qualifier)* type-specifier */
>>       |    COMPLEX_S8_KEYWORD
>>               { $$ = parse_f_type (pstate)->builtin_complex_s8; }
>>       |    COMPLEX_S16_KEYWORD
>> -            { $$ = parse_f_type (pstate)->builtin_complex_s16; }
>> +            { $$ = parse_f_type (pstate)->builtin_complex_s16;
>> +              if ($$->code () == TYPE_CODE_ERROR)
>> +                error (_("unsupported type %s"),
>> +                   TYPE_SAFE_NAME ($$));
>> +            }
>>       |    SINGLE PRECISION
>>               { $$ = parse_f_type (pstate)->builtin_real;}
>>       |    DOUBLE PRECISION
>> @@ -1156,12 +1164,9 @@ push_kind_type (LONGEST val, struct type *type)
>>     type_stack->push (tp_kind);
>>   }
>> -/* Called when a type has a '(kind=N)' modifier after it, for example
>> -   'character(kind=1)'.  The BASETYPE is the type described by 
>> 'character'
>> -   in our example, and KIND is the integer '1'.  This function returns a
>> -   new type that represents the basetype of a specific kind.  */
>> +/* Helper function for convert_to_kind_type.  */
>>   static struct type *
>> -convert_to_kind_type (struct type *basetype, int kind)
>> +convert_to_kind_type_1 (struct type *basetype, int kind)
>>   {
>>     if (basetype == parse_f_type (pstate)->builtin_character)
>>       {
>> @@ -1211,13 +1216,25 @@ convert_to_kind_type (struct type *basetype, 
>> int kind)
>>       return parse_f_type (pstate)->builtin_integer_s8;
>>       }
>> -  error (_("unsupported kind %d for type %s"),
>> -     kind, TYPE_SAFE_NAME (basetype));
>> -
>> -  /* Should never get here.  */
>>     return nullptr;
>>   }
>> +/* Called when a type has a '(kind=N)' modifier after it, for example
>> +   'character(kind=1)'.  The BASETYPE is the type described by 
>> 'character'
>> +   in our example, and KIND is the integer '1'.  This function returns a
>> +   new type that represents the basetype of a specific kind.  */
>> +static struct type *
>> +convert_to_kind_type (struct type *basetype, int kind)
>> +{
>> +  struct type *res = convert_to_kind_type_1 (basetype, kind);
>> +
>> +  if (res == nullptr || res->code () == TYPE_CODE_ERROR)
>> +    error (_("unsupported kind %d for type %s"),
>> +       kind, TYPE_SAFE_NAME (basetype));
>> +
>> +  return res;
>> +}
>> +
>>   struct f_token
>>   {
>>     /* The string to match against.  */
>> diff --git a/gdb/testsuite/gdb.fortran/intrinsics.exp 
>> b/gdb/testsuite/gdb.fortran/intrinsics.exp
>> index 60c79f956dc..060bb53db97 100644
>> --- a/gdb/testsuite/gdb.fortran/intrinsics.exp
>> +++ b/gdb/testsuite/gdb.fortran/intrinsics.exp
>> @@ -112,10 +112,14 @@ gdb_test "ptype cmplx (4,4)" "= complex\\*4"
>>   gdb_test "p cmplx (-14,-4)" "= \\(-14,-4\\)"
>>   gdb_test "p cmplx (4,4,4)" "\\(4,4\\)"
>>   gdb_test "p cmplx (4,4,8)" "\\(4,4\\)"
>> -gdb_test "p cmplx (4,4,16)" "\\(4,4\\)"
>> +set re_unsupported_kind_16 \
>> +    [string_to_regexp "unsupported kind 16 for type complex*4"]
>> +gdb_test "p cmplx (4,4,16)" \
>> +    ([string_to_regexp " = (4,4)"]|$re_unsupported_kind_16)
>>   gdb_test "ptype cmplx (4,4,4)" "= complex\\*4"
>>   gdb_test "ptype cmplx (4,4,8)" "= complex\\*8"
>> -gdb_test "ptype cmplx (4,4,16)" "= complex\\*16"
>> +gdb_test "ptype cmplx (4,4,16)" \
>> +    ([string_to_regexp " = complex*16"]|$re_unsupported_kind_16)
>>   gdb_test "p cmplx (4,4,1)" "unsupported kind 1 for type complex\\*4"
>>   gdb_test "p cmplx (4,4,-1)" "unsupported kind -1 for type complex\\*4"
>> diff --git a/gdb/testsuite/gdb.fortran/type-kinds.exp 
>> b/gdb/testsuite/gdb.fortran/type-kinds.exp
>> index ab5f19f97a4..a6f2aa4e870 100644
>> --- a/gdb/testsuite/gdb.fortran/type-kinds.exp
>> +++ b/gdb/testsuite/gdb.fortran/type-kinds.exp
>> @@ -43,12 +43,20 @@ proc test_basic_parsing_of_type_kinds {} {
>>       test_cast_1_to_type_kind "complex" "" "\\(1,0\\)" "8"
>>       test_cast_1_to_type_kind "complex" "4" "\\(1,0\\)" "8"
>>       test_cast_1_to_type_kind "complex" "8" "\\(1,0\\)" "16"
>> -    test_cast_1_to_type_kind "complex" "16" "\\(1,0\\)" "32"
>> +    set re_unsupported_kind \
>> +    [string_to_regexp "unsupported kind 16 for type complex*4"]
>> +    test_cast_1_to_type_kind "complex" "16" \
>> +    [string_to_regexp (1,0)]|$re_unsupported_kind \
>> +    32|$re_unsupported_kind
>>       test_cast_1_to_type_kind "real" "" "1" "4"
>>       test_cast_1_to_type_kind "real" "4" "1" "4"
>>       test_cast_1_to_type_kind "real" "8" "1" "8"
>> -    test_cast_1_to_type_kind "real" "16" "1" "16"
>> +    set re_unsupported_kind \
>> +    [string_to_regexp "unsupported kind 16 for type real*4"]
>> +    test_cast_1_to_type_kind "real" "16" \
>> +    1|$re_unsupported_kind \
>> +    16|$re_unsupported_kind
>>       test_cast_1_to_type_kind "logical" "" "\\.TRUE\\." "4"
>>       test_cast_1_to_type_kind "logical" "1" "\\.TRUE\\." "1"
>> @@ -83,11 +91,17 @@ proc test_old_star_type_sizes {} {
>>       gdb_test "p ((complex*4) 1)" " = \\(1,0\\)"
>>       gdb_test "p ((complex*8) 1)" " = \\(1,0\\)"
>> -    gdb_test "p ((complex*16) 1)" " = \\(1,0\\)"
>> +    set re_unsupported_kind \
>> +    [string_to_regexp "unsupported kind 16 for type complex*4"]
>> +    gdb_test "p ((complex*16) 1)" \
>> +    [string_to_regexp " = (1,0)"]|$re_unsupported_kind
>>       gdb_test "p ((real*4) 1)" " = 1"
>>       gdb_test "p ((real*8) 1)" " = 1"
>> -    gdb_test "p ((real*16) 1)" " = 1"
>> +    set re_unsupported_kind \
>> +    [string_to_regexp "unsupported kind 16 for type real*4"]
>> +    gdb_test "p ((real*16) 1)" \
>> +    "( = 1|$re_unsupported_kind)"
>>       gdb_test "p ((logical*1) 1)" " = \\.TRUE\\."
>>       gdb_test "p ((logical*4) 1)" " = \\.TRUE\\."
>> diff --git a/gdb/testsuite/gdb.fortran/types.exp 
>> b/gdb/testsuite/gdb.fortran/types.exp
>> index 83b109869e6..edbf5abee97 100644
>> --- a/gdb/testsuite/gdb.fortran/types.exp
>> +++ b/gdb/testsuite/gdb.fortran/types.exp
>> @@ -94,7 +94,24 @@ proc test_primitive_types_known {} {
>>       # While TYPE_KIND is allowed as input, GDB will always return the
>>       # Fortran notation TYPE*KIND
>>       regsub -all "_" $type "\*" type_res
>> -    gdb_test "ptype $type" [string_to_regexp "type = $type_res"]
>> +    set re [string_to_regexp "type = $type_res"]
>> +    switch $type {
>> +        real*16 - complex*16 {
>> +        regexp {^[^*_]*} $type base
>> +        set re_unsupported \
>> +            [string_to_regexp \
>> +             "unsupported kind 16 for type $base*4"]
>> +        set re ($re|$re_unsupported)
>> +        }
>> +        real_16 - complex_16 {
>> +        set re_unsupported \
>> +            [string_to_regexp \
>> +             "unsupported type $type_res"]
>> +        set re ($re|$re_unsupported)
>> +        }
>> +    }
>> +
>> +    gdb_test "ptype $type" $re
>>       }
>>   }
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm
  2024-05-29 11:44   ` Tom de Vries
  2024-06-03 14:37     ` Tom Tromey
  2024-06-07 13:36     ` Tom de Vries
@ 2024-06-19 15:42     ` Tom de Vries
  2 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2024-06-19 15:42 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 5/29/24 13:44, Tom de Vries wrote:
> On 5/28/24 17:11, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> For arm-linux, complex long double is 16 bytes, so the target 
>> type is assumed
>> Tom> to be 8 bytes, which is handled by the "case 64", which gets us 
>> double
>> Tom> instead of long double.
>>
>> Tom> Fix this by searching for "long" in the name_hint parameter, and 
>> using long
>> Tom> double instead.
>>
>> Can you file a compiler bug report about this and then put the link in a
>> comment?
>>
> 
> Done ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115272 ).

I've pushed the v2 of this patch.

As for gcc, I posted a prototype patch in bugzilla, and pinged it today, 
hoping to get some indication on whether it makes sense to do the effort 
for a proper submission.

Thanks,
- Tom



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-06-19 15:41 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-27  3:29 [PATCH 1/3] [gdb/exp] Fix target type of complex long double on arm Tom de Vries
2024-05-27  3:29 ` [PATCH 2/3] [gdb/exp] Fix gdb.fortran/intrinsics.exp fail " Tom de Vries
2024-06-11  8:51   ` Tom de Vries
2024-06-13  9:14     ` Tom de Vries
2024-05-27  3:29 ` [PATCH 3/3] [gdb/testsuite] Fix gdb.fortran/array-bounds.exp " Tom de Vries
2024-06-07  6:13   ` Tom de Vries
2024-05-28 15:11 ` [PATCH 1/3] [gdb/exp] Fix target type of complex long double " Tom Tromey
2024-05-29 11:44   ` Tom de Vries
2024-06-03 14:37     ` Tom Tromey
2024-06-07 13:36     ` Tom de Vries
2024-06-19 15:42     ` Tom de Vries
2024-06-03 14:52 ` Luis Machado
2024-06-03 15:04   ` Tom de Vries

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).