[Fortran,PR82904] Fix [11/12/13/14/15 Regression][Coarray] ICE in make_ssa_name_fn, at tree-ssanames.c:261

Message ID 20240710145153.2ccbe68c@vepi2
State New
Headers
Series [Fortran,PR82904] Fix [11/12/13/14/15 Regression][Coarray] ICE in make_ssa_name_fn, at tree-ssanames.c:261 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Andre Vehreschild July 10, 2024, 12:51 p.m. UTC
  Hi all,

the patch attached fixes the use of an uninitialized variable for the string
length in the declaration of the char[1:_len] type (the _len!). The type for
save'd deferred length char arrays is now char*, so that there is no need for
the length in the type declaration anymore. The length is of course still
provided and needed later on.

I hope this fixes the ICE in the IPA: inline phase, because I never saw it. Is
that what you had in mind @Richard?

Regtests ok on x86_64-pc-linux-gnu/Fedora 39. Ok for mainline?

Regards,
	Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
  

Comments

Richard Biener July 10, 2024, 12:56 p.m. UTC | #1
On Wed, 10 Jul 2024, Andre Vehreschild wrote:

> Hi all,
> 
> the patch attached fixes the use of an uninitialized variable for the string
> length in the declaration of the char[1:_len] type (the _len!). The type for
> save'd deferred length char arrays is now char*, so that there is no need for
> the length in the type declaration anymore. The length is of course still
> provided and needed later on.
> 
> I hope this fixes the ICE in the IPA: inline phase, because I never saw it. Is
> that what you had in mind @Richard?

I think this will fix the issue by side-stepping the use of a 
variable-length typed variable.
 
> Regtests ok on x86_64-pc-linux-gnu/Fedora 39. Ok for mainline?
> 
> Regards,
> 	Andre
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
>
  
Andre Vehreschild July 17, 2024, 1:07 p.m. UTC | #2
Hi all,

pinging for attached patch rebased on master and my patch for 78466.

Anyone in for a review?

Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?

Regards,
	Andre

On Wed, 10 Jul 2024 14:51:53 +0200
Andre Vehreschild <vehre@gmx.de> wrote:

> Hi all,
>
> the patch attached fixes the use of an uninitialized variable for the string
> length in the declaration of the char[1:_len] type (the _len!). The type for
> save'd deferred length char arrays is now char*, so that there is no need for
> the length in the type declaration anymore. The length is of course still
> provided and needed later on.
>
> I hope this fixes the ICE in the IPA: inline phase, because I never saw it. Is
> that what you had in mind @Richard?
>
> Regtests ok on x86_64-pc-linux-gnu/Fedora 39. Ok for mainline?
>
> Regards,
> 	Andre
> --
> Andre Vehreschild * Email: vehre ad gmx dot de


--
Andre Vehreschild * Email: vehre ad gmx dot de
  
Paul Richard Thomas July 17, 2024, 3:55 p.m. UTC | #3
Hi Andre,

It looks good to me. I am happy to see that the principle of the patch has
Richi's blessing too.

OK for mainline. I leave it for you (and Richi?) to decide whether to
backport in time for the 14.2 release.

Regards

Paul


On Wed, 17 Jul 2024 at 14:08, Andre Vehreschild <vehre@gmx.de> wrote:

> Hi all,
>
> pinging for attached patch rebased on master and my patch for 78466.
>
> Anyone in for a review?
>
> Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
>
> Regards,
>         Andre
>
> On Wed, 10 Jul 2024 14:51:53 +0200
> Andre Vehreschild <vehre@gmx.de> wrote:
>
> > Hi all,
> >
> > the patch attached fixes the use of an uninitialized variable for the
> string
> > length in the declaration of the char[1:_len] type (the _len!). The type
> for
> > save'd deferred length char arrays is now char*, so that there is no
> need for
> > the length in the type declaration anymore. The length is of course still
> > provided and needed later on.
> >
> > I hope this fixes the ICE in the IPA: inline phase, because I never saw
> it. Is
> > that what you had in mind @Richard?
> >
> > Regtests ok on x86_64-pc-linux-gnu/Fedora 39. Ok for mainline?
> >
> > Regards,
> >       Andre
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
>
  
Andre Vehreschild July 18, 2024, 7:30 a.m. UTC | #4
Hi Paul,

thanks for the review. Applied as gcc-15-2131-g0231b076dc9 .

Regards,
	Andre

> <snipp>
> Hi Andre,
>
> It looks good to me. I am happy to see that the principle of the
> patch has Richi's blessing too.
>
> OK for mainline. I leave it for you (and Richi?) to decide whether to
> backport in time for the 14.2 release.
>
> Regards
>
> Paul


