Bug 41139 - [4.5 Regression] a procedure pointer call as actual argument
Summary: [4.5 Regression] a procedure pointer call as actual argument
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P5 normal
Target Milestone: 4.5.0
Assignee: janus
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2009-08-21 13:22 UTC by janus
Modified: 2009-08-25 14:29 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.4.0
Known to fail: 4.5.0
Last reconfirmed: 2009-08-22 09:19:15


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description janus 2009-08-21 13:22:40 UTC
PROGRAM test

 PROCEDURE(add), POINTER :: f

 ! Passing the function works
 print *,greater(4.,add(1.,2.))

 ! Passing the procedure pointer fails
 f => add
 print *,greater(4.,f(1.,2.))

CONTAINS

 REAL FUNCTION add(x,y)
   REAL, INTENT(in) :: x,y
   print *,"add:",x,y
   add = x+y
 END FUNCTION add

 LOGICAL FUNCTION greater(x,y)
   REAL, INTENT(in) :: x, y
   greater = (x > y)
 END FUNCTION greater

END PROGRAM test


This code produces the following output:

 add:   1.0000000       2.0000000    
 T
 add:   1.0000000       2.0000000    
Segmentation fault


Looking at the ouput of -fdump-tree-original, one can see a difference between the two calls to 'greater':

      D.1563 = add (&C.1561, &C.1562);
      D.1564 = greater (&C.1560, &D.1563);

      D.1570 = greater (&C.1567, f (&C.1568, &C.1569));

I.e. in the first case a temporary variable is inserted for the result of 'add', while in the second case this is not the case (which is the reason for the segfault).

gfortran 4.4 behaves correctly.

Reported by Barron Bichon.
Comment 1 Dominique d'Humieres 2009-08-21 13:35:26 UTC
Beware the forbidden recursive I/Os!-(the test hangs on Darwin, see pr30617). Otherwise, after using a temp for greater, I get a Bus error.

Comment 2 janus 2009-08-21 13:51:36 UTC
> Beware the forbidden recursive I/Os!

This is not the issue here. The following variation has no recursive I/O, but gives the same segfault:

PROGRAM test

 PROCEDURE(add), POINTER :: f
 logical :: g

 ! Passing the function works
 g=greater(4.,add(1.,2.))
 print *,g

 ! Passing the procedure pointer fails
 f => add
 g=greater(4.,f(1.,2.))
 print *,g

CONTAINS

 REAL FUNCTION add(x,y)
   REAL, INTENT(in) :: x,y
   print *,"add:",x,y
   add = x+y
 END FUNCTION add

 LOGICAL FUNCTION greater(x,y)
   REAL, INTENT(in) :: x, y
   greater = (x > y)
 END FUNCTION greater

END PROGRAM test
Comment 3 Dominique d'Humieres 2009-08-21 13:58:10 UTC
> > Beware the forbidden recursive I/Os!
>
> This is not the issue here. ...

Indeed I know! but 

(1) I had to make the change you have posted in comment #2 to run a test.

(2) The code in comment #0 is illegal and should not be used for the test suite.

Comment 4 janus 2009-08-21 14:00:52 UTC
Side note: If one constructs an analogous test case with PPCs, this does not have the missing-temporary problem. But it has a different one:

PROGRAM test

 type :: t
   PROCEDURE(add), POINTER, nopass :: f
 end type
 type(t) :: o
 logical :: g

 ! Passing the function works
 g=greater(4.,add(1.,2.))
 print *,g

 ! Passing the procedure pointer fails
 o%f => add
 g=greater(4.,o%f(1.,2.))
 print *,g

CONTAINS

 REAL FUNCTION add(x,y)
   REAL, INTENT(in) :: x,y
   add = x+y
 END FUNCTION add

 LOGICAL FUNCTION greater(x,y)
   REAL, INTENT(in) :: x, y
   print *,"greater:",x,y
   greater = (x > y)
 END FUNCTION greater

END PROGRAM test


The output is:

 greater:   4.0000000       3.0000000    
 T
 greater:   4.0000000     -1.49897304E-22
 T

The dump shows the reason for this (a double '&'):

    D.1571 = o.f;
    D.1572 = D.1571 (&C.1569, &C.1570);
    g = (logical(kind=4)) greater (&C.1568, &&D.1572);
Comment 5 janus 2009-08-21 14:03:40 UTC
> (1) I had to make the change you have posted in comment #2 to run a test.
> 
> (2) The code in comment #0 is illegal and should not be used for the test
> suite.

