[PATCH] fortran: fix checking of CHARACTER lengths in array constructors [PR70231]

[PATCH] fortran: fix checking of CHARACTER lengths in array constructors [PR70231]

[Harald Anlauf]

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

Dear all,

as correctly analyzed by Jerry, the code for checking the consistency
of character lengths in array constructors did not properly initialize
the auxiliary variable used in "bounds checking".  The attached patch
resolves this by initializing this auxiliary variable with a length
obtained by scanning the constructor.

Interestingly, the failure depended on optimization level and was
apparent only at -O0, and could exhibit both false-positives and
false-negatives.

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

Thanks,
Harald


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

From 14b12cba8b7def5e07a3cac5efd2be8ab49d8133 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Mon, 18 Sep 2023 22:11:40 +0200
Subject: [PATCH] fortran: fix checking of CHARACTER lengths in array
 constructors [PR70231]

gcc/fortran/ChangeLog:

	PR fortran/70231
	* trans-array.cc (trans_array_constructor): In absence of a typespec,
	use string length determined by get_array_ctor_strlen() to reasonably
	initialize auxiliary variable for bounds-checking.

gcc/testsuite/ChangeLog:

	PR fortran/70231
	* gfortran.dg/bounds_check_fail_7.f90: New test.
---
 gcc/fortran/trans-array.cc                    | 17 ++++++++++++++++
 .../gfortran.dg/bounds_check_fail_7.f90       | 20 +++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 1640587cd71..e0fc8ebc46b 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -2852,6 +2852,23 @@ trans_array_constructor (gfc_ss * ss, locus * where)
 	  const_string = get_array_ctor_strlen (&outer_loop->pre, c,
 						&ss_info->string_length);
 	  force_new_cl = true;
+
+	  /* Initialize "len" with string length for bounds checking.  */
+	  if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)
+	      && !typespec_chararray_ctor
+	      && ss_info->string_length)
+	    {
+	      gfc_se length_se;
+
+	      gfc_init_se (&length_se, NULL);
+	      gfc_add_modify (&length_se.pre, first_len_val,
+			      fold_convert (TREE_TYPE (first_len_val),
+					    ss_info->string_length));
+	      ss_info->string_length = gfc_evaluate_now (ss_info->string_length,
+							 &length_se.pre);
+	      gfc_add_block_to_block (&outer_loop->pre, &length_se.pre);
+	      gfc_add_block_to_block (&outer_loop->post, &length_se.post);
+	    }
 	}

       /* Complex character array constructors should have been taken care of
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90
new file mode 100644
index 00000000000..6a8dafc27a8
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90
@@ -0,0 +1,20 @@
+! { dg-do run }
+! { dg-additional-options "-fcheck=bounds -g" }
+! { dg-output "At line 18 .*" }
+! { dg-shouldfail "Different CHARACTER lengths (32/0) in array constructor" }
+!
+! PR fortran/70231 - CHARACTER lengths in array constructors
+
+program p
+  implicit none
+  integer, parameter  :: char_len = 32
+  integer             :: l = 0
+  character(char_len) :: ch = "a"
+  character(char_len), allocatable :: ch_array(:), res1(:), res2(:)
+
+  allocate(ch_array(0))
+  res1 = [ ch_array, ch ]               ! was false positive
+  print *, res1
+  res2 = [[ch_array, ch(1:l)], ch(1:l)] ! was false negative on x86
+  print *, res2
+end
--
2.35.3

Re: [PATCH] fortran: fix checking of CHARACTER lengths in array constructors [PR70231]

[Jerry D]

On 9/18/23 1:27 PM, Harald Anlauf via Fortran wrote:
> Dear all,
> 
> as correctly analyzed by Jerry, the code for checking the consistency
> of character lengths in array constructors did not properly initialize
> the auxiliary variable used in "bounds checking".  The attached patch
> resolves this by initializing this auxiliary variable with a length
> obtained by scanning the constructor.
> 
> Interestingly, the failure depended on optimization level and was
> apparent only at -O0, and could exhibit both false-positives and
> false-negatives.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> 
> Thanks,
> Harald
> 

Wow 2016 when I looked at that one. The patch looks reasonable to me.

OK for mainline.

Jerry