Bug 38894 - c_f_procpointer/c_f_pointer - add missing argument checking
Summary: c_f_procpointer/c_f_pointer - add missing argument checking
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.4.0
: P3 normal
Target Milestone: ---
Assignee: janus
URL:
Keywords: accepts-invalid
Depends on:
Blocks: 32630
  Show dependency treegraph
 
Reported: 2009-01-17 09:17 UTC by Tobias Burnus
Modified: 2013-03-25 17:48 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-06-22 22:21:56


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2009-01-17 09:17:06 UTC
Found in PR 38871. The following is invalid as C_F_FUNPTR takes function/procedure pointers as arguments.

TODO: Check also that C_F_POINTER doesn't take a type(c_ptr) as argument.

use iso_c_binding
type(c_ptr) :: fun
integer, pointer :: bar
call c_f_funpointer(fun,bar)
end
Comment 1 Tobias Burnus 2009-01-17 10:46:28 UTC
The example was wrong as the name is c_f_PROCpointer (no -fun- only in type(c_funptr) and c_FUNloc). Working example:

use iso_c_binding
type(c_ptr) :: fun
procedure(), pointer :: bar
call c_f_procpointer(fun,bar)  ! << fun is c_ptr not c_funptr
end

The same for C_F_POINTER:

use iso_c_binding
type(c_funptr) :: fun
integer, pointer :: bar
call c_f_pointer(fun,bar) ! << fun is c_funptr not a normal c_ptr
end

The "bar" argument seems to be correctly checked.
Comment 2 janus 2009-06-22 21:56:09 UTC
The missing checks are due to the following code in interface.c (compare_parameter):

  if (formal->ts.type == BT_DERIVED
      && formal->ts.derived && formal->ts.derived->ts.is_iso_c
      && actual->ts.type == BT_DERIVED
      && actual->ts.derived && actual->ts.derived->ts.is_iso_c)
    return 1;

Either this should be removed altogether, or at least the derived types should be checked for equality.

However, removing it causes iso_c_binding_rename_1.f03 to fail.
Comment 3 janus 2009-06-22 22:21:56 UTC
This patch gives the correct error messages for comment #1, while avoiding the testsuite failure of iso_c_binding_rename_1.f03:


Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(revision 148816)
+++ gcc/fortran/interface.c	(working copy)
@@ -1382,7 +1382,9 @@ compare_parameter (gfc_symbol *formal, g
   if (formal->ts.type == BT_DERIVED
       && formal->ts.derived && formal->ts.derived->ts.is_iso_c
       && actual->ts.type == BT_DERIVED
-      && actual->ts.derived && actual->ts.derived->ts.is_iso_c)
+      && actual->ts.derived && actual->ts.derived->ts.is_iso_c
+      && (formal->ts.derived->attr.use_rename
+	  || actual->ts.derived->attr.use_rename))
     return 1;
 
   if (actual->ts.type == BT_PROCEDURE)
Comment 4 Tobias Burnus 2009-06-23 08:34:29 UTC
(In reply to comment #3)
> This patch gives the correct error messages for comment #1

> +      && actual->ts.derived && actual->ts.derived->ts.is_iso_c
> +      && (formal->ts.derived->attr.use_rename
> +         || actual->ts.derived->attr.use_rename))

I think that is wrong - the problem is TYPE(c_PTR) vs TYPE(C_FUNPTR) and not the renaming. I think that will fail for:

use iso_c_binding, c_ptr2 => c_ptr
type(c_ptr2) :: fun
procedure(), pointer :: bar
call c_f_procpointer(fun,bar)  ! << fun is c_ptr not c_funptr
end

Maybe something using
 (fsym.intmod_sym_id == actual.intmod_sym_id)
