Summary: | c_f_procpointer/c_f_pointer - add missing argument checking | ||
---|---|---|---|
Product: | gcc | Reporter: | Tobias Burnus <burnus> |
Component: | fortran | Assignee: | janus |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | burnus, gcc-bugs, janus |
Priority: | P3 | Keywords: | accepts-invalid |
Version: | 4.4.0 | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2009-06-22 22:21:56 | |
Bug Depends on: | |||
Bug Blocks: | 32630 |
Description
Tobias Burnus
2009-01-17 09:17:06 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. 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. 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) (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.) (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. 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). (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. 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. */ 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. 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 FIXED on the 4.9 trunk. |