public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r10-10554] fortran: Backport associate character length fixes [PR104228]
@ 2022-04-24 16:54 Mikael Morin
  0 siblings, 0 replies; only message in thread
From: Mikael Morin @ 2022-04-24 16:54 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:4c8e037d32e91c0ec4ac577d1d0723ab9d6ec809

commit r10-10554-g4c8e037d32e91c0ec4ac577d1d0723ab9d6ec809
Author: Mikael Morin <mikael@gcc.gnu.org>
Date:   Fri Jan 28 22:00:57 2022 +0100

    fortran: Backport associate character length fixes [PR104228]
    
    Backport commits:
    r12-7217-g57da34939703a6e6d3267a0d25d1fb9369d3ac0e
    r12-7726-g907811ddc35da6c1701ed22355ece63a8c3ed7fb
    
    --
    
    fortran: Unshare associate var charlen [PR104228]
    
    PR104228 showed that character lengths were shared between associate
    variable and associate targets.  This is problematic when the associate
    target is itself a variable and gets a variable to hold the length, as
    the length variable is added (and all the variables following it in the chain)
    to both the associate variable scope and the target variable scope.
    This caused an ICE when compiling with -O0 -fsanitize=address.
    
    This change forces the creation of a separate character length for the
    associate variable.  It also forces the initialization of the character
    length variable to avoid regressing associate_32 and associate_47 tests.
    
    --
    
    fortran: Separate associate character lengths earlier [PR104570]
    
    This change workarounds an ICE in the evaluation of the character length
    of an array expression referencing an associate variable; the code is
    not prepared to see a non-scalar expression as it doesn’t initialize the
    scalarizer.
    
    Before this change, associate length symbols get a new gfc_charlen at
    resolution stage to unshare them from the associate expression, so that
    at translation stage it is a decl specific to the associate symbol that
    is initialized, not the decl of some other symbol.  This
    reinitialization of gfc_charlen happens after expressions referencing
    the associate symbol have been parsed, so that those expressions retain
    the original gfc_charlen they have copied from the symbol.
    At translation stage, the gfc_charlen for the associate symbol is setup
    with the decl holding the actual length value, but the expressions have
    retained the original gfc_charlen without any decl.  So they need to
    evaluate the character length, and this is where the ICE happens.
    
    This change moves the reinitialization of gfc_charlen earlier at parsing
    stage, so that at resolution stage the gfc_charlen can be retained as
    it’s already not shared with any other symbol, and the expressions which
    now share their gfc_charlen with the symbol are automatically updated
    when the length decl is setup at translation stage.  There is no need
    any more to evaluate the character length as it has all the required
    information, and the ICE doesn’t happen.
    
    The first resolve.c hunk is necessary to avoid regressing on the
    associate_35.f90 testcase.
    
    --
    
            PR fortran/104228
            PR fortran/104570
    
    gcc/fortran/ChangeLog:
    
            * parse.c (parse_associate): Use a new distinct gfc_charlen if
            the copied type has one whose length is not known to be
            constant.
            * resolve.c (resolve_assoc_var): Also create a new character
            length for non-dummy associate targets.  Reset charlen if it’s
            shared with the associate target regardless of the expression
            type.  Don’t reinitialize charlen if it’s deferred.
            * trans-stmt.c (trans_associate_var): Initialize character
            length even if no temporary is used for the associate variable.
    
    gcc/testsuite/ChangeLog:
    
            * gfortran.dg/asan_associate_58.f90: New test.
            * gfortran.dg/asan_associate_59.f90: New test.
            * gfortran.dg/associate_58.f90: New test.

Diff:
---
 gcc/fortran/parse.c                             | 18 ++++++++++++++++++
 gcc/fortran/resolve.c                           | 10 ++++++----
 gcc/fortran/trans-stmt.c                        |  2 +-
 gcc/testsuite/gfortran.dg/asan_associate_58.f90 | 19 +++++++++++++++++++
 gcc/testsuite/gfortran.dg/asan_associate_59.f90 | 19 +++++++++++++++++++
 gcc/testsuite/gfortran.dg/associate_58.f90      | 21 +++++++++++++++++++++
 6 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 1501abfffd2..6ddc5171e2f 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -4678,6 +4678,24 @@ parse_associate (void)
 	 in case of association to a derived-type.  */
       sym->ts = a->target->ts;
 
