[PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Harald Anlauf]

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

Dear all,

when assigning character pointers, we have a check for same length,
which however does not trigger for character pointers within a
structure constructor.

The attached patch extends the character checks slightly to fix
this loophole.  I've verified that NAG and Crayftn behave similarly,
while Intel does not detect the length difference.

Regtested on x86_64-pc-linux-gnu.

OK for mainline?  Would it be still ok for 12, or rather wait until
branching for 13?

Thanks,
Harald


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fortran-character-length-of-pointer-assignments-in-s.patch --]
[-- Type: text/x-patch, Size: 3279 bytes --]

From 3b88c941619bc4996ef7d4ba247086f04deb8683 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Sun, 27 Mar 2022 21:35:15 +0200
Subject: [PATCH] Fortran: character length of pointer assignments in structure
 constructors

gcc/fortran/ChangeLog:

	PR fortran/50549
	* resolve.cc (resolve_structure_cons): Reject pointer assignments
	of character with different lengths in structure constructor.

gcc/testsuite/ChangeLog:

	PR fortran/50549
	* gfortran.dg/char_pointer_assign_7.f90: New test.
---
 gcc/fortran/resolve.cc                        | 13 ++++++-
 .../gfortran.dg/char_pointer_assign_7.f90     | 38 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 5522be75199..b4400a7ab8d 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1375,11 +1375,22 @@ resolve_structure_cons (gfc_expr *expr, int init)
 	  && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT
 	  && cons->expr->ts.u.cl && cons->expr->ts.u.cl->length
 	  && cons->expr->ts.u.cl->length->expr_type == EXPR_CONSTANT
