[PATCH] fortran: fix checking of CHARACTER lengths in array constructors [PR70231]
Dear all, as correctly analyzed by Jerry, the code for checking the consistency of character lengths in array constructors did not properly initialize the auxiliary variable used in "bounds checking". The attached patch resolves this by initializing this auxiliary variable with a length obtained by scanning the constructor. Interestingly, the failure depended on optimization level and was apparent only at -O0, and could exhibit both false-positives and false-negatives. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 14b12cba8b7def5e07a3cac5efd2be8ab49d8133 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 18 Sep 2023 22:11:40 +0200 Subject: [PATCH] fortran: fix checking of CHARACTER lengths in array constructors [PR70231] gcc/fortran/ChangeLog: PR fortran/70231 * trans-array.cc (trans_array_constructor): In absence of a typespec, use string length determined by get_array_ctor_strlen() to reasonably initialize auxiliary variable for bounds-checking. gcc/testsuite/ChangeLog: PR fortran/70231 * gfortran.dg/bounds_check_fail_7.f90: New test. --- gcc/fortran/trans-array.cc| 17 .../gfortran.dg/bounds_check_fail_7.f90 | 20 +++ 2 files changed, 37 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 1640587cd71..e0fc8ebc46b 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -2852,6 +2852,23 @@ trans_array_constructor (gfc_ss * ss, locus * where) const_string = get_array_ctor_strlen (&outer_loop->pre, c, &ss_info->string_length); force_new_cl = true; + + /* Initialize "len" with string length for bounds checking. */ + if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) + && !typespec_chararray_ctor + && ss_info->string_length) + { + gfc_se length_se; + + gfc_init_se (&length_se, NULL); + gfc_add_modify (&length_se.pre, first_len_val, + fold_convert (TREE_TYPE (first_len_val), + ss_info->string_length)); + ss_info->string_length = gfc_evaluate_now (ss_info->string_length, + &length_se.pre); + gfc_add_block_to_block (&outer_loop->pre, &length_se.pre); + gfc_add_block_to_block (&outer_loop->post, &length_se.post); + } } /* Complex character array constructors should have been taken care of diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90 new file mode 100644 index 000..6a8dafc27a8 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_7.f90 @@ -0,0 +1,20 @@ +! { dg-do run } +! { dg-additional-options "-fcheck=bounds -g" } +! { dg-output "At line 18 .*" } +! { dg-shouldfail "Different CHARACTER lengths (32/0) in array constructor" } +! +! PR fortran/70231 - CHARACTER lengths in array constructors + +program p + implicit none + integer, parameter :: char_len = 32 + integer :: l = 0 + character(char_len) :: ch = "a" + character(char_len), allocatable :: ch_array(:), res1(:), res2(:) + + allocate(ch_array(0)) + res1 = [ ch_array, ch ] ! was false positive + print *, res1 + res2 = [[ch_array, ch(1:l)], ch(1:l)] ! was false negative on x86 + print *, res2 +end -- 2.35.3
Re: [PATCH] Fortran: improve bounds-checking for array sections [PR30802]
Hi Paul, On 9/15/23 11:13, Paul Richard Thomas via Gcc-patches wrote: Hi Harald, The statement, in array_bound_check_elemental is redundant since the call is determined by a more restrictive condition. + if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)) +return; yeah, this was left over from playing, since I thought the outlined function could be used more than once. I've removed those lines before pushing. Thanks for the review! Harald Apart from that, it looks good to me. OK for mainline. Thanks for the patch. Paul On Thu, 14 Sept 2023 at 21:22, Harald Anlauf via Fortran wrote: Dear all, array bounds checking was missing a few cases of array sections that are handled via gfc_conv_expr_descriptor. Bounds checking was done for the dimensions with ranges, but not for elemental dimensions. The attached patch implements that and fixes pr30802 and also pr97039, maybe a few more similar cases. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald
[PATCH] Fortran: improve bounds-checking for array sections [PR30802]
Dear all, array bounds checking was missing a few cases of array sections that are handled via gfc_conv_expr_descriptor. Bounds checking was done for the dimensions with ranges, but not for elemental dimensions. The attached patch implements that and fixes pr30802 and also pr97039, maybe a few more similar cases. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From bb2a765f56b440c8d086329f55c8ff0eaee2b97d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 14 Sep 2023 22:08:26 +0200 Subject: [PATCH] Fortran: improve bounds-checking for array sections [PR30802] gcc/fortran/ChangeLog: PR fortran/30802 * trans-array.cc (trans_array_bound_check): Add optional argument COMPNAME for explicit specification of array component name. (array_bound_check_elemental): Helper function for generating bounds-checking code for elemental dimensions. (gfc_conv_expr_descriptor): Use bounds-checking also for elemental dimensions, i.e. those not handled by the scalarizer. gcc/testsuite/ChangeLog: PR fortran/30802 * gfortran.dg/bounds_check_fail_6.f90: New test. --- gcc/fortran/trans-array.cc| 72 ++- .../gfortran.dg/bounds_check_fail_6.f90 | 29 2 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 6ca58e98547..71123e37477 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3452,7 +3452,8 @@ gfc_conv_array_ubound (tree descriptor, int dim) static tree trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, - locus * where, bool check_upper) + locus * where, bool check_upper, + const char *compname = NULL) { tree fault; tree tmp_lo, tmp_up; @@ -3474,6 +3475,10 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, if (VAR_P (descriptor)) name = IDENTIFIER_POINTER (DECL_NAME (descriptor)); + /* Use given (array component) name. */ + if (compname) +name = compname; + /* If upper bound is present, include both bounds in the error message. */ if (check_upper) { @@ -3524,6 +3529,67 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, } +/* Generate code for bounds checking for elemental dimensions. */ + +static void +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; + int dim; + + if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)) +return; + + 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); + + 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.dimen > 0) + { + ar = &ref->u.ar; + for (dim = 0; dim < ar->dimen; dim++) + { + if (ar->dimen_type[dim] == DIMEN_ELEMENT) + { + gfc_se indexse; + gfc_init_se (&indexse, NULL); + gfc_conv_expr_type (&indexse, ar->start[dim], + gfc_array_index_type); + trans_array_bound_check (se, ss, indexse.expr, dim, + &ar->where, + ar->as->type != AS_ASSUMED_SIZE + || dim < ar->dimen - 1, + var_name); + } + } + } + } +} +} + + /* Return the offset for an index. Performs bound checking for elemental dimensions. Single element references are processed separately. DIM is the array dimension, I is the loop dimension. */ @@ -7823,6 +7889,10 @@ gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr) /* Setup the scalarizing loops and bounds. */ gfc_conv_ss_startstride (&loop); + /* Add bounds-checking for elemental dimensions. */ + if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) && !expr->no_bounds_check) +array_bound_check_elemental (se, ss, expr); + if (need_tmp) { if (expr->ts.type == BT_CHARACTER diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90 new file mode 100644 index 000..90329131158 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90 @@ -0,0 +1,29 @@ +! { dg-do run } +! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" } +! { dg-output "At line 18 .*" } +! { dg-shouldfail "dimension 3 of array 'u%z' outside of expected range" } +! +! PR fortran/30802 - improve bounds-checking for array sections + +program test + implicit none + integer :: k = 0 + int
Re: [PATCH] fortran: Undo new symbols in all namespaces [PR110996]
Hi Mikael, On 9/11/23 14:38, Mikael Morin via Gcc-patches wrote: Hello, this fixes a memory management issue in the fortran frontend. I have included the (reduced) testcase from the PR, even if it wasn't failing here on x86_64 with the test harness. Tested on x86_64-pc-linux-gnu and manually checked the testcase with valgrind. OK for master? nice fix! This is OK for mainline. Thanks for the patch! Harald -- >8 -- Remove new symbols from all namespaces they may have been added to in case a statement mismatches in the end and all the symbols referenced in it have to be reverted. This fixes memory errors and random internal compiler errors caused by a new symbol left with dangling pointers but not properly removed from the namespace at statement rejection. Before this change, new symbols were removed from their own namespace (their ns field) only. This change additionally removes them from the namespaces pointed to by respectively the gfc_current_ns global variable, and the symbols' formal_ns field. PR fortran/110996 gcc/fortran/ChangeLog: * gfortran.h (gfc_release_symbol): Set return type to bool. * symbol.cc (gfc_release_symbol): Ditto. Return whether symbol was freed. (delete_symbol_from_ns): New, outline code from... (gfc_restore_last_undo_checkpoint): ... here. Delete new symbols from two more namespaces. gcc/testsuite/ChangeLog: * gfortran.dg/pr110996.f90: New test. --- gcc/fortran/gfortran.h | 2 +- gcc/fortran/symbol.cc | 57 -- gcc/testsuite/gfortran.dg/pr110996.f90 | 16 3 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr110996.f90 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 371f8743312..f4a1c106cea 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3514,7 +3514,7 @@ gfc_symtree *gfc_get_unique_symtree (gfc_namespace *); gfc_user_op *gfc_get_uop (const char *); gfc_user_op *gfc_find_uop (const char *, gfc_namespace *); void gfc_free_symbol (gfc_symbol *&); -void gfc_release_symbol (gfc_symbol *&); +bool gfc_release_symbol (gfc_symbol *&); gfc_symbol *gfc_new_symbol (const char *, gfc_namespace *); gfc_symtree* gfc_find_symtree_in_proc (const char *, gfc_namespace *); int gfc_find_symbol (const char *, gfc_namespace *, int, gfc_symbol **); diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index 2cba2ea0bed..a6078bc608a 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc @@ -3105,13 +3105,14 @@ gfc_free_symbol (gfc_symbol *&sym) } -/* Decrease the reference counter and free memory when we reach zero. */ +/* Decrease the reference counter and free memory when we reach zero. + Returns true if the symbol has been freed, false otherwise. */ -void +bool gfc_release_symbol (gfc_symbol *&sym) { if (sym == NULL) -return; +return false; if (sym->formal_ns != NULL && sym->refs == 2 && sym->formal_ns != sym->ns && (!sym->attr.entry || !sym->module)) @@ -3125,10 +3126,11 @@ gfc_release_symbol (gfc_symbol *&sym) sym->refs--; if (sym->refs > 0) -return; +return false; gcc_assert (sym->refs == 0); gfc_free_symbol (sym); + return true; } @@ -3649,6 +3651,29 @@ gfc_drop_last_undo_checkpoint (void) } +/* Remove the reference to the symbol SYM in the symbol tree held by NS + and free SYM if the last reference to it has been removed. + Returns whether the symbol has been freed. */ + +static bool +delete_symbol_from_ns (gfc_symbol *sym, gfc_namespace *ns) +{ + if (ns == nullptr) +return false; + + /* The derived type is saved in the symtree with the first + letter capitalized; the all lower-case version to the + derived type contains its associated generic function. */ + const char *sym_name = gfc_fl_struct (sym->attr.flavor) +? gfc_dt_upper_string (sym->name) +: sym->name; + + gfc_delete_symtree (&ns->sym_root, sym_name); + + return gfc_release_symbol (sym); +} + + /* Undoes all the changes made to symbols since the previous checkpoint. This subroutine is made simpler due to the fact that attributes are never removed once added. */ @@ -3703,15 +3728,23 @@ gfc_restore_last_undo_checkpoint (void) } if (p->gfc_new) { - /* The derived type is saved in the symtree with the first -letter capitalized; the all lower-case version to the -derived type contains its associated generic function. */ - if (gfc_fl_struct (p->attr.flavor)) - gfc_delete_symtree (&p->ns->sym_root,gfc_dt_upper_string (p->name)); - else - gfc_delete_symtree (&p->ns->sym_root, p->name); + bool freed = delete_symbol_from_ns (p, p->ns); - gfc_release_symbol (p); + /* If the symbol is a procedure (function or subrou
Re: [PATCH] fortran: Remove redundant tree walk to delete element
Am 08.09.23 um 12:04 schrieb Mikael Morin via Gcc-patches: Hello, this avoids some redundant work in the symbol deletion code, which is used a lot by the parser to cancel statements that fail to match in the end. I haven't tried to measure the performance effect, if any, on a pathological example; just passed the fortran testsuite on x86_64-pc-linux-gnu. OK for master? This is OK. Thanks, Harald -- >8 -- Remove preliminary walk of the symbol tree when we are about to remove an element. This preliminary walk was necessary because the deletion function updated the tree without reporting back to the caller the element it had removed. But knowing that element is necessary to free its memory, so one had to first get that element before it was removed from the tree. This change updates the main deletion function delete_treap and its public wrapper gfc_delete_bbt so that the removed element can be known by the caller. This makes the preliminary walk in gfc_delete_symtree redundant, permitting its removal. gcc/fortran/ChangeLog: * bbt.cc (delete_treap): Add argument REMOVED, set it to the removed element from the tree. Change NULL to nullptr. (gfc_delete_bbt): Return the removed element from the tree. * gfortran.h (gfc_delete_symtree): Remove prototype. (gfc_delete_bbt): Set return type to pointer. * symbol.cc (gfc_delete_symtree): Make static. Get the element to be freed from the result of gfc_delete_bbt. Remove the preliminary walk to get it. --- gcc/fortran/bbt.cc | 27 +++ gcc/fortran/gfortran.h | 3 +-- gcc/fortran/symbol.cc | 6 ++ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/gcc/fortran/bbt.cc b/gcc/fortran/bbt.cc index 851e5e92c7b..2a032083c5c 100644 --- a/gcc/fortran/bbt.cc +++ b/gcc/fortran/bbt.cc @@ -168,31 +168,42 @@ delete_root (gfc_bbt *t) Returns the new root node of the tree. */ static gfc_bbt * -delete_treap (gfc_bbt *old, gfc_bbt *t, compare_fn compare) +delete_treap (gfc_bbt *old, gfc_bbt *t, compare_fn compare, gfc_bbt **removed) { int c; - if (t == NULL) -return NULL; + if (t == nullptr) +{ + if (removed) + *removed = nullptr; + return nullptr; +} c = (*compare) (old, t); if (c < 0) -t->left = delete_treap (old, t->left, compare); +t->left = delete_treap (old, t->left, compare, removed); if (c > 0) -t->right = delete_treap (old, t->right, compare); +t->right = delete_treap (old, t->right, compare, removed); if (c == 0) -t = delete_root (t); +{ + if (removed) + *removed = t; + t = delete_root (t); +} return t; } -void +void * gfc_delete_bbt (void *root, void *old, compare_fn compare) { gfc_bbt **t; + gfc_bbt *removed; t = (gfc_bbt **) root; - *t = delete_treap ((gfc_bbt *) old, *t, compare); + *t = delete_treap ((gfc_bbt *) old, *t, compare, &removed); + + return (void *) removed; } diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index b37c6bb9ad4..371f8743312 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3510,7 +3510,6 @@ bool gfc_reference_st_label (gfc_st_label *, gfc_sl_type); gfc_namespace *gfc_get_namespace (gfc_namespace *, int); gfc_symtree *gfc_new_symtree (gfc_symtree **, const char *); gfc_symtree *gfc_find_symtree (gfc_symtree *, const char *); -void gfc_delete_symtree (gfc_symtree **, const char *); gfc_symtree *gfc_get_unique_symtree (gfc_namespace *); gfc_user_op *gfc_get_uop (const char *); gfc_user_op *gfc_find_uop (const char *, gfc_namespace *); @@ -3911,7 +3910,7 @@ bool gfc_inline_intrinsic_function_p (gfc_expr *); /* bbt.cc */ typedef int (*compare_fn) (void *, void *); void gfc_insert_bbt (void *, void *, compare_fn); -void gfc_delete_bbt (void *, void *, compare_fn); +void * gfc_delete_bbt (void *, void *, compare_fn); /* dump-parse-tree.cc */ void gfc_dump_parse_tree (gfc_namespace *, FILE *); diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index aa3cdc98c86..2cba2ea0bed 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc @@ -2948,7 +2948,7 @@ gfc_new_symtree (gfc_symtree **root, const char *name) /* Delete a symbol from the tree. Does not free the symbol itself! */ -void +static void gfc_delete_symtree (gfc_symtree **root, const char *name) { gfc_symtree st, *st0; @@ -2963,10 +2963,8 @@ gfc_delete_symtree (gfc_symtree **root, const char *name) else p = name; - st0 = gfc_find_symtree (*root, p); - st.name = gfc_get_string ("%s", p); - gfc_delete_bbt (root, &st, compare_symtree); + st0 = (gfc_symtree *) gfc_delete_bbt (root, &st, compare_symtree); free (st0); }
Re: [PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059]
Hi Mikael, On 9/1/23 10:41, Mikael Morin via Gcc-patches wrote: Le 31/08/2023 à 22:42, Harald Anlauf via Fortran a écrit : Dear all, gfortran's array bounds-checking code does a mostly reasonable job for array sections in expressions and assignments, but forgot the case that (rank-1) expressions can involve array constructors, which have a shape ;-) The attached patch walks over the loops generated by the scalarizer, checks for the presence of a constructor, and takes the first shape found as reference. (If several constructors are present, discrepancies in their shape seems to be already detected at compile time). For more details on what will be caught now see testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is OK. I've pushed this is the first step. May I suggest to handle functions the same way? I'll have a look at them, but will need to gather a few suitable testcases first. Thanks for the review! Harald Thanks. Thanks, Harald
[PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059]
Dear all, gfortran's array bounds-checking code does a mostly reasonable job for array sections in expressions and assignments, but forgot the case that (rank-1) expressions can involve array constructors, which have a shape ;-) The attached patch walks over the loops generated by the scalarizer, checks for the presence of a constructor, and takes the first shape found as reference. (If several constructors are present, discrepancies in their shape seems to be already detected at compile time). For more details on what will be caught now see testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 944a35909e8eeb79c92e398ae3f27e94708584e6 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 31 Aug 2023 22:19:58 +0200 Subject: [PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059] gcc/fortran/ChangeLog: PR fortran/31059 * trans-array.cc (gfc_conv_ss_startstride): For array bounds checking, consider also array constructors in expressions, and use their shape. gcc/testsuite/ChangeLog: PR fortran/31059 * gfortran.dg/bounds_check_fail_5.f90: New test. --- gcc/fortran/trans-array.cc| 23 .../gfortran.dg/bounds_check_fail_5.f90 | 26 +++ 2 files changed, 49 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_5.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 90a7d4e9aef..6ca58e98547 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -4740,6 +4740,29 @@ done: for (n = 0; n < loop->dimen; n++) size[n] = NULL_TREE; + /* If there is a constructor involved, derive size[] from its shape. */ + for (ss = loop->ss; ss != gfc_ss_terminator; ss = ss->loop_chain) + { + gfc_ss_info *ss_info; + + ss_info = ss->info; + info = &ss_info->data.array; + + if (ss_info->type == GFC_SS_CONSTRUCTOR && info->shape) + { + for (n = 0; n < loop->dimen; n++) + { + if (size[n] == NULL) + { + gcc_assert (info->shape[n]); + size[n] = gfc_conv_mpz_to_tree (info->shape[n], + gfc_index_integer_kind); + } + } + break; + } + } + for (ss = loop->ss; ss != gfc_ss_terminator; ss = ss->loop_chain) { stmtblock_t inner; diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_5.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_5.f90 new file mode 100644 index 000..436cc96621d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_5.f90 @@ -0,0 +1,26 @@ +! { dg-do run } +! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" } +! { dg-output "At line 13 .*" } +! { dg-shouldfail "Array bound mismatch for dimension 1 of array 'ivec' (2/3)" } +! +! PR fortran/31059 - runtime bounds-checking in presence of array constructors + +program p + integer :: jvec(3) = [1,2,3] + integer, allocatable :: ivec(:), kvec(:), lvec(:), mvec(:), nvec(:) + ivec= [1,2] ! (re)allocation + kvec= [4,5,6] ! (re)allocation + ivec(:) = [4,5,6] ! runtime error (->dump) + ! not reached ... + print *, jvec + [1,2,3] ! OK & no check generated + print *, [4,5,6] + jvec ! OK & no check generated + print *, lvec + [1,2,3] ! check generated (->dump) + print *, [4,5,6] + mvec ! check generated (->dump) + nvec(:) = jvec ! check generated (->dump) +end + +! { dg-final { scan-tree-dump-times "Array bound mismatch " 4 "original" } } +! { dg-final { scan-tree-dump-times "Array bound mismatch .*ivec" 1 "original" } } +! { dg-final { scan-tree-dump-times "Array bound mismatch .*lvec" 1 "original" } } +! { dg-final { scan-tree-dump-times "Array bound mismatch .*mvec" 1 "original" } } +! { dg-final { scan-tree-dump-times "Array bound mismatch .*nvec" 1 "original" } } -- 2.35.3
Re: [PATCH] fortran: Restore interface to its previous state on error [PR48776]
Hi Mikael, On 8/27/23 21:22, Mikael Morin via Gcc-patches wrote: Hello, this fixes an old error-recovery bug. Tested on x86_64-pc-linux-gnu. OK for master? I have only a minor comment: +/* Free the leading members of the gfc_interface linked list given in INTR + up to the END element (exclusive: the END element is not freed). + If END is not nullptr, it is assumed that END is in the linked list starting + with INTR. */ + +static void +free_interface_elements_until (gfc_interface *intr, gfc_interface *end) +{ + gfc_interface *next; + + for (; intr != end; intr = next) Would it make sense to add a protection for intr == NULL, i.e.: + for (; intr && intr != end; intr = next) Just to prevent a NULL pointer dereference in case there is a corruption of the chain or something else went wrong. Otherwise it looks good to me. It appears that your patch similarly fixes PR107923. :-) Thanks for the patch! Harald
[PATCH] Fortran: improve bounds checking for DATA with implied-do [PR35095]
Dear all, the attached patch adds stricter bounds-checking for DATA statements with implied-do. I chose to allow overindexing (for arrays of rank greater than 1) for -std=legacy, as there might be codes in the wild that need this (and this is accepted by some other compilers, while NAG is strict here). We now get a warning with -std=gnu, and an error with -std=f. Regtested on x86_64-pc-linux-gnu. OK for mainline? (The PR is over 15 years old, so no backport intended... ;-) Thanks, Harald From 420804e7399dbc307a80f084cfb840444b8ebfe7 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 24 Aug 2023 23:16:25 +0200 Subject: [PATCH] Fortran: improve bounds checking for DATA with implied-do [PR35095] gcc/fortran/ChangeLog: PR fortran/35095 * data.cc (get_array_index): Add bounds-checking code and return error status. Overindexing will be allowed as an extension for -std=legacy and generate an error in standard-conforming mode. (gfc_assign_data_value): Use error status from get_array_index for graceful error recovery. gcc/testsuite/ChangeLog: PR fortran/35095 * gfortran.dg/data_bounds_1.f90: Adjust options to disable warnings. * gfortran.dg/data_bounds_2.f90: New test. --- gcc/fortran/data.cc | 47 ++--- gcc/testsuite/gfortran.dg/data_bounds_1.f90 | 2 +- gcc/testsuite/gfortran.dg/data_bounds_2.f90 | 9 3 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/data_bounds_2.f90 diff --git a/gcc/fortran/data.cc b/gcc/fortran/data.cc index 7c2537dd3f0..0589fc3906f 100644 --- a/gcc/fortran/data.cc +++ b/gcc/fortran/data.cc @@ -43,13 +43,14 @@ static void formalize_init_expr (gfc_expr *); /* Calculate the array element offset. */ -static void +static bool get_array_index (gfc_array_ref *ar, mpz_t *offset) { gfc_expr *e; int i; mpz_t delta; mpz_t tmp; + bool ok = true; mpz_init (tmp); mpz_set_si (*offset, 0); @@ -59,13 +60,42 @@ get_array_index (gfc_array_ref *ar, mpz_t *offset) e = gfc_copy_expr (ar->start[i]); gfc_simplify_expr (e, 1); - if ((gfc_is_constant_expr (ar->as->lower[i]) == 0) - || (gfc_is_constant_expr (ar->as->upper[i]) == 0) - || (gfc_is_constant_expr (e) == 0)) - gfc_error ("non-constant array in DATA statement %L", &ar->where); + if (!gfc_is_constant_expr (ar->as->lower[i]) + || !gfc_is_constant_expr (ar->as->upper[i]) + || !gfc_is_constant_expr (e)) + { + gfc_error ("non-constant array in DATA statement %L", &ar->where); + ok = false; + break; + } mpz_set (tmp, e->value.integer); gfc_free_expr (e); + + /* Overindexing is only allowed as a legacy extension. */ + if (mpz_cmp (tmp, ar->as->lower[i]->value.integer) < 0 + && !gfc_notify_std (GFC_STD_LEGACY, + "Subscript at %L below array lower bound " + "(%ld < %ld) in dimension %d", &ar->c_where[i], + mpz_get_si (tmp), + mpz_get_si (ar->as->lower[i]->value.integer), + i+1)) + { + ok = false; + break; + } + if (mpz_cmp (tmp, ar->as->upper[i]->value.integer) > 0 + && !gfc_notify_std (GFC_STD_LEGACY, + "Subscript at %L above array upper bound " + "(%ld > %ld) in dimension %d", &ar->c_where[i], + mpz_get_si (tmp), + mpz_get_si (ar->as->upper[i]->value.integer), + i+1)) + { + ok = false; + break; + } + mpz_sub (tmp, tmp, ar->as->lower[i]->value.integer); mpz_mul (tmp, tmp, delta); mpz_add (*offset, tmp, *offset); @@ -77,6 +107,8 @@ get_array_index (gfc_array_ref *ar, mpz_t *offset) } mpz_clear (delta); mpz_clear (tmp); + + return ok; } /* Find if there is a constructor which component is equal to COM. @@ -298,7 +330,10 @@ gfc_assign_data_value (gfc_expr *lvalue, gfc_expr *rvalue, mpz_t index, } if (ref->u.ar.type == AR_ELEMENT) - get_array_index (&ref->u.ar, &offset); + { + if (!get_array_index (&ref->u.ar, &offset)) + goto abort; + } else mpz_set (offset, index); diff --git a/gcc/testsuite/gfortran.dg/data_bounds_1.f90 b/gcc/testsuite/gfortran.dg/data_bounds_1.f90 index 24cdc7c9815..1e6321a2884 100644 --- a/gcc/testsuite/gfortran.dg/data_bounds_1.f90 +++ b/gcc/testsuite/gfortran.dg/data_bounds_1.f90 @@ -1,5 +1,5 @@ ! { dg-do compile } -! { dg-options "-std=gnu" } +! { dg-options "-std=gnu -w" } ! Checks the fix for PR32315, in which the bounds checks below were not being done. ! ! Contributed by Tobias Burnus diff --git a/gcc/testsuite/gfortran.dg/data_bounds_2.f90 b/gcc/testsuite/gfortran.dg/data_bounds_2.f90 new file mode 100644 index 000..1aa9fd4c423 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/data_bounds_2.f90 @@ -0,0 +1,9 @@ +! { dg-do compile } +! { dg-options "-std=f2018" } +! PR fortran/35095 - Improve bounds checking for DATA with implied-do + +program chkdata + character(len=2), dimension(2,2) :: str + data (str(i,1),i=1,3) / 'A','B','C' / ! { dg-error "a
[PATCH] Fortran: improve diagnostic message for COMMON with automatic object [PR32986]
Dear all, here's a simple patch for a very old PR that suggests a more helpful error message for an automatic object in a COMMON. The patch also suppresses the less helpful old error message after the new one has been emitted. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 829c0c06fe7ba2cf3e83508b95999b884b21236d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 23 Aug 2023 21:08:01 +0200 Subject: [PATCH] Fortran: improve diagnostic message for COMMON with automatic object [PR32986] gcc/fortran/ChangeLog: PR fortran/32986 * resolve.cc (is_non_constant_shape_array): Add forward declaration. (resolve_common_vars): Diagnose automatic array object in COMMON. (resolve_symbol): Prevent confusing follow-on error. gcc/testsuite/ChangeLog: PR fortran/32986 * gfortran.dg/common_28.f90: New test. --- gcc/fortran/resolve.cc | 15 ++- gcc/testsuite/gfortran.dg/common_28.f90 | 7 +++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/common_28.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index ce8261d646a..1042b8c18e8 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -959,6 +959,10 @@ cleanup: } +/* Forward declaration. */ +static bool is_non_constant_shape_array (gfc_symbol *sym); + + /* Resolve common variables. */ static void resolve_common_vars (gfc_common_head *common_block, bool named_common) @@ -1007,6 +1011,15 @@ resolve_common_vars (gfc_common_head *common_block, bool named_common) gfc_error_now ("%qs at %L cannot appear in COMMON " "[F2008:C5100]", csym->name, &csym->declared_at); + if (csym->attr.dimension && is_non_constant_shape_array (csym)) + { + gfc_error_now ("Automatic object %qs at %L cannot appear in " + "COMMON at %L", csym->name, &csym->declared_at, + &common_block->where); + /* Avoid confusing follow-on error. */ + csym->error = 1; + } + if (csym->ts.type != BT_DERIVED) continue; @@ -16612,7 +16625,7 @@ resolve_symbol (gfc_symbol *sym) /* Resolve array specifier. Check as well some constraints on COMMON blocks. */ - check_constant = sym->attr.in_common && !sym->attr.pointer; + check_constant = sym->attr.in_common && !sym->attr.pointer && !sym->error; /* Set the formal_arg_flag so that check_conflict will not throw an error for host associated variables in the specification diff --git a/gcc/testsuite/gfortran.dg/common_28.f90 b/gcc/testsuite/gfortran.dg/common_28.f90 new file mode 100644 index 000..9b583b9948d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/common_28.f90 @@ -0,0 +1,7 @@ +! { dg-do compile } +! PR fortran/32986 - Improve diagnostic message for COMMON with automatic object + +function a(n) + real :: x(n) ! { dg-error "Automatic object" } + common /c/ x ! { dg-error "cannot appear in COMMON" } +end function -- 2.35.3
Re: [PATCH] Fortran: implement vector sections in DATA statements [PR49588]
Hi Paul, Am 22.08.23 um 08:32 schrieb Paul Richard Thomas via Gcc-patches: Hi Harald, It all looks good to me and does indeed make the code clearer. OK for trunk. Thanks for the patch. thanks for the review! I was shocked to find that there are 217 older bugs than 49588. Does anybody test older bugs to check if any of them have been fixed? I am not aware of this being done systematically. At the same time, we have over 100 PRs marked as regression, with a few being fixed on mainline but not backported (or undecided whether to backport). Fixing and/or closing them might be low-hanging fruits. There are also far more than 100 TODOs in gcc/fortran/*.cc ... And with the usual PRs, there's enough work left for all kinds of contributions. Cheers, Harald Paul On Mon, 21 Aug 2023 at 20:48, Harald Anlauf via Fortran wrote: Dear all, the attached patch implements vector sections in DATA statements. The implementation is simpler than the size of the patch suggests, as part of changes try to clean up the existing code to make it easier to understand, as ordinary sections (start:end:stride) and vector sections may actually share some common code. The basisc idea of the implementation is that one needs a temporary vector that keeps track of the offsets into the array constructors for the indices in the array reference that are vectors. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald
[PATCH] Fortran: implement vector sections in DATA statements [PR49588]
Dear all, the attached patch implements vector sections in DATA statements. The implementation is simpler than the size of the patch suggests, as part of changes try to clean up the existing code to make it easier to understand, as ordinary sections (start:end:stride) and vector sections may actually share some common code. The basisc idea of the implementation is that one needs a temporary vector that keeps track of the offsets into the array constructors for the indices in the array reference that are vectors. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 96cc0333cdaa8459ef516ae8e74158cdb6302853 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 21 Aug 2023 21:23:57 +0200 Subject: [PATCH] Fortran: implement vector sections in DATA statements [PR49588] gcc/fortran/ChangeLog: PR fortran/49588 * data.cc (gfc_advance_section): Derive next index set and next offset into DATA variable also for array references using vector sections. Use auxiliary array to keep track of offsets into indexing vectors. (gfc_get_section_index): Set up initial indices also for DATA variables with array references using vector sections. * data.h (gfc_get_section_index): Adjust prototype. (gfc_advance_section): Likewise. * resolve.cc (check_data_variable): Pass vector offsets. gcc/testsuite/ChangeLog: PR fortran/49588 * gfortran.dg/data_vector_section.f90: New test. --- gcc/fortran/data.cc | 161 +++--- gcc/fortran/data.h| 4 +- gcc/fortran/resolve.cc| 5 +- .../gfortran.dg/data_vector_section.f90 | 26 +++ 4 files changed, 134 insertions(+), 62 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/data_vector_section.f90 diff --git a/gcc/fortran/data.cc b/gcc/fortran/data.cc index d29eb12c1b1..7c2537dd3f0 100644 --- a/gcc/fortran/data.cc +++ b/gcc/fortran/data.cc @@ -634,65 +634,102 @@ abort: void gfc_advance_section (mpz_t *section_index, gfc_array_ref *ar, - mpz_t *offset_ret) + mpz_t *offset_ret, int *vector_offset) { int i; mpz_t delta; mpz_t tmp; bool forwards; int cmp; - gfc_expr *start, *end, *stride; + gfc_expr *start, *end, *stride, *elem; + gfc_constructor_base base; for (i = 0; i < ar->dimen; i++) { - if (ar->dimen_type[i] != DIMEN_RANGE) - continue; + bool advance = false; - if (ar->stride[i]) + switch (ar->dimen_type[i]) { - stride = gfc_copy_expr(ar->stride[i]); - if(!gfc_simplify_expr(stride, 1)) - gfc_internal_error("Simplification error"); - mpz_add (section_index[i], section_index[i], - stride->value.integer); - if (mpz_cmp_si (stride->value.integer, 0) >= 0) - forwards = true; + case DIMEN_ELEMENT: + /* Loop to advance the next index. */ + advance = true; + break; + + case DIMEN_RANGE: + if (ar->stride[i]) + { + stride = gfc_copy_expr(ar->stride[i]); + if(!gfc_simplify_expr(stride, 1)) + gfc_internal_error("Simplification error"); + mpz_add (section_index[i], section_index[i], + stride->value.integer); + if (mpz_cmp_si (stride->value.integer, 0) >= 0) + forwards = true; + else + forwards = false; + gfc_free_expr(stride); + } else - forwards = false; - gfc_free_expr(stride); - } - else - { - mpz_add_ui (section_index[i], section_index[i], 1); - forwards = true; - } + { + mpz_add_ui (section_index[i], section_index[i], 1); + forwards = true; + } - if (ar->end[i]) -{ - end = gfc_copy_expr(ar->end[i]); - if(!gfc_simplify_expr(end, 1)) - gfc_internal_error("Simplification error"); - cmp = mpz_cmp (section_index[i], end->value.integer); - gfc_free_expr(end); - } - else - cmp = mpz_cmp (section_index[i], ar->as->upper[i]->value.integer); + if (ar->end[i]) + { + end = gfc_copy_expr(ar->end[i]); + if(!gfc_simplify_expr(end, 1)) + gfc_internal_error("Simplification error"); + cmp = mpz_cmp (section_index[i], end->value.integer); + gfc_free_expr(end); + } + else + cmp = mpz_cmp (section_index[i], ar->as->upper[i]->value.integer); - if ((cmp > 0 && forwards) || (cmp < 0 && !forwards)) - { - /* Reset index to start, then loop to advance the next index. */ - if (ar->start[i]) + if ((cmp > 0 && forwards) || (cmp < 0 && !forwards)) { - start = gfc_copy_expr(ar->start[i]); - if(!gfc_simplify_expr(start, 1)) - gfc_internal_error("Simplification error"); + /* Reset index to start, then loop to advance the next index. */ + if (ar->start[i]) + { + start = gfc_copy_expr(ar->start[i]); + if(!gfc_simplify_expr(start, 1)) + gfc_internal_error("Simplification error"); + mpz_set (section_index[i], start->value.integer); + gfc_free_expr(start); + } + else + mpz_set (section_index[i], ar->as->lower[i]->value.integer); + advance = true; + } +
[PATCH,committed] Fortran: fix memleak for character,value dummy of bind(c) procedure [PR110360]
Dear all, the attached simple patch fixes a memleak in the frontend when a character literal is passed to a character,value dummy of a bind(c) procedure, by relying on gfc_replace_expr to do the cleanup. (This can be tested e.g. with gfortran.dg/bind_c_usage_13.f03 and running f951 under valgrind). The patch was OK'ed in the PR by Mikael. Pushed as r14-3254-g9ade70bb86c874 after partial regtesting on x86_64-pc-linux-gnu. Thanks, Harald From 9ade70bb86c8744f4416a48bb69cf4705f00905a Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 16 Aug 2023 22:00:49 +0200 Subject: [PATCH] Fortran: fix memleak for character,value dummy of bind(c) procedure [PR110360] Testcase gfortran.dg/bind_c_usage_13.f03 exhibited a memleak in the frontend occuring when passing a character literal to a character,value dummy of a bind(c) procedure, due to a missing cleanup in the conversion of the actual argument expression. Reduced testcase: program p interface subroutine val_c (c) bind(c) use iso_c_binding, only: c_char character(len=1,kind=c_char), value :: c end subroutine val_c end interface call val_c ("A") end gcc/fortran/ChangeLog: PR fortran/110360 * trans-expr.cc (conv_scalar_char_value): Use gfc_replace_expr to avoid leaking replaced gfc_expr. --- gcc/fortran/trans-expr.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 52cd88f5b00..6e9e76cd5c9 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -4044,8 +4044,9 @@ conv_scalar_char_value (gfc_symbol *sym, gfc_se *se, gfc_expr **expr) gfc_typespec ts; gfc_clear_ts (&ts); - *expr = gfc_get_int_expr (gfc_default_character_kind, NULL, -(*expr)->value.character.string[0]); + gfc_expr *tmp = gfc_get_int_expr (gfc_default_character_kind, NULL, + (*expr)->value.character.string[0]); + gfc_replace_expr (*expr, tmp); } else if (se != NULL && (*expr)->expr_type == EXPR_VARIABLE) { -- 2.35.3
Re: [PATCH] Fortran: Avoid accessing gfc_charlen when not looking at BT_CHARACTER (PR 110677)
Hi Martin, Am 14.08.23 um 19:39 schrieb Martin Jambor: Hello, this patch addresses an issue uncovered by the undefined behavior sanitizer. In function resolve_structure_cons in resolve.cc there is a test starting with: if (cons->expr->ts.type == BT_CHARACTER && comp->ts.u.cl && comp->ts.u.cl->length && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT and UBSAN complained of loads from comp->ts.u.cl->length->expr_type of integer value 1818451807 which is outside of the value range expr_t enum. If I understand the code correctly it the entire load was unwanted because comp->ts.type in those cases is BT_CLASS and not BT_CHARACTER. This patch simply adds a check to make sure it is only accessed in those cases. I have verified that the UPBSAN failure goes away with this patch, it also passes bootstrap and testing on x86_64-linux. OK for master? this looks good to me. Looking at that code block, there is a potential other UB a few lines below, where (hopefully integer) string lengths are to be passed to mpz_cmp. If the string length is ill-defined (e.g. non-integer), value.integer is undefined. We've seen this elsewhere, where on BE platforms that undefined value was interpreted as some large integer and giving failures on those platforms. One could similarly add the following checks here (on top of your patch): diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index e7c8d919bef..43095406c16 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -1401,6 +1401,8 @@ resolve_structure_cons (gfc_expr *expr, int init) && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT && cons->expr->ts.u.cl && cons->expr->ts.u.cl->length && cons->expr->ts.u.cl->length->expr_type == EXPR_CONSTANT + && cons->expr->ts.u.cl->length->ts.type == BT_INTEGER + && comp->ts.u.cl->length->ts.type == BT_INTEGER && mpz_cmp (cons->expr->ts.u.cl->length->value.integer, comp->ts.u.cl->length->value.integer) != 0) { It is up to you whether you want to add this. Thanks for the patch! Harald Thanks, Martin gcc/fortran/ChangeLog: 2023-08-14 Martin Jambor PR fortran/110677 * resolve.cc (resolve_structure_cons): Check comp->ts is character type before accessing stuff through comp->ts.u.cl. --- gcc/fortran/resolve.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index e7c8d919bef..5b4dfc5fcd2 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -1396,8 +1396,9 @@ resolve_structure_cons (gfc_expr *expr, int init) the one of the structure, ensure this if the lengths are known at compile time and when we are dealing with PARAMETER or structure constructors. */ - if (cons->expr->ts.type == BT_CHARACTER && comp->ts.u.cl - && comp->ts.u.cl->length + if (cons->expr->ts.type == BT_CHARACTER + && comp->ts.type == BT_CHARACTER + && comp->ts.u.cl && comp->ts.u.cl->length && comp->ts.u.cl->length->expr_type == EXPR_CONSTANT && cons->expr->ts.u.cl && cons->expr->ts.u.cl->length && cons->expr->ts.u.cl->length->expr_type == EXPR_CONSTANT
Re: [PATCH 0/3] fortran: fix length one character dummy args [PR110419]
Hi Mikael, Am 09.08.23 um 22:21 schrieb Mikael Morin via Gcc-patches: Hello, I propose with this patchset a fix for the test value_9.f90 which has been failing on 32 bits powerpc since it was added a few weeks back (see PR110360 and PR110419). The problem is an argument type mismatch between a procedure declaration, and the argument value for a call of that same procedure, in the specific case of length one character dummy arguments with the value attribute. Admittedly, our argument passing conventions [1] for those are currently unspecified. Before PR110360, character dummy arguments with value attribute were arrays passed by value, but the actual argument was still passed as reference. PR110360 changed that to pass length one dummies as bare character (i.e. scalar integer), like in the bind(c) case (but with length argument still present). However, the argument type in the function declaration wasn't changed at the same time, so the test was failing on big-endian 32 bits targets. Surprisingly, on most targets the middle-end, back-end and runtime are happy to get a scalar value passed where a length one array is expected. This can be fixed, either by reverting back to arguments represented as arrays passed by value with calls fixed, or by keeping the new representation with single characters for arguments and fixing the procedure types accordingly. I haven't really tried the first way, this is using the second one. The first patch is a preliminary refactoring. The main change is the second patch. It modifies the types of length one character dummy argsuments with value attribute in the procedure declarations, so that they are scalar integer types, consistently with how arguments are passed for calls. The third patch is a change of error codes in the testcase. I have regression tested this on x86_64-unknown-linux-gnu, and powerpc64-unknown-linux-gnu (both -m32 and -m64). OK for master? this looks good to me. There was only one thing I was uncertain what the right way is: you chose to use mpz_cmp_ui in the length check in the new helper function gfc_length_one_character_type_p, while in many other places the length check uses mpz_cmp_si. Admittedly, a negative (effective/declared) character length can never occur, except maybe at intermediate times during resolution before this is fixed up in accordance with the standard. So this is probably more a cosmetic decision, and you can decide to use either variant. Thanks for the patch! Harald [1] https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html Mikael Morin (3): fortran: New predicate gfc_length_one_character_type_p fortran: Fix length one character dummy arg type [PR110419] testsuite: Use distinct explicit error codes in value_9.f90 gcc/fortran/check.cc | 7 +- gcc/fortran/decl.cc | 4 +- gcc/fortran/gfortran.h| 15 +++ gcc/fortran/trans-expr.cc | 39 --- gcc/fortran/trans-types.cc| 5 +- gcc/testsuite/gfortran.dg/bind_c_usage_13.f03 | 8 +- gcc/testsuite/gfortran.dg/value_9.f90 | 108 +- 7 files changed, 103 insertions(+), 83 deletions(-)
[PATCH] Fortran: do not pass hidden character length for TYPE(*) dummy [PR110825]
Dear all, when passing a character actual argument to an assumed-type dummy (TYPE(*)), we should not pass the character length for that argument, as otherwise other hidden arguments that are passed as part of the gfortran ABI will not be interpreted correctly. This is in line with the current way the procedure decl is generated. The attached patch fixes the caller and clarifies the behavior in the documentation. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 199e09c9862f5afe7e583839bc1b108c741a7efb Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 27 Jul 2023 21:30:26 +0200 Subject: [PATCH] Fortran: do not pass hidden character length for TYPE(*) dummy [PR110825] gcc/fortran/ChangeLog: PR fortran/110825 * gfortran.texi: Clarify argument passing convention. * trans-expr.cc (gfc_conv_procedure_call): Do not pass the character length as hidden argument when the declared dummy argument is assumed-type. gcc/testsuite/ChangeLog: PR fortran/110825 * gfortran.dg/assumed_type_18.f90: New test. --- gcc/fortran/gfortran.texi | 3 +- gcc/fortran/trans-expr.cc | 1 + gcc/testsuite/gfortran.dg/assumed_type_18.f90 | 52 +++ 3 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/assumed_type_18.f90 diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 7786d23265f..f476a3719f5 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -3750,7 +3750,8 @@ front ends of GCC, e.g. to GCC's C99 compiler for @code{_Bool} or GCC's Ada compiler for @code{Boolean}.) For arguments of @code{CHARACTER} type, the character length is passed -as a hidden argument at the end of the argument list. For +as a hidden argument at the end of the argument list, except when the +corresponding dummy argument is declared as @code{TYPE(*)}. For deferred-length strings, the value is passed by reference, otherwise by value. The character length has the C type @code{size_t} (or @code{INTEGER(kind=C_SIZE_T)} in Fortran). Note that this is diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index ef3e6d08f78..764565476af 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -7521,6 +7521,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && !(fsym && fsym->ts.type == BT_DERIVED && fsym->ts.u.derived && fsym->ts.u.derived->intmod_sym_id == ISOCBINDING_PTR && fsym->ts.u.derived->from_intmod == INTMOD_ISO_C_BINDING ) + && !(fsym && fsym->ts.type == BT_ASSUMED) && !(fsym && UNLIMITED_POLY (fsym))) vec_safe_push (stringargs, parmse.string_length); diff --git a/gcc/testsuite/gfortran.dg/assumed_type_18.f90 b/gcc/testsuite/gfortran.dg/assumed_type_18.f90 new file mode 100644 index 000..a3d791919a2 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/assumed_type_18.f90 @@ -0,0 +1,52 @@ +! { dg-do run } +! PR fortran/110825 - TYPE(*) and character actual arguments + +program foo + use iso_c_binding, only: c_loc, c_ptr, c_associated + implicit none + character(100):: not_used = "" + character(:), allocatable :: deferred + character :: c42(6,7) = "*" + call sub (not_used, "123") + call sub ("0" , "123") + deferred = "d" + call sub (deferred , "123") + call sub2 ([1.0,2.0], "123") + call sub2 (["1","2"], "123") + call sub3 (c42 , "123") + +contains + + subroutine sub (useless_var, print_this) +type(*), intent(in) :: useless_var +character(*), intent(in) :: print_this +if (len (print_this) /= 3) stop 1 +if (len_trim (print_this) /= 3) stop 2 + end + + subroutine sub2 (a, c) +type(*), intent(in) :: a(:) +character(*), intent(in) :: c +if (len (c) /= 3) stop 10 +if (len_trim (c) /= 3) stop 11 +if (size (a) /= 2) stop 12 + end + + subroutine sub3 (a, c) +type(*), intent(in), target, optional :: a(..) +character(*), intent(in) :: c +type(c_ptr) :: cpt +if (len (c) /= 3) stop 20 +if (len_trim (c) /= 3) stop 21 +if (.not. present (a)) stop 22 +if (rank (a) /= 2) stop 23 +if (size (a)/= 42) stop 24 +if (any (shape (a) /= [6,7])) stop 25 +if (any (lbound (a) /= [1,1])) stop 26 +if (any (ubound (a) /= [6,7])) stop 27 +if (.not. is_contiguous (a)) stop 28 +cpt = c_loc (a) +if (.not. c_associated (cpt)) stop 29 + end + +end -- 2.35.3
[PATCH, v3] Fortran: diagnose strings of non-constant length in DATA statements [PR68569]
I am going to get the brown bag for today... This is now the right corrected patch. Sorry for all the noise! Harald Am 26.07.23 um 21:17 schrieb Harald Anlauf via Gcc-patches: Dear all, the original submission missed the adjustments of the expected patterns of 2 tests. This is corrected in the new attachments. Am 26.07.23 um 21:10 schrieb Harald Anlauf via Gcc-patches: Dear all, the attached patch fixes an ICE-on-invalid after use of strings of non-constant length in DATA statements. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Thanks, Harald From d872b8ffc121fd57d47aa7d3d12d9ba86389f092 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 26 Jul 2023 21:12:45 +0200 Subject: [PATCH] Fortran: diagnose strings of non-constant length in DATA statements [PR68569] gcc/fortran/ChangeLog: PR fortran/68569 * resolve.cc (check_data_variable): Do not accept strings with deferred length or non-constant length in a DATA statement. Reject also substrings of string variables of non-constant length. gcc/testsuite/ChangeLog: PR fortran/68569 * gfortran.dg/data_char_4.f90: Adjust expected diagnostic. * gfortran.dg/data_char_5.f90: Likewise. * gfortran.dg/data_char_6.f90: New test. --- gcc/fortran/resolve.cc| 22 ++- gcc/testsuite/gfortran.dg/data_char_4.f90 | 2 +- gcc/testsuite/gfortran.dg/data_char_5.f90 | 8 +++ gcc/testsuite/gfortran.dg/data_char_6.f90 | 26 +++ 4 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/data_char_6.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index f7cfdfc133f..3cd470ddcca 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -16771,7 +16771,6 @@ check_data_variable (gfc_data_variable *var, locus *where) return false; ar = NULL; - mpz_init_set_si (offset, 0); e = var->expr; if (e->expr_type == EXPR_FUNCTION && e->value.function.isym @@ -16838,8 +16837,24 @@ check_data_variable (gfc_data_variable *var, locus *where) "attribute", ref->u.c.component->name, &e->where); return false; } + + /* Reject substrings of strings of non-constant length. */ + if (ref->type == REF_SUBSTRING + && ref->u.ss.length + && ref->u.ss.length->length + && !gfc_is_constant_expr (ref->u.ss.length->length)) + goto bad_charlen; } + /* Reject strings with deferred length or non-constant length. */ + if (e->ts.type == BT_CHARACTER + && (e->ts.deferred + || (e->ts.u.cl->length + && !gfc_is_constant_expr (e->ts.u.cl->length +goto bad_charlen; + + mpz_init_set_si (offset, 0); + if (e->rank == 0 || has_pointer) { mpz_init_set_ui (size, 1); @@ -16967,6 +16982,11 @@ check_data_variable (gfc_data_variable *var, locus *where) mpz_clear (offset); return t; + +bad_charlen: + gfc_error ("Non-constant character length at %L in DATA statement", + &e->where); + return false; } diff --git a/gcc/testsuite/gfortran.dg/data_char_4.f90 b/gcc/testsuite/gfortran.dg/data_char_4.f90 index ed0782ce8a0..fa5e0a0134a 100644 --- a/gcc/testsuite/gfortran.dg/data_char_4.f90 +++ b/gcc/testsuite/gfortran.dg/data_char_4.f90 @@ -4,7 +4,7 @@ program p character(l) :: c(2) ! { dg-error "must have constant character length" } - data c /'a', 'b'/ + data c /'a', 'b'/! { dg-error "Non-constant character length" } common c end diff --git a/gcc/testsuite/gfortran.dg/data_char_5.f90 b/gcc/testsuite/gfortran.dg/data_char_5.f90 index ea26687e3d5..7556e63c01b 100644 --- a/gcc/testsuite/gfortran.dg/data_char_5.f90 +++ b/gcc/testsuite/gfortran.dg/data_char_5.f90 @@ -4,12 +4,12 @@ subroutine sub () integer :: ll = 4 block -character(ll) :: c(2) ! { dg-error "non-constant" } -data c /'a', 'b'/ +character(ll) :: c(2) +data c /'a', 'b'/ ! { dg-error "Non-constant character length" } end block contains subroutine sub1 () -character(ll) :: d(2) ! { dg-error "non-constant" } -data d /'a', 'b'/ +character(ll) :: d(2) +data d /'a', 'b'/ ! { dg-error "Non-constant character length" } end subroutine sub1 end subroutine sub diff --git a/gcc/testsuite/gfortran.dg/data_char_6.f90 b/gcc/testsuite/gfortran.dg/data_char_6.f90 new file mode 100644 index 000..4e32c647d4d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/data_char_6.f90 @@ -0,0 +1,26 @@ +! { dg-do compile } +! PR fortran/68569 - ICE with automatic character object and DATA +! Contributed by G. Steinmetz + +subroutine s1 (n) + implicit none + integer, intent(in) :: n + character(n) :: x + data x /'a'/
[PATCH, v2] Fortran: diagnose strings of non-constant length in DATA statements [PR68569]
Dear all, the original submission missed the adjustments of the expected patterns of 2 tests. This is corrected in the new attachments. Am 26.07.23 um 21:10 schrieb Harald Anlauf via Gcc-patches: Dear all, the attached patch fixes an ICE-on-invalid after use of strings of non-constant length in DATA statements. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Thanks, Harald diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index f7cfdfc133f..cd8e223edce 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -16771,7 +16787,6 @@ check_data_variable (gfc_data_variable *var, locus *where) return false; ar = NULL; - mpz_init_set_si (offset, 0); e = var->expr; if (e->expr_type == EXPR_FUNCTION && e->value.function.isym @@ -16838,8 +16853,24 @@ check_data_variable (gfc_data_variable *var, locus *where) "attribute", ref->u.c.component->name, &e->where); return false; } + + /* Reject substrings of strings of non-constant length. */ + if (ref->type == REF_SUBSTRING + && ref->u.ss.length + && ref->u.ss.length->length + && !gfc_is_constant_expr (ref->u.ss.length->length)) + goto bad_charlen; } + /* Reject deferred length character and strings of non-constant length. */ + if (e->ts.type == BT_CHARACTER + && (e->ts.deferred + || (e->ts.u.cl->length + && !gfc_is_constant_expr (e->ts.u.cl->length +goto bad_charlen; + + mpz_init_set_si (offset, 0); + if (e->rank == 0 || has_pointer) { mpz_init_set_ui (size, 1); @@ -16967,6 +16998,11 @@ check_data_variable (gfc_data_variable *var, locus *where) mpz_clear (offset); return t; + +bad_charlen: + gfc_error ("Non-constant character length at %L in DATA statement", + &e->where); + return false; }
[PATCH] Fortran: diagnose strings of non-constant length in DATA statements [PR68569]
Dear all, the attached patch fixes an ICE-on-invalid after use of strings of non-constant length in DATA statements. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From b5b13db48c169ef18a8b75739bd4f22f9fd5654e Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 26 Jul 2023 20:46:50 +0200 Subject: [PATCH] Fortran: diagnose strings of non-constant length in DATA statements [PR68569] gcc/fortran/ChangeLog: PR fortran/68569 * resolve.cc (check_data_variable): Do not accept strings with deferred length or non-constant length in a DATA statement. Reject also substrings of string variables of non-constant length. gcc/testsuite/ChangeLog: PR fortran/68569 * gfortran.dg/data_char_6.f90: New test. --- gcc/fortran/resolve.cc| 22 ++- gcc/testsuite/gfortran.dg/data_char_6.f90 | 26 +++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/data_char_6.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index f7cfdfc133f..3cd470ddcca 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -16771,7 +16771,6 @@ check_data_variable (gfc_data_variable *var, locus *where) return false; ar = NULL; - mpz_init_set_si (offset, 0); e = var->expr; if (e->expr_type == EXPR_FUNCTION && e->value.function.isym @@ -16838,8 +16837,24 @@ check_data_variable (gfc_data_variable *var, locus *where) "attribute", ref->u.c.component->name, &e->where); return false; } + + /* Reject substrings of strings of non-constant length. */ + if (ref->type == REF_SUBSTRING + && ref->u.ss.length + && ref->u.ss.length->length + && !gfc_is_constant_expr (ref->u.ss.length->length)) + goto bad_charlen; } + /* Reject strings with deferred length or non-constant length. */ + if (e->ts.type == BT_CHARACTER + && (e->ts.deferred + || (e->ts.u.cl->length + && !gfc_is_constant_expr (e->ts.u.cl->length +goto bad_charlen; + + mpz_init_set_si (offset, 0); + if (e->rank == 0 || has_pointer) { mpz_init_set_ui (size, 1); @@ -16967,6 +16982,11 @@ check_data_variable (gfc_data_variable *var, locus *where) mpz_clear (offset); return t; + +bad_charlen: + gfc_error ("Non-constant character length at %L in DATA statement", + &e->where); + return false; } diff --git a/gcc/testsuite/gfortran.dg/data_char_6.f90 b/gcc/testsuite/gfortran.dg/data_char_6.f90 new file mode 100644 index 000..4e32c647d4d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/data_char_6.f90 @@ -0,0 +1,26 @@ +! { dg-do compile } +! PR fortran/68569 - ICE with automatic character object and DATA +! Contributed by G. Steinmetz + +subroutine s1 (n) + implicit none + integer, intent(in) :: n + character(n) :: x + data x /'a'/ ! { dg-error "Non-constant character length" } +end + +subroutine s2 (n) + implicit none + integer, intent(in) :: n + character(n) :: x + data x(1:1) /'a'/! { dg-error "Non-constant character length" } +end + +subroutine s3 () + implicit none + type t + character(:) :: c ! { dg-error "must be a POINTER or ALLOCATABLE" } + end type t + type(t) :: tp + data tp%c /'a'/ ! { dg-error "Non-constant character length" } +end -- 2.35.3
[PATCH] Fortran: intrinsics and deferred-length character arguments [PR95947,PR110658]
Dear all, some intrinsics may return character results with the same characteristics as their first argument (e.g. PACK, MINVAL, ...). If the first argument is of deferred-length, we need to derive the character length of the result from the first argument, like in the assumed-length case, but we must not handle it as deferred-length, as that has a different argument passing convention. The attached - almost trivial and obvious - patch fixes that. Regtested on x86_64-pc-linux-gnu. OK for mainline? As this is a rather simple fix for a wrong-code bug, I would like to backport this at least to 13-branch, unless there are major concerns. Thanks, Harald From 88d2694eb1278b0ad0d542565e0542c39fe6b466 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 16 Jul 2023 22:17:27 +0200 Subject: [PATCH] Fortran: intrinsics and deferred-length character arguments [PR95947,PR110658] gcc/fortran/ChangeLog: PR fortran/95947 PR fortran/110658 * trans-expr.cc (gfc_conv_procedure_call): For intrinsic procedures whose result characteristics depends on the first argument and which can be of type character, the character length will not be deferred. gcc/testsuite/ChangeLog: PR fortran/95947 PR fortran/110658 * gfortran.dg/deferred_character_37.f90: New test. --- gcc/fortran/trans-expr.cc | 7 +- .../gfortran.dg/deferred_character_37.f90 | 88 +++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/deferred_character_37.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index dbb04f8c434..d1570b31a82 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -7654,7 +7654,12 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, (and other intrinsics?) and dummy functions. In the case of SPREAD, we take the character length of the first argument for the result. For dummies, we have to look through the formal argument list for - this function and use the character length found there.*/ + this function and use the character length found there. + Likewise, we handle the case of deferred-length character dummy + arguments to intrinsics that determine the characteristics of + the result, which cannot be deferred-length. */ + if (expr->value.function.isym) + ts.deferred = false; if (ts.deferred) cl.backend_decl = gfc_create_var (gfc_charlen_type_node, "slen"); else if (!sym->attr.dummy) diff --git a/gcc/testsuite/gfortran.dg/deferred_character_37.f90 b/gcc/testsuite/gfortran.dg/deferred_character_37.f90 new file mode 100644 index 000..8a5a8c5daf8 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/deferred_character_37.f90 @@ -0,0 +1,88 @@ +! { dg-do run } +! PR fortran/95947 +! PR fortran/110658 +! +! Test deferred-length character arguments to selected intrinsics +! that may return a character result of same length as first argument: +! CSHIFT, EOSHIFT, MAXVAL, MERGE, MINVAL, PACK, SPREAD, TRANSPOSE, UNPACK + +program p + implicit none + call pr95947 () + call pr110658 () + call s () + +contains + + subroutine pr95947 +character(len=:), allocatable :: m(:) + +m = [ character(len=10) :: 'ape','bat','cat','dog','eel','fly','gnu'] +m = pack (m, mask=(m(:)(2:2) == 'a')) + +! print *, "m = '", m,"' ", "; expected is ['bat','cat']" +if (.not. all (m == ['bat','cat'])) stop 1 + +! print *, "size(m) = ", size(m), "; expected is 2" +if (size (m) /= 2) stop 2 + +! print *, "len(m) = ", len(m), "; expected is 10" +if (len (m) /= 10) stop 3 + +! print *, "len_trim(m) = ", len_trim(m), "; expected is 3 3" +if (.not. all (len_trim(m) == [3,3])) stop 4 + end + + subroutine pr110658 +character(len=:), allocatable :: array(:), array2(:,:) +character(len=:), allocatable :: res, res1(:), res2(:) + +array = ["bb", "aa", "cc"] + +res = minval (array) +if (res /= "aa") stop 11 + +res = maxval (array, mask=[.true.,.true.,.false.]) +if (res /= "bb") stop 12 + +res1 = cshift (array, 1) +if (any (res1 /= ["aa","cc","bb"])) stop 13 + +res2 = eoshift (res1, -1) +if (any (res2 /= [" ", "aa", "cc"])) stop 14 + +res2 = pack (array, mask=[.true.,.false.,.true.]) +if (any (res2 /= ["bb","cc"])) stop 15 + +res2 = unpack (res2, mask=[.true.,.false.,.true.], field="aa") +if (any (res2 /= array)) stop 16 + +res2 = merge (res2, array, [.true.,.false.,.true.]) +if (any (res2 /= array)) stop 17 + +array2 = spread (array, dim=2, ncopies=2) +array2 = transpose (array2) +if (any (shape (array2) /= [2,3])) stop 18 +if (any (array2(2,:) /= array))stop 19 + end + + subroutine s +character(:), allocatable :: array1(:), array2(:) +array1 = ["aa","cc","bb"] +array2 = copy (array1) +if (any (array1 /= array2)) stop 20 + end + + function copy (arg) result (res) +character(:), allocatabl
Re: [PATCH 0/3] Fix argument evaluation order [PR92178]
Hi Mikael, Am 11.07.23 um 12:32 schrieb Mikael Morin via Gcc-patches: Hello, this is a followup to Harald's recent work [1] on the evaluation order of arguments, when one of them is passed to an intent(out) allocatable dummy and is deallocated before the call. This extends Harald's fix to support: - scalars passed to assumed rank dummies (patch 1), - scalars passed to assumed rank dummies with the data reference depending on its own content (patch 2), - arrays with the data reference depending on its own content (patch 3). There is one (last?) case which is not supported, for which I have opened a separate PR [2]. Regression tested on x86_64-pc-linux-gnu. OK for master? this is an impressive improvement for the CLASS case. Maybe Paul wants to have another look at it, but it is OK from my side. Thanks for the patch! Harald [1] https://gcc.gnu.org/pipermail/fortran/2023-July/059562.html [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110618 Mikael Morin (3): fortran: defer class wrapper initialization after deallocation [PR92178] fortran: Factor data references for scalar class argument wrapping [PR92178] fortran: Reorder array argument evaluation parts [PR92178] gcc/fortran/trans-array.cc | 3 + gcc/fortran/trans-expr.cc | 130 +--- gcc/fortran/trans.cc| 28 + gcc/fortran/trans.h | 8 +- gcc/testsuite/gfortran.dg/intent_out_19.f90 | 22 gcc/testsuite/gfortran.dg/intent_out_20.f90 | 33 + gcc/testsuite/gfortran.dg/intent_out_21.f90 | 33 + 7 files changed, 236 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/intent_out_19.f90 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_20.f90 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_21.f90
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Andre, I forgot to answer your other question: Am 11.07.23 um 18:23 schrieb Andre Vehreschild via Gcc-patches: I tried to use a pdt within a derived type as a component. Is that not allowed by the standard? I know, I could hunt in the standard for it, but when someone knows out of his head, he could greatly help me out. You mean something like the following is rejected with a strange error: type pdt(n) integer, len :: n = 8 character(len=n) :: c end type pdt type t type(pdt(42)) :: q end type t type(t) :: u end pr102003.f90:1:10: 1 | type pdt(n) | 1 Error: Cannot convert TYPE(Pdtpdt) to TYPE(pdt) at (1) ISTR that there is an existing PR...
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Andre, this looks much better now! This looks mostly good to me, except for a typo in the testcase: + if (p% ci% len /= 42) stop 4 There is no component "ci", only "c". The testsuite would fail. Regarding the memleak: replacing // TODO: Fix leaking expr tmp, when simplify is done twice. tmp = gfc_copy_expr (*newp); by if (inquiry->next) { gfc_free_expr (tmp); tmp = gfc_copy_expr (*newp); } or rather if (inquiry->next) gfc_replace_expr (tmp, *newp); at least shrinks the leak a bit. (Untested otherwise). OK with one of the above changes (provided it survives regtesting). Thanks for the patch! Harald Am 11.07.23 um 18:23 schrieb Andre Vehreschild via Gcc-patches: Hi Harald, attached is a new version of the patch. This now also respects inquiry-LEN. Btw, there is a potential memory leak in the simplify for inquiry functions. I have added a note into the code. I tried to use a pdt within a derived type as a component. Is that not allowed by the standard? I know, I could hunt in the standard for it, but when someone knows out of his head, he could greatly help me out. Regtests ok on x86_64-linux-gnu/F37. Regards, Andre On Mon, 10 Jul 2023 20:55:29 +0200 Harald Anlauf wrote: Hi Andre, thanks for looking into this! While it fixes the original PR, here is a minor extension of the testcase that ICEs here with your patch: program pr102003 type pdt(n) integer, len :: n = 8 character(len=n) :: c end type pdt type(pdt(42)) :: p integer, parameter :: m = len (p% c) integer, parameter :: n = p% c% len if (m /= 42) stop 1 if (len (p% c) /= 42) stop 2 print *, p% c% len ! OK if (p% c% len /= 42) stop 3 ! OK print *, n ! ICE end I get: pdt_33.f03:14:27: 14 | integer, parameter :: n = p% c% len | 1 Error: non-constant initialization expression at (1) pdt_33.f03:20:31: 20 | print *, n ! ICE | 1 internal compiler error: tree check: expected record_type or union_type or qual_union_type, have integer_type in gfc_conv_component_ref, at fortran/trans-expr.cc:2757 0x84286c tree_check_failed(tree_node const*, char const*, int, char const*, ...) ../../gcc-trunk/gcc/tree.cc:8899 0xa6d6fb tree_check3(tree_node*, char const*, int, char const*, tree_code, tree_code, tree_code) ../../gcc-trunk/gcc/tree.h:3617 0xa90847 gfc_conv_component_ref(gfc_se*, gfc_ref*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:2757 0xa91bbc gfc_conv_variable ../../gcc-trunk/gcc/fortran/trans-expr.cc:3137 0xaa8e9c gfc_conv_expr(gfc_se*, gfc_expr*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:9594 0xaa92ae gfc_conv_expr_reference(gfc_se*, gfc_expr*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:9713 0xad67f6 gfc_trans_transfer(gfc_code*) ../../gcc-trunk/gcc/fortran/trans-io.cc:2607 0xa43cb7 trans_code ../../gcc-trunk/gcc/fortran/trans.cc:2449 0xad37c6 build_dt ../../gcc-trunk/gcc/fortran/trans-io.cc:2051 0xa43cd7 trans_code ../../gcc-trunk/gcc/fortran/trans.cc:2421 0xa84711 gfc_generate_function_code(gfc_namespace*) ../../gcc-trunk/gcc/fortran/trans-decl.cc:7762 0x9d9ca7 translate_all_program_units ../../gcc-trunk/gcc/fortran/parse.cc:6929 0x9d9ca7 gfc_parse_file() ../../gcc-trunk/gcc/fortran/parse.cc:7235 0xa40a1f gfc_be_parse_file ../../gcc-trunk/gcc/fortran/f95-lang.cc:229 The fortran-dump confirms that n is not simplified to a constant. So while you're at it, do you also see a solution to this variant? Harald Am 10.07.23 um 17:48 schrieb Andre Vehreschild via Gcc-patches: Hi all, while browsing the pdt meta-bug I came across 102003 and thought to myself: Well, that one is easy. How foolish of me... Anyway, the solution attached prevents a pdt_len (or pdt_kind) expression in a function call (e.g. len() or kind()) to mark the whole expression as a pdt one. The second part of the patch in simplify.cc then takes care of either generating the correct component ref or when a constant expression (i.e. gfc_init_expr_flag is set) is required to look this up from the actual symbol (not from the type, because there the default value is stored). Regtested ok on x86_64-linux-gnu/Fedora 37. Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de -- Andre Vehreschild * Email: vehre ad gmx dot de
[PATCH] Fortran: formal symbol attributes for intrinsic procedures [PR110288]
Dear all, for intrinsic procedures we derive the typespec of the formal symbol attributes from the actual arguments. This can have an undesired effect for character actual arguments, as the argument passing conventions differ for deferred-length (length is passed by reference) and otherwise (length is passed by value). The testcase in the PR nicely demonstrates the issue for FINDLOC(array,value,...), when either array or value are deferred-length. We therefore need take care that we do not copy ts.deferred, but rather set it to false if the formal argument is neither allocatable or pointer. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is actually a 11/12/13/14 regression (and I found a potential "culprit" in 11-development that touched the call chain in question), so the patch might finally need backporting as far as seems reasonable. Thanks, Harald From 3b2c523ae31b68fc3b8363b458a55eec53a44365 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 11 Jul 2023 21:21:25 +0200 Subject: [PATCH] Fortran: formal symbol attributes for intrinsic procedures [PR110288] gcc/fortran/ChangeLog: PR fortran/110288 * symbol.cc (gfc_copy_formal_args_intr): When deriving the formal argument attributes from the actual ones for intrinsic procedure calls, take special care of CHARACTER arguments that we do not wrongly treat them formally as deferred-length. gcc/testsuite/ChangeLog: PR fortran/110288 * gfortran.dg/findloc_10.f90: New test. --- gcc/fortran/symbol.cc| 7 +++ gcc/testsuite/gfortran.dg/findloc_10.f90 | 13 + 2 files changed, 20 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/findloc_10.f90 diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index 37a9e8fa0ae..90023f0ad73 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc @@ -4725,6 +4725,13 @@ gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src, formal_arg->sym->attr.flavor = FL_VARIABLE; formal_arg->sym->attr.dummy = 1; + /* Do not treat an actual deferred-length character argument wrongly + as template for the formal argument. */ + if (formal_arg->sym->ts.type == BT_CHARACTER + && !(formal_arg->sym->attr.allocatable + || formal_arg->sym->attr.pointer)) + formal_arg->sym->ts.deferred = false; + if (formal_arg->sym->ts.type == BT_CHARACTER) formal_arg->sym->ts.u.cl = gfc_new_charlen (gfc_current_ns, NULL); diff --git a/gcc/testsuite/gfortran.dg/findloc_10.f90 b/gcc/testsuite/gfortran.dg/findloc_10.f90 new file mode 100644 index 000..4d5ecd2306a --- /dev/null +++ b/gcc/testsuite/gfortran.dg/findloc_10.f90 @@ -0,0 +1,13 @@ +! { dg-do run } +! { dg-options "-fdump-tree-original" } +! PR fortran/110288 - FINDLOC and deferred-length character arguments + +program test + character(len=:), allocatable :: array(:) + character(len=:), allocatable :: value + array = ["bb", "aa"] + value = "aa" + if (findloc (array, value, dim=1) /= 2) stop 1 +end program test + +! { dg-final { scan-tree-dump "_gfortran_findloc2_s1 \\(.*, \\.array, \\.value\\)" "original" } } -- 2.35.3
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Andre, thanks for looking into this! While it fixes the original PR, here is a minor extension of the testcase that ICEs here with your patch: program pr102003 type pdt(n) integer, len :: n = 8 character(len=n) :: c end type pdt type(pdt(42)) :: p integer, parameter :: m = len (p% c) integer, parameter :: n = p% c% len if (m /= 42) stop 1 if (len (p% c) /= 42) stop 2 print *, p% c% len ! OK if (p% c% len /= 42) stop 3 ! OK print *, n ! ICE end I get: pdt_33.f03:14:27: 14 | integer, parameter :: n = p% c% len | 1 Error: non-constant initialization expression at (1) pdt_33.f03:20:31: 20 | print *, n ! ICE | 1 internal compiler error: tree check: expected record_type or union_type or qual_union_type, have integer_type in gfc_conv_component_ref, at fortran/trans-expr.cc:2757 0x84286c tree_check_failed(tree_node const*, char const*, int, char const*, ...) ../../gcc-trunk/gcc/tree.cc:8899 0xa6d6fb tree_check3(tree_node*, char const*, int, char const*, tree_code, tree_code, tree_code) ../../gcc-trunk/gcc/tree.h:3617 0xa90847 gfc_conv_component_ref(gfc_se*, gfc_ref*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:2757 0xa91bbc gfc_conv_variable ../../gcc-trunk/gcc/fortran/trans-expr.cc:3137 0xaa8e9c gfc_conv_expr(gfc_se*, gfc_expr*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:9594 0xaa92ae gfc_conv_expr_reference(gfc_se*, gfc_expr*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:9713 0xad67f6 gfc_trans_transfer(gfc_code*) ../../gcc-trunk/gcc/fortran/trans-io.cc:2607 0xa43cb7 trans_code ../../gcc-trunk/gcc/fortran/trans.cc:2449 0xad37c6 build_dt ../../gcc-trunk/gcc/fortran/trans-io.cc:2051 0xa43cd7 trans_code ../../gcc-trunk/gcc/fortran/trans.cc:2421 0xa84711 gfc_generate_function_code(gfc_namespace*) ../../gcc-trunk/gcc/fortran/trans-decl.cc:7762 0x9d9ca7 translate_all_program_units ../../gcc-trunk/gcc/fortran/parse.cc:6929 0x9d9ca7 gfc_parse_file() ../../gcc-trunk/gcc/fortran/parse.cc:7235 0xa40a1f gfc_be_parse_file ../../gcc-trunk/gcc/fortran/f95-lang.cc:229 The fortran-dump confirms that n is not simplified to a constant. So while you're at it, do you also see a solution to this variant? Harald Am 10.07.23 um 17:48 schrieb Andre Vehreschild via Gcc-patches: Hi all, while browsing the pdt meta-bug I came across 102003 and thought to myself: Well, that one is easy. How foolish of me... Anyway, the solution attached prevents a pdt_len (or pdt_kind) expression in a function call (e.g. len() or kind()) to mark the whole expression as a pdt one. The second part of the patch in simplify.cc then takes care of either generating the correct component ref or when a constant expression (i.e. gfc_init_expr_flag is set) is required to look this up from the actual symbol (not from the type, because there the default value is stored). Regtested ok on x86_64-linux-gnu/Fedora 37. Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Patch, fortran] Fix default type bugs in gfortran [PR99139, PR99368]
Hi Paul, thanks for taking this. I have just a minor comment regards coding style: + if (tmp + && tmp->attr.generic + && (tmp = gfc_find_dt_in_generic (tmp))) + { + if (tmp->attr.flavor == FL_DERIVED) My reading of the guidelines says that I should rather write if (tmp && tmp->attr.generic) { tmp = gfc_find_dt_in_generic (tmp); if (tmp && tmp->attr.flavor == FL_DERIVED) Both variants are equally readable, though. I haven't though long enough about possible minor memleaks, i.e. if a freeing of gfc_symbol tmp is advised. Running f951 under valgrind might give you a hint. Thanks, Harald Am 08.07.23 um 16:23 schrieb Paul Richard Thomas via Gcc-patches: The attached patch incorporates two of Steve's "Orphaned Patches" - https://gcc.gnu.org/pipermail/fortran/2023-June/059423.html They have in common that they both involve faults in use of default type and that I was the ultimate cause of the bugs. The patch regtests with the attached testcases. I will commit in the next 24 hours unless there are any objections. Paul Fortran: Fix default type bugs in gfortran [PR99139, PR99368] 2023-07-08 Steve Kargl gcc/fortran PR fortran/99139 PR fortran/99368 * match.cc (gfc_match_namelist): Check for host associated or defined types before applying default type. (gfc_match_select_rank): Apply default type to selector of unlnown type if possible. * resolve.cc (resolve_fl_variable): Do not apply local default initialization to assumed rank entities. gcc/testsuite/ PR fortran/999139 * gfortran.dg/pr99139.f90 : New test PR fortran/99368 * gfortran.dg/pr99368.f90 : New test Fortran: Fix default type bugs in gfortran [PR99139, PR99368] 2023-07-08 Steve Kargl gcc/fortran PR fortran/99139 PR fortran/99368 * match.cc (gfc_match_namelist): Check for host associated or defined types before applying default type. (gfc_match_select_rank): Apply default type to selector of unlnown type if possible. * resolve.cc (resolve_fl_variable): Do not apply local default initialization to assumed rank entities. gcc/testsuite/ PR fortran/999139 * gfortran.dg/pr99139.f90 : New test PR fortran/99368 * gfortran.dg/pr99368.f90 : New test
Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]
Hi Mikael, Am 08.07.23 um 14:07 schrieb Mikael Morin: here is what I'm finally coming to. This patch fixes my example, but is otherwise untested. The patch has grown enough that I'm tempted to fix my example separately, in its own commit. alright. I've interpreted this as a green light for v2 of my patch and pushed it as r14-2395-gb1079fc88f082d https://gcc.gnu.org/g:b1079fc88f082d3c5b583c8822c08c5647810259 so that you can build upon it. Mikael Thanks, Harald
[PATCH] Fortran: simplification of FINDLOC for constant complex arguments [PR110585]
Dear all, I intend to commit the attached obvious patch within 24h unless someone objects. gfc_compare_expr() did not handle the case of complex constants, which may be compared for equality. This case is needed in the simplification of the FINDLOC intrinsic. Regtested on x86_64-pc-linux-gnu. Thanks, Harald From b6c4f70d2dac4863335874f0bd3486ea7db348d7 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 7 Jul 2023 20:25:06 +0200 Subject: [PATCH] Fortran: simplification of FINDLOC for constant complex arguments [PR110585] gcc/fortran/ChangeLog: PR fortran/110585 * arith.cc (gfc_compare_expr): Handle equality comparison of constant complex gfc_expr arguments. gcc/testsuite/ChangeLog: PR fortran/110585 * gfortran.dg/findloc_9.f90: New test. --- gcc/fortran/arith.cc| 5 + gcc/testsuite/gfortran.dg/findloc_9.f90 | 19 +++ 2 files changed, 24 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/findloc_9.f90 diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index 86d56406047..f9c6658f860 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -1120,6 +1120,11 @@ gfc_compare_expr (gfc_expr *op1, gfc_expr *op2, gfc_intrinsic_op op) || (op1->value.logical && !op2->value.logical)); break; +case BT_COMPLEX: + gcc_assert (op == INTRINSIC_EQ); + rc = mpc_cmp (op1->value.complex, op2->value.complex); + break; + default: gfc_internal_error ("gfc_compare_expr(): Bad basic type"); } diff --git a/gcc/testsuite/gfortran.dg/findloc_9.f90 b/gcc/testsuite/gfortran.dg/findloc_9.f90 new file mode 100644 index 000..05974476cb3 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/findloc_9.f90 @@ -0,0 +1,19 @@ +! { dg-do compile } +! { dg-options "-fdump-tree-original" } +! PR fortran/110585 - simplification of FINDLOC for constant complex arguments + +program mvce + implicit none + integer, parameter :: a(*) = findloc([(1.,0.),(2.,1.)], (2.,0.)) + integer, parameter :: b(*) = findloc([(1.,0.),(2.,1.)], (2.,0.), back=.true.) + integer, parameter :: c(*) = findloc([(1.,0.),(2.,1.)], (2.,1.)) + integer, parameter :: d(*) = findloc([(1.,0.),(2.,1.)], (2.,1.), back=.true.) + integer, parameter :: e= findloc([(1.,0.),(2.,1.)], (2.,1.), dim=1) + if (a(1) /= 0) stop 1 + if (b(1) /= 0) stop 2 + if (c(1) /= 2) stop 3 + if (d(1) /= 2) stop 4 + if (e/= 2) stop 5 +end + +! { dg-final { scan-tree-dump-not "_gfortran_stop_numeric" "original" } } -- 2.35.3
Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]
Hi Mikael, Am 07.07.23 um 14:21 schrieb Mikael Morin: I'm attaching what I have (lightly) tested so far, which doesn't work. It seems gfc_conv_class_to_class reevaluates part of the original expression, which is not correct after deallocation. this looks much more elegant than my attempt that passed an additional argument to gfc_conv_class_to_class, to achieve what your patch does. Will have a look again tonight. Great. Harald
Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]
Hi Mikael, Am 05.07.23 um 16:54 schrieb Mikael Morin: Here is an example, admittedly artificial. Fails with the above change, but fails with master as well. program p implicit none type t integer :: i end type t type u class(t), allocatable :: ta(:) end type u type(u), allocatable, target :: c(:) c = [u([t(1), t(3)]), u([t(4), t(9)])] call bar (allocated (c(c(1)%ta(1)%i)%ta), c(c(1)%ta(1)%i)%ta, allocated (c(c(1)%ta(1)%i)%ta)) if (allocated(c(1)%ta)) stop 11 if (.not. allocated(c(2)%ta)) stop 12 contains subroutine bar (alloc, x, alloc2) logical :: alloc, alloc2 class(t), allocatable, intent(out) :: x(:) if (allocated (x)) stop 1 if (.not. alloc) stop 2 if (.not. alloc2) stop 3 end subroutine bar end while it looks artificial, it is valid, and IMHO it is a beast... I've played around and added another argument gfc_se *convse to gfc_conv_class_to_class in an attempt to implement what I thought you suggested (to get the .pre/.post separately), but in the end this did not lead to working code. And the tree-dump for your example above is beyond what I can grasp. I've noticed that my attempt does not properly handle the parmse.post; at least this is what the above example shows: there is a small part after the call to bar that should have been executed before that call, which I attribute to .post. But my attempts in moving that part regresses on a couple of testcases with class and intent(out). I am at a loss now. I am attaching the latest version of my patch to give you or Paul or others the opportunity to see what is wrong or add the missing pieces. Thanks for your help so far. Harald From 989030fc04eacf97a034ab1f7ed85b932669f82d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 5 Jul 2023 22:21:09 +0200 Subject: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178] gcc/fortran/ChangeLog: PR fortran/92178 * trans-expr.cc (gfc_conv_procedure_call): Check procedures for allocatable dummy arguments with INTENT(OUT) and move deallocation of actual arguments after evaluation of argument expressions before the procedure is executed. gcc/testsuite/ChangeLog: PR fortran/92178 * gfortran.dg/intent_out_16.f90: New test. * gfortran.dg/intent_out_17.f90: New test. * gfortran.dg/intent_out_18.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/trans-expr.cc | 54 +++-- gcc/testsuite/gfortran.dg/intent_out_16.f90 | 89 + gcc/testsuite/gfortran.dg/intent_out_17.f90 | 46 +++ gcc/testsuite/gfortran.dg/intent_out_18.f90 | 31 +++ 4 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/intent_out_16.f90 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_17.f90 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_18.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 30946ba3f63..7017b652d6e 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6085,9 +6085,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else info = NULL; - stmtblock_t post, clobbers; + stmtblock_t post, clobbers, dealloc_blk; gfc_init_block (&post); gfc_init_block (&clobbers); + gfc_init_block (&dealloc_blk); gfc_init_interface_mapping (&mapping); if (!comp) { @@ -6117,6 +6118,32 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && UNLIMITED_POLY (sym) && comp && (strcmp ("_copy", comp->name) == 0); + /* Scan for allocatable actual arguments passed to allocatable dummy + arguments with INTENT(OUT). As the corresponding actual arguments are + deallocated before execution of the procedure, we evaluate actual + argument expressions to avoid problems with possible dependencies. */ + bool force_eval_args = false; + gfc_formal_arglist *tmp_formal; + for (arg = args, tmp_formal = formal; arg != NULL; + arg = arg->next, tmp_formal = tmp_formal ? tmp_formal->next : NULL) +{ + e = arg->expr; + fsym = tmp_formal ? tmp_formal->sym : NULL; + if (e && fsym + && e->expr_type == EXPR_VARIABLE + && fsym->attr.intent == INTENT_OUT + && (fsym->ts.type == BT_CLASS && fsym->attr.class_ok + ? CLASS_DATA (fsym)->attr.allocatable + : fsym->attr.allocatable) + && e->symtree + && e->symtree->n.sym + && gfc_variable_attr (e, NULL).allocatable) + { + force_eval_args = true; + break; + } +} + /* Evaluate the arguments. */ for (arg = args, argc = 0; arg != NULL; arg = arg->next, formal = formal ? formal->next : NULL, ++argc) @@ -6680,7 +6707,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else tmp = gfc_finish_block (&block); - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } /* A class array element needs converting back to be a @
Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]
Hi Mikael, all, I think I've found it: there is a call to gfc_conv_class_to_class that - according to a comment - does a repackaging to a class array. Deferring that repackaging along with the deallocation not only fixes the regression, but also the cases I tested. Attached is a "sneak preview", hoping that the experts (Paul, Mikael, ...) can tell if I am going down the wrong road. I'll wrap up all pieces and resubmit when the dust settles. We can then address the other findings later. Harald Am 04.07.23 um 15:35 schrieb Mikael Morin: Le 03/07/2023 à 22:49, Harald Anlauf a écrit : Hi Mikael, Am 03.07.23 um 13:46 schrieb Mikael Morin: These look good, but I'm surprised that there is no similar change at the 6819 line. This is the class array actual vs class array dummy case. It seems to be checked by the "bar" subroutine in your testcase, except that the intent(out) argument comes last there, whereas it was coming first with the original testcases in the PR. Can you double check? I believe I tried that before and encountered regressions. The change diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 16e8f037cfc..43e013fa720 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6844,7 +6844,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else tmp = gfc_finish_block (&block); - gfc_add_expr_to_block (&se->pre, tmp); +// gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } /* The conversion does not repackage the reference to a class regresses on: gfortran.dg/class_array_16.f90 gfortran.dg/finalize_12.f90 gfortran.dg/optional_class_1.f90 A simplified testcase for further study: program p implicit none class(*), allocatable :: c(:) c = [3, 4] call bar (allocated (c), c, allocated (c)) if (allocated (c)) stop 14 contains subroutine bar (alloc, x, alloc2) logical :: alloc, alloc2 class(*), allocatable, intent(out) :: x(:) if (allocated (x)) stop 5 if (.not. alloc) stop 6 if (.not. alloc2) stop 16 end subroutine bar end (This fails in a different place for the posted patch and for the above trial change. Need to go to the drawing board...) I've had a quick look. The code originally generated looks like: D.4343 = (void *[0:] * restrict) c._data.data != 0B; if (c._data.data != 0B) // free c._data.data c._data.data = 0B; ... class.3._data = c._data; ... D.4345 = (void *[0:] * restrict) c._data.data != 0B; bar (&D.4343, &class.3, &D.4345); this fails because D.4345 has the wrong value. With your change, it becomes: D.4343 = (void *[0:] * restrict) c._data.data != 0B; ... class.3._data = c._data; ... D.4345 = (void *[0:] * restrict) c._data.data != 0B; if (c._data.data != 0B) // free c._data.data c._data.data = 0B; bar (&D.4343, &class.3, &D.4345); and then it is class.3._data that has the wrong value. So basically the initialization of class.3 should move with the deallocation. I can reproduce a similar problem with your unmodified patch on the following variant: program p implicit none class(*), allocatable :: c c = 3 call bar (c, allocated (c)) if (allocated (c)) stop 14 contains subroutine bar (x, alloc2) logical :: alloc, alloc2 class(*), allocatable, intent(out) :: x(..) if (allocated (x)) stop 5 if (.not. alloc) stop 6 if (.not. alloc2) stop 16 end subroutine bar end diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 16e8f037cfc..a68c8d33acc 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6804,6 +6804,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, /* Pass a class array. */ parmse.use_offset = 1; gfc_conv_expr_descriptor (&parmse, e); + bool defer_repackage = false; /* If an ALLOCATABLE dummy argument has INTENT(OUT) and is allocated on entry, it must be deallocated. */ @@ -6844,7 +6845,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else tmp = gfc_finish_block (&block); - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); + defer_repackage = true; } /* The conversion does not repackage the reference to a class @@ -6858,6 +6860,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && e->symtree->n.sym->attr.optional, CLASS_DATA (fsym)->attr.class_pointer || CLASS_DATA (fsym)->attr.allocatable); + + /* Defer repackaging after deallocation. */ + if (defer_repackage) + gfc_add_block_to_block (&dealloc_blk, &parmse.pre); } else { @@ -7131,17 +7137,12 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, /* If any actual argu
Re: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]
Hi Mikael, Am 03.07.23 um 13:46 schrieb Mikael Morin: A few thing to double check below. diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 30946ba3f63..16e8f037cfc 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc (...) @@ -6117,6 +6118,33 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && UNLIMITED_POLY (sym) && comp && (strcmp ("_copy", comp->name) == 0); + /* First scan argument list for allocatable actual arguments passed to + allocatable dummy arguments with INTENT(OUT). As the corresponding + actual arguments are deallocated before execution of the procedure, we + evaluate actual argument expressions to avoid problems with possible + dependencies. */ + bool force_eval_args = false; + gfc_formal_arglist *tmp_formal; + for (arg = args, tmp_formal = formal; arg != NULL; + arg = arg->next, tmp_formal = tmp_formal ? tmp_formal->next : NULL) + { + e = arg->expr; + fsym = tmp_formal ? tmp_formal->sym : NULL; + if (e && fsym + && e->expr_type == EXPR_VARIABLE + && fsym->attr.intent == INTENT_OUT + && (fsym->ts.type == BT_CLASS && fsym->attr.class_ok + ? CLASS_DATA (fsym)->attr.allocatable + : fsym->attr.allocatable) + && e->symtree + && e->symtree->n.sym + && gfc_variable_attr (e, NULL).allocatable) + { + force_eval_args = true; + break; + } + } + The function is already big enough, would you mind outlining this to its own function? This can be done. At least it is not part of the monster loop. /* Evaluate the arguments. */ for (arg = args, argc = 0; arg != NULL; arg = arg->next, formal = formal ? formal->next : NULL, ++argc) @@ -6680,7 +6708,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else tmp = gfc_finish_block (&block); - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } /* A class array element needs converting back to be a @@ -6980,7 +7008,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, build_empty_stmt (input_location)); } if (tmp != NULL_TREE) - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } tmp = parmse.expr; @@ -7004,7 +7032,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, void_type_node, gfc_conv_expr_present (e->symtree->n.sym), tmp, build_empty_stmt (input_location)); - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } } } These look good, but I'm surprised that there is no similar change at the 6819 line. This is the class array actual vs class array dummy case. It seems to be checked by the "bar" subroutine in your testcase, except that the intent(out) argument comes last there, whereas it was coming first with the original testcases in the PR. Can you double check? I believe I tried that before and encountered regressions. The change diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 16e8f037cfc..43e013fa720 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6844,7 +6844,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else tmp = gfc_finish_block (&block); - gfc_add_expr_to_block (&se->pre, tmp); +// gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } /* The conversion does not repackage the reference to a class regresses on: gfortran.dg/class_array_16.f90 gfortran.dg/finalize_12.f90 gfortran.dg/optional_class_1.f90 A simplified testcase for further study: program p implicit none class(*), allocatable :: c(:) c = [3, 4] call bar (allocated (c), c, allocated (c)) if (allocated (c)) stop 14 contains subroutine bar (alloc, x, alloc2) logical :: alloc, alloc2 class(*), allocatable, intent(out) :: x(:) if (allocated (x)) stop 5 if (.not. alloc) stop 6 if (.not. alloc2) stop 16 end subroutine bar end (This fails in a different place for the posted patch and for the above trial change. Need to go to the drawing board...) @@ -7101,6 +7129,21 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, } } + /* If any actual argument of the procedure is allocatable and passed + to an allocatable dummy with INTENT(OUT), we conservatively + evaluate all actual argument expressions before deallocations are + performed and the procedure is executed. This ensures we conform + to F2023:15.5.3, 15.5.4. Create temporaries except for constants, + variables, and functio
[PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178]
Dear all, the attached patch fixes a long-standing issue with the order of evaluation of procedure argument expressions and deallocation of allocatable actual arguments passed to allocatable dummies with intent(out) attribute. It is based on an initial patch by Steve, handles issues pointed out by Tobias, and includes a suggestion by Tobias to scan the procedure arguments first to decide whether the creation of temporaries is needed. There is one unresolved issue left that might be more general: it appears to affect character arguments (only) in that quite often there still is no temporary generated. I haven't found the reason why and would like to defer this, unless someone has a good suggestion. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 609ba636927811cddc74fb815cb18809c7d33565 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 2 Jul 2023 22:14:19 +0200 Subject: [PATCH] Fortran: fixes for procedures with ALLOCATABLE,INTENT(OUT) arguments [PR92178] gcc/fortran/ChangeLog: PR fortran/92178 * trans-expr.cc (gfc_conv_procedure_call): Check procedures for allocatable dummy arguments with INTENT(OUT) and move deallocation of actual arguments after evaluation of argument expressions before the procedure is executed. gcc/testsuite/ChangeLog: PR fortran/92178 * gfortran.dg/pr92178.f90: New test. * gfortran.dg/pr92178_2.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/trans-expr.cc | 52 ++-- gcc/testsuite/gfortran.dg/pr92178.f90 | 83 + gcc/testsuite/gfortran.dg/pr92178_2.f90 | 46 ++ 3 files changed, 177 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr92178.f90 create mode 100644 gcc/testsuite/gfortran.dg/pr92178_2.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 30946ba3f63..16e8f037cfc 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6085,9 +6085,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else info = NULL; - stmtblock_t post, clobbers; + stmtblock_t post, clobbers, dealloc_blk; gfc_init_block (&post); gfc_init_block (&clobbers); + gfc_init_block (&dealloc_blk); gfc_init_interface_mapping (&mapping); if (!comp) { @@ -6117,6 +6118,33 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && UNLIMITED_POLY (sym) && comp && (strcmp ("_copy", comp->name) == 0); + /* First scan argument list for allocatable actual arguments passed to + allocatable dummy arguments with INTENT(OUT). As the corresponding + actual arguments are deallocated before execution of the procedure, we + evaluate actual argument expressions to avoid problems with possible + dependencies. */ + bool force_eval_args = false; + gfc_formal_arglist *tmp_formal; + for (arg = args, tmp_formal = formal; arg != NULL; + arg = arg->next, tmp_formal = tmp_formal ? tmp_formal->next : NULL) +{ + e = arg->expr; + fsym = tmp_formal ? tmp_formal->sym : NULL; + if (e && fsym + && e->expr_type == EXPR_VARIABLE + && fsym->attr.intent == INTENT_OUT + && (fsym->ts.type == BT_CLASS && fsym->attr.class_ok + ? CLASS_DATA (fsym)->attr.allocatable + : fsym->attr.allocatable) + && e->symtree + && e->symtree->n.sym + && gfc_variable_attr (e, NULL).allocatable) + { + force_eval_args = true; + break; + } +} + /* Evaluate the arguments. */ for (arg = args, argc = 0; arg != NULL; arg = arg->next, formal = formal ? formal->next : NULL, ++argc) @@ -6680,7 +6708,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else tmp = gfc_finish_block (&block); - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } /* A class array element needs converting back to be a @@ -6980,7 +7008,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, build_empty_stmt (input_location)); } if (tmp != NULL_TREE) - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } tmp = parmse.expr; @@ -7004,7 +7032,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, void_type_node, gfc_conv_expr_present (e->symtree->n.sym), tmp, build_empty_stmt (input_location)); - gfc_add_expr_to_block (&se->pre, tmp); + gfc_add_expr_to_block (&dealloc_blk, tmp); } } } @@ -7101,6 +7129,21 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, } } + /* If any actual argument of the procedure is allocatable and passed + to an allocatable dummy with INTENT(OUT), we conservatively + evaluate all actual argument expressions before deallocations are + performed and the procedure is executed. This ensures we conform + to F2023:15.5.3, 15.5.4. Create temporaries except for constants, + variables, and functions returnin
Re: PR82943 - Suggested patch to fix
Hi Alex, welcome to the gfortran community. It is great that you are trying to get actively involved. You already did quite a few things right: patches shall be sent to the gcc-patches ML, but Fortran reviewers usually notice them only where they are copied to the fortran ML. There are some general recommendations on the formatting of C code, like indentation, of the patches, and of the commit log entries. Regarding coding standards, see https://www.gnu.org/prep/standards/ . Regarding testcases, a recommendation is to have a look at existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then decide if the testcase shall test the compile-time or run-time behaviour, and add the necessary dejagnu directives. You should also verify if your patch passes regression testing. For changes to gfortran, it is usually sufficient to run make check-fortran -j where is the number of parallel tests. You would need to report also the platform where you tested on. There is also a legal issue to consider before non-trivial patches can be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal If your patch is accepted and if you do not have write-access to the repository, one of the maintainers will likely take care of it. If you become a regular contributor, you will probably want to consider getting write access. Cheers, Harald On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote: Hello, I am new to the GFortran community. Over the past two weeks I created a patch that should fix PR82943 for GFortran. I have attached it to this email. The patch allows the code below to compile successfully. I am working on creating test cases next, but I am new to the process so it may take me some time. After I make test cases, do I email them to you as well? Do I need to make a pull-request on github in order to get the patch reviewed? Thank you, Alexander Westbrooks module testmod public :: foo type, public :: tough_lvl_0(a, b) integer, kind :: a = 1 integer, len :: b contains procedure :: foo end type type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c) integer, len :: c contains procedure :: bar end type type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d) integer, len :: d contains procedure :: foobar end type contains subroutine foo(this) class(tough_lvl_0(1,*)), intent(inout) :: this end subroutine subroutine bar(this) class(tough_lvl_1(1,*,*)), intent(inout) :: this end subroutine subroutine foobar(this) class(tough_lvl_2(1,*,*,*)), intent(inout) :: this end subroutine end module PROGRAM testprogram USE testmod TYPE(tough_lvl_0(1,5)) :: test_pdt_0 TYPE(tough_lvl_1(1,5,6)) :: test_pdt_1 TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2 CALL test_pdt_0%foo() CALL test_pdt_1%foo() CALL test_pdt_1%bar() CALL test_pdt_2%foo() CALL test_pdt_2%bar() CALL test_pdt_2%foobar() END PROGRAM testprogram
[PATCH, part3, committed] Fortran: ABI for scalar CHARACTER(LEN=1),VALUE dummy argument [PR110360]
Dear all, the previous patches to this PR unfortunately caused a regression, seen on Power big-endian systems/-m32 (pr110419), and while trying to investigate on x86 also showed a regression (ICE) on cases that were not covered in the testsuite before. The original fix did not properly handle the dereferencing of string arguments that were not constant, and it was lacking the truncation of strings to length one that is needed when passing a character on the stack. This patch has been regtested on x86_64-pc-linux-gnu, and the extended testcase was scrutinized with -m64 and -m32. Pushed after discussion in the PR with Mikael as commit r14-2171-g8736d6b14a4dfdfb58c80ccd398981b0fb5d00aa https://gcc.gnu.org/g:8736d6b14a4dfdfb58c80ccd398981b0fb5d00aa Will keep the PR open as long as the issues on Power big-endian are not confirmed resolved. Thanks, Harald From 8736d6b14a4dfdfb58c80ccd398981b0fb5d00aa Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 28 Jun 2023 22:16:18 +0200 Subject: [PATCH] Fortran: ABI for scalar CHARACTER(LEN=1),VALUE dummy argument [PR110360] gcc/fortran/ChangeLog: PR fortran/110360 * trans-expr.cc (gfc_conv_procedure_call): For non-constant string argument passed to CHARACTER(LEN=1),VALUE dummy, ensure proper dereferencing and truncation of string to length 1. gcc/testsuite/ChangeLog: PR fortran/110360 * gfortran.dg/value_9.f90: Add tests for intermediate regression. --- gcc/fortran/trans-expr.cc | 15 ++- gcc/testsuite/gfortran.dg/value_9.f90 | 23 +++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index ad0cdf902ba..30946ba3f63 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6395,7 +6395,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, /* ABI: actual arguments to CHARACTER(len=1),VALUE dummy arguments are actually passed by value. - Constant strings are truncated to length 1. + Strings are truncated to length 1. The BIND(C) case is handled elsewhere. */ if (fsym->ts.type == BT_CHARACTER && !fsym->ts.is_c_interop @@ -6405,10 +6405,15 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, (fsym->ts.u.cl->length->value.integer, 1) == 0)) { if (e->expr_type != EXPR_CONSTANT) - parmse.expr = gfc_string_to_single_character - (build_int_cst (gfc_charlen_type_node, 1), - parmse.expr, - e->ts.kind); + { + 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; + } else if (e->value.character.length > 1) { e->value.character.length = 1; diff --git a/gcc/testsuite/gfortran.dg/value_9.f90 b/gcc/testsuite/gfortran.dg/value_9.f90 index f6490645e27..1a2fa80ed0d 100644 --- a/gcc/testsuite/gfortran.dg/value_9.f90 +++ b/gcc/testsuite/gfortran.dg/value_9.f90 @@ -9,7 +9,12 @@ program p character (kind=4), allocatable :: ca4 character (kind=4), pointer :: cp4 character(len=:,kind=4), allocatable :: cd4 + character:: c = "1" + character (kind=4) :: c4 = 4_"4" + character(len=3) :: d = "210" + character(len=3,kind=4) :: d4 = 4_"321" integer :: a = 65 + integer :: l = 2 allocate (ca, cp, ca4, cp4) ! Check len=1 actual argument cases first @@ -20,15 +25,21 @@ program p call val ("A",char(a)) call val ("A",mychar(65)) call val ("A",mychar(a)) + call val ("1",c) + call val ("1",(c)) call val4 (4_"C",4_"C") call val4 (4_"A",char(65,kind=4)) call val4 (4_"A",char(a, kind=4)) + call val4 (4_"4",c4) + call val4 (4_"4",(c4)) call val (ca,ca) call val (cp,cp) call val (cd,cd) + call val (ca,(ca)) call val4 (ca4,ca4) call val4 (cp4,cp4) call val4 (cd4,cd4) + call val4 (cd4,(cd4)) call sub ("S") call sub4 (4_"T") @@ -37,6 +48,18 @@ program p call val4 (4_"V**",4_"V//") call sub ( "WTY") call sub4 (4_"ZXV") + call val ( "234", d) + call val4 (4_"345", d4 ) + call val ( "234", (d) ) + call val4 (4_"345", (d4) ) + call val ( "234", d (1:2)) + call val4 (4_"345", d4(1:2)) + call val ( "234", d (1:l)) + call val4 (4_"345", d4(1:l)) + call val ("1",c // d) + call val ("1",trim (c // d)) + call val4 (4_"4",c4 // d4) + call val4 (4_"4",trim (c4 // d4)) cd = "gkl"; cd4 = 4_"hmn" call val (cd,cd) call val4 (cd4,cd4) -- 2.35.3
Re: [Patch, fortran] PR49213 - [OOP] gfortran rejects structure constructor expression
Hi Paul, this is much better now. I have only a minor comment left: in the calculation of the size of a character string you are using an intermediate gfc_array_index_type, whereas I have learned to use gfc_charlen_type_node now, which seems like the natural type here. OK for trunk, and thanks for your patience! Harald On 6/27/23 12:30, Paul Richard Thomas via Gcc-patches wrote: Hi Harald, Let's try again :-) OK for trunk? Regards Paul Fortran: Enable class expressions in structure constructors [PR49213] 2023-06-27 Paul Thomas gcc/fortran PR fortran/49213 * expr.cc (gfc_is_ptr_fcn): Remove reference to class_pointer. * resolve.cc (resolve_assoc_var): Call gfc_is_ptr_fcn to allow associate names with pointer function targets to be used in variable definition context. * trans-decl.cc (get_symbol_decl): Remove extraneous line. * trans-expr.cc (alloc_scalar_allocatable_subcomponent): Obtain size of intrinsic and character expressions. (gfc_trans_subcomponent_assign): Expand assignment to class components to include intrinsic and character expressions. gcc/testsuite/ PR fortran/49213 * gfortran.dg/pr49213.f90 : New test On Sat, 24 Jun 2023 at 20:50, Harald Anlauf wrote: Hi Paul! On 6/24/23 15:18, Paul Richard Thomas via Gcc-patches wrote: I have included the adjustment to 'gfc_is_ptr_fcn' and eliminating the extra blank line, introduced by my last patch. I played safe and went exclusively for class functions with attr.class_pointer set on the grounds that these have had all the accoutrements checked and built (ie. class_ok). I am still not sure if this is necessary or not. maybe it is my fault, but I find the version in the patch confusing: @@ -816,7 +816,7 @@ bool gfc_is_ptr_fcn (gfc_expr *e) { return e != NULL && e->expr_type == EXPR_FUNCTION - && (gfc_expr_attr (e).pointer + && ((e->ts.type != BT_CLASS && gfc_expr_attr (e).pointer) || (e->ts.type == BT_CLASS && CLASS_DATA (e)->attr.class_pointer)); } The caller 'gfc_is_ptr_fcn' has e->expr_type == EXPR_FUNCTION, so gfc_expr_attr (e) boils down to: if (e->value.function.esym && e->value.function.esym->result) { gfc_symbol *sym = e->value.function.esym->result; attr = sym->attr; if (sym->ts.type == BT_CLASS && sym->attr.class_ok) { attr.dimension = CLASS_DATA (sym)->attr.dimension; attr.pointer = CLASS_DATA (sym)->attr.class_pointer; attr.allocatable = CLASS_DATA (sym)->attr.allocatable; } } ... else if (e->symtree) attr = gfc_variable_attr (e, NULL); So I thought this should already do what you want if you do gfc_is_ptr_fcn (gfc_expr *e) { return e != NULL && e->expr_type == EXPR_FUNCTION && gfc_expr_attr (e).pointer; } or what am I missing? The additional checks in gfc_expr_attr are there to avoid ICEs in case CLASS_DATA (sym) has issues, and we all know Gerhard who showed that he is an expert in exploiting this. To sum up, I'd prefer to use the safer form if it works. If it doesn't, I would expect a latent issue. The rest of the code looked good to me, but I was suspicious about the handling of CHARACTER. Nasty as I am, I modified the testcase to use character(kind=4) instead of kind=1 (see attached). This either fails here (stop 10), or if I activate the marked line !cont = tContainer('hello!') ! ### ICE! ### I get an ICE. Can you have another look? Thanks, Harald OK for trunk? Paul Fortran: Enable class expressions in structure constructors [PR49213] 2023-06-24 Paul Thomas gcc/fortran PR fortran/49213 * expr.cc (gfc_is_ptr_fcn): Guard pointer attribute to exclude class expressions. * resolve.cc (resolve_assoc_var): Call gfc_is_ptr_fcn to allow associate names with pointer function targets to be used in variable definition context. * trans-decl.cc (get_symbol_decl): Remove extraneous line. * trans-expr.cc (alloc_scalar_allocatable_subcomponent): Obtain size of intrinsic and character expressions. (gfc_trans_subcomponent_assign): Expand assignment to class components to include intrinsic and character expressions. gcc/testsuite/ PR fortran/49213 * gfortran.dg/pr49213.f90 : New test
Re: [Patch, fortran] PR49213 - [OOP] gfortran rejects structure constructor expression
Hi Paul! On 6/24/23 15:18, Paul Richard Thomas via Gcc-patches wrote: I have included the adjustment to 'gfc_is_ptr_fcn' and eliminating the extra blank line, introduced by my last patch. I played safe and went exclusively for class functions with attr.class_pointer set on the grounds that these have had all the accoutrements checked and built (ie. class_ok). I am still not sure if this is necessary or not. maybe it is my fault, but I find the version in the patch confusing: @@ -816,7 +816,7 @@ bool gfc_is_ptr_fcn (gfc_expr *e) { return e != NULL && e->expr_type == EXPR_FUNCTION - && (gfc_expr_attr (e).pointer + && ((e->ts.type != BT_CLASS && gfc_expr_attr (e).pointer) || (e->ts.type == BT_CLASS && CLASS_DATA (e)->attr.class_pointer)); } The caller 'gfc_is_ptr_fcn' has e->expr_type == EXPR_FUNCTION, so gfc_expr_attr (e) boils down to: if (e->value.function.esym && e->value.function.esym->result) { gfc_symbol *sym = e->value.function.esym->result; attr = sym->attr; if (sym->ts.type == BT_CLASS && sym->attr.class_ok) { attr.dimension = CLASS_DATA (sym)->attr.dimension; attr.pointer = CLASS_DATA (sym)->attr.class_pointer; attr.allocatable = CLASS_DATA (sym)->attr.allocatable; } } ... else if (e->symtree) attr = gfc_variable_attr (e, NULL); So I thought this should already do what you want if you do gfc_is_ptr_fcn (gfc_expr *e) { return e != NULL && e->expr_type == EXPR_FUNCTION && gfc_expr_attr (e).pointer; } or what am I missing? The additional checks in gfc_expr_attr are there to avoid ICEs in case CLASS_DATA (sym) has issues, and we all know Gerhard who showed that he is an expert in exploiting this. To sum up, I'd prefer to use the safer form if it works. If it doesn't, I would expect a latent issue. The rest of the code looked good to me, but I was suspicious about the handling of CHARACTER. Nasty as I am, I modified the testcase to use character(kind=4) instead of kind=1 (see attached). This either fails here (stop 10), or if I activate the marked line !cont = tContainer('hello!') ! ### ICE! ### I get an ICE. Can you have another look? Thanks, Harald OK for trunk? Paul Fortran: Enable class expressions in structure constructors [PR49213] 2023-06-24 Paul Thomas gcc/fortran PR fortran/49213 * expr.cc (gfc_is_ptr_fcn): Guard pointer attribute to exclude class expressions. * resolve.cc (resolve_assoc_var): Call gfc_is_ptr_fcn to allow associate names with pointer function targets to be used in variable definition context. * trans-decl.cc (get_symbol_decl): Remove extraneous line. * trans-expr.cc (alloc_scalar_allocatable_subcomponent): Obtain size of intrinsic and character expressions. (gfc_trans_subcomponent_assign): Expand assignment to class components to include intrinsic and character expressions. gcc/testsuite/ PR fortran/49213 * gfortran.dg/pr49213.f90 : New test ! { dg-do run } ! ! Contributed by Neil Carlson ! program main ! character(2) :: c character(2,kind=4) :: c type :: S integer :: n end type type(S) :: Sobj type, extends(S) :: S2 integer :: m end type type(S2) :: S2obj type :: T class(S), allocatable :: x end type type(T) :: Tobj Sobj = S(1) Tobj = T(Sobj) S2obj = S2(1,2) Tobj = T(S2obj)! Failed here select type (x => Tobj%x) type is (S2) if ((x%n .ne. 1) .or. (x%m .ne. 2)) stop 1 class default stop 2 end select c = 4_" " call pass_it (T(Sobj)) if (c .ne. 4_"S ") stop 3 call pass_it (T(S2obj))! and here if (c .ne. 4_"S2") stop 4 call bar contains subroutine pass_it (foo) type(T), intent(in) :: foo select type (x => foo%x) type is (S) c = 4_"S " if (x%n .ne. 1) stop 5 type is (S2) c = 4_"S2" if ((x%n .ne. 1) .or. (x%m .ne. 2)) stop 6 class default stop 7 end select end subroutine subroutine bar ! Test from comment #29 of the PR - due to Janus Weil type tContainer class(*), allocatable :: x end type integer, parameter :: i = 0 character(7,kind=4) :: chr = 4_"goodbye" type(tContainer) :: cont cont%x = i ! linker error: undefined reference to `__copy_INTEGER_4_.3804' cont = tContainer(i+42) ! Failed here select type (z => cont%x) type is (integer) if (z .ne. 42) stop 8 class default stop 9 end select !cont = tContainer('hello!') ! ### ICE! ### cont = tContainer(4_'hello!') select type (z => cont%x) type is (character(*,kind=4)) if (z .ne. 4_'hello!') stop 10 class default stop 11 end select cont = tContainer(chr) select type (z => cont%x) type is (character(*,kind=4)) if (z .ne. 4_'goodbye') stop 12 class default
[PATCH, part2, committed] Fortran: ABI for scalar CHARACTER(LEN=1),VALUE dummy argument [PR110360]
Dear all, the first part of the patch came with a testcase that also exercised code for constant string arguments, which was not touched by that patch but seems to have caused runtime failures on big-endian platforms (e.g. Power-* BE) for all optimization levels, and on x86 / -m32 at -O1 and higher (not at -O0). I did not see any issues on x86 / -m64 and any optimization level, but could reproduce a problem with x86 / -m32 at -O1, which appears to be related how arguments that are to be passed by value are handled when there is a mismatch between the function prototype and the passed argument. The solution is to truncate too long constant string arguments, fixed by the attached patch, pushed as: https://gcc.gnu.org/g:3f97d10aa1ff5984d6fd657f246d3f251b254ff1 and see attached. * * * I found gcc-testresults quite helpful in checking whether my patch caused trouble on architectures different from the one I'm working on. The value (pun intended) would have been even greater if output of runtime failures would also be made available. Many (Fortran) tests provide either a stop code, or some hopefully helpful diagnostic output on stdout intended for locating errors on platforms where one has no direct access to, or is less familiar with. Far better than a plain FAIL: gfortran.dg/value_9.f90 -O1 execution test * * * Thanks, Harald From 3f97d10aa1ff5984d6fd657f246d3f251b254ff1 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 24 Jun 2023 20:36:53 +0200 Subject: [PATCH] Fortran: ABI for scalar CHARACTER(LEN=1),VALUE dummy argument [PR110360] gcc/fortran/ChangeLog: PR fortran/110360 * trans-expr.cc (gfc_conv_procedure_call): Truncate constant string argument of length > 1 passed to scalar CHARACTER(1),VALUE dummy. --- gcc/fortran/trans-expr.cc | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index c92fccd0be2..63e3cf9681e 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6395,20 +6395,25 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, /* ABI: actual arguments to CHARACTER(len=1),VALUE dummy arguments are actually passed by value. - The BIND(C) case is handled elsewhere. - TODO: truncate constant strings to length 1. */ + Constant strings are truncated to length 1. + The BIND(C) case is handled elsewhere. */ if (fsym->ts.type == BT_CHARACTER && !fsym->ts.is_c_interop && fsym->ts.u.cl->length->expr_type == EXPR_CONSTANT && fsym->ts.u.cl->length->ts.type == BT_INTEGER && (mpz_cmp_ui - (fsym->ts.u.cl->length->value.integer, 1) == 0) - && e->expr_type != EXPR_CONSTANT) + (fsym->ts.u.cl->length->value.integer, 1) == 0)) { - parmse.expr = gfc_string_to_single_character - (build_int_cst (gfc_charlen_type_node, 1), - parmse.expr, - e->ts.kind); + if (e->expr_type != EXPR_CONSTANT) + parmse.expr = gfc_string_to_single_character + (build_int_cst (gfc_charlen_type_node, 1), + parmse.expr, + e->ts.kind); + else if (e->value.character.length > 1) + { + e->value.character.length = 1; + gfc_conv_expr (&parmse, e); + } } if (fsym->attr.optional -- 2.35.3
[PATCH] Fortran: ABI for scalar CHARACTER(LEN=1),VALUE dummy argument [PR110360]
Dear all, gfortran's ABI specifies that actual arguments to CHARACTER(LEN=1),VALUE dummy arguments are passed by value in the scalar case. That did work for constant strings being passed, but not in several other cases, where pointers were passed, resulting in subsequent random junk... The attached patch fixes this for the case of a non-constant string argument. It does not touch the character,value bind(c) case - this is a different thing and may need separate work, as Mikael pointed out - and there is a missed optimization for the case of actual constant string arguments of length larger than 1: it appears that the full string is pushed to the stack. I did not address that, as the primary aim here is to get correctly working code. (I added a TODO in a comment.) Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From bea1e14490e4abc4b67bae8fdca5196bb93acd2d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 22 Jun 2023 22:07:41 +0200 Subject: [PATCH] Fortran: ABI for scalar CHARACTER(LEN=1),VALUE dummy argument [PR110360] gcc/fortran/ChangeLog: PR fortran/110360 * trans-expr.cc (gfc_conv_procedure_call): Pass actual argument to scalar CHARACTER(1),VALUE dummy argument by value. gcc/testsuite/ChangeLog: PR fortran/110360 * gfortran.dg/value_9.f90: New test. --- gcc/fortran/trans-expr.cc | 19 +++ gcc/testsuite/gfortran.dg/value_9.f90 | 78 +++ 2 files changed, 97 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/value_9.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 3c209bcde97..c92fccd0be2 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6392,6 +6392,25 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, else { gfc_conv_expr (&parmse, e); + + /* ABI: actual arguments to CHARACTER(len=1),VALUE + dummy arguments are actually passed by value. + The BIND(C) case is handled elsewhere. + TODO: truncate constant strings to length 1. */ + if (fsym->ts.type == BT_CHARACTER + && !fsym->ts.is_c_interop + && fsym->ts.u.cl->length->expr_type == EXPR_CONSTANT + && fsym->ts.u.cl->length->ts.type == BT_INTEGER + && (mpz_cmp_ui + (fsym->ts.u.cl->length->value.integer, 1) == 0) + && e->expr_type != EXPR_CONSTANT) + { + parmse.expr = gfc_string_to_single_character + (build_int_cst (gfc_charlen_type_node, 1), + parmse.expr, + e->ts.kind); + } + if (fsym->attr.optional && fsym->ts.type != BT_CLASS && fsym->ts.type != BT_DERIVED) diff --git a/gcc/testsuite/gfortran.dg/value_9.f90 b/gcc/testsuite/gfortran.dg/value_9.f90 new file mode 100644 index 000..f6490645e27 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/value_9.f90 @@ -0,0 +1,78 @@ +! { dg-do run } +! PR fortran/110360 - ABI for scalar character(len=1),value dummy argument + +program p + implicit none + character, allocatable :: ca + character, pointer :: cp + character(len=:),allocatable :: cd + character (kind=4), allocatable :: ca4 + character (kind=4), pointer :: cp4 + character(len=:,kind=4), allocatable :: cd4 + integer :: a = 65 + allocate (ca, cp, ca4, cp4) + + ! Check len=1 actual argument cases first + ca = "a"; cp = "b"; cd = "c" + ca4 = 4_"d"; cp4 = 4_"e"; cd4 = 4_"f" + call val ("B","B") + call val ("A",char(65)) + call val ("A",char(a)) + call val ("A",mychar(65)) + call val ("A",mychar(a)) + call val4 (4_"C",4_"C") + call val4 (4_"A",char(65,kind=4)) + call val4 (4_"A",char(a, kind=4)) + call val (ca,ca) + call val (cp,cp) + call val (cd,cd) + call val4 (ca4,ca4) + call val4 (cp4,cp4) + call val4 (cd4,cd4) + call sub ("S") + call sub4 (4_"T") + + ! Check that always the first character of the string is finally used + call val ( "U++", "U--") + call val4 (4_"V**",4_"V//") + call sub ( "WTY") + call sub4 (4_"ZXV") + cd = "gkl"; cd4 = 4_"hmn" + call val (cd,cd) + call val4 (cd4,cd4) + call sub (cd) + call sub4 (cd4) + deallocate (ca, cp, ca4, cp4, cd, cd4) +contains + subroutine val (x, c) +character(kind=1), intent(in) :: x ! control: pass by reference +character(kind=1), value :: c +print *, "by value(kind=1): ", c +if (c /= x) stop 1 +c = "*" +if (c /= "*") stop 2 + end + + subroutine val4 (x, c) +character(kind=4), intent(in) :: x ! control: pass by reference +character(kind=4), value :: c +print *, "by value(kind=4): ", c +if (c /= x) stop 3 +c = 4_"#" +if (c /= 4_"#") stop 4 + end + + subroutine sub (s) +character(*), intent(in) :: s +call val (s, s) + end + subroutine sub4 (s) +character(kind=4,len=*), intent(in) :: s +call val4 (s, s) + end + + character function mychar (i) +integer, intent(in) :: i +mychar = char (i) + end +end -- 2.35.3
Re: [Patch, fortran] PR107900 Select type with intrinsic type inside associate causes ICE / Segmenation fault
Hi Paul, while I only had a minor question regarding gfc_is_ptr_fcn(), can you still try to enlighten me why that second part was necessary? (I believed it to be redundant and may have overlooked the obvious.) Cheers, Harald On 6/21/23 18:12, Paul Richard Thomas via Gcc-patches wrote: Committed as r14-2022-g577223aebc7acdd31e62b33c1682fe54a622ae27 Thanks for the help and the review Harald. Thanks to Steve too for picking up Neil Carlson's bugs. Cheers Paul On Tue, 20 Jun 2023 at 22:57, Harald Anlauf wrote: Hi Paul, On 6/20/23 12:54, Paul Richard Thomas via Gcc-patches wrote: Hi Harald, Fixing the original testcase in this PR turned out to be slightly more involved than I expected. However, it resulted in an open door to fix some other PRs and the attached much larger patch. This time, I did remember to include the testcases in the .diff :-) indeed! :-) I've only had a superficial look so far although it looks very good. (I have to trust your experience with unlimited polymorphism.) However, I was wondering about the following helper function: +bool +gfc_is_ptr_fcn (gfc_expr *e) +{ + return e != NULL && e->expr_type == EXPR_FUNCTION + && (gfc_expr_attr (e).pointer + || (e->ts.type == BT_CLASS + && CLASS_DATA (e)->attr.class_pointer)); +} + + /* Copy a shape array. */ Is there a case where gfc_expr_attr (e).pointer returns false and you really need the || part? Looking at gfc_expr_attr and the present context, it might just not be necessary. I believe that, between the Change.Logs and the comments, it is reasonably self-explanatory. OK for trunk? OK from my side. Thanks for the patch! Harald Regards Paul Fortran: Fix some bugs in associate [PR87477] 2023-06-20 Paul Thomas gcc/fortran PR fortran/87477 PR fortran/88688 PR fortran/94380 PR fortran/107900 PR fortran/110224 * decl.cc (char_len_param_value): Fix memory leak. (resolve_block_construct): Remove unnecessary static decls. * expr.cc (gfc_is_ptr_fcn): New function. (gfc_check_vardef_context): Use it to permit pointer function result selectors to be used for associate names in variable definition context. * gfortran.h: Prototype for gfc_is_ptr_fcn. * match.cc (build_associate_name): New function. (gfc_match_select_type): Use the new function to replace inline version and to build a new associate name for the case where the supplied associate name is already used for that purpose. * resolve.cc (resolve_assoc_var): Call gfc_is_ptr_fcn to allow associate names with pointer function targets to be used in variable definition context. * trans-decl.cc (gfc_get_symbol_decl): Unlimited polymorphic variables need deferred initialisation of the vptr. (gfc_trans_deferred_vars): Do the vptr initialisation. * trans-stmt.cc (trans_associate_var): Ensure that a pointer associate name points to the target of the selector and not the selector itself. gcc/testsuite/ PR fortran/87477 PR fortran/107900 * gfortran.dg/pr107900.f90 : New test PR fortran/110224 * gfortran.dg/pr110224.f90 : New test PR fortran/88688 * gfortran.dg/pr88688.f90 : New test PR fortran/94380 * gfortran.dg/pr94380.f90 : New test PR fortran/95398 * gfortran.dg/pr95398.f90 : Set -std=f2008, bump the line numbers in the error tests by two and change the text in two.
Re: [Patch, fortran] PR107900 Select type with intrinsic type inside associate causes ICE / Segmenation fault
Hi Paul, On 6/20/23 12:54, Paul Richard Thomas via Gcc-patches wrote: Hi Harald, Fixing the original testcase in this PR turned out to be slightly more involved than I expected. However, it resulted in an open door to fix some other PRs and the attached much larger patch. This time, I did remember to include the testcases in the .diff :-) indeed! :-) I've only had a superficial look so far although it looks very good. (I have to trust your experience with unlimited polymorphism.) However, I was wondering about the following helper function: +bool +gfc_is_ptr_fcn (gfc_expr *e) +{ + return e != NULL && e->expr_type == EXPR_FUNCTION + && (gfc_expr_attr (e).pointer + || (e->ts.type == BT_CLASS + && CLASS_DATA (e)->attr.class_pointer)); +} + + /* Copy a shape array. */ Is there a case where gfc_expr_attr (e).pointer returns false and you really need the || part? Looking at gfc_expr_attr and the present context, it might just not be necessary. I believe that, between the Change.Logs and the comments, it is reasonably self-explanatory. OK for trunk? OK from my side. Thanks for the patch! Harald Regards Paul Fortran: Fix some bugs in associate [PR87477] 2023-06-20 Paul Thomas gcc/fortran PR fortran/87477 PR fortran/88688 PR fortran/94380 PR fortran/107900 PR fortran/110224 * decl.cc (char_len_param_value): Fix memory leak. (resolve_block_construct): Remove unnecessary static decls. * expr.cc (gfc_is_ptr_fcn): New function. (gfc_check_vardef_context): Use it to permit pointer function result selectors to be used for associate names in variable definition context. * gfortran.h: Prototype for gfc_is_ptr_fcn. * match.cc (build_associate_name): New function. (gfc_match_select_type): Use the new function to replace inline version and to build a new associate name for the case where the supplied associate name is already used for that purpose. * resolve.cc (resolve_assoc_var): Call gfc_is_ptr_fcn to allow associate names with pointer function targets to be used in variable definition context. * trans-decl.cc (gfc_get_symbol_decl): Unlimited polymorphic variables need deferred initialisation of the vptr. (gfc_trans_deferred_vars): Do the vptr initialisation. * trans-stmt.cc (trans_associate_var): Ensure that a pointer associate name points to the target of the selector and not the selector itself. gcc/testsuite/ PR fortran/87477 PR fortran/107900 * gfortran.dg/pr107900.f90 : New test PR fortran/110224 * gfortran.dg/pr110224.f90 : New test PR fortran/88688 * gfortran.dg/pr88688.f90 : New test PR fortran/94380 * gfortran.dg/pr94380.f90 : New test PR fortran/95398 * gfortran.dg/pr95398.f90 : Set -std=f2008, bump the line numbers in the error tests by two and change the text in two.
Re: [Patch, fortran] PR107900 Select type with intrinsic type inside associate causes ICE / Segmenation fault
Hi Paul, On 6/17/23 11:14, Paul Richard Thomas via Gcc-patches wrote: Hi All, The attached patch is amply described by the comments and the changelog. It also includes the fix for the memory leak in decl.cc, as promised some days ago. OK for trunk? I hate to say it, but you forgot to add the testcase again... :-( The patch fixes your "extended" testcase in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107900#c2 but the original one in comment ICEs for me here: % gfc-14 pr107900.f90 f951: internal compiler error: Segmentation fault 0x1025c2f crash_signal ../../gcc-trunk/gcc/toplev.cc:314 0x9d31d3 resolve_select_type ../../gcc-trunk/gcc/fortran/resolve.cc:9791 0x9cef5e gfc_resolve_code(gfc_code*, gfc_namespace*) ../../gcc-trunk/gcc/fortran/resolve.cc:12588 0x9d2431 resolve_codes ../../gcc-trunk/gcc/fortran/resolve.cc:18057 0x9d24fe gfc_resolve(gfc_namespace*) ../../gcc-trunk/gcc/fortran/resolve.cc:18092 0x9cf0ee gfc_resolve(gfc_namespace*) ../../gcc-trunk/gcc/fortran/resolve.cc:18077 0x9cf0ee resolve_block_construct ../../gcc-trunk/gcc/fortran/resolve.cc:10971 0x9cf0ee gfc_resolve_code(gfc_code*, gfc_namespace*) ../../gcc-trunk/gcc/fortran/resolve.cc:12596 0x9d2431 resolve_codes ../../gcc-trunk/gcc/fortran/resolve.cc:18057 0x9d24fe gfc_resolve(gfc_namespace*) ../../gcc-trunk/gcc/fortran/resolve.cc:18092 0x9b11f1 resolve_all_program_units ../../gcc-trunk/gcc/fortran/parse.cc:6864 0x9b11f1 gfc_parse_file() ../../gcc-trunk/gcc/fortran/parse.cc:7120 0xa033ef gfc_be_parse_file ../../gcc-trunk/gcc/fortran/f95-lang.cc:229 It hits an assert here: 9790 st = gfc_find_symtree (ns->sym_root, name); 9791 gcc_assert (st->n.sym->assoc); My tree is slightly modified, but the changes should not have any effect here. Can you please have a look, too? Thanks, Harald Regards Paul PS This leaves 89645 and 99065 as the only real blockers to PR87477. These will take a little while to fix. They come about because the type of the associate name is determined by that of a derived type function that hasn't been parsed at the time that component references are being parsed. If the order of the contained procedures is reversed, both test cases compile correctly. The fix will comprise matching the component name to the accessible derived types, while keeping track of all the references in case the match is ambiguous and has to be fixed up later.
Re: [PATCH] Fortran: fix passing of zero-sized array arguments to procedures [PR86277]
Hi Steve, On 6/13/23 19:45, Steve Kargl via Gcc-patches wrote: On Mon, Jun 12, 2023 at 11:12:45PM +0200, Harald Anlauf via Fortran wrote: Dear all, the attached - actually rather small - patch is the result of a rather intensive session with Mikael in an attempt to fix the situation that we did not create proper temporaries when passing zero-sized array arguments to procedures. When the dummy argument was declared as OPTIONAL, in many cases it was mis-detected as non-present. This also depended on the type of argument, and was different for different intrinsic types, notably character, and derived types, and should explain the rather large ratio of the size of the provided testcases to the actual fix... (What the patch does not address: we still generate too much code for unneeded temporaries, often two temporaries instead of just one. I'll open a separate PR to track this.) Regtested on x86_64-pc-linux-gnu. OK for mainline? If this survives long enough on 14-trunk, would this be eligible for a backport to 13-branch in time for 13.2? OK to commit. I've reviewed the bugzilla exchange between Mikael and you, and agree with committing this and opening a new PR to track the unneeded temporaries issue. this is tracked here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110241 Thanks for the review! Harald
[PATCH] Fortran: fix passing of zero-sized array arguments to procedures [PR86277]
Dear all, the attached - actually rather small - patch is the result of a rather intensive session with Mikael in an attempt to fix the situation that we did not create proper temporaries when passing zero-sized array arguments to procedures. When the dummy argument was declared as OPTIONAL, in many cases it was mis-detected as non-present. This also depended on the type of argument, and was different for different intrinsic types, notably character, and derived types, and should explain the rather large ratio of the size of the provided testcases to the actual fix... (What the patch does not address: we still generate too much code for unneeded temporaries, often two temporaries instead of just one. I'll open a separate PR to track this.) Regtested on x86_64-pc-linux-gnu. OK for mainline? If this survives long enough on 14-trunk, would this be eligible for a backport to 13-branch in time for 13.2? Thanks, Harald From 773b2aae412145d61638a0423c5891c4dfd0f945 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 12 Jun 2023 23:08:48 +0200 Subject: [PATCH] Fortran: fix passing of zero-sized array arguments to procedures [PR86277] gcc/fortran/ChangeLog: PR fortran/86277 * trans-array.cc (gfc_trans_allocate_array_storage): When passing a zero-sized array with fixed (= non-dynamic) size, allocate temporary by the caller, not by the callee. gcc/testsuite/ChangeLog: PR fortran/86277 * gfortran.dg/zero_sized_14.f90: New test. * gfortran.dg/zero_sized_15.f90: New test. Co-authored-by: Mikael Morin --- gcc/fortran/trans-array.cc | 2 +- gcc/testsuite/gfortran.dg/zero_sized_14.f90 | 181 gcc/testsuite/gfortran.dg/zero_sized_15.f90 | 114 3 files changed, 296 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/zero_sized_14.f90 create mode 100644 gcc/testsuite/gfortran.dg/zero_sized_15.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index e1c75e9fe02..e7c51bae052 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -1117,7 +1117,7 @@ gfc_trans_allocate_array_storage (stmtblock_t * pre, stmtblock_t * post, desc = info->descriptor; info->offset = gfc_index_zero_node; - if (size == NULL_TREE || integer_zerop (size)) + if (size == NULL_TREE || (dynamic && integer_zerop (size))) { /* A callee allocated array. */ gfc_conv_descriptor_data_set (pre, desc, null_pointer_node); diff --git a/gcc/testsuite/gfortran.dg/zero_sized_14.f90 b/gcc/testsuite/gfortran.dg/zero_sized_14.f90 new file mode 100644 index 000..32c7ae28e3a --- /dev/null +++ b/gcc/testsuite/gfortran.dg/zero_sized_14.f90 @@ -0,0 +1,181 @@ +! { dg-do run } +! PR fortran/86277 +! +! Check proper detection of presence of optional array dummy arguments +! for zero-sized actual array arguments or array constructors: +! tests for REAL (as non-character intrinsic type) and empty derived type + +program test + implicit none + real, parameter :: m(0) = 42. + real, parameter :: n(1) = 23. + real :: x(0) = 1. + real :: z(1) = 2. + real :: w(0) + real, pointer :: p(:) + real, allocatable :: y(:) + integer :: k = 0, l = 0 ! Test/failure counter + type dt + ! Empty type + end type dt + type(dt), parameter :: t0(0) = dt() + type(dt), parameter :: t1(1) = dt() + type(dt) :: t2(0) = dt() + type(dt) :: t3(1) = dt() + type(dt) :: t4(0) + type(dt), allocatable :: tt(:) + ! + allocate (p(0)) + allocate (y(0)) + allocate (tt(0)) + call a0 () + call a1 () + call a2 () + call a3 () + call all_missing () + print *, "Total tests:", k, " failed:", l +contains + subroutine a0 () +print *, "Variables as actual argument" +call i (m) +call i (n) +call i (x) +call i (w) +call i (y) +call i (p) +call j (t0) +call j (t1) +call j (t2) +call j (t3) +call j (t4) +call j (tt) +print *, "Array section as actual argument" +call i (m(1:0)) +call i (n(1:0)) +call i (x(1:0)) +call i (w(1:0)) +call i (z(1:0)) +call i (p(1:0)) +call j (t0(1:0)) +call j (t1(1:0)) +call j (t2(1:0)) +call j (t3(1:0)) +call j (t4(1:0)) +call j (tt(1:0)) + end subroutine a0 + ! + subroutine a1 () +print *, "Explicit temporary as actual argument" +call i ((m)) +call i ((n)) +call i ((n(1:0))) +call i ((x)) +call i ((w)) +call i ((z(1:0))) +call i ((y)) +call i ((p)) +call i ((p(1:0))) +call j ((t0)) +call j ((t1)) +call j ((tt)) +call j ((t1(1:0))) +call j ((tt(1:0))) + end subroutine a1 + ! + subroutine a2 () +print *, "Array constructor as actual argument" +call i ([m]) +call i ([n]) +call i ([x]) +call i ([w]) +call i ([z]) +call i ([m(1:0)]) +call i ([n(1:0)]) +call i ([m,n(1:0)])
Re: [PATCH] Fortran: add IEEE_QUIET_* and IEEE_SIGNALING_* comparisons
Hi FX, Am 06.06.23 um 21:29 schrieb FX Coudert via Gcc-patches: Hi, This is a repost of the patch at https://gcc.gnu.org/pipermail/gcc-patches/2022-September/600887.html which never really got green light, but I stopped pushing because stage 1 was closing and I was out of time. I just looked at that thread. I guess if you answer Mikael's questions at https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601744.html the patch will be fine. It depends on a middle-end patch adding a type-generic __builtin_iseqsig(), which I posted for review at: https://gcc.gnu.org/pipermail/gcc-patches/2023-June/620801.html Bootstrapped and regtested on x86_64-pc-linux-gnu, OK to commit (once the middle-end patch is accepted)? FX Thanks, Harald
Re: [Patch, fortran] PR87477 - (associate) - [meta-bug] [F03] issues concerning the ASSOCIATE statement
On 6/8/23 09:46, Mikael Morin wrote: Le 08/06/2023 à 07:57, Paul Richard Thomas via Fortran a écrit : Hi Harald, In answer to your question: void gfc_replace_expr (gfc_expr *dest, gfc_expr *src) { free_expr0 (dest); *dest = *src; free (src); } So it does indeed do the job. Sure, but his comment was about the case gfc_replace_expr is *not* executed. Right. The following legal code exhibits the leak, pointing to the gfc_copy_expr: subroutine paul (n) integer :: n character(n) :: c end I should perhaps have remarked that, following the divide error, gfc_simplify_expr was returning a mutilated version of the expression and this was somehow connected with successfully simplifying the parentheses. Copying and replacing on no errors deals with the problem. Is the expression mutilated enough that it can't be safely freed?
Re: [Patch, fortran] PR87477 - (associate) - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi Paul! On 6/7/23 18:10, Paul Richard Thomas via Gcc-patches wrote: Hi All, Three more fixes for PR87477. Please note that PR99350 was a blocker but, as pointed out in comment #5 of the PR, this has nothing to do with the associate construct. All three fixes are straight forward and the .diff + ChangeLog suffice to explain them. 'rankguessed' was made redundant by the last PR87477 fix. Regtests on x86_64 - good for mainline? Paul Fortran: Fix some more blockers in associate meta-bug [PR87477] 2023-06-07 Paul Thomas gcc/fortran PR fortran/99350 * decl.cc (char_len_param_value): Simplify a copy of the expr and replace the original if there is no error. This seems to lack a gfc_free_expr (p) in case the gfc_replace_expr is not executed, leading to a possible memleak. Can you check? @@ -1081,10 +1082,10 @@ char_len_param_value (gfc_expr **expr, bool *deferred) if (!gfc_expr_check_typed (*expr, gfc_current_ns, false)) return MATCH_ERROR; - /* If gfortran gets an EXPR_OP, try to simplify it. This catches things - like CHARACTER(([1])). */ - if ((*expr)->expr_type == EXPR_OP) -gfc_simplify_expr (*expr, 1); + /* Try to simplify the expression to catch things like CHARACTER(([1])). */ + p = gfc_copy_expr (*expr); + if (gfc_is_constant_expr (p) && gfc_simplify_expr (p, 1)) +gfc_replace_expr (*expr, p); else gfc_free_expr (p); * gfortran.h : Remove the redundant field 'rankguessed' from 'gfc_association_list'. * resolve.cc (resolve_assoc_var): Remove refs to 'rankguessed'. PR fortran/107281 * resolve.cc (resolve_variable): Associate names with constant or structure constructor targets cannot have array refs. PR fortran/109451 * trans-array.cc (gfc_conv_expr_descriptor): Guard expression character length backend decl before using it. Suppress the assignment if lhs equals rhs. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of associate variables pointing to a variable. Add comment. * trans-stmt.cc (trans_associate_var): Remove requirement that the character length be deferred before assigning the value returned by gfc_conv_expr_descriptor. Also, guard the backend decl before testing with VAR_P. gcc/testsuite/ PR fortran/99350 * gfortran.dg/pr99350.f90 : New test. PR fortran/107281 * gfortran.dg/associate_5.f03 : Changed error message. * gfortran.dg/pr107281.f90 : New test. PR fortran/109451 * gfortran.dg/associate_61.f90 : New test Otherwise LGTM. Thanks for the patch! Harald
Re: [PATCH] Fortran: add Fortran 2018 IEEE_{MIN,MAX} functions
Hi FX, On 6/6/23 21:11, FX Coudert via Gcc-patches wrote: Hi, I cannot see if there is proper support for kind=17 in your patch; at least the libgfortran/ieee/ieee_arithmetic.F90 part does not seem to have any related code. Can real(kind=17) ever be an IEEE mode? If so, something seriously wrong happened, because the IEEE modules have no kind=17 mention in them anywhere. Actually, where is the kind=17 documented? FX I was hoping for Thomas to come forward with some comment, as he was quite involved in related work. There are several threads on IEEE128 for Power on the fortran ML e.g. around November/December 2021, January 2022. I wasn't meaning to block your work, just wondering if the Power platform needs more attention here. Harald
Re: [PATCH] Fortran: add Fortran 2018 IEEE_{MIN,MAX} functions
Hi FX, On 6/6/23 15:19, FX via Gcc-patches wrote: Hi, This patch adds four IEEE functions from the Fortran 2018 standard: IEEE_MIN_NUM, IEEE_MAX_NUM, IEEE_MIN_NUM_MAG, and IEEE_MAX_NUM_MAG. Bootstrapped and regtested on x86_64-pc-linux-gnu, both 32 and 64-bit. OK to commit? FX it would be great if someone with access to a Power platform and knowledge of IEEE128 thereon could have a look. I cannot see if there is proper support for kind=17 in your patch; at least the libgfortran/ieee/ieee_arithmetic.F90 part does not seem to have any related code. Thanks, Harald
Re: [PATCH, committed] Fortran: fix diagnostics for SELECT RANK [PR100607]
Hi Paul, On 6/3/23 07:48, Paul Richard Thomas via Gcc-patches wrote: Hi Harald, It looks good to me. Thanks to you and Steve for the fix. I suggest that it is such and obvious one that it deserved back-porting. alright, I'll check how far this makes sense. Cheers, Harald Cheers Paul On Fri, 2 Jun 2023 at 19:06, Harald Anlauf via Fortran wrote: Dear all, I've committed that attached simple patch on behalf of Steve after discussion in the PR and regtesting on x86_64-pc-linux-gnu. It fixes a duplicate error message and an ICE. Pushed as r14-1505-gfae09dfc0e6bf4cfe35d817558827aea78c6426f . Thanks, Harald
Re: [Patch, fortran] PR37336 finalization
Hi Paul, all, On 6/3/23 15:16, Paul Richard Thomas via Gcc-patches wrote: Hi Thomas, I want to get something approaching correct finalization to the distros, which implies 12-branch at present. Hopefully I can do the same with associate in a month or two's time. IMHO it is not only distros, but also installations at (scientific) computing centers with a larger user base and a large software stack. Migrating to a different major version of gcc/gfortran is not a trivial task for them. I'd fully support the idea of backporting the finalization fixes, as IIUC this on the one hand touches a rather isolated part, and on the other hand already got quite some testing. It is also already in the 13-branch (or only mostly?). Given that 12.3 was released recently and 12.4 is far away, there'd be sufficient time to fix any fallout. Regarding the associate fixes, we could get as much of those into 13.2, which we'd normally expect in just a few months. As long as spare time to work on gfortran is limited, I'd rather prefer to get as much fixed for that release. (This is not a no: I simply expect that real regression testing for the associate changes may take more time.) I am dithering about changing the F2003/08 part of finalization since the default is 2018 compliance. That said, it does need a change since the suppression of constructor finalization is also suppressing finalization of function results within the compilers. I'll do that first, perhaps? That sounds like a good idea. Cheers, Harald Cheers Paul On Sat, 3 Jun 2023 at 06:50, Thomas Koenig wrote: Hi Paul, I propose to backport r13-6747-gd7caf313525a46f200d7f5db1ba893f853774aee to 12-branch very soon. Is this something that we usually do? While finalization was basically broken before, some people still used working subsets (or subsets that were broken, and they adapted or wrote their code accordingly). What is the general opinion on that? I'm undecided. Before that, I propose to remove the F2003/2008 finalization of structure and array constructors in 13- and 14-branches. I can see why it was removed from the standard in a correction to F2008 and think that it is likely to cause endless confusion and maintenance complications. However, finalization of function results within constructors will be retained. That, I agree with. Should it be noted somewhere as an intentional deviation from the standard? Best regards Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[PATCH, committed] Fortran: fix diagnostics for SELECT RANK [PR100607]
Dear all, I've committed that attached simple patch on behalf of Steve after discussion in the PR and regtesting on x86_64-pc-linux-gnu. It fixes a duplicate error message and an ICE. Pushed as r14-1505-gfae09dfc0e6bf4cfe35d817558827aea78c6426f . Thanks, Harald From fae09dfc0e6bf4cfe35d817558827aea78c6426f Mon Sep 17 00:00:00 2001 From: Steve Kargl Date: Fri, 2 Jun 2023 19:44:11 +0200 Subject: [PATCH] Fortran: fix diagnostics for SELECT RANK [PR100607] gcc/fortran/ChangeLog: PR fortran/100607 * resolve.cc (resolve_select_rank): Remove duplicate error. (resolve_fl_var_and_proc): Prevent NULL pointer dereference and suppress error message for temporary. gcc/testsuite/ChangeLog: PR fortran/100607 * gfortran.dg/select_rank_6.f90: New test. --- gcc/fortran/resolve.cc | 10 ++--- gcc/testsuite/gfortran.dg/select_rank_6.f90 | 48 + 2 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/select_rank_6.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 2ba3101f1fe..fd059dddf05 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -10020,11 +10020,6 @@ resolve_select_rank (gfc_code *code, gfc_namespace *old_ns) || gfc_expr_attr (code->expr1).pointer)) gfc_error ("RANK (*) at %L cannot be used with the pointer or " "allocatable selector at %L", &c->where, &code->expr1->where); - - if (case_value == -1 && (gfc_expr_attr (code->expr1).allocatable - || gfc_expr_attr (code->expr1).pointer)) - gfc_error ("RANK (*) at %L cannot be used with the pointer or " - "allocatable selector at %L", &c->where, &code->expr1->where); } /* Add EXEC_SELECT to switch on rank. */ @@ -13262,7 +13257,10 @@ resolve_fl_var_and_proc (gfc_symbol *sym, int mp_flag) if (allocatable) { - if (dimension && as->type != AS_ASSUMED_RANK) + if (dimension + && as + && as->type != AS_ASSUMED_RANK + && !sym->attr.select_rank_temporary) { gfc_error ("Allocatable array %qs at %L must have a deferred " "shape or assumed rank", sym->name, &sym->declared_at); diff --git a/gcc/testsuite/gfortran.dg/select_rank_6.f90 b/gcc/testsuite/gfortran.dg/select_rank_6.f90 new file mode 100644 index 000..d0121777bb5 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/select_rank_6.f90 @@ -0,0 +1,48 @@ +! { dg-do compile } +! PR fortran/100607 - fix diagnostics for SELECT RANK +! Contributed by T.Burnus + +program p + implicit none + integer, allocatable :: A(:,:,:) + + allocate(a(5:6,-2:2, 99:100)) + call foo(a) + call bar(a) + +contains + + subroutine foo(x) +integer, allocatable :: x(..) +if (rank(x) /= 3) stop 1 +if (any (lbound(x) /= [5, -2, 99])) stop 2 + +select rank (x) +rank(3) + if (any (lbound(x) /= [5, -2, 99])) stop 3 +end select + +select rank (x) ! { dg-error "pointer or allocatable selector at .2." } +rank(*) ! { dg-error "pointer or allocatable selector at .2." } + if (rank(x) /= 1) stop 4 + if (lbound(x, 1) /= 1) stop 5 +end select + end + + subroutine bar(x) +integer :: x(..) +if (rank(x) /= 3) stop 6 +if (any (lbound(x) /= 1)) stop 7 + +select rank (x) +rank(3) + if (any (lbound(x) /= 1)) stop 8 +end select + +select rank (x) +rank(*) + if (rank(x) /= 1) stop 9 + if (lbound(x, 1) /= 1) stop 10 +end select + end +end -- 2.35.3
Re: [PATCH] Fortran: force error on bad KIND specifier [PR88552]
Hi Mikael, Am 01.06.23 um 22:33 schrieb Mikael Morin: Hello, Le 01/06/2023 à 21:05, Harald Anlauf via Fortran a écrit : Dear all, we sometimes silently accept wrong declarations with unbalanced parentheses, as the PR and testcases therein show. It appears that the fix is obvious: use the existing error paths in gfc_match_kind_spec and error return from gfc_match_decl_type_spec. I'm still posting it here in case I have missed something not so obvious. The patch regtests cleanly on x86_64-pc-linux-gnu. OK for mainline? It looks good, but... Thanks, Harald From a30ff5af130c4d33c086fd136978d5f49cb8bde4 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 1 Jun 2023 20:56:11 +0200 Subject: [PATCH] Fortran: force error on bad KIND specifier [PR88552] gcc/fortran/ChangeLog: PR fortran/88552 * decl.cc (gfc_match_kind_spec): Use error path on missing right parenthesis. (gfc_match_decl_type_spec): Use error return when an error occurred during matching a KIND specifier. gcc/testsuite/ChangeLog: PR fortran/88552 * gfortran.dg/pr88552.f90: New test. --- gcc/fortran/decl.cc | 4 gcc/testsuite/gfortran.dg/pr88552.f90 | 6 ++ 2 files changed, 10 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr88552.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index 1de2b231242..deb20647fb9 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -3366,6 +3366,7 @@ close_brackets: else gfc_error ("Missing right parenthesis at %C"); m = MATCH_ERROR; + goto no_match; } else /* All tests passed. */ @@ -4716,6 +4717,9 @@ get_kind: return MATCH_ERROR; } + if (m == MATCH_ERROR) + return MATCH_ERROR; + ... can you move this up to the place where m is set? OK with that change. I was afraid that this would regress on the existing testcases pr91660_[12].f90 that depend on an error message emitted just before that hunk, but this turned out not to happen. Adjusted version committed as: r14-1477-gff8f45d20f9ea6acc99442ad29212d177f58e8fe . Thanks Thanks for the review! Harald
[PATCH] Fortran: force error on bad KIND specifier [PR88552]
Dear all, we sometimes silently accept wrong declarations with unbalanced parentheses, as the PR and testcases therein show. It appears that the fix is obvious: use the existing error paths in gfc_match_kind_spec and error return from gfc_match_decl_type_spec. I'm still posting it here in case I have missed something not so obvious. The patch regtests cleanly on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From a30ff5af130c4d33c086fd136978d5f49cb8bde4 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 1 Jun 2023 20:56:11 +0200 Subject: [PATCH] Fortran: force error on bad KIND specifier [PR88552] gcc/fortran/ChangeLog: PR fortran/88552 * decl.cc (gfc_match_kind_spec): Use error path on missing right parenthesis. (gfc_match_decl_type_spec): Use error return when an error occurred during matching a KIND specifier. gcc/testsuite/ChangeLog: PR fortran/88552 * gfortran.dg/pr88552.f90: New test. --- gcc/fortran/decl.cc | 4 gcc/testsuite/gfortran.dg/pr88552.f90 | 6 ++ 2 files changed, 10 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr88552.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index 1de2b231242..deb20647fb9 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -3366,6 +3366,7 @@ close_brackets: else gfc_error ("Missing right parenthesis at %C"); m = MATCH_ERROR; + goto no_match; } else /* All tests passed. */ @@ -4716,6 +4717,9 @@ get_kind: return MATCH_ERROR; } + if (m == MATCH_ERROR) +return MATCH_ERROR; + /* Defer association of the KIND expression of function results until after USE and IMPORT statements. */ if ((gfc_current_state () == COMP_NONE && gfc_error_flag_test ()) diff --git a/gcc/testsuite/gfortran.dg/pr88552.f90 b/gcc/testsuite/gfortran.dg/pr88552.f90 new file mode 100644 index 000..15e1b372f8f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr88552.f90 @@ -0,0 +1,6 @@ +! { dg-do compile } +! PR fortran/88552 +! Contributed by G.Steinmetz + +integer(len((c)) :: n ! { dg-error "must be CHARACTER" } +end -- 2.35.3
[PATCH] Fortran: reject bad DIM argument of SIZE intrinsic in simplification [PR104350]
Dear all, the attached almost obvious patch fixes an ICE on invalid that may occur when we attempt to simplify an initialization expression with SIZE for an out-of-range DIM argument. Returning gfc_bad_expr allows for a more graceful error recovery. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 738bdcce46bd760fcafd1eb56700c8824621266f Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 24 May 2023 21:04:43 +0200 Subject: [PATCH] Fortran: reject bad DIM argument of SIZE intrinsic in simplification [PR104350] gcc/fortran/ChangeLog: PR fortran/104350 * simplify.cc (simplify_size): Reject DIM argument of intrinsic SIZE with error when out of valid range. gcc/testsuite/ChangeLog: PR fortran/104350 * gfortran.dg/size_dim_2.f90: New test. --- gcc/fortran/simplify.cc | 12 +++- gcc/testsuite/gfortran.dg/size_dim_2.f90 | 19 +++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/size_dim_2.f90 diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index 3f77203e62e..81680117f70 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -7594,7 +7594,17 @@ simplify_size (gfc_expr *array, gfc_expr *dim, int k) if (dim->expr_type != EXPR_CONSTANT) return NULL; - d = mpz_get_ui (dim->value.integer) - 1; + if (array->rank == -1) + return NULL; + + d = mpz_get_si (dim->value.integer) - 1; + if (d < 0 || d > array->rank - 1) + { + gfc_error ("DIM argument (%d) to intrinsic SIZE at %L out of range " + "(1:%d)", d+1, &array->where, array->rank); + return &gfc_bad_expr; + } + if (!gfc_array_dimen_size (array, d, &size)) return NULL; } diff --git a/gcc/testsuite/gfortran.dg/size_dim_2.f90 b/gcc/testsuite/gfortran.dg/size_dim_2.f90 new file mode 100644 index 000..27a71d90a47 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/size_dim_2.f90 @@ -0,0 +1,19 @@ +! { dg-do compile } +! PR fortran/104350 - ICE with SIZE and bad DIM in initialization expression +! Contributed by G. Steinmetz + +program p + implicit none + integer :: k + integer, parameter :: x(2,3) = 42 + integer, parameter :: s(*) = [(size(x,dim=k),k=1,rank(x))] + integer, parameter :: t(*) = [(size(x,dim=k),k=1,3)] ! { dg-error "out of range" } + integer, parameter :: u(*) = [(size(x,dim=k),k=0,3)] ! { dg-error "out of range" } + integer, parameter :: v = product(shape(x)) + integer, parameter :: w = product([(size(x,k),k=0,3)]) ! { dg-error "out of range" } + print *,([(size(x,dim=k),k=1,rank(x))]) + print *, [(size(x,dim=k),k=1,rank(x))] + print *, [(size(x,dim=k),k=0,rank(x))] + print *, product([(size(x,dim=k),k=1,rank(x))]) + print *, product([(size(x,dim=k),k=0,rank(x))]) +end -- 2.35.3
[PATCH] Fortran: checking and simplification of RESHAPE intrinsic [PR103794]
Dear all, checking and simplification of the RESHAPE intrinsic could fail in various ways for sufficiently complicated arguments, like array constructors. Debugging revealed that in these cases we determined that the array arguments were constant but we did not properly simplify and expand the constructors. A possible solution is the extend the test for constant arrays - which already does an expansion for initialization expressions - to also perform an expansion for small constructors in the non-initialization case. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From bfb708fdb6c313473a3054be710c630dcdebf69d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 21 May 2023 22:25:29 +0200 Subject: [PATCH] Fortran: checking and simplification of RESHAPE intrinsic [PR103794] gcc/fortran/ChangeLog: PR fortran/103794 * check.cc (gfc_check_reshape): Expand constant arguments SHAPE and ORDER before checking. * gfortran.h (gfc_is_constant_array_expr): Add prototype. * iresolve.cc (gfc_resolve_reshape): Expand constant argument SHAPE. * simplify.cc (is_constant_array_expr): If array is determined to be constant, expand small array constructors if needed. (gfc_is_constant_array_expr): Wrapper for is_constant_array_expr. (gfc_simplify_reshape): Fix check for insufficient elements in SOURCE when no padding specified. gcc/testsuite/ChangeLog: PR fortran/103794 * gfortran.dg/reshape_10.f90: New test. * gfortran.dg/reshape_11.f90: New test. --- gcc/fortran/check.cc | 6 +++-- gcc/fortran/gfortran.h | 1 + gcc/fortran/iresolve.cc | 2 +- gcc/fortran/simplify.cc | 25 ++--- gcc/testsuite/gfortran.dg/reshape_10.f90 | 34 gcc/testsuite/gfortran.dg/reshape_11.f90 | 15 +++ 6 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/reshape_10.f90 create mode 100644 gcc/testsuite/gfortran.dg/reshape_11.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 3dd1711aa14..4086dc71d34 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -4723,7 +4723,7 @@ gfc_check_reshape (gfc_expr *source, gfc_expr *shape, } gfc_simplify_expr (shape, 0); - shape_is_const = gfc_is_constant_expr (shape); + shape_is_const = gfc_is_constant_array_expr (shape); if (shape->expr_type == EXPR_ARRAY && shape_is_const) { @@ -4732,6 +4732,8 @@ gfc_check_reshape (gfc_expr *source, gfc_expr *shape, for (i = 0; i < shape_size; ++i) { e = gfc_constructor_lookup_expr (shape->value.constructor, i); + if (e == NULL) + break; if (e->expr_type != EXPR_CONSTANT) continue; @@ -4764,7 +4766,7 @@ gfc_check_reshape (gfc_expr *source, gfc_expr *shape, if (!type_check (order, 3, BT_INTEGER)) return false; - if (order->expr_type == EXPR_ARRAY && gfc_is_constant_expr (order)) + if (order->expr_type == EXPR_ARRAY && gfc_is_constant_array_expr (order)) { int i, order_size, dim, perm[GFC_MAX_DIMENSIONS]; gfc_expr *e; diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 9dd6b45f112..8cfa8fd3afd 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3970,6 +3970,7 @@ bool gfc_fix_implicit_pure (gfc_namespace *); void gfc_convert_mpz_to_signed (mpz_t, int); gfc_expr *gfc_simplify_ieee_functions (gfc_expr *); +bool gfc_is_constant_array_expr (gfc_expr *); bool gfc_is_size_zero_array (gfc_expr *); /* trans-array.cc */ diff --git a/gcc/fortran/iresolve.cc b/gcc/fortran/iresolve.cc index 7880aba63bb..571e1bd3441 100644 --- a/gcc/fortran/iresolve.cc +++ b/gcc/fortran/iresolve.cc @@ -2424,7 +2424,7 @@ gfc_resolve_reshape (gfc_expr *f, gfc_expr *source, gfc_expr *shape, break; } - if (shape->expr_type == EXPR_ARRAY && gfc_is_constant_expr (shape)) + if (shape->expr_type == EXPR_ARRAY && gfc_is_constant_array_expr (shape)) { gfc_constructor *c; f->shape = gfc_get_shape (f->rank); diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index 6ba2040e61c..3f77203e62e 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -254,12 +254,19 @@ is_constant_array_expr (gfc_expr *e) break; } - /* Check and expand the constructor. */ - if (!array_OK && gfc_init_expr_flag && e->rank == 1) + /* Check and expand the constructor. We do this when either + gfc_init_expr_flag is set or for not too large array constructors. */ + bool expand; + expand = (e->rank == 1 + && e->shape + && (mpz_cmp_ui (e->shape[0], flag_max_array_constructor) < 0)); + + if (!array_OK && (gfc_init_expr_flag || expand) && e->rank == 1) { + bool saved_init_expr_flag = gfc_init_expr_flag; array_OK = gfc_reduce_init_expr (e); /* gfc_reduce_init_expr resets the flag. */ - gfc_init_expr_flag = true; + gfc_init_expr_flag = saved_init_expr_flag; } else return array_
[PATCH] Fortran: set shape of initializers of zero-sized arrays [PR95374,PR104352]
Dear all, the attached patch is neat, because it fixes a bug by removing code ;-) When generating the initializer for a parameter array, we excepted the case of size 0, which however prevented the detection of array bounds violations and lead to ICEs in various places. The solution which removes the comparison for size > 0 also has the bonus that it fixes a minor memory leak for the size==0 case... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 9d2995d2c1cf5708e3297fc7cffb5184d45a65cb Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 17 May 2023 20:39:18 +0200 Subject: [PATCH] Fortran: set shape of initializers of zero-sized arrays [PR95374,PR104352] gcc/fortran/ChangeLog: PR fortran/95374 PR fortran/104352 * decl.cc (add_init_expr_to_sym): Set shape of initializer also for zero-sized arrays, so that bounds violations can be detected later. gcc/testsuite/ChangeLog: PR fortran/95374 PR fortran/104352 * gfortran.dg/zero_sized_13.f90: New test. --- gcc/fortran/decl.cc | 3 +-- gcc/testsuite/gfortran.dg/zero_sized_13.f90 | 28 + 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/zero_sized_13.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index 9c4b40d4ac4..4c578d01ad4 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -2239,8 +2239,7 @@ add_init_expr_to_sym (const char *name, gfc_expr **initp, locus *var_locus) && gfc_is_constant_expr (init) && (init->expr_type == EXPR_CONSTANT || init->expr_type == EXPR_STRUCTURE) - && spec_size (sym->as, &size) - && mpz_cmp_si (size, 0) > 0) + && spec_size (sym->as, &size)) { array = gfc_get_array_expr (init->ts.type, init->ts.kind, &init->where); diff --git a/gcc/testsuite/gfortran.dg/zero_sized_13.f90 b/gcc/testsuite/gfortran.dg/zero_sized_13.f90 new file mode 100644 index 000..4035d458b32 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/zero_sized_13.f90 @@ -0,0 +1,28 @@ +! { dg-do compile } +! { dg-options "-w" } +! +! PR fortran/95374 +! PR fortran/104352 - Various ICEs for bounds violation with zero-sized arrays +! +! Contributed by G. Steinmetz + +program p + implicit none + integer :: i + integer, parameter :: a(0)= 0 + integer, parameter :: b(0:-5) = 0 + integer, parameter :: c(*) = [(a(i:i), i=0,0)] ! { dg-error "out of bounds" } + integer, parameter :: d(*) = [(b(i:i), i=1,1)] ! { dg-error "out of bounds" } + integer, parameter :: e(1) = [(a(i) , i=1,1)] ! { dg-error "out of bounds" } + integer, parameter :: f(1) = [(a(i:i), i=1,1)] ! { dg-error "out of bounds" } + integer:: g(1) = [(a(i:i), i=0,0)] ! { dg-error "out of bounds" } + integer:: h(1) = [(a(i:i), i=1,1)] ! { dg-error "out of bounds" } + print *, [(a(i:i), i=0,0)] ! { dg-error "out of bounds" } + print *, [(a(i:i), i=1,1)] ! { dg-error "out of bounds" } + print *, any (a(1:1) == 1) ! { dg-error "out of bounds" } + print *, all (a(0:0) == 1) ! { dg-error "out of bounds" } + print *, sum (a(1:1)) ! { dg-error "out of bounds" } + print *, iall (a(0:0)) ! { dg-error "out of bounds" } + print *, minloc (a(0:0),1) ! { dg-error "out of bounds" } + print *, dot_product(a(1:1),a(1:1)) ! { dg-error "out of bounds" } +end -- 2.35.3
[PATCH] Fortran: CLASS pointer function result in variable definition context [PR109846]
Dear all, Fortran allows functions in variable definition contexts when the result variable is a pointer. We already handle this for the non-CLASS case (in 11+), but the logic that checks the pointer attribute was looking in the wrong place for the CLASS case. Once found, the fix is simple and obvious, see attached patch. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 6406f19855a3b664597d75369f0935d3d31384dc Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 14 May 2023 21:53:51 +0200 Subject: [PATCH] Fortran: CLASS pointer function result in variable definition context [PR109846] gcc/fortran/ChangeLog: PR fortran/109846 * expr.cc (gfc_check_vardef_context): Check appropriate pointer attribute for CLASS vs. non-CLASS function result in variable definition context. gcc/testsuite/ChangeLog: PR fortran/109846 * gfortran.dg/ptr-func-5.f90: New test. --- gcc/fortran/expr.cc | 2 +- gcc/testsuite/gfortran.dg/ptr-func-5.f90 | 39 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/ptr-func-5.f90 diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index d91722e6ac6..09a16c9b367 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -6256,7 +6256,7 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, bool alloc_obj, && !(sym->attr.flavor == FL_PROCEDURE && sym == sym->result) && !(sym->attr.flavor == FL_PROCEDURE && sym->attr.proc_pointer) && !(sym->attr.flavor == FL_PROCEDURE - && sym->attr.function && sym->attr.pointer)) + && sym->attr.function && attr.pointer)) { if (context) gfc_error ("%qs in variable definition context (%s) at %L is not" diff --git a/gcc/testsuite/gfortran.dg/ptr-func-5.f90 b/gcc/testsuite/gfortran.dg/ptr-func-5.f90 new file mode 100644 index 000..05fd56703ca --- /dev/null +++ b/gcc/testsuite/gfortran.dg/ptr-func-5.f90 @@ -0,0 +1,39 @@ +! { dg-do compile } +! PR fortran/109846 +! CLASS pointer function result in variable definition context + +module foo + implicit none + type :: parameter_list + contains +procedure :: sublist, sublist_nores + end type +contains + function sublist (this) result (slist) +class(parameter_list), intent(inout) :: this +class(parameter_list), pointer :: slist +allocate (slist) + end function + function sublist_nores (this) +class(parameter_list), intent(inout) :: this +class(parameter_list), pointer :: sublist_nores +allocate (sublist_nores) + end function +end module + +program example + use foo + implicit none + type(parameter_list) :: plist + call sub1 (plist%sublist()) + call sub1 (plist%sublist_nores()) + call sub2 (plist%sublist()) + call sub2 (plist%sublist_nores()) +contains + subroutine sub1 (plist) +type(parameter_list), intent(inout) :: plist + end subroutine + subroutine sub2 (plist) +type(parameter_list) :: plist + end subroutine +end program -- 2.35.3
Re: [Patch, fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
On 5/9/23 20:29, Steve Kargl via Gcc-patches wrote: On Tue, May 09, 2023 at 08:24:16PM +0200, Harald Anlauf wrote: Hi Paul, On 5/9/23 17:51, Paul Richard Thomas via Gcc-patches wrote: Hi All, Thanks to Steve Kargl for the fix. It caused finalize_8.f03 to fail because this testcase checked that finalizable derived types could not be specified in a submodule. I have replaced the original test with a test of the patch. Thanks also to Malcolm Cohen for guidance on this. OK for trunk? the patch looks good to me. However: @@ -11637,8 +11637,9 @@ gfc_match_final_decl (void) block = gfc_state_stack->previous->sym; ^ See below. gcc_assert (block); - if (!gfc_state_stack->previous || !gfc_state_stack->previous->previous - || gfc_state_stack->previous->previous->state != COMP_MODULE) + if (gfc_state_stack->previous->previous + && gfc_state_stack->previous->previous->state != COMP_MODULE + && gfc_state_stack->previous->previous->state != COMP_SUBMODULE) { gfc_error ("Derived type declaration with FINAL at %C must be in the" " specification part of a MODULE"); I am wondering if we should keep the protection against a potential NULL pointer dereference (i.e. gfc_state_stack->previous == NULL) for possibly invalid code. I have failed to produce a simple testcase, but others may have "better" ideas. It's not needed. See above. gfc_state_stack->previous is referenced a few lines above the if-stmt. The reference will segfault if the pointer is NULL. You're absolutely right. So it is OK as is.
Re: [Patch, fortran] PR103716 - [10/11/12/13/14 Regression] ICE in gimplify_expr, at gimplify.c:15964
Hi Paul, On 5/9/23 18:00, Paul Richard Thomas via Gcc-patches wrote: Hi All, This problem caused the gimplifier failure because the reference chain ending in an inquiry_len still retained a full array reference. This had already been corrected for deferred character lengths but the fix extends this to all characters without a length expression and integer expressions, which is the correct type of course, that retain a full array_spec. The nullification of the se->string length in conv_inquiry is a belts-and-braces measure to stop it from winding up as a hidden argument in procedure calls. OK for trunk and, after a decent delay, backporting? ENOTESTCASE. Nevertheless the patch LGTM and is also OK for backporting. Thanks for fixing this! Harald Cheers Paul Fortran: Fix assumed length chars and len inquiry [PR103716] 2023-05-09 Paul Thomas gcc/fortran PR fortran/103716 * resolve.cc (gfc_resolve_ref): Conversion of array_ref into an element should be done for all characters without a len expr, not just deferred lens, and for integer expressions. * trans-expr.cc (conv_inquiry): For len and kind inquiry refs, set the se string_length to NULL_TREE. gcc/testsuite/ PR fortran/103716 * gfortran.dg/pr103716 : New test.
Re: [Patch, fortran] PR97122 - Spurious FINAL ... must be in the specification part of a MODULE
Hi Paul, On 5/9/23 17:51, Paul Richard Thomas via Gcc-patches wrote: Hi All, Thanks to Steve Kargl for the fix. It caused finalize_8.f03 to fail because this testcase checked that finalizable derived types could not be specified in a submodule. I have replaced the original test with a test of the patch. Thanks also to Malcolm Cohen for guidance on this. OK for trunk? the patch looks good to me. However: @@ -11637,8 +11637,9 @@ gfc_match_final_decl (void) block = gfc_state_stack->previous->sym; gcc_assert (block); - if (!gfc_state_stack->previous || !gfc_state_stack->previous->previous - || gfc_state_stack->previous->previous->state != COMP_MODULE) + if (gfc_state_stack->previous->previous + && gfc_state_stack->previous->previous->state != COMP_MODULE + && gfc_state_stack->previous->previous->state != COMP_SUBMODULE) { gfc_error ("Derived type declaration with FINAL at %C must be in the" " specification part of a MODULE"); I am wondering if we should keep the protection against a potential NULL pointer dereference (i.e. gfc_state_stack->previous == NULL) for possibly invalid code. I have failed to produce a simple testcase, but others may have "better" ideas. I'll leave it to you to amend the patch or leave as is. Thanks, Harald Paul Fortran: Allow declaration of finalizable DT in a submodule [PR97122] 2023-05-09 Paul Thomas Steven G. Kargl gcc/fortran PR fortran/97122 * decl.cc (variable_decl): Clean up white space issues. (gfc_match_final_decl): Declaration of finalizable derived type is allowed in a submodule. gcc/testsuite/ PR fortran/97122 * gfortran.dg/finalize_8.f03 : Replace testcase that checks declaration of finalizable derived types in submodules works.
Re: [patch, fortran] PR109662 Namelist input with comma after name accepted
Steve, On 5/8/23 02:13, Steve Kargl via Gcc-patches wrote: Harald, Thanks for keeping us honest. I didn't check what other separators might cause a problem. After 2 decades of working on gfortran, I've come to conclusion that -std=f2018 should be the default. When f2023 is ratified, the default becomes -std=f2023. All GNU fortran extension should be behind an option, and we should be aggressive eliminating extensions. Yes, this means that 'real*4' and similar would require a -fallow-nonstandard-declaration option. please don't let us get off-topic. The issue behind the PR was F2018: 13.11.3.1, Namelist input, which has Input for a namelist input statement consists of (1) optional blanks and namelist comments, (2) the character & followed immediately by the namelist-group-name as specified in the NAMELIST statement, (3) one or more blanks, where "blanks" was to be interpreted. Separators are discussed separately. Jerry has resolved "," and ";". Good. There is another weird issue that is visible in the testcase output in my previous mail for "!". Reducing that further now suggests that the EOF condition of the namelist read of the single line affects the namelist read of the next multi-line read. So this one is actually a different bug, likely libgfortran's internal state.
Re: [patch, fortran] PR109662 Namelist input with comma after name accepted
Hi Jerry, I've made a small compiler survey how they behave on namelist read from an internal unit when: 1.) there is a single input line of the type "&stuff" // testchar // " n = 666/" 2.) the input spans 2 lines split after the testchar 3.) same as 2.) but first line right-adjusted See attached source code. Competitors: Intel, NAG, NVidia, gfortran at r14-547 with -std=f2018. My findings were (last column is iostat, next-to-last is n read or -1): NAG: Compiler version = NAG Fortran Compiler Release 7.1(Hanzomon) Build 7101 1-line: > < 666 0 2-line/left: > < 666 0 2-line/right: > < 666 0 1-line: >!< -1 187 2-line/left: >!< -1 187 2-line/right: >!< -1 187 1-line: >/< -1 187 2-line/left: >/< -1 187 2-line/right: >/< -1 187 1-line: >,< -1 187 2-line/left: >,< -1 187 2-line/right: >,< -1 187 1-line: >;< -1 187 2-line/left: >;< -1 187 2-line/right: >;< -1 187 1-line: tab 666 0 2-line/left: tab 666 0 2-line/right: tab 666 0 1-line: lf -1 187 2-line/left: lf -1 187 2-line/right: lf -1 187 1-line: ret -1 187 2-line/left: ret -1 187 2-line/right: ret -1 187 My interpretation of this is that NAG treats tab as (white)space, everything else gives an error. This is the strictest compiler. Intel: Compiler version = Intel(R) Fortran Intel(R) 64 Compiler Classic for applications running on Intel(R) 64, Version 2021.9.0 Build 20230302_00 1-line: > < 666 0 2-line/left: > < 666 0 2-line/right: > < 666 0 1-line: >!< -1 -1 2-line/left: >!< 666 0 2-line/right: >!< 666 0 1-line: >/< -1 0 2-line/left: >/< -1 0 2-line/right: >/< -1 0 1-line: >,< -1 17 2-line/left: >,< -1 17 2-line/right: >,< -1 17 1-line: >;< -1 17 2-line/left: >;< -1 17 2-line/right: >;< -1 17 1-line: tab 666 0 2-line/left: tab 666 0 2-line/right: tab 666 0 1-line: lf 666 0 2-line/left: lf 666 0 2-line/right: lf 666 0 1-line: ret -1 17 2-line/left: ret -1 17 2-line/right: ret -1 17 Nvidia: Compiler version = nvfortran 23.3-0 1-line: > < 6660 2-line/left: > < 6660 2-line/right: > < 6660 1-line: >!< -1 -1 2-line/left: >!< -1 -1 2-line/right: >!< -1 -1 1-line: >/< -1 -1 2-line/left: >/< -1 -1 2-line/right: >/< -1 -1 1-line: >,< -1 -1 2-line/left: >,< -1 -1 2-line/right: >,< -1 -1 1-line: >;< -1 -1 2-line/left: >;< -1 -1 2-line/right: >;< -1 -1 1-line: tab 6660 2-line/left: tab 6660 2-line/right: tab 6660 1-line: lf -1 -1 2-line/left: lf 6660 2-line/right: lf 6660 1-line: ret 6660 2-line/left: ret 6660 2-line/right: ret 6660 gfortran (see above): Compiler version = GCC version 14.0.0 20230506 (experimental) 1-line: > < 666 0 2-line/left: > < 666 0 2-line/right: > < 666 0 1-line: >!< -1 -1 2-line/left: >!< -1 0 2-line/right: >!< 666 0 1-line: >/< -1 0 2-line/left: >/< -1 0 2-line/right: >/< -1 0 1-line: >,< 6665010 2-line/left: >,< 6665010 2-line/right: >,< 6665010 1-line: >;< 666 0 2-line/left: >;< 666 0 2-line/right: >;< 666 0 1-line: tab 666 0 2-line/left: tab 666 0 2-line/right: tab 666 0 1-line: lf 666 0 2-line/left: lf 666 0 2-line/right: lf 666 0 1-line: ret 666 0 2-line/left: ret 666 0 2-line/right: ret 666 0 So there seems to be a consensus that "," and ";" must be rejected, and tab is accepted (makes real sense), but already the termination character "/" and comment character "!" are treated differently. And how do we want to treat lf and ret in internal files with -std=f20xx? Cheers,
Re: [patch, fortran] PR109662 Namelist input with comma after name accepted
Hi Jerry, Steve, I think I have to pour a little water into the wine. The patch fixes the reported issue only for a comma after the namelist name, but we still accept a few other illegal characters, e.g. ';', because: #define is_separator(c) (c == '/' || c == ',' || c == '\n' || c == ' ' \ || c == '\t' || c == '\r' || c == ';' || \ (dtp->u.p.namelist_mode && c == '!')) We don't want that in standard conformance mode, or do we? Cheers, Harald On 5/6/23 06:02, Steve Kargl via Gcc-patches wrote: On Fri, May 05, 2023 at 08:41:48PM -0700, Jerry D via Fortran wrote: The attached patch adds a check for the invalid comma and emits a runtime error if -std=f95,f2003,f2018 are specified at compile time. Attached patch includes a new test case. Regression tested on x86_64-linux-gnu. OK for mainline? Yes. Thanks for the fix. It's been a long time since I looked at libgfortran code and couldn't quite determine where to start to fix this.
Re: [PATCH] Fortran: overloading of intrinsic binary operators [PR109641]
Hi Mikael, On 5/5/23 13:43, Mikael Morin wrote: Hello, Le 01/05/2023 à 18:29, Harald Anlauf via Fortran a écrit : +/* Given two expressions, check that their rank is conformable, i.e. either + both have the same rank or at least one is a scalar. */ + +bool +gfc_op_rank_conformable (gfc_expr *op1, gfc_expr *op2) +{ +// if (op1->expr_type == EXPR_VARIABLE && op1->ref) Please remove this, and the other one below. oops, that was a leftover from debugging sessions, which I missed during my final pass. Fixed and pushed as r14-529-g185da7c2014ba41f38dd62cc719873ebf020b076. Thanks for the review! Harald + if (op1->expr_type == EXPR_VARIABLE) + gfc_expression_rank (op1); +// if (op2->expr_type == EXPR_VARIABLE && op2->ref) + if (op2->expr_type == EXPR_VARIABLE) + gfc_expression_rank (op2); + + return (op1->rank == 0 || op2->rank == 0 || op1->rank == op2->rank); +} + + static void add_caf_get_intrinsic (gfc_expr *e) { The rest looks good. OK for master, and backport as well. Thanks Mikael
[PATCH] Fortran: overloading of intrinsic binary operators [PR109641]
Dear all, the attached patch is mostly self-explaining: we mishandled the overloading of intrinsic binary operators in the case the actual operands were of intrinsic numeric type and the ranks of the operands were not conformable, i.e. both were of non-zero and different ranks. In that case the operators could be converted to the same type before the correct user-defined operator was resolved, leading to either rejects-valid or accepts-invalid or wrong resolution (= wrong code). Regtested on x86_64-pc-linux-gnu. OK for mainline? The patch is actually very limited in impact, but the bug is sort of annoying. Would it be OK to backport to 13.2 after some waiting? Thanks, Harald From 50c8d3d4adeed1ecf44216075d1fb53a3ef0 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 1 May 2023 18:01:25 +0200 Subject: [PATCH] Fortran: overloading of intrinsic binary operators [PR109641] Fortran allows overloading of intrinsic operators also for operands of numeric intrinsic types. The intrinsic operator versions are used according to the rules of F2018 table 10.2 and imply type conversion as long as the operand ranks are conformable. Otherwise no type conversion shall be performed to allow the resolution of a matching user-defined operator. gcc/fortran/ChangeLog: PR fortran/109641 * arith.cc (eval_intrinsic): Check conformability of ranks of operands for intrinsic binary operators before performing type conversions. * gfortran.h (gfc_op_rank_conformable): Add prototype. * resolve.cc (resolve_operator): Check conformability of ranks of operands for intrinsic binary operators before performing type conversions. (gfc_op_rank_conformable): New helper function to compare ranks of operands of binary operator. gcc/testsuite/ChangeLog: PR fortran/109641 * gfortran.dg/overload_5.f90: New test. --- gcc/fortran/arith.cc | 6 ++ gcc/fortran/gfortran.h | 1 + gcc/fortran/resolve.cc | 39 gcc/testsuite/gfortran.dg/overload_5.f90 | 118 +++ 4 files changed, 164 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/overload_5.f90 diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index d1d814b3ae1..86d56406047 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -1663,6 +1663,12 @@ eval_intrinsic (gfc_intrinsic_op op, if (!gfc_numeric_ts (&op1->ts) || !gfc_numeric_ts (&op2->ts)) goto runtime; + /* Do not perform conversions if operands are not conformable as + required for the binary intrinsic operators (F2018:10.1.5). + Defer to a possibly overloading user-defined operator. */ + if (!gfc_op_rank_conformable (op1, op2)) + goto runtime; + /* Insert any necessary type conversions to make the operands compatible. */ diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index a15ff90e228..ac21e1813d9 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3730,6 +3730,7 @@ void gfc_free_association_list (gfc_association_list *); /* resolve.cc */ void gfc_expression_rank (gfc_expr *); +bool gfc_op_rank_conformable (gfc_expr *, gfc_expr *); bool gfc_resolve_ref (gfc_expr *); bool gfc_resolve_expr (gfc_expr *); void gfc_resolve (gfc_namespace *); diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index c3d508fb45d..341909d7de7 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -4200,6 +4200,17 @@ resolve_operator (gfc_expr *e) case INTRINSIC_POWER: if (gfc_numeric_ts (&op1->ts) && gfc_numeric_ts (&op2->ts)) { + /* Do not perform conversions if operands are not conformable as + required for the binary intrinsic operators (F2018:10.1.5). + Defer to a possibly overloading user-defined operator. */ + if (!gfc_op_rank_conformable (op1, op2)) + { + dual_locus_error = true; + snprintf (msg, sizeof (msg), + _("Inconsistent ranks for operator at %%L and %%L")); + goto bad_op; + } + gfc_type_convert_binary (e, 1); break; } @@ -4372,6 +4383,17 @@ resolve_operator (gfc_expr *e) if (gfc_numeric_ts (&op1->ts) && gfc_numeric_ts (&op2->ts)) { + /* Do not perform conversions if operands are not conformable as + required for the binary intrinsic operators (F2018:10.1.5). + Defer to a possibly overloading user-defined operator. */ + if (!gfc_op_rank_conformable (op1, op2)) + { + dual_locus_error = true; + snprintf (msg, sizeof (msg), + _("Inconsistent ranks for operator at %%L and %%L")); + goto bad_op; + } + gfc_type_convert_binary (e, 1); e->ts.type = BT_LOGICAL; @@ -5644,6 +5666,23 @@ done: } +/* Given two expressions, check that their rank is conformable, i.e. either + both have the same rank or at least one is a scalar. */ + +bool +gfc_op_rank_conformable (gfc_expr *op1, gfc_expr *op2) +{ +// if (op1->expr_type == EXPR_VARIABLE && op1->ref) + if (op1->expr_type == EXPR_VARIABLE) +gfc
Re: [Patch, fortran] PRs 105152, 100193, 87946, 103389, 104429 and 82774
Hi Paul, Am 22.04.23 um 10:32 schrieb Paul Richard Thomas via Gcc-patches: Hi All, As usual, I received a string of emails on retargeting for PRs for which I was either responsible or was on the cc list. This time I decided to take a look at them all, in order to reward the tireless efforts of Richi, Jakub and Martin with some attention at least. I have fixed the PRs in the title line: See the attached changelog, patch and testcases. OK for 14-branch? the patch looks essentially good to me. Can you please have a look at testcase pr100193.f90, which fails for me because the module file is not generated and there is no corresponding dg-pattern: FAIL: gfortran.dg/pr100193.f90 -O (test for excess errors) === gfortran Summary === # of expected passes1 # of unexpected failures1 You could either simply omit the main program or add a pattern. (The shortened testcase would still fail w/o the patch.) Of the others: PR100815 - fixed already for 12-branch on. Martin located the fix from Tobias, for which thanks. It's quite large but has stood the test of time. Should I backport to 11-branch? PR103366 - fixed on 12-branch on. I closed it. PR103715 - might be fixed but the report is for gcc with checking enabled. I will give that a go. PR103716 - a gimple problem with assumed shape characters. A TODO. PR103931 - I couldn't reproduce the bug, which involves 'ambiguous c_ptr'. To judge by the comments, it seems that this bug is a bit elusive. PR65381 - Seems to be fixed for 12-branch on PR82064 - Seems to be fixed. PR83209 - Coarray allocation - seems to be fixed. PR84244 - Coarray segfault. I have no acquaintance with the inner works of coarrays and so don't think that I can fix this one. PR87674 - Segfault in runtime with non-overridable proc-pointer. A TODO. PR96087 - A module procedure problem. A TODO. I have dejagnu-ified testcases for the already fixed PRs ready to go. Should these be committed or do we assume that the fixes already provided adequate tests? I think this depends. A testcase that is "sufficiently orthogonal" to those in the testsuite may be worth to be added. Otherwise randomly adding testcases might just increase the runtime for regression testing, which could be counter-productive for the development process. So better really decide on a case-by-case basis? (Of course this is only my opinion, and other may have a different view upon this.) I checked PR100815. The testcase in comment#0 appears to work for me for all open branches (10 to 14). The commit that supposedly fixed the issue applies to 12-branch and newer. Either there is something else in 11-branch which fixed it in a different way, or the bisecting unfortunately pointed to the wrong commit. And since there is no traceback information in the PR, I am simply confused. So do you think this testcase improves coverage and thus adds value? PR103715: with valgrind I get invalid reads, so I guess there is something lurking here and it only appears to be fixed. PR103931: it is indeed a bit elusive, but very sensitive to code changes. Also Bernhard had a look at it. Given that there are a couple of bugs related to module reading, and rename-on-use, I'd recommend to leave that open for further analysis. PR65381: works for me even on 11-branch. I think this looks very much like a duplicate of a PR that was fixed by Tobias. Still fails on 10-branch, but might not be worth fixing there. Simply close it as 10-only? PR103716: I think that one is interesting, as there are a couple of PRs involving inquiry functions. Regards Paul Cheers, Harald
Re: [PATCH] Fortran: function results never have the ALLOCATABLE attribute [PR109500]
Hi Mikael, Am 22.04.23 um 11:25 schrieb Mikael Morin: Hello, Le 20/04/2023 à 22:01, Harald Anlauf via Fortran a écrit : Dear all, Fortran 2018 added a clarification that the *result* of a function whose result *variable* has the ALLOCATABLE attribute is a *value* that itself does not have the ALLOCATABLE attribute. For those interested: there was a thread on the J3 mailing list some time ago (for links see the PR). The patch which implements a related check was co-authored with Steve and regtested by him. Testcase verified against NAG. OK for mainline (gcc-14)? Looks good in principle, but I think the real fix should be in the gfc_expr_attr function, which copies all the attributes (including allocatable) in the EXPR_FUNCTION case. How would the testsuite react if that attribute was cleared there? Is your patch still needed if gfc_expr_attr is fixed? you mean like the following? diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 00d35a71770..7517efc5414 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -2775,6 +2775,7 @@ gfc_expr_attr (gfc_expr *e) attr.pointer = CLASS_DATA (sym)->attr.class_pointer; attr.allocatable = CLASS_DATA (sym)->attr.allocatable; } + attr.allocatable = 0; } else if (e->value.function.isym && e->value.function.isym->transformational While this leads to a rejection of the testcase, I see regressions e.g. on allocatable_function_1.f90 and allocatable_function_8.f90 because the function result from a previous invocation does not get freed, and on a subsequent function reference the result variable should always be unallocated. Not sure if the "catch-22" Steve mentions is a good characterization, but a function reference with assignment of the result to an (allocatable) variable, like integer, allocatable :: p p = f() is semantically different from an ordinary assignment to an allocatable variable, where the r.h.s. is an allocatable variable, because the function result variable *must* be deallocated after the assignment, whereas an ordinary variable on the r.h.s must remain unaltered. So I guess it is much less risky to approach the issue by not allowing argument association to an allocatable dummy for an actual argument that is a function reference. (I initially had an even stricter idea to allow only an allocatable *variable* for the actual argument, but did not check the lengthy text on argument association). Mikael Harald
[PATCH] Fortran: function results never have the ALLOCATABLE attribute [PR109500]
Dear all, Fortran 2018 added a clarification that the *result* of a function whose result *variable* has the ALLOCATABLE attribute is a *value* that itself does not have the ALLOCATABLE attribute. For those interested: there was a thread on the J3 mailing list some time ago (for links see the PR). The patch which implements a related check was co-authored with Steve and regtested by him. Testcase verified against NAG. OK for mainline (gcc-14)? Thanks, Harald & Steve From 2cebc8f9e7b399b7747c9ad0392831de91851b5b Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 20 Apr 2023 21:47:34 +0200 Subject: [PATCH] Fortran: function results never have the ALLOCATABLE attribute [PR109500] Fortran 2018 8.5.3 (ALLOCATABLE attribute) explains in Note 1 that the result of referencing a function whose result variable has the ALLOCATABLE attribute is a value that does not itself have the ALLOCATABLE attribute. gcc/fortran/ChangeLog: PR fortran/109500 * interface.cc (gfc_compare_actual_formal): Reject allocatable functions being used as actual argument for allocable dummy. gcc/testsuite/ChangeLog: PR fortran/109500 * gfortran.dg/allocatable_function_11.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/interface.cc | 12 +++ .../gfortran.dg/allocatable_function_11.f90 | 36 +++ 2 files changed, 48 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/allocatable_function_11.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index e9843e9549c..968ee193c07 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -3638,6 +3638,18 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, goto match; } + if (a->expr->expr_type == EXPR_FUNCTION + && a->expr->value.function.esym + && f->sym->attr.allocatable) + { + if (where) + gfc_error ("Actual argument for %qs at %L is a function result " + "and the dummy argument is ALLOCATABLE", + f->sym->name, &a->expr->where); + ok = false; + goto match; + } + /* Check intent = OUT/INOUT for definable actual argument. */ if (!in_statement_function && (f->sym->attr.intent == INTENT_OUT diff --git a/gcc/testsuite/gfortran.dg/allocatable_function_11.f90 b/gcc/testsuite/gfortran.dg/allocatable_function_11.f90 new file mode 100644 index 000..1a2831e186f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocatable_function_11.f90 @@ -0,0 +1,36 @@ +! { dg-do compile } +! PR fortran/109500 - check F2018:8.5.3 Note 1 +! +! The result of referencing a function whose result variable has the +! ALLOCATABLE attribute is a value that does not itself have the +! ALLOCATABLE attribute. + +program main + implicit none + integer, allocatable :: p + procedure(f), pointer :: pp + pp => f + p = f() + print *, allocated (p) + print *, is_allocated (p) + print *, is_allocated (f()) ! { dg-error "is a function result" } + print *, is_allocated (pp()) ! { dg-error "is a function result" } + call s (p) + call s (f()) ! { dg-error "is a function result" } + call s (pp()) ! { dg-error "is a function result" } + +contains + subroutine s(p) +integer, allocatable :: p + end subroutine s + + function f() +integer, allocatable :: f +allocate (f, source=42) + end function + + logical function is_allocated(p) +integer, allocatable :: p +is_allocated = allocated(p) + end function +end program -- 2.35.3
Re: [PATCH] testsuite: fix scan-tree-dump patterns [PR83904, PR100297]
On 4/19/23 17:14, Bernhard Reutner-Fischer via Gcc-patches wrote: On Wed, 19 Apr 2023 at 03:03, Jerry D via Fortran wrote: On 4/18/23 12:39 PM, Harald Anlauf via Fortran wrote: Dear all, the attached patch adjusts the scan-tree-dump patterns of the reported testcases which likely were run in a location such that a path in an error message showing in the tree-dump might have accidentally matched "free" or "data", respectively. For the testcase gfortran.dg/reshape_8.f90 I checked with a failing gfortran-11 that the pattern is appropriate. OK for mainline? Thanks, Harald Yes, OK I'm certainly not opposed to this specific incarnation of such a fix. These failures are really unpleasant :) As proposed in https://inbox.sourceware.org/gcc-patches/20220426010029.2b476337@nbbrfq/ we could add a -fno-file to suppress the assembler .file output (whatever the prefix looks like depends on the assembler dialect). Or we could nuke the .file directives by a sed(1), but that would probably be cumbersome for remote targets. I don't have a better idea than -fno-file or -ffile=foo.c . Fixing them case-by-case does not scale all that well IMHO. Thoughts? ? It wasn't the tree-dumps being at fault, it was the scan patterns.
[PATCH] testsuite: fix scan-tree-dump patterns [PR83904,PR100297]
Dear all, the attached patch adjusts the scan-tree-dump patterns of the reported testcases which likely were run in a location such that a path in an error message showing in the tree-dump might have accidentally matched "free" or "data", respectively. For the testcase gfortran.dg/reshape_8.f90 I checked with a failing gfortran-11 that the pattern is appropriate. OK for mainline? Thanks, Harald From ad7ea82929f65ef34a13dea5a0fe23d567f220e8 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 18 Apr 2023 21:24:20 +0200 Subject: [PATCH] testsuite: fix scan-tree-dump patterns [PR83904,PR100297] Adjust scan-tree-dump patterns so that they do not accidentally match a valid path. gcc/testsuite/ChangeLog: PR testsuite/83904 PR fortran/100297 * gfortran.dg/allocatable_function_1.f90: Use "__builtin_free " instead of the naive "free". * gfortran.dg/reshape_8.f90: Extend pattern from a simple "data". --- gcc/testsuite/gfortran.dg/allocatable_function_1.f90 | 2 +- gcc/testsuite/gfortran.dg/reshape_8.f90 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gfortran.dg/allocatable_function_1.f90 b/gcc/testsuite/gfortran.dg/allocatable_function_1.f90 index f96ebc499e8..e38953bd777 100644 --- a/gcc/testsuite/gfortran.dg/allocatable_function_1.f90 +++ b/gcc/testsuite/gfortran.dg/allocatable_function_1.f90 @@ -107,4 +107,4 @@ contains end function bar end program alloc_fun -! { dg-final { scan-tree-dump-times "free" 10 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_free " 10 "original" } } diff --git a/gcc/testsuite/gfortran.dg/reshape_8.f90 b/gcc/testsuite/gfortran.dg/reshape_8.f90 index 01799ac5c19..56812124cb8 100644 --- a/gcc/testsuite/gfortran.dg/reshape_8.f90 +++ b/gcc/testsuite/gfortran.dg/reshape_8.f90 @@ -11,4 +11,4 @@ program test a = reshape([1,2,3,4], [2,0]) print *, a end -! { dg-final { scan-tree-dump-times "data" 4 "original" } } +! { dg-final { scan-tree-dump-not "data..0. =" "original" } } -- 2.35.3
Re: [PATCH] Fortran: fix compile-time simplification of SET_EXPONENT [PR109511]
Hi Steve, On 4/14/23 21:33, Steve Kargl via Gcc-patches wrote: On Fri, Apr 14, 2023 at 08:59:24PM +0200, Harald Anlauf via Fortran wrote: the compile-time simplification of intrinsic SET_EXPONENT was broken since the early days of gfortran for argument X < 1 (including negative X) and for I < 0. I identified and fixed several issues in the implementation. The testcase explores argument space comparing compile-time and runtime results and is checked against Intel. Regtested on x86_64-pc-linux-gnu. OK for mainline? Yes, it is certainly better than the current situation. This is not a regression, but can lead to wrong code. Would it be OK to backport to open branches? Sure. Looks simply and fairly specific. OK, thanks, will proceed. I was wondering about the difference between set_exponent() and scale(), and found that set_exponent() talks about IEEE values while scale() doesn't. I'm wondering if we should add the IEEE special cases to the testsuite. Of particular note, I doubt that this is true: If X is an IEEE NaN, the result is the same NaN. program foo real x, y x = 1 y = x - x x = (x - x) / y print '(F4.0,1X,Z8.8)', x, x y = set_exponent(x,1) print '(F4.0,1X,Z8.8)', y, y end program foo gfcx -o z a.f90 && ./z NaN FFC0 NaN 7FC0 Those are not the same NaN. The second is a qNaN. The first looks like a qNaN with the sign bit set. Until now there was no testing at all of SET_EXPONENT in the testsuite. It would be really good to have better coverage of compile-time and runtime behavior of the intrinsics and checking consistency ... ;-) I think you have much more experience in that area. (Hint!) Cheers, Harald
[PATCH] Fortran: fix compile-time simplification of SET_EXPONENT [PR109511]
[Now with subject added...] Dear all, the compile-time simplification of intrinsic SET_EXPONENT was broken since the early days of gfortran for argument X < 1 (including negative X) and for I < 0. I identified and fixed several issues in the implementation. The testcase explores argument space comparing compile-time and runtime results and is checked against Intel. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is not a regression, but can lead to wrong code. Would it be OK to backport to open branches? Thanks, Harald From fa4cb42870df60debdbd51e2ddc6d6ab9e6a Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 14 Apr 2023 20:45:19 +0200 Subject: [PATCH] Fortran: fix compile-time simplification of SET_EXPONENT [PR109511] gcc/fortran/ChangeLog: PR fortran/109511 * simplify.cc (gfc_simplify_set_exponent): Fix implementation of compile-time simplification of intrinsic SET_EXPONENT for argument X < 1 and for I < 0. gcc/testsuite/ChangeLog: PR fortran/109511 * gfortran.dg/set_exponent_1.f90: New test. --- gcc/fortran/simplify.cc | 12 +++ gcc/testsuite/gfortran.dg/set_exponent_1.f90 | 36 2 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/set_exponent_1.f90 diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index ecf0e3558df..b65854c1021 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -7364,7 +7364,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) { gfc_expr *result; mpfr_t exp, absv, log2, pow2, frac; - unsigned long exp2; + long exp2; if (x->expr_type != EXPR_CONSTANT || i->expr_type != EXPR_CONSTANT) return NULL; @@ -7396,19 +7396,19 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) mpfr_abs (absv, x->value.real, GFC_RND_MODE); mpfr_log2 (log2, absv, GFC_RND_MODE); - mpfr_trunc (log2, log2); + mpfr_floor (log2, log2); mpfr_add_ui (exp, log2, 1, GFC_RND_MODE); /* Old exponent value, and fraction. */ mpfr_ui_pow (pow2, 2, exp, GFC_RND_MODE); - mpfr_div (frac, absv, pow2, GFC_RND_MODE); + mpfr_div (frac, x->value.real, pow2, GFC_RND_MODE); /* New exponent. */ - exp2 = (unsigned long) mpz_get_d (i->value.integer); - mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE); + exp2 = mpz_get_si (i->value.integer); + mpfr_mul_2si (result->value.real, frac, exp2, GFC_RND_MODE); - mpfr_clears (absv, log2, pow2, frac, NULL); + mpfr_clears (absv, log2, exp, pow2, frac, NULL); return range_check (result, "SET_EXPONENT"); } diff --git a/gcc/testsuite/gfortran.dg/set_exponent_1.f90 b/gcc/testsuite/gfortran.dg/set_exponent_1.f90 new file mode 100644 index 000..4c063e8330b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/set_exponent_1.f90 @@ -0,0 +1,36 @@ +! { dg-do run } +! PR fortran/109511 +! Check compile-time simplification of SET_EXPONENT against runtime + +program exponent + implicit none + integer :: i + i = 0 + print *, i, set_exponent(1., 0), set_exponent(1., i) + if (set_exponent(1., 0) /= set_exponent(1., i)) stop 1 + i = 1 + print *, i, set_exponent(1., 1), set_exponent(1., i) + if (set_exponent(1., 1) /= set_exponent(1., i)) stop 2 + i = 2 + print *, i, set_exponent(-1.75, 2), set_exponent(-1.75, i) + if (set_exponent(-1.75, 2) /= set_exponent(-1.75, i)) stop 3 + print *, i, set_exponent(0.1875, 2), set_exponent(0.1875, i) + if (set_exponent(0.1875, 2) /= set_exponent(0.1875, i)) stop 4 + i = 3 + print *, i, set_exponent(0.75, 3), set_exponent(0.75, i) + if (set_exponent(0.75, 3) /= set_exponent(0.75, i)) stop 5 + i = 4 + print *, i, set_exponent(-2.5, 4), set_exponent(-2.5, i) + if (set_exponent(-2.5, 4) /= set_exponent(-2.5, i)) stop 6 + i = -1 + print *, i, set_exponent(1., -1), set_exponent(1., i) + if (set_exponent(1., -1) /= set_exponent(1., i)) stop 7 + i = -2 + print *, i, set_exponent(1.125, -2), set_exponent(1.125, i) + if (set_exponent(1.125, -2) /= set_exponent(1.125, i)) stop 8 + print *, i, set_exponent(-0.25, -2), set_exponent(-0.25, i) + if (set_exponent(-0.25, -2) /= set_exponent(-0.25, i)) stop 9 + i = -3 + print *, i, set_exponent(0.75, -3), set_exponent(0.75, i) + if (set_exponent(0.75, -3) /= set_exponent(0.75, i)) stop 10 +end program exponent -- 2.35.3
[no subject]
Dear all, the compile-time simplification of intrinsic SET_EXPONENT was broken since the early days of gfortran for argument X < 1 (including negative X) and for I < 0. I identified and fixed several issues in the implementation. The testcase explores argument space comparing compile-time and runtime results and is checked against Intel. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is not a regression, but can lead to wrong code. Would it be OK to backport to open branches? Thanks, Harald From fa4cb42870df60debdbd51e2ddc6d6ab9e6a Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 14 Apr 2023 20:45:19 +0200 Subject: [PATCH] Fortran: fix compile-time simplification of SET_EXPONENT [PR109511] gcc/fortran/ChangeLog: PR fortran/109511 * simplify.cc (gfc_simplify_set_exponent): Fix implementation of compile-time simplification of intrinsic SET_EXPONENT for argument X < 1 and for I < 0. gcc/testsuite/ChangeLog: PR fortran/109511 * gfortran.dg/set_exponent_1.f90: New test. --- gcc/fortran/simplify.cc | 12 +++ gcc/testsuite/gfortran.dg/set_exponent_1.f90 | 36 2 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/set_exponent_1.f90 diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index ecf0e3558df..b65854c1021 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -7364,7 +7364,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) { gfc_expr *result; mpfr_t exp, absv, log2, pow2, frac; - unsigned long exp2; + long exp2; if (x->expr_type != EXPR_CONSTANT || i->expr_type != EXPR_CONSTANT) return NULL; @@ -7396,19 +7396,19 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) mpfr_abs (absv, x->value.real, GFC_RND_MODE); mpfr_log2 (log2, absv, GFC_RND_MODE); - mpfr_trunc (log2, log2); + mpfr_floor (log2, log2); mpfr_add_ui (exp, log2, 1, GFC_RND_MODE); /* Old exponent value, and fraction. */ mpfr_ui_pow (pow2, 2, exp, GFC_RND_MODE); - mpfr_div (frac, absv, pow2, GFC_RND_MODE); + mpfr_div (frac, x->value.real, pow2, GFC_RND_MODE); /* New exponent. */ - exp2 = (unsigned long) mpz_get_d (i->value.integer); - mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE); + exp2 = mpz_get_si (i->value.integer); + mpfr_mul_2si (result->value.real, frac, exp2, GFC_RND_MODE); - mpfr_clears (absv, log2, pow2, frac, NULL); + mpfr_clears (absv, log2, exp, pow2, frac, NULL); return range_check (result, "SET_EXPONENT"); } diff --git a/gcc/testsuite/gfortran.dg/set_exponent_1.f90 b/gcc/testsuite/gfortran.dg/set_exponent_1.f90 new file mode 100644 index 000..4c063e8330b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/set_exponent_1.f90 @@ -0,0 +1,36 @@ +! { dg-do run } +! PR fortran/109511 +! Check compile-time simplification of SET_EXPONENT against runtime + +program exponent + implicit none + integer :: i + i = 0 + print *, i, set_exponent(1., 0), set_exponent(1., i) + if (set_exponent(1., 0) /= set_exponent(1., i)) stop 1 + i = 1 + print *, i, set_exponent(1., 1), set_exponent(1., i) + if (set_exponent(1., 1) /= set_exponent(1., i)) stop 2 + i = 2 + print *, i, set_exponent(-1.75, 2), set_exponent(-1.75, i) + if (set_exponent(-1.75, 2) /= set_exponent(-1.75, i)) stop 3 + print *, i, set_exponent(0.1875, 2), set_exponent(0.1875, i) + if (set_exponent(0.1875, 2) /= set_exponent(0.1875, i)) stop 4 + i = 3 + print *, i, set_exponent(0.75, 3), set_exponent(0.75, i) + if (set_exponent(0.75, 3) /= set_exponent(0.75, i)) stop 5 + i = 4 + print *, i, set_exponent(-2.5, 4), set_exponent(-2.5, i) + if (set_exponent(-2.5, 4) /= set_exponent(-2.5, i)) stop 6 + i = -1 + print *, i, set_exponent(1., -1), set_exponent(1., i) + if (set_exponent(1., -1) /= set_exponent(1., i)) stop 7 + i = -2 + print *, i, set_exponent(1.125, -2), set_exponent(1.125, i) + if (set_exponent(1.125, -2) /= set_exponent(1.125, i)) stop 8 + print *, i, set_exponent(-0.25, -2), set_exponent(-0.25, i) + if (set_exponent(-0.25, -2) /= set_exponent(-0.25, i)) stop 9 + i = -3 + print *, i, set_exponent(0.75, -3), set_exponent(0.75, i) + if (set_exponent(0.75, -3) /= set_exponent(0.75, i)) stop 10 +end program exponent -- 2.35.3
Re: [Patch, fortran] PR109451 - ICE in gfc_conv_expr_descriptor with ASSOCIATE and substrings
Hi Paul, On 4/14/23 10:18, Paul Richard Thomas via Gcc-patches wrote: Hi Harald, The fix was trivial. An updated patch and testcase are attached. great, this works, and I couldn't break it again this time ... Looks good! Thanks, Harald Thanks Paul Fortran: Fix some deferred character problems in associate [PR109451] 2023-04-14 Paul Thomas gcc/fortran PR fortran/109451 * trans-array.cc (gfc_conv_expr_descriptor): Guard expression character length backend decl before using it. Suppress the assignment if lhs equals rhs. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of associate variables pointing to a variable. Add comment. * trans-stmt.cc (trans_associate_var): Remove requirement that the character length be deferred before assigning the value returned by gfc_conv_expr_descriptor. Also, guard the backend decl before testing with VAR_P. gcc/testsuite/ PR fortran/109451 * gfortran.dg/associate_61.f90 : New test On Thu, 13 Apr 2023 at 07:18, Paul Richard Thomas < paul.richard.tho...@gmail.com> wrote: Hi Harald, That's interesting - the string length '.q' is not set for either of the associate blocks. I'm onto it. Thanks Paul On Wed, 12 Apr 2023 at 20:26, Harald Anlauf wrote: Hi Paul, On 4/12/23 17:25, Paul Richard Thomas via Gcc-patches wrote: Hi All, I think that the changelog says it all. OK for mainline? this looks almost fine, but still fails if one directly uses the dummy argument as the ASSOCIATE target, as in: program p implicit none character(4) :: c(2) = ["abcd","efgh"] call dcs0 (c) ! call dcs0 (["abcd","efgh"]) contains subroutine dcs0(a) character(len=*), intent(in) :: a(:) print *, size(a),len(a) associate (q => a(:)) print *, size(q),len(q) end associate associate (q => a(:)(:)) print *, size(q),len(q) end associate return end subroutine dcs0 end This prints e.g. 2 4 2 0 2 0 (sometimes I also get junk values for the character length). Can you please have another look? Thanks, Harald Paul Fortran: Fix some deferred character problems in associate [PR109451] 2023-04-07 Paul Thomas gcc/fortran PR fortran/109451 * trans-array.cc (gfc_conv_expr_descriptor): Guard expression character length backend decl before using it. Suppress the assignment if lhs equals rhs. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of associate variables pointing to a variable. Add comment. gcc/testsuite/ PR fortran/109451 * gfortran.dg/associate_61.f90 : New test -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, committed] Fortran: call of overloaded ‘abs(long long int&)’ is ambiguous [PR109492]
Dear all, I've committed the attached patch which fixes a portability issue when bootstrapping on Solaris. Discussed and confirmed in the PR by Jonathan for Solaris and regtested by me on x86_64-pc-linux-gnu. https://gcc.gnu.org/g:43816633afd275a9057232a6ebfdc19e441f09ec (Unfortunately the commit message contains Unicode characters that I got by using copy&paste of the error message. I wonder if "git gcc-verify" could have warned me ...) Thanks, Harald From 43816633afd275a9057232a6ebfdc19e441f09ec Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 13 Apr 2023 22:42:23 +0200 Subject: [PATCH] =?UTF-8?q?Fortran:=20call=20of=20overloaded=20=E2=80=98ab?= =?UTF-8?q?s(long=20long=20int&)=E2=80=99=20is=20ambiguous=20[PR109492]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gcc/fortran/ChangeLog: PR fortran/109492 * trans-expr.cc (gfc_conv_power_op): Use absu_hwi and unsigned HOST_WIDE_INT for portability. --- gcc/fortran/trans-expr.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 79367fa2ae0..09cdd9263c4 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -3400,11 +3400,12 @@ gfc_conv_power_op (gfc_se * se, gfc_expr * expr) && TREE_CODE (TREE_TYPE (rse.expr)) == INTEGER_TYPE) { wi::tree_to_wide_ref wlhs = wi::to_wide (lse.expr); - HOST_WIDE_INT v, w; + HOST_WIDE_INT v; + unsigned HOST_WIDE_INT w; int kind, ikind, bit_size; v = wlhs.to_shwi (); - w = abs (v); + w = absu_hwi (v); kind = expr->value.op.op1->ts.kind; ikind = gfc_validate_kind (BT_INTEGER, kind, false); -- 2.35.3
Re: [Patch, fortran] PR109451 - ICE in gfc_conv_expr_descriptor with ASSOCIATE and substrings
Hi Paul, On 4/12/23 17:25, Paul Richard Thomas via Gcc-patches wrote: Hi All, I think that the changelog says it all. OK for mainline? this looks almost fine, but still fails if one directly uses the dummy argument as the ASSOCIATE target, as in: program p implicit none character(4) :: c(2) = ["abcd","efgh"] call dcs0 (c) ! call dcs0 (["abcd","efgh"]) contains subroutine dcs0(a) character(len=*), intent(in) :: a(:) print *, size(a),len(a) associate (q => a(:)) print *, size(q),len(q) end associate associate (q => a(:)(:)) print *, size(q),len(q) end associate return end subroutine dcs0 end This prints e.g. 2 4 2 0 2 0 (sometimes I also get junk values for the character length). Can you please have another look? Thanks, Harald Paul Fortran: Fix some deferred character problems in associate [PR109451] 2023-04-07 Paul Thomas gcc/fortran PR fortran/109451 * trans-array.cc (gfc_conv_expr_descriptor): Guard expression character length backend decl before using it. Suppress the assignment if lhs equals rhs. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of associate variables pointing to a variable. Add comment. gcc/testsuite/ PR fortran/109451 * gfortran.dg/associate_61.f90 : New test
[PATCH] Fortran: fix functions with entry and pointer/allocatable result [PR104312]
Dear all, the testcase in the PR by Gerhard exhibited a mis-treatment of the function decl of the entry master if the function result had a pointer attribute and the translation unit was compiled with -ff2c. We actually should not use the peculiar special treatment for default-real functions in that case, as -ff2c is reserved for function results that can be expressed in Fortran77, and POINTER was not allowed in that standard. Same for complex. Furthermore, it turned out that ALLOCATABLE function results were not yet handled for functions with entries, even without -ff2c. Adding support for this was straightforward. I also fixed a potential buffer overflow for a generated internal symbol. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 60e81b97cf3715347de30ed4fd579be54fdb1997 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 11 Apr 2023 21:44:20 +0200 Subject: [PATCH] Fortran: fix functions with entry and pointer/allocatable result [PR104312] gcc/fortran/ChangeLog: PR fortran/104312 * resolve.cc (resolve_entries): Handle functions with ENTRY and ALLOCATABLE results. * trans-expr.cc (gfc_conv_procedure_call): Functions with a result with the POINTER or ALLOCATABLE attribute shall not get any special treatment with -ff2c, as they cannot be written in Fortran 77. * trans-types.cc (gfc_return_by_reference): Likewise. (gfc_get_function_type): Likewise. gcc/testsuite/ChangeLog: PR fortran/104312 * gfortran.dg/entry_26.f90: New test. * gfortran.dg/entry_27.f90: New test. --- gcc/fortran/resolve.cc | 19 +++- gcc/fortran/trans-expr.cc | 2 + gcc/fortran/trans-types.cc | 4 ++ gcc/testsuite/gfortran.dg/entry_26.f90 | 64 ++ gcc/testsuite/gfortran.dg/entry_27.f90 | 64 ++ 5 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/entry_26.f90 create mode 100644 gcc/testsuite/gfortran.dg/entry_27.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 6e42397c2ea..58013d48dff 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -702,7 +702,8 @@ resolve_entries (gfc_namespace *ns) gfc_code *c; gfc_symbol *proc; gfc_entry_list *el; - char name[GFC_MAX_SYMBOL_LEN + 1]; + /* Provide sufficient space to hold "master.%d.%s". */ + char name[GFC_MAX_SYMBOL_LEN + 1 + 18]; static int master_count = 0; if (ns->proc_name == NULL) @@ -827,6 +828,9 @@ resolve_entries (gfc_namespace *ns) "entries returning variables of different " "string lengths", ns->entries->sym->name, &ns->entries->sym->declared_at); + else if (el->sym->result->attr.allocatable + != ns->entries->sym->result->attr.allocatable) + break; } if (el == NULL) @@ -838,6 +842,8 @@ resolve_entries (gfc_namespace *ns) gfc_set_array_spec (proc, gfc_copy_array_spec (sym->as), NULL); if (sym->attr.pointer) gfc_add_pointer (&proc->attr, NULL); + if (sym->attr.allocatable) + gfc_add_allocatable (&proc->attr, NULL); } else { @@ -869,6 +875,17 @@ resolve_entries (gfc_namespace *ns) "FUNCTION %s at %L", sym->name, ns->entries->sym->name, &sym->declared_at); } + else if (sym->attr.allocatable) + { + if (el == ns->entries) + gfc_error ("FUNCTION result %s cannot be ALLOCATABLE in " + "FUNCTION %s at %L", sym->name, + ns->entries->sym->name, &sym->declared_at); + else + gfc_error ("ENTRY result %s cannot be ALLOCATABLE in " + "FUNCTION %s at %L", sym->name, + ns->entries->sym->name, &sym->declared_at); + } else { ts = &sym->ts; diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index f052d6b9440..79367fa2ae0 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -7800,6 +7800,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, */ if (flag_f2c && sym->ts.type == BT_REAL && sym->ts.kind == gfc_default_real_kind + && !sym->attr.pointer + && !sym->attr.allocatable && !sym->attr.always_explicit) se->expr = fold_convert (gfc_get_real_type (sym->ts.kind), se->expr); diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc index 9c9489a42bd..fc5c221a301 100644 --- a/gcc/fortran/trans-types.cc +++ b/gcc/fortran/trans-types.cc @@ -2962,6 +2962,8 @@ gfc_return_by_reference (gfc_symbol * sym) require an explicit interface, as no compatibility problems can arise there. */ if (flag_f2c && sym->ts.type == BT_COMPLEX + && !sym->attr.pointer + && !sym->attr.allocatable && !sym->attr.intrinsic && !sym->attr.always_explicit) return 1; @@ -3273,6 +3275,8 @@ arg_type_list_done: type = gfc_get_mixed_entry_union (sym->ns); else if (flag_f2c && sym->ts.type == BT_REAL && sym->ts.kind == gfc_default_real_kind + && !sym->attr.pointer + && !sym->attr.
[PATCH, v2] Fortran: resolve correct generic with TYPE(C_PTR) arguments [PR61615]
Hi Jerry, all, On 4/11/23 02:43, Jerry D via Gcc-patches wrote: On 4/10/23 1:49 PM, Harald Anlauf via Fortran wrote: Dear all, when comparing formal and actual arguments of a procedure, there was no check of rank for derived types from intrinsic module ISO_C_BINDING. This could lead to a wrong resolution of generic procedures with dummy argument of related types, see PR. This was likely an oversight. The attached fix is simple and regtests cleanly on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald Looks good to go. Jerry I actually found a flaw in the previous patch regarding the handling of rank, and also realized that a comparison of the types was missing for those from this intrinsic module (and found the related PR99982). I updated the patch accordingly and extended the testcase, see attached. Regtests cleanly on x86_64-pc-linux-gnu. Will wait for 24h for more comments. Thanks, Harald From 3fa9d2ee99afa38f42c267d17aed5266daa22dbc Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 11 Apr 2023 16:44:32 +0200 Subject: [PATCH] Fortran: resolve correct generic with TYPE(C_PTR) arguments [PR61615,PR99982] gcc/fortran/ChangeLog: PR fortran/61615 PR fortran/99982 * interface.cc (compare_parameter): Enable type and rank checks for arguments of derived type from the intrinsic module ISO_C_BINDING. gcc/testsuite/ChangeLog: PR fortran/61615 PR fortran/99982 * gfortran.dg/interface_49.f90: New test. --- gcc/fortran/interface.cc | 18 +++- gcc/testsuite/gfortran.dg/interface_49.f90 | 95 ++ 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/interface_49.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index db79b104dc2..e9843e9549c 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -2361,7 +2361,23 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, && formal->ts.u.derived && formal->ts.u.derived->ts.is_iso_c && actual->ts.type == BT_DERIVED && actual->ts.u.derived && actual->ts.u.derived->ts.is_iso_c) -return true; +{ + if (formal->ts.u.derived->intmod_sym_id + != actual->ts.u.derived->intmod_sym_id) + return false; + + if (ranks_must_agree + && symbol_rank (formal) != actual->rank + && symbol_rank (formal) != -1) + { + if (where) + argument_rank_mismatch (formal->name, &actual->where, +symbol_rank (formal), actual->rank, +NULL); + return false; + } + return true; +} if (formal->ts.type == BT_CLASS && actual->ts.type == BT_DERIVED) /* Make sure the vtab symbol is present when diff --git a/gcc/testsuite/gfortran.dg/interface_49.f90 b/gcc/testsuite/gfortran.dg/interface_49.f90 new file mode 100644 index 000..aef5e0c6609 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/interface_49.f90 @@ -0,0 +1,95 @@ +! { dg-do run } +! PR fortran/61615 - resolve correct generic with TYPE(C_PTR) arguments +! PR fortran/99982 - dto. with C_PTR and C_FUNPTR +! Contributed by Jacob Abel and Scot Breitenfeld + +MODULE foo + USE iso_c_binding, only : c_ptr, c_funptr + IMPLICIT NONE + integer :: rank = -99 + character(8) :: ctyp = "" + INTERFACE bar +MODULE PROCEDURE bar_s +MODULE PROCEDURE bar_a1d +MODULE PROCEDURE bar_a2d +MODULE PROCEDURE bar_fp +MODULE PROCEDURE bar_fp1 +MODULE PROCEDURE bar_fpx + END INTERFACE bar +CONTAINS + SUBROUTINE bar_s(a) +TYPE(c_ptr) :: a +WRITE (0, *) 'in bar_s' +rank = 0 +ctyp = "c_ptr" + END SUBROUTINE bar_s + + SUBROUTINE bar_a1d(a) +TYPE(c_ptr) :: a(:) +WRITE (0, *) 'in bar_a1d' +rank = 1 +ctyp = "c_ptr" + END SUBROUTINE bar_a1d + + SUBROUTINE bar_a2d(a) +TYPE(c_ptr) :: a(:,:) +WRITE (0, *) 'in bar_a2d' +rank = 2 +ctyp = "c_ptr" + END SUBROUTINE bar_a2d + + SUBROUTINE bar_fp(a) +TYPE(c_funptr) :: a +WRITE (0, *) 'in bar_fp' +rank = 0 +ctyp = "c_funptr" + END SUBROUTINE bar_fp + + SUBROUTINE bar_fp1(a) +TYPE(c_funptr) :: a(:) +WRITE (0, *) 'in bar_fp1' +rank = 1 +ctyp = "c_funptr" + END SUBROUTINE bar_fp1 + + SUBROUTINE bar_fpx(a, b) +TYPE(c_funptr) :: a(..) +TYPE(c_ptr):: b +WRITE (0, *) 'in bar_fpx' +rank = -1 +ctyp = "c_funptr" + END SUBROUTINE bar_fpx +END MODULE foo + +PROGRAM cptr_array_vs_scalar_arg + USE foo + USE iso_c_binding, only : c_ptr, c_loc, c_funptr + IMPLICIT NONE + INTEGER, TARGET :: i + TYPE(c_ptr) :: a, b(1), c(1,1) + type(c_funptr) :: fp, fp1(1), fp2(1,1) + a= C_LOC(i) + b(1) = C_LOC(i) + CALL bar(a) + if (rank /= 0 .or. ctyp /= "c_ptr") stop 1 + CALL bar(b) + if (rank /= 1 .or. ctyp /= "c_ptr") stop 2 + CALL bar(c) + if (rank /= 2 .or. ctyp /= "c_ptr") stop 3 + rank = -99 + ctyp = "" + CALL bar((a)) + if (rank /= 0 .or. ctyp /= "c_ptr") stop 4 + CALL bar((b)) + if (rank /= 1 .or. ctyp /= "c_ptr") stop 5 + rank = -99 + ctyp = "" + CALL bar
[PATCH] Fortran: resolve correct generic with TYPE(C_PTR) arguments [PR61615]
Dear all, when comparing formal and actual arguments of a procedure, there was no check of rank for derived types from intrinsic module ISO_C_BINDING. This could lead to a wrong resolution of generic procedures with dummy argument of related types, see PR. This was likely an oversight. The attached fix is simple and regtests cleanly on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From d41aa0f60b53799a5d28743f168fbf312461f51f Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 10 Apr 2023 22:39:52 +0200 Subject: [PATCH] Fortran: resolve correct generic with TYPE(C_PTR) arguments [PR61615] gcc/fortran/ChangeLog: PR fortran/61615 * interface.cc (compare_parameter): Enable rank check for arguments of derived type from the intrinsic module ISO_C_BINDING. gcc/testsuite/ChangeLog: PR fortran/61615 * gfortran.dg/interface_49.f90: New test. --- gcc/fortran/interface.cc | 14 ++- gcc/testsuite/gfortran.dg/interface_49.f90 | 43 ++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/interface_49.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index db79b104dc2..8682dc999be 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -2361,7 +2361,19 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, && formal->ts.u.derived && formal->ts.u.derived->ts.is_iso_c && actual->ts.type == BT_DERIVED && actual->ts.u.derived && actual->ts.u.derived->ts.is_iso_c) -return true; +{ + if (ranks_must_agree + && ((actual->rank == 0 && formal->attr.dimension) + || (actual->rank != 0 && !formal->attr.dimension))) + { + if (where) + argument_rank_mismatch (formal->name, &actual->where, +symbol_rank (formal), actual->rank, +NULL); + return false; + } + return true; +} if (formal->ts.type == BT_CLASS && actual->ts.type == BT_DERIVED) /* Make sure the vtab symbol is present when diff --git a/gcc/testsuite/gfortran.dg/interface_49.f90 b/gcc/testsuite/gfortran.dg/interface_49.f90 new file mode 100644 index 000..67d3e3f871b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/interface_49.f90 @@ -0,0 +1,43 @@ +! { dg-do run } +! PR fortran/61615 - resolve correct generic with TYPE(C_PTR) arguments +! Contributed by Jacob Abel + +MODULE foo + USE iso_c_binding, only : c_ptr + IMPLICIT NONE + integer :: rank = -99 + INTERFACE bar +MODULE PROCEDURE bar_s +MODULE PROCEDURE bar_a1d + END INTERFACE bar +CONTAINS + SUBROUTINE bar_s(a) +TYPE(c_ptr) :: a +WRITE (0, *) 'in bar_s' +rank = 0 + END SUBROUTINE bar_s + + SUBROUTINE bar_a1d(a) +TYPE(c_ptr) :: a(:) +WRITE (0, *) 'in bar_a1d' +rank = 1 + END SUBROUTINE bar_a1d +END MODULE foo + +PROGRAM cptr_array_vs_scalar_arg + USE foo + USE iso_c_binding, only : c_ptr, c_loc + IMPLICIT NONE + INTEGER, TARGET :: i + TYPE(c_ptr) :: a, b(1) + a= C_LOC(i) + b(1) = C_LOC(i) + CALL bar(a) + if (rank /= 0) stop 1 + CALL bar(b) + if (rank /= 1) stop 2 + CALL bar((a)) + if (rank /= 0) stop 3 + CALL bar((b)) + if (rank /= 1) stop 4 +END PROGRAM cptr_array_vs_scalar_arg -- 2.35.3
Re: Ping! [Patch, fortran] PR87477 - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi Paul, On 4/7/23 23:35, Paul Richard Thomas via Gcc-patches wrote: Hi Harald, Well done on noticing the memory leak :-) I have a fix for it that I was going to post separately. Actually, it is a trivial one liner, which I could include with the patch. thanks for addressing this! I can confirm that this correction does the job. Great work! Harald @@ -2554,23 +2559,25 @@ gfc_conv_string_length (gfc_charlen * cl, gfc_expr * expr, stmtblock_t * pblock) expr_flat = gfc_copy_expr (expr); flatten_array_ctors_without_strlen (expr_flat); gfc_resolve_expr (expr_flat); - - gfc_conv_expr (&se, expr_flat); - gfc_add_block_to_block (pblock, &se.pre); - cl->backend_decl = convert (gfc_charlen_type_node, se.string_length); - + if (expr_flat->rank) + gfc_conv_expr_descriptor (&se, expr_flat); + else + gfc_conv_expr (&se, expr_flat); + if (expr_flat->expr_type != EXPR_VARIABLE) + gfc_add_block_to_block (pblock, &se.pre); + se.expr = convert (gfc_charlen_type_node, se.string_length); + gfc_add_block_to_block (pblock, &se.post); // <<>> gfc_free_expr (expr_flat); - return; } - - /* Convert cl->length. */ - - gcc_assert (cl->length); - - gfc_conv_expr_type (&se, cl->length, gfc_charlen_type_node); - se.expr = fold_build2_loc (input_location, MAX_EXPR, gfc_charlen_type_node, -se.expr, build_zero_cst (TREE_TYPE (se.expr))); - gfc_add_block_to_block (pblock, &se.pre); + else +{ + /* Convert cl->length. */ + gfc_conv_expr_type (&se, cl->length, gfc_charlen_type_node); + se.expr = fold_build2_loc (input_location, MAX_EXPR, +gfc_charlen_type_node, se.expr, +build_zero_cst (TREE_TYPE (se.expr))); + gfc_add_block_to_block (pblock, &se.pre); +} if (cl->backend_decl && VAR_P (cl->backend_decl)) gfc_add_modify (pblock, cl->backend_decl, se.expr); Cheers Paul On Fri, 7 Apr 2023 at 20:28, Harald Anlauf wrote: Hi Paul, On 4/7/23 15:53, Paul Richard Thomas via Gcc-patches wrote: duuuh! Please find them attached. the patch LGTM. Thanks! However, I have comments on the new testcase associate_60.f90: subroutine pr93813 is missing an allocation of x, e.g.: allocate (t :: x) otherwise it would be invalid. Please check and fix. Interestingly, subroutine pr92779 exhibits a small memory leak with memory allocated by the spread intrinsic. I played a little and found that the leak depends on the presence of trim(): omitting trim() removes the leak. But looking at the related pr, it seems that trim() was essential, so omitting it is likely not an option. I think the best way is to proceed and to open a PR on the memory leak rather than leaving pr92779 open. What do you think? Cheers, Harald Thanks Paul On Fri, 7 Apr 2023 at 10:41, Harald Anlauf wrote: Hi Paul, I don't see the new testcases. Is this an issue on my side, or did you forget to attach them? Thanks, Harald On 4/7/23 09:07, Paul Richard Thomas via Gcc-patches wrote: Dear All, Please find attached a slightly updated version of the patch with a consolidated testcase. The three additional testcases are nothing to do with associate and test fixes of character related bugs. OK for mainline? Cheers Paul Fortran: Fix some of the bugs in associate [PR87477] 2023-04-07 Paul Thomas gcc/fortran PR fortran/87477 * resolve.cc (resolve_assoc_var): Handle parentheses around the target expression. (resolve_block_construct): Remove unnecessary static decls. * trans-array.cc (gfc_conv_expr_descriptor): Guard string len expression in condition. Improve handling of string length and span, especially for substrings of the descriptor. (duplicate_allocatable): Make element type more explicit with 'eltype'. * trans_decl.cc (gfc_get_symbol_decl): Emit a fatal error with appropriate message instead of ICE if symbol type is unknown. * trans-expr.cc (gfc_get_expr_charlen): Retain last charlen in 'previous' and use if end expression in substring reference is null. (gfc_conv_string_length): Use gfc_conv_expr_descriptor if 'expr_flat' is an array. (gfc_trans_alloc_subarray_assign): If this is a deferred string length component, store the string length in the hidden comp. Update the typespec length accordingly. Generate a new type spec for the call to gfc_duplicate-allocatable in this case. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of deferred character array components. gcc/testsuite/ PR fortran/87477 * gfortran.dg/finalize_47.f90 : Enable substring test. * gfortran.dg/finalize_51.f90 : Update an error message. PR fortran/85686 PR fortran/88247 PR fortran/91941 PR fortran/92779 PR fortran/93339 PR fortran/93813 PR fortran/100948 PR fortran/102106 * gfortran.dg/associate_60.f90 : New test PR fortran/98408 * gfortran.dg/pr98408.f90 : New test PR fortran/105205 * gfortran.dg/pr105205.f90 : New test PR
Re: Ping! [Patch, fortran] PR87477 - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi Paul, On 4/7/23 15:53, Paul Richard Thomas via Gcc-patches wrote: duuuh! Please find them attached. the patch LGTM. Thanks! However, I have comments on the new testcase associate_60.f90: subroutine pr93813 is missing an allocation of x, e.g.: allocate (t :: x) otherwise it would be invalid. Please check and fix. Interestingly, subroutine pr92779 exhibits a small memory leak with memory allocated by the spread intrinsic. I played a little and found that the leak depends on the presence of trim(): omitting trim() removes the leak. But looking at the related pr, it seems that trim() was essential, so omitting it is likely not an option. I think the best way is to proceed and to open a PR on the memory leak rather than leaving pr92779 open. What do you think? Cheers, Harald Thanks Paul On Fri, 7 Apr 2023 at 10:41, Harald Anlauf wrote: Hi Paul, I don't see the new testcases. Is this an issue on my side, or did you forget to attach them? Thanks, Harald On 4/7/23 09:07, Paul Richard Thomas via Gcc-patches wrote: Dear All, Please find attached a slightly updated version of the patch with a consolidated testcase. The three additional testcases are nothing to do with associate and test fixes of character related bugs. OK for mainline? Cheers Paul Fortran: Fix some of the bugs in associate [PR87477] 2023-04-07 Paul Thomas gcc/fortran PR fortran/87477 * resolve.cc (resolve_assoc_var): Handle parentheses around the target expression. (resolve_block_construct): Remove unnecessary static decls. * trans-array.cc (gfc_conv_expr_descriptor): Guard string len expression in condition. Improve handling of string length and span, especially for substrings of the descriptor. (duplicate_allocatable): Make element type more explicit with 'eltype'. * trans_decl.cc (gfc_get_symbol_decl): Emit a fatal error with appropriate message instead of ICE if symbol type is unknown. * trans-expr.cc (gfc_get_expr_charlen): Retain last charlen in 'previous' and use if end expression in substring reference is null. (gfc_conv_string_length): Use gfc_conv_expr_descriptor if 'expr_flat' is an array. (gfc_trans_alloc_subarray_assign): If this is a deferred string length component, store the string length in the hidden comp. Update the typespec length accordingly. Generate a new type spec for the call to gfc_duplicate-allocatable in this case. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of deferred character array components. gcc/testsuite/ PR fortran/87477 * gfortran.dg/finalize_47.f90 : Enable substring test. * gfortran.dg/finalize_51.f90 : Update an error message. PR fortran/85686 PR fortran/88247 PR fortran/91941 PR fortran/92779 PR fortran/93339 PR fortran/93813 PR fortran/100948 PR fortran/102106 * gfortran.dg/associate_60.f90 : New test PR fortran/98408 * gfortran.dg/pr98408.f90 : New test PR fortran/105205 * gfortran.dg/pr105205.f90 : New test PR fortran/106918 * gfortran.dg/pr106918.f90 : New test
Re: Ping! [Patch, fortran] PR87477 - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi Paul, I don't see the new testcases. Is this an issue on my side, or did you forget to attach them? Thanks, Harald On 4/7/23 09:07, Paul Richard Thomas via Gcc-patches wrote: Dear All, Please find attached a slightly updated version of the patch with a consolidated testcase. The three additional testcases are nothing to do with associate and test fixes of character related bugs. OK for mainline? Cheers Paul Fortran: Fix some of the bugs in associate [PR87477] 2023-04-07 Paul Thomas gcc/fortran PR fortran/87477 * resolve.cc (resolve_assoc_var): Handle parentheses around the target expression. (resolve_block_construct): Remove unnecessary static decls. * trans-array.cc (gfc_conv_expr_descriptor): Guard string len expression in condition. Improve handling of string length and span, especially for substrings of the descriptor. (duplicate_allocatable): Make element type more explicit with 'eltype'. * trans_decl.cc (gfc_get_symbol_decl): Emit a fatal error with appropriate message instead of ICE if symbol type is unknown. * trans-expr.cc (gfc_get_expr_charlen): Retain last charlen in 'previous' and use if end expression in substring reference is null. (gfc_conv_string_length): Use gfc_conv_expr_descriptor if 'expr_flat' is an array. (gfc_trans_alloc_subarray_assign): If this is a deferred string length component, store the string length in the hidden comp. Update the typespec length accordingly. Generate a new type spec for the call to gfc_duplicate-allocatable in this case. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of deferred character array components. gcc/testsuite/ PR fortran/87477 * gfortran.dg/finalize_47.f90 : Enable substring test. * gfortran.dg/finalize_51.f90 : Update an error message. PR fortran/85686 PR fortran/88247 PR fortran/91941 PR fortran/92779 PR fortran/93339 PR fortran/93813 PR fortran/100948 PR fortran/102106 * gfortran.dg/associate_60.f90 : New test PR fortran/98408 * gfortran.dg/pr98408.f90 : New test PR fortran/105205 * gfortran.dg/pr105205.f90 : New test PR fortran/106918 * gfortran.dg/pr106918.f90 : New test
Re: [Patch, fortran] PR87477 - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi Paul, On 4/7/23 09:02, Paul Richard Thomas via Gcc-patches wrote: Hi All, Please find attached the patch to fix the dg directives and remove a lot of trailing white space. Unless there are any objections, I will commit as obvious over the weekend. this is OK. Thanks for the patch! Harald Cheers Paul Fortran: Fix dg directives and remove trailing whitespaces in testsuite 2023-04-07 Paul Thomas * gfortran.dg/c-interop/allocatable-optional-pointer.f90 : Fix dg directive and remove trailing whitespace. * gfortran.dg/c-interop/c407a-1.f90 : ditto * gfortran.dg/c-interop/c407b-1.f90 : ditto * gfortran.dg/c-interop/c407b-2.f90 : ditto * gfortran.dg/c-interop/c407c-1.f90 : ditto * gfortran.dg/c-interop/c535a-1.f90 : ditto * gfortran.dg/c-interop/c535a-2.f90 : ditto * gfortran.dg/c-interop/c535b-1.f90 : ditto * gfortran.dg/c-interop/c535b-2.f90 : ditto * gfortran.dg/c-interop/c535b-3.f90 : ditto * gfortran.dg/c-interop/c535c-1.f90 : ditto * gfortran.dg/c-interop/c535c-2.f90 : ditto * gfortran.dg/c-interop/deferred-character-1.f90 : ditto * gfortran.dg/c-interop/removed-restrictions-1.f90 : ditto * gfortran.dg/c-interop/removed-restrictions-2.f90 : ditto * gfortran.dg/c-interop/removed-restrictions-4.f90 : ditto * gfortran.dg/c-interop/tkr.f90 : ditto * gfortran.dg/class_result_10.f90 : ditto * gfortran.dg/dtio_35.f90 : ditto * gfortran.dg/goacc/array-with-dt-2.f90 : ditto * gfortran.dg/gomp/affinity-clause-1.f90 : ditto * gfortran.dg/pr103258.f90 : ditto * gfortran.dg/pr59107.f90 : ditto * gfortran.dg/pr93835.f08 : ditto On Wed, 29 Mar 2023 at 09:53, Paul Richard Thomas < paul.richard.tho...@gmail.com> wrote: Hi Manfred, Indeed I do :-) Thanks for the spot. I have decided that it will be less messy if I roll all the testcases into one or, perhaps two => associate_xx.f90 Forgetting the space before the final brace seems to be rife! Cheers Paul On Wed, 29 Mar 2023 at 09:24, Manfred Schwarb wrote: Am 28.03.23 um 23:04 schrieb Paul Richard Thomas via Fortran: Hi All, I have made a start on ASSOCIATE issues. Some of the low(-ish) hanging fruit are already fixed but I have yet to check that they a really fixed and to close them: pr102106, pr102111, pr104430, pr106048, pr85510, pr87460, pr92960 & pr93338 The attached patch picks up those PRs involving deferred length characters in one guise or another. I believe that it is all pretty straightforward. Structure constructors with allocatable, deferred length, character array components just weren't implemented and so this is the biggest part of the patch. I found two other, non-associate PRs(106918 & 105205) that are fixed and there are probably more. The chunk in trans-io.cc is something of a kludge, which I will come back to. Some descriptors come through with a data pointer that looks as if it should be OK but I thought to submit this now to get it out of the way. The ratio of PRs fixed to the size of the patch warrants this. The next stage is going to be rather messy and so "I might take a little while" (cross talk between associate and select type, in particular). Regtests OK - good for mainline? Paul, you have some "dg-do-run" and "dg-do-compile" statements in your testcases, could you change them into their single-minus-sign variants? Cheers, Manfred BTW: I just ran my script again and found the following testsuite issues (note that outer-most braces need to be space-padded): ./c-interop/removed-restrictions-1.f90:! { dg-do compile} ./c-interop/removed-restrictions-2.f90:! { dg-do compile} ./c-interop/removed-restrictions-3.f90:! { dg-do compile} ./c-interop/removed-restrictions-4.f90:! { dg-do compile} ./c-interop/tkr.f90:! { dg-do compile} ./c-interop/c407c-1.f90:! { dg-do compile} ./c-interop/deferred-character-1.f90:! { dg-do compile} ./c-interop/allocatable-optional-pointer.f90:! { dg-do compile} ./c-interop/c407a-1.f90:! { dg-do compile} ./c-interop/c407b-1.f90:! { dg-do compile} ./c-interop/c407b-2.f90:! { dg-do compile} ./c-interop/c535a-1.f90:! { dg-do compile} ./c-interop/c535a-2.f90:! { dg-do compile} ./c-interop/c535b-1.f90:! { dg-do compile} ./c-interop/c535b-2.f90:! { dg-do compile} ./c-interop/c535b-3.f90:! { dg-do compile} ./c-interop/c535c-1.f90:! { dg-do compile} ./c-interop/c535c-2.f90:! { dg-do compile} ./gomp/affinity-clause-1.f90:! { dg final { scan-tree-dump-times "#pragma omp task affinity\\(iterator\\(integer\\(kind=4\\) i=D\\.\[0-9\]+:5:1\\):b\\\[\\(.* ? \\+ -1\\\]\\) affinity\\(iterator\\(integer\\(kind=4\\) i=D\\.\[0-9\]+:5:1\\):d\\\[\\(\\(integer\\(kind=8\\)\\) i \\+ -1\\) \\* 6\\\]\\)" 1 "original" } } ./class_result_10.f90:! { dg-do run} ./pr103258.f90:! { dg-do compile} ./dtio_35.f90:! { dg-compile } ./pr93835.f08:! {dg-do run } ./pr59107.f90:! { dg-compile } Cheers Paul Fortran: Fix some of the bugs in associate [PR87477] 2023-03-28 Paul Thomas gcc/fortran PR fortran/87477 * trans-array.cc (gfc_conv_expr_descriptor): Guard string len expression in condition.
Re: [Patch, fortran] PR104272 - finalizer gets called during allocate
On 4/5/23 20:50, Harald Anlauf via Gcc-patches wrote: can you have a look again at the logic in the hunk touching trans-stmt.cc (gfc_trans_allocate)? I haven't checked in detail, but it seems possible that you get a stale tmp in the gfc_prepend_expr_to_block if (code->ext.alloc.expr3_not_explicit == 0). Oops, I meant if (code->ext.alloc.expr3_not_explicit != 0) Wouldn't it make more sense to move this condition before the braces as part of the overall condition? OK for mainline? Otherwise this LGTM. Thanks for the patch! Harald
Re: [Patch, fortran] PR104272 - finalizer gets called during allocate
Hi Paul, On 4/5/23 08:53, Paul Richard Thomas via Gcc-patches wrote: Hi All, This is a first in my recent experience - a very old bug that produces too many finalizations! It results from a bit of a fix up, where class objects are allocated using the derived typespec, rather than a source or mold expression. This occurs upstream in resolve.cc, where the default initializer is assigned to expr3 but no means are provided to identify what it is. The patch applies a signaling bit-field to the ext field of gfc_code, which then suppresses the deallocation of allocatable components in the allocate expression. I have checked that this does not cause memory leaks, even though the number of builtin_frees in class_result_8.f90 goes down by one. can you have a look again at the logic in the hunk touching trans-stmt.cc (gfc_trans_allocate)? I haven't checked in detail, but it seems possible that you get a stale tmp in the gfc_prepend_expr_to_block if (code->ext.alloc.expr3_not_explicit == 0). Wouldn't it make more sense to move this condition before the braces as part of the overall condition? OK for mainline? Otherwise this LGTM. Thanks for the patch! Harald Paul Fortran: Fix and excess finalization during allocation [PR104272] 2023-04-04 Paul Thomas gcc/fortran PR fortran/104272 * gfortran.h : Add expr3_not_explicit bit field to gfc_code. * resolve.cc (resolve_allocate_expr): Set bit field when the default initializer is applied to expr3. * trans-stmt.cc (gfc_trans_allocate): If expr3_not_explicit is set, do not deallocate expr3. gcc/testsuite/ PR fortran/104272 * gfortran.dg/class_result_8.f90 : Number of builtin_frees down from 6 to 5 without memory leaks. * gfortran.dg/finalize_52.f90: New test
Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks
Hi Bernhard, there is neither context nor a related PR with a testcase showing that this patch fixes issues seen there. On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote: From: Bernhard Reutner-Fischer Cc: fort...@gcc.gnu.org gcc/fortran/ChangeLog: * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing. * expr.cc (find_array_section): Fix mpz memory leak. * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in error paths. (gfc_simplify_set_exponent): Fix mpfr memory leak. --- gcc/fortran/array.cc| 3 +++ gcc/fortran/expr.cc | 8 gcc/fortran/simplify.cc | 7 ++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc index be5eb8b6a0f..8b1e816a859 100644 --- a/gcc/fortran/array.cc +++ b/gcc/fortran/array.cc @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end) return t; default: + mpz_clear (lower); + mpz_clear (stride); + mpz_clear (upper); gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type"); } What is the point of clearing variables before issuing a gfc_internal_error? diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 7fb33f81788..b4736804eda 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) mpz_init_set_ui (delta_mpz, one); mpz_init_set_ui (nelts, one); mpz_init (tmp_mpz); + mpz_init (ptr); /* Do the initialization now, so that we can cleanup without keeping track of where we were. */ @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) mpz_mul (delta_mpz, delta_mpz, tmp_mpz); } - mpz_init (ptr); cons = gfc_constructor_first (base); /* Now clock through the array reference, calculating the index in @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) "at %L requires an increase of the allowed %d " "upper limit. See %<-fmax-array-constructor%> " "option", &expr->where, flag_max_array_constructor); - return false; + t = false; + goto cleanup; } cons = gfc_constructor_lookup (base, limit); @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) gfc_copy_expr (cons->expr), NULL); } - mpz_clear (ptr); - cleanup: mpz_clear (delta_mpz); @@ -1765,6 +1764,7 @@ cleanup: mpz_clear (ctr[d]); mpz_clear (stride[d]); } + mpz_clear (ptr); gfc_constructor_free (base); return t; } diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index ecf0e3558df..d1f06335e79 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a " "negative value %d for dimension %d", &shape_exp->where, shape[rank], rank+1); + mpz_clear (index); return &gfc_bad_expr; } @@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, { gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different", &order_exp->where, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } @@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, { gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different", &order_exp->where, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } @@ -6918,6 +6921,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, "in the range [1, ..., %d] for the RESHAPE intrinsic " "near %L", order[i], &order_exp->where, rank, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } @@ -6926,6 +6930,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, { gfc_error ("ORDER at %L is not a permutation of the size of " "SHAPE at %L", &order_exp->where, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } x[order[i]] = 1; @@ -7408,7 +7413,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) exp2 = (unsigned long) mpz_get_d (i->value.integer); mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE); - mpfr_clears (absv, log2, pow2, frac, NULL); + mpfr_clears (exp, absv, log2, pow2, frac, NULL); return range_check (result, "SET_EXPONENT"); }
[PATCH] Fortran: reject module variable as character length in PARAMETER [PR104349]
Dear all, the attached patch fixes an ICE-on-invalid for a PARAMETER expression where the character length was a MODULE variable. The ICE seemed strange, as we were catching related erroneous code for declarations in programs or subroutines. Removing a seemingly bogus check of restricted expressions is the simplest way to fix this. (We could also catch this differently in decl.cc). Besides, this also fixes an accepts-invalid, see testcase. :-) Regtested on x86_64-pc-linux-gnu. OK for mainline (13) or rather wait? Thanks, Harald From 37136ce94b44149dd013b3d7fed7adba769241e6 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 3 Apr 2023 21:34:01 +0200 Subject: [PATCH] Fortran: reject module variable as character length in PARAMETER [PR104349] gcc/fortran/ChangeLog: PR fortran/104349 * expr.cc (check_restricted): Adjust check for valid variables in restricted expressions: make no exception for module variables. gcc/testsuite/ChangeLog: PR fortran/104349 * gfortran.dg/der_charlen_1.f90: Adjust dg-patterns. * gfortran.dg/pr104349.f90: New test. --- gcc/fortran/expr.cc | 2 -- gcc/testsuite/gfortran.dg/der_charlen_1.f90 | 2 ++ gcc/testsuite/gfortran.dg/pr104349.f90 | 8 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr104349.f90 diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 7fb33f81788..02028f993fd 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -3504,8 +3504,6 @@ check_restricted (gfc_expr *e) || sym->attr.implied_index || sym->attr.flavor == FL_PARAMETER || is_parent_of_current_ns (sym->ns) - || (sym->ns->proc_name != NULL - && sym->ns->proc_name->attr.flavor == FL_MODULE) || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns))) { t = true; diff --git a/gcc/testsuite/gfortran.dg/der_charlen_1.f90 b/gcc/testsuite/gfortran.dg/der_charlen_1.f90 index 9f394c73f25..1246522d516 100644 --- a/gcc/testsuite/gfortran.dg/der_charlen_1.f90 +++ b/gcc/testsuite/gfortran.dg/der_charlen_1.f90 @@ -22,3 +22,5 @@ CONTAINS type(T), intent(in) :: X end subroutine end module another_core + +! { dg-prune-output "cannot appear in the expression" } diff --git a/gcc/testsuite/gfortran.dg/pr104349.f90 b/gcc/testsuite/gfortran.dg/pr104349.f90 new file mode 100644 index 000..2bea4a37214 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr104349.f90 @@ -0,0 +1,8 @@ +! { dg-do compile } +! PR fortran/104349 - reject module variable as character length in PARAMETER +! Contributed by G.Steinmetz + +module m + character(n), parameter :: a(1) = 'b' ! { dg-error "cannot appear" } + character(n), parameter :: c= 'b' ! { dg-error "cannot appear" } +end -- 2.35.3
Re: [PATCH, commited] Fortran: remove dead code [PR104321]
Hi Paul, > If you will excuse the British cultural reference, that's a Norwegian Blue > alright! Good spot. ROTFL! I first had to look up the "Norwegian Blue", and then I remembered. :) You're bringing back the fun to gfortran hacking! Cheers, Harald On Sat, 25 Mar 2023 at 19:13, Harald Anlauf via Fortran mailto:fort...@gcc.gnu.org]> wrote:Dear all, I've committed the attached patch from the PR that removes a dead code snippet, see discussion. Regtested originally by Tobias, and reconfirmed on x86_64-pc-linux-gnu. Pushed as r13-6862-gb5fce899dbbd72 . Thanks, Harald -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[PATCH, commited] Fortran: remove dead code [PR104321]
Dear all, I've committed the attached patch from the PR that removes a dead code snippet, see discussion. Regtested originally by Tobias, and reconfirmed on x86_64-pc-linux-gnu. Pushed as r13-6862-gb5fce899dbbd72 . Thanks, Harald From b5fce899dbbd7246d003209b2fe3b04f8738 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 25 Mar 2023 19:59:45 +0100 Subject: [PATCH] Fortran: remove dead code [PR104321] gcc/fortran/ChangeLog: PR fortran/104321 * trans-decl.cc (gfc_conv_cfi_to_gfc): Remove dead code. --- gcc/fortran/trans-decl.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc index 77610df340b..25737881ae0 100644 --- a/gcc/fortran/trans-decl.cc +++ b/gcc/fortran/trans-decl.cc @@ -7165,9 +7165,6 @@ gfc_conv_cfi_to_gfc (stmtblock_t *init, stmtblock_t *finally, size_var, LT_EXPR, build_int_cst (TREE_TYPE (idx), 1), gfc_finish_block (&loop_body)); /* if (cond) { block2 } */ - tmp = fold_build2_loc (input_location, MODIFY_EXPR, void_type_node, - data, fold_convert (TREE_TYPE (data), - null_pointer_node)); tmp = build3_v (COND_EXPR, cond_var, gfc_finish_block (&block2), build_empty_stmt (input_location)); gfc_add_expr_to_block (&block, tmp); -- 2.35.3
[PATCH, committed] Fortran: fix FE memleak with BOZ expressions
Dear all, while looking at variations of testcases in pr107560, I discovered a minor FE memleak that was introduced in the BOZ rework and is fixed by the attached simple patch. Regtested on x86_64-pc-linux-gnu on OK'ed in the PR by Steve. Thanks, Harald From 833233a4aefc9981b671c1bda34676c20b76cc90 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 24 Mar 2023 22:07:37 +0100 Subject: [PATCH] Fortran: fix FE memleak with BOZ expressions. gcc/fortran/ChangeLog: * expr.cc (free_expr0): Free also BOZ strings as part of an expression. --- gcc/fortran/expr.cc | 4 1 file changed, 4 insertions(+) diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 4662328bf31..7fb33f81788 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -466,6 +466,10 @@ free_expr0 (gfc_expr *e) mpc_clear (e->value.complex); break; + case BT_BOZ: + free (e->boz.str); + break; + default: break; } -- 2.35.3
[PATCH, committed] Fortran: improve checking of FINAL subroutine arguments [PR104572]
Dear all, I've committed the attached simple patch after discussion with Steve (see PR). We need to reject alternate return arguments of FINAL subroutines. Regtested on x86_64-pc-linux-gnu. Thanks, Harald From 3e791f45ded89626bc1f9f8013728f6e035801b2 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 22 Mar 2023 19:20:41 +0100 Subject: [PATCH] Fortran: improve checking of FINAL subroutine arguments [PR104572] gcc/fortran/ChangeLog: PR fortran/104572 * resolve.cc (gfc_resolve_finalizers): Argument of a FINAL subroutine cannot be an alternate return. gcc/testsuite/ChangeLog: PR fortran/104572 * gfortran.dg/pr104572.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/resolve.cc | 7 +++ gcc/testsuite/gfortran.dg/pr104572.f90 | 14 ++ 2 files changed, 21 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr104572.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 1a03e458d99..f6ec76acb0b 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -13986,6 +13986,13 @@ gfc_resolve_finalizers (gfc_symbol* derived, bool *finalizable) } arg = dummy_args->sym; + if (!arg) + { + gfc_error ("Argument of FINAL procedure at %L must be of type %qs", + &list->proc_sym->declared_at, derived->name); + goto error; + } + if (arg->as && arg->as->type == AS_ASSUMED_RANK && ((list != derived->f2k_derived->finalizers) || list->next)) { diff --git a/gcc/testsuite/gfortran.dg/pr104572.f90 b/gcc/testsuite/gfortran.dg/pr104572.f90 new file mode 100644 index 000..59fd688d798 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr104572.f90 @@ -0,0 +1,14 @@ +! { dg-do compile } +! { dg-additional-options "-w" } +! PR fortran/104572 - ICE in gfc_resolve_finalizers +! Contributed by G. Steinmetz + +module m + type t + contains + final :: s + end type +contains + subroutine s(*) ! { dg-error "Argument of FINAL procedure" } + end +end -- 2.35.3
Re: [PATCH] Fortran: reject MODULE PROCEDURE outside generic module interface [PR99036]
Hi Tobias, Am 21.03.23 um 09:31 schrieb Tobias Burnus: On 20.03.23 21:57, Harald Anlauf via Gcc-patches wrote: --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -9998,6 +9998,7 @@ gfc_match_modproc (void) if ((gfc_state_stack->state != COMP_INTERFACE && gfc_state_stack->state != COMP_CONTAINS) || gfc_state_stack->previous == NULL + || !current_interface.type || current_interface.type == INTERFACE_NAMELESS || current_interface.type == INTERFACE_ABSTRACT) { First, I do not like '!var' comparisons for enum values, only for Booleans/logicals and pointer. I was hesitating to do this and thought about adding an enum value that it 0 numerically, but ... Secondly, I am not sure that it is really guaranteed that the value is 0. ... had assumed that this would be guaranteed. I think something like the following makes more sense and, as just tried, it also regtests (w/ your testcase included). If you agree, feel free to package and commit it. diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index c8f0bb83c2c..233bf244d62 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -9996,7 +9996,8 @@ gfc_match_modproc (void) gfc_interface *old_interface_head, *interface; - if ((gfc_state_stack->state != COMP_INTERFACE - && gfc_state_stack->state != COMP_CONTAINS) - || gfc_state_stack->previous == NULL + if (gfc_state_stack->previous == NULL + || (gfc_state_stack->state != COMP_INTERFACE + && (gfc_state_stack->state != COMP_CONTAINS + || gfc_state_stack->previous->state != COMP_INTERFACE)) || current_interface.type == INTERFACE_NAMELESS || current_interface.type == INTERFACE_ABSTRACT) Yes, that's a much cleaner solution. Pushed as: https://gcc.gnu.org/g:dd282b16bfd3c6e218dffb7798a375365b10ae22 commit r13-6790-gdd282b16bfd3c6e218dffb7798a375365b10ae22 Thanks for the review! Harald Thanks for working on this and all the other issues! Tobias - 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, doc] Explicitly mention undefined overflow
Hi Thomas, Am 20.03.23 um 08:14 schrieb Thomas Koenig via Gcc-patches: so it the general problem is not restricted to -O3 and not to current trunk, it depends on the details. I doubt that the result from 9.4.0 was expected, but rather nobody noticed. Or, bringing out the pseudo-RNG into a different setting changed things. So... any suggestions on how to improve the current wording? how about changing: "... relying on a specific, non-standard behavior may now generate unexpected results." to "... relying on a specific, non-standard behavior may generate unexpected results depending on optimization level and other compiler flags." We cannot know all the codes used in the wild ... Cheers, Harald Best regards Thomas
[PATCH] Fortran: reject MODULE PROCEDURE outside generic module interface [PR99036]
Dear all, the attached trivial patch catches a MODULE PROCEDURE outside of a module interface before we run into an internal error. Regtested on x86_64-pc-linux-gnu. OK for mainline? This PR is marked as an 11/12/13 regression, so this is a candidate for backporting. Thanks, Harald From 9c59709fad91c99041a9cb770b98da17af01d260 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 20 Mar 2023 21:50:59 +0100 Subject: [PATCH] Fortran: reject MODULE PROCEDURE outside generic module interface [PR99036] gcc/fortran/ChangeLog: PR fortran/99036 * decl.cc (gfc_match_modproc): Reject MODULE PROCEDURE if not in a generic module interface. gcc/testsuite/ChangeLog: PR fortran/99036 * gfortran.dg/pr99036.f90: New test. --- gcc/fortran/decl.cc | 1 + gcc/testsuite/gfortran.dg/pr99036.f90 | 9 + 2 files changed, 10 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr99036.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index c8f0bb83c2c..b29f491fe1f 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -9998,6 +9998,7 @@ gfc_match_modproc (void) if ((gfc_state_stack->state != COMP_INTERFACE && gfc_state_stack->state != COMP_CONTAINS) || gfc_state_stack->previous == NULL + || !current_interface.type || current_interface.type == INTERFACE_NAMELESS || current_interface.type == INTERFACE_ABSTRACT) { diff --git a/gcc/testsuite/gfortran.dg/pr99036.f90 b/gcc/testsuite/gfortran.dg/pr99036.f90 new file mode 100644 index 000..a6e396f6f71 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr99036.f90 @@ -0,0 +1,9 @@ +! { dg-do compile } +! PR fortran/99036 - ICE in gfc_current_interface_head +! Contributed by G. Steinmetz + +module m +contains + module procedure s ! { dg-error "must be in a generic module interface" } + end +end -- 2.35.3
[PATCH] Fortran: fix documentation of -fno-underscoring [PR109216]
Dear all, as reported, the implicit documentation of -funderscoring, which is found under -fno-underscoring, has gone sideways long time ago. The attached patch should fix it. OK for mainline, or did I miss something? Thanks, Harald From c296196044248f974b4907bb2f5bdeeea24adb5b Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 20 Mar 2023 20:55:00 +0100 Subject: [PATCH] Fortran: fix documentation of -fno-underscoring [PR109216] gcc/fortran/ChangeLog: PR fortran/109216 * invoke.texi: Correct documentation of how underscores are appended to external names. --- gcc/fortran/invoke.texi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi index 5679e2f2650..cbe7f377507 100644 --- a/gcc/fortran/invoke.texi +++ b/gcc/fortran/invoke.texi @@ -1573,7 +1573,7 @@ Do not transform names of entities specified in the Fortran source file by appending underscores to them. With @option{-funderscoring} in effect, GNU Fortran appends one -underscore to external names with no underscores. This is done to ensure +underscore to external names. This is done to ensure compatibility with code produced by many UNIX Fortran compilers. @emph{Caution}: The default behavior of GNU Fortran is @@ -1596,7 +1596,7 @@ I = J() + MAX_COUNT (MY_VAR, LVAR) @noindent is implemented as something akin to: @smallexample -i = j_() + max_count__(&my_var__, &lvar); +i = j_() + max_count_(&my_var, &lvar); @end smallexample With @option{-fno-underscoring}, the same statement is implemented as: -- 2.35.3
[PATCH] Fortran: simplification of NEAREST for large argument [PR109186]
Dear all, I intend to commit the attached obvious patch within 24h unless there are comments. The issue is an off-by-one error in setting up the maximum exponent of the real kind that is passed to mpfr, so that model numbers between huge(x)/2 and huge(x), when given as an argument to NEAREST(arg,-1.0), are rounded down to huge(x)/2 during compile-time simplification. As no such issue is observed at run-time, the testcase compares the compile-time and run-time results for corner cases. Regtested on x86_64-pc-linux-gnu. As this is sort of a wrong-code issue, I intend to backport to all open branches. (The issue was apparently introduced in r0-84566-gb6f63e898498e6 without noticing, so it is technically a regression.) Thanks, Harald From 9391bd0eeef8e069d9e49f9aa277160b43aaf4f3 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 19 Mar 2023 21:29:46 +0100 Subject: [PATCH] Fortran: simplification of NEAREST for large argument [PR109186] gcc/fortran/ChangeLog: PR fortran/109186 * simplify.cc (gfc_simplify_nearest): Fix off-by-one error in setting up real kind-specific maximum exponent for mpfr. gcc/testsuite/ChangeLog: PR fortran/109186 * gfortran.dg/nearest_6.f90: New test. --- gcc/fortran/simplify.cc | 2 +- gcc/testsuite/gfortran.dg/nearest_6.f90 | 26 + 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/nearest_6.f90 diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index 20ea38e0007..ecf0e3558df 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -6114,7 +6114,7 @@ gfc_simplify_nearest (gfc_expr *x, gfc_expr *s) kind = gfc_validate_kind (BT_REAL, x->ts.kind, 0); mpfr_set_emin ((mpfr_exp_t) gfc_real_kinds[kind].min_exponent - mpfr_get_prec(result->value.real) + 1); - mpfr_set_emax ((mpfr_exp_t) gfc_real_kinds[kind].max_exponent - 1); + mpfr_set_emax ((mpfr_exp_t) gfc_real_kinds[kind].max_exponent); mpfr_check_range (result->value.real, 0, MPFR_RNDU); if (mpfr_sgn (s->value.real) > 0) diff --git a/gcc/testsuite/gfortran.dg/nearest_6.f90 b/gcc/testsuite/gfortran.dg/nearest_6.f90 new file mode 100644 index 000..00d1ebe618c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/nearest_6.f90 @@ -0,0 +1,26 @@ +! { dg-do run } +! PR fortran/109186 - Verify that NEAREST produces same results at +! compile-time and run-time for corner cases +! Reported by John Harper + +program p + implicit none + integer, parameter :: sp = selected_real_kind (6) + integer, parameter :: dp = selected_real_kind (13) + real(sp), parameter :: x1 = huge (1._sp), t1 = tiny (1._sp) + real(dp), parameter :: x2 = huge (1._dp), t2 = tiny (1._dp) + real(sp), volatile :: y1, z1 + real(dp), volatile :: y2, z2 + y1 = x1 + z1 = nearest (y1, -1._sp) + if (nearest (x1, -1._sp) /= z1) stop 1 + y2 = x2 + z2 = nearest (y2, -1._dp) + if (nearest (x2, -1._dp) /= z2) stop 2 + y1 = t1 + z1 = nearest (y1, 1._sp) + if (nearest (t1, 1._sp) /= z1) stop 3 + y2 = t2 + z2 = nearest (y2, 1._dp) + if (nearest (t2, 1._dp) /= z2) stop 4 +end -- 2.35.3
Re: [PATCH] Fortran: procedures with BIND(C) attribute require explicit interface [PR85877]
Hi Thomas, Am 19.03.23 um 08:34 schrieb Thomas Koenig via Gcc-patches: Hi Harald, Am 18.03.23 um 19:52 schrieb Thomas Koenig via Gcc-patches: Hi Harald, the Fortran standard requires an explicit procedure interface in certain situations, such as when they have a BIND(C) attribute (F2018:15.4.2.2). The attached patch adds a check for this. Regtested on x86_64-pc-linux-gnu. OK for mainline? While this fixes the ICE, it misses function f() bind(c) f = 42. end subroutine p bind(c) f ! { dg-error "must be explicit" } x = f() end what do you mean by "it misses"? Sorry, that was caused by confusion on my part (and it is better to test an assumption of what the compiler actually does :-) Patch is OK, also for backport. Maybe you can also include the test above, just to make sure. I've added your suggestion to the testcase. Pushed as: https://gcc.gnu.org/g:5426ab34643d9e6502f3ee572891a03471fa33ed Best regards Thomas Thanks, Harald
Re: [patch, wwwdocs] Mention random number generators in porting_to.html
Hi Thomas, Am 18.03.23 um 19:23 schrieb Thomas Koenig via Gcc-patches: Hi, Text says it all. OK for web pages? Best regards Thomas Mention issues with integer owerflow for random number generators. This mentions the issues with integer overflow and how to work around them. it's basically fine, although I'd prefer a formulation replacing + GCC 13 includes new optimizations which expose reliance on + non-standard behavior for integer overflow, which was often used + for linear congruential pseudo-random number generators in old + programs. It is recommended to use the intrinsic by something like: GCC 13 includes new optimizations which may change behavior on integer overflow. Traditional code, like linear congruential pseudo-random number generators in old programs and relying on a specific, non-standard behavior may now generate unexpected results. In such cases it is recommended to use the intrinsic ... Thanks for updating the documentation! Harald
Re: [PATCH] Fortran: procedures with BIND(C) attribute require explicit interface [PR85877]
Hi Thomas, Am 18.03.23 um 19:52 schrieb Thomas Koenig via Gcc-patches: Hi Harald, the Fortran standard requires an explicit procedure interface in certain situations, such as when they have a BIND(C) attribute (F2018:15.4.2.2). The attached patch adds a check for this. Regtested on x86_64-pc-linux-gnu. OK for mainline? While this fixes the ICE, it misses function f() bind(c) f = 42. end subroutine p bind(c) f ! { dg-error "must be explicit" } x = f() end what do you mean by "it misses"? I do not see an explicit interface here, only a global symbol. All compiler I tried with the above code reject it one way or the other, e g. Cray: x = f() ^ ftn-954 crayftn: ERROR P, File = pr85877-2.f90, Line = 12, Column = 3 Procedure "f", referenced at line 6 (pr85877-2.f90) must have an explicit interface because one or more arguments have the BIND attribute. subroutine s interface function g() bind(c) end function g end interface x = g() end where the interface is picked up via a global symbol. Is it really true that this is in the spirit of the standard? Is the global declaration above really equivalent to an explicit interface? I would expect legal code to be like: function g() bind(c) g = 42. end subroutine s interface function g() bind(c) end function g end interface x = g() end unless you do it in the context of a module and in the right way. This code may not be valid; nagfor rejects it, but I cannot find a constraint at least in the F2022 draft that prohibits it. I thought that F2018:15.4.2.2 and F2023:15.4.2.2(7) are rather clear and "explicit" on this. IMHO the case of "f" above seems to be excluded and thus not conforming. Hm... might it be better to check for attr->module_procedure || attr->if_source == IFSRC_IFBODY? Maybe we should find a set of legal and illegal cases that we can agree upon. Thanks, Harald Best regards Thomas