+      /* Don’t share the character length information between associate
+	 variable and target if the length is not a compile-time constant,
+	 as we don’t want to touch some other character length variable when
+	 we try to initialize the associate variable’s character length
+	 variable.
+	 We do it here rather than later so that expressions referencing the
+	 associate variable will automatically have the correctly setup length
+	 information.  If we did it at resolution stage the expressions would
+	 use the original length information, and the variable a new different
+	 one, but only the latter one would be correctly initialized at
+	 translation stage, and the former one would need some additional setup
+	 there.  */
+      if (sym->ts.type == BT_CHARACTER
+	  && sym->ts.u.cl
+	  && !(sym->ts.u.cl->length
+	       && sym->ts.u.cl->length->expr_type == EXPR_CONSTANT))
+	sym->ts.u.cl = gfc_new_charlen (gfc_current_ns, NULL);
+
       /* Check if the target expression is array valued.  This cannot always
 	 be done by looking at target.rank, because that might not have been
 	 set yet.  Therefore traverse the chain of refs, looking for the last
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index dfc22e25e43..15c88b23927 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -9141,8 +9141,7 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
       if (!sym->ts.u.cl)
 	sym->ts.u.cl = target->ts.u.cl;
 
-      if (sym->ts.deferred && target->expr_type == EXPR_VARIABLE
-	  && target->symtree->n.sym->attr.dummy
+      if (sym->ts.deferred
 	  && sym->ts.u.cl == target->ts.u.cl)
 	{
 	  sym->ts.u.cl = gfc_new_charlen (sym->ns, NULL);
@@ -9161,8 +9160,11 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
 		|| sym->ts.u.cl->length->expr_type != EXPR_CONSTANT)
 		&& target->expr_type != EXPR_VARIABLE)
 	{
-	  sym->ts.u.cl = gfc_new_charlen (sym->ns, NULL);
-	  sym->ts.deferred = 1;
+	  if (!sym->ts.deferred)
+	    {
+	      sym->ts.u.cl = gfc_new_charlen (sym->ns, NULL);
+	      sym->ts.deferred = 1;
+	    }
 
 	  /* This is reset in trans-stmt.c after the assignment
 	     of the target expression to the associate name.  */
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 27d885d1c89..cecb7430bd5 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -1890,7 +1890,7 @@ trans_associate_var (gfc_symbol *sym, gfc_wrapped_block *block)
       gfc_conv_expr_descriptor (&se, e);
 
       if (sym->ts.type == BT_CHARACTER
-	  && !se.direct_byref && sym->ts.deferred
+	  && sym->ts.deferred
 	  && !sym->attr.select_type_temporary
 	  && VAR_P (sym->ts.u.cl->backend_decl)
 	  && se.string_length != sym->ts.u.cl->backend_decl)
diff --git a/gcc/testsuite/gfortran.dg/asan_associate_58.f90 b/gcc/testsuite/gfortran.dg/asan_associate_58.f90
new file mode 100644
index 00000000000..b5ea75498b7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/asan_associate_58.f90
@@ -0,0 +1,19 @@
+! { dg-do compile }
+! { dg-additional-options "-O0" }
+!
+! PR fortran/104228
+! The code generated code for the program below wrongly pushed the Y character
+! length variable to both P and S scope, which was leading to an ICE when
+! address sanitizer was in effect
+
+program p
+   character(:), save, allocatable :: x(:)
+   call s
+contains
+   subroutine s
+      associate (y => x)
+         y = [x]
+      end associate
+   end
+end
+
diff --git a/gcc/testsuite/gfortran.dg/asan_associate_59.f90 b/gcc/testsuite/gfortran.dg/asan_associate_59.f90
new file mode 100644
index 00000000000..9bfb2bfbafb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/asan_associate_59.f90
@@ -0,0 +1,19 @@
+! { dg-do compile }
+! { dg-additional-options "-O0" }
+!
+! PR fortran/104228
+! The code generated code for the program below wrongly pushed the Y character
+! length variable to both P and S scope, which was leading to an ICE when
+! address sanitizer was in effect
+
+program p
+   character(:), allocatable :: x(:)
+   call s
+contains
+   subroutine s
+      associate (y => x)
+         y = [x]
+      end associate
+   end
+end
+
diff --git a/gcc/testsuite/gfortran.dg/associate_58.f90 b/gcc/testsuite/gfortran.dg/associate_58.f90
new file mode 100644
index 00000000000..9c24f35c0d8
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associate_58.f90
@@ -0,0 +1,21 @@
+! { dg-do compile }
+!
+! PR fortran/104570
+! The following used to cause an ICE because the string length
+! evaluation of the (y) expression was not prepared to handle
+! a non-scalar expression.
+
+program p
+   character(:), allocatable :: x(:)
+   x = ['abc']
+   call s
+contains
+   subroutine s
+      associate (y => x)
+         associate (z => (y))
+            print *, z
+         end associate
+      end associate
+   end
+end
+


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-04-24 16:54 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-24 16:54 [gcc r10-10554] fortran: Backport associate character length fixes [PR104228] Mikael Morin

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