public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fortran: reject procedures and procedure pointers as output item [PR107074]
@ 2022-10-04 19:27 Harald Anlauf
  2022-10-05 10:34 ` Mikael Morin
  0 siblings, 1 reply; 6+ messages in thread
From: Harald Anlauf @ 2022-10-04 19:27 UTC (permalink / raw)
  To: fortran, gcc-patches

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

Dear all,

when looking at output item lists we didn't catch procedures
and procedure pointers and ran into a gfc_internal_error().
Such items are not allowed by the Fortran standard, e.g. for
procedure pointers there is

C1233 (R1217) An expression that is an output-item shall not
              have a value that is a procedure pointer.

Attached patch generates an error instead.

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

Thanks,
Harald


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

From 3b15fe83830c1e75339114e0241e9d2158393017 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Tue, 4 Oct 2022 21:19:21 +0200
Subject: [PATCH] Fortran: reject procedures and procedure pointers as output
 item [PR107074]

gcc/fortran/ChangeLog:

	PR fortran/107074
	* trans-io.cc (transfer_expr): A procedure or a procedure pointer
	cannot be output items.

gcc/testsuite/ChangeLog:

	PR fortran/107074
	* gfortran.dg/pr107074.f90: New test.
---
 gcc/fortran/trans-io.cc                | 14 ++++++++++++++
 gcc/testsuite/gfortran.dg/pr107074.f90 | 11 +++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/pr107074.f90

diff --git a/gcc/fortran/trans-io.cc b/gcc/fortran/trans-io.cc
index 9f86815388c..c4e1537eed6 100644
--- a/gcc/fortran/trans-io.cc
+++ b/gcc/fortran/trans-io.cc
@@ -2430,6 +2430,20 @@ transfer_expr (gfc_se * se, gfc_typespec * ts, tree addr_expr,

       break;

+    case BT_PROCEDURE:
+      if (code->expr1
+	  && code->expr1->symtree
+	  && code->expr1->symtree->n.sym)
+	{
+	  if (code->expr1->symtree->n.sym->attr.proc_pointer)
+	    gfc_error ("Procedure pointer at %C cannot be an output item");
+	  else
+	    gfc_error ("Procedure at %C cannot be an output item");
+	  return;
+	}
+      /* If a PROCEDURE item gets through to here, fall through and ICE.  */
+      gcc_fallthrough ();
+
     case_bt_struct:
     case BT_CLASS:
       if (gfc_bt_struct (ts->type) || ts->type == BT_CLASS)
diff --git a/gcc/testsuite/gfortran.dg/pr107074.f90 b/gcc/testsuite/gfortran.dg/pr107074.f90
new file mode 100644
index 00000000000..a09088c2e9d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr107074.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR fortran/107074 - ICE: Bad IO basetype (8)
+! Contributed by G.Steinmetz
+
+program p
+  implicit none
+  integer, external        :: a
+  procedure(real), pointer :: b
+  print *, merge (a, a, .true.) ! { dg-error "Procedure" }
+  print *, merge (b, b, .true.) ! { dg-error "Procedure pointer" }
+end
--
2.35.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fortran: reject procedures and procedure pointers as output item [PR107074]
  2022-10-04 19:27 [PATCH] Fortran: reject procedures and procedure pointers as output item [PR107074] Harald Anlauf
@ 2022-10-05 10:34 ` Mikael Morin
  2022-10-05 20:40   ` [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074] Harald Anlauf
  0 siblings, 1 reply; 6+ messages in thread
From: Mikael Morin @ 2022-10-05 10:34 UTC (permalink / raw)
  To: Harald Anlauf, fortran, gcc-patches

Hello

Le 04/10/2022 à 21:27, Harald Anlauf via Fortran a écrit :
> Dear all,
> 
> when looking at output item lists we didn't catch procedures
> and procedure pointers and ran into a gfc_internal_error().
> Such items are not allowed by the Fortran standard, e.g. for
> procedure pointers there is
> 
> C1233 (R1217) An expression that is an output-item shall not
>                have a value that is a procedure pointer.
> 
> Attached patch generates an error instead.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> 
Please move the check to resolve_transfer in resolve.cc.

Strangely, the patch doesn't seem to fix the problem on the testcase 
here.  There is an outer parenthese expression preventing the condition 
you added from triggering.  Can you double check?

If we take the standard to the letter, only output items are forbidden, 
so a check is missing for writing context.  I don't know how it can work 
for input items though, so maybe not worth it.  In any case, the error 
shouldn't mention output items in reading context.

Here is a variant of the testcase with procedure pointer components, 
that fails differently but can probably be caught as well.

program p
   implicit none
   type :: t
     procedure(f), pointer, nopass :: b
   end type t
   type(t) :: a

   interface
     real function f()
     end function f
   end interface

   print *, merge (a%b, a%b, .true.)
end



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, v2] Fortran: reject procedures and procedure pointers as IO  element [PR107074]
  2022-10-05 10:34 ` Mikael Morin
