[PATCH] PR libfortran/103634 - Runtime crash with PACK on zero-sized arrays

2021-12-09 Thread Harald Anlauf via Fortran
Dear all,

I had thought that we had fixed this in the past (see PR31001),
but it did fail for me with all gcc versions I have tried (7-12)
for a slightly more elaborate case as in the old testcase.

The loop in pack_internal did try to access the first element of
the array argument to PACK even if one (or more) extents were zero.
This is not good.

Solution: check the extents and return early.  (We already do a
related check for the vector argument if present).

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

As this segfaults on valid code at runtime: I am considering
backporting this, if there are no objections.

Thanks,
Harald

From dfa1e1ac5d8e43f1ca8f13b64330825581174f36 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Thu, 9 Dec 2021 20:55:08 +0100
Subject: [PATCH] Fortran: PACK intrinsic should not try to read from
 zero-sized array

libgfortran/ChangeLog:

	PR libfortran/103634
	* intrinsics/pack_generic.c (pack_internal): Handle case when the
	array argument of PACK has one extent of size zero to avoid
	invalid reads.

gcc/testsuite/ChangeLog:

	PR libfortran/103634
	* gfortran.dg/zero_sized_13.f90: New test.
---
 gcc/testsuite/gfortran.dg/zero_sized_13.f90 | 20 
 libgfortran/intrinsics/pack_generic.c   |  4 
 2 files changed, 24 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/zero_sized_13.f90