Of course. Thanks for pointing this out :)
Comment 6 janus 2009-08-21 14:50:38 UTC
This simple patch fixes comment #2:

Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c	(revision 150987)
+++ gcc/fortran/trans-expr.c	(working copy)
@@ -2679,6 +2679,7 @@ gfc_conv_procedure_call (gfc_se * se, gf
 		}
 	      else if (e->expr_type == EXPR_FUNCTION
 		       && e->symtree->n.sym->result
+		       && e->symtree->n.sym->result != e->symtree->n.sym
 		       && e->symtree->n.sym->result->attr.proc_pointer)
 		{
 		  /* Functions returning procedure pointers.  */
Comment 7 janus 2009-08-21 15:11:03 UTC
(In reply to comment #4)
>     D.1571 = o.f;
>     D.1572 = D.1571 (&C.1569, &C.1570);
>     g = (logical(kind=4)) greater (&C.1568, &&D.1572);

Btw, it seems unnecessary to me that every PPC call generates a temporary (D.1571 is this case). This is fixed by the following patchlet:

Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c	(revision 150987)
+++ gcc/fortran/trans-expr.c	(working copy)
@@ -3512,8 +3513,7 @@ gfc_get_proc_ptr_comp (gfc_se *se, gfc_e
   e2 = gfc_copy_expr (e);
   e2->expr_type = EXPR_VARIABLE;
   gfc_conv_expr (&comp_se, e2);
-  comp_se.expr = build_fold_addr_expr_loc (input_location, comp_se.expr);
-  return gfc_evaluate_now (comp_se.expr, &se->pre);  
+  return build_fold_addr_expr_loc (input_location, comp_se.expr);
 }
 
 
Comment 8 Dominique d'Humieres 2009-08-21 17:08:14 UTC
With the patch in comment #7, compiling gcc/fortran/trans-expr.c fails with:

...
cc1: warnings being treated as errors
../../gcc-4.5-work/gcc/fortran/trans-expr.c: In function 'gfc_get_proc_ptr_comp':
../../gcc-4.5-work/gcc/fortran/trans-expr.c:3508:32: error: unused parameter 'se'
...
Comment 9 Tobias Burnus 2009-08-21 18:02:54 UTC
Working: 2009-07-10, r149458
Failing: 2009-07-17, r149734
Comment 10 Tobias Burnus 2009-08-21 18:03:53 UTC
(In reply to comment #9)
> Working: 2009-07-10, r149458
> Failing: 2009-07-17, r149734
Wrong PR - sorry, that should go to PR debug/40660
Comment 11 janus 2009-08-21 20:27:03 UTC
Here is another variant of the test case which fails at runtime:

PROGRAM test

 type :: t
   PROCEDURE(three), POINTER, nopass :: f
 end type
 type(t) :: o
 logical :: g

 o%f => three
 g=greater(4.,o%f())
 print *,g

CONTAINS

 REAL FUNCTION three()
   three = 3.
 END FUNCTION

 LOGICAL FUNCTION greater(x,y)
   REAL, INTENT(in) :: x, y
   print *,"greater:",x,y
   greater = (x > y)
 END FUNCTION greater

END PROGRAM test


The dump shows:

    g = (logical(kind=4)) greater (&C.1561, &o.f);
Comment 12 Dominique d'Humieres 2009-08-22 08:57:06 UTC
In order to avoid the error reported in comment #8, I have tested the following patches:

(1)

Index: gcc/fortran/trans-expr.c
===================================================================
--- ../_gcc_clean/gcc/fortran/trans-expr.c	2009-08-21 17:41:19.000000000 +0200
+++ gcc/fortran/trans-expr.c	2009-08-21 19:20:16.000000000 +0200
@@ -2679,6 +2679,7 @@ gfc_conv_procedure_call (gfc_se * se, gf
		}
	      else if (e->expr_type == EXPR_FUNCTION
		       && e->symtree->n.sym->result
+		       && e->symtree->n.sym->result != e->symtree->n.sym
		       && e->symtree->n.sym->result->attr.proc_pointer)
		{
		  /* Functions returning procedure pointers.  */
@@ -3504,7 +3505,7 @@ gfc_conv_statement_function (gfc_se * se
 /* Return the backend_decl for a procedure pointer component.  */
 
 tree
-gfc_get_proc_ptr_comp (gfc_se *se, gfc_expr *e)
+gfc_get_proc_ptr_comp (__attribute__((unused)) gfc_se *se, gfc_expr *e)
 {
   gfc_se comp_se;
   gfc_expr *e2;
@@ -3512,8 +3513,7 @@ gfc_get_proc_ptr_comp (gfc_se *se, gfc_e
   e2 = gfc_copy_expr (e);
   e2->expr_type = EXPR_VARIABLE;
   gfc_conv_expr (&comp_se, e2);
-  comp_se.expr = build_fold_addr_expr_loc (input_location, comp_se.expr);
-  return gfc_evaluate_now (comp_se.expr, &se->pre);  
+  return build_fold_addr_expr_loc (input_location, comp_se.expr);
 }

(2)

Index: gcc/fortran/trans-expr.c
===================================================================
--- ../_gcc_clean/gcc/fortran/trans-expr.c	2009-08-21 11:46:44.000000000 +0200
+++ gcc/fortran/trans-expr.c	2009-08-21 21:38:04.000000000 +0200
@@ -1508,7 +1508,7 @@ conv_function_val (gfc_se * se, gfc_symb
   tree tmp;
 
   if (gfc_is_proc_ptr_comp (expr, NULL))
-    tmp = gfc_get_proc_ptr_comp (se, expr);
+    tmp = gfc_get_proc_ptr_comp (expr);
   else if (sym->attr.dummy)
     {
       tmp = gfc_get_symbol_decl (sym);
@@ -2679,6 +2679,7 @@ gfc_conv_procedure_call (gfc_se * se, gf
		}
	      else if (e->expr_type == EXPR_FUNCTION
		       && e->symtree->n.sym->result
+		       && e->symtree->n.sym->result != e->symtree->n.sym
		       && e->symtree->n.sym->result->attr.proc_pointer)
		{
		  /* Functions returning procedure pointers.  */
@@ -3504,7 +3505,7 @@ gfc_conv_statement_function (gfc_se * se
 /* Return the backend_decl for a procedure pointer component.  */
 
 tree
-gfc_get_proc_ptr_comp (gfc_se *se, gfc_expr *e)
+gfc_get_proc_ptr_comp (gfc_expr *e)
 {
   gfc_se comp_se;
   gfc_expr *e2;
@@ -3512,8 +3513,7 @@ gfc_get_proc_ptr_comp (gfc_se *se, gfc_e
   e2 = gfc_copy_expr (e);
   e2->expr_type = EXPR_VARIABLE;
   gfc_conv_expr (&comp_se, e2);
-  comp_se.expr = build_fold_addr_expr_loc (input_location, comp_se.expr);
-  return gfc_evaluate_now (comp_se.expr, &se->pre);  
+  return build_fold_addr_expr_loc (input_location, comp_se.expr);
 }
 
 
Index: gcc/fortran/trans-tmt.h
===================================================================
--- ../_gcc_clean/gcc/fortran/trans-stmt.h	2009-05-10 12:01:34.000000000 +0200
+++ gcc/fortran/trans-stmt.h	2009-08-21 21:38:41.000000000 +0200
@@ -29,7 +29,7 @@ tree gfc_trans_code (gfc_code *);
 tree gfc_trans_assign (gfc_code *);
 tree gfc_trans_pointer_assign (gfc_code *);
 tree gfc_trans_init_assign (gfc_code *);
-tree gfc_get_proc_ptr_comp (gfc_se *, gfc_expr *);
+tree gfc_get_proc_ptr_comp (gfc_expr *);
 
 /* trans-stmt.c */
 tree gfc_trans_cycle (gfc_code *);

Both allow gfortran to be built, they fix the segmentation fault reported in comment #2, but not the wrong code for comments #4 and #11.  With the second patch, my tests passed and gfortran regtested fine (powerpc-apple-darwin9).
Comment 13 janus 2009-08-22 09:19:14 UTC
(In reply to comment #12)
> @@ -3512,8 +3513,7 @@ gfc_get_proc_ptr_comp (gfc_se *se, gfc_e
>    e2 = gfc_copy_expr (e);
>    e2->expr_type = EXPR_VARIABLE;
>    gfc_conv_expr (&comp_se, e2);
> -  comp_se.expr = build_fold_addr_expr_loc (input_location, comp_se.expr);
> -  return gfc_evaluate_now (comp_se.expr, &se->pre);  
> +  return build_fold_addr_expr_loc (input_location, comp_se.expr);
>  }

This seems to work in practice, but I think the dump looks not quite right for the PPC call. Dominique, can you post your dump for comment #4 (especially the call to o%f)?

Btw, I have a patch for the remaining trouble in comment #4 and #11.
Comment 14 Dominique d'Humieres 2009-08-22 09:27:00 UTC
> Dominique, can you post your dump for comment #4 (especially the call to o%f)?

Is this what you want?

  o.f = add;
  {
    real(kind=4) D.1533;
    static real(kind=4) C.1532 = 2.0e+0;
    static real(kind=4) C.1531 = 1.0e+0;
    static real(kind=4) C.1530 = 4.0e+0;

    D.1533 = f (&C.1531, &C.1532);
    g = (logical(kind=4)) greater (&C.1530, &&D.1533);
  }
Comment 15 janus 2009-08-22 09:43:27 UTC
(In reply to comment #14)
> Is this what you want?

Jep.

>     D.1533 = f (&C.1531, &C.1532);

In principle the 'f' here should be an 'o.f'. Maybe we can postpone this issue until the other stuff is fixed (will post a patch soon).

I think the gfc_evaluate_now part was Paul's invention originally. Perhaps he can give some advice.
Comment 16 janus 2009-08-23 21:15:24 UTC
(In reply to comment #15)
> >     D.1533 = f (&C.1531, &C.1532);
> 
> In principle the 'f' here should be an 'o.f'.

I opened PR 41149 to track this issue.

Comment 17 janus 2009-08-25 14:27:00 UTC
Subject: Bug 41139

Author: janus
Date: Tue Aug 25 14:26:44 2009
New Revision: 151081

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151081
Log:
2009-08-25  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/41139
	* primary.c (gfc_match_varspec): Make sure EXPR_PPC is only used for
	calls to procedure pointer components, other references to procedure
	pointer components are EXPR_VARIABLE.
	* resolve.c (resolve_actual_arglist): Bugfix (there can be calls without
	actual arglist).
	* trans-expr.c (gfc_get_proc_ptr_comp): Renamed to 'get_proc_ptr_comp',
	removed argument 'se' and made static. Avoid inserting a temporary
	variable for calling the PPC.
	(conv_function_val): Renamed gfc_get_proc_ptr_comp.
	(gfc_conv_procedure_call): Distinguish functions returning a procedure
	pointer from calls to a procedure pointer. Distinguish calls to
	procedure pointer components from procedure pointer components as
	actual arguments.
	* trans-stmt.h (gfc_get_proc_ptr_comp): Make it static.


2009-08-25  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/41139
	* gfortran.dg/proc_ptr_25.f90: New.
	* gfortran.dg/proc_ptr_comp_18.f90: New.
	* gfortran.dg/proc_ptr_comp_19.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/proc_ptr_25.f90
    trunk/gcc/testsuite/gfortran.dg/proc_ptr_comp_18.f90
    trunk/gcc/testsuite/gfortran.dg/proc_ptr_comp_19.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/primary.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans-stmt.h
    trunk/gcc/testsuite/ChangeLog

Comment 18 janus 2009-08-25 14:29:31 UTC
Fixed with r151081. Closing.
Comment 19 hjl@gcc.gnu.org 2009-08-30 02:07:06 UTC
Subject: Bug 41139

Author: hjl
Date: Sun Aug 30 02:06:32 2009
New Revision: 151218

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151218
Log:
2009-08-29  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline:
	2009-08-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR fortran/41162
	* gfortran.dg/pr41162.f: New.

	2009-08-26  Richard Guenther  <rguenther@suse.de>

	PR middle-end/41163
	* gcc.c-torture/compile/pr41163.c: New testcase.

	2009-08-25  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/41139
	* gfortran.dg/proc_ptr_25.f90: New.
	* gfortran.dg/proc_ptr_comp_18.f90: New.
	* gfortran.dg/proc_ptr_comp_19.f90: New.

	2009-08-20  Michael Matz  <matz@suse.de>

	PR fortran/41126
	* gfortran.dg/pr41126.f90: New test.

	2009-08-20  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/41121
	* gfortran.dg/intrinsic_5.f90: New.

	2009-08-19  Jason Merrill  <jason@redhat.com>

	PR c++/41120
	* g++.dg/other/gc4.C: New.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/other/gc4.C
      - copied unchanged from r151217, trunk/gcc/testsuite/g++.dg/other/gc4.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/compile/pr41163.c
      - copied unchanged from r151217, trunk/gcc/testsuite/gcc.c-torture/compile/pr41163.c
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/intrinsic_5.f90
      - copied unchanged from r151217, trunk/gcc/testsuite/gfortran.dg/intrinsic_5.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/pr41126.f90
      - copied unchanged from r151217, trunk/gcc/testsuite/gfortran.dg/pr41126.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/pr41162.f
      - copied unchanged from r151216, trunk/gcc/testsuite/gfortran.dg/pr41162.f
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/proc_ptr_25.f90
      - copied unchanged from r151217, trunk/gcc/testsuite/gfortran.dg/proc_ptr_25.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/proc_ptr_comp_18.f90
      - copied unchanged from r151217, trunk/gcc/testsuite/gfortran.dg/proc_ptr_comp_18.f90
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/proc_ptr_comp_19.f90
      - copied unchanged from r151217, trunk/gcc/testsuite/gfortran.dg/proc_ptr_comp_19.f90
Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog