diff mbox series

Fortran: improve checking of character length specification [PR96025]

Message ID trinity-70379611-3104-41fb-b298-b55372325a13-1676925762184@3c-app-gmx-bap60
State Accepted
Headers
Series Fortran: improve checking of character length specification [PR96025] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Harald Anlauf Feb. 20, 2023, 8:42 p.m. UTC 
  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

Comments

Thomas Koenig Feb. 21, 2023, 7:19 a.m. UTC | #1 
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
Harald Anlauf Feb. 21, 2023, 6:11 p.m. UTC | #2 
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
>
diff mbox series

Patch

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