where intmod_sym_id is either ISOCBINDING_PTR / ISOCBINDING_FUNPTR. (That may fail - I don't know how the interface is defined for c_f_{proc}pointer, it might miss this information.)
Comment 5 janus 2009-06-23 15:43:53 UTC
(In reply to comment #2)
> The missing checks are due to the following code in interface.c
> (compare_parameter):
> 
>   if (formal->ts.type == BT_DERIVED
>       && formal->ts.derived && formal->ts.derived->ts.is_iso_c
>       && actual->ts.type == BT_DERIVED
>       && actual->ts.derived && actual->ts.derived->ts.is_iso_c)
>     return 1;

In my opinion this code is some kind of workaround for a lower-lying problem. It just turns off the type checking, which is definitely not the right thing to do (as this PR shows).

The actual bug, which causes the failure of iso_c_binding_rename_1.f03, seems to be specific to ISO_C_BINDING with use-renaming. I tried to reproduce this by use-renaming ordinary derived types, but did not succeed.

If you look at the error messages in iso_c_binding_rename_1.f03, something definitely goes wrong there:

iso_c_binding_rename_1.f03:29.29:

    if(.not. my_c_associated(my_ptr)) then
                             1
Error: Type mismatch in argument 'c_ptr_1' at (1); passed TYPE(my_c_ptr) to TYPE(my_c_ptr_0)

This error happens in the routine 'sub1', where the type 'my_c_ptr_0' is not even accessible.
Comment 6 janus 2009-06-23 15:54:59 UTC
Here is a maximally reduced test case, which yields the same error as iso_c_binding_rename_1.f90 (if the code from comment #2 is removed):

module rename
  use, intrinsic :: iso_c_binding, only: my_c_ptr_0 => c_ptr
end module rename

program p
  use, intrinsic :: iso_c_binding, my_c_ptr => c_ptr
  type(my_c_ptr) :: my_ptr
  print *,c_associated(my_ptr)
contains
  subroutine sub()
    use rename   ! (***)
  end subroutine
end

The funny thing is: If the line marked with (***) is removed, the error goes away (while in principle this line should not have any effect at all).
Comment 7 janus 2012-04-25 17:57:11 UTC
(In reply to comment #6)
> Here is a maximally reduced test case, which yields the same error as
> iso_c_binding_rename_1.f90 (if the code from comment #2 is removed):

Another variant:


program p
  use iso_c_binding, only: c_ptr, c_associated, &
                           my_cptr_1 => c_ptr, my_cptr_2 => c_ptr
  implicit none
  type(c_ptr) :: my_ptr
  type(my_cptr_1) :: my_ptr_1
  type(my_cptr_2) :: my_ptr_2
  print *, c_associated (my_ptr)    ! passed TYPE(c_ptr) to TYPE(my_cptr_2)
  print *, c_associated (my_ptr_1)  ! passed TYPE(my_cptr_1) to TYPE(my_cptr_2)
  print *, c_associated (my_ptr_2)  ! works
end


The problem is apparently that the type of c_associated's argument is being renamed to 'my_cptr_2'. I also constructed an analogous test case with a derived type instead of c_ptr, which works correctly:


module m
  type :: t
  end type
contains
  logical function test(x)
    type(t) :: x
  end function
end module

program p
  use m, only: t, test, t1 => t, t2 => t
  implicit none
  type(t) :: y
  type(t1) :: y1
  type(t2) :: y2
  print *, test (y)   ! works
  print *, test (y1)  ! works
  print *, test (y2)  ! works
end 


This means the problem is special to ISO_C_BINDING.
Comment 8 janus 2012-04-25 19:47:52 UTC
The errors in comment #5 - #7 can be fixed by the following patch:


Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(revision 186596)
+++ gcc/fortran/interface.c	(working copy)
@@ -404,6 +404,13 @@ gfc_compare_derived_types (gfc_symbol *derived1, g
       && strcmp (derived1->module, derived2->module) == 0)
     return 1;
 
+  /* Types from intrinsinc modules (like ISO_C_BINDING) might be renamed,
+     but can still be identified via their 'intmod_sym_id'.  */
+  if (derived1->from_intmod != INTMOD_NONE
+      && derived1->from_intmod == derived2->from_intmod
+      && derived1->intmod_sym_id == derived2->intmod_sym_id)
+    return 1;
+
   /* Compare type via the rules of the standard.  Both types must have
      the SEQUENCE or BIND(C) attribute to be equal.  */
Comment 9 janus 2012-04-25 20:19:42 UTC
Combining comments #2 and #8 still produces testsuite failures:

FAIL: gfortran.dg/c_ptr_tests_14.f90  -O0  (test for excess errors)
FAIL: gfortran.dg/c_ptr_tests_15.f90  -O  (test for excess errors)


c_ptr_tests_14.f90:33.18:

  if(c_associated(file%gsl_func)) call abort()
                  1
Error: Type mismatch in argument 'c_ptr_1' at (1); passed TYPE(c_funptr) to TYPE(c_ptr)
c_ptr_tests_14.f90:39.18:

  if(c_associated(file%gsl_func)) call abort()
                  1
Error: Type mismatch in argument 'c_ptr_1' at (1); passed TYPE(c_funptr) to TYPE(c_ptr)


The problem seems to be that C_ASSOCIATED's formal args are TYPE(c_ptr), although c_ptr and c_funptr are allowed.
We already build two versions of C_ASSOCIATED (with one and two arguments, respectively). Probably we need two more with c_funptr arguments.
Comment 10 Tobias Burnus 2013-03-25 15:53:52 UTC
Author: burnus
Date: Mon Mar 25 15:40:26 2013
New Revision: 197053

URL: http://gcc.gnu.org/viewcvs?rev=197053&root=gcc&view=rev
Log:
2013-03-25  Tobias Burnus  <burnus@net-b.de>

        PR fortran/38536
        PR fortran/38813
        PR fortran/38894
        PR fortran/39288
        PR fortran/40963
        PR fortran/45824
        PR fortran/47023
        PR fortran/47034
        PR fortran/49023
        PR fortran/50269
        PR fortran/50612
        PR fortran/52426
        PR fortran/54263
        PR fortran/55343
        PR fortran/55444
        PR fortran/55574
        PR fortran/56079
        PR fortran/56378
        * check.c (gfc_var_strlen): Properly handle 0-sized string.
        (gfc_check_c_sizeof): Use is_c_interoperable, add checks.
        (is_c_interoperable, gfc_check_c_associated, gfc_check_c_f_pointer,
        gfc_check_c_f_procpointer, gfc_check_c_funloc, gfc_check_c_loc): New
        functions.
        * expr.c (check_inquiry): Add c_sizeof, compiler_version and
        compiler_options.
        (gfc_check_pointer_assign): Refine function result check.
        gfortran.h (gfc_isym_id): Add GFC_ISYM_C_ASSOCIATED,
        GFC_ISYM_C_F_POINTER, GFC_ISYM_C_F_PROCPOINTER, GFC_ISYM_C_FUNLOC,
        GFC_ISYM_C_LOC.
        (iso_fortran_env_symbol, iso_c_binding_symbol): Handle
        NAMED_SUBROUTINE.
        (generate_isocbinding_symbol): Update prototype.
        (get_iso_c_sym): Remove.
        (gfc_isym_id_by_intmod, gfc_isym_id_by_intmod_sym): New prototypes.
        * intrinsic.c (gfc_intrinsic_subroutine_by_id): New function.
        (gfc_intrinsic_sub_interface): Use it.
        (add_functions, add_subroutines): Add missing C-binding intrinsics.
        (gfc_intrinsic_func_interface): Add special case for c_loc.
        gfc_isym_id_by_intmod, gfc_isym_id_by_intmod_sym): New functions.
        (gfc_intrinsic_func_interface, gfc_intrinsic_sub_interface): Use them.
        * intrinsic.h (gfc_check_c_associated, gfc_check_c_f_pointer,
        gfc_check_c_f_procpointer, gfc_check_c_funloc, gfc_check_c_loc,
        gfc_resolve_c_loc, gfc_resolve_c_funloc): New prototypes.
        * iresolve.c (gfc_resolve_c_loc, gfc_resolve_c_funloc): New
        functions.
        * iso-c-binding.def: Split PROCEDURE into NAMED_SUBROUTINE and
        NAMED_FUNCTION.
        * iso-fortran-env.def: Add NAMED_SUBROUTINE for completeness.
        * module.c (create_intrinsic_function): Support subroutines and
        derived-type results.
        (use_iso_fortran_env_module): Update calls.
        (import_iso_c_binding_module): Ditto; update calls to
        generate_isocbinding_symbol.
        * resolve.c (find_arglists): Skip for intrinsic symbols.
        (gfc_resolve_intrinsic): Find intrinsic subs via id.
        (is_scalar_expr_ptr, gfc_iso_c_func_interface,
        set_name_and_label, gfc_iso_c_sub_interface): Remove.
        (resolve_function, resolve_specific_s0): Remove calls to those.
        (resolve_structure_cons): Fix handling.
        * symbol.c (gen_special_c_interop_ptr): Update c_ptr/c_funptr
        generation.
        (gen_cptr_param, gen_fptr_param, gen_shape_param,
        build_formal_args, get_iso_c_sym): Remove.
        (std_for_isocbinding_symbol): Handle NAMED_SUBROUTINE.
        (generate_isocbinding_symbol): Support hidden symbols and
        using c_ptr/c_funptr symtrees for nullptr defs.
        * target-memory.c (gfc_target_encode_expr): Fix handling
        of c_ptr/c_funptr.
        * trans-expr.c (conv_isocbinding_procedure): Remove.
        (gfc_conv_procedure_call): Remove call to it.
        (gfc_trans_subcomponent_assign, gfc_conv_expr): Update handling
        of c_ptr/c_funptr.
        * trans-intrinsic.c (conv_isocbinding_function,
        conv_isocbinding_subroutine): New.
        (gfc_conv_intrinsic_function, gfc_conv_intrinsic_subroutine):
        Call them.
        * trans-io.c (transfer_expr): Fix handling of c_ptr/c_funptr.
        * trans-types.c (gfc_typenode_for_spec,
        gfc_get_derived_type): Ditto.
        (gfc_init_c_interop_kinds): Handle NAMED_SUBROUTINE.

2013-03-25  Tobias Burnus  <burnus@net-b.de>

        PR fortran/38536
        PR fortran/38813
        PR fortran/38894
        PR fortran/39288
        PR fortran/40963
        PR fortran/45824
        PR fortran/47023
        PR fortran/47034
        PR fortran/49023
        PR fortran/50269
        PR fortran/50612
        PR fortran/52426
        PR fortran/54263
        PR fortran/55343
        PR fortran/55444
        PR fortran/55574
        PR fortran/56079
        PR fortran/56378
        * gfortran.dg/c_assoc_2.f03: Update dg-error wording.
        * gfortran.dg/c_f_pointer_shape_test.f90: Ditto.
        * gfortran.dg/c_f_pointer_shape_tests_3.f03: Ditto.
        * gfortran.dg/c_f_pointer_tests_5.f90: Ditto.
        * gfortran.dg/c_funloc_tests_2.f03: Ditto.
        * gfortran.dg/c_funloc_tests_5.f03: Ditto.
        * gfortran.dg/c_funloc_tests_6.f90: Ditto.
        * gfortran.dg/c_loc_tests_10.f03: Add -std=f2008.
        * gfortran.dg/c_loc_tests_11.f03: Ditto, update dg-error.
        * gfortran.dg/c_loc_tests_16.f90: Ditto.
        * gfortran.dg/c_loc_tests_4.f03: Ditto.
        * gfortran.dg/c_loc_tests_15.f90: Update dg-error wording.
        * gfortran.dg/c_loc_tests_3.f03: Valid since F2003 TC5.
        * gfortran.dg/c_loc_tests_8.f03: Ditto.
        * gfortran.dg/c_ptr_tests_14.f90: Update scan-tree-dump-times.
        * gfortran.dg/c_ptr_tests_15.f90: Ditto.
        * gfortran.dg/c_sizeof_1.f90: Fix invalid code.
        * gfortran.dg/iso_c_binding_init_expr.f03: Update dg-error wording.
        * gfortran.dg/pr32601_1.f03: Ditto.
        * gfortran.dg/storage_size_2.f08: Remove dg-error.
        * gfortran.dg/blockdata_7.f90: New.
        * gfortran.dg/c_assoc_4.f90: New.
        * gfortran.dg/c_f_pointer_tests_6.f90: New.
        * gfortran.dg/c_f_pointer_tests_7.f90: New.
        * gfortran.dg/c_funloc_tests_8.f90: New.
        * gfortran.dg/c_loc_test_17.f90: New.
        * gfortran.dg/c_loc_test_18.f90: New.
        * gfortran.dg/c_loc_test_19.f90: New.
        * gfortran.dg/c_loc_test_20.f90: New.
        * gfortran.dg/c_sizeof_5.f90: New.
        * gfortran.dg/iso_c_binding_rename_3.f90: New.
        * gfortran.dg/transfer_resolve_2.f90: New.
        * gfortran.dg/transfer_resolve_3.f90: New.
        * gfortran.dg/transfer_resolve_4.f90: New.
        * gfortran.dg/pr32601.f03: Update dg-error.
        * gfortran.dg/c_ptr_tests_13.f03: Update dg-error.
        * gfortran.dg/c_ptr_tests_9.f03: Fix test case.


Added:
    trunk/gcc/testsuite/gfortran.dg/blockdata_7.f90
    trunk/gcc/testsuite/gfortran.dg/c_assoc_4.f90
    trunk/gcc/testsuite/gfortran.dg/c_f_pointer_tests_6.f90
    trunk/gcc/testsuite/gfortran.dg/c_f_pointer_tests_7.f90
    trunk/gcc/testsuite/gfortran.dg/c_funloc_tests_8.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_test_17.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_test_18.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_test_19.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_test_20.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_17.f90
    trunk/gcc/testsuite/gfortran.dg/c_sizeof_5.f90
    trunk/gcc/testsuite/gfortran.dg/iso_c_binding_rename_3.f90
    trunk/gcc/testsuite/gfortran.dg/transfer_resolve_2.f90
    trunk/gcc/testsuite/gfortran.dg/transfer_resolve_3.f90
    trunk/gcc/testsuite/gfortran.dg/transfer_resolve_4.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/check.c
    trunk/gcc/fortran/expr.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/intrinsic.c
    trunk/gcc/fortran/intrinsic.h
    trunk/gcc/fortran/iresolve.c
    trunk/gcc/fortran/iso-c-binding.def
    trunk/gcc/fortran/iso-fortran-env.def
    trunk/gcc/fortran/module.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/symbol.c
    trunk/gcc/fortran/target-memory.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/fortran/trans-io.c
    trunk/gcc/fortran/trans-types.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/c_assoc_2.f03
    trunk/gcc/testsuite/gfortran.dg/c_f_pointer_shape_test.f90
    trunk/gcc/testsuite/gfortran.dg/c_f_pointer_shape_tests_3.f03
    trunk/gcc/testsuite/gfortran.dg/c_f_pointer_tests_5.f90
    trunk/gcc/testsuite/gfortran.dg/c_funloc_tests_2.f03
    trunk/gcc/testsuite/gfortran.dg/c_funloc_tests_5.f03
    trunk/gcc/testsuite/gfortran.dg/c_funloc_tests_6.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_10.f03
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_11.f03
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_15.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_16.f90
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_3.f03
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_4.f03
    trunk/gcc/testsuite/gfortran.dg/c_loc_tests_8.f03
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_13.f03
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_14.f90
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_15.f90
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_9.f03
    trunk/gcc/testsuite/gfortran.dg/c_sizeof_1.f90
    trunk/gcc/testsuite/gfortran.dg/iso_c_binding_init_expr.f03
    trunk/gcc/testsuite/gfortran.dg/pr32601.f03
    trunk/gcc/testsuite/gfortran.dg/pr32601_1.f03
    trunk/gcc/testsuite/gfortran.dg/storage_size_2.f08
Comment 11 Tobias Burnus 2013-03-25 17:48:57 UTC
FIXED on the 4.9 trunk.