-	  && cons->expr->rank != 0
 	  && mpz_cmp (cons->expr->ts.u.cl->length->value.integer,
 		      comp->ts.u.cl->length->value.integer) != 0)
 	{
+	  if (comp->attr.pointer)
+	    {
+	      long len_a, len_b;
+	      len_a = mpz_get_si (comp->ts.u.cl->length->value.integer);
+	      len_b = mpz_get_si (cons->expr->ts.u.cl->length->value.integer);
+	      gfc_error ("Unequal character lengths (%ld/%ld) for pointer "
+			 "component %qs in constructor at %L",
+			 len_a, len_b, comp->name, &cons->expr->where);
+	      t = false;
+	    }
+
 	  if (cons->expr->expr_type == EXPR_VARIABLE
+	      && cons->expr->rank != 0
 	      && cons->expr->symtree->n.sym->attr.flavor == FL_PARAMETER)
 	    {
 	      /* Wrap the parameter in an array constructor (EXPR_ARRAY)
diff --git a/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90 b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
new file mode 100644
index 00000000000..08bdf176d8b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
@@ -0,0 +1,38 @@
+! { dg-do compile }
+! PR fortran/50549 - should reject pointer assignments of different lengths
+! in structure constructors
+
+program test
+  implicit none
+  type t
+     character(2), pointer ::  p2
+  end type t
+  type t2
+     character(2), pointer ::  p(:)
+  end type t2
+  type td
+     character(:), pointer ::  pd
+  end type td
+  interface
+     function f1 ()
+       character(1), pointer :: f1
+     end function f1
+     function f2 ()
+       character(2), pointer :: f2
+     end function f2
+  end interface
+
+  character(1),    target  ::  p1
+  character(1),    pointer ::  q1(:)
+  character(2),    pointer ::  q2(:)
+  type(t)  :: u
+  type(t2) :: u2
+  type(td) :: v
+  u  = t(p1)    ! { dg-error "Unequal character lengths" }
+  u  = t(f1())  ! { dg-error "Unequal character lengths" }
+  u  = t(f2())  ! OK
+  u2 = t2(q1)   ! { dg-error "Unequal character lengths" }
+  u2 = t2(q2)   ! OK
+  v  = td(p1)   ! OK
+  v  = td(f1()) ! OK
+end
--
2.34.1

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Tobias Burnus]

Hi Harald,

On 27.03.22 21:44, Harald Anlauf via Fortran wrote:
> when assigning character pointers, we have a check for same length,
> which however does not trigger for character pointers within a
> structure constructor.
>
> The attached patch extends the character checks slightly to fix
> this loophole.  I've verified that NAG and Crayftn behave similarly,
> while Intel does not detect the length difference.
>
> Regtested on x86_64-pc-linux-gnu.
>
> OK for mainline?

Thanks for the patch! LGTM and I think GCC 12 is still okay.

However, I have a nit:

> --- a/gcc/fortran/resolve.cc
> +++ b/gcc/fortran/resolve.cc
> @@ -1375,11 +1375,22 @@ resolve_structure_cons (gfc_expr *expr, int init)
> ...
> +           long len_a, len_b;
> +           len_a = mpz_get_si (comp->ts.u.cl->length->value.integer);
> +           len_b = mpz_get_si (cons->expr->ts.u.cl->length->value.integer);
> +           gfc_error ("Unequal character lengths (%ld/%ld) for pointer "
> +                      "component %qs in constructor at %L",
> +                      len_a, len_b, comp->name, &cons->expr->where);

'long' might be int32_t instead of int64_t (e.g. on Windows, I think both
MinGW32 and MinGW64, but I am not quite sure). Thus, I wonder whether it
makes more sense to use:

   HOST_WIDE_INT, gfc_mpz_get_hwi() and '%wd'

I note that '%wd' (and '%lld') is only supported since last August
(commit https://gcc.gnu.org/r12-3044-g1b507b1e3c5 ), but now that it is,
I think we should use it at places where the value can be larger than INT_MAX.

I think at some point, we should also check the rest of the code and
change those mpz_get_si to gfc_mpz_get_hwi which can exceed INT_MAX.
Likewise, some of the %ld/%lu or %lld/%llu code should be also converted to %wd/%wu.

Tobias

PS: For using HWI with 'sprintf' instead of diagnostic's error/warning,
HOST_WIDE_INT_PRINT_DEC exists and has to be used.

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Harald Anlauf]

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

Hi Tobias,

Am 28.03.22 um 12:05 schrieb Tobias Burnus:
> Thanks for the patch! LGTM and I think GCC 12 is still okay.
>
> However, I have a nit:
>
>> --- a/gcc/fortran/resolve.cc
>> +++ b/gcc/fortran/resolve.cc
>> @@ -1375,11 +1375,22 @@ resolve_structure_cons (gfc_expr *expr, int init)
>> ...
>> +           long len_a, len_b;
>> +           len_a = mpz_get_si (comp->ts.u.cl->length->value.integer);
>> +           len_b = mpz_get_si
>> (cons->expr->ts.u.cl->length->value.integer);
>> +           gfc_error ("Unequal character lengths (%ld/%ld) for pointer "
>> +                      "component %qs in constructor at %L",
>> +                      len_a, len_b, comp->name, &cons->expr->where);
>
> 'long' might be int32_t instead of int64_t (e.g. on Windows, I think both
> MinGW32 and MinGW64, but I am not quite sure). Thus, I wonder whether it
> makes more sense to use:
>
>    HOST_WIDE_INT, gfc_mpz_get_hwi() and '%wd'
>
> I note that '%wd' (and '%lld') is only supported since last August
> (commit https://gcc.gnu.org/r12-3044-g1b507b1e3c5 ), but now that it is,
> I think we should use it at places where the value can be larger than
> INT_MAX.

using HOST_WIDE_INT as in the updated patch (sort of) works, but for
some reason I do not yet understand the format check kicks in for
gfc_error, producing:

../../gcc-trunk/gcc/fortran/resolve.cc: In function 'bool
resolve_structure_cons(gfc_expr*, int)':
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: unknown
conversion type character 'w' in format [-Wformat=]
      la, lb, comp->name, &cons->expr->where);
                                            ^
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: unknown
conversion type character 'w' in format [-Wformat=]
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: format '%s'
expects argument of type 'char*', but argument 2 has type 'long int'
[-Wformat=]
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: format '%L'
expects argument of type 'locus*', but argument 3 has type 'long int'
[-Wformat=]
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: too many
arguments for format [-Wformat-extra-args]

This would likely lead to a bootstrap error.

Do I need to add some forgotten include?  Or some annotation to
suppress the warning?

Or should I rather convert the character lengths via sprintf first
before generating the error message?  (That would be the quick fix.)

> I think at some point, we should also check the rest of the code and
> change those mpz_get_si to gfc_mpz_get_hwi which can exceed INT_MAX.
> Likewise, some of the %ld/%lu or %lld/%llu code should be also converted
> to %wd/%wu.
>
> Tobias
>
> PS: For using HWI with 'sprintf' instead of diagnostic's error/warning,
> HOST_WIDE_INT_PRINT_DEC exists and has to be used.

All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
'sprintf', and I did not find any other use of %wd/%wu.  So the
mentioned implementation is not really stressed yet... ;-)

Thanks,
Harald

> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201,
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer:
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München;
> Registergericht München, HRB 106955
>

[-- Attachment #2: 0001-Fortran-character-length-of-pointer-assignments-in-s.patch --]
[-- Type: text/x-patch, Size: 3184 bytes --]

From 7efd0613261c5d2120e189387f4b916917c25683 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Sun, 27 Mar 2022 21:35:15 +0200
Subject: [PATCH] Fortran: character length of pointer assignments in structure
 constructors

gcc/fortran/ChangeLog:

	PR fortran/50549
	* resolve.cc (resolve_structure_cons): Reject pointer assignments
	of character with different lengths in structure constructor.

gcc/testsuite/ChangeLog:

	PR fortran/50549
	* gfortran.dg/char_pointer_assign_7.f90: New test.
---
 gcc/fortran/resolve.cc                        | 13 ++++++-
 .../gfortran.dg/char_pointer_assign_7.f90     | 38 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 5522be75199..290767723d8 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1375,11 +1375,22 @@ resolve_structure_cons (gfc_expr *expr, int init)
 	  && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT
 	  && cons->expr->ts.u.cl && cons->expr->ts.u.cl->length
 	  && cons->expr->ts.u.cl->length->expr_type == EXPR_CONSTANT
-	  && cons->expr->rank != 0
 	  && mpz_cmp (cons->expr->ts.u.cl->length->value.integer,
 		      comp->ts.u.cl->length->value.integer) != 0)
 	{
+	  if (comp->attr.pointer)
+	    {
+	      HOST_WIDE_INT la, lb;
+	      la = gfc_mpz_get_hwi (comp->ts.u.cl->length->value.integer);
+	      lb = gfc_mpz_get_hwi (cons->expr->ts.u.cl->length->value.integer);
+	      gfc_error ("Unequal character lengths (%wd/%wd) for pointer "
+			 "component %qs in constructor at %L",
+			 la, lb, comp->name, &cons->expr->where);
+	      t = false;
+	    }
+
 	  if (cons->expr->expr_type == EXPR_VARIABLE
+	      && cons->expr->rank != 0
 	      && cons->expr->symtree->n.sym->attr.flavor == FL_PARAMETER)
 	    {
 	      /* Wrap the parameter in an array constructor (EXPR_ARRAY)
diff --git a/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90 b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
new file mode 100644
index 00000000000..08bdf176d8b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
@@ -0,0 +1,38 @@
+! { dg-do compile }
+! PR fortran/50549 - should reject pointer assignments of different lengths
+! in structure constructors
+
+program test
+  implicit none
+  type t
+     character(2), pointer ::  p2
+  end type t
+  type t2
+     character(2), pointer ::  p(:)
+  end type t2
+  type td
+     character(:), pointer ::  pd
+  end type td
+  interface
+     function f1 ()
+       character(1), pointer :: f1
+     end function f1
+     function f2 ()
+       character(2), pointer :: f2
+     end function f2
+  end interface
+
+  character(1),    target  ::  p1
+  character(1),    pointer ::  q1(:)
+  character(2),    pointer ::  q2(:)
+  type(t)  :: u
+  type(t2) :: u2
+  type(td) :: v
+  u  = t(p1)    ! { dg-error "Unequal character lengths" }
+  u  = t(f1())  ! { dg-error "Unequal character lengths" }
+  u  = t(f2())  ! OK
+  u2 = t2(q1)   ! { dg-error "Unequal character lengths" }
+  u2 = t2(q2)   ! OK
+  v  = td(p1)   ! OK
+  v  = td(f1()) ! OK
+end
-- 
2.34.1

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Harald Anlauf]

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

Hi Tobias,

Am 28.03.22 um 12:05 schrieb Tobias Burnus:
> Thanks for the patch! LGTM and I think GCC 12 is still okay.
> 
> However, I have a nit:
> 
>> --- a/gcc/fortran/resolve.cc
>> +++ b/gcc/fortran/resolve.cc
>> @@ -1375,11 +1375,22 @@ resolve_structure_cons (gfc_expr *expr, int init)
>> ...
>> +           long len_a, len_b;
>> +           len_a = mpz_get_si (comp->ts.u.cl->length->value.integer);
>> +           len_b = mpz_get_si 
>> (cons->expr->ts.u.cl->length->value.integer);
>> +           gfc_error ("Unequal character lengths (%ld/%ld) for pointer "
>> +                      "component %qs in constructor at %L",
>> +                      len_a, len_b, comp->name, &cons->expr->where);
> 
> 'long' might be int32_t instead of int64_t (e.g. on Windows, I think both
> MinGW32 and MinGW64, but I am not quite sure). Thus, I wonder whether it
> makes more sense to use:
> 
>    HOST_WIDE_INT, gfc_mpz_get_hwi() and '%wd'
> 
> I note that '%wd' (and '%lld') is only supported since last August
> (commit https://gcc.gnu.org/r12-3044-g1b507b1e3c5 ), but now that it is,
> I think we should use it at places where the value can be larger than 
> INT_MAX.

using HOST_WIDE_INT as in the updated patch (sort of) works, but for
some reason I do not yet understand the format check kicks in for
gfc_error, producing:

../../gcc-trunk/gcc/fortran/resolve.cc: In function 'bool 
resolve_structure_cons(gfc_expr*, int)':
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: unknown 
conversion type character 'w' in format [-Wformat=]
      la, lb, comp->name, &cons->expr->where);
                                            ^
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: unknown 
conversion type character 'w' in format [-Wformat=]
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: format '%s' 
expects argument of type 'char*', but argument 2 has type 'long int' 
[-Wformat=]
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: format '%L' 
expects argument of type 'locus*', but argument 3 has type 'long int' 
[-Wformat=]
../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: too many 
arguments for format [-Wformat-extra-args]

This would likely lead to a bootstrap error.

Do I need to add some forgotten include?  Or some annotation to
suppress the warning?

Or should I rather convert the character lengths via sprintf first
before generating the error message?  (That would be the quick fix.)

> I think at some point, we should also check the rest of the code and
> change those mpz_get_si to gfc_mpz_get_hwi which can exceed INT_MAX.
> Likewise, some of the %ld/%lu or %lld/%llu code should be also converted 
> to %wd/%wu.
> 
> Tobias
> 
> PS: For using HWI with 'sprintf' instead of diagnostic's error/warning,
> HOST_WIDE_INT_PRINT_DEC exists and has to be used.

All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
'sprintf', and I did not find any other use of %wd/%wu.  So the
mentioned implementation is not really stressed yet... ;-)

Thanks,
Harald

> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: 
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; 
> Registergericht München, HRB 106955
> 

[-- Attachment #2: 0001-Fortran-character-length-of-pointer-assignments-in-s.patch --]
[-- Type: text/x-patch, Size: 3184 bytes --]

From 7efd0613261c5d2120e189387f4b916917c25683 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Sun, 27 Mar 2022 21:35:15 +0200
Subject: [PATCH] Fortran: character length of pointer assignments in structure
 constructors

gcc/fortran/ChangeLog:

	PR fortran/50549
	* resolve.cc (resolve_structure_cons): Reject pointer assignments
	of character with different lengths in structure constructor.

gcc/testsuite/ChangeLog:

	PR fortran/50549
	* gfortran.dg/char_pointer_assign_7.f90: New test.
---
 gcc/fortran/resolve.cc                        | 13 ++++++-
 .../gfortran.dg/char_pointer_assign_7.f90     | 38 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 5522be75199..290767723d8 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1375,11 +1375,22 @@ resolve_structure_cons (gfc_expr *expr, int init)
 	  && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT
 	  && cons->expr->ts.u.cl && cons->expr->ts.u.cl->length
 	  && cons->expr->ts.u.cl->length->expr_type == EXPR_CONSTANT
-	  && cons->expr->rank != 0
 	  && mpz_cmp (cons->expr->ts.u.cl->length->value.integer,
 		      comp->ts.u.cl->length->value.integer) != 0)
 	{
+	  if (comp->attr.pointer)
+	    {
+	      HOST_WIDE_INT la, lb;
+	      la = gfc_mpz_get_hwi (comp->ts.u.cl->length->value.integer);
+	      lb = gfc_mpz_get_hwi (cons->expr->ts.u.cl->length->value.integer);
+	      gfc_error ("Unequal character lengths (%wd/%wd) for pointer "
+			 "component %qs in constructor at %L",
+			 la, lb, comp->name, &cons->expr->where);
+	      t = false;
+	    }
+
 	  if (cons->expr->expr_type == EXPR_VARIABLE
+	      && cons->expr->rank != 0
 	      && cons->expr->symtree->n.sym->attr.flavor == FL_PARAMETER)
 	    {
 	      /* Wrap the parameter in an array constructor (EXPR_ARRAY)
diff --git a/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90 b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
new file mode 100644
index 00000000000..08bdf176d8b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
@@ -0,0 +1,38 @@
+! { dg-do compile }
+! PR fortran/50549 - should reject pointer assignments of different lengths
+! in structure constructors
+
+program test
+  implicit none
+  type t
+     character(2), pointer ::  p2
+  end type t
+  type t2
+     character(2), pointer ::  p(:)
+  end type t2
+  type td
+     character(:), pointer ::  pd
+  end type td
+  interface
+     function f1 ()
+       character(1), pointer :: f1
+     end function f1
+     function f2 ()
+       character(2), pointer :: f2
+     end function f2
+  end interface
+
+  character(1),    target  ::  p1
+  character(1),    pointer ::  q1(:)
+  character(2),    pointer ::  q2(:)
+  type(t)  :: u
+  type(t2) :: u2
+  type(td) :: v
+  u  = t(p1)    ! { dg-error "Unequal character lengths" }
+  u  = t(f1())  ! { dg-error "Unequal character lengths" }
+  u  = t(f2())  ! OK
+  u2 = t2(q1)   ! { dg-error "Unequal character lengths" }
+  u2 = t2(q2)   ! OK
+  v  = td(p1)   ! OK
+  v  = td(f1()) ! OK
+end
-- 
2.34.1

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Harald Anlauf]

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

Hi Tobias,

sorry for replying to myself now, but

Am 28.03.22 um 22:03 schrieb Harald Anlauf via Fortran:
> All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
> 'sprintf', and I did not find any other use of %wd/%wu.  So the
> mentioned implementation is not really stressed yet... ;-)

using HOST_WIDE_INT_PRINT_DEC in the format argument to gfc_error
instead of using %wd does not produce a warning and works.
(Also verified with insane character lengths on x86_64).

Harald

[-- Attachment #2: 0001-Fortran-character-length-of-pointer-assignments-in-s.patch --]
[-- Type: text/x-patch, Size: 3234 bytes --]

From f6b337c8c5f38acc40787ac6bef029c5321a3f4a Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Sun, 27 Mar 2022 21:35:15 +0200
Subject: [PATCH] Fortran: character length of pointer assignments in structure
 constructors

gcc/fortran/ChangeLog:

	PR fortran/50549
	* resolve.cc (resolve_structure_cons): Reject pointer assignments
	of character with different lengths in structure constructor.

gcc/testsuite/ChangeLog:

	PR fortran/50549
	* gfortran.dg/char_pointer_assign_7.f90: New test.
---
 gcc/fortran/resolve.cc                        | 14 ++++++-
 .../gfortran.dg/char_pointer_assign_7.f90     | 38 +++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 5522be75199..57362a75baa 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1375,11 +1375,23 @@ resolve_structure_cons (gfc_expr *expr, int init)
 	  && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT
 	  && cons->expr->ts.u.cl && cons->expr->ts.u.cl->length
 	  && cons->expr->ts.u.cl->length->expr_type == EXPR_CONSTANT
-	  && cons->expr->rank != 0
 	  && mpz_cmp (cons->expr->ts.u.cl->length->value.integer,
 		      comp->ts.u.cl->length->value.integer) != 0)
 	{
+	  if (comp->attr.pointer)
+	    {
+	      HOST_WIDE_INT la, lb;
+	      la = gfc_mpz_get_hwi (comp->ts.u.cl->length->value.integer);
+	      lb = gfc_mpz_get_hwi (cons->expr->ts.u.cl->length->value.integer);
+	      gfc_error ("Unequal character lengths ("
+			 HOST_WIDE_INT_PRINT_DEC "/" HOST_WIDE_INT_PRINT_DEC
+			 ") for pointer component %qs in constructor at %L",
+			 la, lb, comp->name, &cons->expr->where);
+	      t = false;
+	    }
+
 	  if (cons->expr->expr_type == EXPR_VARIABLE
+	      && cons->expr->rank != 0
 	      && cons->expr->symtree->n.sym->attr.flavor == FL_PARAMETER)
 	    {
 	      /* Wrap the parameter in an array constructor (EXPR_ARRAY)
diff --git a/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90 b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
new file mode 100644
index 00000000000..08bdf176d8b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
@@ -0,0 +1,38 @@
+! { dg-do compile }
+! PR fortran/50549 - should reject pointer assignments of different lengths
+! in structure constructors
+
+program test
+  implicit none
+  type t
+     character(2), pointer ::  p2
+  end type t
+  type t2
+     character(2), pointer ::  p(:)
+  end type t2
+  type td
+     character(:), pointer ::  pd
+  end type td
+  interface
+     function f1 ()
+       character(1), pointer :: f1
+     end function f1
+     function f2 ()
+       character(2), pointer :: f2
+     end function f2
+  end interface
+
+  character(1),    target  ::  p1
+  character(1),    pointer ::  q1(:)
+  character(2),    pointer ::  q2(:)
+  type(t)  :: u
+  type(t2) :: u2
+  type(td) :: v
+  u  = t(p1)    ! { dg-error "Unequal character lengths" }
+  u  = t(f1())  ! { dg-error "Unequal character lengths" }
+  u  = t(f2())  ! OK
+  u2 = t2(q1)   ! { dg-error "Unequal character lengths" }
+  u2 = t2(q2)   ! OK
+  v  = td(p1)   ! OK
+  v  = td(f1()) ! OK
+end
-- 
2.34.1

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Harald Anlauf]

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

Hi Tobias,

sorry for replying to myself now, but

Am 28.03.22 um 22:03 schrieb Harald Anlauf via Fortran:
> All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
> 'sprintf', and I did not find any other use of %wd/%wu.  So the
> mentioned implementation is not really stressed yet... ;-)

using HOST_WIDE_INT_PRINT_DEC in the format argument to gfc_error
instead of using %wd does not produce a warning and works.
(Also verified with insane character lengths on x86_64).

Harald

[-- Attachment #2: 0001-Fortran-character-length-of-pointer-assignments-in-s.patch --]
[-- Type: text/x-patch, Size: 3234 bytes --]

From f6b337c8c5f38acc40787ac6bef029c5321a3f4a Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Sun, 27 Mar 2022 21:35:15 +0200
Subject: [PATCH] Fortran: character length of pointer assignments in structure
 constructors

gcc/fortran/ChangeLog:

	PR fortran/50549
	* resolve.cc (resolve_structure_cons): Reject pointer assignments
	of character with different lengths in structure constructor.

gcc/testsuite/ChangeLog:

	PR fortran/50549
	* gfortran.dg/char_pointer_assign_7.f90: New test.
---
 gcc/fortran/resolve.cc                        | 14 ++++++-
 .../gfortran.dg/char_pointer_assign_7.f90     | 38 +++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 5522be75199..57362a75baa 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1375,11 +1375,23 @@ resolve_structure_cons (gfc_expr *expr, int init)
 	  && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT
 	  && cons->expr->ts.u.cl && cons->expr->ts.u.cl->length
 	  && cons->expr->ts.u.cl->length->expr_type == EXPR_CONSTANT
-	  && cons->expr->rank != 0
 	  && mpz_cmp (cons->expr->ts.u.cl->length->value.integer,
 		      comp->ts.u.cl->length->value.integer) != 0)
 	{
+	  if (comp->attr.pointer)
+	    {
+	      HOST_WIDE_INT la, lb;
+	      la = gfc_mpz_get_hwi (comp->ts.u.cl->length->value.integer);
+	      lb = gfc_mpz_get_hwi (cons->expr->ts.u.cl->length->value.integer);
+	      gfc_error ("Unequal character lengths ("
+			 HOST_WIDE_INT_PRINT_DEC "/" HOST_WIDE_INT_PRINT_DEC
+			 ") for pointer component %qs in constructor at %L",
+			 la, lb, comp->name, &cons->expr->where);
+	      t = false;
+	    }
+
 	  if (cons->expr->expr_type == EXPR_VARIABLE
+	      && cons->expr->rank != 0
 	      && cons->expr->symtree->n.sym->attr.flavor == FL_PARAMETER)
 	    {
 	      /* Wrap the parameter in an array constructor (EXPR_ARRAY)
diff --git a/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90 b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
new file mode 100644
index 00000000000..08bdf176d8b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/char_pointer_assign_7.f90
@@ -0,0 +1,38 @@
+! { dg-do compile }
+! PR fortran/50549 - should reject pointer assignments of different lengths
+! in structure constructors
+
+program test
+  implicit none
+  type t
+     character(2), pointer ::  p2
+  end type t
+  type t2
+     character(2), pointer ::  p(:)
+  end type t2
+  type td
+     character(:), pointer ::  pd
+  end type td
+  interface
+     function f1 ()
+       character(1), pointer :: f1
+     end function f1
+     function f2 ()
+       character(2), pointer :: f2
+     end function f2
+  end interface
+
+  character(1),    target  ::  p1
+  character(1),    pointer ::  q1(:)
+  character(2),    pointer ::  q2(:)
+  type(t)  :: u
+  type(t2) :: u2
+  type(td) :: v
+  u  = t(p1)    ! { dg-error "Unequal character lengths" }
+  u  = t(f1())  ! { dg-error "Unequal character lengths" }
+  u  = t(f2())  ! OK
+  u2 = t2(q1)   ! { dg-error "Unequal character lengths" }
+  u2 = t2(q2)   ! OK
+  v  = td(p1)   ! OK
+  v  = td(f1()) ! OK
+end
-- 
2.34.1

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Joseph Myers]

On Mon, 28 Mar 2022, Harald Anlauf via Gcc-patches wrote:

> Hi Tobias,
> 
> sorry for replying to myself now, but
> 
> Am 28.03.22 um 22:03 schrieb Harald Anlauf via Fortran:
> > All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
> > 'sprintf', and I did not find any other use of %wd/%wu.  So the
> > mentioned implementation is not really stressed yet... ;-)
> 
> using HOST_WIDE_INT_PRINT_DEC in the format argument to gfc_error
> instead of using %wd does not produce a warning and works.
> (Also verified with insane character lengths on x86_64).

Using string concatenation with a macro is not appropriate in a message 
argument to a diagnostic function, because it means the full string (which 
has to be host-independent) doesn't get extracted for translation.

HOST_WIDE_INT_PRINT_* are printf formats for the host printf function (for 
example, they might use %I64d on Windows host), and are not generally 
understood by the diagnostic.cc machinery at all; functions using the GCC 
diagnostic machinery need to use GCC diagnostic formats (which are 
independent of the host printf function), such as %wd/%wu.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Tobias Burnus]

Hi Harald,

On 28.03.22 22:03, Harald Anlauf via Fortran wrote:
> All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
> 'sprintf', and I did not find any other use of %wd/%wu.  So the
> mentioned implementation is not really stressed yet... ;-)

That's all your fault ;-)

(Your commit
https://gcc.gnu.org/r12-3273-ge4cb3bb9ac11b4126ffa718287dd509a4b10a658
did remove the only user.)

> ../../gcc-trunk/gcc/fortran/resolve.cc: In function 'bool
> resolve_structure_cons(gfc_expr*, int)':
> ../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: unknown
> conversion type character 'w' in format [-Wformat=]
>      la, lb, comp->name, &cons->expr->where);
>                                            ^

That's only a warning. Have you tried whether it works at runtime?
My bet is that it does!

Question: Do you build with --disable-bootstrap ? Or do you do a proper bootstrap?

I am asking because:
* Here, it bootstraps *without* warnings/errors (I do a full bootstrap)
* GCC is bootstrapped with -Werror, i.e. I had expected an error and not a warning,
   while with --disable-bootstrap, the -Werror is not used as some random
   compiler may have additional (correct or bogus) warnings.
* %wd is only supported since GCC 12.
   The supported formats + the warning is bound to the compiler version,
   i.e. an older compiler does not support newer flags and, thus, warns for
   them when compiling. (But as the support depends on the current source,
   the compile-time warning of older compilers can be ignored.)

   * * *

Can you check & try again?  I don't mind getting a format warning with
GCC < GCC 12. But with GCC 12 compiled (either installed compiler or
when bootstrapping) it should compile without errors.

If you can confirm my suspicion, the patch LGTM.

Tobias

PS: I played around a bit. And with a GCC 12 bootstrap,
I get as expected an error (not a warning) for something unsupported (%Wd)
while %wd is supported. Additionally, I see a '|' in the output.

The "|" appeared with GCC 9. Thus, I wonder whether you compile w/o
bootstrapping with GCC 8?


../gcc/fortran/resolve.cc:1386:55: error: unknown conversion type character ‘W’ in format [-Werror=format=]
  1386 |               gfc_error ("Unequal character lengths (%Wd/%wd) for pointer "
       |                                                       ^

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Harald Anlauf]

Hi Tobias,

Am 29.03.22 um 09:14 schrieb Tobias Burnus:
> Hi Harald,
>
> On 28.03.22 22:03, Harald Anlauf via Fortran wrote:
>> All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
>> 'sprintf', and I did not find any other use of %wd/%wu.  So the
>> mentioned implementation is not really stressed yet... ;-)
>
> That's all your fault ;-)

true; I'm pleading guilty for that one.

> (Your commit
> https://gcc.gnu.org/r12-3273-ge4cb3bb9ac11b4126ffa718287dd509a4b10a658
> did remove the only user.)

I've now made good for it.  ;-)

> That's only a warning. Have you tried whether it works at runtime?
> My bet is that it does!

Yes, it did work, it was just the warning alerting me ...

> Question: Do you build with --disable-bootstrap ? Or do you do a proper
> bootstrap?

... because I did build with --disable-bootstrap to save on time for
building the compiler on my local machine, and the system's default
gcc is older.

> Can you check & try again?  I don't mind getting a format warning with
> GCC < GCC 12. But with GCC 12 compiled (either installed compiler or
> when bootstrapping) it should compile without errors.
>
> If you can confirm my suspicion, the patch LGTM.

I've just pushed that version as

   https://gcc.gnu.org/g:0712f356374c2cf26015cccfa3141537e42cbb12

Sorry for the noise, and thanks for the review!

Harald

> Tobias

Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors

[Harald Anlauf]

Hi Tobias,

Am 29.03.22 um 09:14 schrieb Tobias Burnus:
> Hi Harald,
> 
> On 28.03.22 22:03, Harald Anlauf via Fortran wrote:
>> All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use
>> 'sprintf', and I did not find any other use of %wd/%wu.  So the
>> mentioned implementation is not really stressed yet... ;-)
> 
> That's all your fault ;-)

true; I'm pleading guilty for that one.

> (Your commit
> https://gcc.gnu.org/r12-3273-ge4cb3bb9ac11b4126ffa718287dd509a4b10a658
> did remove the only user.)

I've now made good for it.  ;-)

> That's only a warning. Have you tried whether it works at runtime?
> My bet is that it does!

Yes, it did work, it was just the warning alerting me ...

> Question: Do you build with --disable-bootstrap ? Or do you do a proper 
> bootstrap?

... because I did build with --disable-bootstrap to save on time for
building the compiler on my local machine, and the system's default
gcc is older.

> Can you check & try again?  I don't mind getting a format warning with
> GCC < GCC 12. But with GCC 12 compiled (either installed compiler or
> when bootstrapping) it should compile without errors.
> 
> If you can confirm my suspicion, the patch LGTM.

I've just pushed that version as

   https://gcc.gnu.org/g:0712f356374c2cf26015cccfa3141537e42cbb12

Sorry for the noise, and thanks for the review!

Harald

> Tobias