On Wed, 10 Jul 2024 14:51:53 +0200
Andre Vehreschild <vehre@gmx.de> wrote:

> Hi all,
>
> the patch attached fixes the use of an uninitialized variable for the
> string length in the declaration of the char[1:_len] type (the
> _len!). The type for save'd deferred length char arrays is now char*,
> so that there is no need for the length in the type declaration
> anymore. The length is of course still provided and needed later on.
>
> I hope this fixes the ICE in the IPA: inline phase, because I never
> saw it. Is that what you had in mind @Richard?
>
> Regtests ok on x86_64-pc-linux-gnu/Fedora 39. Ok for mainline?
>
> Regards,
> 	Andre
> --
> Andre Vehreschild * Email: vehre ad gmx dot de



--
Andre Vehreschild * Email: vehre ad gcc dot gnu dot org
  

Patch

From 48f0a67b4ea241567f660052302f6f021778b232 Mon Sep 17 00:00:00 2001
From: Andre Vehreschild <vehre@gcc.gnu.org>
Date: Wed, 10 Jul 2024 14:37:37 +0200
Subject: [PATCH] Fortran: Use char* for deferred length character arrays
 [PR82904]

Randomly during compiling the pass IPA: inline would ICE.  This was
caused by a saved deferred length string.  The length variable was not
set, but the variable was used in the array's declaration.  Now using a
character pointer to prevent this.

gcc/fortran/ChangeLog:

	* trans-types.cc (gfc_sym_type): Use type `char*` for saved
	deferred length char arrays.
	* trans.cc (get_array_span): Get `.span` also for `char*` typed
	arrays, i.e. for those that have INTEGER_TYPE instead of
	ARRAY_TYPE.

gcc/testsuite/ChangeLog:

	* gfortran.dg/deferred_character_38.f90: New test.
---
 gcc/fortran/trans-types.cc                    |  6 ++++--
 gcc/fortran/trans.cc                          |  4 +++-
 .../gfortran.dg/deferred_character_38.f90     | 20 +++++++++++++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/deferred_character_38.f90

diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc
index 42a7934db9d..c76cdca4eae 100644
--- a/gcc/fortran/trans-types.cc
+++ b/gcc/fortran/trans-types.cc
@@ -2320,8 +2320,10 @@  gfc_sym_type (gfc_symbol * sym, bool is_bind_c)
 	  || ((sym->attr.result || sym->attr.value)
 	      && sym->ns->proc_name
 	      && sym->ns->proc_name->attr.is_bind_c)
-	  || (sym->ts.deferred && (!sym->ts.u.cl
-				   || !sym->ts.u.cl->backend_decl))
+	  || (sym->ts.deferred
+	      && (!sym->ts.u.cl
+		  || !sym->ts.u.cl->backend_decl
+		  || sym->attr.save))
 	  || (sym->attr.dummy
 	      && sym->attr.value
 	      && gfc_length_one_character_type_p (&sym->ts))))
diff --git a/gcc/fortran/trans.cc b/gcc/fortran/trans.cc
index 1067e032621..d4c54093cbc 100644
--- a/gcc/fortran/trans.cc
+++ b/gcc/fortran/trans.cc
@@ -398,7 +398,9 @@  get_array_span (tree type, tree decl)
     return gfc_conv_descriptor_span_get (decl);

   /* Return the span for deferred character length array references.  */
-  if (type && TREE_CODE (type) == ARRAY_TYPE && TYPE_STRING_FLAG (type))
+  if (type
+      && (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == INTEGER_TYPE)
+      && TYPE_STRING_FLAG (type))
     {
       if (TREE_CODE (decl) == PARM_DECL)
 	decl = build_fold_indirect_ref_loc (input_location, decl);
diff --git a/gcc/testsuite/gfortran.dg/deferred_character_38.f90 b/gcc/testsuite/gfortran.dg/deferred_character_38.f90
new file mode 100644
index 00000000000..d5a6c0e5013
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/deferred_character_38.f90
@@ -0,0 +1,20 @@ 
+! { dg-do run }
+
+! Check for PR fortran/82904
+! Contributed by G.Steinmetz  <gscfq@t-online.de>
+
+! This test checks that 'IPA pass: inline' passes.
+! The initial version of the testcase contained coarrays, which does not work
+! yet.
+
+program p
+   save
+   character(:), allocatable :: x
+   character(:), allocatable :: y
+   allocate (character(3) :: y)
+   allocate (x, source='abc')
+   y = x
+
+   if (y /= 'abc') stop 1
+end
+
--
2.45.2