diff --git a/gcc/testsuite/gfortran.dg/zero_sized_13.f90 b/gcc/testsuite/gfortran.dg/zero_sized_13.f90
new file mode 100644
index 000..5620514334c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/zero_sized_13.f90
@@ -0,0 +1,20 @@
+! { dg-do run }
+! PR libfortran/103634 - Runtime crash with PACK on zero-sized arrays
+
+program p
+  implicit none
+  type t
+ real :: r(24) = -99.
+  end type
+  type(t), allocatable :: new(:), old(:)
+  logical, allocatable :: mask(:)
+  integer  :: n, m
+! m = 1! works
+  m = 0! failed with SIGSEGV in pack_internal
+  allocate (old(m), mask(m))
+  mask(:) = .false.
+  n = count (mask)
+  allocate (new(n))
+  new(:) = pack (old, mask)
+  print *, size (new)
+end
diff --git a/libgfortran/intrinsics/pack_generic.c b/libgfortran/intrinsics/pack_generic.c
index cad2fbbfbcd..f629e0e8469 100644
--- a/libgfortran/intrinsics/pack_generic.c
+++ b/libgfortran/intrinsics/pack_generic.c
@@ -126,6 +126,10 @@ pack_internal (gfc_array_char *ret, const gfc_array_char *array,
   if (mstride[0] == 0)
 mstride[0] = mask_kind;

+  for (n = 0; n < dim; n++)
+if (extent[n] == 0)
+  return;
+
   if (ret->base_addr == NULL || unlikely (compile_options.bounds_check))
 {
   /* Count the elements, either for allocating memory or
--
2.26.2



Re: [PATCH] PR libfortran/103634 - Runtime crash with PACK on zero-sized arrays

2021-12-09 Thread Mikael Morin

Hello,

On 09/12/2021 21:05, Harald Anlauf via Fortran wrote:

Dear all,

I had thought that we had fixed this in the past (see PR31001),
but it did fail for me with all gcc versions I have tried (7-12)
for a slightly more elaborate case as in the old testcase.

The loop in pack_internal did try to access the first element of
the array argument to PACK even if one (or more) extents were zero.
This is not good.

Solution: check the extents and return early.  (We already do a
related check for the vector argument if present).


If there is a vector argument, aren’t we supposed to copy it to the result ?
There is something else to pay attention for, the early return should 
come at least after the return array bounds have been set.  In the 
testcase an array with the correct bounds has been allocated beforehand 
to hold the return value, but it’s not always the case.


For what it’s worth, the non-generic variant in pack.m4 (or in 
pack_{i,f,c}{1,2,4,8,10,16}.c) has a zero extent check and it clears the 
source ptr in that case, which makes it setup the return array and then 
jump to the vector copy at the end of the function.


[PATCH, v2] PR fortran/103418 - random_number() does not accept pointer, intent(in) array argument

2021-12-09 Thread Harald Anlauf via Fortran

Hi Mikael,

Am 08.12.21 um 10:32 schrieb Mikael Morin:

On 07/12/2021 21:46, Harald Anlauf wrote:

Hi Mikael,

Am 07.12.21 um 21:17 schrieb Mikael Morin:

The existing code looks dubious to me (or at least difficult to
understand), and your patch doesn’t make that any better.
I would rather try to remove the whole block, and fix the fallout on
move_alloc by adding calls to gfc_check_vardef_context in
gfc_check_move_alloc.
Can you try that instead?


I hadn't thought that far but will think about a possibly better
solution.


Hello,

I thought about it some more over night, and it is probably a poor
suggestion to restrict the check to move_alloc only.  The existing code
was added for move_alloc, but it has a broader scope.  Still,
gfc_check_vardef_context has the correct checks and is the one to be used.


I have played a little, and it took some time to understand the fallout.
Your suggestion to rely on gfc_check_vardef_context actually helped to
uncover another bug: a bad check for CLASS pointer.

See attached for an updated patch and the extended testcase.

Regtested again.  OK now?

Thanks,
Harald

From dec60c90d47211d55048e7034e95f3e6fb10a2d4 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Thu, 9 Dec 2021 22:57:13 +0100
Subject: [PATCH] Fortran: fix check for pointer dummy arguments with
 INTENT(IN)

gcc/fortran/ChangeLog:

	PR fortran/103418
	* check.c (variable_check): Replace previous check of procedure
	dummy arguments with INTENT(IN) attribute when passed to intrinsic
	procedures by gfc_check_vardef_context.
	* expr.c (gfc_check_vardef_context): Correct check of INTENT(IN)
	dummy arguments for the case of sub-components of a CLASS pointer.

gcc/testsuite/ChangeLog:

	PR fortran/103418
	* gfortran.dg/move_alloc_8.f90: Adjust error messages.
	* gfortran.dg/pointer_intent_9.f90: New test.
---
 gcc/fortran/check.c   | 32 --
 gcc/fortran/expr.c|  9 +++--
 gcc/testsuite/gfortran.dg/move_alloc_8.f90|  4 +--
 .../gfortran.dg/pointer_intent_9.f90  | 33 +++
 4 files changed, 47 insertions(+), 31 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pointer_intent_9.f90

diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index ee3a51ee253..3934336df2e 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1011,33 +1011,13 @@ variable_check (gfc_expr *e, int n, bool allow_proc)
   if (e->expr_type == EXPR_VARIABLE
   && e->symtree->n.sym->attr.intent == INTENT_IN
   && (gfc_current_intrinsic_arg[n]->intent == INTENT_OUT
-	  || gfc_current_intrinsic_arg[n]->intent == INTENT_INOUT))
+	  || gfc_current_intrinsic_arg[n]->intent == INTENT_INOUT)
+  && !gfc_check_vardef_context (e, false, true, false, NULL))
 {
-  gfc_ref *ref;
-  bool pointer = e->symtree->n.sym->ts.type == BT_CLASS
-		 && CLASS_DATA (e->symtree->n.sym)
-		 ? CLASS_DATA (e->symtree->n.sym)->attr.class_pointer
-		 : e->symtree->n.sym->attr.pointer;
-
-  for (ref = e->ref; ref; ref = ref->next)
-	{
-	  if (pointer && ref->type == REF_COMPONENT)
-	break;
-	  if (ref->type == REF_COMPONENT
-	  && ((ref->u.c.component->ts.type == BT_CLASS
-		   && CLASS_DATA (ref->u.c.component)->attr.class_pointer)
-		  || (ref->u.c.component->ts.type != BT_CLASS
-		  && ref->u.c.component->attr.pointer)))
-	break;
-	}
-
-  if (!ref)
-	{
-	  gfc_error ("%qs argument of %qs intrinsic at %L cannot be "
-		 "INTENT(IN)", gfc_current_intrinsic_arg[n]->name,
-		 gfc_current_intrinsic, &e->where);
-	  return false;
-	}
+  gfc_error ("%qs argument of %qs intrinsic at %L cannot be INTENT(IN)",
+		 gfc_current_intrinsic_arg[n]->name,
+		 gfc_current_intrinsic, &e->where);
+  return false;
 }

   if (e->expr_type == EXPR_VARIABLE
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 87089321a3b..b874607db1d 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -6254,10 +6254,13 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, bool alloc_obj,
 {
   if (ptr_component && ref->type == REF_COMPONENT)
 	check_intentin = false;
-  if (ref->type == REF_COMPONENT && ref->u.c.component->attr.pointer)
+  if (ref->type == REF_COMPONENT)
 	{
-	  ptr_component = true;
-	  if (!pointer)
+	  gfc_component *comp = ref->u.c.component;
+	  ptr_component = (comp->ts.type == BT_CLASS && comp->attr.class_ok)
+			? CLASS_DATA (comp)->attr.class_pointer
+			: comp->attr.pointer;
+	  if (ptr_component && !pointer)
 	check_intentin = false;
 	}
   if (ref->type == REF_INQUIRY
diff --git a/gcc/testsuite/gfortran.dg/move_alloc_8.f90 b/gcc/testsuite/gfortran.dg/move_alloc_8.f90
index f624b703cc9..d968ea0e5cd 100644
--- a/gcc/testsuite/gfortran.dg/move_alloc_8.f90
+++ b/gcc/testsuite/gfortran.dg/move_alloc_8.f90
@@ -60,7 +60,7 @@ subroutine test2 (x, px)
   integer, allocatable :: a
   type(t2), pointer :: ta

-  call move_alloc (px, ta)  ! { dg-error "cannot be INTEN