[PATCH] Fortran: improve checking of character length specification [PR96025]

[PATCH] Fortran: improve checking of character length specification [PR96025]

[Harald Anlauf]

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

Dear all,

the attached patch fixes an ICE on invalid (non-integer)
specification expressions for character length in function
declarations.  It appears that the error handling was
already in place (mostly) and we need to essentially
prevent run-on errors.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

The PR is marked as a 10/11/12/13 regression, so I would
like to backport this as far as it seems reasonable.

Thanks,
Harald


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr96025.diff --]
[-- Type: text/x-patch, Size: 3667 bytes --]

From f581f63e206b54278c27a5c888c2566cb5077f11 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Mon, 20 Feb 2023 21:28:09 +0100
Subject: [PATCH] Fortran: improve checking of character length specification
 [PR96025]

gcc/fortran/ChangeLog:

	PR fortran/96025
	* parse.cc (check_function_result_typed): Improve type check of
	specification expression for character length and return status.
	(parse_spec): Use status from above.
	* resolve.cc (resolve_fntype): Prevent use of invalid specification
	expression for character length.

gcc/testsuite/ChangeLog:

	PR fortran/96025
	* gfortran.dg/pr96025.f90: New test.
---
 gcc/fortran/parse.cc                  | 23 ++++++++++++++++-------
 gcc/fortran/resolve.cc                |  4 +++-
 gcc/testsuite/gfortran.dg/pr96025.f90 | 11 +++++++++++
 3 files changed, 30 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr96025.f90

diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index f5154d97ae8..47876a3833e 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -3974,21 +3974,30 @@ match_deferred_characteristics (gfc_typespec * ts)
    For return types specified in a FUNCTION prefix, the IMPLICIT rules of the
    scope are not yet parsed so this has to be delayed up to parse_spec.  */

-static void
+static bool
 check_function_result_typed (void)
 {
   gfc_typespec ts;

   gcc_assert (gfc_current_state () == COMP_FUNCTION);

-  if (!gfc_current_ns->proc_name->result) return;
+  if (!gfc_current_ns->proc_name->result)
+    return true;

   ts = gfc_current_ns->proc_name->result->ts;

   /* Check type-parameters, at the moment only CHARACTER lengths possible.  */
   /* TODO:  Extend when KIND type parameters are implemented.  */
   if (ts.type == BT_CHARACTER && ts.u.cl && ts.u.cl->length)
-    gfc_expr_check_typed (ts.u.cl->length, gfc_current_ns, true);
+    {
+      /* Reject invalid type of specification expression for length.  */
+      if (ts.u.cl->length->ts.type != BT_INTEGER)
+	  return false;
+
+      gfc_expr_check_typed (ts.u.cl->length, gfc_current_ns, true);
+    }
+
+  return true;
 }


@@ -4097,8 +4106,8 @@ loop:

       if (verify_now)
 	{
-	  check_function_result_typed ();
-	  function_result_typed = true;
+	  if (check_function_result_typed ())
+	    function_result_typed = true;
 	}
     }

@@ -4111,8 +4120,8 @@ loop:
     case ST_IMPLICIT:
       if (!function_result_typed)
 	{
-	  check_function_result_typed ();
-	  function_result_typed = true;
+	  if (check_function_result_typed ())
+	    function_result_typed = true;
 	}
       goto declSt;

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index fb0745927ac..427f901a438 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -17419,7 +17419,9 @@ resolve_fntype (gfc_namespace *ns)
 	  }
       }

-  if (sym->ts.type == BT_CHARACTER)
+  if (sym->ts.type == BT_CHARACTER
+      && sym->ts.u.cl->length
+      && sym->ts.u.cl->length->ts.type == BT_INTEGER)
     gfc_traverse_expr (sym->ts.u.cl->length, sym, flag_fn_result_spec, 0);
 }

diff --git a/gcc/testsuite/gfortran.dg/pr96025.f90 b/gcc/testsuite/gfortran.dg/pr96025.f90
new file mode 100644
index 00000000000..ce292bd9664
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr96025.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR fortran/96025 - ICE in expr_check_typed_help
+! Contributed by G.Steinmetz
+
+program p
+  print *, f()
+contains
+  character(char(1)) function f() ! { dg-error "must be of INTEGER type" }
+    f = 'f'
+  end
+end
--
2.35.3

Re: [PATCH] Fortran: improve checking of character length specification [PR96025]

[Thomas Koenig]

Hi Harald,

> the attached patch fixes an ICE on invalid (non-integer)
> specification expressions for character length in function
> declarations.  It appears that the error handling was
> already in place (mostly) and we need to essentially
> prevent run-on errors.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
As a very minor matter of style, you might want to write

           function_result_typed = check_function_result_typed ();

instead of

	  if (check_function_result_typed ())
	    function_result_typed = true;

OK either way.

> The PR is marked as a 10/11/12/13 regression, so I would
> like to backport this as far as it seems reasonable.

Also OK.

Thanks for the patch!

Best regards

	Thomas

Re: [PATCH] Fortran: improve checking of character length specification [PR96025]

[Harald Anlauf]

Hi Thomas,

Am 21.02.23 um 08:19 schrieb Thomas Koenig via Gcc-patches:
> Hi Harald,
>
>> the attached patch fixes an ICE on invalid (non-integer)
>> specification expressions for character length in function
>> declarations.  It appears that the error handling was
>> already in place (mostly) and we need to essentially
>> prevent run-on errors.
>>
>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> As a very minor matter of style, you might want to write
>
>            function_result_typed = check_function_result_typed ();
>
> instead of
>
>        if (check_function_result_typed ())
>          function_result_typed = true;

I was considering that too, but believed that the logic around
these places (a loop and an if) would confuse readers.
Thinking again and rechecking, I've changed the patch to follow
your suggestion, including a minor style cleanup.

Committed as:

https://gcc.gnu.org/g:6c1b825b3d6499dfeacf7c79dcf4b56a393ac204

commit r13-6265-g6c1b825b3d6499dfeacf7c79dcf4b56a393ac204
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Mon Feb 20 21:28:09 2023 +0100

> OK either way.
>
>> The PR is marked as a 10/11/12/13 regression, so I would
>> like to backport this as far as it seems reasonable.
>
> Also OK.
>
> Thanks for the patch!

Thanks for the review!

Harald


> Best regards
>
>      Thomas
>