[PATCH] Fortran: error recovery on arithmetic overflow on unary operations [PR113799]
Dear all, the attached patch improves error recovery when we encounter an array constructor where a unary operator (e.g. minus) is applied and -frange-check is active. The solution is not to terminate early in that case to avoid inconsistencies between check_result and reduce_unary when such a situation occurs. (There might be similar issues for binary operators, not treated here.) Regtested on x86_64-pc-linux-gnu. OK for mainline? The ICE/memory corruption is actually a 10+ regression. Do we need a backport? Thanks, Harald From eec039211e396e35204b55588013d74289a984cd Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 8 Feb 2024 21:51:38 +0100 Subject: [PATCH] Fortran: error recovery on arithmetic overflow on unary operations [PR113799] PR fortran/113799 gcc/fortran/ChangeLog: * arith.cc (reduce_unary): Remember any overflow encountered during reduction of unary arithmetic operations on array constructors and continue, and return error status, but terminate on serious errors. gcc/testsuite/ChangeLog: * gfortran.dg/arithmetic_overflow_2.f90: New test. --- gcc/fortran/arith.cc| 11 --- gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 | 12 2 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index 0598f6ac51b..d17d1aaa1d9 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -1323,6 +1323,7 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **), gfc_expr *op, gfc_constructor *c; gfc_expr *r; arith rc; + bool ov = false; if (op->expr_type == EXPR_CONSTANT) return eval (op, result); @@ -1336,13 +1337,17 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **), gfc_expr *op, { rc = reduce_unary (eval, c->expr, &r); - if (rc != ARITH_OK) + /* Remember any overflow encountered during reduction and continue, + but terminate on serious errors. */ + if (rc == ARITH_OVERFLOW) + ov = true; + else if (rc != ARITH_OK) break; gfc_replace_expr (c->expr, r); } - if (rc != ARITH_OK) + if (rc != ARITH_OK && rc != ARITH_OVERFLOW) gfc_constructor_free (head); else { @@ -1363,7 +1368,7 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **), gfc_expr *op, *result = r; } - return rc; + return ov ? ARITH_OVERFLOW : rc; } diff --git a/gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 b/gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 new file mode 100644 index 000..6ca27f74215 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 @@ -0,0 +1,12 @@ +! { dg-do compile } +! { dg-additional-options "-frange-check" } +! +! PR fortran/113799 - handle arithmetic overflow on unary minus + +program p + implicit none + real, parameter :: inf = real(z'7F80') + real, parameter :: someInf(*) = [inf, 0.] + print *, -someInf ! { dg-error "Arithmetic overflow" } + print *, minval(-someInf) ! { dg-error "Arithmetic overflow" } +end -- 2.35.3
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Hi Alex, I've been unable to apply your patch to my local trunk, likely due to whitespace issues my newsreader handles differently from your site. I see it inline instead of attached. A few general remarks: Please follow the general recommendation regarding style if possible, see https://www.gnu.org/prep/standards/standards.html#Formatting regarding formatting/whitespace use (5.1) and comments (5.2) Also, when an error message text spans multiple lines, please place the whitespace at the end of a line, not at the beginning of the new one: + if ( resolve_bindings_derived->attr.pdt_template && + !gfc_pdt_is_instance_of(resolve_bindings_derived, + CLASS_DATA(me_arg)->ts.u.derived)) +{ + gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of" +" the parametric derived-type %qs", me_arg->name, proc->name, gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of " "the parametric derived-type %qs", me_arg->name, proc->name, +me_arg->name, &where, resolve_bindings_derived->name); + goto error; +} The following change is almost unreadable: the lnegthy comment is split over three parts and almost hides the code. Couldn't this be combined into one comment before the function? diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index fddf68f8398..11f4bac0415 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc @@ -5172,6 +5172,35 @@ gfc_type_is_extension_of (gfc_symbol *t1, gfc_symbol *t2) return gfc_compare_derived_types (t1, t2); } +/* Check if a parameterized derived type t2 is an instance of a PDT template t1 */ + +bool +gfc_pdt_is_instance_of(gfc_symbol *t1, gfc_symbol *t2) +{ + if ( !t1->attr.pdt_template || !t2->attr.pdt_type ) +return false; + + /* +in decl.cc, gfc_get_pdt_instance, a pdt instance is given a 3 character prefix "Pdt", followed +by an underscore list of the kind parameters, up to a maximum of 8. + +So to check if a PDT Type corresponds to the template, extract the core derive_type name, +and then see if it is type compatible by name... + +For example: + +Pdtf_2_2 -> extract out the 'f' -> see if the derived type 'f' is compatible with symbol t1 + */ + + // Starting at index 3 of the string in order to skip past the 'Pdt' prefix + // Also, here the length of the template name is used in order to avoid the + // kind parameter suffixes that are placed at the end of PDT instance names. + if ( !(strncmp(&(t2->name[3]), t1->name, strlen(t1->name)) == 0) ) +return false; + + return true; +} + /* Check if two typespecs are type compatible (F03:5.1.1.2): If ts1 is nonpolymorphic, ts2 must be the same type. The following testcase tests for errors. I tried Intel and NAG on it after commenting the 'contains' section of the type desclaration. Both complained about subroutine deferred_len_param, e.g. Intel: A colon may only be used as a type parameter value in the declaration of an object that has the POINTER or ALLOCATABLE attribute. [THIS] class(param_deriv_type(:)), intent(inout) :: this NAG: Entity THIS of type PARAM_DERIV_TYPE(A=:) has a deferred length type parameter but is not a data pointer or allocatable Do we detect this after your patch? If the answer is yes, can we add another subroutine where we check for this error? (the dg-error suggests we only expect assumed len type parameters.) If no, maybe add a comment in the testcase that this subroutine may need updating later. diff --git a/gcc/testsuite/gfortran.dg/pdt_37.f03 b/gcc/testsuite/gfortran.dg/pdt_37.f03 new file mode 100644 index 000..68d376fad25 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pdt_37.f03 @@ -0,0 +1,34 @@ +! { dg-do compile } +! +! Tests the fixes for PR82943. +! +! This test focuses on the errors produced by incorrect LEN parameters for dummy +! arguments of PDT Typebound Procedures. +! +! Contributed by Alexander Westbrooks +! +module test_len_param + + type :: param_deriv_type(a) + integer, len :: a + contains + procedure :: assumed_len_param ! Good. No error expected. + procedure :: deferred_len_param ! { dg-error "All LEN type parameters of the passed dummy argument" } + procedure :: fixed_len_param ! { dg-error "All LEN type parameters of the passed dummy argument" } + end type + +contains +subroutine assumed_len_param(this) + class(param_deriv_type(*)), intent(inout) :: this +end subroutine + +subroutine deferred_len_param(this) +class(param_deriv_type(:)), intent(inout) :: this +end subroutine + +subroutine fixed_len_param(this) +class(param_deriv_type(10)), intent(inout) :: this +end subroutine + +end module +
[PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Dear all, the attached patch fixes a mis-handling of optional dummy arguments passed to optional dummy arguments of procedures with the bind(c) attribute. When those procedures are expecting CFI descriptors, there is no special treatment like a presence check necessary that by default passes a NULL pointer as default. The testcase tries to exercise various combinations of passing assumed-length character between bind(c) and non-bind(c), which apparently was insufficiently covered in the testsuite. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 87d1b973a4d6a561dc3f3a0c4c10f76d155fa000 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 12 Feb 2024 21:39:09 +0100 Subject: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866] PR fortran/113866 gcc/fortran/ChangeLog: * trans-expr.cc (gfc_conv_procedure_call): When passing an optional dummy argument to an optional dummy argument of a bind(c) procedure and the dummy argument is passed via a CFI descriptor, no special presence check and passing of a default NULL pointer is needed. gcc/testsuite/ChangeLog: * gfortran.dg/bind_c_optional-2.f90: New test. --- gcc/fortran/trans-expr.cc | 6 +- .../gfortran.dg/bind_c_optional-2.f90 | 104 ++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 67abca9f6ba..a0593b76f18 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -7269,8 +7269,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, with an interface indicating an optional argument. When we call an intrinsic subroutine, however, fsym is NULL, but we might still have an optional argument, so we proceed to the substitution - just in case. */ - if (e && (fsym == NULL || fsym->attr.optional)) + just in case. Arguments passed to bind(c) procedures via CFI + descriptors are handled elsewhere. */ + if (e && (fsym == NULL || fsym->attr.optional) + && !(sym->attr.is_bind_c && is_CFI_desc (fsym, NULL))) { /* If an optional argument is itself an optional dummy argument, check its presence and substitute a null if absent. This is diff --git a/gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 b/gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 new file mode 100644 index 000..b8b4c87775e --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 @@ -0,0 +1,104 @@ +! { dg-do run } +! PR fortran/113866 +! +! Check interoperability of assumed-length character (optional and +! non-optional) dummies between bind(c) and non-bind(c) procedures + +module bindcchar + implicit none + integer, parameter :: n = 100, l = 10 +contains + subroutine bindc_optional (c2, c4) bind(c) +character(*), optional :: c2, c4(n) +! print *, c2(1:3) +! print *, c4(5)(1:3) +if (.not. present (c2) .or. .not. present (c4)) stop 8 +if (c2(1:3)/= "a23") stop 1 +if (c4(5)(1:3) /= "bcd") stop 2 +if (len (c2) /= l .or. len (c4) /= l) stop 81 + end + + subroutine bindc (c2, c4) bind(c) +character(*) :: c2, c4(n) +if (c2(1:3)/= "a23") stop 3 +if (c4(5)(1:3) /= "bcd") stop 4 +if (len (c2) /= l .or. len (c4) /= l) stop 82 +call bindc_optional (c2, c4) + end + + subroutine not_bindc_optional (c1, c3) +character(*), optional :: c1, c3(n) +if (.not. present (c1) .or. .not. present (c3)) stop 5 +call bindc_optional (c1, c3) +call bindc (c1, c3) +if (len (c1) /= l .or. len (c3) /= l) stop 83 + end + + subroutine not_bindc_optional_deferred (c5, c6) +character(:), allocatable, optional :: c5, c6(:) +if (.not. present (c5) .or. .not. present (c6)) stop 6 +call not_bindc_optional (c5, c6) +call bindc_optional (c5, c6) +call bindc (c5, c6) +if (len (c5) /= l .or. len (c6) /= l) stop 84 + end + + subroutine not_bindc_optional2 (c7, c8) +character(*), optional :: c7, c8(:) +if (.not. present (c7) .or. .not. present (c8)) stop 7 +call bindc_optional (c7, c8) +call bindc (c7, c8) +if (len (c7) /= l .or. len (c8) /= l) stop 85 + end + + subroutine bindc_optional2 (c2, c4) bind(c) +character(*), optional :: c2, c4(n) +if (.not. present (c2) .or. .not. present (c4)) stop 8 +if (c2(1:3)/= "a23") stop 9 +if (c4(5)(1:3) /= "bcd") stop 10 +call bindc_optional (c2, c4) +call not_bindc_optional (c2, c4) +if (len (c2) /= l .or. len (c4) /= l) stop 86 + end + + subroutine bindc_optional_missing (c1, c2, c3, c4, c5) bind(c) +character(*), optional :: c1, c2(n), c3(:), c4(..), c5(*) +if (present (c1)) stop 11 +if (present (c2)) stop 12 +if (present (c3)) stop 13 +if (present (c4)) sto
Re: [patch, libgfortran] PR109358
Hi Jerry. Am 12.02.24 um 22:28 schrieb Jerry D: The attached patch fixes this PR by properly adjusting some variables When using stream io. See log below. New test case included. Regression tested on x86_64. OK for trunk and backport? the patch looks good to me. As it is simple and very local, feel free to backport at your discretion. Thanks for the patch! Harald Regards, Jerry ChangeLog: libgfortran: Adjust bytes_left and pos for access="STREAM". During tab edits, the pos (position) and bytes_used Variables were not being set correctly for stream I/O. Since stream I/O does not have 'real' records, the format buffer active length must be used instead of the record length variable. libgfortran/ChangeLog: PR libgfortran/109358 * io/transfer.c (formatted_transfer_scalar_write): Adjust bytes_used and pos variable for stream access. gcc/testsuite/ChangeLog: PR libgfortran/109358 * gfortran.dg/pr109358.f90: New test.
Re: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Hi Steve, Am 13.02.24 um 18:21 schrieb Steve Kargl: On Mon, Feb 12, 2024 at 09:57:08PM +0100, Harald Anlauf wrote: Dear all, the attached patch fixes a mis-handling of optional dummy arguments passed to optional dummy arguments of procedures with the bind(c) attribute. When those procedures are expecting CFI descriptors, there is no special treatment like a presence check necessary that by default passes a NULL pointer as default. The testcase tries to exercise various combinations of passing assumed-length character between bind(c) and non-bind(c), which apparently was insufficiently covered in the testsuite. Regtested on x86_64-pc-linux-gnu. OK for mainline? Yes. Thanks for filling out the more detailed testcase. indeed the new testcase just regressed due to commit r14-8947-g6caec7d9ec37e6 ... :-( Reduced testcase which fails on trunk: program p implicit none integer, parameter :: n = 100, l = 10 character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' character(:), allocatable :: d, e(:) allocate (d, source=a) allocate (e, source=b) print *, len (d), len (e), size (e) call not_bindc_optional_deferred (d, e) deallocate (d, e) contains subroutine not_bindc_optional_deferred (c5, c6) character(:), allocatable, optional :: c5, c6(:) if (.not. present (c5) .or. .not. present (c6)) stop 6 print *, len (c5), len (c6), size (c6) if (len (c5) /= l .or. len (c6) /= l) stop 84 end end Expected: 10 10 100 10 10 100 After above commit: 10 10 100 10 0 100 STOP 84 Will have to wait until the cause is found and fixed...
Re: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Am 13.02.24 um 19:13 schrieb Harald Anlauf: indeed the new testcase just regressed due to commit r14-8947-g6caec7d9ec37e6 ... :-( Reduced testcase which fails on trunk: program p implicit none integer, parameter :: n = 100, l = 10 character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' character(:), allocatable :: d, e(:) allocate (d, source=a) allocate (e, source=b) print *, len (d), len (e), size (e) call not_bindc_optional_deferred (d, e) deallocate (d, e) contains subroutine not_bindc_optional_deferred (c5, c6) character(:), allocatable, optional :: c5, c6(:) if (.not. present (c5) .or. .not. present (c6)) stop 6 print *, len (c5), len (c6), size (c6) if (len (c5) /= l .or. len (c6) /= l) stop 84 end end Expected: 10 10 100 10 10 100 After above commit: 10 10 100 10 0 100 STOP 84 This is now tracked as:: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113911 Will have to wait until the cause is found and fixed... As it is sufficient to disable the deferred-length test, I've done that and pushed the amended patch as https://gcc.gnu.org/g:f4935df217ad89f884f908f39086b322e80123d0 Thanks, Harald
Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
Hi Peter, thanks for your contribution to gfortran! You've found indeed a solution for a potentially annoying bug. Am 15.02.24 um 18:50 schrieb Peter Hill: Dear all, The attached patch fixes PR105658 by forcing an array temporary to be created. This is required when passing an array component, but this didn't happen if the dummy argument was an unlimited polymorphic type. The problem bit of code is in `gfc_conv_expr_descriptor`, near L7828: subref_array_target = (is_subref_array (expr) && (se->direct_byref || expr->ts.type == BT_CHARACTER)); need_tmp = (gfc_ref_needs_temporary_p (expr->ref) && !subref_array_target); where `need_tmp` is being evaluated to 0. The logic here isn't clear to me, and this function is used in several places, which is why I went with setting `parmse.force_tmp = 1` in `gfc_conv_procedure_call` and using the same conditional as the later branch for the non-polymorphic case (near the call to `gfc_conv_subref_array_arg`) If this patch is ok, please could someone commit it for me? This is my first patch for GCC, so apologies in advance if the commit message is missing something. Your patch mostly does the right thing. Note that when fsym is an unlimited polymorphic, some of its attributes are buried deep within its internal representation. I would also prefer to move the code to gfc_conv_intrinsic_to_class where it seems to fit better, like: diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..db906caa52e 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1019,6 +1019,14 @@ gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e, tmp = gfc_typenode_for_spec (&class_ts); var = gfc_create_var (tmp, "class"); + /* Force a temporary for component or substring references. */ + if (unlimited_poly + && class_ts.u.derived->components->attr.dimension + && !class_ts.u.derived->components->attr.class_pointer + && !class_ts.u.derived->components->attr.allocatable + && is_subref_array (e)) +parmse->force_tmp = 1; + /* Set the vptr. */ ctree = gfc_class_vptr_get (var); (I am not entirely sure whether we need to exclude pointer and allocatable attributes here explicitly, given the constraints in F2023:15.5.2.6, but other may have an opinion, too. The above should be safe anyway.) Tested on x86_64-pc-linux-gnu. The bug is present in gfortran back to 4.9, so should it also be backported? I think we'll target 14-mainline and might consider a backport to 13-branch. Cheers, Peter PR fortran/105658 gcc/fortran/ChangeLog * trans-expr.cc (gfc_conv_procedure_call): When passing an array component reference of intrinsic type to a procedure with an unlimited polymorphic dummy argument, a temporary should be created. gcc/testsuite/ChangeLog * gfortran.dg/PR105658.f90: New test. --- gcc/fortran/trans-expr.cc | 8 gcc/testsuite/gfortran.dg/PR105658.f90 | 25 + 2 files changed, 33 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..7fd3047c4e9 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6439,6 +6439,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, CLASS object for the unlimited polymorphic formal. */ gfc_find_vtab (&e->ts); gfc_init_se (&parmse, se); + /* The actual argument is a component reference to an array + of derived types, so we need to force creation of a + temporary */ + if (e->expr_type == EXPR_VARIABLE + && is_subref_array (e) + && !(fsym && fsym->attr.pointer)) + parmse.force_tmp = 1; + gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts); } diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90 b/gcc/testsuite/gfortran.dg/PR105658.f90 new file mode 100644 index 000..407ee25f77c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/PR105658.f90 @@ -0,0 +1,25 @@ +! { dg-do compile } +! { dg-options "-Warray-temporaries" } +! Test fix for incorrectly passing array component to unlimited polymorphic procedure + +module test_PR105658_mod + implicit none + type :: foo +integer :: member1 +integer :: member2 + end type foo +contains + subroutine print_poly(array) +class(*), dimension(:), intent(in) :: array +select type(array) +type is (integer) + print*, array +end select + end subroutine print_poly + + subroutine do_print(thing) +type(foo), dimension(3), intent(in) :: thing +call print_poly(thing%member1) ! { dg-warning "array temporary" } + end subroutine do_print + +end module test_PR105658_mod One could extend this testcase to cover substrings as well: module test_PR105658_mod implicit none type :: foo integer :: member1 integer :: member2 end type foo contains subroutine print_poly(arra
[PATCH] Fortran: deferred length of character variables shall not get lost [PR113911]
Dear all, this patch fixes a regression which was a side-effect of r14-8947, losing the length of a deferred-length character variable when passed as a dummy. The new testcase provides a workout for deferred length to improve coverage in the testsuite. Another temporarily disabled test was re-enabled. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 07fcdf7c9f9272d8e4752c23f04795d02d4ad440 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 16 Feb 2024 22:33:16 +0100 Subject: [PATCH] Fortran: deferred length of character variables shall not get lost [PR113911] PR fortran/113911 gcc/fortran/ChangeLog: * trans-array.cc (gfc_trans_deferred_array): Do not clobber deferred length for a character variable passed as dummy argument. gcc/testsuite/ChangeLog: * gfortran.dg/allocatable_length_2.f90: New test. * gfortran.dg/bind_c_optional-2.f90: Enable deferred-length test. --- gcc/fortran/trans-array.cc| 2 +- .../gfortran.dg/allocatable_length_2.f90 | 107 ++ .../gfortran.dg/bind_c_optional-2.f90 | 3 +- 3 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/allocatable_length_2.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 2181990aa04..3673fa40720 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -11531,7 +11531,7 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block) if (sym->ts.type == BT_CHARACTER && !INTEGER_CST_P (sym->ts.u.cl->backend_decl)) { - if (sym->ts.deferred && !sym->ts.u.cl->length) + if (sym->ts.deferred && !sym->ts.u.cl->length && !sym->attr.dummy) gfc_add_modify (&init, sym->ts.u.cl->backend_decl, build_zero_cst (TREE_TYPE (sym->ts.u.cl->backend_decl))); gfc_conv_string_length (sym->ts.u.cl, NULL, &init); diff --git a/gcc/testsuite/gfortran.dg/allocatable_length_2.f90 b/gcc/testsuite/gfortran.dg/allocatable_length_2.f90 new file mode 100644 index 000..2fd64efdc25 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocatable_length_2.f90 @@ -0,0 +1,107 @@ +! { dg-do run } +! PR fortran/113911 +! +! Test that deferred length is not lost + +module m + integer, parameter:: n = 100, l = 10 + character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' + character(:), allocatable :: c1, c2(:) +end + +program p + use m, only : l, n, a, b, x => c1, y => c2 + implicit none + character(:), allocatable :: d, e(:) + allocate (d, source=a) + allocate (e, source=b) + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 12 + call plain_deferred (d, e) + call optional_deferred (d, e) + call optional_deferred_ar (d, e) + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 13 + deallocate (d, e) + call alloc (d, e) + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 14 + deallocate (d, e) + call alloc_host_assoc () + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 15 + deallocate (d, e) + call alloc_use_assoc () + if (len (x) /= l .or. len (y) /= l .or. size (y) /= n) stop 16 + call indirect (x, y) + if (len (x) /= l .or. len (y) /= l .or. size (y) /= n) stop 17 + deallocate (x, y) +contains + subroutine plain_deferred (c1, c2) +character(:), allocatable :: c1, c2(:) +if (.not. allocated (c1) .or. .not. allocated (c2)) stop 1 +if (len (c1) /= l) stop 2 +if (len (c2) /= l) stop 3 +if (c1(1:3)/= "a23") stop 4 +if (c2(5)(1:3) /= "bcd") stop 5 + end + + subroutine optional_deferred (c1, c2) +character(:), allocatable, optional :: c1, c2(:) +if (.not. present (c1) .or. .not. present (c2)) stop 6 +if (.not. allocated (c1) .or. .not. allocated (c2)) stop 7 +if (len (c1) /= l) stop 8 +if (len (c2) /= l) stop 9 +if (c1(1:3)/= "a23") stop 10 +if (c2(5)(1:3) /= "bcd") stop 11 + end + + ! Assumed rank + subroutine optional_deferred_ar (c1, c2) +character(:), allocatable, optional :: c1(..) +character(:), allocatable, optional :: c2(..) +if (.not. present (c1) .or. & +.not. present (c2)) stop 21 +if (.not. allocated (c1) .or. & +.not. allocated (c2)) stop 22 + +select rank (c1) +rank (0) +if (len (c1) /= l) stop 23 + if (c1(1:3) /= "a23") stop 24 +rank default + stop 25 +end select + +select rank (c2) +rank (1) + if (len (c2) /= l) stop 26 + if (c2(5)(1:3) /= "bcd") stop 27 +rank default + stop 28 +end select + end + + ! Allocate dummy arguments + subroutine alloc (c1, c2) +character(:), allocatable :: c1, c2(:) +allocate (c1, source=a) +allocate (c2, source=b) + end + + ! Allocate host-associated variables + subroutine allo
Re: [PATCH] fortran: gfc_trans_subcomponent_assign fixes [PR113503]
Hi Jakub, On 2/17/24 10:02, Jakub Jelinek wrote: Hi! The r14-870 changes broke xtb package tests (reduced testcase is the first one below) and caused ICEs on a test derived from that (the second one). [...] thanks for your detailed analysis and for the patch, which puts things in straight order to actually fix two issues here! Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK for trunk, except for the minor nit below. --- gcc/testsuite/gfortran.dg/pr113503_1.f90.jj 2024-02-16 14:16:17.937153094 +0100 +++ gcc/testsuite/gfortran.dg/pr113503_1.f902024-02-16 14:16:10.124258815 +0100 @@ -0,0 +1,18 @@ +! PR fortran/113503 +! { dg-do compile } +! { dg-options "-O2 -fno-inline -Wuninitialized" } + +program pr113503 + implicit none + type :: T +character(len=:), allocatable :: u + end type + character(len=20) :: us(1) = 'foobar' + type(T) :: x + x = T(u = trim (us(1))) ! { dg-bogus "is used uninitialized" } tab here not allowed in Fortran My newsreader shows a tab here, giving a warning when running the test. Also, applying your patch on top of r14-9045 I do not see the uninitialized warning, which could have been fixed by r14-8947. Please recheck and adjust accordingly. + call foo +contains + subroutine foo +if (x%u /= 'foobar') stop 1 + end subroutine +end Thanks, Harald
Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
Hi Peter, On 2/19/24 16:19, Peter Hill wrote: Hi Harald, Thanks for your help, please see the updated and signed-off patch below. great! This is fine, and I'll commit it tomorrow unless others have further comments. It also occurred to me that array temporaries aren't _required_ here (for arrays of derived type components), but in the general case with a type with differently sized components, the stride wouldn't be a multiple of the component's type's size. Is it possible in principle to have an arbitrary stride? It is possible to have an arbitrary (fixed, non-unit) stride, but it is not always taken advantage of. If you take the last version of the testcase and compile with option -fdump-tree-original, you can see that the cases commented with "no temp needed" actually create a suitable descriptor. E.g. call print_poly (uu(2,2::2)) becomes: { struct __class__STAR_1_0t class.28; struct array01_integer(kind=4) parm.29; class.28._vptr = (struct __vtype__STAR * {ref-all}) &__vtab_INTEGER_4_; parm.29.span = 4; parm.29.dtype = {.elem_len=4, .version=0, .rank=1, .type=1}; parm.29.dim[0].lbound = 1; parm.29.dim[0].ubound = 3; parm.29.dim[0].stride = 10; parm.29.data = (void *) &uu[6]; parm.29.offset = -10; class.28._data = parm.29; class.28._len = 0; print_poly (&class.28); } Since we know that 'uu' is a contiguous array, we can calculate the stride (10) for the 1-d section. The case of the section of the character array is quite similar, but the variant with the substring reference would need further work to avoid the temporary. (It would be possible.) But as you say, the general case, which may involve types/classes, does not map to a simple descriptor. Thanks for your patch! Harald
Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
On 2/19/24 16:19, Peter Hill wrote: Hi Harald, Thanks for your help, please see the updated and signed-off patch below. Pushed: https://gcc.gnu.org/g:14ba8d5b87acd5f91ab8b8c02165a0fd53dcc2f2
Re: [PATCH] Fix fortran/PR114024
On 2/21/24 20:41, Jerry D wrote: On 2/21/24 10:30 AM, Steve Kargl wrote: I have attached a patch to PR114024, see https://gcc.gnu.org/pipermail/gcc-bugs/2024-February/854651.html The patch contains a new testcase and passes regression testing on x86_64-*-freebsd. Could someone castr an eye over the patch and commit it? Hi Steve, I looked it over and looks reasonable. I will try to apply it next few days and test here. If OK, I will commit. Jerry Actually the patch has two issues: - a minor one: a new front-end memleak which can be avoided by using either gfc_replace_expr (see its other uses) Hint: try valgrind on f951 - it still fails on the following code, because the traversal of the refs is incomplete / wrong: program foo implicit none complex :: cmp(3) real, pointer :: pp(:) class(*), allocatable :: uu(:) type t real :: re real :: im end type t type u type(t) :: tt(3) end type u type(u) :: cc cmp = (3.45,6.78) cc% tt% re = cmp% re cc% tt% im = cmp% im allocate (pp, source = cc% tt% im) ! ICE print *, pp allocate (uu, source = cc% tt% im) ! ICE end This still crashes for me for the indicated cases. Harald
Re: [PATCH] Fix fortran/PR114024
On 2/21/24 22:00, Steve Kargl wrote: Unfortunately, valgrind does not work on AMD FX-8350 cpu. Do you mean valgrind does not work at all? For gcc, you need to configure --enable-valgrind-annotations to not get bogus warnings. memleak vs ICE. I think I'll take one over the other. Probably need to free code->expr3 before the copy. Yep. I tried gfc_replace_expr in an earlier patch. It did not work. - it still fails on the following code, because the traversal of the refs is incomplete / wrong: program foo implicit none complex :: cmp(3) real, pointer :: pp(:) class(*), allocatable :: uu(:) type t real :: re real :: im end type t type u type(t) :: tt(3) end type u type(u) :: cc cmp = (3.45,6.78) cc% tt% re = cmp% re cc% tt% im = cmp% im allocate (pp, source = cc% tt% im) ! ICE cc%tt%im isn't a complex-part-ref, so this seems to be a different (maybe related) issue. Does the code compile with 'source = (cc%tt%im)'? If so, perhaps, detecting a component reference and doing the simply wrapping with parentheses can be done. Yes, that's why I tried to make up the above example. I think %re and %im are not too special, they work here pretty much like component refs elsewhere. print *, pp allocate (uu, source = cc% tt% im) ! ICE Ditto. Not to mention I know nothing about the implementation of CLASS in gfortran. You can ignore this one for now. It works if one places parens around the source expr as for the other cases. Harald
Re: [PATCH] Fix fortran/PR114024
Hi Steve! On 2/22/24 01:52, Steve Kargl wrote: On Wed, Feb 21, 2024 at 01:42:32PM -0800, Steve Kargl wrote: On Wed, Feb 21, 2024 at 10:20:43PM +0100, Harald Anlauf wrote: On 2/21/24 22:00, Steve Kargl wrote: memleak vs ICE. I think I'll take one over the other. Probably need to free code->expr3 before the copy. Yep. I tried gfc_replace_expr in an earlier patch. It did not work. I tried freeing code->expr3 before assigning the new expression. That leads to % gfcx -c ~/gcc/gccx/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 pid 69473 comm f951 has trashed its stack, killing gfortran: internal compiler error: Illegal instruction signal terminated program f951 Right. I also don't see what the lifetimes of the expressions are. But is the gfc_copy_expr really needed? Wouldn't the following suffice? code->expr3 = gfc_get_parentheses (code->expr3); If I don't free code->expr3 but simply assign the new expression from gfc_get_parentheses(), your example now compiles are executes are expected. It now allocate_with_source_28.f90. Caveat: I don't know how to test the CLASS uu. - it still fails on the following code, because the traversal of the refs is incomplete / wrong: program foo implicit none complex :: cmp(3) real, pointer :: pp(:) class(*), allocatable :: uu(:) type t real :: re real :: im end type t type u type(t) :: tt(3) end type u type(u) :: cc cmp = (3.45,6.78) cc% tt% re = cmp% re cc% tt% im = cmp% im allocate (pp, source = cc% tt% im) ! ICE cc%tt%im isn't a complex-part-ref, so this seems to be a different (maybe related) issue. Does the code compile with 'source = (cc%tt%im)'? If so, perhaps, detecting a component reference and doing the simply wrapping with parentheses can be done. Yes, that's why I tried to make up the above example. I think %re and %im are not too special, they work here pretty much like component refs elsewhere. I see. The %re and %im complex-part-ref correspond to ref->u.i == INQUIRY_RE and INQUIRY_IM, respectively. A part-ref for a user-defined type doesn't have an INQUIRY_xxx, so we'll need to see if there is a way to easily identify, e.g., cc%tt%re from your testcase. The attach patch uses ref->type == REF_COMPONENT to deal with the above code. I actually wanted to draw your attention away from the real/complex stuff, because that is not really the point. When do we actually need to enforce the parentheses? I tried the following, and it seems to work: if (code->expr3->expr_type == EXPR_VARIABLE && is_subref_array (code->expr3)) code->expr3 = gfc_get_parentheses (code->expr3); (Beware: this is not regtested!) On the positive side, it not only seems to fix the cases in question, but also substring references etc., like the following: program foo implicit none complex :: cmp(3) = (3.45,6.78) real, pointer :: pp(:) integer, allocatable :: aa(:) class(*), allocatable :: uu(:), vv(:) type t ! pseudo "complex" type real :: re real :: im end type t type ci ! "complex integer" type integer :: re integer :: im end type ci type u type(t) :: tt(3) type(ci) :: ii(3) end type u type(u) :: cc character(3) :: str(3) = ["abc","def","ghi"] character(:), allocatable :: ac(:) allocate (ac, source=str(1::2)(2:3)) print *, str(1::2)(2:3) call my_print (ac) cc% tt% re = cmp% re cc% tt% im = cmp% im cc% ii% re = nint (cmp% re) cc% ii% im = nint (cmp% im) print *, "re=", cc% tt% re print *, "im=", cc% tt% im allocate (pp, source = cc% tt% re) print *, pp allocate (uu, source = cc% tt% im) call my_print (uu) allocate (vv, source = cc% ii% im) call my_print (vv) contains subroutine my_print (x) class(*), intent(in) :: x(:) select type (x) type is (real) print *, "'real':", x type is (integer) print *, "'integer':", x type is (character(*)) print *, "'character':", x end select end subroutine my_print end Cheers, Harald
Re: [PATCH] Fix fortran/PR114024
On 2/22/24 22:01, Steve Kargl wrote: On Thu, Feb 22, 2024 at 09:22:37PM +0100, Harald Anlauf wrote: On the positive side, it not only seems to fix the cases in question, but also substring references etc., like the following: If the above passes a regression test, then by all means we should use it. I did not consider the substring case. Even if unneeded parentheses are inserted, which may cause generation of a temporary variable, I hope users are not using 'allocate(x,source=z%re)' is some deeply nested crazy loops structure. First thing is code correctness. There are cases where the allocation shall preserve the array bounds, which is where we must avoid the parentheses at all cost. But these cases should be very limited. (There are some code comments/TODOs regarding this and an open PR by Tobias(?)). The cases we are currently discussing are even requiring(!) the resetting of the lower bounds to 1, so your suggestion to enforce parentheses does not look unreasonable. BTW: If someone uses allocate in a tight loop, he/she deserves to be punished anyway... BTW, my patch and I suspect your improved patch also fixes 'allocate(x,mold=z%re)'. Consider, complex z(3) real, allocatable :: x(:) z = 42ha allocate(x, mold=z%re) print *, size(x) end % gfortran13 -o z a.f90 a.f90:9:25: 9 |allocate(x, mold=z%re) | 1 internal compiler error: in retrieve_last_ref, at fortran/trans-array.cc:6070 0x247d7a679 __libc_start1 /usr/src/lib/libc/csu/libc_start1.c:157 % gfcx -o z a.f90 && ./z 3 Nice! I completely forgot about MOLD... So the only missing pieces are a really comprehensive testcase and successful regtests... Cheers, Harald
[PATCH, v2] Fix fortran/PR114024
Hi Steve, all, here's an updated patch with an enhanced testcase that also checks MOLD= besides SOURCE=. Regtested on x86_64-pc-linux-gnu. Is it OK for mainline? Cheers, Harald On 2/22/24 22:32, Harald Anlauf wrote: On 2/22/24 22:01, Steve Kargl wrote: BTW, my patch and I suspect your improved patch also fixes 'allocate(x,mold=z%re)'. Consider, complex z(3) real, allocatable :: x(:) z = 42ha allocate(x, mold=z%re) print *, size(x) end % gfortran13 -o z a.f90 a.f90:9:25: 9 | allocate(x, mold=z%re) | 1 internal compiler error: in retrieve_last_ref, at fortran/trans-array.cc:6070 0x247d7a679 __libc_start1 /usr/src/lib/libc/csu/libc_start1.c:157 % gfcx -o z a.f90 && ./z 3 Nice! I completely forgot about MOLD... So the only missing pieces are a really comprehensive testcase and successful regtests... From a176c2f44f812d82aeb430fadf23ab4b6dd5bd65 Mon Sep 17 00:00:00 2001 From: Steve Kargl Date: Fri, 23 Feb 2024 22:05:04 +0100 Subject: [PATCH] Fortran: ALLOCATE statement, SOURCE/MOLD expressions with subrefs [PR114024] PR fortran/114024 gcc/fortran/ChangeLog: * trans-stmt.cc (gfc_trans_allocate): When a source expression has substring references, part-refs, or %re/%im inquiries, wrap the entity in parentheses to force evaluation of the expression. gcc/testsuite/ChangeLog: * gfortran.dg/allocate_with_source_27.f90: New test. * gfortran.dg/allocate_with_source_28.f90: New test. Co-Authored-By: Harald Anlauf --- gcc/fortran/trans-stmt.cc | 10 ++- .../gfortran.dg/allocate_with_source_27.f90 | 20 + .../gfortran.dg/allocate_with_source_28.f90 | 90 +++ 3 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc index 5247d3d39d7..e09828e218b 100644 --- a/gcc/fortran/trans-stmt.cc +++ b/gcc/fortran/trans-stmt.cc @@ -6355,8 +6355,14 @@ gfc_trans_allocate (gfc_code * code, gfc_omp_namelist *omp_allocate) vtab_needed = (al->expr->ts.type == BT_CLASS); gfc_init_se (&se, NULL); - /* When expr3 is a variable, i.e., a very simple expression, - then convert it once here. */ + /* When expr3 is a variable, i.e., a very simple expression, then + convert it once here. If one has a source expression that has + substring references, part-refs, or %re/%im inquiries, wrap the + entity in parentheses to force evaluation of the expression. */ + if (code->expr3->expr_type == EXPR_VARIABLE + && is_subref_array (code->expr3)) + code->expr3 = gfc_get_parentheses (code->expr3); + if (code->expr3->expr_type == EXPR_VARIABLE || code->expr3->expr_type == EXPR_ARRAY || code->expr3->expr_type == EXPR_CONSTANT) diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 new file mode 100644 index 000..d0f0f3c4a84 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 @@ -0,0 +1,20 @@ +! +! { dg-do run } +! +! fortran/PR114024 +! https://github.com/fujitsu/compiler-test-suite +! Modified from Fortran/0093/0093_0130.f90 +! +program foo + implicit none + complex :: cmp(3) + real, allocatable :: xx(:), yy(:), zz(:) + cmp = (3., 6.78) + allocate(xx, source = cmp%re) ! This caused an ICE. + allocate(yy, source = cmp(1:3)%re) ! This caused an ICE. + allocate(zz, source = (cmp%re)) + if (any(xx /= [3., 3., 3.])) stop 1 + if (any(yy /= [3., 3., 3.])) stop 2 + if (any(zz /= [3., 3., 3.])) stop 3 +end program foo + diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 new file mode 100644 index 000..976c567cf22 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 @@ -0,0 +1,90 @@ +! { dg-do run } +! +! PR fortran/114024 + +program foo + implicit none + complex :: cmp(3) = (3.,4.) + type ci ! pseudo "complex integer" type + integer :: re + integer :: im + end type ci + type cr ! pseudo "complex" type + real :: re + real :: im + end type cr + type u + type(ci) :: ii(3) + type(cr) :: rr(3) + end type u + type(u) :: cc + + cc% ii% re = nint (cmp% re) + cc% ii% im = nint (cmp% im) + cc% rr% re = cmp% re + cc% rr% im = cmp% im + + call test_substring () + call test_int_real () + call test_poly () + +contains + + subroutine test_substring () +character(4) :: str(3) = ["abcd","efgh","ijkl"] +character(:), allocatable :: ac(:) +allocate (ac, source=str(1::2)(2:4)) +if (size (ac) /= 2 .or. len (ac) /= 3) stop 11 +if (ac(
[PATCH] Fortran: do not evaluate polymorphic functions twice in assignment [PR114012]
Dear all, the attached simple patch fixes an issue where we evaluated polymorphic functions twice in assignments: once for the _data component, and once for the _vptr. Using save_expr prevents the double evaluation. Regtested on x86_64-pc-linux-gnu. OK for mainline? And a backport to 13-branch after some delay? Thanks, Harald From 7a16143448ee21b716b54a94f83f9ee477af1b63 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 25 Feb 2024 21:18:23 +0100 Subject: [PATCH] Fortran: do not evaluate polymorphic functions twice in assignment [PR114012] PR fortran/114012 gcc/fortran/ChangeLog: * trans-expr.cc (gfc_conv_procedure_call): Evaluate non-trivial arguments just once before assigning to an unlimited polymorphic dummy variable. gcc/testsuite/ChangeLog: * gfortran.dg/pr114012.f90: New test. --- gcc/fortran/trans-expr.cc | 4 ++ gcc/testsuite/gfortran.dg/pr114012.f90 | 81 ++ 2 files changed, 85 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr114012.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 118dfd7c9b2..d63c304661a 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6691,6 +6691,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, { tree efield; + /* Evaluate arguments just once. */ + if (e->expr_type != EXPR_VARIABLE) +parmse.expr = save_expr (parmse.expr); + /* Set the _data field. */ tmp = gfc_class_data_get (var); efield = fold_convert (TREE_TYPE (tmp), diff --git a/gcc/testsuite/gfortran.dg/pr114012.f90 b/gcc/testsuite/gfortran.dg/pr114012.f90 new file mode 100644 index 000..9dbb031c664 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr114012.f90 @@ -0,0 +1,81 @@ +! { dg-do run } +! PR fortran/114012 +! +! Polymorphic functions were evaluated twice in assignment + +program test + implicit none + + type :: custom_int + integer :: val = 2 + end type + + interface assignment(=) + procedure assign + end interface + interface operator(-) + procedure neg + end interface + + type(custom_int) :: i + integer :: count_assign, count_neg + + count_assign = 0 + count_neg= 0 + + i = 1 + if (count_assign /= 1 .or. count_neg /= 0) stop 1 + + i = -i + if (count_assign /= 2 .or. count_neg /= 1) stop 2 + if (i% val /= -1) stop 3 + + i = neg(i) + if (count_assign /= 3 .or. count_neg /= 2) stop 4 + if (i% val /= 1) stop 5 + + i = (neg(i)) + if (count_assign /= 4 .or. count_neg /= 3) stop 6 + if (i% val /= -1) stop 7 + + i = - neg(i) + if (count_assign /= 5 .or. count_neg /= 5) stop 8 + if (i% val /= -1) stop 9 + +contains + + subroutine assign (field, val) +type(custom_int), intent(out) :: field +class(*), intent(in) :: val + +count_assign = count_assign + 1 + +select type (val) +type is (integer) +! print *, " in assign(integer)", field%val, val + field%val = val +type is (custom_int) +! print *, " in assign(custom)", field%val, val%val + field%val = val%val +class default + error stop +end select + + end subroutine assign + + function neg (input_field) result(output_field) +type(custom_int), intent(in), target :: input_field +class(custom_int), allocatable :: output_field +allocate (custom_int :: output_field) + +count_neg = count_neg + 1 + +select type (output_field) +type is (custom_int) +! print *, " in neg", output_field%val, input_field%val + output_field%val = -input_field%val +class default + error stop +end select + end function neg +end program test -- 2.35.3
Re: [patch, libgfortran] PR105456 Child I/O does not propage iostat
Hi Jerry, On 2/22/24 20:11, Jerry D wrote: Hi all, The attached fix adds a check for an error condition from a UDDTIO procedure in the case where there is no actual underlying error, but the user defines an error by setting the iostat variable manually before returning to the parent READ. the libgfortran fix LGTM. Regarding the testcase code, the following looks like you left some debugging code in it: + rewind (10) + read (10,*) x + print *, myerror, mymessage + write (*,'(10(A))') "Read: '",x%ch,"'" myerror and mymessage are never set and never tested. I suggest to either remove them or to enhance the testcase e.g. like rewind (10) read (10,*,iostat=myerror,iomsg=mymessage) x if (myerror /= 42 .or. mymessage /= "The users message") stop 1 rewind (10) read (10,*) x write (*,'(10(A))') "Read: '",x%ch,"'" I'll leave that up to you. I did not address the case of a formatted WRITE or unformatted READ/WRITE until I get some feedback on the approach. If this approach is OK I would like to commit and then do a separate patch for the cases I just mentioned. I haven't thought about this long enough, but I do not anything wrong with your patch. Feedback appreciated. Regression tested on x86_64. OK for trunk? This is OK. Thanks, Harald Jerry Author: Jerry DeLisle Date: Thu Feb 22 10:48:39 2024 -0800 libgfortran: Propagate user defined iostat and iomsg. PR libfortran/105456 libgfortran/ChangeLog: * io/list_read.c (list_formatted_read_scalar): Add checks for the case where a user defines their own error codes and error messages and generate the runtime error. gcc/testsuite/ChangeLog: * gfortran.dg/pr105456.f90: New test.
[PATCH] Fortran testsuite: fix invalid Fortran in testcase
Dear all, the attached patch fixes invalid Fortran in testcase gfortran.dg/pr101026.f, which might prohibit progress in fixing pr111781. (Note that the testcase was for a tree-optimizer issue, not the Fortran frontend.) OK for mainline? Will commit within 24h unless there are comments. Thanks, Harald From 75724b6b42a1c46383d8e6deedbfb8d2ebd0fa12 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 27 Feb 2024 21:51:53 +0100 Subject: [PATCH] Fortran testsuite: fix invalid Fortran in testcase gcc/testsuite/ChangeLog: * gfortran.dg/pr101026.f: Let variables used in specification expression be passed as dummy arguments --- gcc/testsuite/gfortran.dg/pr101026.f | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gfortran.dg/pr101026.f b/gcc/testsuite/gfortran.dg/pr101026.f index 9576d8802ca..e05e21c898a 100644 --- a/gcc/testsuite/gfortran.dg/pr101026.f +++ b/gcc/testsuite/gfortran.dg/pr101026.f @@ -1,6 +1,6 @@ ! { dg-do compile } ! { dg-options "-Ofast -frounding-math" } - SUBROUTINE PASSB4 (CC,CH) + SUBROUTINE PASSB4 (CC,CH,IDO,L1) DIMENSION CC(IDO,4,L1), CH(IDO,L1,*) DO 103 I=2,IDO,2 TI4 = CC0-CC(I,4,K) -- 2.35.3
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Hi Alex, this is now mostly correct, with the following exceptions: First, you should notice that the formatting of the commit message, when checked using "git gcc-verify", needs minor corrections. You will be guided how to fix this yourself. Second, testcase pdt_37.f03 has an undeclared dummy argument, which can be detected by adding "implicit none" (I usually use that whenever implicit typing is not wanted explicitly). I would get: pdt_37.f03:33:47: 33 | subroutine assumed_len_param_ptr(this, that) | 1 Error: Symbol 'that' at (1) has no IMPLICIT type; did you mean 'this'? I assume you want to uncomment the declaration of dummy 'that'. Third, I still see a - minor - indentation/tabbing/space issue here: diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 44f89f6afb4..852e0820e6a 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc [...] + if ( resolve_bindings_derived->attr.pdt_template + && gfc_pdt_is_instance_of (resolve_bindings_derived, + CLASS_DATA (me_arg)->ts.u.derived) + && (me_arg->param_list != NULL) + && (gfc_spec_list_type (me_arg->param_list, +CLASS_DATA(me_arg)->ts.u.derived) +!= SPEC_ASSUMED)) OK with the above fixed. Thanks for the patch! Harald On 2/28/24 07:24, Alexander Westbrooks wrote: Harald, Jerry helped me figure out my editor settings so that I could fix whitespace and formatting issues in my code. With my editor configured correctly, I saw that my code was not conforming to coding standards as I previously thought it was. I have fixed those things and updated my patch. Thank you for your patience. Let me know if this is okay to push to the trunk. Thanks, Alexander Westbrooks On Sun, Feb 25, 2024 at 2:40 PM Alexander Westbrooks wrote: Harald, Thank you for reviewing my code. I've been doing research and debugging to investigate the error thrown by Intel and NAG for the deferred parameter in the dummy variable declaration. I found where the problem was and added the fix as part of my patch. I've attached the patch as a file, which also includes your feedback and suggested fixes. I've updated the test case pdt_37.f03 to check for the POINTER or ALLOCATABLE error as you suggested. All regression tests pass, including the new ones, after including the fix for the POINTER or ALLOCATABLE error for CLASS declarations of PDTs when deferred length parameters are used. This was tested on WSL 2, with Ubuntu 20.04 distro. Is this okay to push to the trunk? Thanks, Alexander Westbrooks On Sun, Feb 11, 2024 at 2:11 PM Harald Anlauf wrote: Hi Alex, I've been unable to apply your patch to my local trunk, likely due to whitespace issues my newsreader handles differently from your site. I see it inline instead of attached. A few general remarks: Please follow the general recommendation regarding style if possible, see https://www.gnu.org/prep/standards/standards.html#Formatting regarding formatting/whitespace use (5.1) and comments (5.2) Also, when an error message text spans multiple lines, please place the whitespace at the end of a line, not at the beginning of the new one: + if ( resolve_bindings_derived->attr.pdt_template && + !gfc_pdt_is_instance_of(resolve_bindings_derived, + CLASS_DATA(me_arg)->ts.u.derived)) +{ + gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of" +" the parametric derived-type %qs", me_arg->name, proc->name, gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of " "the parametric derived-type %qs", me_arg->name, proc->name, +me_arg->name, &where, resolve_bindings_derived->name); + goto error; +} The following change is almost unreadable: the lnegthy comment is split over three parts and almost hides the code. Couldn't this be combined into one comment before the function? diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index fddf68f8398..11f4bac0415 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc @@ -5172,6 +5172,35 @@ gfc_type_is_extension_of (gfc_symbol *t1, gfc_symbol *t2) return gfc_compare_derived_types (t1, t2); } +/* Check if a parameterized derived type t2 is an instance of a PDT template t1 */ + +bool +gfc_pdt_is_instance_of(gfc_symbol *t1, gfc_symbol *t2) +{ + if ( !t1->attr.pdt_template || !t2->attr.pdt_type ) +return false; + + /* +in decl.cc, gfc_get_pdt_instance, a pdt instance is given a 3 character prefix "Pdt", followed +by an underscore list of the kind parameters, up to a maximum of 8. + +So to check if a PDT Type
[PATCH] Fortran: improve checks of NULL without MOLD as actual argument [PR104819]
Dear all, here's a first patch addressing issues with NULL as actual argument: if the dummy is assumed-rank or assumed length, MOLD shall be present. There is also an interp on interoperability of c_sizeof and NULL pointers, for which we have a partially incorrect testcase (gfortran.dg/pr101329.f90) which gets fixed. See https://j3-fortran.org/doc/year/22/22-101r1.txt for more. Furthermore, nested NULL()s are now handled. Regtested on x86_64-pc-linux-gnu. OK for mainline? I consider this part as safe and would like to backport to 13-branch. Objections? Thanks, Harald From ce7199b16872b3014be68744329a8f19ddd64b05 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 29 Feb 2024 21:43:53 +0100 Subject: [PATCH] Fortran: improve checks of NULL without MOLD as actual argument [PR104819] gcc/fortran/ChangeLog: PR fortran/104819 * check.cc (gfc_check_null): Handle nested NULL()s. (is_c_interoperable): Check for MOLD argument of NULL() as part of the interoperability check. * interface.cc (gfc_compare_actual_formal): Extend checks for NULL() actual arguments for presence of MOLD argument when required by Interp J3/22-146. gcc/testsuite/ChangeLog: PR fortran/104819 * gfortran.dg/pr101329.f90: Adjust testcase to conform to interp. * gfortran.dg/null_actual_4.f90: New test. --- gcc/fortran/check.cc| 5 ++- gcc/fortran/interface.cc| 30 ++ gcc/testsuite/gfortran.dg/null_actual_4.f90 | 35 + gcc/testsuite/gfortran.dg/pr101329.f90 | 4 +-- 4 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/null_actual_4.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index d661cf37f01..db74dcf3f40 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -4384,6 +4384,9 @@ gfc_check_null (gfc_expr *mold) if (mold == NULL) return true; + if (mold->expr_type == EXPR_NULL) +return true; + if (!variable_check (mold, 0, true)) return false; @@ -5216,7 +5219,7 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr) { *msg = NULL; - if (expr->expr_type == EXPR_NULL) + if (expr->expr_type == EXPR_NULL && expr->ts.type == BT_UNKNOWN) { *msg = "NULL() is not interoperable"; return false; diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index 231f2f252af..64b90550be2 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -3296,6 +3296,36 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, && a->expr->ts.type != BT_ASSUMED) gfc_find_vtab (&a->expr->ts); + /* Interp J3/22-146: + "If the context of the reference to NULL is an + corresponding to an dummy argument, MOLD shall be + present." */ + if (a->expr->expr_type == EXPR_NULL + && a->expr->ts.type == BT_UNKNOWN + && f->sym->as + && f->sym->as->type == AS_ASSUMED_RANK) + { + gfc_error ("Intrinsic % without % argument at %L " + "passed to assumed-rank dummy %qs", + &a->expr->where, f->sym->name); + ok = false; + goto match; + } + + if (a->expr->expr_type == EXPR_NULL + && a->expr->ts.type == BT_UNKNOWN + && f->sym->ts.type == BT_CHARACTER + && !f->sym->ts.deferred + && f->sym->ts.u.cl + && f->sym->ts.u.cl->length == NULL) + { + gfc_error ("Intrinsic % without % argument at %L " + "passed to assumed-length dummy %qs", + &a->expr->where, f->sym->name); + ok = false; + goto match; + } + if (a->expr->expr_type == EXPR_NULL && ((f->sym->ts.type != BT_CLASS && !f->sym->attr.pointer && (f->sym->attr.allocatable || !f->sym->attr.optional diff --git a/gcc/testsuite/gfortran.dg/null_actual_4.f90 b/gcc/testsuite/gfortran.dg/null_actual_4.f90 new file mode 100644 index 000..e03d5c8f7de --- /dev/null +++ b/gcc/testsuite/gfortran.dg/null_actual_4.f90 @@ -0,0 +1,35 @@ +! { dg-do compile } +! PR fortran/104819 +! +! Reject NULL without MOLD as actual to an assumed-rank dummy. +! See also interpretation request at +! https://j3-fortran.org/doc/year/22/22-101r1.txt +! +! Test nested NULL() + +program p + implicit none + integer, pointer :: a, a3(:,:,:) + character(10), pointer :: c + + call foo (a) + call foo (a3) + call foo (null (a)) + call foo (null (a3)) + call foo (null (null (a))) ! Valid: nested NULL()s + call foo (null (null (a3))) ! Valid: nested NULL()s + call foo (null ()) ! { dg-error "passed to assumed-rank dummy" } + + call str (null (c)) + call str (null (null (c))) + call str (null ()) ! { dg-error "passed to ass
Re: [Patch, fortran PR89645/99065 No IMPLICIT type error with: ASSOCIATE( X => function() )
Hi Paul, welcome back! On 3/3/24 17:04, Paul Richard Thomas wrote: Hi Harald, Please find an updated version of the patch that rolls in Steve's patch for PR114141, fixes unlimited polymorphic function selectors and cures the memory leaks. I apologise for not working on this sooner but, as I informed you, I have been away for an extended trip to Australia. The chunks that fix PR114141 are picked out in comment 14 to the PR and the cures to the problems that you found in the first review are found at trans-stmt.cc:2047-49. Regtests fine. OK for trunk, bearing in mind that most of the patch is ring fenced by the inferred_type flag? I would say that it is almost fine. Two things that I found: - Testcase associate_65.f90 does not compile with -std=f2023, because IMAG is a GNU extension, while AIMAG is the standard version. Could you please adjust that? - I think the handling of parentheses and functions returning pointers does not work correctly. Consider: program paul implicit none type t integer :: i end type t type(t), pointer :: p(:) allocate (p(-3:3)) associate (q => p) print *, lbound (q), ubound (q) ! Should print -3 3 (OK) end associate associate (q => set_ptr()) print *, lbound (q), ubound (q) ! Should print -3 3 (OK) end associate associate (q => (p)) print *, lbound (q), ubound (q) ! Should print 1 7 (OK) end associate associate (q => (set_ptr())) ! <- are these parentheses lost? print *, lbound (q), ubound (q) ! Should print 1 7 end associate contains function set_ptr () result (res) type(t), pointer :: res(:) res => p end function set_ptr end While the first three variants give the right bounds, the last version - after applying your patch - is mishandled and the testcase now prints: -3 3 -3 3 1 7 -3 3 Both NAG and Intel support my expectation, namely that the last line should equal the next-to-last. Can you recheck the logic for that particular corner case? With these points addressed, your patch is OK from my side. Thanks for the patch and your endurance! Harald Cheers Paul On Mon, 8 Jan 2024 at 21:53, Harald Anlauf wrote: Hi Paul, your patch looks already very impressive! Regarding the patch as is, I am still trying to grok it, even with your explanations at hand... While the testcase works as advertised, I noticed that it exhibits a runtime memleak that occurs for (likely) each case where the associate target is an allocatable, class-valued function result. I tried to produce a minimal testcase using class(*), which apparently is not handled by your patch (it ICEs for me): program p implicit none class(*), allocatable :: x(:) x = foo() call prt (x) deallocate (x) ! up to here no memleak... associate (var => foo()) call prt (var) end associate contains function foo() result(res) class(*), allocatable :: res(:) res = [42] end function foo subroutine prt (x) class(*), intent(in) :: x(:) select type (x) type is (integer) print *, x class default stop 99 end select end subroutine prt end Traceback (truncated): foo.f90:9:18: 9 | call prt (var) | 1 internal compiler error: tree check: expected record_type or union_type or qual_union_type, have function_type in gfc_class_len_get, at fortran/trans-expr.cc:271 0x19fd5d5 tree_check_failed(tree_node const*, char const*, int, char const*, ...) ../../gcc-trunk/gcc/tree.cc:8952 0xe1562d tree_check3(tree_node*, char const*, int, char const*, tree_code, tree_code, tree_code) ../../gcc-trunk/gcc/tree.h:3652 0xe3e264 gfc_class_len_get(tree_node*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:271 0xecda48 trans_associate_var ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2325 0xecdd09 gfc_trans_block_construct(gfc_code*) ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2383 [...] I don't see anything wrong with it: NAG groks it, like Nvidia and Flang, while Intel crashes at runtime. Can you have another brief look? Thanks, Harald On 1/6/24 18:26, Paul Richard Thomas wrote: These PRs come about because of gfortran's single pass parsing. If the function in the title is parsed after the associate construct, then its type and rank are not known. The point at which this becomes a problem is when expressions within the associate block are parsed. primary.cc (gfc_match_varspec) could already deal with intrinsic types and so component references were the trigger for the problem. The two major parts of this patch are the fixup needed in gfc_match_varspec and the resolution of expressions with references in resolve.cc (gfc_fixup_inferred_type_refs). The former relies on the two new functions in symbol.cc to search for derived types with an appropri
[PATCH] Fortran: error recovery while simplifying expressions [PR103707,PR106987]
Dear all, error recovery on arithmetic errors during simplification has bugged me for a long time, especially since the occurence of ICEs depended on whether -frange-check is specified or not, whether array ctors were involved, etc. I've now come up with the attached patch that classifies the arithmetic result codes into "hard" and "soft" errors. A "soft" error means that it is an overflow or other exception (e.g. NaN) that is ignored with -fno-range-check. After the patch, a soft error will not stop simplification (a hard one will), and error status will be passed along. I took this opportunity to change the emitted error for division by zero for real and complex division dependent on whether the numerator is regular or not. This makes e.g. (0.)/0 a NaN and now says so, in accordance with some other brands. Regtested on x86_64-pc-linux-gnu. OK for mainline? Other comments? Thanks, Harald From d9b87bea6af77fbc794e1f21cfecb0468c68cb72 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 5 Mar 2024 21:54:26 +0100 Subject: [PATCH] Fortran: error recovery while simplifying expressions [PR103707,PR106987] When an exception is encountered during simplification of arithmetic expressions, the result may depend on whether range-checking is active (-frange-check) or not. However, the code path in the front-end should stay the same for "soft" errors for which the exception is triggered by the check, while "hard" errors should always terminate the simplification, so that error recovery is independent of the flag. Separation of arithmetic error codes into "hard" and "soft" errors shall be done consistently via is_hard_arith_error(). PR fortran/103707 PR fortran/106987 gcc/fortran/ChangeLog: * arith.cc (is_hard_arith_error): New helper function to determine whether an arithmetic error is "hard" or not. (check_result): Use it. (gfc_arith_divide): Set "Division by zero" only for regular numerators of real and complex divisions. (reduce_unary): Use is_hard_arith_error to determine whether a hard or (recoverable) soft error was encountered. Terminate immediately on hard error, otherwise remember code of first soft error. (reduce_binary_ac): Likewise. (reduce_binary_ca): Likewise. (reduce_binary_aa): Likewise. gcc/testsuite/ChangeLog: * gfortran.dg/pr99350.f90: * gfortran.dg/arithmetic_overflow_3.f90: New test. --- gcc/fortran/arith.cc | 134 -- .../gfortran.dg/arithmetic_overflow_3.f90 | 48 +++ gcc/testsuite/gfortran.dg/pr99350.f90 | 2 +- 3 files changed, 143 insertions(+), 41 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/arithmetic_overflow_3.f90 diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index d17d1aaa1d9..b373c25e5e1 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -130,6 +130,30 @@ gfc_arith_error (arith code) } +/* Check if a certain arithmetic error code is severe enough to prevent + further simplification, as opposed to errors thrown by the range check + (e.g. overflow) or arithmetic exceptions that are tolerated with + -fno-range-check. */ + +static bool +is_hard_arith_error (arith code) +{ + switch (code) +{ +case ARITH_OK: +case ARITH_OVERFLOW: +case ARITH_UNDERFLOW: +case ARITH_NAN: +case ARITH_DIV0: +case ARITH_ASYMMETRIC: + return false; + +default: + return true; +} +} + + /* Get things ready to do math. */ void @@ -579,10 +603,10 @@ check_result (arith rc, gfc_expr *x, gfc_expr *r, gfc_expr **rp) val = ARITH_OK; } - if (val == ARITH_OK || val == ARITH_OVERFLOW) -*rp = r; - else + if (is_hard_arith_error (val)) gfc_free_expr (r); + else +*rp = r; return val; } @@ -792,23 +816,26 @@ gfc_arith_divide (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp) break; case BT_REAL: - if (mpfr_sgn (op2->value.real) == 0 && flag_range_check == 1) - { - rc = ARITH_DIV0; - break; - } + /* Set "Division by zero" only for regular numerator. */ + if (flag_range_check == 1 + && mpfr_zero_p (op2->value.real) + && mpfr_regular_p (op1->value.real)) + rc = ARITH_DIV0; mpfr_div (result->value.real, op1->value.real, op2->value.real, GFC_RND_MODE); break; case BT_COMPLEX: - if (mpc_cmp_si_si (op2->value.complex, 0, 0) == 0 - && flag_range_check == 1) - { - rc = ARITH_DIV0; - break; - } + /* Set "Division by zero" only for regular numerator. */ + if (flag_range_check == 1 + && mpfr_zero_p (mpc_realref (op2->value.complex)) + && mpfr_zero_p (mpc_imagref (op2->value.complex)) + && ((mpfr_regular_p (mpc_realref (op1->value.complex)) + && mpfr_number_p (mpc_imagref (op1->value.complex
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (&dtp->common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, on further thought, do we sanitize 'child_iomsg'? We pass it to snprintf as format. Wouldn't a strncpy be sufficient? Harald On 3/5/24 22:37, Harald Anlauf wrote: Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (&dtp->common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
Re: [PATCH] Fortran: error recovery while simplifying expressions [PR103707, PR106987]
Hi Paul, thanks for reviewing the patch, and your trust in me :-) Backporting to 13-branch seems easily feasible (needs another small queued backport on which this patch depends), but going further is definitely out of the question... Will wait a couple of weeks though. Harald On 3/6/24 11:51, Paul Richard Thomas wrote: Hi Harald, This all looks good to me. OK for mainline and, according to intestinal fortitude on your part, earlier branches. Thanks Paul On Tue, 5 Mar 2024 at 21:24, Harald Anlauf wrote: Dear all, error recovery on arithmetic errors during simplification has bugged me for a long time, especially since the occurence of ICEs depended on whether -frange-check is specified or not, whether array ctors were involved, etc. I've now come up with the attached patch that classifies the arithmetic result codes into "hard" and "soft" errors. A "soft" error means that it is an overflow or other exception (e.g. NaN) that is ignored with -fno-range-check. After the patch, a soft error will not stop simplification (a hard one will), and error status will be passed along. I took this opportunity to change the emitted error for division by zero for real and complex division dependent on whether the numerator is regular or not. This makes e.g. (0.)/0 a NaN and now says so, in accordance with some other brands. Regtested on x86_64-pc-linux-gnu. OK for mainline? Other comments? Thanks, Harald
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, can you please replace the user message in e.g. your new testcase pr105456-wf.f90 by say: piomsg="The users message containing % and %% and %s and other stuff" This behaves as expected with Intel, but dies horribly with gfortran after your patch! Cheers, Harald On 3/6/24 05:06, Jerry D wrote: On 3/5/24 1:51 PM, Harald Anlauf wrote: Hi Jerry, on further thought, do we sanitize 'child_iomsg'? We pass it to snprintf as format. Wouldn't a strncpy be sufficient? Harald Just to be safe I will bump char message[IOMSG_LEN] to char message[IOMSG_LEN + 1] This is like a C string vs a Fortran string length situation. snprintf guarantees we don't exceed the child_iomsg_len and null terminates it. I added 1 to: child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1 Because snprintf was chopping off the last character of the fortran string to put the null in. (zero based vs one based char array). I test this with a very long string which exceeded the length and then reduced it until I could see the desired end. I have not tried running a test case with sanitize. I did check with valgrind. I will try the sanitize flags to see if we get a problem. If not will push. Thanks for comments, Jerry - On 3/5/24 22:37, Harald Anlauf wrote: Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (&dtp->common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
[PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
Dear all, after playing for some time with NAG and Intel, and an off-list discussion with Jerry, I am getting more and more convinced that simpler runtime error messages (also simpler to parse by a human) are superior to awkward solutions. This is also what Intel does: use only the name of the array (component) in the message whose indices are out of bounds. (NAG's solution appears also inconsistent for nested derived types.) So no x%z, or x%_data, etc. in runtime error messages any more. Please give it a spin... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 1/30/24 11:46, Mikael Morin wrote: Le 30/01/2024 à 11:38, Mikael Morin a écrit : Another (easier) way to clarify the data reference would be rephrasing the message so that the array part is separate from the scalar part, like so (there are too many 'of', but I lack inspiration): Index '0' of dimension 1 of component 'zz' of element from 'x1%vv' below lower bound of 1 This has the same number of 'of' but sounds better maybe: Out of bounds accessing component 'zz' of element from 'x1%yy': index '0' of dimension 1 below lower bound of 1 From cdf3b197beed0ce1649661b2132643b54cbade8d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 10 Mar 2024 22:14:30 +0100 Subject: [PATCH] Fortran: use name of array component in runtime error message [PR30802] gcc/fortran/ChangeLog: PR fortran/30802 * trans-array.cc (trans_array_bound_check): Find name of component to use in runtime error message. (array_bound_check_elemental): Likewise. (gfc_conv_array_ref): Likewise. gcc/testsuite/ChangeLog: PR fortran/30802 * gfortran.dg/bounds_check_17.f90: Adjust dg-pattern. * gfortran.dg/bounds_check_fail_6.f90: Likewise. * gfortran.dg/pr92050.f90: Likewise. * gfortran.dg/bounds_check_fail_8.f90: New test. --- gcc/fortran/trans-array.cc| 60 +-- gcc/testsuite/gfortran.dg/bounds_check_17.f90 | 2 +- .../gfortran.dg/bounds_check_fail_6.f90 | 7 ++- .../gfortran.dg/bounds_check_fail_8.f90 | 48 +++ gcc/testsuite/gfortran.dg/pr92050.f90 | 2 +- 5 files changed, 83 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..9c62b070c50 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3497,6 +3497,8 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, tree descriptor; char *msg; const char * name = NULL; + gfc_expr *expr; + gfc_ref *ref; if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)) return index; @@ -3509,6 +3511,24 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, name = ss->info->expr->symtree->n.sym->name; gcc_assert (name != NULL); + /* When we have a component ref, get name of the array section. + Note that there can only be one part ref. */ + expr = ss->info->expr; + if (expr->ref && !compname) +{ + for (ref = expr->ref; ref; ref = ref->next) + { + /* Remember component name. */ + if (ref->type == REF_COMPONENT) + { + name = ref->u.c.component->name; + continue; + } + if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION) + break; + } +} + if (VAR_P (descriptor)) name = IDENTIFIER_POINTER (DECL_NAME (descriptor)); @@ -3574,29 +3594,20 @@ array_bound_check_elemental (gfc_se * se, gfc_ss * ss, gfc_expr * expr) gfc_array_ref *ar; gfc_ref *ref; gfc_symbol *sym; - char *var_name = NULL; - size_t len; + const char *var_name = NULL; int dim; if (expr->expr_type == EXPR_VARIABLE) { sym = expr->symtree->n.sym; - len = strlen (sym->name) + 1; - - for (ref = expr->ref; ref; ref = ref->next) - if (ref->type == REF_COMPONENT) - len += 2 + strlen (ref->u.c.component->name); - - var_name = XALLOCAVEC (char, len); - strcpy (var_name, sym->name); + var_name = sym->name; for (ref = expr->ref; ref; ref = ref->next) { - /* Append component name. */ + /* Get component name. */ if (ref->type == REF_COMPONENT) { - strcat (var_name, "%%"); - strcat (var_name, ref->u.c.component->name); + var_name = ref->u.c.component->name; continue; } @@ -4001,7 +4012,7 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr, gfc_se indexse; gfc_se tmpse; gfc_symbol * sym = expr->symtree->n.sym; - char *var_name = NULL; + const char *var_name = NULL; if (ar->dimen == 0) { @@ -4035,30 +4046,17 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr, if (gfc_option.rtcheck & GFC_RTCHECK_BOUN
[PATCH] Fortran: handle procedure pointer component in DT array [PR110826]
Dear all, the attached patch fixes an ICE-on-valid code when assigning a procedure pointer that is a component of a DT array and the function in question is array-valued. (The procedure pointer itself cannot be an array.) Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From a9be17cf987b796c49684cde2f20dac3839c736c Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 11 Mar 2024 22:05:51 +0100 Subject: [PATCH] Fortran: handle procedure pointer component in DT array [PR110826] gcc/fortran/ChangeLog: PR fortran/110826 * array.cc (gfc_array_dimen_size): When walking the ref chain of an array and the ultimate component is a procedure pointer, do not try to figure out its dimension even if it is a array-valued function. gcc/testsuite/ChangeLog: PR fortran/110826 * gfortran.dg/proc_ptr_comp_53.f90: New test. --- gcc/fortran/array.cc | 7 .../gfortran.dg/proc_ptr_comp_53.f90 | 41 +++ 2 files changed, 48 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc index 3a6e3a7c95b..e9934f1491b 100644 --- a/gcc/fortran/array.cc +++ b/gcc/fortran/array.cc @@ -2597,6 +2597,13 @@ gfc_array_dimen_size (gfc_expr *array, int dimen, mpz_t *result) case EXPR_FUNCTION: for (ref = array->ref; ref; ref = ref->next) { + /* Ultimate component is a procedure pointer. */ + if (ref->type == REF_COMPONENT + && !ref->next + && ref->u.c.component->attr.function + && IS_PROC_POINTER (ref->u.c.component)) + return false; + if (ref->type != REF_ARRAY) continue; diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 new file mode 100644 index 000..881ddd3558f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 @@ -0,0 +1,41 @@ +! { dg-do compile } +! PR fortran/110826 - procedure pointer component in DT array + +module m + implicit none + + type pp +procedure(func_template), pointer, nopass :: f =>null() + end type pp + + abstract interface + function func_template(state) result(dstate) + implicit none + real, dimension(:,:), intent(in) :: state + real, dimension(size(state,1), size(state,2)) :: dstate + end function + end interface + +contains + + function zero_state(state) result(dstate) +real, dimension(:,:), intent(in) :: state +real, dimension(size(state,1), size(state,2)) :: dstate +dstate = 0. + end function zero_state + +end module m + +program test_func_array + use m + implicit none + + real, dimension(4,6) :: state + type(pp) :: func_scalar + type(pp) :: func_array(4) + + func_scalar %f => zero_state + func_array(1)%f => zero_state + print *, func_scalar %f(state) + print *, func_array(1)%f(state) +end program test_func_array -- 2.35.3
Re: [Patch, fortran PR89645/99065 No IMPLICIT type error with: ASSOCIATE( X => function() )
Hi Paul, On 3/12/24 15:54, Paul Richard Thomas wrote: Hi All, This is the last posting of this patch before I push it. Harald is OK with it on the grounds that the inferred_type flag guards the whole lot, except for the chunks in trans-stmt.cc. In spite of Harald's off-list admonition not to try to fix everything at once, this version fixes most of the inquiry reference bugs (associate_68.f90) with the exception of character(kind=4) function selectors. The reason for this is that I have some housekeeping to do before release on finalization and then I want to replace this patch in 15-branch with two pass parsing. My first attempts at the latter were a partial success. you wouldn't stop trying to fix everything, would you? ;-) It regtests OK on x86_64. Unless there are objections, I will commit on Thursday evening. No objections, just one wish: could you improve the text of the following comments so that mere mortals understand them? diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 12e7bf3c873..0ab69bb9dce 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc [...] + /* If there is a usable inquiry reference not there are no matching +derived types, force the inquiry reference by setting unknown the +type of the primary expression. */ I have a hard time parsing the first part of that sentence. diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index 5d9852c79e0..16adb2a7efb 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc [...] +/* Find all derived types in the uppermost namespace that have a component + a component called name and stash them in the assoc field of an + associate name variable. "a component" too much? Thanks, Harald Cheers Paul
[PATCH] Fortran: fix IS_CONTIGUOUS for polymorphic dummy arguments [PR114001]
Dear all, here's another small fix: IS_CONTIGUOUS did erroneously always return .true. for CLASS dummy arguments. The solution was to adjust the logic in gfc_is_simply_contiguous to also handle CLASS symbols. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 8f535b19bd0cb6a7c99ac9ba4c07778f86698a1c Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 12 Mar 2024 22:58:39 +0100 Subject: [PATCH] Fortran: fix IS_CONTIGUOUS for polymorphic dummy arguments [PR114001] gcc/fortran/ChangeLog: PR fortran/114001 * expr.cc (gfc_is_simply_contiguous): Adjust logic so that CLASS symbols are also handled. gcc/testsuite/ChangeLog: PR fortran/114001 * gfortran.dg/is_contiguous_4.f90: New test. --- gcc/fortran/expr.cc | 19 ++--- gcc/testsuite/gfortran.dg/is_contiguous_4.f90 | 81 +++ 2 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/is_contiguous_4.f90 diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 37ea95d0185..82a642b01f7 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -6025,15 +6025,16 @@ gfc_is_simply_contiguous (gfc_expr *expr, bool strict, bool permit_element) } sym = expr->symtree->n.sym; - if (expr->ts.type != BT_CLASS - && ((part_ref - && !part_ref->u.c.component->attr.contiguous - && part_ref->u.c.component->attr.pointer) - || (!part_ref - && !sym->attr.contiguous - && (sym->attr.pointer - || (sym->as && sym->as->type == AS_ASSUMED_RANK) - || (sym->as && sym->as->type == AS_ASSUMED_SHAPE) + if ((part_ref + && part_ref->u.c.component + && !part_ref->u.c.component->attr.contiguous + && IS_POINTER (part_ref->u.c.component)) + || (!part_ref + && expr->ts.type != BT_CLASS + && !sym->attr.contiguous + && (sym->attr.pointer + || (sym->as && sym->as->type == AS_ASSUMED_RANK) + || (sym->as && sym->as->type == AS_ASSUMED_SHAPE return false; if (!ar || ar->type == AR_FULL) diff --git a/gcc/testsuite/gfortran.dg/is_contiguous_4.f90 b/gcc/testsuite/gfortran.dg/is_contiguous_4.f90 new file mode 100644 index 000..cb066f8836b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/is_contiguous_4.f90 @@ -0,0 +1,81 @@ +! { dg-do run } +! PR fortran/114001 - IS_CONTIGUOUS and polymorphic dummy + +program main + implicit none + integer :: i, cnt = 0 + logical :: expect + integer, target :: m(10) = [(i,i=1,size(m))] + integer, pointer :: p(:) + type t + integer :: j + end type t + type(t), pointer :: tt(:), tp(:) ! Type pointer + class(t), pointer :: ct(:), cp(:) ! Class pointer + + p => m(1:3) + expect = is_contiguous (p) + print *, "is_contiguous (p)=", expect + if (.not. expect) stop 91 + call sub_star (p, expect) + p => m(1::3) + expect = is_contiguous (p) + print *, "is_contiguous (p)=", expect + if (expect) stop 92 + call sub_star (p, expect) + + allocate (tt(10)) + tt(:)% j = m + tp => tt(4:6) + expect = is_contiguous (tp) + if (.not. expect) stop 96 + print *, "is_contiguous (tp)=", expect + call sub_t (tp, expect) + tp => tt(4::3) + expect = is_contiguous (tp) + if (expect) stop 97 + print *, "is_contiguous (tp)=", expect + call sub_t (tp, expect) + + allocate (ct(10)) + ct(:)% j = m + cp => ct(7:9) + expect = is_contiguous (cp) + print *, "is_contiguous (cp)=", expect + if (.not. expect) stop 98 + call sub_t (cp, expect) + cp => ct(4::3) + expect = is_contiguous (cp) + print *, "is_contiguous (cp)=", expect + if (expect) stop 99 + call sub_t (cp, expect) + +contains + + subroutine sub_star (x, expect) +class(*), intent(in) :: x(:) +logical, intent(in) :: expect +cnt = cnt + 10 +if (is_contiguous (x) .neqv. expect) then + print *, "sub_star(1): is_contiguous (x)=", is_contiguous (x), expect + stop (cnt + 1) +end if +select type (x) +type is (integer) + if (is_contiguous (x) .neqv. expect) then + print *, "sub_star(2): is_contiguous (x)=", is_contiguous (x), expect + stop (cnt + 2) + end if +end select + end + + subroutine sub_t (x, expect) +class(t), intent(in) :: x(:) +logical, intent(in) :: expect +cnt = cnt + 10 +if (is_contiguous (x) .neqv. expect) then + print *, "sub_t: is_contiguous (x)=", is_contiguous (x), expect + stop (cnt + 3) +end if + end +end -- 2.35.3
Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
Hi Mikael, On 3/15/24 17:31, Mikael Morin wrote: Le 10/03/2024 à 22:31, Harald Anlauf a écrit : Dear all, after playing for some time with NAG and Intel, and an off-list discussion with Jerry, I am getting more and more convinced that simpler runtime error messages (also simpler to parse by a human) are superior to awkward solutions. This is also what Intel does: use only the name of the array (component) in the message whose indices are out of bounds. (NAG's solution appears also inconsistent for nested derived types.) So no x%z, or x%_data, etc. in runtime error messages any more. That's a pity. What about providing the root variable and the failing component only? ... dimension 1 of array component 'z...%x' above array bound ... The data reference doesn't look great, but it provides valuable (in my opinion) information. OK, that sounds interesting. To clarify the options: - for ordinary array x it would stay 'x' - when z is a DT scalar, and z%x is the array in question, use 'z%x' (here z...%x would look strange to me) - when z is a DT array, and x some component further down, 'z...%x' I would rather not make the error message text vary too much to avoid to run into issues with translation. Would it be fine with you to have ... dimension 1 of array 'z...%x' above array bound ... only? Anything else? Cheers, Harald Please give it a spin... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 1/30/24 11:46, Mikael Morin wrote: Le 30/01/2024 à 11:38, Mikael Morin a écrit : Another (easier) way to clarify the data reference would be rephrasing the message so that the array part is separate from the scalar part, like so (there are too many 'of', but I lack inspiration): Index '0' of dimension 1 of component 'zz' of element from 'x1%vv' below lower bound of 1 This has the same number of 'of' but sounds better maybe: Out of bounds accessing component 'zz' of element from 'x1%yy': index '0' of dimension 1 below lower bound of 1
[PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135]
Dear all, as there has been some good progress in the handling of optional dummy arguments, I looked again at this PR and a patch for it that I withdrew as it turned out incomplete. It turned out that it now needs only a minor adjustment for optional dummy arguments of procedures with bind(c) attribute so that ubsan checking does not trigger. Along this way I extended the previous testcase to exercise to some extent combinations of bind(c) and non-bind(c) procedures and found one failure (since at least gcc-9) that is genuine: passing a missing optional from a bind(c) procedure to an assumed-rank dummy, see PR114355. The corresponding test is commented in the testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 2/6/22 22:04, Harald Anlauf wrote: Hi Mikael, Am 04.02.22 um 11:45 schrieb Mikael Morin: Hello, Le 29/01/2022 à 22:41, Harald Anlauf via Fortran a écrit : The least invasive change - already pointed out by the reporter - is to check the presence of the argument before dereferencing the data pointer after the offset calculation. This requires adjusting the checking pattern for gfortran.dg/missing_optional_dummy_6a.f90. Regtesting reminded me that procedures with bind(c) attribute are doing their own stuff, which is why they need to be excluded here, otherwise testcase bind-c-contiguous-4.f90 would regress on the expected output. only after submitting the patch I figured that the patch is incomplete. When we have a call chain of procedures with and without bind(c), there are still cases left where the failure with the sanitizer is not fixed. Just add "bind(c)" to subroutine test_wrapper only in the original PR. I have added a corresponding comment in the PR. There is a potential alternative solution which I did not pursue, as I think it is more invasive, but also that I didn't succeed to implement: A non-present dummy array argument should not need to get its descriptor set up. Pursuing this is probably not the right thing to do during the current stage of development and could be implemented later. If somebody believes this is important, feel free to open a PR for this. I have an other (equally unimportant) concern that it may create an unnecessary conditional when passing a subobject of an optional argument. In that case we can assume that the optional is present. It’s not a correctness issue, so let’s not bother at this stage. Judging from the dump tree of the cases I looked at I did not see anything that would pose a problem to the optimizer. Regtested on x86_64-pc-linux-gnu. OK for mainline? OK. Given my latest observations I'd rather withdraw the current version of the patch and rethink. I also did not see an issue with bind(c) procedures calling alikes. It would help if one would not only know the properties of the actual argument, but also of the formal one, which is not available at that point in the code. I'll have another look and resubmit. Thanks. Thanks for the review! Harald From b3079a82a220477704f8156207239e4af4103ea9 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 15 Mar 2024 20:14:07 +0100 Subject: [PATCH] Fortran: fix for absent array argument passed to optional dummy [PR101135] gcc/fortran/ChangeLog: PR fortran/101135 * trans-array.cc (gfc_get_dataptr_offset): Check for optional arguments being present before dereferencing data pointer. gcc/testsuite/ChangeLog: PR fortran/101135 * gfortran.dg/missing_optional_dummy_6a.f90: Adjust diagnostic pattern. * gfortran.dg/ubsan/missing_optional_dummy_8.f90: New test. --- gcc/fortran/trans-array.cc| 11 ++ .../gfortran.dg/missing_optional_dummy_6a.f90 | 2 +- .../ubsan/missing_optional_dummy_8.f90| 108 ++ 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/ubsan/missing_optional_dummy_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..a7717a8107e 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -7526,6 +7526,17 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, /* Set the target data pointer. */ offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp); + + /* Check for optional dummy argument being present. Arguments of BIND(C) + procedures are excepted here since they are handled differently. */ + if (expr->expr_type == EXPR_VARIABLE + && expr->symtree->n.sym->attr.dummy + && expr->symtree->n.sym->attr.optional + && !is_CFI_desc (NULL, expr)) +offset = build3_loc (input_location, COND_EXPR, TREE_TYPE (offset), + gfc_conv_expr_present (expr->symtree->n.sym), offset, + fold_convert (TREE_TYPE (offset), gfc_index_zero_node)); + gfc_conv_descriptor_data_set (block, parm, offset); } diff --git a/gcc/tes
Re: [PATCH v2 2/2] fortran: Fix specification expression error with dummy procedures [PR111781]
Hi Mikael, thanks for the patch! Regarding the first part of the patch, I think that fixing bad testcases can be done at any time. Retaining identified, broken testcases means that one may hit bogus regressions, hindering progress. The second part of the patch looks at first glance fine to me. And as the patch changes less than its size suggests, in particular due to code refactoring, I don't see a reason to postpone it to stage 1. (On the contrary, deferring it to stage 1 might make future backports from 15 for later patches on top of that code harder. This is my opinion, and others might see this differently.) On 3/17/24 17:57, Mikael Morin wrote: * expr.cc (check_restricted): Remove the case where symbol is dummy and declared in the current ns. Use gfc_get_spec_ns to get the right namespace. Looking at the original and new code, I don't fully understand that part of the commit message: the changed check comes into play when the symbol is *not* in_common, ..., a dummy, ... So technically, we didn't access the (now removed) formal_arg_flag here in those cases. Or am I missing something? diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 82a642b01f7..0852bc5f493 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -3509,19 +3509,13 @@ check_restricted (gfc_expr *e) if (!check_references (e->ref, &check_restricted)) break; - /* gfc_is_formal_arg broadcasts that a formal argument list is being -processed in resolve.cc(resolve_formal_arglist). This is done so -that host associated dummy array indices are accepted (PR23446). -This mechanism also does the same for the specification expressions -of array-valued functions. */ if (e->error || sym->attr.in_common || sym->attr.use_assoc || sym->attr.dummy || sym->attr.implied_index || sym->attr.flavor == FL_PARAMETER - || is_parent_of_current_ns (sym->ns) - || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns))) + || is_parent_of_current_ns (gfc_get_spec_ns (sym))) { t = true; break; diff --git a/gcc/testsuite/gfortran.dg/spec_expr_8.f90 b/gcc/testsuite/gfortran.dg/spec_expr_8.f90 new file mode 100644 index 000..5885810d421 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/spec_expr_8.f90 @@ -0,0 +1,24 @@ +! { dg-do compile } +! +! PR fortran/111781 +! We used to reject the example below because the dummy procedure g was +! setting the current namespace without properly restoring it, which broke +! the specification expression check for the dimension of A later on. +! +! Contributed by Markus Vikhamar-Sandberg Is the reporter's first name Markus or Rasmus? Thanks, Harald
Re: [PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135]
Hi Mikael, On 3/17/24 22:04, Mikael Morin wrote: diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..a7717a8107e 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -7526,6 +7526,17 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, /* Set the target data pointer. */ offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp); + + /* Check for optional dummy argument being present. Arguments of BIND(C) + procedures are excepted here since they are handled differently. */ + if (expr->expr_type == EXPR_VARIABLE + && expr->symtree->n.sym->attr.dummy + && expr->symtree->n.sym->attr.optional + && !is_CFI_desc (NULL, expr)) I think the condition could additionally check the lack of subreferences. But it's maybe not worth the trouble, and the patch is conservatively correct as is, so OK. I have thought about the conditions here for some time and did not find better ones. They need to be broad enough to catch the case in gfortran.dg/missing_optional_dummy_6a.f90 that (according to the tree-dump) was not properly handled previously and would have triggered ubsan at some point in the future when someone tried to change that testcase from currently dg-do compile to dg-do run... (After the patch it would pass, but I didn't dare to change the dg-do). I have pushed the patch as-is, but feel free to post testcases not covered (or improperly covered) to narrow this down further... Thanks for the patch. Thanks for the review! Harald + offset = build3_loc (input_location, COND_EXPR, TREE_TYPE (offset), + gfc_conv_expr_present (expr->symtree->n.sym), offset, + fold_convert (TREE_TYPE (offset), gfc_index_zero_node)); + gfc_conv_descriptor_data_set (block, parm, offset); }
[PATCH, committed] Fortran: error recovery in frontend optimization [PR103715]
Dear all, I've committed the attached simple & obvious patch for an ICE due to an invalid read in frontend optimization after regtesting and an OK by Jerry in the PR. Pushed: https://gcc.gnu.org/g:3be2b8f475f22c531d6cef1b041c0573b3ea5133 As this PR is marked as a regression, I plan to backport to open branches. Thanks, Harald From 3be2b8f475f22c531d6cef1b041c0573b3ea5133 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 18 Mar 2024 19:36:59 +0100 Subject: [PATCH] Fortran: error recovery in frontend optimization [PR103715] gcc/fortran/ChangeLog: PR fortran/103715 * frontend-passes.cc (check_externals_expr): Prevent invalid read in case of mismatch of external subroutine with function. gcc/testsuite/ChangeLog: PR fortran/103715 * gfortran.dg/pr103715.f90: New test. --- gcc/fortran/frontend-passes.cc | 3 +++ gcc/testsuite/gfortran.dg/pr103715.f90 | 12 2 files changed, 15 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr103715.f90 diff --git a/gcc/fortran/frontend-passes.cc b/gcc/fortran/frontend-passes.cc index 06dfa1a3232..3c06018fdbb 100644 --- a/gcc/fortran/frontend-passes.cc +++ b/gcc/fortran/frontend-passes.cc @@ -5807,6 +5807,9 @@ check_externals_expr (gfc_expr **ep, int *walk_subtrees ATTRIBUTE_UNUSED, if (e->expr_type != EXPR_FUNCTION) return 0; + if (e->symtree && e->symtree->n.sym->attr.subroutine) +return 0; + sym = e->value.function.esym; if (sym == NULL) return 0; diff --git a/gcc/testsuite/gfortran.dg/pr103715.f90 b/gcc/testsuite/gfortran.dg/pr103715.f90 new file mode 100644 index 000..72c5a31fb21 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr103715.f90 @@ -0,0 +1,12 @@ +! { dg-do compile } +! PR fortran/103715 - ICE in gfc_find_gsymbol +! +! valgrind did report an invalid read in check_externals_procedure + +program p + select type (y => g()) ! { dg-error "Selector shall be polymorphic" } + end select + call g() +end + +! { dg-prune-output "already being used as a FUNCTION" } -- 2.35.3
[PATCH, v3] Fortran: improve array component description in runtime error message [PR30802]
Hi Mikael, all, here's now the third version of the patch that implements the following scheme: On 3/15/24 20:29, Mikael Morin wrote: Le 15/03/2024 à 18:26, Harald Anlauf a écrit : OK, that sounds interesting. To clarify the options: - for ordinary array x it would stay 'x' - when z is a DT scalar, and z%x is the array in question, use 'z%x' (here z...%x would look strange to me) Yes, the ellipsis would look strange to me as well. - when z is a DT array, and x some component further down, 'z...%x' This case also applies when z is a DT scalar and x is more than one level deep. I would rather not make the error message text vary too much to avoid to run into issues with translation. Would it be fine with you to have ... dimension 1 of array 'z...%x' above array bound ... only? OK, let's drop "component". Anything else? No, I think you covered everything. I've created a new helper function that centralizes the generation of the abbreviated name of the array (component) and use it to simplify related code in multiple places. If we change our mind how a bounds violation error message should look like, it will be easier to adjust in the future. Is this OK for 14-mainline? Thanks, Harald From 30d7cef086d440262b206bc39bcbcac89491b792 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 20 Mar 2024 20:59:24 +0100 Subject: [PATCH] Fortran: improve array component description in runtime error message [PR30802] Runtime error messages for array bounds violation shall use the following scheme for a coherent, abridged description of arrays or array components of derived types: (1) If x is an ordinary array variable, use "x" (2) if z is a DT scalar and x an array component at level 1, use "z%x" (3) if z is a DT scalar and x an array component at level > 1, or if z is a DT array and x an array (at any level), use "z...%x" Use a new helper function abridged_ref_name for construction of that name. gcc/fortran/ChangeLog: PR fortran/30802 * trans-array.cc (abridged_ref_name): New helper function. (trans_array_bound_check): Use it. (array_bound_check_elemental): Likewise. (gfc_conv_array_ref): Likewise. gcc/testsuite/ChangeLog: PR fortran/30802 * gfortran.dg/bounds_check_17.f90: Adjust pattern. * gfortran.dg/bounds_check_fail_8.f90: New test. --- gcc/fortran/trans-array.cc| 132 +++--- gcc/testsuite/gfortran.dg/bounds_check_17.f90 | 2 +- .../gfortran.dg/bounds_check_fail_8.f90 | 56 3 files changed, 142 insertions(+), 48 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 0a453828bad..30b84762346 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3485,6 +3485,78 @@ gfc_conv_array_ubound (tree descriptor, int dim) } +/* Generate abridged name of a part-ref for use in bounds-check message. + Cases: + (1) for an ordinary array variable x return "x" + (2) for z a DT scalar and array component x (at level 1) return "z%%x" + (3) for z a DT scalar and array component x (at level > 1) or + for z a DT array and array x (at any number of levels): "z...%%x" + */ + +static char * +abridged_ref_name (gfc_expr * expr, gfc_array_ref * ar) +{ + gfc_ref *ref; + gfc_symbol *sym; + char *ref_name = NULL; + const char *comp_name = NULL; + int len_sym, last_len = 0, level = 0; + bool sym_is_array; + + gcc_assert (expr->expr_type == EXPR_VARIABLE && expr->ref != NULL); + + sym = expr->symtree->n.sym; + sym_is_array = (sym->ts.type != BT_CLASS + ? sym->as != NULL + : IS_CLASS_ARRAY (sym)); + len_sym = strlen (sym->name); + + /* Scan ref chain to get name of the array component (when ar != NULL) or + array section, determine depth and remember its component name. */ + for (ref = expr->ref; ref; ref = ref->next) +{ + if (ref->type == REF_COMPONENT + && strcmp (ref->u.c.component->name, "_data") != 0) + { + level++; + comp_name = ref->u.c.component->name; + continue; + } + + if (ref->type != REF_ARRAY) + continue; + + if (ar) + { + if (&ref->u.ar == ar) + break; + } + else if (ref->u.ar.type == AR_SECTION) + break; +} + + if (level > 0) +last_len = strlen (comp_name); + + /* Provide a buffer sufficiently large to hold "x...%%z". */ + ref_name = XNEWVEC (char, len_sym + last_len + 6); + strcpy (ref_name, sym->name); + + if (level == 1 && !sym_is_array) +{ + strcat (ref_name, "%%"); + strcat (ref_name, comp_name); +} + else if (level > 0) +{ + strcat (ref_name, "...%%"); + strcat (ref_name, comp_name); +} + + return ref_name; +} + +
Re: [PATCH] fortran: Ignore use statements on error [PR107426]
Hi Mikael, this looks all good to me. I wouldn't mind the minor side-effects of better error recovery, as you are (successfully) trying hard to keep the namespaces sane. So OK for mainline. Thanks for the patch! Harald On 3/21/24 17:27, Mikael Morin wrote: Hello, here is a fix for an ICE caused by dangling pointers to ISO_C_BINDING's C_PTR symbol in the global intrinsic symbol for C_LOC. I tried to fix it by making the intrinsic symbol use its own copy of C_PTR, but it regressed heavily. Instead, I propose this which is based on a patch I attached to the PR one year ago. It's sufficient to remove the access to freed memory. However, an underlying problem remains that successive use-associations of ISO_C_BINDING's symbols in different scopes cause the return type of the C_LOC global intrinsic symbol to be set to the C_PTR from each scope successively, with the last one "winning". Not very pretty. Anyway, there are two changed messages in the testsuite as a side-effect of the proposed change. I regard them as acceptable, albeit slightly worse. No regression otherwise on x86_64-pc-linux-gnu. Ok for 14 master? Mikael -- >8 -- This fixes an access to freed memory on the testcase from the PR. The problem comes from an invalid subroutine statement in an interface, which is ignored and causes the following statements forming the procedure body to be rejected. One of them use-associates the intrinsic ISO_C_BINDING module, which imports new symbols in a namespace that is freed at the time the statement is rejected. However, this creates dangling pointers as ISO_C_BINDING is special and its import creates a reference to the imported C_PTR symbol in the return type of the global intrinsic symbol for C_LOC (see the function create_intrinsic_function). This change saves and restores the list of use statements, so that rejected use statements are removed before they have a chance to be applied to the current namespace and create dangling pointers. PR fortran/107426 gcc/fortran/ChangeLog: * gfortran.h (gfc_save_module_list, gfc_restore_old_module_list): New declarations. * module.cc (old_module_list_tail): New global variable. (gfc_save_module_list, gfc_restore_old_module_list): New functions. (gfc_use_modules): Set module_list and old_module_list_tail. * parse.cc (next_statement): Save module_list before doing any work. (reject_statement): Restore module_list to its saved value. gcc/testsuite/ChangeLog: * gfortran.dg/pr89943_3.f90: Update error pattern. * gfortran.dg/pr89943_4.f90: Likewise. * gfortran.dg/use_31.f90: New test. --- gcc/fortran/gfortran.h | 2 ++ gcc/fortran/module.cc | 31 + gcc/fortran/parse.cc| 4 gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +- gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +- gcc/testsuite/gfortran.dg/use_31.f90| 25 6 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/use_31.f90 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index c7039730fad..fec7b53ff1a 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3926,6 +3926,8 @@ void gfc_module_done_2 (void); void gfc_dump_module (const char *, int); bool gfc_check_symbol_access (gfc_symbol *); void gfc_free_use_stmts (gfc_use_list *); +void gfc_save_module_list (); +void gfc_restore_old_module_list (); const char *gfc_dt_lower_string (const char *); const char *gfc_dt_upper_string (const char *); diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc index d1de53cbdb4..c565b84d61b 100644 --- a/gcc/fortran/module.cc +++ b/gcc/fortran/module.cc @@ -195,7 +195,12 @@ static const char *module_name; /* The name of the .smod file that the submodule will write to. */ static const char *submodule_name; +/* The list of use statements to apply to the current namespace + before parsing the non-use statements. */ static gfc_use_list *module_list; +/* The end of the MODULE_LIST list above at the time the recognition + of the current statement started. */ +static gfc_use_list **old_module_list_tail; /* If we're reading an intrinsic module, this is its ID. */ static intmod_id current_intmod; @@ -7561,6 +7566,8 @@ gfc_use_modules (void) gfc_use_module (module_list); free (module_list); } + module_list = NULL; + old_module_list_tail = &module_list; gfc_rename_list = NULL; } @@ -7584,6 +7591,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts) } +/* Remember the end of the MODULE_LIST list, so that the list can be restored + to its previous state if the current statement is erroneous. */ + +void +gfc_save_module_list () +{ + gfc_use_list **tail = &module_list; + while (*tail != NULL) +tail = &(*tail)->next; + old_module_list_tail = tail; +} + + +/* Restore the MODULE_LIST lis
[PATCH] Fortran: no size check passing NULL() without MOLD argument [PR55978]
Dear all, here's a simple and obvious patch for a rejects-valid case when we pass a NULL() actual to an optional dummy for variants where there is no MOLD argument and it is also not required. The testcase is an extended version of PR55978 comment#16 and cross-checked with Intel and NAG. Regtested on x86_64-pc-linux-gnu. I intend to commit soon unless there are objections. Thanks, Harald From e92244c5539a537cff338b781d15acd58d4c86f1 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 22 Mar 2024 18:17:15 +0100 Subject: [PATCH] Fortran: no size check passing NULL() without MOLD argument [PR55978] gcc/fortran/ChangeLog: PR fortran/55978 * interface.cc (gfc_compare_actual_formal): Skip size check for NULL() actual without MOLD argument. gcc/testsuite/ChangeLog: PR fortran/55978 * gfortran.dg/null_actual_5.f90: New test. --- gcc/fortran/interface.cc| 4 ++ gcc/testsuite/gfortran.dg/null_actual_5.f90 | 76 + 2 files changed, 80 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/null_actual_5.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index 64b90550be2..7b86a338bc1 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -3439,6 +3439,10 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, if (f->sym->ts.type == BT_CLASS) goto skip_size_check; + /* Skip size check for NULL() actual without MOLD argument. */ + if (a->expr->expr_type == EXPR_NULL && a->expr->ts.type == BT_UNKNOWN) + goto skip_size_check; + actual_size = get_expr_storage_size (a->expr); formal_size = get_sym_storage_size (f->sym); if (actual_size != 0 && actual_size < formal_size diff --git a/gcc/testsuite/gfortran.dg/null_actual_5.f90 b/gcc/testsuite/gfortran.dg/null_actual_5.f90 new file mode 100644 index 000..1198715b7c8 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/null_actual_5.f90 @@ -0,0 +1,76 @@ +! { dg-do compile } +! PR fortran/55978 +! +! Passing of NULL() with and without MOLD as actual argument +! +! Testcase derived from pr55978 comment#16 + +program pr55978_c16 + implicit none + + integer, pointer :: p(:) + integer, allocatable :: a(:) + character(10), pointer :: c + character(10), pointer :: cp(:) + + type t +integer, pointer :: p(:) +integer, allocatable :: a(:) + end type + + type(t) :: d + + ! (1) pointer + p => null() + call sub (p) + + ! (2) allocatable + call sub (a) + call sub (d%a) + + ! (3) pointer component + d%p => null () + call sub (d%p) + + ! (4) NULL + call sub (null (a)) ! OK + call sub (null (p)) ! OK + call sub (null (d%a)) ! OK + call sub (null (d%p)) ! OK + call sub (null ())! was erroneously rejected with: + ! Actual argument contains too few elements for dummy argument 'x' (1/4) + + call bla (null(c)) + call bla (null()) ! was erroneously rejected with: + ! Actual argument contains too few elements for dummy argument 'x' (1/10) + + call foo (null(cp)) + call foo (null()) + + call bar (null(cp)) + call bar (null()) ! was erroneously rejected with: + ! Actual argument contains too few elements for dummy argument 'x' (1/70) + +contains + + subroutine sub(x) +integer, intent(in), optional :: x(4) +if (present (x)) stop 1 + end + + subroutine bla(x) +character(len=10), intent(in), optional :: x +if (present (x)) stop 2 + end + + subroutine foo(x) +character(len=10), intent(in), optional :: x(:) +if (present (x)) stop 3 + end + + subroutine bar(x) +character(len=10), intent(in), optional :: x(7) +if (present (x)) stop 4 + end + +end -- 2.35.3
Re: [patch, libgfortran] PR107031 - endfile truncates file at wrong position
Hi Jerry, Am 26.03.24 um 04:18 schrieb Jerry D: Hi all, There has been a bit of discussio on which way to go on this. I took a look today and this trivial patch gives the behavior concluded on Fortran Discourse. See the bugzilla for all the relevant information. Regresion tested on x86-64. I will do the appropriate changelog. OK for trunk? Attached is a new test case and the patch here: diff --git a/libgfortran/io/file_pos.c b/libgfortran/io/file_pos.c index 2bc05b293f8..d169961f997 100644 --- a/libgfortran/io/file_pos.c +++ b/libgfortran/io/file_pos.c @@ -352,7 +352,6 @@ st_endfile (st_parameter_filepos *fpp) dtp.common = fpp->common; memset (&dtp.u.p, 0, sizeof (dtp.u.p)); dtp.u.p.current_unit = u; - next_record (&dtp, 1); } unit_truncate (u, stell (u->s), &fpp->common); this is OK from my side. Given the discussion on "dg-do run", wouldn't this be a perfect example where it is sufficient to run the testcase just once? The change is in libgfortran, not in the frontend or middle-end. Thanks for the patch! Harald
[PATCH] Fortran: fix DATA and derived types with pointer components [PR114474]
Dear all, the attached patch fixes a 10+ regression for cases where a derived type with a pointer component is used in a DATA statement. The failure looked obscure, see testcase comments, and pointed to a possible issue in the resolution (order). For the failing test, the target variable was seen with ts.type == BT_PROCEDURE instead of its actual type. For this reason, I restricted the fixup as much as possible. For details, please see the commit message. Testcase cross-checked with NAG. Regtested on x86_64-pc-linux-gnu. OK for mainline? If this fix survives broader testing, I would like to backport. Thanks, Harald P.S.: while trying to extend coverage of conforming code, I had much fun also with other compilers (e.g. NAG panicking...) From d5fda38243a22e1aef4367653d92521e53f2000d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 27 Mar 2024 21:18:04 +0100 Subject: [PATCH] Fortran: fix DATA and derived types with pointer components [PR114474] When matching actual arguments in match_actual_arg, these are initially treated as a possible dummy procedure, assuming that the correct type is determined later. This resolution could fail when the procedure is a derived type constructor with a pointer component and appears in a DATA statement, where the pointer shall be associated with an initial data target. Check for those cases where the type obviously has not been resolved yet, and which were missed because there was no component reference. gcc/fortran/ChangeLog: PR fortran/114474 * primary.cc (gfc_variable_attr): Catch variables used in structure constructors within DATA statements that are still tagged with a temporary type BT_PROCEDURE from match_actual_arg and which have the target attribute, and fix their typespec. gcc/testsuite/ChangeLog: PR fortran/114474 * gfortran.dg/data_pointer_3.f90: New test. --- gcc/fortran/primary.cc | 12 +++ gcc/testsuite/gfortran.dg/data_pointer_3.f90 | 77 2 files changed, 89 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/data_pointer_3.f90 diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 0ab69bb9dce..5dd6875a4a6 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -2804,6 +2804,18 @@ gfc_variable_attr (gfc_expr *expr, gfc_typespec *ts) if (ts != NULL && expr->ts.type == BT_UNKNOWN) *ts = sym->ts; + /* Catch left-overs from match_actual_arg, where an actual argument of a + procedure is given a temporary ts.type == BT_PROCEDURE. The fixup is + needed for structure constructors in DATA statements, where a pointer + is associated with a data target, and the argument has not been fully + resolved yet. Components references are dealt with further below. */ + if (ts != NULL + && expr->ts.type == BT_PROCEDURE + && expr->ref == NULL + && attr.flavor != FL_PROCEDURE + && attr.target) +*ts = sym->ts; + has_inquiry_part = false; for (ref = expr->ref; ref; ref = ref->next) if (ref->type == REF_INQUIRY) diff --git a/gcc/testsuite/gfortran.dg/data_pointer_3.f90 b/gcc/testsuite/gfortran.dg/data_pointer_3.f90 new file mode 100644 index 000..f0325cd5bcb --- /dev/null +++ b/gcc/testsuite/gfortran.dg/data_pointer_3.f90 @@ -0,0 +1,77 @@ +! { dg-do compile } +! PR fortran/114474 - DATA and derived types with pointer components + +program pr114474 + implicit none + integer, target :: ii = 42! initial data target + + integer, target :: jj = 24 + integer, pointer:: qq => jj + ! ii and jj resolve slightly differently when the data statement below + ! is reached, as jj is resolved outside the structure constructor first + + type t + integer, pointer :: h + end type t + + integer, target :: kk(7) = 23 + integer, pointer:: ll(:) => kk + + type t1 + integer :: m(7) + end type t1 + + type(t) :: x1, x2, x3, x4, x5 + type(t), parameter :: z1 = t(null()) + + type(t1), target:: tt = t1([1,2,3,4,5,6,7]) + type(t1), parameter :: vv = t1(22) + type(t1):: w1, w2 + integer, pointer :: p1(:) => tt% m + + data x1 / t(null()) / + data x2 / t(ii) / ! ii is initial data target + data x3 / t(jj) / ! jj is resolved differently... + data x4 / t(tt%m(3)) / ! pointer association with 3rd element + + data w1 / t1(12) / + data w2 / t1(vv%m) / + + if ( associated (x1% h)) stop 1 + if (.not. associated (x2% h)) stop 2 + if (.not. associated (x3% h)) stop 3 + if (.not. associated (x4% h)) stop 4 + if (x2% h /= 42) stop 5 + if (x3% h /= 24) stop 6 + if (x4% h /= 3) stop 7 + + if (any (w1%m /= 12 )) stop 8 + if (any (w2%m /= vv%m)) stop 9 +end + + +subroutine sub + implicit none + + interface + real function myfun (x) + real, intent(in) :: x + end function myfun + end interface + + type u + procedure(myfun),
[PATCH] Fortran: fix NULL pointer dereference on overlapping initialization [PR50410]
Dear all, the attached simple, obvious and ancient patch from the PR fixes a NULL pointer dereference that occurs on overlapping initializations of derived types/DT components in DATA statements. Gfortran currently does not detect or report overlapping initializations in such cases, and some other compilers also do not (Intel) or give only a warning (e.g. Nvidia). For this reason I decided to add -std=legacy to the options in the testcase. Detecting the overlapping initializations appears to require deeper changes in the way we look up DT components when handling DATA statements, which is beyond the current stage. Regtested on x86_64-pc-linux-gnu. I intend to commit soon unless there are objections. Thanks, Harald From b3970a30679959eed159dffa816899e4430e9da5 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 28 Mar 2024 22:34:40 +0100 Subject: [PATCH] Fortran: fix NULL pointer dereference on overlapping initialization [PR50410] gcc/fortran/ChangeLog: PR fortran/50410 * trans-expr.cc (gfc_conv_structure): Check for NULL pointer. gcc/testsuite/ChangeLog: PR fortran/50410 * gfortran.dg/data_initialized_4.f90: New test. --- gcc/fortran/trans-expr.cc| 2 +- gcc/testsuite/gfortran.dg/data_initialized_4.f90 | 16 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/data_initialized_4.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 76bed9830c4..7ce798ab8a5 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -9650,7 +9650,7 @@ gfc_conv_structure (gfc_se * se, gfc_expr * expr, int init) cm = expr->ts.u.derived->components; for (c = gfc_constructor_first (expr->value.constructor); - c; c = gfc_constructor_next (c), cm = cm->next) + c && cm; c = gfc_constructor_next (c), cm = cm->next) { /* Skip absent members in default initializers and allocatable components. Although the latter have a default initializer diff --git a/gcc/testsuite/gfortran.dg/data_initialized_4.f90 b/gcc/testsuite/gfortran.dg/data_initialized_4.f90 new file mode 100644 index 000..156b6607edf --- /dev/null +++ b/gcc/testsuite/gfortran.dg/data_initialized_4.f90 @@ -0,0 +1,16 @@ +! { dg-do compile } +! { dg-additional-options "-std=legacy" } +! +! PR fortran/50410 +! +! Silently allow overlapping initialization in legacy mode (used to ICE) + +program p + implicit none + type t + integer :: g = 1 + end type t + type(t) :: u = t(2) + data u%g /3/ + print *, u! this might print "2" +end -- 2.35.3
Re: [Patch, fortran] PR110987 and PR113885 - gimplifier ICEs and wrong results in finalization
Hi Paul, Am 28.03.24 um 16:39 schrieb Paul Richard Thomas: Hi All, The attached patch has two elements: (i) A fix for gimplifier ICEs with derived type having no components. The reporter himself suggested (thanks Kirill!): - if (derived && derived->attr.zero_comp) + if (derived && (derived->components == NULL)) As far as I can tell, this is the correct fix. I tried setting attr.zero_comp in resolve.cc for all the OK types without components but this caused all sorts of fallout. (ii) Final calls were occurring in the wrong place for finalizable elemental function calls within scalarizer loops. This caused incorrect results even for derived types with components. This is also fixed. yes, this looks good here. It should be noted that finalizer calls from the rhs of an assignment are occurring at the wrong time, since F2018/24-7.5.6.3 requires: "If an executable construct references a nonpointer function, the result is finalized after execution of the innermost executable construct containing the reference.", while in the present implementation, this happening just before assignment to the lhs temporary. Fixing this is going to be really tough and invasive, so I decided that getting the right results and the correct number of finalization should be sufficient for the 14-branch release. As it happens, I had been mulling over how to do this for finalizations hidden in constructors and other contexts than assignment (eg. write statements or allocation with source). It's a few months away and will be appropriate for stage 1. Regtests on x86_64 - OK for mainline and then, after a bit, for backporting to 13-branch? The patch looks rather "conservative" (read: safe) and appears to fix the regressions very well, so go ahead as planned. Thanks for the patch! Harald Regards to all Paul Fortran: Fix a gimplifier ICE/wrong result with finalization [PR104555] 2024-03-28 Paul Thomas gcc/fortran PR fortran/36337 PR fortran/110987 PR fortran/113885 * trans-expr.cc (gfc_trans_assignment_1): Place finalization block before rhs post block for elemental rhs. * trans.cc (gfc_finalize_tree_expr): Check directly if a type has no components, rather than the zero components attribute. Treat elemental zero component expressions in the same way as scalars. gcc/testsuite/ PR fortran/113885 * gfortran.dg/finalize_54.f90: New test. * gfortran.dg/finalize_55.f90: New test. gcc/testsuite/ PR fortran/110987 * gfortran.dg/finalize_56.f90: New test.
Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab
Hi Paul, I had only a quick glance at your patch. I guess you unintentionally forgot to remove those parts that you already committed for PR110987, along with the finalize-testcases. I am still trying to find the precise paragraph in the standard you refer to regarding INTENT(OUT) and default initialization. While at it, I think I found a minor nit in testcase pr112407a.f90: component x%i appears undefined the first time it is printed. This can be verified by either adding an explicit x% i = -42 in the main after the allocate(x). Alternatively, running the code with Intel and using MALLOC_PERTURB_ shows a random arg1%i, but is otherwise fine. However, if by chance (random memory) x% i = +42 then the test would likely fail everywhere. Cheers, Harald Am 30.03.24 um 10:06 schrieb Paul Richard Thomas: Hi All, This bug emerged in a large code and involves possible recursion with a "hidden" module procedure; ie. where the symtree name starts with '@'. This throws the format decoder. As the last message in the PR shows, I have vacillated between silently passing on the possible recursion or adding an alternative warning message. In the end, as a conservative choice I went for emitting the message. In the course of trying to develop a compact test case, I found that type bound procedures were not being tested for recursion and that class dummies, with intent out, were being incorrectly initialized with an empty default initializer. Both of these have been fixed. Unfortunately, the most compact reproducer that Tomas was able to come up with required more than 100kbytes of module files. I tried from the bottom up but failed. Both the tests check the fixes for the other bugs. Regtests on x86_64 - OK for mainline and, in a couple of weeks, 13-branch? Paul Fortran: Fix wrong recursive errors and class initialization [PR112407] 2024-03-30 Paul Thomas gcc/fortran PR fortran/112407 *resolve.cc (resolve_procedure_expression): Change the test for for recursion in the case of hidden procedures from modules. (resolve_typebound_static): Add warning for possible recursive calls to typebound procedures. * trans-expr.cc (gfc_trans_class_init_assign): Do not apply default initializer to class dummy where component initializers are all null. gcc/testsuite/ PR fortran/112407 * gfortran.dg/pr112407a.f90: New test. * gfortran.dg/pr112407b.f90: New test.
Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab
Hi Paul! Am 31.03.24 um 14:08 schrieb Paul Richard Thomas: Hi Harald, I had only a quick glance at your patch. I guess you unintentionally forgot to remove those parts that you already committed for PR110987, along with the finalize-testcases. Guilty as charged. I guess I got out of the wrong side of the bed :-) I am still trying to find the precise paragraph in the standard you refer to regarding INTENT(OUT) and default initialization. Page 114 of the draft F2023 standard: "The INTENT (OUT) attribute for a nonpointer dummy argument specifies that the dummy argument becomes undefined on invocation of the procedure, except for any subcomponents that are default-initialized (7.5.4.6)." With the fix, gfortran behaves in the same way as ifort and nagfor. On rereading the patch, I think that s/"and use the passed value"/"and leave undefined"/ or some such is in order. Yes, something along this line is better. I also did test with NAG and Intel, and was surprised (confused?) at how the count of finalizer calls changes if component "i" gets a default value or not. Something one wouldn't do right after getting out of bed! So the patch looks good to me - except for one philosophical question: Fortran 2018 makes procedures recursive by default, but this is not yet implemented as such, and NON_RECURSIVE is not yet implemented. The new testcase pr112407b.f90 compiles with nagfor -f2018 without any warnings, and gives an error with nagfor -f2008. It appears that it works in the testsuite after the patch and when adding "-std=f2008" instead of using the default "-std=gnu". Would you mind adding "-std=f2008" as dg-option to that testcase? This would avoid one bogus regression when gfortran moves forward. Thanks for the patch! Harald While at it, I think I found a minor nit in testcase pr112407a.f90: component x%i appears undefined the first time it is printed. Fixed - thanks for pointing it out. A correct patch is attached. Thanks for looking at the previous, overloaded version. Paul 2024-03-30 Paul Thomas gcc/fortran PR fortran/112407 *resolve.cc (resolve_procedure_expression): Change the test for for recursion in the case of hidden procedures from modules. (resolve_typebound_static): Add warning for possible recursive calls to typebound procedures. * trans-expr.cc (gfc_trans_class_init_assign): Do not apply default initializer to class dummy where component initializers are all null. gcc/testsuite/ PR fortran/112407 * gfortran.dg/pr112407a.f90: New test. * gfortran.dg/pr112407b.f90: New test.
Re: [Patch, fortran] PR106999 [11/12/13/14 Regression] ICE tree check: expected record_type or union_type or qual_union_type, have function_type in gfc_class_data_get, at fortran/trans-expr.cc:233
Hi Paul, On 3/31/24 15:01, Paul Richard Thomas wrote: This regression has a relatively simple fix. The passing of a subroutine procedure pointer component to a dummy variable was being missed completely. The error has been added. Conversely, an error was generated for a procedure pointer variable but no use was being made of the interface, if one was available. This has been corrected. OK for mainline and backporting in a couple of weeks? this is all OK. Thanks for the patch! Harald Paul Fortran: Add error for subroutine passed to a variable dummy [PR106999] 2024-03-31 Paul Thomas gcc/fortran PR fortran/106999 *interface.cc (gfc_compare_interfaces): Add error for a subroutine proc pointer passed to a variable formal. (compare_parameter): If a procedure pointer is being passed to a non-procedure formal arg, and there is an an interface, use gfc_compare_interfaces to check and provide a more useful error message. gcc/testsuite/ PR fortran/106999 * gfortran.dg/pr106999.f90: New test.
[PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500]
Dear all, the attached patch fixes argument checking of: - C_SIZEOF - rejects-valid (see below) and ICE-on-valid - C_F_POINTER - ICE-on-invalid The interesting part is that C_SIZEOF was not well specified until after F2018, where an interp request lead to an edit that actually loosened restrictions and makes the checking much more straightforward, since expressions and function results are now allowed. I've added references to the relevant text and interp in the commit message. While updating the checking code shared between C_SIZEOF and C_F_POINTER, I figured that the latter missed a check preventing an ICE-on-invalid when a function returning a pointer was passed. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 6f412a6399a7e125db835584d3d2489a52150c27 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 8 Apr 2024 21:43:24 +0200 Subject: [PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500] The interpretation of the F2018 standard regarding valid arguments to the intrinsic C_SIZEOF(X) was clarified in an edit to 18-007r1: https://j3-fortran.org/doc/year/22/22-101r1.txt loosening restrictions and giving examples. The F2023 text has: ! F2023:18.2.3.8 C_SIZEOF (X) ! ! X shall be a data entity with interoperable type and type parameters, ! and shall not be an assumed-size array, an assumed-rank array that ! is associated with an assumed-size array, an unallocated allocatable ! variable, or a pointer that is not associated. where ! 3.41 data entity ! data object, result of the evaluation of an expression, or the ! result of the execution of a function reference Update the checking code for interoperable arguments accordingly, and extend to reject functions returning pointer as FPTR argument to C_F_POINTER. gcc/fortran/ChangeLog: PR fortran/106500 * check.cc (is_c_interoperable): Fix checks for C_SIZEOF. (gfc_check_c_f_pointer): Reject function returning a pointer as FPTR. gcc/testsuite/ChangeLog: PR fortran/106500 * gfortran.dg/c_sizeof_6.f90: Remove wrong dg-error. * gfortran.dg/c_f_pointer_tests_9.f90: New test. * gfortran.dg/c_sizeof_7.f90: New test. --- gcc/fortran/check.cc | 21 ++ .../gfortran.dg/c_f_pointer_tests_9.f90 | 21 ++ gcc/testsuite/gfortran.dg/c_sizeof_6.f90 | 2 +- gcc/testsuite/gfortran.dg/c_sizeof_7.f90 | 42 +++ 4 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_sizeof_7.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index db74dcf3f40..b7f60575c67 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5299,18 +5299,14 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr) return false; } - if (!c_loc && expr->rank > 0 && expr->expr_type != EXPR_ARRAY) + /* Checks for C_SIZEOF need to take into account edits to 18-007r1, see + https://j3-fortran.org/doc/year/22/22-101r1.txt . */ + if (!c_loc && !c_f_ptr && expr->rank > 0 && expr->expr_type == EXPR_VARIABLE) { gfc_array_ref *ar = gfc_find_array_ref (expr); - if (ar->type != AR_FULL) + if (ar->type == AR_FULL && ar->as->type == AS_ASSUMED_SIZE) { - *msg = "Only whole-arrays are interoperable"; - return false; - } - if (!c_f_ptr && ar->as->type != AS_EXPLICIT - && ar->as->type != AS_ASSUMED_SIZE) - { - *msg = "Only explicit-size and assumed-size arrays are interoperable"; + *msg = "Assumed-size arrays are not interoperable"; return false; } } @@ -5475,6 +5471,13 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) return false; } + if (attr.function) +{ + gfc_error ("FPTR at %L to C_F_POINTER is a function returning a pointer", + &fptr->where); + return false; +} + if (fptr->rank > 0 && !is_c_interoperable (fptr, &msg, false, true)) return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR " "at %L to C_F_POINTER: %s", &fptr->where, msg); diff --git a/gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 b/gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 new file mode 100644 index 000..bb6d3281b02 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 @@ -0,0 +1,21 @@ +! { dg-do compile } +! +! A function returning a pointer cannot be interoperable +! and cannot be used as FPTR argument to C_F_POINTER. + +subroutine s () + use, intrinsic :: iso_c_binding + implicit none + type(c_ptr) :: cPtr + call c_f_pointer (cPtr, p0)! { dg-error "function returning a pointer" } + call c_f_pointer (cPtr, p1, shape=[2
[PATCH, v2] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500]
Hi FX! On 4/9/24 09:32, FX Coudert wrote: Hi Harald, Thanks for the patch. + if (attr.function) +{ + gfc_error ("FPTR at %L to C_F_POINTER is a function returning a pointer", + &fptr->where); + return false; +} + if (fptr->rank > 0 && !is_c_interoperable (fptr, &msg, false, true)) return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR " "at %L to C_F_POINTER: %s", &fptr->where, msg); In both of these gfc_error(), could we change our error message to say "FPTR argument” instead of “FPTR”? “FPTR to C_F_POINTER” does not really make sense to me. This would be more in line with what the generally do: Error: 'x' argument of 'sqrt' intrinsic at (1) must be REAL or COMPLEX So maybe “FPTR argument to C_F_POINTER at %L” ? That’s much more readable to me. Good point! I did indeed feel a little uncomfortable with the text and adjusted both messages accordingly to your suggestion. I also forgot to add one update of a pattern, and found a cornercase where the tightening of checks for C_F_POINTER was too strong. Corrected and now covered in an extension of the corresponding testcase. Otherwise, OK. FX Thanks for the review! If there are no further comments, I will commit tomorrow. Thanks, Harald From 5983a07f11c88d920241141732fa742735cdb8ea Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 9 Apr 2024 23:07:59 +0200 Subject: [PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500] The interpretation of the F2018 standard regarding valid arguments to the intrinsic C_SIZEOF(X) was clarified in an edit to 18-007r1: https://j3-fortran.org/doc/year/22/22-101r1.txt loosening restrictions and giving examples. The F2023 text has: ! F2023:18.2.3.8 C_SIZEOF (X) ! ! X shall be a data entity with interoperable type and type parameters, ! and shall not be an assumed-size array, an assumed-rank array that ! is associated with an assumed-size array, an unallocated allocatable ! variable, or a pointer that is not associated. where ! 3.41 data entity ! data object, result of the evaluation of an expression, or the ! result of the execution of a function reference Update the checking code for interoperable arguments accordingly, and extend to reject functions returning pointer as FPTR argument to C_F_POINTER. gcc/fortran/ChangeLog: PR fortran/106500 * check.cc (is_c_interoperable): Fix checks for C_SIZEOF. (gfc_check_c_f_pointer): Reject function returning a pointer as FPTR, and improve an error message. gcc/testsuite/ChangeLog: PR fortran/106500 * gfortran.dg/c_sizeof_6.f90: Remove wrong dg-error. * gfortran.dg/sizeof_2.f90: Adjust pattern. * gfortran.dg/c_f_pointer_tests_9.f90: New test. * gfortran.dg/c_sizeof_7.f90: New test. --- gcc/fortran/check.cc | 26 +++- .../gfortran.dg/c_f_pointer_tests_9.f90 | 37 gcc/testsuite/gfortran.dg/c_sizeof_6.f90 | 2 +- gcc/testsuite/gfortran.dg/c_sizeof_7.f90 | 42 +++ gcc/testsuite/gfortran.dg/sizeof_2.f90| 2 +- 5 files changed, 96 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_sizeof_7.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index db74dcf3f40..2f50d84b876 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5299,18 +5299,14 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr) return false; } - if (!c_loc && expr->rank > 0 && expr->expr_type != EXPR_ARRAY) + /* Checks for C_SIZEOF need to take into account edits to 18-007r1, see + https://j3-fortran.org/doc/year/22/22-101r1.txt . */ + if (!c_loc && !c_f_ptr && expr->rank > 0 && expr->expr_type == EXPR_VARIABLE) { gfc_array_ref *ar = gfc_find_array_ref (expr); - if (ar->type != AR_FULL) + if (ar->type == AR_FULL && ar->as->type == AS_ASSUMED_SIZE) { - *msg = "Only whole-arrays are interoperable"; - return false; - } - if (!c_f_ptr && ar->as->type != AS_EXPLICIT - && ar->as->type != AS_ASSUMED_SIZE) - { - *msg = "Only explicit-size and assumed-size arrays are interoperable"; + *msg = "Assumed-size arrays are not interoperable"; return false; } } @@ -5475,9 +5471,17 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) return false; } + if (fptr->ts.type == BT_PROCEDURE && attr.function) +{ + gfc_error ("FPTR argument to C_F_POINTER at %L is a function " + "returning a pointer", &fptr->where); + return false; +} + if (fptr->rank > 0 && !is_c_interoperable
Re: [Patch, fortran] PR113363 - ICE on ASSOCIATE and unlimited polymorphic function
Hi Paul! On 4/10/24 10:25, Paul Richard Thomas wrote: Hi All, This patch corrects incorrect results from assignment of unlimited polymorphic function results both in assignment statements and allocation with source. The first chunk in trans-array.cc ensures that the array dtype is set to the source dtype. The second chunk ensures that the lhs _len field does not default to zero and so is specific to dynamic types of character. The addition to trans-stmt.cc transforms the source expression, aka expr3, from a derived type of type "STAR" into a proper unlimited polymorphic expression ready for assignment to the newly allocated entity. I am wondering about the following snippet in trans-stmt.cc: + /* Copy over the lhs _data component ref followed by the +full array reference for source expressions with rank. +Otherwise, just copy the _data component ref. */ + if (code->expr3->rank + && ref && ref->next && !ref->next->next) + { + rhs->ref = gfc_copy_ref (ref); + rhs->ref->next = gfc_copy_ref (ref->next); + break; + } Why the two gfc_copy_ref? valgrind pointed my to the tail of gfc_copy_ref which already has: dest->next = gfc_copy_ref (src->next); so this looks redundant and leaks frontend memory? *** Playing with the testcase, I find several invalid writes with valgrind, or a heap buffer overflow with -fsanitize=address . It is sufficient to look at a mini-test where the class(*) function result is assigned to the class(*), allocatable in the main: x = foo () deallocate (x) The dump tree suggests that array bounds in foo() are read before they are properly set. These invalid writes do not occur with 13-branch, so this might be a regression. Can you have a look yourself? Thanks, Harald OK for mainline? Paul Fortran: Fix wrong code in unlimited polymorphic assignment [PR113363] 2024-04-10 Paul Thomas gcc/fortran PR fortran/113363 * trans-array.cc (gfc_array_init_size): Use the expr3 dtype so that the correct element size is used. (gfc_alloc_allocatable_for_assignment): Set the _len field for unlimited polymorphic assignments. * trans-stmt.cc (gfc_trans_allocate): Build a correct rhs for the assignment of an unlimited polymorphic 'source'. gcc/testsuite/ PR fortran/113363 * gfortran.dg/pr113363.f90: New test.
[PATCH] Fortran: ALLOCATE of fixed-length CHARACTER with SOURCE/MOLD [PR113793]
Dear all, the attached patch adds the following: - diagnostics of different string length of allocate-object and of the source-expr (SOURCE/MOLD) as hard error when it can be determined at compile-time - a runtime-diagnostics und -fcheck=bounds (reuse of existing checks) - a fallback solution (GNU extension) to use the length of allocate-object if the length mismatch is not diagnosed at compile-time or runtime. This avoids heap corruption and leads to string truncation or padding during assignment. F2008 demands same values of the kind type parameters, and this is diagnosed by NAG. It also always gives a hard error, even at runtime. Some brands (NVidia, AMD flang) tolerate a length mismatch silently and perform string truncation or padding, without crashing. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From b9ece695a178319e35cd9f36cc731855302dd57f Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 13 Apr 2024 19:09:24 +0200 Subject: [PATCH] Fortran: ALLOCATE of fixed-length CHARACTER with SOURCE/MOLD [PR113793] F2008 requires for ALLOCATE with SOURCE= or MOLD= specifier that the kind type parameters of allocate-object and source-expr have the same values. Add compile-time diagnostics for different character length and a runtime check (under -fcheck=bounds). Use length from allocate-object to prevent heap corruption and to allow string padding or truncation on assignment. gcc/fortran/ChangeLog: PR fortran/113793 * resolve.cc (resolve_allocate_expr): Reject ALLOCATE with SOURCE= or MOLD= specifier for unequal length. * trans-stmt.cc (gfc_trans_allocate): If an allocatable character variable has fixed length, use it and do not use the source length. With bounds-checking enabled, add a runtime check for same length. gcc/testsuite/ChangeLog: PR fortran/113793 * gfortran.dg/allocate_with_source_29.f90: New test. * gfortran.dg/allocate_with_source_30.f90: New test. * gfortran.dg/allocate_with_source_31.f90: New test. --- gcc/fortran/resolve.cc| 10 gcc/fortran/trans-stmt.cc | 36 +++-- .../gfortran.dg/allocate_with_source_29.f90 | 48 + .../gfortran.dg/allocate_with_source_30.f90 | 51 +++ .../gfortran.dg/allocate_with_source_31.f90 | 38 ++ 5 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_29.f90 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_30.f90 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_31.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 4cbf7186119..6b3e5ba4fcb 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -8278,6 +8278,16 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code, bool *array_alloc_wo_spec) goto failure; } + /* Check F2008:C639: "Corresponding kind type parameters of + allocate-object and source-expr shall have the same values." */ + if (e->ts.type == BT_CHARACTER + && !e->ts.deferred + && e->ts.u.cl->length + && code->expr3->ts.type == BT_CHARACTER + && !gfc_check_same_strlen (e, code->expr3, "ALLOCATE with " + "SOURCE= or MOLD= specifier")) + goto failure; + /* Check TS18508, C702/C703. */ if (code->expr3->ts.type == BT_DERIVED && ((codimension && gfc_expr_attr (code->expr3).event_comp) diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc index 7997c167bae..c34e0b4c0cd 100644 --- a/gcc/fortran/trans-stmt.cc +++ b/gcc/fortran/trans-stmt.cc @@ -6829,10 +6829,26 @@ gfc_trans_allocate (gfc_code * code, gfc_omp_namelist *omp_allocate) in the array is needed, which is the product of the len and esize for char arrays. For unlimited polymorphics len can be zero, therefore take the maximum of len and one. */ + tree lhs_len; + + /* If an allocatable character variable has fixed length, use it. + Otherwise use source length. As different lengths are not + allowed by the standard, generate a runtime check. */ + if (expr->ts.type == BT_CHARACTER && !expr->ts.deferred) + { + gfc_trans_same_strlen_check ("ALLOCATE with SOURCE= or MOLD=", + &code->expr3->where, + se.string_length, expr3_len, + &block); + lhs_len = fold_convert (TREE_TYPE (expr3_len), se.string_length); + } + else + lhs_len = expr3_len; + tmp = fold_build2_loc (input_location, MAX_EXPR, TREE_TYPE (expr3_len), - expr3_len, fold_convert (TREE_TYPE (expr3_len), - integer_one_node)); + lhs_len, fold_convert (TREE_TYPE (expr3_len), + integer_one_node)); tmp = fold_build2_loc (input_location, MULT_EXPR, TREE_TYPE (expr3_esize), expr3_esize, fold_convert (TREE_TYPE (expr3_esi
Re: [Patch, fortran] PR103471 - [11/12/13/14 Regression] ICE in gfc_typenode_for_spec, at fortran/trans-types.c:1114
Hi Paul, the patch is OK, but I had to manually fix it. I wonder how you managed to produce: diff --git a/gcc/testsuite/gfortran.dg/pr93484.f90 b/gcc/testsuite/gfortran.dg/pr93484.f90 new file mode 100644 index 000..4dcad47e8da --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr103471.f90 @@ -0,0 +1,13 @@ A minor comment on the error message and the testcase. Take for example: subroutine sub implicit none real, external :: x real :: y(10) integer :: kk print *, [real(x(k))] ! print *, [real(y(k))] end The original testcase in the PR would - without implicit none - resemble the function invocation x(k) here and emit the error: Fatal Error: k at (1) has no default type compilation terminated. while commenting the first print and uncommenting the second would emit the message Error: Symbol 'k' at (1) has no IMPLICIT type; did you mean 'kk'? Thus I have the impression that the testcase tests something different on the one hand, and on the other I wonder if we would want to change the error message and replace "no default type" to "no IMPLICIT type". It still would not hit the fuzzy check, but that is something that might not be important now. Thanks, Harald On 4/19/24 18:52, Paul Richard Thomas wrote: Hi All, This is a more or less obvious patch. The action is in resolve.cc. The chunk in symbol.cc is a tidy up of a diagnostic marker to distinguish where the 'no IMPLICIT type' error was coming from and the chunk in trans-decl.cc follows from discussion with Harald on the PR. Regtests fine. OK for mainline and backporting in a couple of weeks? Paul Fortran: Detect 'no implicit type' error in right place [PR103471] 2024-04-19 Paul Thomas gcc/fortran PR fortran/103471 * resolve.cc (gfc_resolve_index_1): Block index expressions of unknown type from being converted to default integer, avoiding the fatal error in trans-decl.cc. * symbol.cc (gfc_set_default_type): Remove '(symbol)' from the 'no IMPLICIT type' error message. * trans-decl.cc (gfc_get_symbol_decl): Change fatal error locus to that of the symbol declaration. (gfc_trans_deferred_vars): Remove two trailing tabs. gcc/testsuite/ PR fortran/103471 * gfortran.dg/pr103471.f90: New test.
Re: [Patch, fortran] PR103471 - [11/12/13/14 Regression] ICE in gfc_typenode_for_spec, at fortran/trans-types.c:1114
Hi Paul! On 4/20/24 09:54, Paul Richard Thomas wrote: subroutine sub implicit none real, external :: x real :: y(10) integer :: kk print *, [real(x(k))] ! print *, [real(y(k))] end This is another problem, somewhere upstream from resolve.cc, which I have just spent an hour failing to find. In the presence of both print statements, in no matter which order, it is the error in trans-decl.cc that applies. Indeed, the gfc_fatal_error always wins. (I had tried to replace it with gfc_error()/return NULL_TREE, but then I hit an ICE later on. When trying to find out who added the said code, guess whom I found :) Thus I have the impression that the testcase tests something different on the one hand, and on the other I wonder if we would want to change the error message and replace "no default type" to "no IMPLICIT type". It still would not hit the fuzzy check, but that is something that might not be important now. The fuzzy check was intended to ensure that the error was being detected in the "right" place. I want to keep the "no default type" message for the time being at least so as to identify exactly where it comes from. Getting to trans-decl.cc with an unknown type is just wrong. True. I'll come back to you on this. This PR is marked as a regression. Depending on your progress, it might be worth to consider fixing what you think is needed to get rid of the regression marker and defer the improvement of the diagnostics to a second patch. Harald Thanks for the report. Paul
Re: [Patch, fortran] PR93678 - [11/12/13/14 Regression] ICE with TRANSFER and typebound procedures
Hi Paul, On 4/24/24 18:26, Paul Richard Thomas wrote: Hi there, This regression turned out to be low hanging fruit, although it has taken four years to reach it :-( The ChangeLog says it all. OK for mainline and backporting after a suitable delay? yes to both. Thanks for "picking" this fruit! Harald Paul Fortran: Fix ICE in gfc_trans_create_temp_array from bad type [PR93678] 2024-04-24 Paul Thomas gcc/fortran PR fortran/93678 * trans-expr.cc (gfc_conv_procedure_call): Use the interface, where possible, to obtain the type of character procedure pointers of class entities. gcc/testsuite/ PR fortran/93678 * gfortran.dg/pr93678.f90: New test.
[PATCH] Fortran: fix argument passing to CONTIGUOUS,TARGET dummy [PR97592]
Dear all, the attached simple patch fixes a (9+) regression for passing to a CONTIGUOUS,TARGET dummy an *effective argument* that is contiguous, although the actual argument is not simply-contiguous (it is a pointer without the CONTIGOUS attribute in the PR). Since a previous attempt for a patch lead to regressions in gfortran.dg/bind-c-contiguous-3.f90, which is rather dense, I decided to enhance the current testcase with various combinations of actual and dummy arguments that allow to study whether a _gfortran_internal_pack is generated in places where we want to. (_gfortran_internal_pack does not create a temporary when no packing is needed). Regtested on x86_64-pc-linux-gnu. OK for mainline? I would like to backport this - after a grace period - to at least 13-branch. Any objections here? Thanks, Harald From d8765bd669e501781672c0bec976b2f5fd7acff6 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 16 Dec 2023 19:14:55 +0100 Subject: [PATCH] Fortran: fix argument passing to CONTIGUOUS,TARGET dummy [PR97592] gcc/fortran/ChangeLog: PR fortran/97592 * trans-expr.cc (gfc_conv_procedure_call): For a contiguous dummy with the TARGET attribute, the effective argument may still be contiguous even if the actual argument is not simply-contiguous. Allow packing to be decided at runtime by _gfortran_internal_pack. gcc/testsuite/ChangeLog: PR fortran/97592 * gfortran.dg/contiguous_15.f90: New test. --- gcc/fortran/trans-expr.cc | 4 +- gcc/testsuite/gfortran.dg/contiguous_15.f90 | 234 2 files changed, 237 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/contiguous_15.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index f4185db5b7f..218fede6a82 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -7124,7 +7124,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, INTENT_IN, fsym->attr.pointer); } else if (fsym && fsym->attr.contiguous - && !gfc_is_simply_contiguous (e, false, true) + && (fsym->attr.target + ? gfc_is_not_contiguous (e) + : !gfc_is_simply_contiguous (e, false, true)) && gfc_expr_is_variable (e)) { gfc_conv_subref_array_arg (&parmse, e, nodesc_arg, diff --git a/gcc/testsuite/gfortran.dg/contiguous_15.f90 b/gcc/testsuite/gfortran.dg/contiguous_15.f90 new file mode 100644 index 000..424eb080fd1 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/contiguous_15.f90 @@ -0,0 +1,234 @@ +! { dg-do run } +! { dg-additional-options "-fdump-tree-original" } +! +! PR fortran/97592 - fix argument passing to CONTIGUOUS,TARGET dummy +! +! { dg-final { scan-tree-dump-times "_gfortran_internal_pack \\(&b_2d" 1 "original" } } +! { dg-final { scan-tree-dump-times "_gfortran_internal_pack \\(&p1" 3 "original" } } +! +! N.B.: there is no reliable count of _gfortran_internal_pack on temporaries parm.* + +program pr97592 + implicit none + integer :: i, k + integer, target :: a(10) + integer, pointer :: p1(:), p2(:), tgt(:), expect(:) + integer, pointer, contiguous :: cp(:) + integer, allocatable, target :: b(:) + + !-- + ! Code from original PR + !-- + call RemappingTest () + + !- + ! Additional 1-d tests + !- + a = [(i, i=1,size(a))] + b = a + + ! Set p1 to an actually contiguous pointer + p1(13:) => a(3::2) + print *, lbound (p1), ubound (p1), is_contiguous (p1) + + ! non-contiguous pointer actual argument + expect => p1 + call chk_cont (p1) + + expect => p1 + call chk_tgt_cont (p1) + + expect => p1 + call chk_ptr (p1, p2) + if (any (p2 /= p1)) stop 1 + + expect => p1 + call chk_tgt (p1, p2) + if (any (p2 /= p1)) stop 2 + + ! non-contiguous target actual argument + expect => b(3::2) + call chk_tgt_cont (b(3::2)) + + expect => b(3::2) + call chk_tgt (b(3::2), p2) + if (any (p2 /= p1)) stop 3 + + expect => b(3::2) + call chk_ptr (b(3::2), p2) + if (any (p2 /= p1)) stop 4 + + ! Set p1 to an actually contiguous pointer + cp(17:) => a(3:9:1) + p1 => cp + print *, lbound (cp), ubound (cp), is_contiguous (cp) + print *, lbound (p1), ubound (p1), is_contiguous (p1) + + expect => p1 + call chk_tgt (p1, p2) + if (any (p2 /= cp)) stop 31 + + expect => cp + call chk_tgt (cp, p2) + if (any (p2 /= cp)) stop 32 + + expect => cp + call chk_tgt_cont (cp, p2) + if (any (p2 /= cp)) stop 33 + + expect => cp + call chk_tgt_expl (cp, p2, size (cp)) + if (any (p2 /= cp)) stop 34 + + ! See F2018:15.5.2.4 and F2018:C.10.4 + expect => p1 + call chk_tgt_cont (p1, p2) +! print *, p2 + if (any (p2 /= cp)) stop 35 + + expect => p1 + call chk_tgt_expl (p1, p2, size (p1)) + if (any (p2 /= cp)) stop 36 + + expect => c
[PATCH] Fortran: update DATE_AND_TIME intrinsic for Fortran 2018 [PR96580]
Dear all, starting with Fortran 2018, DATE_AND_TIME allowed a non-default integer VALUES argument. While gfortran accepts this silently, this failed at runtime because the library implementation beyond kind=4 and kind=8 was missing. Futhermore, the standard effectively requires that the integer kind is at least 2. The attached patch adds several checks, plus adds checking for the array size to be sufficient, extends the implementation to the remaining kinds, and corrects several issues in the related documentation. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 1850bb6cbae7229e2c26e66a0a621817339f85e9 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 18 Dec 2023 18:59:02 +0100 Subject: [PATCH] Fortran: update DATE_AND_TIME intrinsic for Fortran 2018 [PR96580] Fortran 2018 allows a non-default integer kind for its VALUES argument if it has a decimal exponent range of at least four. Update checks, library implementation and documentation. gcc/fortran/ChangeLog: PR fortran/96580 * check.cc (array_size_check): New helper function. (gfc_check_date_and_time): Use it for checking minimum size of VALUES argument. Update kind check to Fortran 2018. * intrinsic.texi: Fix documentation of DATE_AND_TIME. libgfortran/ChangeLog: PR fortran/96580 * intrinsics/date_and_time.c (date_and_time): Handle VALUES argument for kind=2 and kind=16 (if available). gcc/testsuite/ChangeLog: PR fortran/96580 * gfortran.dg/date_and_time_2.f90: New test. * gfortran.dg/date_and_time_3.f90: New test. * gfortran.dg/date_and_time_4.f90: New test. --- gcc/fortran/check.cc | 48 +++ gcc/fortran/intrinsic.texi| 39 +++ gcc/testsuite/gfortran.dg/date_and_time_2.f90 | 21 gcc/testsuite/gfortran.dg/date_and_time_3.f90 | 29 +++ gcc/testsuite/gfortran.dg/date_and_time_4.f90 | 30 libgfortran/intrinsics/date_and_time.c| 32 +++-- 6 files changed, 177 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/date_and_time_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/date_and_time_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/date_and_time_4.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 3b1a0f9f4f4..b91a743be42 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -1251,6 +1251,33 @@ gfc_check_same_strlen (const gfc_expr *a, const gfc_expr *b, const char *name) } } +/* Check size of an array argument against a required size. + Returns true if the requirement is satisfied or if the size cannot be + determined, otherwise return false and raise a gfc_error */ + +static bool +array_size_check (gfc_expr *a, int n, long size_min) +{ + bool ok = true; + mpz_t size; + + if (gfc_array_size (a, &size)) +{ + HOST_WIDE_INT sz = gfc_mpz_get_hwi (size); + if (size_min >= 0 && sz < size_min) + { + gfc_error ("Size of %qs argument of %qs intrinsic at %L " + "too small (%wd/%ld)", + gfc_current_intrinsic_arg[n]->name, + gfc_current_intrinsic, &a->where, sz, size_min); + ok = false; + } + mpz_clear (size); +} + + return ok; +} + /* Check functions */ @@ -6539,6 +6566,27 @@ gfc_check_date_and_time (gfc_expr *date, gfc_expr *time, return false; if (!variable_check (values, 3, false)) return false; + if (!array_size_check (values, 3, 8)) + return false; + + if (values->ts.kind != gfc_default_integer_kind + && !gfc_notify_std (GFC_STD_F2018, "VALUES argument of " + "DATE_AND_TIME at %L has non-default kind", + &values->where)) + return false; + + /* F2018:16.9.59 DATE_AND_TIME + "VALUES shall be a rank-one array of type integer + with a decimal exponent range of at least four." + This is a hard limit also required by the implementation in + libgfortran. */ + if (values->ts.kind < 2) + { + gfc_error ("VALUES argument of DATE_AND_TIME at %L must have " + "a decimal exponent range of at least four", + &values->where); + return false; + } } return true; diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi index d4dd47fef0f..2c37cf4286a 100644 --- a/gcc/fortran/intrinsic.texi +++ b/gcc/fortran/intrinsic.texi @@ -4729,22 +4729,22 @@ end program test_ctime @item @emph{Description}: @code{DATE_AND_TIME(DATE, TIME, ZONE, VALUES)} gets the corresponding date and time information from the real-time system clock. @var{DATE} is -@code{INTENT(OUT)} and has form ccyymmdd. @var{TIME} is @code{INTENT(OUT)} and -has form hhmmss.sss. @var{ZONE} is @code{INTENT(OUT)} and has form (+-)hhmm, -representing the difference with respect to Coordinated Universal Time (UTC). -Unavailable time and date parameters return blanks. +@code{INTENT(OUT)} and of th
Re: Fortran: Use non conflicting file extensions for intermediates [PR81615]
Am 20.12.23 um 05:32 schrieb Rimvydas Jasinskas: Dear all, In the spirit of c/c++ using the .i/.ii extensions for intermediates, use the .fi/.fii intermediate extensions for gfortran fixed/free form sources when -save-temps is invoked to avoid various issues. I checked with Jerry on Mattermost that there are no objections regarding the suffixes. Pushed: https://gcc.gnu.org/g:ba615557a4c698d27042a5fe058ea6e721a03b12 Thanks for the patch! The documentation part will be submitted separately, because it involves adding a "Developer options" mini-section (as suggested by Harald) and moving the -fdump-* options from "Options for debugging your program or GNU Fortran" section. Documentation improvements are always great! Regards, Rimvydas Cheers, Harald
Re: Fortran: Use non conflicting file extensions for intermediates [PR81615]
Hi Rimvydas! Am 24.12.23 um 02:33 schrieb Rimvydas Jasinskas: Documentation part. The makeinfo gcc/fortran/gfortran.texi does not seem to have any new warnings. The patch is almost fine, except for a strange wording here: +@smallexample +gfortran -save-temps -c foo.F90 +@end smallexample + +preprocesses to in @file{foo.fii}, compiles to an intermediate +@file{foo.s}, and then assembles to the (implied) output file +@file{foo.o}, whereas: I understand the formulation is copied from gcc/doc/invoke.texi, where it does not fully make sense to me either. How about: "preprocesses input file @file{foo.F90} to @file{foo.fii}, ..." Furthermore, +@smallexample +gfortran -save-temps -S foo.F +@end smallexample + +saves the (no longer) temporary preprocessed file in @file{foo.fi}, and +then compiles to the (implied) output file @file{foo.s}. Even if this is copied from the gcc texinfo file, how about: "saves the preprocessor output in @file{foo.fi}, ..." which I find easier to read. Can you also add a reference to the PR number in the commit message? Is there a specific reason thy -fc-prototypes (Interoperability Options section) is excluded from manpage? Can you be more specific? I get here (since gcc-9): % man /opt/gcc/14/share/man/man1/gfortran.1 |grep -A 1 "Interoperability Options" Interoperability Options -fc-prototypes -fc-prototypes-external although no detailed explanation (-> gfortran.info). Regards, Rimvydas Thanks, Harald
Re: Fortran: Use non conflicting file extensions for intermediates [PR81615]
Hi Rimvydas! Am 28.12.23 um 08:09 schrieb Rimvydas Jasinskas: On Wed, Dec 27, 2023 at 10:34 PM Harald Anlauf wrote: The patch is almost fine, except for a strange wording here: +@smallexample +gfortran -save-temps -c foo.F90 +@end smallexample + +preprocesses to in @file{foo.fii}, compiles to an intermediate +@file{foo.s}, and then assembles to the (implied) output file +@file{foo.o}, whereas: I understand the formulation is copied from gcc/doc/invoke.texi, where it does not fully make sense to me either. How about: "preprocesses input file @file{foo.F90} to @file{foo.fii}, ..." Furthermore, +@smallexample +gfortran -save-temps -S foo.F +@end smallexample + +saves the (no longer) temporary preprocessed file in @file{foo.fi}, and +then compiles to the (implied) output file @file{foo.s}. Even if this is copied from the gcc texinfo file, how about: "saves the preprocessor output in @file{foo.fi}, ..." which I find easier to read. Can you also add a reference to the PR number in the commit message? I agree, wording sounds a lot better, included in v2 together with PR number. Yes, this is OK. Pushed: https://gcc.gnu.org/g:2cb93e6686e4af5725d8c919cf19f535a7f3aa33 Thanks for the patch! Is there a specific reason thy -fc-prototypes (Interoperability Options section) is excluded from manpage? Can you be more specific? I get here (since gcc-9): % man /opt/gcc/14/share/man/man1/gfortran.1 |grep -A 1 "Interoperability Options" Interoperability Options -fc-prototypes -fc-prototypes-external although no detailed explanation (-> gfortran.info). The https://gcc.gnu.org/onlinedocs/gfortran/Invoking-GNU-Fortran.html does contain a working link to https://gcc.gnu.org/onlinedocs/gfortran/Interoperability-Options.html However the manpage has Interoperability section explicitly disabled with "@c man end" ... "@c man begin ENVIRONMENT". After digging into git log it seems that Interoperability section was unintentionally added after this comment mark in https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=e655a6cc43 Yes, that might have been unintentional. Can you open a PR, and if you have a fix, attach it there? Thanks, Harald Best regards, Rimvydas
Re: Fortran: Use non conflicting file extensions for intermediates [PR81615]
Hi Thomas! Am 30.12.23 um 12:08 schrieb Thomas Koenig: Replying to myself... I think this also desevers a mention in changes.html. Here is something that I came up with. OK? Or does anybody have suggestions for a better wording? Or maybe this is better: diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index 4b83037a..d232f631 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -282,8 +282,14 @@ a work-in-progress. - - +Fortran + + With the -save-temps option, preprocessed files + with the .fii extension will be generated for + free-form source files such as .F90 and + .fi for fixed-form files such as .F. + + I slightly prefer this variant. I wonder if it were better to write "generated from" instead of "generated for". A native speaker might help here. While at it: gfortran now accepts "-std=f2023", which implies that the limit for line-length in free-form has been increased to 1 characters, and statements may have up to 1 million characters. (See Tobias' commit r14-5553-gb9eba3baf54b4f). I'd consider this as important as the other change... ;-) Thanks, Harald
Re: [Patch] Fortran: Accept -std=f2023, update line-length for Fortran 2023
Dear all, we might want to update changes.html to reflect this. How about: diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index 403feb06..9b16f5e3 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -284,6 +284,11 @@ a work-in-progress. Fortran + The compiler now accepts the -std=f2023 option, which +has been added in preparation of support of Fortran 2023. This option +increases the line-length limit for source in free-form to 1, and +statements may have up to 1 million characters. + With the -save-temps option, preprocessed files with the .fii extension will be generated from free-form source files such as .F90 and Cheers, Harald Am 17.11.23 um 12:38 schrieb Tobias Burnus: Hi Harald, hi all, On 16.11.23 20:30, Harald Anlauf wrote: On 11/16/23 14:01, Tobias Burnus wrote: This adds -std=f2023, which is mostly a prep patch for future changes. ... (B) In "6.3.2.6 Free form statements": Fortran 2018: "A statement shall not have more than 255 continuation lines." Fortran 2023: "A statement shall not have more than one million characters." this is really a funny change: we're not really prepared to handle this. I can confirm this. I tried to get it working in scanner.cc but due to the re-parsing it is quite difficult to get it right; the main problem is that we keep reparsing code ("gfc_current_locus = old_loc"), such that a simple count will be wrong. → Now tracked at https://gcc.gnu.org/PR112586 According to the standard one can have 99 lines with only "&" and then an ";", but then only 100 lines with 1 characters. I believe a single '&' is not valid, you either need '&&' or something else + '&'; thus, you can have only half a million lines + 1. In the code, I still use 1,000,000 but now with a comment. There is a similar wording for fixed-form which you overlooked: Ups - fixed. If you think that we need testcases for fixed-form, add them, or forget them. I don't bother. I added one. - there are existing testcases continuation_5.f, continuation_6.f, thus I suggest to rename your new continuation_{5,6}.f90 to continuation_17.f90+ . Done. We are rather inconsistent whether we enumerate .f{,90} together or separately; as the suffix is shown, either works. - I don't understand your new testcase line_length_14.f90 . This is supposed to test -std=gnu, but then -std=gnu is not a standard but a moving target, which is why you had to adjust existing testcases. So what does it buy us beyond line_length_1{2,3}.f90 ? Well, it ensures that the warning is not only shown for -std=f2023 but also for -std=f2028 and (current -std=gnu). In general, I think it is useful to check the lower and the upper bound. I have now removed it - as it is unlikely that we would regress on such changes. PPS: I did not bother adding .f23 as file extension; I believe that also .f18 is unsupported. I never use extensions other than .f90 for portable code. Likewise - especially as '.f95' starts out as Fortran code that complies to -std=f95 but slowly Fortran 2003 or later code creeps in. I think that's fine but then one can also directly use .f90. (Most code does so.) Unless there are follow up comments, I will commit it later today. Thanks for the comments! Tobias PS: I fixed the wording issue in the subject line of the email and header. I first wrote 'support' but that sounded a bit as if F2023 is supported. Hence, I wrote 'Accept' and did not remove 'support'. - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Re: [Patch] Fortran: Accept -std=f2023, update line-length for Fortran 2023
Am 02.01.24 um 20:37 schrieb Steve Kargl: On Tue, Jan 02, 2024 at 08:31:15PM +0100, Harald Anlauf wrote: we might want to update changes.html to reflect this. How about: diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index 403feb06..9b16f5e3 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -284,6 +284,11 @@ a work-in-progress. Fortran + The compiler now accepts the -std=f2023 option, which +has been added in preparation of support of Fortran 2023. This option +increases the line-length limit for source in free-form to 1, and +statements may have up to 1 million characters. + With the -save-temps option, preprocessed files with the .fii extension will be generated from free-form source files such as .F90 and LGTM. Thanks, this is now pushed.
[PATCH, committed] Fortran: fix FE memleak
Dear all, I've committed the attached, simple & obvious patch for a gmp memory leak in gfc_get_nodesc_array_type that shows up when running f951 under valgrind e.g. on testcase gfortran.dg/class_optional_2.f90, after regtesting on x86_64-pc-linux-gnu. (Note that this does not address the underlying issues of pr55978). Thanks, Harald From 93c96e3ad0024a397115aa17bf29c7efc6b535a1 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 3 Jan 2024 20:21:00 +0100 Subject: [PATCH] Fortran: fix FE memleak gcc/fortran/ChangeLog: * trans-types.cc (gfc_get_nodesc_array_type): Clear used gmp variables. --- gcc/fortran/trans-types.cc | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc index e6db1c95450..676014e9b98 100644 --- a/gcc/fortran/trans-types.cc +++ b/gcc/fortran/trans-types.cc @@ -1795,7 +1795,7 @@ gfc_get_nodesc_array_type (tree etype, gfc_array_spec * as, gfc_packed packed, TYPE_LANG_SPECIFIC (type) = TYPE_LANG_SPECIFIC (TREE_TYPE (type)); } - return type; + goto array_type_done; } if (known_stride) @@ -1814,10 +1814,6 @@ gfc_get_nodesc_array_type (tree etype, gfc_array_spec * as, gfc_packed packed, layout_type (type); - mpz_clear (offset); - mpz_clear (stride); - mpz_clear (delta); - /* Represent packed arrays as multi-dimensional if they have rank > 1 and with proper bounds, instead of flat arrays. This makes for better debug info. */ @@ -1848,6 +1844,12 @@ gfc_get_nodesc_array_type (tree etype, gfc_array_spec * as, gfc_packed packed, GFC_ARRAY_TYPE_P (type) = 1; TYPE_LANG_SPECIFIC (type) = TYPE_LANG_SPECIFIC (TREE_TYPE (type)); } + +array_type_done: + mpz_clear (offset); + mpz_clear (stride); + mpz_clear (delta); + return type; } -- 2.35.3
[PATCH] Fortran: bogus warnings with REPEAT intrinsic and -Wconversion-extra [PR96724]
Dear all, the attached patch picks up a submission by Jose that was never reviewed: https://gcc.gnu.org/pipermail/fortran/2020-August/054902.html The original patch was unnecessarily complex, as it could invoke more conversions than necessary. I chose to only convert to the essential - and common - gfc_charlen_int_kind. It's almost trivial now. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 18f212aaca8a13fbd2f40cc7636b1a20185cc01e Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 5 Jan 2024 22:24:25 +0100 Subject: [PATCH] Fortran: bogus warnings with REPEAT intrinsic and -Wconversion-extra [PR96724] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gcc/fortran/ChangeLog: PR fortran/96724 * iresolve.cc (gfc_resolve_repeat): Force conversion of NCOPIES to gfc_charlen_int_kind before call to gfc_multiply. gcc/testsuite/ChangeLog: PR fortran/96724 * gfortran.dg/repeat_8.f90: New test. Co-authored-by: José Rui Faustino de Sousa --- gcc/fortran/iresolve.cc| 7 +- gcc/testsuite/gfortran.dg/repeat_8.f90 | 123 + 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/repeat_8.f90 diff --git a/gcc/fortran/iresolve.cc b/gcc/fortran/iresolve.cc index 5bb0bbc477b..f25a5ba3aef 100644 --- a/gcc/fortran/iresolve.cc +++ b/gcc/fortran/iresolve.cc @@ -2363,7 +2363,12 @@ gfc_resolve_repeat (gfc_expr *f, gfc_expr *string, } if (tmp) -f->ts.u.cl->length = gfc_multiply (tmp, gfc_copy_expr (ncopies)); +{ + /* Force-convert ncopies to gfc_charlen_int_kind. */ + gfc_expr *e = gfc_copy_expr (ncopies); + gfc_convert_type_warn (e, &tmp->ts, 2, 0); + f->ts.u.cl->length = gfc_multiply (tmp, e); +} } diff --git a/gcc/testsuite/gfortran.dg/repeat_8.f90 b/gcc/testsuite/gfortran.dg/repeat_8.f90 new file mode 100644 index 000..9dd379ac96b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/repeat_8.f90 @@ -0,0 +1,123 @@ +! { dg-do compile } +! { dg-additional-options "-Wconversion-extra" } +! +! Test fix for PR fortran/96724 +! +! Contributed by José Rui Faustino de Sousa + +program repeat_p + use, intrinsic :: iso_fortran_env, only: int8, int16, int32, int64 + implicit none + + integer, parameter :: n = 20 + integer, parameter :: ucs4 = selected_char_kind ('ISO_10646') + + integer(kind=int8), parameter :: p08 = int(n, kind=int8) + integer(kind=int16), parameter :: p16 = int(n, kind=int16) + integer(kind=int16), parameter :: p32 = int(n, kind=int32) + integer(kind=int16), parameter :: p64 = int(n, kind=int64) + + integer(kind=int8) :: i08 + integer(kind=int16) :: i16 + integer(kind=int32) :: i32 + integer(kind=int64) :: i64 + + character(len=n,kind=1):: c + character(len=n,kind=ucs4) :: d + + i08 = p08 + c = repeat('X', 20_int8) + c = repeat('X', i08) + c = repeat('X', p08) + c = repeat('X', len08(c)) + d = repeat(ucs4_'X', 20_int8) + d = repeat(ucs4_'X', i08) + d = repeat(ucs4_'X', p08) + d = repeat(ucs4_'X', len08(c)) + i16 = p16 + c = repeat('X', 20_int16) + c = repeat('X', i16) + c = repeat('X', p16) + c = repeat('X', len16(c)) + d = repeat(ucs4_'X', 20_int16) + d = repeat(ucs4_'X', i16) + d = repeat(ucs4_'X', p16) + d = repeat(ucs4_'X', len16(c)) + i32 = p32 + c = repeat('X', 20_int32) + c = repeat('X', i32) + c = repeat('X', p32) + c = repeat('X', len32(c)) + d = repeat(ucs4_'X', 20_int32) + d = repeat(ucs4_'X', i32) + d = repeat(ucs4_'X', p32) + d = repeat(ucs4_'X', len32(c)) + i64 = p64 + c = repeat('X', 20_int64) + c = repeat('X', i64) + c = repeat('X', p64) + c = repeat('X', len64(c)) + d = repeat(ucs4_'X', 20_int64) + d = repeat(ucs4_'X', i64) + d = repeat(ucs4_'X', p64) + d = repeat(ucs4_'X', len64(c)) + +contains + + function len08(x) result(l) +character(len=*), intent(in) :: x +integer(kind=int8) :: l + +l = int(len(x), kind=int8) + end function len08 + + function len16(x) result(l) +character(len=*), intent(in) :: x +integer(kind=int16) :: l + +l = int(len(x), kind=int16) + end function len16 + + function len32(x) result(l) +character(len=*), intent(in) :: x +integer(kind=int32) :: l + +l = int(len(x), kind=int32) + end function len32 + + function len64(x) result(l) +character(len=*), intent(in) :: x +integer(kind=int64) :: l + +l = int(len(x), kind=int64) + end function len64 + + function ulen08(x) result(l) +character(len=*,kind=ucs4), intent(in) :: x +integer(kind=int8) :: l + +l = int(len(x), kind=int8)
Re: [libgfortran, patch] PR113223 NAMELIST internal write missing leading blank character
Hi Jerry! On 1/7/24 19:40, Jerry D wrote: Committed as simple and obvious. Initial patch thanks to Steve. When using git gcc-commit-mklog how does one add in the coauthor? % git help gcc-commit-mklog ... --co CO Add Co-Authored-By trailer (comma separated) However, I usually add this line manually later. Regarding the format, have a look at existing log messages. Cheers, Harald The master branch has been updated by Jerry DeLisle : https://gcc.gnu.org/g:add995ec117d756e61d207041cd32f937c1a1cd9 commit r14-6986-gadd995ec117d756e61d207041cd32f937c1a1cd9 Author: Jerry DeLisle Date: Sun Jan 7 10:22:19 2024 -0800 libgfortran: Emit a space at beginning of internal unit NML. PR libgfortran/113223 libgfortran/ChangeLog: * io/write.c (namelist_write): If internal_unit precede with space. gcc/testsuite/ChangeLog: * gfortran.dg/dtio_25.f90: Update. * gfortran.dg/namelist_57.f90: Update. * gfortran.dg/namelist_65.f90: Update.
[PATCH] Fortran: SIZE optional DIM argument having OPTIONAL+VALUE attributes [PR113245]
Dear all, the attached, actually rather obvious patch fixes an issue when an optional dummy with the value attribute was passed as DIM argument to the SIZE intrinsic. Instead of some hand-crafted, incomplete presence check for the argument, it makes more sense to rely on gfc_conv_expr_present(). Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 49f5c89f6bdddbb225ca70f8df78a75252b0b2d5 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 7 Jan 2024 22:24:25 +0100 Subject: [PATCH] Fortran: SIZE optional DIM argument having OPTIONAL+VALUE attributes [PR113245] gcc/fortran/ChangeLog: PR fortran/113245 * trans-intrinsic.cc (gfc_conv_intrinsic_size): Use gfc_conv_expr_present() for proper check of optional DIM argument. gcc/testsuite/ChangeLog: PR fortran/113245 * gfortran.dg/size_optional_dim_2.f90: New test. --- gcc/fortran/trans-intrinsic.cc| 4 +-- .../gfortran.dg/size_optional_dim_2.f90 | 31 +++ 2 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/size_optional_dim_2.f90 diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc index d973c49380c..74139262657 100644 --- a/gcc/fortran/trans-intrinsic.cc +++ b/gcc/fortran/trans-intrinsic.cc @@ -8025,9 +8025,6 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr) argse.data_not_needed = 1; gfc_conv_expr (&argse, actual->expr); gfc_add_block_to_block (&se->pre, &argse.pre); - cond = fold_build2_loc (input_location, NE_EXPR, logical_type_node, - argse.expr, null_pointer_node); - cond = gfc_evaluate_now (cond, &se->pre); /* 'block2' contains the arg2 absent case, 'block' the arg2 present case; size_var can be used in both blocks. */ tree size_var = gfc_create_var (TREE_TYPE (size), "size"); @@ -8038,6 +8035,7 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr) tmp = fold_build2_loc (input_location, MODIFY_EXPR, TREE_TYPE (size_var), size_var, size); gfc_add_expr_to_block (&block2, tmp); + cond = gfc_conv_expr_present (actual->expr->symtree->n.sym); tmp = build3_v (COND_EXPR, cond, gfc_finish_block (&block), gfc_finish_block (&block2)); gfc_add_expr_to_block (&se->pre, tmp); diff --git a/gcc/testsuite/gfortran.dg/size_optional_dim_2.f90 b/gcc/testsuite/gfortran.dg/size_optional_dim_2.f90 new file mode 100644 index 000..698702b0974 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/size_optional_dim_2.f90 @@ -0,0 +1,31 @@ +! { dg-do run } +! { dg-additional-options "-fdump-tree-original" } +! PR fortran/113245 - SIZE, optional DIM argument, w/ OPTIONAL+VALUE attributes + +program p + implicit none + real:: a(2,3) + integer :: expect + expect = size (a,2) + call ref (a,2) + call val (a,2) + expect = size (a) + call ref (a) + call val (a) +contains + subroutine ref (x, dim) +real, intent(in) :: x(:,:) +integer, optional, intent(in) :: dim +print *, "present(dim), size(a,dim) =", present (dim), size (x,dim=dim) +if (size (x,dim=dim) /= expect) stop 1 + end + subroutine val (x, dim) +real, intent(in) :: x(:,:) +integer, optional, value :: dim +print *, "present(dim), size(a,dim) =", present (dim), size (x,dim=dim) +if (size (x,dim=dim) /= expect) stop 2 + end +end + +! Ensure inline code is generated: +! { dg-final { scan-tree-dump-not "_gfortran_size" "original" } } -- 2.35.3
Re: [Patch, fortran PR89645/99065 No IMPLICIT type error with: ASSOCIATE( X => function() )
Hi Paul, your patch looks already very impressive! Regarding the patch as is, I am still trying to grok it, even with your explanations at hand... While the testcase works as advertised, I noticed that it exhibits a runtime memleak that occurs for (likely) each case where the associate target is an allocatable, class-valued function result. I tried to produce a minimal testcase using class(*), which apparently is not handled by your patch (it ICEs for me): program p implicit none class(*), allocatable :: x(:) x = foo() call prt (x) deallocate (x) ! up to here no memleak... associate (var => foo()) call prt (var) end associate contains function foo() result(res) class(*), allocatable :: res(:) res = [42] end function foo subroutine prt (x) class(*), intent(in) :: x(:) select type (x) type is (integer) print *, x class default stop 99 end select end subroutine prt end Traceback (truncated): foo.f90:9:18: 9 | call prt (var) | 1 internal compiler error: tree check: expected record_type or union_type or qual_union_type, have function_type in gfc_class_len_get, at fortran/trans-expr.cc:271 0x19fd5d5 tree_check_failed(tree_node const*, char const*, int, char const*, ...) ../../gcc-trunk/gcc/tree.cc:8952 0xe1562d tree_check3(tree_node*, char const*, int, char const*, tree_code, tree_code, tree_code) ../../gcc-trunk/gcc/tree.h:3652 0xe3e264 gfc_class_len_get(tree_node*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:271 0xecda48 trans_associate_var ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2325 0xecdd09 gfc_trans_block_construct(gfc_code*) ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2383 [...] I don't see anything wrong with it: NAG groks it, like Nvidia and Flang, while Intel crashes at runtime. Can you have another brief look? Thanks, Harald On 1/6/24 18:26, Paul Richard Thomas wrote: These PRs come about because of gfortran's single pass parsing. If the function in the title is parsed after the associate construct, then its type and rank are not known. The point at which this becomes a problem is when expressions within the associate block are parsed. primary.cc (gfc_match_varspec) could already deal with intrinsic types and so component references were the trigger for the problem. The two major parts of this patch are the fixup needed in gfc_match_varspec and the resolution of expressions with references in resolve.cc (gfc_fixup_inferred_type_refs). The former relies on the two new functions in symbol.cc to search for derived types with an appropriate component to match the component reference and then set the associate name to have a matching derived type. gfc_fixup_inferred_type_refs is called in resolution and so the type of the selector function is known. gfc_fixup_inferred_type_refs ensures that the component references use this derived type and that array references occur in the right place in expressions and match preceding array specs. Most of the work in preparing the patch was sorting out cases where the selector was not a derived type but, instead, a class function. If it were not for this, the patch would have been submitted six months ago :-( The patch is relatively safe because most of the chunks are guarded by testing for the associate name being an inferred type, which is set in gfc_match_varspec. For this reason, I do not think it likely that the patch will cause regressions. However, it is more than possible that variants not appearing in the submitted testcase will throw up new bugs. Jerry has already given the patch a whirl and found that it applies cleanly, regtests OK and works as advertised. OK for trunk? Paul Fortran: Fix class/derived type function associate selectors [PR87477] 2024-01-06 Paul Thomas gcc/fortran PR fortran/87477 PR fortran/89645 PR fortran/99065 * class.cc (gfc_change_class): New function needed for associate names, when rank changes or a derived type is produced by resolution * dump-parse-tree.cc (show_code_node): Make output for SELECT TYPE more comprehensible. * gfortran.h : Add 'gfc_association_list' to structure 'gfc_association_list'. Add prototypes for 'gfc_find_derived_types', 'gfc_fixup_inferred_type_refs' and 'gfc_change_class'. Add macro IS_INFERRED_TYPE. * match.cc (copy_ts_from_selector_to_associate): Add bolean arg 'select_type' with default false. If this is a select type name and the selector is a inferred type, build the class type and apply it to the associate name. (build_associate_name): Pass true to 'select_type' in call to previous. * parse.cc (parse_associate): If the selector is a inferred type the associate name is too. Make sure that function selector class and rank, if known, are passed to the associate name. If a function result exists, pass its typespec to the associate name. * primary.cc (gfc_match_varspec): If a scalar derived type select type temporary has an array reference, matc
[PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305]
Dear all, we are accepting loop annotations IVDEP, UNROLL n, VECTOR, and NOVECTOR for ordinary do loops, but ICE when such an annotation is specified before a DO CONCURRENT loop. Since at least the Intel compilers recognize some of the annotations also for DO CONCURRENT, it seems natural to extend gfortran instead of rejecting or ignoring the attributes. The attached patch handles the annotations as needed for the control structures of FORALL/DO CONCURRENT. Regarding the UNROLL directive, I don't have good references, so feedback is welcome. The current patch applies UNROLL only to the first loop control variable (for the case of loop nests), which translates into the innermost loop in gcc's representation. Regtested on x86_64-pc-linux-gnu. OK for mainline? Comments? Thanks, Harald From 0df60f02c399a6bf65850ecd5719b25b3de6676f Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 10 Jan 2024 23:10:02 +0100 Subject: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305] gcc/fortran/ChangeLog: PR fortran/113305 * gfortran.h: Add annotation controls to gfc_forall_iterator. * gfortran.texi: Document annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. * parse.cc (parse_do_block): Parse annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. Apply UNROLL only to first loop control variable. * trans-stmt.cc (gfc_trans_forall_loop): Annotate loops with IVDEP, UNROLL n, VECTOR, NOVECTOR as needed for DO CONCURRENT. (gfc_trans_forall_1): Handle annotations IVDEP, UNROLL n, VECTOR, NOVECTOR. gcc/testsuite/ChangeLog: PR fortran/113305 * gfortran.dg/do_concurrent_7.f90: New test. --- gcc/fortran/gfortran.h| 4 +++ gcc/fortran/gfortran.texi | 12 gcc/fortran/parse.cc | 26 - gcc/fortran/trans-stmt.cc | 29 ++- gcc/testsuite/gfortran.dg/do_concurrent_7.f90 | 26 + 5 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/do_concurrent_7.f90 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 82f388c05f8..88502c1e3f0 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2926,6 +2926,10 @@ gfc_dt; typedef struct gfc_forall_iterator { gfc_expr *var, *start, *end, *stride; + unsigned short unroll; + bool ivdep; + bool vector; + bool novector; struct gfc_forall_iterator *next; } gfc_forall_iterator; diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 5615fee2897..371666dcbb6 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -3262,6 +3262,9 @@ It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. N is an integer constant specifying the unrolling factor. The values of 0 and 1 block any unrolling of the loop. +For @code{DO CONCURRENT} constructs the unrolling specification applies +only to the first loop control variable. + @node BUILTIN directive @subsection BUILTIN directive @@ -3300,6 +3303,9 @@ whether a particular loop is vectorizable due to potential dependencies between iterations. The purpose of the directive is to tell the compiler that vectorization is safe. +For @code{DO CONCURRENT} constructs this annotation is implicit to all +loop control variables. + This directive is intended for annotation of existing code. For new code it is recommended to consider OpenMP SIMD directives as potential alternative. @@ -3316,6 +3322,9 @@ This directive tells the compiler to vectorize the following loop. It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. +For @code{DO CONCURRENT} constructs this annotation applies to all loops +specified in the concurrent header. + @node NOVECTOR directive @subsection NOVECTOR directive @@ -3328,6 +3337,9 @@ This directive tells the compiler to not vectorize the following loop. It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. +For @code{DO CONCURRENT} constructs this annotation applies to all loops +specified in the concurrent header. + @node Non-Fortran Main Program @section Non-Fortran Main Program diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc index d8b38cfb5ac..f41cc7d3510 100644 --- a/gcc/fortran/parse.cc +++ b/gcc/fortran/parse.cc @@ -5307,7 +5307,31 @@ parse_do_block (void) do_op = new_st.op; s.ext.end_do_label = new_st.label1; - if (new_st.ext.iterator != NULL) + if (do_op == EXEC_DO_CONCURRENT) +{ + gfc_forall_iterator *fa; + for (fa = new_st.ext.forall_iterator; fa; fa = fa->next) + { + /* Apply unroll only to innermost loop (first control + variable). */ + if (directive_unroll != -1) + { + fa->unroll = directive_unroll; + directive_unroll = -1; + } + if (directive_ivdep) + fa->ivde
[PATCH, v2] Fortran: annotations for DO CONCURRENT loops [PR113305]
Hi Bernhard, On 1/12/24 10:44, Bernhard Reutner-Fischer wrote: On Wed, 10 Jan 2024 23:24:22 +0100 Harald Anlauf wrote: diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 82f388c05f8..88502c1e3f0 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2926,6 +2926,10 @@ gfc_dt; typedef struct gfc_forall_iterator { gfc_expr *var, *start, *end, *stride; + unsigned short unroll; + bool ivdep; + bool vector; + bool novector; struct gfc_forall_iterator *next; } [] diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc index a718dce237f..59a9cf99f9b 100644 --- a/gcc/fortran/trans-stmt.cc +++ b/gcc/fortran/trans-stmt.cc @@ -41,6 +41,10 @@ typedef struct iter_info tree start; tree end; tree step; + unsigned short unroll; + bool ivdep; + bool vector; + bool novector; struct iter_info *next; } Given that we already have in gfortran.h typedef struct { gfc_expr *var, *start, *end, *step; unsigned short unroll; bool ivdep; bool vector; bool novector; } gfc_iterator; would it make sense to break out these loop annotation flags into its own let's say struct gfc_iterator_flags and use pointers to that flags instead? I've created a struct gfc_loop_annot and use that directly as I think using pointers to it is probably not very efficient. Well, the struct is smaller than a pointer on a 64-bit system... LGTM otherwise. Thanks for the patch! Thanks for the review! If there are no further comments, I'll commit the attached version soon. Harald From 31d8957a95455663577a0e60109679d56aac234d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 12 Jan 2024 19:51:11 +0100 Subject: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305] gcc/fortran/ChangeLog: PR fortran/113305 * gfortran.h (gfc_loop_annot): New. (gfc_iterator, gfc_forall_iterator): Use for annotation control. * array.cc (gfc_copy_iterator): Adjust. * gfortran.texi: Document annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. * parse.cc (parse_do_block): Parse annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. Apply UNROLL only to first loop control variable. * trans-stmt.cc (iter_info): Use gfc_loop_annot. (gfc_trans_simple_do): Adjust. (gfc_trans_forall_loop): Annotate loops with IVDEP, UNROLL n, VECTOR, NOVECTOR as needed for DO CONCURRENT. (gfc_trans_forall_1): Handle loop annotations. gcc/testsuite/ChangeLog: PR fortran/113305 * gfortran.dg/do_concurrent_7.f90: New test. --- gcc/fortran/array.cc | 5 +- gcc/fortran/gfortran.h| 11 - gcc/fortran/gfortran.texi | 12 + gcc/fortran/parse.cc | 34 -- gcc/fortran/trans-stmt.cc | 46 ++- gcc/testsuite/gfortran.dg/do_concurrent_7.f90 | 26 +++ 6 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/do_concurrent_7.f90 diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc index 19456baf103..81fa99d219f 100644 --- a/gcc/fortran/array.cc +++ b/gcc/fortran/array.cc @@ -2308,10 +2308,7 @@ gfc_copy_iterator (gfc_iterator *src) dest->start = gfc_copy_expr (src->start); dest->end = gfc_copy_expr (src->end); dest->step = gfc_copy_expr (src->step); - dest->unroll = src->unroll; - dest->ivdep = src->ivdep; - dest->vector = src->vector; - dest->novector = src->novector; + dest->annot = src->annot; return dest; } diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 82f388c05f8..fd73e4ce431 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2830,14 +2830,22 @@ gfc_case; #define gfc_get_case() XCNEW (gfc_case) +/* Annotations for loop constructs. */ typedef struct { - gfc_expr *var, *start, *end, *step; unsigned short unroll; bool ivdep; bool vector; bool novector; } +gfc_loop_annot; + + +typedef struct +{ + gfc_expr *var, *start, *end, *step; + gfc_loop_annot annot; +} gfc_iterator; #define gfc_get_iterator() XCNEW (gfc_iterator) @@ -2926,6 +2934,7 @@ gfc_dt; typedef struct gfc_forall_iterator { gfc_expr *var, *start, *end, *stride; + gfc_loop_annot annot; struct gfc_forall_iterator *next; } gfc_forall_iterator; diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 5615fee2897..371666dcbb6 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -3262,6 +3262,9 @@ It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. N is an integer constant specifying the unrolling factor. The values of 0 and 1 block any unrolling of the loop. +For @code{DO CONCURRENT} constructs the unrolling specification applies +only to the first loop control variable. + @node BUILTIN directive @subsection BUILTIN directive
[PATCH] Fortran: intrinsic ISHFTC and missing optional argument SIZE [PR67277]
Dear all, the testcase given in PR67277 actually consists of two separate issues: (1) passing an optional dummy argument to an elemental (intrinsic) procedure (2) a missing optional argument for SIZE to the ISHFTC intrinsic shall be equivalent to using BIT_SIZE(I). I've created a separate PR113377 for (1), as this looks like a more general issue with the scalarizer. The attached, rather simple and obvious patch thus fixes (2). Besides testing that the patch works as advertised, the testcase also contains variations that need fixing of PR113377 before they can be uncommented. Regtested on x86_64-pc-linux-gnu. OK for mainline? As I consider the patch safe, I'd like to backport to 13-branch later. Thanks, Harald P.S.: if someone out there feels familiar with the scalarizer, a look at PR113377 is appreciated. From 20da56165273c8814b3c53e6d71549ba6a37e0cd Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 13 Jan 2024 22:00:21 +0100 Subject: [PATCH] Fortran: intrinsic ISHFTC and missing optional argument SIZE [PR67277] gcc/fortran/ChangeLog: PR fortran/67277 * trans-intrinsic.cc (gfc_conv_intrinsic_ishftc): Handle optional dummy argument for SIZE passed to ISHFTC. Set default value to BIT_SIZE(I) when missing. gcc/testsuite/ChangeLog: PR fortran/67277 * gfortran.dg/ishftc_optional_size_1.f90: New test. --- gcc/fortran/trans-intrinsic.cc| 14 +++ .../gfortran.dg/ishftc_optional_size_1.f90| 97 +++ 2 files changed, 111 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc index 74139262657..0468dfae2b1 100644 --- a/gcc/fortran/trans-intrinsic.cc +++ b/gcc/fortran/trans-intrinsic.cc @@ -6863,9 +6863,23 @@ gfc_conv_intrinsic_ishftc (gfc_se * se, gfc_expr * expr) if (num_args == 3) { + gfc_expr *size = expr->value.function.actual->next->next->expr; + /* Use a library function for the 3 parameter version. */ tree int4type = gfc_get_int_type (4); + /* Treat optional SIZE argument when it is passed as an optional + dummy. If SIZE is absent, the default value is BIT_SIZE(I). */ + if (size->expr_type == EXPR_VARIABLE + && size->symtree->n.sym->attr.dummy + && size->symtree->n.sym->attr.optional) + { + tree type_of_size = TREE_TYPE (args[2]); + args[2] = build3_loc (input_location, COND_EXPR, type_of_size, +gfc_conv_expr_present (size->symtree->n.sym), +args[2], fold_convert (type_of_size, nbits)); + } + /* We convert the first argument to at least 4 bytes, and convert back afterwards. This removes the need for library functions for all argument sizes, and function will be diff --git a/gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 b/gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 new file mode 100644 index 000..1ccf4b38caa --- /dev/null +++ b/gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 @@ -0,0 +1,97 @@ +! { dg-do run } +! +! PR fortran/67277 - ISHFTC and missing optional argument SIZE + +module m + implicit none +contains + ! Optional argument passed by reference + elemental function ishftc4_ref (i, shift, size_) result(r) +integer(4), intent(in) :: i +integer,intent(in) :: shift +integer,intent(in), optional :: size_ +integer :: r +r = ishftc (i, shift=shift, size=size_) + end + + elemental function ishftc1_ref (i, shift, size_) result(r) +integer(1), intent(in) :: i +integer,intent(in) :: shift +integer(1), intent(in), optional :: size_ +integer(1) :: r +r = ishftc (i, shift=shift, size=size_) + end + + ! Array valued argument i + function ishftc4_ref_4 (i, shift, size_) result(r) +integer(4), intent(in) :: i(4) +integer,intent(in) :: shift +integer,intent(in), optional :: size_ +integer :: r(size(i)) +r = ishftc (i, shift=shift, size=size_) + end + + ! Optional argument passed by value + elemental function ishftc4_val (i, shift, size_) result(r) +integer(4), intent(in) :: i +integer,intent(in) :: shift +integer,value, optional :: size_ +integer :: r +r = ishftc (i, shift=shift, size=size_) + end + + elemental function ishftc1_val (i, shift, size_) result(r) +integer(1), intent(in) :: i +integer,intent(in) :: shift +integer(1), value, optional :: size_ +integer(1) :: r +r = ishftc (i, shift=shift, size=size_) + end + + ! Array valued argument i + function ishftc4_val_4 (i, shift, size_) result(r) +integer(4), intent(in) :: i(4) +integer,intent(in) ::
[PATCH, committed] Fortran: fix wrong array bounds check [PR113471]
Dear all, I've pushed the attached obvious patch for a regression due to a wrong array bounds check after regtesting on x86_64-pc-linux-gnu and verification of the fix by the reporter in the PR. https://gcc.gnu.org/g:94b2e6cb1cc4feb122bf77f19a657c97bffa9b42 Thanks, Harald From 94b2e6cb1cc4feb122bf77f19a657c97bffa9b42 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 19 Jan 2024 21:20:44 +0100 Subject: [PATCH] Fortran: fix wrong array bounds check [PR113471] gcc/fortran/ChangeLog: PR fortran/113471 * trans-array.cc (array_bound_check_elemental): Array bounds check shall apply here to elemental dimensions of an array section only. gcc/testsuite/ChangeLog: PR fortran/113471 * gfortran.dg/bounds_check_24.f90: New test. --- gcc/fortran/trans-array.cc| 2 +- gcc/testsuite/gfortran.dg/bounds_check_24.f90 | 28 +++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_24.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 26e7adaa03f..878a92aff18 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3600,7 +3600,7 @@ array_bound_check_elemental (gfc_se * se, gfc_ss * ss, gfc_expr * expr) continue; } - if (ref->type == REF_ARRAY && ref->u.ar.dimen > 0) + if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION) { ar = &ref->u.ar; for (dim = 0; dim < ar->dimen; dim++) diff --git a/gcc/testsuite/gfortran.dg/bounds_check_24.f90 b/gcc/testsuite/gfortran.dg/bounds_check_24.f90 new file mode 100644 index 000..d0251e8455b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bounds_check_24.f90 @@ -0,0 +1,28 @@ +! { dg-do compile } +! { dg-additional-options "-fcheck=bounds -fdump-tree-original" } +! +! PR fortran/113471 - wrong array bounds check + +program pr113471 + implicit none + type t + integer, dimension(2) :: c1 = 0 + end type t + type(t) :: cc(7), bb(7) + integer :: kk = 1 + + ! no bounds check (can be determined at compile time): + call foo (cc(7)% c1) + + ! bounds check involving kk, but no "outside of expected range" + call foo (bb(kk)% c1) + +contains + subroutine foo (c) +integer, intent(in) :: c(:) + end +end + +! { dg-final { scan-tree-dump-times "below lower bound" 2 "original" } } +! { dg-final { scan-tree-dump-times "above upper bound" 2 "original" } } +! { dg-final { scan-tree-dump-not "outside of expected range" "original" } } -- 2.35.3
Re: PR82943 - Suggested patch to fix
Am 20.01.24 um 20:08 schrieb Jerry D: On 1/20/24 10:46 AM, Alexander Westbrooks wrote: Hello and Happy New Year! I wanted to follow up on this patch I made to address PR82943 for GFortran. Has anyone had a chance to review it? Thanks, Alexander Westbrooks Inserting myself in here just a little bit. I will apply and test today if I can. Paul is unavailable for a few weeks. Harald can chime in. Do you have commit rights for gcc? I am not aware of Alex having a copyright assignment on file, or a DCO certificate, and the patch is not signed off. But I might be wrong. Your efforts are appreciated. Regards, Jerry
Re: PR82943 - Suggested patch to fix
Am 20.01.24 um 21:37 schrieb Jerry D: On 1/20/24 12:08 PM, Harald Anlauf wrote: Am 20.01.24 um 20:08 schrieb Jerry D: On 1/20/24 10:46 AM, Alexander Westbrooks wrote: Hello and Happy New Year! I wanted to follow up on this patch I made to address PR82943 for GFortran. Has anyone had a chance to review it? Thanks, Alexander Westbrooks Inserting myself in here just a little bit. I will apply and test today if I can. Paul is unavailable for a few weeks. Harald can chime in. Do you have commit rights for gcc? I am not aware of Alex having a copyright assignment on file, or a DCO certificate, and the patch is not signed off. But I might be wrong. --- snip --- I do not mind committing this but need clarifications regarding the copyright (copyleft?) rules in this case. In the past we have allowed small contributions like this. This may be a little more than minor. It is. This is why I pointed to: https://gcc.gnu.org/dco.html Regardless it appears to do the job! Jerry
[PATCH] Fortran: passing of optional scalar arguments with VALUE attribute [PR113377]
Dear all, here's the first part of an attempt to fix issues with optional dummy arguments as actual arguments to optional dummies. This patch rectifies the case of scalar dummies with the VALUE attribute, which in gfortran's argument passing convention are passed on the stack when they are of intrinsic type, and have a hidden variable for the presence status. The testcase tries to cover valid combinations of actual and dummy argument. A few tests that are not standard-conforming but would still work with gfortran (due to the argument passing convention) are left there but commented out with a pointer to the standard (thanks, Mikael!). Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From f6a65138391c902d2782973665059d7d059a50d1 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 20 Jan 2024 22:18:02 +0100 Subject: [PATCH] Fortran: passing of optional scalar arguments with VALUE attribute [PR113377] gcc/fortran/ChangeLog: PR fortran/113377 * trans-expr.cc (gfc_conv_procedure_call): Fix handling of optional scalar arguments of intrinsic type with the VALUE attribute. gcc/testsuite/ChangeLog: PR fortran/113377 * gfortran.dg/optional_absent_9.f90: New test. --- gcc/fortran/trans-expr.cc | 5 + .../gfortran.dg/optional_absent_9.f90 | 324 ++ 2 files changed, 329 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/optional_absent_9.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 9dd1f4086f4..2f47a75955c 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6526,6 +6526,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, gfc_init_se (&argse, NULL); argse.want_pointer = 1; gfc_conv_expr (&argse, e); + if (e->symtree->n.sym->attr.dummy +&& POINTER_TYPE_P (TREE_TYPE (argse.expr))) + argse.expr = gfc_build_addr_expr (NULL_TREE, +argse.expr); cond = fold_convert (TREE_TYPE (argse.expr), null_pointer_node); cond = fold_build2_loc (input_location, NE_EXPR, @@ -7256,6 +7260,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && e->symtree->n.sym->attr.optional && (((e->rank != 0 && elemental_proc) || e->representation.length || e->ts.type == BT_CHARACTER + || (e->rank == 0 && e->symtree->n.sym->attr.value) || (e->rank != 0 && (fsym == NULL || (fsym->as diff --git a/gcc/testsuite/gfortran.dg/optional_absent_9.f90 b/gcc/testsuite/gfortran.dg/optional_absent_9.f90 new file mode 100644 index 000..495a6c00d7f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/optional_absent_9.f90 @@ -0,0 +1,324 @@ +! { dg-do run } +! PR fortran/113377 +! +! Test passing of missing optional scalar dummies of intrinsic type + +module m_int + implicit none +contains + subroutine test_int () +integer :: k = 1 +call one (k) +call one_val (k) +call one_all (k) +call one_ptr (k) + end + + subroutine one (i, j) +integer, intent(in) :: i +integer ,optional :: j +integer, allocatable :: aa +integer, pointer :: pp => NULL() +if (present (j)) error stop "j is present" +call two (i, j) +call two_val (i, j) +call two (i, aa) +call two (i, pp) + end + + subroutine one_val (i, j) +integer, intent(in) :: i +integer, value, optional :: j +if (present (j)) error stop "j is present" +call two (i, j) +call two_val (i, j) + end + + subroutine one_all (i, j) +integer, intent(in) :: i +integer, allocatable,optional :: j +if (present (j)) error stop "j is present" +! call two (i, j) ! invalid per F2018:15.5.2.12, par. 3, clause 8 +! call two_val (i, j) ! invalid (*) F2018:15.5.2.12, par. 3, clause 8 +call two_all (i, j) + end +! (*) gfortran argument passing conventions ("scalar dummy arguments of type +! INTEGER, LOGICAL, REAL, COMPLEX, and CHARACTER(len=1) with VALUE attribute +! pass the presence status separately") may still allow this case pass + + subroutine one_ptr (i, j) +integer, intent(in) :: i +integer, pointer,optional :: j +if (present (j)) error stop "j is present" +! call two (i, j) ! invalid per F2018:15.5.2.12, par. 3, clause 7 +! call two_val (i, j) ! invalid (*) F2018:15.5.2.12, par. 3, clause 7 +call two_ptr (i, j) + end + + subroutine two (i, j) +integer, intent(in) :: i +integer, intent(in), optional :: j +if (present (j)) error stop 11 + end + + subroutine two_val (i, j) +integer, intent(in) :: i +integer, value, optional :: j +if (present (j)) error stop 12 + end + + subroutine two_all (i, j) +integer, intent(in)
Re: [PATCH] Fortran: passing of optional scalar arguments with VALUE attribute [PR113377]
Hi Mikael! Am 21.01.24 um 11:50 schrieb Mikael Morin: Hello, Le 20/01/2024 à 22:58, Harald Anlauf a écrit : Dear all, here's the first part of an attempt to fix issues with optional dummy arguments as actual arguments to optional dummies. This patch rectifies the case of scalar dummies with the VALUE attribute, which in gfortran's argument passing convention are passed on the stack when they are of intrinsic type, and have a hidden variable for the presence status. The testcase tries to cover valid combinations of actual and dummy argument. A few tests that are not standard-conforming but would still work with gfortran (due to the argument passing convention) are left there but commented out with a pointer to the standard (thanks, Mikael!). Regtested on x86_64-pc-linux-gnu. OK for mainline? Well, not yet. diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 9dd1f4086f4..2f47a75955c 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6526,6 +6526,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, gfc_init_se (&argse, NULL); argse.want_pointer = 1; gfc_conv_expr (&argse, e); + if (e->symtree->n.sym->attr.dummy + && POINTER_TYPE_P (TREE_TYPE (argse.expr))) + argse.expr = gfc_build_addr_expr (NULL_TREE, + argse.expr); The second part of the condition looks superfluous: if argse.want_pointer was set, we can expect to get a pointer result. But more important, I don't understand the need for this whole part, the new test seems to pass without it. And here is an example that regresses with it. program p type :: t integer, allocatable :: c end type call s2(t()) contains subroutine s1(a) integer, value, optional :: a if (present(a)) stop 1 end subroutine subroutine s2(a) type(t) :: a call s1(a%c) end subroutine end program Thanks for this example! I've taken the liberty to add a slightly extended version of it to the testcase. I was taken astray by the attempt to handle the (invalid by the standard) variant of passing an absent allocatable scalar to an optional scalar dummy with the value attribute under since we use a hidden variable for the present status. Without the code above there is an unprotected pointer dereference. I think that it still could be done, but it is probably not worth it. So I followed your suggestion and removed that part. cond = fold_convert (TREE_TYPE (argse.expr), null_pointer_node); cond = fold_build2_loc (input_location, NE_EXPR, @@ -7256,6 +7260,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && e->symtree->n.sym->attr.optional && (((e->rank != 0 && elemental_proc) || e->representation.length || e->ts.type == BT_CHARACTER + || (e->rank == 0 && e->symtree->n.sym->attr.value) This looks good. || (e->rank != 0 && (fsym == NULL || (fsym->as diff --git a/gcc/testsuite/gfortran.dg/optional_absent_9.f90 b/gcc/testsuite/gfortran.dg/optional_absent_9.f90 new file mode 100644 index 000..495a6c00d7f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/optional_absent_9.f90 @@ -0,0 +1,324 @@ +! { dg-do run } +! PR fortran/113377 +! +! Test passing of missing optional scalar dummies of intrinsic type + +module m_int + implicit none +contains + subroutine test_int () + integer :: k = 1 + call one (k) + call one_val (k) + call one_all (k) + call one_ptr (k) + end + + subroutine one (i, j) + integer, intent(in) :: i + integer ,optional :: j + integer, allocatable :: aa + integer, pointer :: pp => NULL() + if (present (j)) error stop "j is present" + call two (i, j) + call two_val (i, j) + call two (i, aa) + call two (i, pp) To be complete, you could check two_val(i, aa) and two_val(i, pp) as well. Both seem to pass already without the patch, so not absolutely needed. Your call. It is already contained in testcase gfortran.dg/value_optional_1.f90, (see call sub there), but then it may be helpful to have it here too. Thus added. + end + I think the patch is OK with the first trans-expr.cc hunk removed. Thanks. That's what I have done and pushed as r14-8317-g68862e5c75ef0e. Mikael Thanks for the review! Harald
Re: PR82943 - Suggested patch to fix
Am 20.01.24 um 23:42 schrieb Alexander Westbrooks: Based on what I am reading here, I can either do the DCO path or the copy right form path? Or is it both, where I send in the copy right forms and then on every commit I put the DCO? I thought the text is pretty clear. As already mentioned, https://gcc.gnu.org/contribute.html#legal gives you the options. One of these options is: "Alternatively, a contributor can certify the Developer Certificate of Origin for their contribution by adding the Signed-off-by: tag to their submission." If you opt for this variant, https://gcc.gnu.org/dco.html tells you everything. Basically (but please read yourself): "The sign-off is a simple line at the end of the explanation for the patch, which certifies that you wrote it or otherwise have the right to pass it on as a free software patch. ..." [...] "then you just add a line saying: Signed-off-by: Random J Developer using your real name (sorry, no pseudonyms or anonymous contributions.) ..." I think this would be sufficient. On Sat, Jan 20, 2024 at 3:40 PM Harald Anlauf wrote: Am 20.01.24 um 21:37 schrieb Jerry D: On 1/20/24 12:08 PM, Harald Anlauf wrote: Am 20.01.24 um 20:08 schrieb Jerry D: On 1/20/24 10:46 AM, Alexander Westbrooks wrote: Hello and Happy New Year! I wanted to follow up on this patch I made to address PR82943 for GFortran. Has anyone had a chance to review it? Thanks, Alexander Westbrooks Inserting myself in here just a little bit. I will apply and test today if I can. Paul is unavailable for a few weeks. Harald can chime in. Do you have commit rights for gcc? I am not aware of Alex having a copyright assignment on file, or a DCO certificate, and the patch is not signed off. But I might be wrong. --- snip --- I do not mind committing this but need clarifications regarding the copyright (copyleft?) rules in this case. In the past we have allowed small contributions like this. This may be a little more than minor. It is. This is why I pointed to: https://gcc.gnu.org/dco.html Regardless it appears to do the job! Jerry
[PATCH] Fortran: passing of optional dummies to elemental procedures [PR113377]
Dear all, here's the second part of a series for the treatment of missing optional arguments passed to optional dummies, now fixing the case that the latter procedures are elemental. Adjustments were necessary when the missing dummy has the VALUE attribute. I factored the code for the treatment of VALUE, hoping that the monster loop in gfc_conv_procedure_call will become slightly easier to overlook. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From bd97af4225bf596260610ea37241ee503842435e Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 23 Jan 2024 21:23:42 +0100 Subject: [PATCH] Fortran: passing of optional dummies to elemental procedures [PR113377] gcc/fortran/ChangeLog: PR fortran/113377 * trans-expr.cc (conv_dummy_value): New. (gfc_conv_procedure_call): Factor code for handling dummy arguments with the VALUE attribute in the scalar case into conv_dummy_value(). Reuse and adjust for calling elemental procedures. gcc/testsuite/ChangeLog: PR fortran/113377 * gfortran.dg/optional_absent_10.f90: New test. --- gcc/fortran/trans-expr.cc | 198 +--- .../gfortran.dg/optional_absent_10.f90| 219 ++ 2 files changed, 333 insertions(+), 84 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/optional_absent_10.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 128add47516..0fac0523670 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6075,6 +6075,105 @@ conv_cond_temp (gfc_se * parmse, gfc_expr * e, tree cond) } +/* Helper function for the handling of (currently) scalar dummy variables + with the VALUE attribute. Argument parmse should already be set up. */ +static void +conv_dummy_value (gfc_se * parmse, gfc_expr * e, gfc_symbol * fsym, + vec *& optionalargs) +{ + tree tmp; + + gcc_assert (fsym && fsym->attr.value && !fsym->attr.dimension); + + /* Absent actual argument for optional scalar dummy. */ + if (e == NULL && fsym->attr.optional && !fsym->attr.dimension) +{ + /* For scalar arguments with VALUE attribute which are passed by + value, pass "0" and a hidden argument for the optional status. */ + if (fsym->ts.type == BT_CHARACTER) + { + /* Pass a NULL pointer for an absent CHARACTER arg and a length of + zero. */ + parmse->expr = null_pointer_node; + parmse->string_length = build_int_cst (gfc_charlen_type_node, 0); + } + else + parmse->expr = fold_convert (gfc_sym_type (fsym), + integer_zero_node); + vec_safe_push (optionalargs, boolean_false_node); + + return; +} + + /* gfortran argument passing conventions: + actual arguments to CHARACTER(len=1),VALUE + dummy arguments are actually passed by value. + Strings are truncated to length 1. */ + if (gfc_length_one_character_type_p (&fsym->ts)) +{ + if (e->expr_type == EXPR_CONSTANT + && e->value.character.length > 1) + { + e->value.character.length = 1; + gfc_conv_expr (parmse, e); + } + + tree slen1 = build_int_cst (gfc_charlen_type_node, 1); + gfc_conv_string_parameter (parmse); + parmse->expr = gfc_string_to_single_character (slen1, parmse->expr, + e->ts.kind); + /* Truncate resulting string to length 1. */ + parmse->string_length = slen1; +} + + if (fsym->attr.optional + && fsym->ts.type != BT_CLASS + && fsym->ts.type != BT_DERIVED) +{ + /* F2018:15.5.2.12 Argument presence and + restrictions on arguments not present. */ + if (e->expr_type == EXPR_VARIABLE + && e->rank == 0 + && (gfc_expr_attr (e).allocatable + || gfc_expr_attr (e).pointer)) + { + gfc_se argse; + tree cond; + gfc_init_se (&argse, NULL); + argse.want_pointer = 1; + gfc_conv_expr (&argse, e); + cond = fold_convert (TREE_TYPE (argse.expr), null_pointer_node); + cond = fold_build2_loc (input_location, NE_EXPR, + logical_type_node, + argse.expr, cond); + vec_safe_push (optionalargs, + fold_convert (boolean_type_node, cond)); + /* Create "conditional temporary". */ + conv_cond_temp (parmse, e, cond); + } + else if (e->expr_type != EXPR_VARIABLE + || !e->symtree->n.sym->attr.optional + || (e->ref != NULL && e->ref->type != REF_ARRAY)) + vec_safe_push (optionalargs, boolean_true_node); + else + { + tmp = gfc_conv_expr_present (e->symtree->n.sym); + if (e->ts.type != BT_CHARACTER && !e->symtree->n.sym->attr.value) + parmse->expr + = fold_build3_loc (input_location, COND_EXPR, + TREE_TYPE (parmse->expr), + tmp, parmse->expr, + fold_convert (TREE_TYPE (parmse->expr), + integer_zero_node)); + + vec_safe_push (optionalargs, +
Re: [Fortran] half-cycle trig functions and atan[d] fixes
Am 24.01.24 um 10:13 schrieb Janne Blomqvist: On Wed, Jan 24, 2024 at 10:28 AM FX Coudert wrote: Now, if the OS adds cospi() to libm and it's in libm's symbol map, then the cospi() used by gfortran depends on the search order of the loaded libraries. We only include the fallback math functions in libgfortran when they are not available on the system. configure detects what is present in the libc being targeted, and conditionally compiles the necessary fallback functions (and only them). Exactly. However, there is the (corner?) case when libgfortran has been compiled, and cospi() not found and thus the fallback implementation is included, and then later libc is updated to a version that does provide cospi(). I believe in that case which version gets used is down to the library search order (i.e. the order that "ldd /path/to/binary" prints the libs), it will use the first symbol it finds. Also, it's not necessary to do some ifdef tricks with gfortran.map, if a symbol listed there isn't found in the library it's just ignored. So the *pi() trig functions can be unconditionally added there, and then depending on whether the target libm includes those or not they are then included in the exported symbol list. It's possible to override this to look for specific symbol versions etc., but that probably goes deep into the weeds of target-specific stuff (e.g. are we looking for cospi@FBSD_1.7, cospi@GLIBC_X.Y.Z, or something else?). I'm sure you don't wanna go there. Isn't this something that could be addressed by __attribute__(__weakref__)?
Re: [PATCH] Fortran: passing of optional dummies to elemental procedures [PR113377]
Hi Mikael, Am 24.01.24 um 19:46 schrieb Mikael Morin: Le 23/01/2024 à 21:36, Harald Anlauf a écrit : Dear all, here's the second part of a series for the treatment of missing optional arguments passed to optional dummies, now fixing the case that the latter procedures are elemental. Adjustments were necessary when the missing dummy has the VALUE attribute. I factored the code for the treatment of VALUE, hoping that the monster loop in gfc_conv_procedure_call will become slightly easier to overlook. Regtested on x86_64-pc-linux-gnu. OK for mainline? Looks good, but... Thanks, Harald diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 128add47516..0fac0523670 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6392,12 +6479,23 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, } } + /* Scalar dummy arguments of intrinsic type with VALUE attribute. */ + if (fsym + && fsym->attr.value + && !fsym->attr.dimension + // && (fsym->ts.type != BT_CHARACTER + // || gfc_length_one_character_type_p (&fsym->ts)) ... please remove the commented code here. OK with that change. Duh! I completely missed that during cleanup. The !fsym->attr.dimension condition could be removed as well as we are in the case of an elemental procedure at this point, but it doesn't harm if you prefer keeping it. You're absolutely right. I've removed it. Thanks for the patch. Thanks for the review! Harald Mikael + && fsym->ts.type != BT_DERIVED + && fsym->ts.type != BT_CLASS) + conv_dummy_value (&parmse, e, fsym, optionalargs); + /* If we are passing an absent array as optional dummy to an elemental procedure, make sure that we pass NULL when the data pointer is NULL. We need this extra conditional because of scalarization which passes arrays elements to the procedure, ignoring the fact that the array can be absent/unallocated/... */ - if (ss->info->can_be_null_ref && ss->info->type != GFC_SS_REFERENCE) + else if (ss->info->can_be_null_ref + && ss->info->type != GFC_SS_REFERENCE) { tree descriptor_data;
[PATCH] Fortran: use name of array component in runtime error message [PR30802]
Dear all, this patch is actually only a followup fix to generate the proper name of an array reference in derived-type components for the runtime error message generated for the bounds-checking code. Without the proper part ref, not only a user may get confused: I was, too... The testcase is compile-only, as it is only important to check the strings used in the error messages. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 43c0185764ec878576ef2255d9df24fbb1961af4 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 24 Jan 2024 22:28:31 +0100 Subject: [PATCH] Fortran: use name of array component in runtime error message [PR30802] gcc/fortran/ChangeLog: PR fortran/30802 * trans-array.cc (trans_array_bound_check): Derive name of component for use in runtime error message. gcc/testsuite/ChangeLog: PR fortran/30802 * gfortran.dg/bounds_check_fail_8.f90: New test. --- gcc/fortran/trans-array.cc| 34 ++ .../gfortran.dg/bounds_check_fail_8.f90 | 35 +++ 2 files changed, 69 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 878a92aff18..f6ddce2d023 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3497,6 +3497,10 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, tree descriptor; char *msg; const char * name = NULL; + char *var_name = NULL; + gfc_expr *expr; + gfc_ref *ref; + size_t len; if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)) return index; @@ -3509,6 +3513,36 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, name = ss->info->expr->symtree->n.sym->name; gcc_assert (name != NULL); + /* When we have a component ref, compile name for the array section. + Note that there can only be one part ref. */ + expr = ss->info->expr; + if (expr->ref && !compname) +{ + len = strlen (name) + 1; + + /* Find a safe length. */ + for (ref = expr->ref; ref; ref = ref->next) + if (ref->type == REF_COMPONENT) + len += 2 + strlen (ref->u.c.component->name); + + var_name = XALLOCAVEC (char, len); + strcpy (var_name, name); + + for (ref = expr->ref; ref; ref = ref->next) + { + /* Append component name. */ + if (ref->type == REF_COMPONENT) + { + strcat (var_name, "%%"); + strcat (var_name, ref->u.c.component->name); + continue; + } + if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION) + break; + } + name = var_name; +} + if (VAR_P (descriptor)) name = IDENTIFIER_POINTER (DECL_NAME (descriptor)); diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 new file mode 100644 index 000..3397e953ba6 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 @@ -0,0 +1,35 @@ +! { dg-do compile } +! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" } +! { dg-output "At line 22 .*" } +! { dg-shouldfail "dimension 3 of array 'uu%z' outside of expected range" } +! +! PR fortran/30802 - improve bounds-checking for array references +! +! Checking the proper component references is the most important part here. + +program test + implicit none + integer :: k = 0 + type t + real, dimension(10,20,30) :: z = 23 + end type t + type u + type(t) :: vv(4,5) + end type u + type(t) :: uu, ww(1) + type(u) :: x1, x2, y1(1), y2(1) + + print *, uu % z(1,k,:) ! runtime check only for dimension 2 of z + print *, ww(1)% z(1,:,k) ! runtime check only for dimension 3 of z + print *, x1 % vv(2,3)% z(1,:,k) ! runtime check only for dimension 3 of z + print *, x2 % vv(k,:)% z(1,2,3) ! runtime check only for dimension 1 of vv + print *, y1(1)% vv(2,3)% z(1,:,k) ! runtime check only for dimension 3 of z + print *, y2(1)% vv(:,k)% z(1,2,3) ! runtime check only for dimension 2 of vv +end program test + +! { dg-final { scan-tree-dump-times "'uu%%z.' outside of expected range" 2 "original" } } +! { dg-final { scan-tree-dump-times "'ww%%z.' outside of expected range" 2 "original" } } +! { dg-final { scan-tree-dump-times "'x1%%vv%%z.' outside of expected range" 2 "original" } } +! { dg-final { scan-tree-dump-times "'x2%%vv.' outside of expected range" 2 "original" } } +! { dg-final { scan-tree-dump-times "'y1%%vv%%z.' outside of expected range" 2 "original" } } +! { dg-final { scan-tree-dump-times "'y2%%vv.' outside of expected range" 2 "original" } } -- 2.35.3
[PATCH] Fortran: NULL actual to optional dummy with VALUE attribute [PR113377]
Dear all, this is the third patch in a series that addresses dummy arguments with the VALUE attribute, now handling the passing of NULL actual arguments. It is based on the refactoring in the previous patch and reuses the handling of absent arguments. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From a0509b34d52b32a2e3511daefcb7dc308c755931 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 25 Jan 2024 22:19:10 +0100 Subject: [PATCH] Fortran: NULL actual to optional dummy with VALUE attribute [PR113377] gcc/fortran/ChangeLog: PR fortran/113377 * trans-expr.cc (conv_dummy_value): Treat NULL actual argument to optional dummy with the VALUE attribute as not present. (gfc_conv_procedure_call): Likewise. gcc/testsuite/ChangeLog: PR fortran/113377 * gfortran.dg/optional_absent_11.f90: New test. --- gcc/fortran/trans-expr.cc | 11 ++- .../gfortran.dg/optional_absent_11.f90| 99 +++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/optional_absent_11.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 3dc521fab9a..67abca9f6ba 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6086,7 +6086,7 @@ conv_dummy_value (gfc_se * parmse, gfc_expr * e, gfc_symbol * fsym, gcc_assert (fsym && fsym->attr.value && !fsym->attr.dimension); /* Absent actual argument for optional scalar dummy. */ - if (e == NULL && fsym->attr.optional && !fsym->attr.dimension) + if ((e == NULL || e->expr_type == EXPR_NULL) && fsym->attr.optional) { /* For scalar arguments with VALUE attribute which are passed by value, pass "0" and a hidden argument for the optional status. */ @@ -6354,7 +6354,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, e->ts = temp_ts; } - if (e == NULL) + if (e == NULL + || (e->expr_type == EXPR_NULL + && fsym + && fsym->attr.value + && fsym->attr.optional + && !fsym->attr.dimension + && fsym->ts.type != BT_DERIVED + && fsym->ts.type != BT_CLASS)) { if (se->ignore_optional) { diff --git a/gcc/testsuite/gfortran.dg/optional_absent_11.f90 b/gcc/testsuite/gfortran.dg/optional_absent_11.f90 new file mode 100644 index 000..1f63def46fa --- /dev/null +++ b/gcc/testsuite/gfortran.dg/optional_absent_11.f90 @@ -0,0 +1,99 @@ +! { dg-do run } +! PR fortran/113377 +! +! Test that a NULL actual argument to an optional dummy is not present +! (see also F2018:15.5.2.12 on argument presence) + +program test_null_actual_is_absent + implicit none + integer :: k(4) = 1 + character :: c(4) = "#" + call one (k) + call three (c) +contains + subroutine one (i) +integer, intent(in) :: i(4) +integer :: kk = 2 +integer, allocatable :: aa +integer, pointer :: pp => NULL() +print *, "Scalar integer" +call two (kk, aa) +call two (kk, pp) +call two (kk, NULL()) +call two (kk, NULL(aa)) +call two (kk, NULL(pp)) +print *, "Elemental integer" +call two (i, aa) +call two (i, pp) +call two (i, NULL()) +call two (i, NULL(aa)) +call two (i, NULL(pp)) +print *, "Scalar integer; value" +call two_val (kk, aa) +call two_val (kk, pp) +call two_val (kk, NULL()) +call two_val (kk, NULL(aa)) +call two_val (kk, NULL(pp)) +print *, "Elemental integer; value" +call two_val (i, aa) +call two_val (i, pp) +call two_val (i, NULL()) +call two_val (i, NULL(aa)) +call two_val (i, NULL(pp)) + end + + elemental subroutine two (i, j) +integer, intent(in) :: i +integer, intent(in), optional :: j +if (present (j)) error stop 11 + end + + elemental subroutine two_val (i, j) +integer, intent(in) :: i +integer, value, optional :: j +if (present (j)) error stop 12 + end + + subroutine three (y) +character, intent(in) :: y(4) +character :: zz = "*" +character, allocatable :: aa +character, pointer :: pp => NULL() +print *, "Scalar character" +call four (zz, aa) +call four (zz, pp) +call four (zz, NULL()) +call four (zz, NULL(aa)) +call four (zz, NULL(pp)) +print *, "Elemental character" +call four (y, aa) +call four (y, pp) +call four (y, NULL()) +call four (y, NULL(aa)) +call four (y, NULL(pp)) +print *, "Scalar character; value" +call four_val (zz, aa) +call four_val (zz, pp) +call four_val (zz, NULL()) +call four_val (zz, NULL(aa)) +call four_val (zz, NULL(pp)) +p
[PATCH, committed] Fortran: fix bounds-checking errors for CLASS array dummies [PR104908]
Dear all, commit r11-1235 for pr95331 addressed array bounds issues with unlimited polymorphic array dummies, but caused regressions for CLASS array dummies that lead to either wrong code with bounds-checking, or an ICE. The solution is simple: add a check whether the dummy is unlimited polymorphic and otherwise restore the previous behavior. The attached patch regtested fine on x86_64-pc-linux-gnu and was OK'ed in the PR by Jerry. Pushed as: r14-8471-gce61de1b8a1bb3 Since this is a 11/12/13/14 regression and appears safe otherwise, I intend to backport as suitable, unless there are comments. Thanks, Harald From ce61de1b8a1bb3a22118e900376f380768f2ba59 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 27 Jan 2024 17:41:43 +0100 Subject: [PATCH] Fortran: fix bounds-checking errors for CLASS array dummies [PR104908] Commit r11-1235 addressed issues with bounds of unlimited polymorphic array dummies. However, using the descriptor from sym->backend_decl does break the case of CLASS array dummies. The obvious solution is to restrict the fix to the unlimited polymorphic case, thus keeping the original descriptor in the ordinary case. gcc/fortran/ChangeLog: PR fortran/104908 * trans-array.cc (gfc_conv_array_ref): Restrict use of transformed descriptor (sym->backend_decl) to the unlimited polymorphic case. gcc/testsuite/ChangeLog: PR fortran/104908 * gfortran.dg/pr104908.f90: New test. --- gcc/fortran/trans-array.cc | 5 +++- gcc/testsuite/gfortran.dg/pr104908.f90 | 32 ++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/pr104908.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 878a92aff18..1e0d698a949 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -4063,7 +4063,10 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr, } decl = se->expr; - if (IS_CLASS_ARRAY (sym) && sym->attr.dummy && ar->as->type != AS_DEFERRED) + if (UNLIMITED_POLY(sym) + && IS_CLASS_ARRAY (sym) + && sym->attr.dummy + && ar->as->type != AS_DEFERRED) decl = sym->backend_decl; cst_offset = offset = gfc_index_zero_node; diff --git a/gcc/testsuite/gfortran.dg/pr104908.f90 b/gcc/testsuite/gfortran.dg/pr104908.f90 new file mode 100644 index 000..c3a30b0003c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr104908.f90 @@ -0,0 +1,32 @@ +! { dg-do compile } +! { dg-additional-options "-fcheck=bounds -fdump-tree-original" } +! +! PR fortran/104908 - incorrect out-of-bounds runtime error + +program test + implicit none + type vec + integer :: x(3) = [2,4,6] + end type vec + type(vec) :: w(2) + call sub(w) +contains + subroutine sub (v) +class(vec), intent(in) :: v(:) +integer :: k, q(3) +q = [ (v(1)%x(k), k = 1, 3) ] ! <-- was failing here after r11-1235 +print *, q + end +end + +subroutine sub2 (zz) + implicit none + type vec + integer :: x(2,1) + end type vec + class(vec), intent(in) :: zz(:) ! used to ICE after r11-1235 + integer :: k + k = zz(1)%x(2,1) +end + +! { dg-final { scan-tree-dump-times " above upper bound " 4 "original" } } -- 2.35.3
Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
Hi Mikael, Am 28.01.24 um 12:39 schrieb Mikael Morin: Le 24/01/2024 à 22:39, Harald Anlauf a écrit : Dear all, this patch is actually only a followup fix to generate the proper name of an array reference in derived-type components for the runtime error message generated for the bounds-checking code. Without the proper part ref, not only a user may get confused: I was, too... The testcase is compile-only, as it is only important to check the strings used in the error messages. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Hello, the change proper looks good, and is an improvement. But I'm a little concerned by the production of references like in the test x1%vv%z which could be confusing and is strictly speaking invalid fortran (multiple non-scalar components). Did you consider generating x1%vv(?,?)%zz or x1%vv(...)%z or similar? yes, that seems very reasonable, given that this is what NAG does. We also have spurious %_data in some error messages that I'll try to get rid off. There is another nit about the test, which has dg-output and dg-shouldfail despite being only a compile-time test. I'll remove that. With gcc-13 the runtime check would trigger in the wrong line but the failure of the check is dealt with by another testcase in gcc-14. Otherwise looks good. Mikael I'll come up with a revised patch. Thanks for the feedback so far! Harald
Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
Am 28.01.24 um 22:43 schrieb Steve Kargl: On Sun, Jan 28, 2024 at 08:56:24PM +0100, Harald Anlauf wrote: Am 28.01.24 um 12:39 schrieb Mikael Morin: Le 24/01/2024 à 22:39, Harald Anlauf a écrit : Dear all, this patch is actually only a followup fix to generate the proper name of an array reference in derived-type components for the runtime error message generated for the bounds-checking code. Without the proper part ref, not only a user may get confused: I was, too... The testcase is compile-only, as it is only important to check the strings used in the error messages. Regtested on x86_64-pc-linux-gnu. OK for mainline? the change proper looks good, and is an improvement. But I'm a little concerned by the production of references like in the test x1%vv%z which could be confusing and is strictly speaking invalid fortran (multiple non-scalar components). Did you consider generating x1%vv(?,?)%zz or x1%vv(...)%z or similar? yes, that seems very reasonable, given that this is what NAG does. We also have spurious %_data in some error messages that I'll try to get rid off. I haven't looked at the patch, but sometimes (if not always) things like _data are marked with attr.artificial. You might see if this will help with suppressing spurious messages. I was talking about the generated format strings of runtime error messages. program p implicit none type t real :: zzz(10) = 42 end type t class(t), allocatable :: xx(:) integer :: j j = 0 allocate (t :: xx(1)) print *, xx(1)% zzz(j) end This is generating the following error at runtime since at least gcc-7: Fortran runtime error: Index '0' of dimension 1 of array 'xx%_data%zzz' below lower bound of 1 I believe you were recalling bogus warnings at compile time. There are no warnings here, and there shouldn't.
Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
Am 29.01.24 um 18:25 schrieb Harald Anlauf: I was talking about the generated format strings of runtime error messages. program p implicit none type t real :: zzz(10) = 42 end type t class(t), allocatable :: xx(:) integer :: j j = 0 allocate (t :: xx(1)) print *, xx(1)% zzz(j) end This is generating the following error at runtime since at least gcc-7: Fortran runtime error: Index '0' of dimension 1 of array 'xx%_data%zzz' below lower bound of 1 Of course this is easily suppressed by: diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 1e0d698a949..fa0e00a28a6 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -4054,7 +4054,8 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr, { if (ref->type == REF_ARRAY && &ref->u.ar == ar) break; - if (ref->type == REF_COMPONENT) + if (ref->type == REF_COMPONENT + && strcmp (ref->u.c.component->name, "_data") != 0) { strcat (var_name, "%%"); strcat (var_name, ref->u.c.component->name); I have been contemplating the generation the full chain of references as suggested by Mikael and supported by NAG. The main issue is: how do I easily generate that call? gfc_trans_runtime_check is a vararg function, but what I would rather have is a function that takes either a (chained?) list of trees or an array of trees holding the (co-)indices of the reference. Is there an example, or a recommendation which variant to prefer? Thanks, Harald
Re: [PATCH] Fortran: Mark internal symbols as artificial [PR88009,PR68800]
Am 29.01.24 um 21:45 schrieb Bernhard Reutner-Fischer: On Wed, 17 Nov 2021 21:32:14 +0100 Harald Anlauf wrote: Do you have testcases/reproducers demonstrating that the patch actually fixes the issues you're describing? I believe that marking artificial symbols as such is obvious and i did use the existing tests to verify that the changes do not regress but behave as intended. I did check that the memory leak in gfc_find_derived_vtab is fixed with the patch. Ok for stage 1 if the rebased regression test passes? thanks Am 17.11.21 um 09:12 schrieb Bernhard Reutner-Fischer via Gcc-patches: On Tue, 16 Nov 2021 21:46:32 +0100 Harald Anlauf via Fortran wrote: Hi Bernhard, I'm trying to understand your patch. What does it really try to solve? Compiler generated symbols should be marked artificial. The fix for PR88009 ( f8add009ce300f24b75e9c2e2cc5dd944a020c28 , r9-5194 ) added artificial just to the _final component and left out all the rest. Note that the majority of compiler generated symbols in class.c already had artificial set properly. The proposed patch amends the other generated symbols to be marked artificial, too. The other parts fix memory leaks. PR88009 is closed and seems to have nothing to do with this. Well it marked only _final as artificial and forgot to adjust the others as well. We can remove the reference to PR88009 if you prefer? thanks! Harald Am 14.11.21 um 23:17 schrieb Bernhard Reutner-Fischer via Fortran: Hi! Amend fix for PR88009 to mark all these class components as artificial. gcc/fortran/ChangeLog: * class.c (gfc_build_class_symbol, generate_finalization_wrapper, (gfc_find_derived_vtab, find_intrinsic_vtab): Use stringpool for names. Mark internal symbols as artificial. * decl.c (gfc_match_decl_type_spec, gfc_match_end): Fix indentation. (gfc_match_derived_decl): Fix indentation. Check extension level before incrementing refs counter. * parse.c (parse_derived): Fix style. * resolve.c (resolve_global_procedure): Likewise. * symbol.c (gfc_check_conflict): Do not ignore artificial symbols. (gfc_add_flavor): Reorder condition, cheapest first. (gfc_new_symbol, gfc_get_sym_tree, generate_isocbinding_symbol): Fix style. * trans-expr.c (gfc_trans_subcomponent_assign): Remove restriction on !artificial. * match.c (gfc_match_equivalence): Special-case CLASS_DATA for warnings. --- gfc_match_equivalence(), too, should not bail-out early on the first error but should diagnose all errors. I.e. not goto cleanup but set err=true and continue in order to diagnose all constraints of a statement. Maybe Sandra or somebody else will eventually find time to tweak that. I think it also plugs a very minor leak of name in gfc_find_derived_vtab so i also tagged it [PR68800]. At least that was the initial motiviation to look at that spot. We were doing - name = xasprintf ("__vtab_%s", tname); ... gfc_set_sym_referenced (vtab); - name = xasprintf ("__vtype_%s", tname); Bootstrapped and regtested without regressions on x86_64-unknown-linux. Ok for trunk? Can you please post the patch here so that we can review it?
Re: [patch, libgfortran] PR111022 ES0.0E0 format gave ES0.dE0 output with d too high.
Hi Jerry, Am 30.01.24 um 19:15 schrieb Jerry D: The attached patch attempts to fix the handling of the EN0.0E0 and ES0.0E0 formatting by correctly calculating the number of digits needed for the exponents and building those exponents into the float string. while your patch addresses ENw.dE0 and ESw.dE0 formatting, it does not fix Ew.dE0, which can be seen with the following test: write(buffer,"(E0.3E0)") .6660_4 print *, buffer write(buffer,"(E0.3)") .6660_4 print *, buffer I get even with your patch: 0.666 0.666 but would have expected: 0.666E+0 ! F2018 & F2023, table 13.1 0.666E+0 ! F2023, table 13.1 The latter one is a bit ambiguous in F2018, but certainly gfortran's current output in wrong. Can you please check, and if you can fix that too, it would be great. And if we do not want to be dependent on the standard version at runtime, I'd rather go for F2023. My editor judiciously deleted trailing blank spaces in a number of places. I apologize for the clutter, but we might as well get rid of it now. Two existing test cases needed to be adjusted and I am adding one new test case to capture the changes in our testsuite. Regression tested on X86_64. OK for trunk? Do we need to backport this? If the above is fixed, I would not object a backport to the 13-branch, but only if that change has sinked in for a while, and someone else agrees on it. Thanks so far! Harald Regards, Jerry Author: Jerry DeLisle Date: Tue Jan 30 09:45:49 2024 -0800 libgfortran: EN0.0E0 and ES0.0E0 format editing. PR libgfortran/111022 F2018 and F2023 standards added zero width exponents. This required additional special handing in the process of building formatted floating point strings. libgfortran/ChangeLog: * io/write.c (select_buffer): Whitespace. (write_real): Whitespace. (write_real_w0): Adjust logic for d==0. * io/write_float.def (determine_precision): Whitespace. (build_float_string): Calculate the width of the E0 exponents. (build_infnan_string): Whitespace. (CALCULATE_EXP): Whitespace (quadmath_snprintf): Whitespace. (determine_en_precision): Whitespace. gcc/testsuite/ChangeLog: * gfortran.dg/pr96436_4.f90: Changed for ES0 and EN0. * gfortran.dg/pr96436_5.f90: Changed for ES0 and EN0. * gfortran.dg/pr111022.f90: New test.
Re: [patch, libgfortran] PR111022 ES0.0E0 format gave ES0.dE0 output with d too high.
Jerry, Steve, Am 03.02.24 um 04:24 schrieb Steve Kargl: Jerry, The patch looks good to me, but please give Harald a chance to comment. I just tested it a little, and it looked good. We even get a runtime error on E0.0 now as required. :-) Thanks for the patch! Harald
[PATCH] Fortran: copy-out for possibly missing OPTIONAL CLASS arguments [PR112772]
Dear all, the attached rather obvious patch fixes the first testcase of pr112772: we unconditionally generated copy-out code for optional class arguments, while the copy-in properly checked the presence of arguments. Regtested on x86_64-pc-linux-gnu. OK for mainline? (The second testcase is a different issue.) Thanks, Harald From 38433016def0337a72cb0ef0029cd2c05d702282 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 30 Nov 2023 21:53:21 +0100 Subject: [PATCH] Fortran: copy-out for possibly missing OPTIONAL CLASS arguments [PR112772] gcc/fortran/ChangeLog: PR fortran/112772 * trans-expr.cc (gfc_conv_class_to_class): Make copy-out conditional on the presence of an OPTIONAL CLASS argument passed to an OPTIONAL CLASS dummy. gcc/testsuite/ChangeLog: PR fortran/112772 * gfortran.dg/missing_optional_dummy_7.f90: New test. --- gcc/fortran/trans-expr.cc | 9 +++ .../gfortran.dg/missing_optional_dummy_7.f90 | 64 +++ 2 files changed, 73 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/missing_optional_dummy_7.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index bfe9996ced6..6a47af39c57 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1365,6 +1365,15 @@ gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, gfc_typespec class_ts, tmp = build3_loc (input_location, COND_EXPR, void_type_node, cond, tmp, tmp2); gfc_add_expr_to_block (&parmse->pre, tmp); + + if (!elemental && full_array && copyback) + { + tmp2 = build_empty_stmt (input_location); + tmp = gfc_finish_block (&parmse->post); + tmp = build3_loc (input_location, COND_EXPR, void_type_node, + cond, tmp, tmp2); + gfc_add_expr_to_block (&parmse->post, tmp); + } } else gfc_add_block_to_block (&parmse->pre, &block); diff --git a/gcc/testsuite/gfortran.dg/missing_optional_dummy_7.f90 b/gcc/testsuite/gfortran.dg/missing_optional_dummy_7.f90 new file mode 100644 index 000..ad9ecd8f2b6 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/missing_optional_dummy_7.f90 @@ -0,0 +1,64 @@ +! { dg-do run } +! PR fortran/112772 - test absent OPTIONAL, ALLOCATABLE/POINTER class dummies + +program main + implicit none + type t + end type t + call test_c_a () + call test_u_a () + call test_c_p () + call test_u_p () +contains + ! class, allocatable + subroutine test_c_a (msg1) +class(t), optional, allocatable :: msg1(:) +if (present (msg1)) stop 1 +call assert_c_a () +call assert_c_a (msg1) + end + + subroutine assert_c_a (msg2) +class(t), optional, allocatable :: msg2(:) +if (present (msg2)) stop 2 + end + + ! unlimited polymorphic, allocatable + subroutine test_u_a (msg1) +class(*), optional, allocatable :: msg1(:) +if (present (msg1)) stop 3 +call assert_u_a () +call assert_u_a (msg1) + end + + subroutine assert_u_a (msg2) +class(*), optional, allocatable :: msg2(:) +if (present (msg2)) stop 4 + end + + ! class, pointer + subroutine test_c_p (msg1) +class(t), optional, pointer :: msg1(:) +if (present (msg1)) stop 5 +call assert_c_p () +call assert_c_p (msg1) + end + + subroutine assert_c_p (msg2) +class(t), optional, pointer :: msg2(:) +if (present (msg2)) stop 6 + end + + ! unlimited polymorphic, pointer + subroutine test_u_p (msg1) +class(*), optional, pointer :: msg1(:) +if (present (msg1)) stop 7 +call assert_u_p () +call assert_u_p (msg1) + end + + subroutine assert_u_p (msg2) +class(*), optional, pointer :: msg2(:) +if (present (msg2)) stop 8 + end +end -- 2.35.3
Re: [PATCH] Fortran: copy-out for possibly missing OPTIONAL CLASS arguments [PR112772]
Hi Mikael, On 12/1/23 21:24, Mikael Morin wrote: Hello, Le 30/11/2023 à 22:06, Harald Anlauf a écrit : the attached rather obvious patch fixes the first testcase of pr112772: we unconditionally generated copy-out code for optional class arguments, while the copy-in properly checked the presence of arguments. Regtested on x86_64-pc-linux-gnu. OK for mainline? Looks good. Thanks. ok, will commit. (The second testcase is a different issue.) Maybe use a separate PR? Mikael I just found a fix that is regtesting, and which will allow to re-enable the test failing with ASAN in the patch for PR100651. Will merge that fix into the previous patch and submit a v3 later. Thanks, Harald
[PATCH, v3] Fortran: deferred-length character optional dummy arguments [PR93762,PR100651]
Dear all, this patch extends the previous version by adding further code testing the presence of an optional deferred-length character argument also in the function initialization code. This allows to re-enable a commented-out test in v2. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 11/28/23 20:56, Harald Anlauf wrote: Hi FX, On 11/28/23 18:07, FX Coudert wrote: Hi Harald, The patch looks OK to me. Probably wait a bit for another opinion, since I’m not that active and I may have missed something. Thanks, FX thanks for having a look. In the meantime I got an automated mail from the Linaro testers. According to it there is a runtime failure of the testcase on aarch64. I couldn't see any useful traceback or else. I tried the testcase on x86 with different options and found an unexpected result only with -fsanitize=undefined and only for the case of a rank-1 dummy when there is no actual argument and the passed to another subroutine. (valgrind is happy.) Reduced reproducer: ! this fails with -fsanitize=undefined program main call test_rank1 () contains subroutine test_rank1 (msg1) character(:), optional, allocatable :: msg1(:) if (present (msg1)) stop 77 call assert_rank1 () ! <- no problem here call assert_rank1 (msg1) ! <- problematic code path end subroutine assert_rank1 (msg2) character(:), optional, allocatable :: msg2(:) if (present (msg2)) stop 99 ! <- no problem if commented end end As far as I can tell, this could be a pre-existing (latent) issue. By looking at the tree-dump, the only thing that appears fishy has been there before. But then I am only guessing that this is the problem observed on aarch64. I have disabled the related call in the testcase of the attached revised version. As I do not see anything else, I wonder if one could proceed with the current version but open a PR for the reduced case above, unless someone can pinpoint the place that is responsible for the above failure. (Is it the caller, or rather the function entry code in the callee?) Cheers, Harald From b0a169bd70c9cd175c25b4a9543b24058596bf5e Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 1 Dec 2023 22:44:30 +0100 Subject: [PATCH] Fortran: deferred-length character optional dummy arguments [PR93762,PR100651] gcc/fortran/ChangeLog: PR fortran/93762 PR fortran/100651 * trans-array.cc (gfc_trans_deferred_array): Add presence check for optional deferred-length character dummy arguments. * trans-expr.cc (gfc_conv_missing_dummy): The character length for deferred-length dummy arguments is passed by reference, so that its value can be returned. Adjust handling for optional dummies. gcc/testsuite/ChangeLog: PR fortran/93762 PR fortran/100651 * gfortran.dg/optional_deferred_char_1.f90: New test. --- gcc/fortran/trans-array.cc| 9 ++ gcc/fortran/trans-expr.cc | 22 +++- .../gfortran.dg/optional_deferred_char_1.f90 | 100 ++ 3 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/optional_deferred_char_1.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index bbb81f40aa9..82f60a656f3 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -11430,6 +11430,15 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block) { gfc_conv_string_length (sym->ts.u.cl, NULL, &init); gfc_trans_vla_type_sizes (sym, &init); + + /* Presence check of optional deferred-length character dummy. */ + if (sym->ts.deferred && sym->attr.dummy && sym->attr.optional) + { + tmp = gfc_finish_block (&init); + tmp = build3_v (COND_EXPR, gfc_conv_expr_present (sym), + tmp, build_empty_stmt (input_location)); + gfc_add_expr_to_block (&init, tmp); + } } /* Dummy, use associated and result variables don't need anything special. */ diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 6a47af39c57..ea087294249 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -2125,10 +2125,24 @@ gfc_conv_missing_dummy (gfc_se * se, gfc_expr * arg, gfc_typespec ts, int kind) if (ts.type == BT_CHARACTER) { - tmp = build_int_cst (gfc_charlen_type_node, 0); - tmp = fold_build3_loc (input_location, COND_EXPR, gfc_charlen_type_node, - present, se->string_length, tmp); - tmp = gfc_evaluate_now (tmp, &se->pre); + /* Handle deferred-length dummies that pass the character length by + reference so that the value can be returned. */ + if (ts.deferred && INDIRECT_REF_P (se->string_length)) + { + tmp = gfc_build_addr_expr (NULL_TREE, se->string_length); + tmp = fold_build3_loc (input_location, COND_EXPR, TREE_TYPE (tmp), + present, tmp, null_pointer_node); + tmp = gfc_e
[PATCH] Fortran: allow RESTRICT qualifier also for optional arguments [PR100988]
Dear all, the attached patch picks up an observation by Tobias that we did not specify the RESTRICT qualifier for optional arguments even if that was allowed. In principle this might have prevented better optimization. While looking more closely, I found and fixed an issue with CLASS dummy arguments that mishandled this. This revealed a few cases in the testsuite that were matching the wrong patterns... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From aa25d35cb866f7f333b656938224866a70b93a69 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 4 Dec 2023 22:44:53 +0100 Subject: [PATCH] Fortran: allow RESTRICT qualifier also for optional arguments [PR100988] gcc/fortran/ChangeLog: PR fortran/100988 * gfortran.h (IS_PROC_POINTER): New macro. * trans-types.cc (gfc_sym_type): Use macro in determination if the restrict qualifier can be used for a dummy variable. Fix logic to allow the restrict qualifier also for optional arguments, and to not apply it to pointer or proc_pointer arguments. gcc/testsuite/ChangeLog: PR fortran/100988 * gfortran.dg/coarray_poly_6.f90: Adjust pattern. * gfortran.dg/coarray_poly_7.f90: Likewise. * gfortran.dg/coarray_poly_8.f90: Likewise. * gfortran.dg/missing_optional_dummy_6a.f90: Likewise. * gfortran.dg/pr100988.f90: New test. Co-authored-by: Tobias Burnus --- gcc/fortran/gfortran.h| 3 + gcc/fortran/trans-types.cc| 13 ++-- gcc/testsuite/gfortran.dg/coarray_poly_6.f90 | 2 +- gcc/testsuite/gfortran.dg/coarray_poly_7.f90 | 2 +- gcc/testsuite/gfortran.dg/coarray_poly_8.f90 | 2 +- .../gfortran.dg/missing_optional_dummy_6a.f90 | 2 +- gcc/testsuite/gfortran.dg/pr100988.f90| 61 +++ 7 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr100988.f90 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index aa3f6cb70b4..a77441f38e7 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -4008,6 +4008,9 @@ bool gfc_may_be_finalized (gfc_typespec); #define IS_POINTER(sym) \ (sym->ts.type == BT_CLASS && sym->attr.class_ok && CLASS_DATA (sym) \ ? CLASS_DATA (sym)->attr.class_pointer : sym->attr.pointer) +#define IS_PROC_POINTER(sym) \ + (sym->ts.type == BT_CLASS && sym->attr.class_ok && CLASS_DATA (sym) \ + ? CLASS_DATA (sym)->attr.proc_pointer : sym->attr.proc_pointer) /* frontend-passes.cc */ diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc index 084b8c3ae2c..5b11ffc3cc9 100644 --- a/gcc/fortran/trans-types.cc +++ b/gcc/fortran/trans-types.cc @@ -2327,8 +2327,8 @@ gfc_sym_type (gfc_symbol * sym, bool is_bind_c) else byref = 0; - restricted = !sym->attr.target && !sym->attr.pointer - && !sym->attr.proc_pointer && !sym->attr.cray_pointee; + restricted = (!sym->attr.target && !IS_POINTER (sym) + && !IS_PROC_POINTER (sym) && !sym->attr.cray_pointee); if (!restricted) type = gfc_nonrestricted_type (type); @@ -2384,11 +2384,10 @@ gfc_sym_type (gfc_symbol * sym, bool is_bind_c) || (sym->ns->proc_name && sym->ns->proc_name->attr.entry_master)) type = build_pointer_type (type); else - { - type = build_reference_type (type); - if (restricted) - type = build_qualified_type (type, TYPE_QUAL_RESTRICT); - } + type = build_reference_type (type); + + if (restricted) + type = build_qualified_type (type, TYPE_QUAL_RESTRICT); } return (type); diff --git a/gcc/testsuite/gfortran.dg/coarray_poly_6.f90 b/gcc/testsuite/gfortran.dg/coarray_poly_6.f90 index 53b80e442d3..344e12b4eff 100644 --- a/gcc/testsuite/gfortran.dg/coarray_poly_6.f90 +++ b/gcc/testsuite/gfortran.dg/coarray_poly_6.f90 @@ -16,6 +16,6 @@ contains end subroutine foo end ! { dg-final { scan-tree-dump-times "foo \\(struct __class_MAIN___T_0_1t & restrict x, void \\* restrict caf_token.., integer\\(kind=\[48\]\\) caf_offset..\\)" 1 "original" } } -! { dg-final { scan-tree-dump-times "bar \\(struct __class_MAIN___T_0_1t \\* x, void \\* restrict caf_token.., integer\\(kind=\[48\]\\) caf_offset..\\)" 1 "original" } } +! { dg-final { scan-tree-dump-times "bar \\(struct __class_MAIN___T_0_1t \\* restrict x, void \\* restrict caf_token.., integer\\(kind=\[48\]\\) caf_offset..\\)" 1 "original" } } ! { dg-final { scan-tree-dump-times "bar \\(0B, 0B, 0\\);" 1 "original" } } ! { dg-final { scan-tree-dump-times "foo \\(&class.., y._data.token, \\(integer\\(kind=\[48\]\\)\\) class..._data.data - \\(integer\\(kind=\[48\]\\)\\) y._data.data\\);" 1 "original" } } diff --git a/gcc/testsuite/gfortran.dg/coarray_poly_7.f90 b/gcc/testsuite/gfortran.dg/coarray_poly_7.f90 index 44f98e16e09..d8d83aea39b 100644 --- a/gc
Re: {Patch, fortran] PR112834 - Class array function selector causes chain of syntax and other spurious errors
Hi Paul, On 12/6/23 17:09, Paul Richard Thomas wrote: Dear All, This patch was rescued from my ill-fated and long winded attempt to provide a fix-up for function selector references, where the function is parsed after the procedure containing the associate/select type construct (PRs 89645 and 99065). The fix-ups broke down completely once these constructs were enclosed by another associate construct, where the selector is a derived type or class function. My inclination now is to introduce two pass parsing for contained procedures. Returning to PR112834, the patch is simple enough and is well described by the change logs. PR111853 was fixed as a side effect of the bigger patch. Steve Kargl had also posted the same fix on the PR. the patch looks good, but could you please check the coding style? @@ -6550,7 +6551,19 @@ select_type_set_tmp (gfc_typespec *ts) sym = tmp->n.sym; gfc_add_type (sym, ts, NULL); - if (selector->ts.type == BT_CLASS && selector->attr.class_ok + /* If the SELECT TYPE selector is a function we might be able to obtain +a typespec from the result. Since the function might not have been +parsed yet we have to check that there is indeed a result symbol. */ + if (selector->ts.type == BT_UNKNOWN + && gfc_state_stack->construct + && (expr2 = gfc_state_stack->construct->expr2) + && expr2->expr_type == EXPR_FUNCTION + && expr2->symtree + && expr2->symtree->n.sym && expr2->symtree->n.sym->result) Adding a line break before the second '&&' makes it more readable. + selector->ts = expr2->symtree->n.sym->result->ts; @@ -2037,7 +2038,12 @@ trans_associate_var (gfc_symbol *sym, gfc_wrapped_block *block) /* Class associate-names come this way because they are unconditionally associate pointers and the symbol is scalar. */ - if (sym->ts.type == BT_CLASS && CLASS_DATA (sym)->attr.dimension) + if (sym->ts.type == BT_CLASS && e->expr_type ==EXPR_FUNCTION) There should be whitespace before AND after '=='. + { + gfc_conv_expr (&se, e); + se.expr = gfc_evaluate_now (se.expr, &se.pre); + } + else if (sym->ts.type == BT_CLASS && CLASS_DATA (sym)->attr.dimension) Regression tests - OK for trunk and 13-branch? Paul Thanks for the patch! Harald
[PATCH] Fortran: function returning contiguous class array [PR105543]
Dear all, the attached patch fixes a rejects-valid for functions returning a contiguous CLASS result. The problem occurs because attr.class_ok is inconsistent between sym and sym->result at the time the check of the contiguous attribute is done. I first thought that resolve_fl_procedure would be the right place to do this fixup, but this is invoked only later from resolve_symbol. Another attempt to put a fix directly after the recursive call to resolve_symbol for sym->result lead to frightening regressions in the testsuite, so I stayed with the attached simple solution. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 15810999b2f5cb4d8fbd69cb488c9b0c58e6 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 6 Dec 2023 20:42:27 +0100 Subject: [PATCH] Fortran: function returning contiguous class array [PR105543] gcc/fortran/ChangeLog: PR fortran/105543 * resolve.cc (resolve_symbol): For a CLASS-valued function having a RESULT clause, ensure that attr.class_ok is set for its symbol as well as for its resolved result variable. gcc/testsuite/ChangeLog: PR fortran/105543 * gfortran.dg/contiguous_13.f90: New test. --- gcc/fortran/resolve.cc | 5 + gcc/testsuite/gfortran.dg/contiguous_13.f90 | 22 + 2 files changed, 27 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/contiguous_13.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 166b702cd9a..4fe0e7202e5 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -16102,6 +16102,11 @@ resolve_symbol (gfc_symbol *sym) specification_expr = saved_specification_expr; } + /* For a CLASS-valued function with a result variable, affirm that it has + been resolved also when looking at the symbol 'sym'. */ + if (mp_flag && sym->ts.type == BT_CLASS && sym->result->attr.class_ok) +sym->attr.class_ok = sym->result->attr.class_ok; + if (sym->ts.type == BT_CLASS && sym->attr.class_ok && sym->ts.u.derived && CLASS_DATA (sym)) { diff --git a/gcc/testsuite/gfortran.dg/contiguous_13.f90 b/gcc/testsuite/gfortran.dg/contiguous_13.f90 new file mode 100644 index 000..8c6784432c9 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/contiguous_13.f90 @@ -0,0 +1,22 @@ +! { dg-do compile } +! PR fortran/105543 - function returning contiguous class array +! Contributed by martin + +module func_contiguous + implicit none + type :: a + end type a +contains + function create1 () result(x) +class(a), dimension(:), contiguous, pointer :: x + end + function create2 () +class(a), dimension(:), contiguous, pointer :: create2 + end + function create3 () result(x) +class(*), dimension(:), contiguous, pointer :: x + end + function create4 () +class(*), dimension(:), contiguous, pointer :: create4 + end +end module func_contiguous -- 2.35.3
[PATCH] Fortran: allow NULL() for POINTER, OPTIONAL, CONTIGUOUS dummy [PR111503]
Dear all, here's another fix for the CONTIGUOUS attribute: NULL() should derive its characteristics from its MOLD argument; otherwise it is "determined by the entity with which the reference is associated". (F2018:16.9.144). The testcase is cross-checked with Intel. NAG rejects cases where MOLD is a pointer. I think it is wrong here. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From c73b248ec16388ed1ce109fce8a468a87e367085 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 8 Dec 2023 11:11:08 +0100 Subject: [PATCH] Fortran: allow NULL() for POINTER, OPTIONAL, CONTIGUOUS dummy [PR111503] gcc/fortran/ChangeLog: PR fortran/111503 * expr.cc (gfc_is_simply_contiguous): Determine characteristics of NULL() from MOLD argument if present, otherwise treat as present. * primary.cc (gfc_variable_attr): Derive attributes of NULL(MOLD) from MOLD. gcc/testsuite/ChangeLog: PR fortran/111503 * gfortran.dg/contiguous_14.f90: New test. --- gcc/fortran/expr.cc | 14 gcc/fortran/primary.cc | 4 ++- gcc/testsuite/gfortran.dg/contiguous_14.f90 | 39 + 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/contiguous_14.f90 diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index c668baeef8c..709f3c3cbef 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -5958,6 +5958,20 @@ gfc_is_simply_contiguous (gfc_expr *expr, bool strict, bool permit_element) if (expr->expr_type == EXPR_ARRAY) return true; + if (expr->expr_type == EXPR_NULL) +{ + /* F2018:16.9.144 NULL ([MOLD]): + "If MOLD is present, the characteristics are the same as MOLD." + "If MOLD is absent, the characteristics of the result are + determined by the entity with which the reference is associated." + F2018:15.3.2.2 characteristics attributes include CONTIGUOUS. */ + if (expr->ts.type == BT_UNKNOWN) + return true; + else + return (gfc_variable_attr (expr, NULL).contiguous + || gfc_variable_attr (expr, NULL).allocatable); +} + if (expr->expr_type == EXPR_FUNCTION) { if (expr->value.function.isym) diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 7278932b634..f8a1c09d190 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -2627,7 +2627,9 @@ gfc_variable_attr (gfc_expr *expr, gfc_typespec *ts) gfc_component *comp; bool has_inquiry_part; - if (expr->expr_type != EXPR_VARIABLE && expr->expr_type != EXPR_FUNCTION) + if (expr->expr_type != EXPR_VARIABLE + && expr->expr_type != EXPR_FUNCTION + && !(expr->expr_type == EXPR_NULL && expr->ts.type != BT_UNKNOWN)) gfc_internal_error ("gfc_variable_attr(): Expression isn't a variable"); sym = expr->symtree->n.sym; diff --git a/gcc/testsuite/gfortran.dg/contiguous_14.f90 b/gcc/testsuite/gfortran.dg/contiguous_14.f90 new file mode 100644 index 000..21e42311e9c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/contiguous_14.f90 @@ -0,0 +1,39 @@ +! { dg-do compile } +! PR fortran/111503 - passing NULL() to POINTER, OPTIONAL, CONTIGUOUS dummy + +program test + implicit none + integer, pointer, contiguous :: p(:) => null() + integer, allocatable, target :: a(:) + type t + integer, pointer, contiguous :: p(:) => null() + integer, allocatable :: a(:) + end type t + type(t), target :: z + class(t), allocatable, target :: c + print *, is_contiguous (p) + allocate (t :: c) + call one (p) + call one () + call one (null ()) + call one (null (p)) + call one (a) + call one (null (a)) + call one (z% p) + call one (z% a) + call one (null (z% p)) + call one (null (z% a)) + call one (c% p) + call one (c% a) + call one (null (c% p)) + call one (null (c% a)) +contains + subroutine one (x) +integer, pointer, optional, contiguous, intent(in) :: x(:) +print *, present (x) +if (present (x)) then + print *, "->", associated (x) + if (associated (x)) stop 99 +end if + end subroutine one +end -- 2.35.3
Re: [PATCH 1/2] libgfortran: Remove early return if extent is zero [PR112371]
Hi Mikael, Am 06.11.23 um 12:43 schrieb Mikael Morin: Remove the early return present in function templates for transformational functions doing a (masked) reduction of an array along a dimension. This early return, which triggered if the extent in the reduction dimension was zero, was wrong because even if the reduction operation degenerates to a constant value in that case, one has to loop anyway along the other dimensions to initialize every element of the resulting array with that constant value. The offending piece of code was present in several places, and this removes them all. Namely, the impacted m4 files are ifunction.m4 for regular functions and types, ifunction-s.m4 for character minloc and maxloc, and ifunction-s2.m4 for character minval and maxval. I wonder if the correct fix would be to replace (instead of deleting) diff --git a/libgfortran/m4/ifunction.m4 b/libgfortran/m4/ifunction.m4 index c64217ec5db..480649cf691 100644 --- a/libgfortran/m4/ifunction.m4 +++ b/libgfortran/m4/ifunction.m4 @@ -232,8 +232,6 @@ m'name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, } len = GFC_DESCRIPTOR_EXTENT(array,dim); - if (len <= 0) -return; mbase = mask->base_addr; by the following: if (len < 0) len = 0; See ifunction.m4, lines 56ff, which check if the result of len = GFC_DESCRIPTOR_EXTENT(array,dim); is negative. I haven't tried to create a testcase, though. Similarly for the other templates. Thanks, Harald
Re: [PATCH 2/2] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371]
Hi Mikael, Am 06.11.23 um 12:43 schrieb Mikael Morin: Remove the forced overwrite of the first dimension of the result array descriptor to set it to zero extent, in the function templates for transformational functions doing an array reduction along a dimension. This overwrite, which happened before early returning in case the result array was empty, was wrong because an array may have a non-zero extent in the first dimension and still be empty if it has a zero extent in a higher dimension. Overwriting the dimension was resulting in wrong array result upper bound for the first dimension in that case. The offending piece of code was present in several places, and this removes them all. More precisely, there is only one case to fix for logical reduction functions, and there are three cases for other reduction functions, corresponding to non-masked reduction, reduction with array mask, and reduction with scalar mask. The impacted m4 files are ifunction_logical.m4 for logical reduction functions, ifunction.m4 for regular functions and types, ifunction-s.m4 for character minloc and maxloc, ifunction-s2.m4 for character minval and maxval, and ifindloc1.m4 for findloc. while your fix seems mechanical and correct, I wonder if you looked at the following pre-existing irregularity which can be seen in this snippet: diff --git a/libgfortran/m4/ifunction.m4 b/libgfortran/m4/ifunction.m4 index 480649cf691..abc15b430ab 100644 --- a/libgfortran/m4/ifunction.m4 +++ b/libgfortran/m4/ifunction.m4 @@ -96,12 +96,7 @@ name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); if (alloc_size == 0) - { - /* Make sure we have a zero-sized array. */ - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); - return; - - } + return; } else { This is all enclosed in a block which has if (retarray->base_addr == NULL) but allocates and sets retarray->base_addr, while @@ -290,11 +285,7 @@ m'name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, retarray->dtype.rank = rank; if (alloc_size == 0) - { - /* Make sure we have a zero-sized array. */ - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); - return; - } + return; else retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); and @@ -454,11 +445,7 @@ void alloc_size = GFC_DESCRIPTOR_STRIDE(retarray,rank-1) * extent[rank-1]; if (alloc_size == 0) - { - /* Make sure we have a zero-sized array. */ - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); - return; - } + return; else retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); } do not set retarray->base_addr to non-NULL for alloc_size == 0. Do you know if the first snippet can be safely rewritten to avoid the (hopefully pointless) xmallocarray for alloc_size == 0? Thanks, Harald
Re: [PATCH 2/2] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371]
Hi Mikael, Am 06.11.23 um 20:19 schrieb Mikael Morin: This change to the testcase: diff --git a/gcc/testsuite/gfortran.dg/bound_11.f90 b/gcc/testsuite/gfortran.dg/bound_11.f90 index 170eba4ddfd..2e96f843476 100644 --- a/gcc/testsuite/gfortran.dg/bound_11.f90 +++ b/gcc/testsuite/gfortran.dg/bound_11.f90 @@ -88,6 +88,7 @@ contains m4 = .false. i = 1 r = sum(a, dim=i) + if (.not. allocated(r)) stop 210 if (any(shape(r) /= (/ 3, 0, 7 /))) stop 211 if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 212 i = 2 @@ -104,6 +105,7 @@ contains if (any(ubound(r) /= (/ 9, 3, 0 /))) stop 218 i = 1 r = sum(a, dim=i, mask=m1) + if (.not. allocated(r)) stop 220 if (any(shape(r) /= (/ 3, 0, 7 /))) stop 221 if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 222 i = 2 @@ -120,6 +122,7 @@ contains if (any(ubound(r) /= (/ 9, 3, 0 /))) stop 228 i = 1 r = sum(a, dim=i, mask=m4) + if (.not. allocated(r)) stop 230 if (any(shape(r) /= (/ 3, 0, 7 /))) stop 231 if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 232 i = 2 gives me a FAIL with STOP 220 (or STOP 230 if the STOP 220 line is commented); the first one with STOP 210 passes. So it is the first snippet with the xmallocarray (which supports zero values see memory.c) call that is the correct one. Good catch, I will open a separate PR. ah, now I see that the case of allocation of zero elements always allocates one byte, which is needed for r.data to be non-null. Go ahead! Harald Mikael
Re: [PATCH v2 0/3] libgfortran: empty array fixes
Hi Mikael, this is OK. Thanks for the patches! Harald On 11/7/23 11:24, Mikael Morin wrote: Hello, Harald's review of the previous version [1] of these patches spotted a possible misbehaving case in one patch, and a latent bug in the area of the second patch. So here is the second try, bootstraped and regression tested on x86_64-pc-linux-gnu. OK for master? Mikael [1]: https://gcc.gnu.org/pipermail/fortran/2023-November/059896.html https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635305.html Changes from version 1: * Add patch 1/3 to the series fixing the unallocated empty result issue. * In patch 2/3 (formerly 1/2) clamp negative extent to zero. Mikael Morin (3): libgfortran: Don't skip allocation if size is zero [PR112412] libgfortran: Remove early return if extent is zero [PR112371] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371] gcc/testsuite/gfortran.dg/allocated_4.f90 | 195 +++ gcc/testsuite/gfortran.dg/bound_10.f90| 207 gcc/testsuite/gfortran.dg/bound_11.f90| 588 ++ libgfortran/generated/all_l1.c| 9 +- libgfortran/generated/all_l16.c | 9 +- libgfortran/generated/all_l2.c| 9 +- libgfortran/generated/all_l4.c| 9 +- libgfortran/generated/all_l8.c| 9 +- libgfortran/generated/any_l1.c| 9 +- libgfortran/generated/any_l16.c | 9 +- libgfortran/generated/any_l2.c| 9 +- libgfortran/generated/any_l4.c| 9 +- libgfortran/generated/any_l8.c| 9 +- libgfortran/generated/count_16_l.c| 9 +- libgfortran/generated/count_1_l.c | 9 +- libgfortran/generated/count_2_l.c | 9 +- libgfortran/generated/count_4_l.c | 9 +- libgfortran/generated/count_8_l.c | 9 +- libgfortran/generated/findloc1_c10.c | 18 +- libgfortran/generated/findloc1_c16.c | 18 +- libgfortran/generated/findloc1_c17.c | 18 +- libgfortran/generated/findloc1_c4.c | 18 +- libgfortran/generated/findloc1_c8.c | 18 +- libgfortran/generated/findloc1_i1.c | 18 +- libgfortran/generated/findloc1_i16.c | 18 +- libgfortran/generated/findloc1_i2.c | 18 +- libgfortran/generated/findloc1_i4.c | 18 +- libgfortran/generated/findloc1_i8.c | 18 +- libgfortran/generated/findloc1_r10.c | 18 +- libgfortran/generated/findloc1_r16.c | 18 +- libgfortran/generated/findloc1_r17.c | 18 +- libgfortran/generated/findloc1_r4.c | 18 +- libgfortran/generated/findloc1_r8.c | 18 +- libgfortran/generated/findloc1_s1.c | 18 +- libgfortran/generated/findloc1_s4.c | 18 +- libgfortran/generated/iall_i1.c | 30 +- libgfortran/generated/iall_i16.c | 30 +- libgfortran/generated/iall_i2.c | 30 +- libgfortran/generated/iall_i4.c | 30 +- libgfortran/generated/iall_i8.c | 30 +- libgfortran/generated/iany_i1.c | 30 +- libgfortran/generated/iany_i16.c | 30 +- libgfortran/generated/iany_i2.c | 30 +- libgfortran/generated/iany_i4.c | 30 +- libgfortran/generated/iany_i8.c | 30 +- libgfortran/generated/iparity_i1.c| 30 +- libgfortran/generated/iparity_i16.c | 30 +- libgfortran/generated/iparity_i2.c| 30 +- libgfortran/generated/iparity_i4.c| 30 +- libgfortran/generated/iparity_i8.c| 30 +- libgfortran/generated/maxloc1_16_i1.c | 30 +- libgfortran/generated/maxloc1_16_i16.c| 30 +- libgfortran/generated/maxloc1_16_i2.c | 30 +- libgfortran/generated/maxloc1_16_i4.c | 30 +- libgfortran/generated/maxloc1_16_i8.c | 30 +- libgfortran/generated/maxloc1_16_r10.c| 30 +- libgfortran/generated/maxloc1_16_r16.c| 30 +- libgfortran/generated/maxloc1_16_r17.c| 30 +- libgfortran/generated/maxloc1_16_r4.c | 30 +- libgfortran/generated/maxloc1_16_r8.c | 30 +- libgfortran/generated/maxloc1_16_s1.c | 30 +- libgfortran/generated/maxloc1_16_s4.c | 30 +- libgfortran/generated/maxloc1_4_i1.c | 30 +- libgfortran/generated/maxloc1_4_i16.c | 30 +- libgfortran/generated/maxloc1_4_i2.c | 30 +- libgfortran/generated/maxloc1_4_i4.c | 30 +- libgfortran/generated/maxloc1_4_i8.c | 30 +- libgfortran/generated/maxloc1_4_r10.c | 30 +- libgfortran/generated/maxloc1_4_r16.c | 30 +- libgfortran/generated/maxloc1_4_r17.c | 30 +- libgfortran/generated/maxloc1_4_r4.c | 30 +- libgfortran/generated/maxloc1_4_r8.c | 30 +- libgfortran/generated/maxloc1_4_s1.c | 30 +- libgfortran/generated/maxloc1_4_s4.c | 30 +- libgfortran/generated/maxloc1_8_i1.c | 30 +- libgfortran/generated/maxloc1_8_i16.c | 30 +- libgfortran/generated/
Re: [Patch, fortran] PR112459 - gfortran -w option causes derived-type finalization at creation time
Hi Paul, this is OK. Thanks for the patch! Harald Am 11.11.23 um 11:15 schrieb Paul Richard Thomas: Hi All, Evidently -w causes gfc_option.allow_std to be set to default, which allows anything and everything to happen, including these f2003/8 finalizations. The fix is trivial. Regtests fine - OK for mainline and -13 branch? Paul Fortran: Prevent unwanted finalization with -w option [PR112459] 2023-11-11 Paul Thomas gcc/fortran PR fortran/112459 * trans-array.cc (gfc_trans_array_constructor_value): Replace gfc_notification_std with explicit logical expression that selects F2003/2008 and excludes -std=default/gnu. *trans-array.cc (gfc_conv_expr): Ditto. gcc/testsuite/ PR fortran/112459 * gfortran.dg/pr112459.f90: New test.
Re: [Patch] Fortran: Accept -std=f2023 support, update line-length for Fortran 2023
Hi Tobias, On 11/16/23 14:01, Tobias Burnus wrote: This adds -std=f2023, which is mostly a prep patch for future changes. However, Fortran 2023, https://j3-fortran.org/doc/year/23/23-007r1.pdf changes two things which is taken care in this patch: (A) In "6.3.2.1 Free form line length": Fortran 2018: "If a line consists entirely of characters of default kind (7.4.4), it shall contain at most 132 characters" Fortran 2023: "A line shall contain at most ten thousand characters." (B) In "6.3.2.6 Free form statements": Fortran 2018: "A statement shall not have more than 255 continuation lines." Fortran 2023: "A statement shall not have more than one million characters." this is really a funny change: we're not really prepared to handle this. According to the standard one can have 99 lines with only "&" and then an ";", but then only 100 lines with 1 characters. There is a similar wording for fixed-form which you overlooked: 6.3.3.5 Fixed form statements Fortran 2023: "A statement shall not have more than one million characters" Please adjust the fixed-form limits in your patch. If you think that we need testcases for fixed-form, add them, or forget them. I don't bother. I have not added a testcase for exceeding the latter but otherwise there are new tests and I had to add a couple of -std=f2018 to existing tests. Comments, suggestions, approval? I have the following comments: - there are existing testcases continuation_5.f, continuation_6.f, thus I suggest to rename your new continuation_{5,6}.f90 to continuation_17.f90+ . - I don't understand your new testcase line_length_14.f90 . This is supposed to test -std=gnu, but then -std=gnu is not a standard but a moving target, which is why you had to adjust existing testcases. So what does it buy us beyond line_length_1{2,3}.f90 ? Tobias PS: I find it funny that -std=c23, -std=c++23 and -std=f2023 will get added in the same GCC release. :-) PPS: I did not bother adding .f23 as file extension; I believe that also .f18 is unsupported. I never use extensions other than .f90 for portable code. With the above fixed, I am fine with your patch. Thanks, Harald - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955