Re: [PATCH] Fortran: add bounds-checking for ALLOCATE of CHARACTER with type-spec [PR53357]

2024-11-17 Thread Jerry D

On 11/17/24 2:21 PM, Harald Anlauf wrote:

Dear all,

the attached patch fixes a rejects-valid / rejects-potentially-valid code issue
for  ALLOCATE of CHARACTER with type-spec, and add character length checking
with -fcheck=bounds for the case at hand.  It also improves checking of
character function declarations and references slightly, using the diagnostics
of NAG as a guidance.

Some testcases popped up during regtesting, suggesting that one needs to be
careful not to generate too many false positives, so I decided to not spend
to much time on the FIXME's therein.  (Character length might be expressions
in an explicit interface and the actual declaration, where we don't have a
reliable way to compare.)

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

Thanks,
Harald



Looks good, OK for mainline.

Jerry


[PATCH] Fortran: add bounds-checking for ALLOCATE of CHARACTER with type-spec [PR53357]

2024-11-17 Thread Harald Anlauf
Dear all,

the attached patch fixes a rejects-valid / rejects-potentially-valid code issue
for  ALLOCATE of CHARACTER with type-spec, and add character length checking
with -fcheck=bounds for the case at hand.  It also improves checking of
character function declarations and references slightly, using the diagnostics
of NAG as a guidance.

Some testcases popped up during regtesting, suggesting that one needs to be
careful not to generate too many false positives, so I decided to not spend
to much time on the FIXME's therein.  (Character length might be expressions
in an explicit interface and the actual declaration, where we don't have a
reliable way to compare.)

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

Thanks,
Harald

From d09473af7e25c81bad95ff6c66c89e2d184147e6 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Sun, 17 Nov 2024 23:04:58 +0100
Subject: [PATCH] Fortran: add bounds-checking for ALLOCATE of CHARACTER with
 type-spec [PR53357]

Fix a rejects-(potentially)-valid code for ALLOCATE of CHARACTER with
type-spec, and implement a string-length check for -fcheck=bounds.
Implement more detailed errors or warnings when character function
declarations and references do not match.

	PR fortran/53357

gcc/fortran/ChangeLog:

	* dependency.cc (gfc_dep_compare_expr): Return correct result if
	relationship of expressions could not be determined.
	* interface.cc (gfc_check_result_characteristics): Implement error
	messages if character function declations and references do not
	agree, else emit warning in cases where a mismatch is suspected.
	* trans-stmt.cc (gfc_trans_allocate): Implement a string length
	check for -fcheck=bounds.

gcc/testsuite/ChangeLog:

	* gfortran.dg/auto_char_len_4.f90: Adjust patterns.
	* gfortran.dg/typebound_override_1.f90: Likewise.
	* gfortran.dg/bounds_check_strlen_10.f90: New test.
---
 gcc/fortran/dependency.cc |  2 +-
 gcc/fortran/interface.cc  | 27 ---
 gcc/fortran/trans-stmt.cc | 11 
 gcc/testsuite/gfortran.dg/auto_char_len_4.f90 | 25 -
 .../gfortran.dg/bounds_check_strlen_10.f90| 21 +++
 .../gfortran.dg/typebound_override_1.f90  |  4 +--
 6 files changed, 77 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_strlen_10.f90

diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index 2d3db9541bb..1fd65bbadca 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -474,7 +474,7 @@ gfc_dep_compare_expr (gfc_expr *e1, gfc_expr *e2)
   }

   if (e1->expr_type != e2->expr_type)
-return -3;
+return -2;

   switch (e1->expr_type)
 {
diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index 61c506bfdb5..176c7d4a8ed 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -1692,9 +1692,30 @@ gfc_check_result_characteristics (gfc_symbol *s1, gfc_symbol *s2,
 	  return false;

 	case -2:
-	  /* FIXME: Implement a warning for this case.
-	  snprintf (errmsg, err_len, "Possible character length mismatch "
-			"in function result");*/
+	  if (r1->ts.u.cl->length->expr_type == EXPR_CONSTANT)
+		{
+		  snprintf (errmsg, err_len,
+			"Function declared with a non-constant character "
+			"length referenced with a constant length");
+		  return false;
+		}
+	  else if (r2->ts.u.cl->length->expr_type == EXPR_CONSTANT)
+		{
+		  snprintf (errmsg, err_len,
+			"Function declared with a constant character "
+			"length referenced with a non-constant length");
+		  return false;
+		}
+	  /* Warn if length expression types are different, except for
+		  possibly false positives where complex expressions might have
+		  been used.  */
+	  else if ((r1->ts.u.cl->length->expr_type
+			!= r2->ts.u.cl->length->expr_type)
+		   && (r1->ts.u.cl->length->expr_type != EXPR_OP
+			   || r2->ts.u.cl->length->expr_type != EXPR_OP))
+		gfc_warning (0, "Possible character length mismatch in "
+			 "function result between %L and %L",
+			 &r1->declared_at, &r2->declared_at);
 	  break;

 	case 0:
diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
index 520ab505659..a409c25b899 100644
--- a/gcc/fortran/trans-stmt.cc
+++ b/gcc/fortran/trans-stmt.cc
@@ -6393,6 +6393,7 @@ gfc_trans_allocate (gfc_code * code, gfc_omp_namelist *omp_allocate)
   gfc_symtree *newsym = NULL;
   symbol_attribute caf_attr;
   gfc_actual_arglist *param_list;
+  tree ts_string_length = NULL_TREE;

   if (!code->ext.alloc.list)
 return NULL_TREE;
@@ -6741,6 +6742,7 @@ gfc_trans_allocate (gfc_code * code, gfc_omp_namelist *omp_allocate)
 	  gfc_init_se (&se_sz, NULL);
 	  gfc_conv_expr (&se_sz, sz);
 	  gfc_free_expr (sz);
+	  ts_string_length = fold_convert (gfc_charlen_type_node, se_sz.expr);
 	  tmp = gfc_get_char_type (code->ext.alloc.ts.kind);
 	  tmp = TYPE_SIZE_UNIT (tmp);
 	  tmp = fold_convert (TREE_TYPE (se_