@ 2022-10-05 20:40   ` Harald Anlauf
  2022-10-06  8:37     ` Mikael Morin
  0 siblings, 1 reply; 6+ messages in thread
From: Harald Anlauf @ 2022-10-05 20:40 UTC (permalink / raw)
  To: Mikael Morin; +Cc: fortran, gcc-patches

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

Hi Mikael,

> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
> Von: "Mikael Morin" <morin-mikael@orange.fr>
> Please move the check to resolve_transfer in resolve.cc.

I have done this, see attached updated patch.

Regtests cleanly on x86_64-pc-linux-gnu.

> Strangely, the patch doesn't seem to fix the problem on the testcase
> here.  There is an outer parenthese expression preventing the condition
> you added from triggering.  Can you double check?

You are right: I had a one-liner in my worktree from PR105371 that
fixes an issue with gfc_simplify_merge and that seems to help here.
It is now included.

> If we take the standard to the letter, only output items are forbidden,
> so a check is missing for writing context.  I don't know how it can work
> for input items though, so maybe not worth it.  In any case, the error
> shouldn't mention output items in reading context.
>
> Here is a variant of the testcase with procedure pointer components,
> that fails differently but can probably be caught as well.
>
> program p
>    implicit none
>    type :: t
>      procedure(f), pointer, nopass :: b
>    end type t
>    type(t) :: a
>
>    interface
>      real function f()
>      end function f
>    end interface
>
>    print *, merge (a%b, a%b, .true.)
> end

I hadn't thought about this, and found a solution that also fixes this
one.  Great example!  This is now an additional test.

OK for mainline?

And thanks for your comments!

Harald


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

From 70cba7da18023282546b9a5d80e976fc3744d732 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Wed, 5 Oct 2022 22:25:14 +0200
Subject: [PATCH] Fortran: reject procedures and procedure pointers as IO
 element [PR107074]

gcc/fortran/ChangeLog:

	PR fortran/107074
	* resolve.cc (resolve_transfer): A procedure, type-bound procedure
	or a procedure pointer cannot be an element of an IO list.
	* simplify.cc (gfc_simplify_merge): Do not try to reset array lower
	bound for scalars.

gcc/testsuite/ChangeLog:

	PR fortran/107074
	* gfortran.dg/pr107074.f90: New test.
	* gfortran.dg/pr107074b.f90: New test.
---
 gcc/fortran/resolve.cc                  | 31 +++++++++++++++++++++++++
 gcc/fortran/simplify.cc                 |  3 ++-
 gcc/testsuite/gfortran.dg/pr107074.f90  | 11 +++++++++
 gcc/testsuite/gfortran.dg/pr107074b.f90 | 18 ++++++++++++++
 4 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr107074.f90
 create mode 100644 gcc/testsuite/gfortran.dg/pr107074b.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index d133bc2d034..d9d101775f6 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -10137,6 +10137,37 @@ resolve_transfer (gfc_code *code)
 		 "an assumed-size array", &code->loc);
       return;
     }
+
+  /* Check for procedures and procedure pointers.  Fortran 2018 has:
+
+     C1233 (R1217) An expression that is an output-item shall not have a
+     value that is a procedure pointer.
+
+     There does not appear any reason to allow procedure pointers for
+     input, so we disallow them generally, and we reject procedures.  */
+
+  if (exp->expr_type == EXPR_VARIABLE)
+    {
+      /* Check for type-bound procedures.  */
+      for (ref = exp->ref; ref; ref = ref->next)
+	if (ref->type == REF_COMPONENT
+	    && ref->u.c.component->attr.flavor == FL_PROCEDURE)
+	  break;
+
+      /* Procedure or procedure pointer?  */
+      if (exp->ts.type == BT_PROCEDURE
+	  || (ref && ref->u.c.component->attr.flavor == FL_PROCEDURE))
+	{
+	  if (exp->symtree->n.sym->attr.proc_pointer
+	      || (ref && ref->u.c.component->attr.proc_pointer))
+	    gfc_error ("Data transfer element at %L cannot be a procedure "
+		       "pointer", &code->loc);
+	  else
+	    gfc_error ("Data transfer element at %L cannot be a procedure",
+		       &code->loc);
+	  return;
+	}
+    }
 }


diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index 6ac92cf9db8..f0482d349af 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -4915,7 +4915,8 @@ gfc_simplify_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask)
     {
       result = gfc_copy_expr (mask->value.logical ? tsource : fsource);
       /* Parenthesis is needed to get lower bounds of 1.  */
-      result = gfc_get_parentheses (result);
+      if (result->rank)
+	result = gfc_get_parentheses (result);
       gfc_simplify_expr (result, 1);
       return result;
     }
diff --git a/gcc/testsuite/gfortran.dg/pr107074.f90 b/gcc/testsuite/gfortran.dg/pr107074.f90
new file mode 100644
index 00000000000..1363c285912
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr107074.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR fortran/107074 - ICE: Bad IO basetype (8)
+! Contributed by G.Steinmetz
+
+program p
+  implicit none
+  integer, external        :: a
+  procedure(real), pointer :: b
+  print *, merge (a, a, .true.) ! { dg-error "procedure" }
+  print *, merge (b, b, .true.) ! { dg-error "procedure pointer" }
+end
diff --git a/gcc/testsuite/gfortran.dg/pr107074b.f90 b/gcc/testsuite/gfortran.dg/pr107074b.f90
new file mode 100644
index 00000000000..98c3fc0b90a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr107074b.f90
@@ -0,0 +1,18 @@
+! { dg-do compile }
+! Additional test for PR fortran/107074
+! Contributed by M.Morin
+
+program p
+  implicit none
+  type :: t
+    procedure(f), pointer, nopass :: b
+  end type t
+  type(t) :: a
+
+  interface
+    real function f()
+    end function f
+  end interface
+
+  print *, merge (a%b, a%b, .true.) ! { dg-error "procedure pointer" }
+end
--
2.35.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074]
  2022-10-05 20:40   ` [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074] Harald Anlauf
@ 2022-10-06  8:37     ` Mikael Morin
  2022-10-06 20:32       ` Mikael Morin
  0 siblings, 1 reply; 6+ messages in thread
From: Mikael Morin @ 2022-10-06  8:37 UTC (permalink / raw)
  To: Harald Anlauf; +Cc: fortran, gcc-patches

Le 05/10/2022 à 22:40, Harald Anlauf a écrit :
> Hi Mikael,
> 
>> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
>> Von: "Mikael Morin" <morin-mikael@orange.fr>
>> Please move the check to resolve_transfer in resolve.cc.
> 
> I have done this, see attached updated patch.
> 
> Regtests cleanly on x86_64-pc-linux-gnu.
> 
>> Strangely, the patch doesn't seem to fix the problem on the testcase
>> here.  There is an outer parenthese expression preventing the condition
>> you added from triggering.  Can you double check?
> 
> You are right: I had a one-liner in my worktree from PR105371 that
> fixes an issue with gfc_simplify_merge and that seems to help here.
> It is now included.
> 
The rest looks good, but I'm not sure about your one-liner.
I will try to come with a real test later, but in principle, if you have 
a call to FOO(MERGE(A,A,.TRUE.)) you can't simplify it to FOO(A) as 
writes to the argument in FOO should not overwrite the content of A. 
The dummy should be associated with a temporary value, not to A.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074]
  2022-10-06  8:37     ` Mikael Morin
@ 2022-10-06 20:32       ` Mikael Morin
  2022-10-06 21:07         ` Aw: " Harald Anlauf
  0 siblings, 1 reply; 6+ messages in thread
From: Mikael Morin @ 2022-10-06 20:32 UTC (permalink / raw)
  To: Harald Anlauf; +Cc: gcc-patches, fortran

