Tobias,
You are quite right to take me to task. As I wrote in the original
message to the list, I was trying to respond rapidly before stepping
out of the door on vacation. The original patch is attached. The fix
to this problem is to revert that part in
libgfortran/runtime/ISO_Fortran_binding.c. As you implicitly surmised,
I was assuming that 'd' would be initialised in the caller. I cannot
see why this should be the case but sometimes the optimizer seems to
cut away a bit too much code :-(
I have done the reversion in r277204 after regtesting, of course.
I am retesting an update to 9-branch, as requested. I will submit to
the list tomorrow.
Cheers
Paul
2019-10-19 Paul Thomas <pa...@gcc.gnu.org>
PR fortran/91926
* runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Revert
the change made on 2019-10-05.
On Thu, 17 Oct 2019 at 14:39, Tobias Burnus <tob...@codesourcery.com> wrote:
>
> See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92027
> for a tracking bug – I just added also some analysis.
>
> Tobias
>
> PS: A better patch submission, with the actual patch attached, would
> have been nice. Please re-post the committed patch – and the new patch,
> which fixes the fall out. – Thanks!
>
> On 10/9/19 12:26 PM, Paul Richard Thomas wrote:
> > Hi Christophe,
> >
> > Thanks for flagging this up - I am back at base on Saturday and will
> > take it up then.
> >
> > Regards
> >
> > Paul
> >
> > On Wed, 9 Oct 2019 at 11:13, Christophe Lyon <christophe.l...@linaro.org>
> > wrote:
> >> Hi,
> >>
> >>
> >> On Sat, 5 Oct 2019 at 20:31, Paul Richard Thomas
> >> <paul.richard.tho...@gmail.com> wrote:
> >>> I must apologise not posting this before committing. I left for a
> >>> vacation this morning and I thought that this problem and the one
> >>> posted by Gilles were best fixed before departing. The patch only
> >>> touches the new ISO_Fortran binding feature and so I thought that I
> >>> would be safe to do this.
> >>>
> >>> It was fully regtested and only applies to trunk.
> >>>
> >>> Paul
> >>>
> >>> Author: pault
> >>> Date: Sat Oct 5 08:17:55 2019
> >>> New Revision: 276624
> >>>
> >>> URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev
> >>> Log:
> >>> 2019-10-05 Paul Thomas <pa...@gcc.gnu.org>
> >>>
> >>> PR fortran/91926
> >>> * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
> >>> assignment of the attribute field to account correctly for an
> >>> assumed shape dummy. Assign separately to the gfc and cfi
> >>> descriptors since the atribute can be different. Add btanch to
> >>> correctly handle missing optional dummies.
> >>>
> >>> 2019-10-05 Paul Thomas <pa...@gcc.gnu.org>
> >>>
> >>> PR fortran/91926
> >>> * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
> >>> * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
> >>> * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.
> >>>
> >>> 2019-10-05 Paul Thomas <pa...@gcc.gnu.org>
> >>>
> >>> PR fortran/91926
> >>> * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not
> >>> modify the bounds and offset for CFI_other.
> >>>
> >>> Added:
> >>> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
> >>> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
> >>> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
> >>> Modified:
> >>> trunk/gcc/fortran/ChangeLog
> >>> trunk/gcc/fortran/trans-expr.c
> >>> trunk/gcc/testsuite/ChangeLog
> >>> trunk/libgfortran/ChangeLog
> >>> trunk/libgfortran/runtime/ISO_Fortran_binding.c
> >>
> >>
> >> Since this was committed (r276624), I have noticed regressions on
> >> arm-linux-gnueabihf:
> >> FAIL: gfortran.dg/ISO_Fortran_binding_11.f90 -O3 -g execution test
> >> I've seen other reports on gcc-testresults too.
> >>
> >> Christophe
> >>
> >
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c (revision 276620)
--- gcc/fortran/trans-expr.c (working copy)
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5202,5208 ****
--- 5202,5210 ----
tree gfc_desc_ptr;
tree type;
tree cond;
+ tree desc_attr;
int attribute;
+ int cfi_attribute;
symbol_attribute attr = gfc_expr_attr (e);
stmtblock_t block;
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5211,5222 ****
attribute = 2;
if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
{
! if (fsym->attr.pointer)
attribute = 0;
! else if (fsym->attr.allocatable)
attribute = 1;
}
if (e->rank != 0)
{
parmse->force_no_tmp = 1;
--- 5213,5232 ----
attribute = 2;
if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
{
! if (attr.pointer)
attribute = 0;
! else if (attr.allocatable)
attribute = 1;
}
+ /* If the formal argument is assumed shape and neither a pointer nor
+ allocatable, it is unconditionally CFI_attribute_other. */
+ if (fsym->as->type == AS_ASSUMED_SHAPE
+ && !fsym->attr.pointer && !fsym->attr.allocatable)
+ cfi_attribute = 2;
+ else
+ cfi_attribute = attribute;
+
if (e->rank != 0)
{
parmse->force_no_tmp = 1;
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5283,5293 ****
parmse->expr, attr);
}
! /* Set the CFI attribute field. */
! tmp = gfc_conv_descriptor_attribute (parmse->expr);
tmp = fold_build2_loc (input_location, MODIFY_EXPR,
! void_type_node, tmp,
! build_int_cst (TREE_TYPE (tmp), attribute));
gfc_add_expr_to_block (&parmse->pre, tmp);
/* Now pass the gfc_descriptor by reference. */
--- 5293,5304 ----
parmse->expr, attr);
}
! /* Set the CFI attribute field through a temporary value for the
! gfc attribute. */
! desc_attr = gfc_conv_descriptor_attribute (parmse->expr);
tmp = fold_build2_loc (input_location, MODIFY_EXPR,
! void_type_node, desc_attr,
! build_int_cst (TREE_TYPE (desc_attr), cfi_attribute));
gfc_add_expr_to_block (&parmse->pre, tmp);
/* Now pass the gfc_descriptor by reference. */
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5305,5310 ****
--- 5316,5327 ----
gfor_fndecl_gfc_to_cfi, 2, tmp, gfc_desc_ptr);
gfc_add_expr_to_block (&parmse->pre, tmp);
+ /* Now set the gfc descriptor attribute. */
+ tmp = fold_build2_loc (input_location, MODIFY_EXPR,
+ void_type_node, desc_attr,
+ build_int_cst (TREE_TYPE (desc_attr), attribute));
+ gfc_add_expr_to_block (&parmse->pre, tmp);
+
/* The CFI descriptor is passed to the bind_C procedure. */
parmse->expr = cfi_desc_ptr;
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5325,5330 ****
--- 5342,5366 ----
tmp = build_call_expr_loc (input_location,
gfor_fndecl_cfi_to_gfc, 2, gfc_desc_ptr, tmp);
gfc_prepend_expr_to_block (&parmse->post, tmp);
+
+ /* Deal with an optional dummy being passed to an optional formal arg
+ by finishing the pre and post blocks and making their execution
+ conditional on the dummy being present. */
+ if (fsym->attr.optional && e->expr_type == EXPR_VARIABLE
+ && e->symtree->n.sym->attr.optional)
+ {
+ cond = gfc_conv_expr_present (e->symtree->n.sym);
+ tmp = fold_build2 (MODIFY_EXPR, void_type_node,
+ cfi_desc_ptr,
+ build_int_cst (pvoid_type_node, 0));
+ tmp = build3_v (COND_EXPR, cond,
+ gfc_finish_block (&parmse->pre), tmp);
+ gfc_add_expr_to_block (&parmse->pre, tmp);
+ tmp = build3_v (COND_EXPR, cond,
+ gfc_finish_block (&parmse->post),
+ build_empty_stmt (input_location));
+ gfc_add_expr_to_block (&parmse->post, tmp);
+ }
}
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c (nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c (working copy)
***************
*** 0 ****
--- 1,12 ----
+ /* Test the fix for PR91926. */
+
+ /* Contributed by José Rui Faustino de Sousa <jrfso...@hotmail.com> */
+
+ #include <stdlib.h>
+
+ int ifb_echo(void*);
+
+ int ifb_echo(void *this)
+ {
+ return this == NULL ? 1 : 2;
+ }
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90 (working copy)
***************
*** 0 ****
--- 1,39 ----
+ ! { dg-do run { target c99_runtime } }
+ ! { dg-additional-sources ISO_Fortran_binding_13.c }
+ !
+ ! Test the fix for PR91926. The additional source is the main program.
+ !
+ ! Contributed by José Rui Faustino de Sousa <jrfso...@hotmail.com>
+ !
+ program ifb_p
+
+ implicit none
+
+ integer :: i = 42
+
+ interface
+ integer function ifb_echo_aux(this) bind(c, name="ifb_echo")
+ implicit none
+ type(*), dimension(..), & ! removing assumed rank solves segmentation fault
+ optional, intent(in) :: this
+ end function ifb_echo_aux
+ end interface
+
+ if (ifb_echo_aux() .ne. 1) STOP 1 ! worked
+ if (ifb_echo() .ne. 1) stop 2 ! segmentation fault
+ if (ifb_echo_aux(i) .ne. 2) stop 3 ! worked
+ if (ifb_echo(i) .ne. 2) stop 4 ! worked
+
+ stop
+
+ contains
+
+ integer function ifb_echo(this)
+ type(*), dimension(..), &
+ optional, intent(in) :: this
+
+ ifb_echo = ifb_echo_aux(this)
+ return
+ end function ifb_echo
+
+ end program ifb_p
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90 (nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90 (working copy)
***************
*** 0 ****
--- 1,41 ----
+ ! { dg-do run }
+ !
+ ! Correct an error in the eveluation of the CFI descriptor attribute for
+ ! the case where the bind_C formal argument is not an assumed shape array
+ ! and not allocatable or pointer.
+ !
+ ! Contributed by Gilles Gouaillardet <gil...@rist.or.jp>
+ !
+ MODULE FOO
+ INTERFACE
+ SUBROUTINE dummy(buf) BIND(C, name="sync")
+ type(*), dimension(..) :: buf
+ END SUBROUTINE
+ END INTERFACE
+ END MODULE
+
+ PROGRAM main
+ USE FOO
+ IMPLICIT NONE
+ integer(8) :: before, after
+
+ INTEGER, parameter :: n = 1
+
+ INTEGER, ALLOCATABLE :: buf(:)
+ INTEGER :: buf2(n)
+ INTEGER :: i
+
+ ALLOCATE(buf(n))
+ before = LOC(buf(1))
+ CALL dummy (buf)
+ after = LOC(buf(1))
+
+ if (before .NE. after) stop 1
+
+ before = LOC(buf2(1))
+ CALL dummy (buf)
+ after = LOC(buf2(1))
+
+ if (before .NE. after) stop 2
+
+ END PROGRAM
Index: libgfortran/runtime/ISO_Fortran_binding.c
===================================================================
*** libgfortran/runtime/ISO_Fortran_binding.c (revision 276620)
--- libgfortran/runtime/ISO_Fortran_binding.c (working copy)
*************** cfi_desc_to_gfc_desc (gfc_array_void *d,
*** 63,69 ****
d->dtype.version = s->version;
GFC_DESCRIPTOR_RANK (d) = (signed char)s->rank;
! d->dtype.attribute = (signed short)s->attribute;
if (s->rank)
{
--- 63,70 ----
d->dtype.version = s->version;
GFC_DESCRIPTOR_RANK (d) = (signed char)s->rank;
! if (d->dtype.attribute == CFI_attribute_other)
! return;
if (s->rank)
{
