Re: [PATCH 8/8] OpenMP: Fortran "!$omp declare mapper" support

2023-09-18 Thread Julian Brown
On Thu, 14 Sep 2023 17:13:02 +0200
Bernhard Reutner-Fischer via Gcc-patches 
wrote:

> On Tue, 5 Sep 2023 12:28:28 -0700
> Julian Brown  wrote:
> 
> > +  static bool
> > +  equal (const omp_name_type &a,
> > +const omp_name_type &b)
> > +  {
> > +if (a.name == NULL_TREE && b.name == NULL_TREE)
> > +  return a.type == b.type;  
> 
> I'm curious if (and why) the type comparison above is safe and does
> not use gfc_compare_types () ?
> 
> thanks,

Probably ignorance on my part! It works for (derived, class) types which
are canonicalized to exactly the same gfc_typespec, but you're likely
right that a more Fortran-ish notion of type equality should be used
here instead when comparing "declare mapper"s.

Or maybe using gfc_compare_types would smush too many types together
into one? E.g. if b.type is an extension of a.type, do we want those
types to be able to have separate mappers?

I'll have a look at addressing this when it's time to reroll these
patches.

Thanks,

Julian


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

2023-09-18 Thread Harald Anlauf via Fortran
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

From 14b12cba8b7def5e07a3cac5efd2be8ab49d8133 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
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 000..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]

2023-09-18 Thread Jerry D via Fortran

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