Le 06/10/2022 à 10:37, Mikael Morin a écrit :
> Le 05/10/2022 à 22:40, Harald Anlauf a écrit :
>> Hi Mikael,
>>
>>> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
>>> Von: "Mikael Morin" <morin-mikael@orange.fr>
>>> Please move the check to resolve_transfer in resolve.cc.
>>
>> I have done this, see attached updated patch.
>>
>> Regtests cleanly on x86_64-pc-linux-gnu.
>>
>>> Strangely, the patch doesn't seem to fix the problem on the testcase
>>> here.  There is an outer parenthese expression preventing the condition
>>> you added from triggering.  Can you double check?
>>
>> You are right: I had a one-liner in my worktree from PR105371 that
>> fixes an issue with gfc_simplify_merge and that seems to help here.
>> It is now included.
>>
> The rest looks good, but I'm not sure about your one-liner.
> I will try to come with a real test later, but in principle, if you have 
> a call to FOO(MERGE(A,A,.TRUE.)) you can't simplify it to FOO(A) as 
> writes to the argument in FOO should not overwrite the content of A. The 
> dummy should be associated with a temporary value, not to A.
> 
Here is a test that your patch breaks.
Admittedly it's rejected if A has the INTENT(INOUT) attribute, but 
without it, I think it's allowed.

program p
   integer :: b
   b = 1
   call foo(merge(b,b,.true.))
   if (b /= 1) stop 1
contains
   subroutine foo(a)
     integer :: a
     if (a == 1) a = 42
   end subroutine foo
end program p


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Aw: Re: [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074]
  2022-10-06 20:32       ` Mikael Morin
@ 2022-10-06 21:07         ` Harald Anlauf
  0 siblings, 0 replies; 6+ messages in thread
From: Harald Anlauf @ 2022-10-06 21:07 UTC (permalink / raw)
  To: Mikael Morin; +Cc: gcc-patches, fortran

Hi Mikael,

I definitely agree that we need a temporary for the result of
MERGE(a,a,.true.), I just haven't found out how to do that.

The reason for the bad one-liner was that in gfc_simplify_merge

	result = gfc_get_parentheses (result);

actually does have issues, in that the subsequent

      gfc_simplify_expr (result, 1);

seems to fail in interesting cases (as in PR105371).

So that is something to look into...

Cheers,
Harald

> Gesendet: Donnerstag, 06. Oktober 2022 um 22:32 Uhr
> Von: "Mikael Morin" <morin-mikael@orange.fr>
> An: "Harald Anlauf" <anlauf@gmx.de>
> Cc: "gcc-patches" <gcc-patches@gcc.gnu.org>, "fortran" <fortran@gcc.gnu.org>
> Betreff: Re: [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074]
>
> Le 06/10/2022 à 10:37, Mikael Morin a écrit :
> > Le 05/10/2022 à 22:40, Harald Anlauf a écrit :
> >> Hi Mikael,
> >>
> >>> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
> >>> Von: "Mikael Morin" <morin-mikael@orange.fr>
> >>> Please move the check to resolve_transfer in resolve.cc.
> >>
> >> I have done this, see attached updated patch.
> >>
> >> Regtests cleanly on x86_64-pc-linux-gnu.
> >>
> >>> Strangely, the patch doesn't seem to fix the problem on the testcase
> >>> here.  There is an outer parenthese expression preventing the condition
> >>> you added from triggering.  Can you double check?
> >>
> >> You are right: I had a one-liner in my worktree from PR105371 that
> >> fixes an issue with gfc_simplify_merge and that seems to help here.
> >> It is now included.
> >>
> > The rest looks good, but I'm not sure about your one-liner.
> > I will try to come with a real test later, but in principle, if you have 
> > a call to FOO(MERGE(A,A,.TRUE.)) you can't simplify it to FOO(A) as 
> > writes to the argument in FOO should not overwrite the content of A. The 
> > dummy should be associated with a temporary value, not to A.
> > 
> Here is a test that your patch breaks.
> Admittedly it's rejected if A has the INTENT(INOUT) attribute, but 
> without it, I think it's allowed.
> 
> program p
>    integer :: b
>    b = 1
>    call foo(merge(b,b,.true.))
>    if (b /= 1) stop 1
> contains
>    subroutine foo(a)
>      integer :: a
>      if (a == 1) a = 42
>    end subroutine foo
> end program p
> 
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-06 21:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-04 19:27 [PATCH] Fortran: reject procedures and procedure pointers as output item [PR107074] Harald Anlauf
2022-10-05 10:34 ` Mikael Morin
2022-10-05 20:40   ` [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074] Harald Anlauf
2022-10-06  8:37     ` Mikael Morin
2022-10-06 20:32       ` Mikael Morin
2022-10-06 21:07         ` Aw: " Harald Anlauf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).