[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
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, 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] 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
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: 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] 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] 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] 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: 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
[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: 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
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
[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
[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, 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, 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] 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
Re: [PATCH] Fortran: error recovery on invalid CLASS(), PARAMETER declarations [PR105243]
Hi Tobias, Am 30.06.22 um 11:58 schrieb Tobias Burnus: The initial patch is by Steve. I adjusted and moved it slightly so that it also handles CLASS(*) (unlimited polymorphic) at the same time. Shouldn't you then also acknowledge him, e.g. via Co-authored-by? yeah, I noticed that right after submitting the mail and immediately amended the commit message. Pushed as https://gcc.gnu.org/g:4c233cabbe388a6b8957c1507e129090e9267ceb Thanks, Harald
[PATCH] Fortran: error recovery simplifying PACK with invalid arguments [PR106049]
Dear all, poor error recovery while trying to simplify intrinsics with given invalid arguments seems to be a recurrent theme in testcases submitted by Gerhard. In the present case, simplification of PACK() chokes on the array argument being a bad decl. The most general approach that came to my mind is to modify function is_constant_array_expr: when the declared shape of the array indicates a size greater than zero, but the constructor is missing or empty, then something bad may have happened, and the array cannot be considered constant. We thus punt on simplification of something that cannot be simplified. With some luck, this might prevent issues in similar cases elsewhere... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From b70a225cd9ac83cd182938bb8019f9138f85b222 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 5 Jul 2022 22:20:05 +0200 Subject: [PATCH] Fortran: error recovery simplifying PACK with invalid arguments [PR106049] gcc/fortran/ChangeLog: PR fortran/106049 * simplify.cc (is_constant_array_expr): A non-zero-sized constant array shall have a non-empty constructor. When the constructor is empty or missing, treat as non-constant. gcc/testsuite/ChangeLog: PR fortran/106049 * gfortran.dg/pack_simplify_1.f90: New test. --- gcc/fortran/simplify.cc | 12 gcc/testsuite/gfortran.dg/pack_simplify_1.f90 | 15 +++ 2 files changed, 27 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pack_simplify_1.f90 diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index ab59fbca622..fb725994653 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -233,6 +233,18 @@ is_constant_array_expr (gfc_expr *e) if (e->expr_type != EXPR_ARRAY || !gfc_is_constant_expr (e)) return false; + /* A non-zero-sized constant array shall have a non-empty constructor. */ + if (e->rank > 0 && e->shape != NULL && e->value.constructor == NULL) +{ + mpz_init_set_ui (size, 1); + for (int j = 0; j < e->rank; j++) + mpz_mul (size, size, e->shape[j]); + bool not_size0 = (mpz_cmp_si (size, 0) != 0); + mpz_clear (size); + if (not_size0) + return false; +} + for (c = gfc_constructor_first (e->value.constructor); c; c = gfc_constructor_next (c)) if (c->expr->expr_type != EXPR_CONSTANT diff --git a/gcc/testsuite/gfortran.dg/pack_simplify_1.f90 b/gcc/testsuite/gfortran.dg/pack_simplify_1.f90 new file mode 100644 index 000..06bc55a14f3 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pack_simplify_1.f90 @@ -0,0 +1,15 @@ +! { dg-do compile } +! PR fortran/106049 - ICE in gfc_simplify_pack +! Contributed by G.Steinmetz + +program p + type t + end type + logical, parameter :: m(0) = [ logical :: ] + type(t), parameter :: a(0) = [ t :: ] + type(t), parameter :: b(1) = [ t() ] + type(t), parameter :: c(1) = [ t :: ]! { dg-error "Different shape" } + type(t), parameter :: d(0) = pack(a, m) + type(t), parameter :: e(1) = pack(b, [.true.]) + type(t), parameter :: f(1) = pack(c, [.true.]) +end -- 2.35.3
Re: [PATCH] Fortran: error recovery simplifying PACK with invalid arguments [PR106049]
As there were no comments, committed as r13-1650. Am 05.07.22 um 22:31 schrieb Harald Anlauf via Fortran: Dear all, poor error recovery while trying to simplify intrinsics with given invalid arguments seems to be a recurrent theme in testcases submitted by Gerhard. In the present case, simplification of PACK() chokes on the array argument being a bad decl. The most general approach that came to my mind is to modify function is_constant_array_expr: when the declared shape of the array indicates a size greater than zero, but the constructor is missing or empty, then something bad may have happened, and the array cannot be considered constant. We thus punt on simplification of something that cannot be simplified. With some luck, this might prevent issues in similar cases elsewhere... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald
[PATCH, committed] Fortran: error recovery for bad initializers of implied-shape arrays [PR106209]
Dear all, the attached patch introduces error recovery for two cases of using an invalid array in a declaration of an implied-shape array instead of hitting internal errors. Initial patch by Steve. The final version was approved in the PR by Steve. Committed as: https://gcc.gnu.org/g:748f8a8b145dde59c7b63aa68b5a59515b7efc49 Thanks, Harald From 748f8a8b145dde59c7b63aa68b5a59515b7efc49 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 14 Jul 2022 22:24:55 +0200 Subject: [PATCH] Fortran: error recovery for bad initializers of implied-shape arrays [PR106209] gcc/fortran/ChangeLog: PR fortran/106209 * decl.cc (add_init_expr_to_sym): Handle bad initializers for implied-shape arrays. gcc/testsuite/ChangeLog: PR fortran/106209 * gfortran.dg/pr106209.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/decl.cc| 15 +-- gcc/testsuite/gfortran.dg/pr106209.f90 | 9 + 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr106209.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index 339f8b15035..b6400514731 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -2129,10 +2129,21 @@ add_init_expr_to_sym (const char *name, gfc_expr **initp, locus *var_locus) /* The shape may be NULL for EXPR_ARRAY, set it. */ if (init->shape == NULL) { - gcc_assert (init->expr_type == EXPR_ARRAY); + if (init->expr_type != EXPR_ARRAY) + { + gfc_error ("Bad shape of initializer at %L", &init->where); + return false; + } + init->shape = gfc_get_shape (1); if (!gfc_array_size (init, &init->shape[0])) - gfc_internal_error ("gfc_array_size failed"); + { + gfc_error ("Cannot determine shape of initializer at %L", + &init->where); + free (init->shape); + init->shape = NULL; + return false; + } } for (dim = 0; dim < sym->as->rank; ++dim) diff --git a/gcc/testsuite/gfortran.dg/pr106209.f90 b/gcc/testsuite/gfortran.dg/pr106209.f90 new file mode 100644 index 000..44f9233ec2f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr106209.f90 @@ -0,0 +1,9 @@ +! { dg-do compile } +! PR fortran/106209 - ICE in add_init_expr_to_sym +! Contributed by G.Steinmetz + +program p + integer, parameter :: a(:) = 0 ! { dg-error "of deferred shape" } + integer, parameter :: b(*) = a ! { dg-error "Bad shape of initializer" } + integer, parameter :: c(*) = [a] ! { dg-error "Cannot determine shape" } +end -- 2.35.3
[PATCH, committed] Fortran: do not generate conflicting results under -ff2c [PR104313]
Dear all, the attached patch by Steve fixes a regression under -ff2c for functions where the result is not set. There would otherwise be conflicting declarations of the returned result, which gimple doesn't like. I've committed this as obvious after discussion with Steve for him, see PR, as commit r13-1715-g517fb1a78102df43f052c6934c27dd51d786aff7 This fixes a 10/11/12/13 regression, will be backported in the next days. Thanks, Harald From 517fb1a78102df43f052c6934c27dd51d786aff7 Mon Sep 17 00:00:00 2001 From: Steve Kargl Date: Fri, 15 Jul 2022 22:07:15 +0200 Subject: [PATCH] Fortran: do not generate conflicting results under -ff2c [PR104313] gcc/fortran/ChangeLog: PR fortran/104313 * trans-decl.cc (gfc_generate_return): Do not generate conflicting fake results for functions with no result variable under -ff2c. gcc/testsuite/ChangeLog: PR fortran/104313 * gfortran.dg/pr104313.f: New test. --- gcc/fortran/trans-decl.cc| 2 +- gcc/testsuite/gfortran.dg/pr104313.f | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/pr104313.f diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc index 6493cc2f6b1..908a4c6d42e 100644 --- a/gcc/fortran/trans-decl.cc +++ b/gcc/fortran/trans-decl.cc @@ -6474,7 +6474,7 @@ gfc_generate_return (void) NULL_TREE, and a 'return' is generated without a variable. The following generates a 'return __result_XXX' where XXX is the function name. */ - if (sym == sym->result && sym->attr.function) + if (sym == sym->result && sym->attr.function && !flag_f2c) { result = gfc_get_fake_result_decl (sym, 0); result = fold_build2_loc (input_location, MODIFY_EXPR, diff --git a/gcc/testsuite/gfortran.dg/pr104313.f b/gcc/testsuite/gfortran.dg/pr104313.f new file mode 100644 index 000..89c8947cb0a --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr104313.f @@ -0,0 +1,11 @@ +! { dg-do compile } +! { dg-additional-options "-ff2c -fdump-tree-original" } +! +! PR fortran/104313 - ICE verify_gimple failed with -ff2c +! Contributed by G.Steinmetz + + function f(a) + return + end + +! { dg-final { scan-tree-dump-times "return" 1 "original" } } -- 2.35.3
[PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]
Dear all, I intend to commit the attached patch as obvious to mainline within the next 24h unless someone complains. It replaces a lazy gfc_internal_error by an explicit error message and an error recovery path. As a side-effect, we now diagnose a previously missed error in testcase gfortran.dg/associate_54.f90 similarly to Intel. Regtested on x86_64-pc-linux-gnu. Thanks, Harald From e6ecc4d8227afea565b0555e95a4f5dcb8f4ecab Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 18 Jul 2022 22:34:53 +0200 Subject: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590] gcc/fortran/ChangeLog: PR fortran/103590 * resolve.cc (find_array_spec): Change function result to bool to enable error recovery. Generate error message for missing array spec instead of an internal error. (gfc_resolve_ref): Use function result from find_array_spec for error recovery. gcc/testsuite/ChangeLog: PR fortran/103590 * gfortran.dg/associate_54.f90: Adjust. * gfortran.dg/associate_59.f90: New test. --- gcc/fortran/resolve.cc | 13 ++--- gcc/testsuite/gfortran.dg/associate_54.f90 | 3 +-- gcc/testsuite/gfortran.dg/associate_59.f90 | 9 + 3 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/associate_59.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 2ebf076f730..dacd33561d0 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -4976,7 +4976,7 @@ gfc_resolve_dim_arg (gfc_expr *dim) static void resolve_assoc_var (gfc_symbol* sym, bool resolve_target); -static void +static bool find_array_spec (gfc_expr *e) { gfc_array_spec *as; @@ -5004,7 +5004,11 @@ find_array_spec (gfc_expr *e) { case REF_ARRAY: if (as == NULL) - gfc_internal_error ("find_array_spec(): Missing spec"); + { + gfc_error ("Symbol %qs at %L has not been declared as an array", + e->symtree->n.sym->name, &e->where); + return false; + } ref->u.ar.as = as; as = NULL; @@ -5028,6 +5032,8 @@ find_array_spec (gfc_expr *e) if (as != NULL) gfc_internal_error ("find_array_spec(): unused as(2)"); + + return true; } @@ -5346,7 +5352,8 @@ gfc_resolve_ref (gfc_expr *expr) for (ref = expr->ref; ref; ref = ref->next) if (ref->type == REF_ARRAY && ref->u.ar.as == NULL) { - find_array_spec (expr); + if (!find_array_spec (expr)) + return false; break; } diff --git a/gcc/testsuite/gfortran.dg/associate_54.f90 b/gcc/testsuite/gfortran.dg/associate_54.f90 index 003175a47fd..b23a4f160ac 100644 --- a/gcc/testsuite/gfortran.dg/associate_54.f90 +++ b/gcc/testsuite/gfortran.dg/associate_54.f90 @@ -26,9 +26,8 @@ contains integer, intent(in) :: a associate (state => obj%state(TEST_STATES)) ! { dg-error "is used as array" } ! state = a - state(TEST_STATE) = a + state(TEST_STATE) = a ! { dg-error "has not been declared as an array" } end associate end subroutine test_alter_state1 end module test - diff --git a/gcc/testsuite/gfortran.dg/associate_59.f90 b/gcc/testsuite/gfortran.dg/associate_59.f90 new file mode 100644 index 000..2da97731d39 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/associate_59.f90 @@ -0,0 +1,9 @@ +! { dg-do compile } +! PR fortran/103590 - ICE: find_array_spec(): Missing spec +! Contributed by G.Steinmetz + +program p + associate (a => 1) +print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER expression" } + end associate +end -- 2.35.3
Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]
Hi Mikael, Am 19.07.22 um 11:03 schrieb Mikael Morin: Hello, the principle looks good, but... Le 18/07/2022 à 22:43, Harald Anlauf via Fortran a écrit : diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 2ebf076f730..dacd33561d0 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -5004,7 +5004,11 @@ find_array_spec (gfc_expr *e) { case REF_ARRAY: if (as == NULL) - gfc_internal_error ("find_array_spec(): Missing spec"); + { + gfc_error ("Symbol %qs at %L has not been declared as an array", + e->symtree->n.sym->name, &e->where); ... the error here only makes sense if the array reference follows a variable reference. If it follows a derived type component reference, a slightly different error message would be more appropriate. how detailed or tailored should the error message be, or can we just have a more generic message, like "Name at %L ...", or "Invalid subscript reference at %L"? We seem to not hit that internal error very often... I have played only little with invalid code in the present context, but often hit another code path that shows up in associate_54.f90 and gives Error: Associate-name 'state' at (1) is used as array For the testcase in the PR, Intel says: associate_59.f90(7): error #6410: This name has not been declared as an array or a function. [A] print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER expression" } ^ Crayftn 14.0 says: Improper ir tree in expr_semantics. print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER expression" } ^ ftn-873 crayftn: ERROR P, File = associate_59.f90, Line = 7, Column = 26 Invalid subscripted reference of a scalar ASSOCIATE name. gfortran's behavior during error handling is difficult to understand. While the proposed new error message is emitted for associate_54.f90, it never makes it far enough for the testcase of the present PR (associate_59.f90). Thanks, Harald
Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]
Hi Mikael, Am 19.07.22 um 22:53 schrieb Mikael Morin: It could be anything better than the (unhelpfull) internal error it is replacing. I suggest for example "Invalid array reference of a non-array entity at %L". yes, that's much better! The attached updated patch uses this. Committed: r13-1757-gf838d15641d256e21ffc126c3277b290ed743928 gfortran's behavior during error handling is difficult to understand. While the proposed new error message is emitted for associate_54.f90, it never makes it far enough for the testcase of the present PR (associate_59.f90). Indeed. We try to match several types of statement one after the other, and each failed match can possibly register an error. But it is emitted only if all have failed (see gfc_error_check). There is no choice of the best error; the last one registered wins. This buffering behavior is controlled by calls to gfc_buffer_error(...). Error handling during resolution doesn’t need to delay error reporting as far as I know, and the calls to gfc_buffer_error (false) seem to correctly disable buffering at the end of every call to next_statement. I don’t know why it remains active in this case. Alright, I should try to remember this and take a closer look next time I get confused about not getting the error message I wanted... Thanks, Harald
[PATCH, committed] Fortran: fix parsing of omp task affinity iterator clause [PR101330]
Dear all, there was some left-over code - likely from development - that could lead to a compiler segfault when given invalid input. Steve found the offending line. Removing it solves the issue. The fix was acknowledged by Tobias in the PR. Regtested on x86_64-pc-linux-gnu. Pushed as: r13-1767-g26bbe78f77f73bb66af1ac13d0deec888a3c6510 Will backport to 12-branch, as the offending code was introduced there. Thanks, Harald From 26bbe78f77f73bb66af1ac13d0deec888a3c6510 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 20 Jul 2022 20:40:23 +0200 Subject: [PATCH] Fortran: fix parsing of omp task affinity iterator clause [PR101330] gcc/fortran/ChangeLog: PR fortran/101330 * openmp.cc (gfc_match_iterator): Remove left-over code from development that could lead to a crash on invalid input. gcc/testsuite/ChangeLog: PR fortran/101330 * gfortran.dg/gomp/affinity-clause-7.f90: New test. --- gcc/fortran/openmp.cc | 1 - .../gfortran.dg/gomp/affinity-clause-7.f90| 19 +++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/gomp/affinity-clause-7.f90 diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc index bd4ff259fe0..df9cdf43eb7 100644 --- a/gcc/fortran/openmp.cc +++ b/gcc/fortran/openmp.cc @@ -1181,7 +1181,6 @@ gfc_match_iterator (gfc_namespace **ns, bool permit_var) } if (':' == gfc_peek_ascii_char ()) { - step = gfc_get_expr (); if (gfc_match (": %e ", &step) != MATCH_YES) { gfc_free_expr (begin); diff --git a/gcc/testsuite/gfortran.dg/gomp/affinity-clause-7.f90 b/gcc/testsuite/gfortran.dg/gomp/affinity-clause-7.f90 new file mode 100644 index 000..5b1ca85aba3 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/gomp/affinity-clause-7.f90 @@ -0,0 +1,19 @@ +! { dg-do compile } +! PR fortran/101330 - ICE in free_expr0(): Bad expr type +! Contributed by G.Steinmetz + + implicit none + integer :: j, b(10) +!$omp task affinity (iterator(j=1:2:1) : b(j)) +!$omp end task +!$omp task affinity (iterator(j=1:2:) : b(j)) ! { dg-error "Invalid character" } +!!$omp end task +!$omp task affinity (iterator(j=1:2: ! { dg-error "Invalid character" } +!!$omp end task +!$omp task affinity (iterator(j=1:2:) ! { dg-error "Invalid character" } +!!$omp end task +!$omp task affinity (iterator(j=1:2::)! { dg-error "Invalid character" } +!!$omp end task +!$omp task affinity (iterator(j=1:2:))! { dg-error "Invalid character" } +!!$omp end task +end -- 2.35.3
[PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]
Dear all, the rank check for ASSOCIATED (POINTER, TARGET) did not allow all rank combinations that were allowed in pointer assignment for newer versions of the Fortran standard (F2008+). Fix the logic. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 338b43aefece04435d32f961c33d217aaa511095 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 21 Jul 2022 22:02:58 +0200 Subject: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652] gcc/fortran/ChangeLog: PR fortran/77652 * check.cc (gfc_check_associated): Make the rank check of POINTER vs. TARGET match the selected Fortran standard. gcc/testsuite/ChangeLog: PR fortran/77652 * gfortran.dg/associated_target_9a.f90: New test. * gfortran.dg/associated_target_9b.f90: New test. --- gcc/fortran/check.cc | 16 +-- .../gfortran.dg/associated_target_9a.f90 | 27 +++ .../gfortran.dg/associated_target_9b.f90 | 15 +++ 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9a.f90 create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9b.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 91d87a1b2c1..6d3a4701950 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -1502,8 +1502,20 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target) t = false; /* F2018 C838 explicitly allows an assumed-rank variable as the first argument of intrinsic inquiry functions. */ - if (pointer->rank != -1 && !rank_check (target, 0, pointer->rank)) -t = false; + if (pointer->rank != -1 && pointer->rank != target->rank) +{ + if (target->rank != 1) + { + if (!gfc_notify_std (GFC_STD_F2008, "Rank remapping target is not " + "rank 1 at %L", &target->where)) + t = false; + } + else if ((gfc_option.allow_std & GFC_STD_F2003) == 0) + { + if (!rank_check (target, 0, pointer->rank)) + t = false; + } +} if (target->rank > 0 && target->ref) { for (i = 0; i < target->rank; i++) diff --git a/gcc/testsuite/gfortran.dg/associated_target_9a.f90 b/gcc/testsuite/gfortran.dg/associated_target_9a.f90 new file mode 100644 index 000..708645d5bcb --- /dev/null +++ b/gcc/testsuite/gfortran.dg/associated_target_9a.f90 @@ -0,0 +1,27 @@ +! { dg-do run } +! { dg-options "-std=f2018" } +! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped +! Contributed by Paul Thomas + +program p + real, dimension(100), target :: array + real, dimension(:,:), pointer :: matrix + real, dimension(20,5), target :: array2 + real, dimension(:),pointer :: matrix2 + matrix(1:20,1:5) => array + matrix2(1:100) => array2 + ! + ! F2018:16.9.16, ASSOCIATED (POINTER [, TARGET]) + ! Case(v): If TARGET is present and is an array target, the result is + ! true if and only if POINTER is associated with a target that has + ! the same shape as TARGET, ... + if (associated (matrix, array )) stop 1 + if (associated (matrix2,array2)) stop 2 + call check (matrix2, array2) +contains + subroutine check (ptr, tgt) +real, pointer :: ptr(..) +real, target :: tgt(:,:) +if (associated (ptr, tgt)) stop 3 + end subroutine check +end diff --git a/gcc/testsuite/gfortran.dg/associated_target_9b.f90 b/gcc/testsuite/gfortran.dg/associated_target_9b.f90 new file mode 100644 index 000..ca62ab155c0 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/associated_target_9b.f90 @@ -0,0 +1,15 @@ +! { dg-do compile } +! { dg-options "-std=f2003" } +! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped +! Contributed by Paul Thomas + +subroutine s + real, dimension(100), target :: array + real, dimension(:,:), pointer :: matrix + real, dimension(20,5), target :: array2 + real, dimension(:),pointer :: matrix2 +! matrix(1:20,1:5) => array +! matrix2(1:100) => array2 + print *, associated (matrix, array ) ! Technically legal F2003 + print *, associated (matrix2,array2) ! { dg-error "is not rank 1" } +end -- 2.35.3
Re: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]
Hi Mikael, all, a discussion in the Intel compiler forum suggests that the F2018 standard prohibits such use of the ASSOCIATED intrinsic. https://community.intel.com/t5/Intel-Fortran-Compiler/Intel-rejects-ASSOCIATED-pointer-target-for-non-equal-ranks/m-p/1402799/highlight/true#M162159 As a consequence, the PR is likely invalid, as is the patch. Withdrawing. Sorry for the noise! Harald > Gesendet: Montag, 25. Juli 2022 um 12:43 Uhr > Von: "Mikael Morin" > An: "Harald Anlauf" , "fortran" , > "gcc-patches" > Betreff: Re: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank > is remapped [PR77652] > > Le 21/07/2022 à 22:12, Harald Anlauf via Fortran a écrit : > > Dear all, > > > > the rank check for ASSOCIATED (POINTER, TARGET) did not allow all > > rank combinations that were allowed in pointer assignment for > > newer versions of the Fortran standard (F2008+). Fix the logic. > > > So, if I understand correctly the (fixed) logic, it is: > f2008+=> no check > f2003 => check target’s rank different from 1 > up to f95 => check pointer’s rank equals target’s > > > I think one check is missing, that is when pointer is scalar and the > target is non-scalar (either rank 1 or not). This case should also be > rejected for f2003+, not just up to f95. > > Mikael >
Re: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]
Hi Mikael, > > https://community.intel.com/t5/Intel-Fortran-Compiler/Intel-rejects-ASSOCIATED-pointer-target-for-non-equal-ranks/m-p/1402799/highlight/true#M162159 > > > > I disagree with the conclusion. Quoting Steve Lionel’s post: > > What you're missing is this: > > > > TARGET (optional) shall be allowable as the data-target or proc-target in a > > pointer assignment statement (10.2.2) in which POINTER is > > data-pointer-object or proc-pointer-object. > > > > We then go to 10.2.2 which says (emphasis mine): > > > > C1019 (R1033) If bounds-remapping-list is not specified, the ranks of > > data-pointer-object and data-target shall be the same. > > > > So... not valid Fortran 2018. > > except, that there is also this: > > C1018 (R1033) If bounds-remapping-list is specified, the number of > > bounds-remappings shall equal the rank of data-pointer-object. > which practically imposes no conformance rule between > data-pointer-object and data-target. this is also why I initially thought that rank remapping is fine. > Note that in the syntax definition, bounds-remapping-list is not part of > data-pointer-object. In other words, by collating a > bounds-remapping-list next to POINTER, one can construct an allowable > pointer assignment from TARGET to POINTER, which satisfies the > requirement, even if TARGET and POINTER don’t have the same rank. I fully agree with you here. My current state of - sort-of - knowledge: - Crayftn 14.0 allows for rank remapping, accepts code the way you describe, including assumed-rank for the POINTER argument. - Nvidia 22.5 allows for rank remapping, but does not handle assumed-rank. - NAG 7.1 is said to reject non-equal rank. NAG 7.0 does not accept it. - Intel rejects non-equal rank. Steve Lionel even thinks that assumed-rank should not be allowed here. I believe he is wrong here. I would normally trust NAG more than Intel and Cray. If somebody else convinces me to accept that NAG has it wrong this time, I would be happy to proceed. Apart from the above discussion about what the compiler should accept, the library side of gfortran seems to be fine... :-) Harald
[PATCH] Fortran: error recovery from calculation of storage size of a symbol [PR103504]
Dear all, we currently may ICE when array bounds of a dummy argument have a non-integer type, and the procedure with the bad declaration is referenced. The same applies to bad character length of dummies. We could simply punt in such a situation, as the causing error seems to be reliably diagnosed, see testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is a really safe fix and potentially backportable to other open branches. Would that be fine? Thanks, Harald From 04bea97afd7f17083774b4309ee4d3c45e278dd3 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 25 Jul 2022 22:29:50 +0200 Subject: [PATCH] Fortran: error recovery from calculation of storage size of a symbol [PR103504] gcc/fortran/ChangeLog: PR fortran/103504 * interface.cc (get_sym_storage_size): Array bounds and character length can only be of integer type. gcc/testsuite/ChangeLog: PR fortran/103504 * gfortran.dg/pr103504.f90: New test. --- gcc/fortran/interface.cc | 7 +-- gcc/testsuite/gfortran.dg/pr103504.f90 | 28 ++ 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr103504.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index 7ed6e13711f..71eec78259b 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -2792,7 +2792,8 @@ get_sym_storage_size (gfc_symbol *sym) if (sym->ts.type == BT_CHARACTER) { if (sym->ts.u.cl && sym->ts.u.cl->length - && sym->ts.u.cl->length->expr_type == EXPR_CONSTANT) + && sym->ts.u.cl->length->expr_type == EXPR_CONSTANT + && sym->ts.u.cl->length->ts.type == BT_INTEGER) strlen = mpz_get_ui (sym->ts.u.cl->length->value.integer); else return 0; @@ -2809,7 +2810,9 @@ get_sym_storage_size (gfc_symbol *sym) for (i = 0; i < sym->as->rank; i++) { if (sym->as->upper[i]->expr_type != EXPR_CONSTANT - || sym->as->lower[i]->expr_type != EXPR_CONSTANT) + || sym->as->lower[i]->expr_type != EXPR_CONSTANT + || sym->as->upper[i]->ts.type != BT_INTEGER + || sym->as->lower[i]->ts.type != BT_INTEGER) return 0; elements *= mpz_get_si (sym->as->upper[i]->value.integer) diff --git a/gcc/testsuite/gfortran.dg/pr103504.f90 b/gcc/testsuite/gfortran.dg/pr103504.f90 new file mode 100644 index 000..607d1c6c8cc --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr103504.f90 @@ -0,0 +1,28 @@ +! { dg-do compile } +! PR fortran/103504 - ICE in get_sym_storage_size, at fortran/interface.c:2800 +! Contributed by G.Steinmetz + +program p + implicit none + real :: y(1) + character :: b + call s(y) + call t(y) + call u(y) + call c(b) +contains + subroutine s(x) +real :: x(abs(1.):1)! { dg-error "must be of INTEGER type" } + end + subroutine t(x) +real :: x(abs(1.):1)! { dg-error "must be of INTEGER type" } + end + subroutine u(x) +real :: x(1:abs(1.))! { dg-error "must be of INTEGER type" } + end + subroutine c(z) +character(len=abs(1.)) :: z ! { dg-error "must be of INTEGER type" } + end subroutine c +end + +! { dg-prune-output "must be of INTEGER type" } -- 2.35.3
[PATCH, v2] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]
Hi Mikael, Am 26.07.22 um 21:25 schrieb Mikael Morin: Le 25/07/2022 à 22:18, Harald Anlauf a écrit : I would normally trust NAG more than Intel and Cray. … and yourself, it seems. Too bad. If somebody else convinces me to accept that NAG has it wrong this time, I would be happy to proceed. It won’t convince you about NAG, but here are two reasons to proceed: - Consensus among the maintainers is sufficient; it’s the case here. - If uncertain, let’s be rather too permissive than too strict; it’s fine as long as the runtime answer is right. ok, I have thought about your comments in the review process, and played with the Cray compiler. Attached is a refined version of the patch that now rejects in addition these cases for which there are no possible related pointer assignments with bounds remapping: ASSOCIATED (scalar, array) ! impossible, cannot remap bounds ASSOCIATED (array, scalar) ! a scalar is not simply contiguous (Cray would allow those two, but IMHO these should be disallowed). See attached for version 2 with updated testcase, regtested again. I think this is what we could both be happy with... ;-) Thanks, Harald From 5432880ff21de862c64d79626aa19c4eda928cd5 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 27 Jul 2022 21:34:22 +0200 Subject: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652] gcc/fortran/ChangeLog: PR fortran/77652 * check.cc (gfc_check_associated): Make the rank check of POINTER vs. TARGET match the allowed forms of pointer assignment for the selected Fortran standard. gcc/testsuite/ChangeLog: PR fortran/77652 * gfortran.dg/associated_target_9a.f90: New test. * gfortran.dg/associated_target_9b.f90: New test. --- gcc/fortran/check.cc | 23 ++-- .../gfortran.dg/associated_target_9a.f90 | 27 +++ .../gfortran.dg/associated_target_9b.f90 | 23 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9a.f90 create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9b.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index 91d87a1b2c1..1da0b3cbe15 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -1502,8 +1502,27 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target) t = false; /* F2018 C838 explicitly allows an assumed-rank variable as the first argument of intrinsic inquiry functions. */ - if (pointer->rank != -1 && !rank_check (target, 0, pointer->rank)) -t = false; + if (pointer->rank != -1 && pointer->rank != target->rank) +{ + if (pointer->rank == 0 || target->rank == 0) + { + /* There exists no valid pointer assignment using bounds + remapping for scalar => array or array => scalar. */ + if (!rank_check (target, 0, pointer->rank)) + t = false; + } + else if (target->rank != 1) + { + if (!gfc_notify_std (GFC_STD_F2008, "Rank remapping target is not " + "rank 1 at %L", &target->where)) + t = false; + } + else if ((gfc_option.allow_std & GFC_STD_F2003) == 0) + { + if (!rank_check (target, 0, pointer->rank)) + t = false; + } +} if (target->rank > 0 && target->ref) { for (i = 0; i < target->rank; i++) diff --git a/gcc/testsuite/gfortran.dg/associated_target_9a.f90 b/gcc/testsuite/gfortran.dg/associated_target_9a.f90 new file mode 100644 index 000..708645d5bcb --- /dev/null +++ b/gcc/testsuite/gfortran.dg/associated_target_9a.f90 @@ -0,0 +1,27 @@ +! { dg-do run } +! { dg-options "-std=f2018" } +! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped +! Contributed by Paul Thomas + +program p + real, dimension(100), target :: array + real, dimension(:,:), pointer :: matrix + real, dimension(20,5), target :: array2 + real, dimension(:),pointer :: matrix2 + matrix(1:20,1:5) => array + matrix2(1:100) => array2 + ! + ! F2018:16.9.16, ASSOCIATED (POINTER [, TARGET]) + ! Case(v): If TARGET is present and is an array target, the result is + ! true if and only if POINTER is associated with a target that has + ! the same shape as TARGET, ... + if (associated (matrix, array )) stop 1 + if (associated (matrix2,array2)) stop 2 + call check (matrix2, array2) +contains + subroutine check (ptr, tgt) +real, pointer :: ptr(..) +real, target :: tgt(:,:) +if (associated (ptr, tgt)) stop 3 + end subroutine check +end diff --git a/gcc/testsuite/gfortran.dg/associated_target_9b.f90 b/gcc/testsuite/gfortran.dg/associated_target_9b.f90 new file mode 100644 index 000..1daa0a7dde1 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/associated_target_9b.f90 @@ -0,0 +1,23 @@ +! { dg-do compile } +! { dg-options "-std=f2003" } +! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped +! Contributed by Paul Thomas + +subroutine s + real, dimension(100), target :: array + real, dimensio
[PATCH] Fortran: detect blanks within literal constants in free-form mode [PR92805]
Dear all, in free-form mode, blanks are significant, so they cannot appear in literal constants, especially not before or after the "_" that separates the literal and the kind specifier. The initial patch from Steve addressed numerical literals, which I completed by adjusting the parsing of string literals. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From f58c00f5792d6ec0037696df733857580a029ba9 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 28 Jul 2022 22:07:02 +0200 Subject: [PATCH] Fortran: detect blanks within literal constants in free-form mode [PR92805] gcc/fortran/ChangeLog: PR fortran/92805 * primary.cc (get_kind): Do not skip over blanks in free-form mode. (match_string_constant): Likewise. gcc/testsuite/ChangeLog: PR fortran/92805 * gfortran.dg/literal_constants.f: New test. * gfortran.dg/literal_constants.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/primary.cc| 18 -- gcc/testsuite/gfortran.dg/literal_constants.f | 20 .../gfortran.dg/literal_constants.f90 | 24 +++ 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f90 diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 3f01f67cd49..9d200cdf65b 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -92,14 +92,21 @@ get_kind (int *is_iso_c) { int kind; match m; + char c; *is_iso_c = 0; + c = gfc_peek_ascii_char (); + if (gfc_current_form == FORM_FREE && gfc_is_whitespace (c)) +return -2; + if (gfc_match_char ('_') != MATCH_YES) return -2; - m = match_kind_param (&kind, is_iso_c); - if (m == MATCH_NO) + m = MATCH_NO; + c = gfc_peek_ascii_char (); + if ((gfc_current_form == FORM_FREE && gfc_is_whitespace (c)) + || (m = match_kind_param (&kind, is_iso_c)) == MATCH_NO) gfc_error ("Missing kind-parameter at %C"); return (m == MATCH_YES) ? kind : -1; @@ -1074,6 +1081,9 @@ match_string_constant (gfc_expr **result) c = gfc_next_char (); } + if (gfc_current_form == FORM_FREE && gfc_is_whitespace (c)) +goto no_match; + if (c == ' ') { gfc_gobble_whitespace (); @@ -1083,6 +1093,10 @@ match_string_constant (gfc_expr **result) if (c != '_') goto no_match; + c = gfc_peek_ascii_char (); + if (gfc_current_form == FORM_FREE && gfc_is_whitespace (c)) +goto no_match; + gfc_gobble_whitespace (); c = gfc_next_char (); diff --git a/gcc/testsuite/gfortran.dg/literal_constants.f b/gcc/testsuite/gfortran.dg/literal_constants.f new file mode 100644 index 000..4d1f1b7eb4c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/literal_constants.f @@ -0,0 +1,20 @@ +! { dg-do compile } +! { dg-options "-ffixed-form" } +! PR fortran/92805 - blanks within literal constants in fixed-form mode + + implicit none + integer, parameter :: ck = kind ("a") ! default character kind + integer, parameter :: rk = kind (1.0) ! default real kind + print *, 1_"abc" + print *, 1 _"abc" + print *, 1_ "abc" + print *, ck_"a" + print *, ck _"ab" + print *, ck_ "ab" + print *, 3.1415_4 + print *, 3.1415 _4 + print *, 3.1415_ 4 + print *, 3.1415_rk + print *, 3.1415 _rk + print *, 3.1415_ rk + end diff --git a/gcc/testsuite/gfortran.dg/literal_constants.f90 b/gcc/testsuite/gfortran.dg/literal_constants.f90 new file mode 100644 index 000..f8908f9ad76 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/literal_constants.f90 @@ -0,0 +1,24 @@ +! { dg-do compile } +! { dg-options "-ffree-form" } +! PR fortran/92805 - blanks within literal constants in free-form mode + + implicit none + integer, parameter :: ck = kind ("a") ! default character kind + integer, parameter :: rk = kind (1.0) ! default real kind + print *, 1_"abc" + print *, 1 _"abc" ! { dg-error "Syntax error" } + print *, 1_ "abc" ! { dg-error "Missing kind-parameter" } + print *, 1 _ "abc" ! { dg-error "Syntax error" } + print *, ck_"a" + print *, ck _"ab" ! { dg-error "Syntax error" } + print *, ck_ "ab" ! { dg-error "Syntax error" } + print *, ck _ "ab" ! { dg-error "Syntax error" } + print *, 3.1415_4 + print *, 3.1415 _4 ! { dg-error "Syntax error" } + print *, 3.1415_ 4 ! { dg-error "Missing kind-parameter" } + print *, 3.1415 _ 4 ! { dg-error "Syntax error" } + print *, 3.1415_rk + print *, 3.1415 _rk ! { dg-error "Syntax error" } + print *, 3.1415_ rk ! { dg-error "Missing kind-parameter" } + print *, 3.141 _ rk ! { dg-error "Syntax error" } + end -- 2.35.3
[PATCH, v2] Fortran: detect blanks within literal constants in free-form mode [PR92805]
Hi Mikael, Am 29.07.22 um 13:11 schrieb Mikael Morin: Hello, Le 28/07/2022 à 22:11, Harald Anlauf via Fortran a écrit : Dear all, in free-form mode, blanks are significant, so they cannot appear in literal constants, especially not before or after the "_" that separates the literal and the kind specifier. The initial patch from Steve addressed numerical literals, which I completed by adjusting the parsing of string literals. Regtested on x86_64-pc-linux-gnu. OK for mainline? It looks correct, but I think we should continue to have the free vs fixed form abstracted away from the parsing code. yes, that makes sense. So, I suggest instead to remove the calls to gfc_gobble_whitespace in match_string_constant, Indeed, removing these simplifies the function and indeed works! > and use gfc_next_char instead of gfc_match_char in get_kind. There is one important functionality in gfc_match_char(): it manages the locus. We would need then to add this explicitly to get_kind, which does not look to me like a big improvement over the present solution. Otherwise I get test regressions. Mikael I've attached a revised version with improved match_string_constant(). What do you think? Thanks, Harald From f8e7c297b7c9e5a2b22185c7e0d638764c33aa71 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 28 Jul 2022 22:07:02 +0200 Subject: [PATCH] Fortran: detect blanks within literal constants in free-form mode [PR92805] gcc/fortran/ChangeLog: PR fortran/92805 * primary.cc (get_kind): Do not skip over blanks in free-form mode. (match_string_constant): Likewise. gcc/testsuite/ChangeLog: PR fortran/92805 * gfortran.dg/literal_constants.f: New test. * gfortran.dg/literal_constants.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/primary.cc| 19 +++ gcc/testsuite/gfortran.dg/literal_constants.f | 20 .../gfortran.dg/literal_constants.f90 | 24 +++ 3 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f90 diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 3f01f67cd49..604f98a8dd9 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -92,14 +92,21 @@ get_kind (int *is_iso_c) { int kind; match m; + char c; *is_iso_c = 0; + c = gfc_peek_ascii_char (); + if (gfc_current_form == FORM_FREE && gfc_is_whitespace (c)) +return -2; + if (gfc_match_char ('_') != MATCH_YES) return -2; - m = match_kind_param (&kind, is_iso_c); - if (m == MATCH_NO) + m = MATCH_NO; + c = gfc_peek_ascii_char (); + if ((gfc_current_form == FORM_FREE && gfc_is_whitespace (c)) + || (m = match_kind_param (&kind, is_iso_c)) == MATCH_NO) gfc_error ("Missing kind-parameter at %C"); return (m == MATCH_YES) ? kind : -1; @@ -1074,17 +1081,9 @@ match_string_constant (gfc_expr **result) c = gfc_next_char (); } - if (c == ' ') -{ - gfc_gobble_whitespace (); - c = gfc_next_char (); -} - if (c != '_') goto no_match; - gfc_gobble_whitespace (); - c = gfc_next_char (); if (c != '\'' && c != '"') goto no_match; diff --git a/gcc/testsuite/gfortran.dg/literal_constants.f b/gcc/testsuite/gfortran.dg/literal_constants.f new file mode 100644 index 000..4d1f1b7eb4c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/literal_constants.f @@ -0,0 +1,20 @@ +! { dg-do compile } +! { dg-options "-ffixed-form" } +! PR fortran/92805 - blanks within literal constants in fixed-form mode + + implicit none + integer, parameter :: ck = kind ("a") ! default character kind + integer, parameter :: rk = kind (1.0) ! default real kind + print *, 1_"abc" + print *, 1 _"abc" + print *, 1_ "abc" + print *, ck_"a" + print *, ck _"ab" + print *, ck_ "ab" + print *, 3.1415_4 + print *, 3.1415 _4 + print *, 3.1415_ 4 + print *, 3.1415_rk + print *, 3.1415 _rk + print *, 3.1415_ rk + end diff --git a/gcc/testsuite/gfortran.dg/literal_constants.f90 b/gcc/testsuite/gfortran.dg/literal_constants.f90 new file mode 100644 index 000..f8908f9ad76 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/literal_constants.f90 @@ -0,0 +1,24 @@ +! { dg-do compile } +! { dg-options "-ffree-form" } +! PR fortran/92805 - blanks within literal constants in free-form mode + + implicit none + integer, parameter :: ck = kind ("a") ! default character kind + integer, parameter :: rk = kind (1.0) ! default real kind + print *, 1_"abc" + print *, 1 _"abc" ! { dg-error "Syntax error" } + print *, 1_ "abc" ! { dg-error "Missing kind-parameter" } + print *, 1 _ "abc" ! { dg-error "Syntax error" } + print *, ck_"a" + print *, ck _"ab" ! { dg-error "Syntax error" } + print *, ck_ "ab" ! { dg-error "Syntax error
Re: [PATCH, v2] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]
Am 28.07.22 um 22:19 schrieb Mikael Morin: Hello, Le 27/07/2022 à 21:45, Harald Anlauf via Fortran a écrit : ok, I have thought about your comments in the review process, and played with the Cray compiler. Attached is a refined version of the patch that now rejects in addition these cases for which there are no possible related pointer assignments with bounds remapping: ASSOCIATED (scalar, array) ! impossible, cannot remap bounds ASSOCIATED (array, scalar) ! a scalar is not simply contiguous In principle, it could make sense to construct a one-sized array pointer (of any rank) pointing to a scalar, but I agree that if we follow the rules of the standard to the letter, it should be rejected (and we do reject such a pointer assignment). So, with this case eliminated, this patch looks good to me as is. OK, so I will push that version soon. Regarding Toon’s suggestion to ask the fortran committee, it sounds sensible. I propose to prepare something tomorrow. Depending on the answer we can later refine the compile-time check and relax if needed. Harald
[PATCH, v3] Fortran: detect blanks within literal constants in free-form mode [PR92805]
Hi Mikael, Am 29.07.22 um 22:36 schrieb Mikael Morin: Indeed, I overlooked that, but my opinion remains that we shouldn’t play with fixed vs free form considerations here. So the options I can see are: - handle the locus in get_kind; we do it a lot already in matching functions, so it wouldn’t be different here. - implement a variant of gfc_match_char without space gobbling. - use gfc_match(...), which is a bit heavy weight to match a single char string, but otherwise would keep things concise. My preference goes to the third option, but I’m fine with either of them if you have a different one. how about the attached? This introduces the helper function gfc_match_next_char, which is your second option. Thanks, Harald From 0a95d103e4855fbcc20fd24d44b97b690d570333 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 28 Jul 2022 22:07:02 +0200 Subject: [PATCH] Fortran: detect blanks within literal constants in free-form mode [PR92805] gcc/fortran/ChangeLog: PR fortran/92805 * gfortran.h (gfc_match_next_char): Declare it. * primary.cc (get_kind): Do not skip over blanks in free-form mode. (match_string_constant): Likewise. * scanner.cc (gfc_match_next_char): New. Match next character of input, treating whitespace depending on fixed or free form. gcc/testsuite/ChangeLog: PR fortran/92805 * gfortran.dg/literal_constants.f: New test. * gfortran.dg/literal_constants.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/gfortran.h| 1 + gcc/fortran/primary.cc| 17 + gcc/fortran/scanner.cc| 17 + gcc/testsuite/gfortran.dg/literal_constants.f | 20 .../gfortran.dg/literal_constants.f90 | 24 +++ 5 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f90 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 696aadd7db6..645a30e7d49 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3209,6 +3209,7 @@ gfc_char_t gfc_next_char (void); char gfc_next_ascii_char (void); gfc_char_t gfc_peek_char (void); char gfc_peek_ascii_char (void); +match gfc_match_next_char (gfc_char_t); void gfc_error_recovery (void); void gfc_gobble_whitespace (void); void gfc_new_file (void); diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 3f01f67cd49..9fa6779200f 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -92,14 +92,17 @@ get_kind (int *is_iso_c) { int kind; match m; + char c; *is_iso_c = 0; - if (gfc_match_char ('_') != MATCH_YES) + if (gfc_match_next_char ('_') != MATCH_YES) return -2; - m = match_kind_param (&kind, is_iso_c); - if (m == MATCH_NO) + m = MATCH_NO; + c = gfc_peek_ascii_char (); + if ((gfc_current_form == FORM_FREE && gfc_is_whitespace (c)) + || (m = match_kind_param (&kind, is_iso_c)) == MATCH_NO) gfc_error ("Missing kind-parameter at %C"); return (m == MATCH_YES) ? kind : -1; @@ -1074,17 +1077,9 @@ match_string_constant (gfc_expr **result) c = gfc_next_char (); } - if (c == ' ') -{ - gfc_gobble_whitespace (); - c = gfc_next_char (); -} - if (c != '_') goto no_match; - gfc_gobble_whitespace (); - c = gfc_next_char (); if (c != '\'' && c != '"') goto no_match; diff --git a/gcc/fortran/scanner.cc b/gcc/fortran/scanner.cc index 2dff2514700..2d1980c074c 100644 --- a/gcc/fortran/scanner.cc +++ b/gcc/fortran/scanner.cc @@ -1690,6 +1690,23 @@ gfc_peek_ascii_char (void) } +/* Match next character of input. In fixed form mode, we also ignore + spaces. */ + +match +gfc_match_next_char (gfc_char_t c) +{ + locus where; + + where = gfc_current_locus; + if (gfc_next_char () == c) +return MATCH_YES; + + gfc_current_locus = where; + return MATCH_NO; +} + + /* Recover from an error. We try to get past the current statement and get lined up for the next. The next statement follows a '\n' or a ';'. We also assume that we are not within a character diff --git a/gcc/testsuite/gfortran.dg/literal_constants.f b/gcc/testsuite/gfortran.dg/literal_constants.f new file mode 100644 index 000..4d1f1b7eb4c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/literal_constants.f @@ -0,0 +1,20 @@ +! { dg-do compile } +! { dg-options "-ffixed-form" } +! PR fortran/92805 - blanks within literal constants in fixed-form mode + + implicit none + integer, parameter :: ck = kind ("a") ! default character kind + integer, parameter :: rk = kind (1.0) ! default real kind + print *, 1_"abc" + print *, 1 _"abc" + print *, 1_ "abc" + print *, ck_"a" + print *, ck _"ab" + print *, ck_ "ab" + print *, 3.1415_4 + print *, 3.1415 _4 + print *, 3.1415_ 4 + print *, 3.1415_rk + print *, 3.1415 _rk +
Re: [PATCH, v3] Fortran: detect blanks within literal constants in free-form mode [PR92805]
Hi Thomas, Am 30.07.22 um 09:46 schrieb Thomas Koenig via Fortran: Hi Harald, This introduces the helper function gfc_match_next_char, which is your second option. I would be a little bit concerned about compilation times with the additional function call overhead. the function it replaces (gfc_match_char) is also in a different file, thus the overhead is at least neutral. Given that the latter has an additional call to gfc_gobble_whitespace(), we actually get better... The function is used only in one place; would it make sense to put it into primary.cc and declare it static? Can do that. Best regards Thomas Thanks, Harald
[PATCH, v4] Fortran: detect blanks within literal constants in free-form mode [PR92805]
Hi Mikael, Am 30.07.22 um 10:28 schrieb Mikael Morin: Meh! We killed one check for gfc_current_form but the other one is still there. OK, match_kind_param calls two functions that also gobble space, so there is work remaining here. So please make match_small_literal_constant and gfc_match_name space-gobbling wrappers around space-non-gobbling inner functions and call those inner functions instead in match_kind_param. well, here's the shortest solution I could come up with. I added a new argument to 3 functions used in parsing that controls the gobbling of whitespace. We use this to handle whitespace for numerical literals, while the parsing of string literals remains as in the previous version of the patch. This version obviously ignores Thomas' request, as that would require to treat gfc_match_char specially... Regtested again. OK now? Thanks, Harald From cb33d1d0b91b371a864379d920ddaefc15d587f9 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 28 Jul 2022 22:07:02 +0200 Subject: [PATCH] Fortran: detect blanks within literal constants in free-form mode [PR92805] gcc/fortran/ChangeLog: PR fortran/92805 * match.cc (gfc_match_small_literal_int): Make gobbling of leading whitespace optional. (gfc_match_name): Likewise. (gfc_match_char): Likewise. * match.h (gfc_match_small_literal_int): Adjust prototype. (gfc_match_name): Likewise. (gfc_match_char): Likewise. * primary.cc (match_kind_param): Match small literal int or name without gobbling whitespace. (get_kind): Do not skip over blanks in free-form mode. (match_string_constant): Likewise. gcc/testsuite/ChangeLog: PR fortran/92805 * gfortran.dg/literal_constants.f: New test. * gfortran.dg/literal_constants.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/match.cc | 21 +--- gcc/fortran/match.h | 6 ++--- gcc/fortran/primary.cc| 14 +++ gcc/testsuite/gfortran.dg/literal_constants.f | 20 .../gfortran.dg/literal_constants.f90 | 24 +++ 5 files changed, 63 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f90 diff --git a/gcc/fortran/match.cc b/gcc/fortran/match.cc index 1aa3053e70e..c0dc0e89361 100644 --- a/gcc/fortran/match.cc +++ b/gcc/fortran/match.cc @@ -457,7 +457,7 @@ gfc_match_eos (void) will be set to the number of digits. */ match -gfc_match_small_literal_int (int *value, int *cnt) +gfc_match_small_literal_int (int *value, int *cnt, bool gobble_ws) { locus old_loc; char c; @@ -466,7 +466,8 @@ gfc_match_small_literal_int (int *value, int *cnt) old_loc = gfc_current_locus; *value = -1; - gfc_gobble_whitespace (); + if (gobble_ws) +gfc_gobble_whitespace (); c = gfc_next_ascii_char (); if (cnt) *cnt = 0; @@ -611,14 +612,15 @@ gfc_match_label (void) than GFC_MAX_SYMBOL_LEN. */ match -gfc_match_name (char *buffer) +gfc_match_name (char *buffer, bool gobble_ws) { locus old_loc; int i; char c; old_loc = gfc_current_locus; - gfc_gobble_whitespace (); + if (gobble_ws) +gfc_gobble_whitespace (); c = gfc_next_ascii_char (); if (!(ISALPHA (c) || (c == '_' && flag_allow_leading_underscore))) @@ -1052,16 +1054,19 @@ cleanup: } -/* Tries to match the next non-whitespace character on the input. - This subroutine does not return MATCH_ERROR. */ +/* Tries to match the next non-whitespace character on the input. This + subroutine does not return MATCH_ERROR. When gobble_ws is false, do not + skip over leading blanks. +*/ match -gfc_match_char (char c) +gfc_match_char (char c, bool gobble_ws) { locus where; where = gfc_current_locus; - gfc_gobble_whitespace (); + if (gobble_ws) +gfc_gobble_whitespace (); if (gfc_next_ascii_char () == c) return MATCH_YES; diff --git a/gcc/fortran/match.h b/gcc/fortran/match.h index 495c93e0b5c..1f53e0cb67d 100644 --- a/gcc/fortran/match.h +++ b/gcc/fortran/match.h @@ -45,14 +45,14 @@ extern gfc_access gfc_typebound_default_access; match gfc_match_special_char (gfc_char_t *); match gfc_match_space (void); match gfc_match_eos (void); -match gfc_match_small_literal_int (int *, int *); +match gfc_match_small_literal_int (int *, int *, bool = true); match gfc_match_st_label (gfc_st_label **); match gfc_match_small_int (int *); -match gfc_match_name (char *); +match gfc_match_name (char *, bool = true); match gfc_match_symbol (gfc_symbol **, int); match gfc_match_sym_tree (gfc_symtree **, int); match gfc_match_intrinsic_op (gfc_intrinsic_op *); -match gfc_match_char (char); +match gfc_match_char (char, bool = true); match gfc_match (const char *, ...); match gfc_match_iterator (gfc_iterator *, int); match gfc_match_parens (void); diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 3f01f67cd49..19f2e78c8ff 1
Re: [PATCH, v4] Fortran: detect blanks within literal constants in free-form mode [PR92805]
Hi Mikael, Am 31.07.22 um 10:35 schrieb Mikael Morin: Le 30/07/2022 à 21:40, Harald Anlauf a écrit : Hi Mikael, Am 30.07.22 um 10:28 schrieb Mikael Morin: Meh! We killed one check for gfc_current_form but the other one is still there. OK, match_kind_param calls two functions that also gobble space, so there is work remaining here. So please make match_small_literal_constant and gfc_match_name space-gobbling wrappers around space-non-gobbling inner functions and call those inner functions instead in match_kind_param. well, here's the shortest solution I could come up with. I added a new argument to 3 functions used in parsing that controls the gobbling of whitespace. We use this to handle whitespace for numerical literals, while the parsing of string literals remains as in the previous version of the patch. This version obviously ignores Thomas' request, as that would require to treat gfc_match_char specially... Regtested again. OK now? PR fortran/92805 * match.cc (gfc_match_small_literal_int): Make gobbling of leading whitespace optional. (gfc_match_name): Likewise. (gfc_match_char): Likewise. * match.h (gfc_match_small_literal_int): Adjust prototype. (gfc_match_name): Likewise. (gfc_match_char): Likewise. * primary.cc (match_kind_param): Match small literal int or name without gobbling whitespace. (get_kind): Do not skip over blanks in free-form mode. I think the "in free-form mode" applied to the preceding patches but can be dropped now. (match_string_constant): Likewise. diff --git a/gcc/fortran/match.cc b/gcc/fortran/match.cc index 1aa3053e70e..c0dc0e89361 100644 --- a/gcc/fortran/match.cc +++ b/gcc/fortran/match.cc @@ -457,7 +457,7 @@ gfc_match_eos (void) will be set to the number of digits. */ Please add a note about GOBBLE_WS here, like you did for gfc_match_char. match -gfc_match_small_literal_int (int *value, int *cnt) +gfc_match_small_literal_int (int *value, int *cnt, bool gobble_ws) { locus old_loc; char c; (...) @@ -611,14 +612,15 @@ gfc_match_label (void) than GFC_MAX_SYMBOL_LEN. */ Same here. match -gfc_match_name (char *buffer) +gfc_match_name (char *buffer, bool gobble_ws) { locus old_loc; int i; char c; (...) @@ -1052,16 +1054,19 @@ cleanup: } -/* Tries to match the next non-whitespace character on the input. - This subroutine does not return MATCH_ERROR. */ +/* Tries to match the next non-whitespace character on the input. This + subroutine does not return MATCH_ERROR. When gobble_ws is false, do not + skip over leading blanks. +*/ There should be no line feed before end of comment. I've adjusted the patch (see attached) and pushed it as commit r13-1905-gd325e7048c85e13f12ea79aebf9623eddc7ffcaf Thanks, Harald OK with those changes. thanks for your patience. Mikael From d325e7048c85e13f12ea79aebf9623eddc7ffcaf Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 28 Jul 2022 22:07:02 +0200 Subject: [PATCH] Fortran: detect blanks within literal constants in free-form mode [PR92805] gcc/fortran/ChangeLog: PR fortran/92805 * match.cc (gfc_match_small_literal_int): Make gobbling of leading whitespace optional. (gfc_match_name): Likewise. (gfc_match_char): Likewise. * match.h (gfc_match_small_literal_int): Adjust prototype. (gfc_match_name): Likewise. (gfc_match_char): Likewise. * primary.cc (match_kind_param): Match small literal int or name without gobbling whitespace. (get_kind): Do not skip over blanks. (match_string_constant): Likewise. gcc/testsuite/ChangeLog: PR fortran/92805 * gfortran.dg/literal_constants.f: New test. * gfortran.dg/literal_constants.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/match.cc | 24 --- gcc/fortran/match.h | 6 ++--- gcc/fortran/primary.cc| 14 +++ gcc/testsuite/gfortran.dg/literal_constants.f | 20 .../gfortran.dg/literal_constants.f90 | 24 +++ 5 files changed, 65 insertions(+), 23 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f create mode 100644 gcc/testsuite/gfortran.dg/literal_constants.f90 diff --git a/gcc/fortran/match.cc b/gcc/fortran/match.cc index 1aa3053e70e..8b8b6e79c8b 100644 --- a/gcc/fortran/match.cc +++ b/gcc/fortran/match.cc @@ -454,10 +454,11 @@ gfc_match_eos (void) /* Match a literal integer on the input, setting the value on MATCH_YES. Literal ints occur in kind-parameters as well as old-style character length specifications. If cnt is non-NULL it - will be set to the number of digits. */ + will be set to the number of digits. + When gobble_ws is false, do not skip over leading blanks. */ match -gfc_match_small_literal_int (int *value, int *cnt) +gfc_match_small_literal_int (int *value, int *cnt, bool gobble_ws) { locus old_loc; char c; @@ -466,7 +467,8 @@ gfc_match_sm
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(-)
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
[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
[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; + } +
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] 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, 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.
[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
[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
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
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
[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: 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] 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: [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
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 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, 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
[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 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
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] 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
[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, 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
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: Support for NOINLINE attribute
Pushed as: commit 086a1df4374962787db37c1f0d1bd9beb828f9e3 Thanks, Harald On 2/12/23 22:28, Harald Anlauf via Gcc-patches wrote: Hi Rimvydas, Gesendet: Sonntag, 12. Februar 2023 um 07:59 Uhr Von: "Rimvydas Jasinskas" An: "Harald Anlauf" Cc: "fortran" Betreff: Re: Support for NOINLINE attribute On Sat, Feb 11, 2023 at 11:26 PM Harald Anlauf wrote: I am also not a native speaker, like many others contributing, but let me quote the relevant orignal paragraph: "The @code{noreturn} keyword tells the compiler to assume that @code{fatal} cannot return. It can then optimize without regard to what would happen if @code{fatal} ever did return. This makes slightly better code. More importantly, it helps avoid spurious warnings of uninitialized variables." My reading of this original paragraph differs very much from the intention I get from the shortened version. Would you please reread? Same, from extend.texi, see gcc/testsuite/gfortran.dg/noreturn-3.f90 It is about marking dead conditional branches, so that the compiler can prove proper initialization (no -Wmaybe-uninitialized given). It should behave the same as in C frontend. True. And that's the whole point (IMHO), not silencing the compiler. Hmm both look the same to me, the silencing of false positive diagnostics is already implied by spurious. To simplify I have changed it in v2 to just: "add a hint that a given function cannot return" documentation could be expanded later. But shouldn't we rather follow what the C family of compilers in the first place does for a particular target? Most relevant libraries for Fortran code are either C/C++ or Fortran anyway, including any of the common MPI implementations, so should we care about Ada? I agree with you. I have removed SUPPORTS_WEAK check and fixed indentation in v2. Regtested cleany on x86_64-pc-linux-gnu. Regards, Rimvydas this version of the patch looks good to me, so it is basically OK to commit. There is one thing I cannot test, which is the handling of weak symbols on other platforms. A quick glance at the C testcases suggests that someone with access to either an NVPTX or MingGW target might tell whether that particular target should be excluded. So I'd like to wait for 24 hours for others to comment on this. I see that you've signed-off your patch. Do you have commit rights? Otherwise I'll commit for you. (I've CC'ed to gcc-patches@ for this purpose.) Thanks for the patch! Harald
[PATCH, committed] Fortran: error recovery after invalid use of CLASS variable [PR103475]
Dear all, the attached simple and obvious patch fixes a NULL pointer dereference on an invalid use of a CLASS variable. Committed to mainline after regtesting on x86_64-pc-linux-gnu as https://gcc.gnu.org/g:2ce7e2a83e18a27fe9c659f8667fc24f0df4ea9a Thanks, Harald From 2ce7e2a83e18a27fe9c659f8667fc24f0df4ea9a Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 13 Feb 2023 22:02:44 +0100 Subject: [PATCH] Fortran: error recovery after invalid use of CLASS variable [PR103475] gcc/fortran/ChangeLog: PR fortran/103475 * primary.cc (gfc_expr_attr): Avoid NULL pointer dereference for invalid use of CLASS variable. gcc/testsuite/ChangeLog: PR fortran/103475 * gfortran.dg/pr103475.f90: New test. --- gcc/fortran/primary.cc | 2 +- gcc/testsuite/gfortran.dg/pr103475.f90 | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/pr103475.f90 diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 28ce5fea865..1bea17d44fe 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -2770,7 +2770,7 @@ gfc_expr_attr (gfc_expr *e) { gfc_symbol *sym = e->value.function.esym->result; attr = sym->attr; - if (sym->ts.type == BT_CLASS) + 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; diff --git a/gcc/testsuite/gfortran.dg/pr103475.f90 b/gcc/testsuite/gfortran.dg/pr103475.f90 new file mode 100644 index 000..6cce5e8ebf7 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr103475.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! { dg-options "-O2 -Wall" } +! PR fortran/103475 - ICE in gfc_expr_attr +! Contributed by G.Steinmetz + +program p + type t + end type + class(t) :: x ! { dg-error "must be dummy, allocatable or pointer" } + y = x() ! { dg-error "Cannot convert invalid class" } +end -- 2.35.3
Re: nvptx: Adjust 'scan-assembler' in 'gfortran.dg/weak-1.f90' (was: Support for NOINLINE attribute)
Hi Thomas, On 2/14/23 10:35, Thomas Schwinge wrote: Hi! On 2023-02-13T18:50:23+0100, Harald Anlauf via Gcc-patches wrote: Pushed as: commit 086a1df4374962787db37c1f0d1bd9beb828f9e3 On 2/12/23 22:28, Harald Anlauf via Gcc-patches wrote: There is one thing I cannot test, which is the handling of weak symbols on other platforms. A quick glance at the C testcases suggests that someone with access to either an NVPTX or MingGW target might tell whether that particular target should be excluded. Indeed nvptx does use a different assembler syntax; I've pushed to master branch commit 8d8175869ca94c600e64e27b7676787b2a398f6e "nvptx: Adjust 'scan-assembler' in 'gfortran.dg/weak-1.f90'", see attached. thanks for taking care of this. And I'm curious, is '!GCC$ ATTRIBUTES weak' meant to be used only for weak definitions (like in 'gfortran.dg/weak-1.f90'), or also for weak declarations (which, for example, in the C world then evaluate to zero-address unless actually defined)? When I did a quick experiment, that didn't seem to work? (But may be my fault, of course.) And, orthogonally: is '!GCC$ ATTRIBUTES weak' meant to be used only for subroutines (like in 'gfortran.dg/weak-1.f90') and also functions (I suppose; test case?), or also for weak "data" in some way (which, for example, in the C world then evaluates to a zero-address unless actually defined)? It also works for functions, e.g. integer function f () !GCC$ ATTRIBUTES weak :: f print *, "weak f" f = 0 end Regarding symbols beyond procedures (subroutines, functions), I had a look at what Crayftn supports. Its manpage has: ``` WEAK Syntax and use of the WEAK directive. !DIR$ WEAK procedure_name[, procedure_name] ... !DIR$ WEAK procedure_name= stub_name[, procedure_name1= stub_name1] ... [...] The WEAK directive supports the following arguments: procedure_name A weak object in the form of a variable or procedure. stub_name A stub procedure that exists in the code. The stub_name will be called if a strong reference does not exist for procedure_name. The stub_name procedure must have the same name and dummy argument list as procedure_name. ``` However, testing e.g. with a module variable either gave an error message or assembly that suggests that this does not work, at least not with version cce/14.0.0. Could help to at least add a few more test cases, and clarify the documentation? I'm not sure whether we need to support weak symbols other than procedures in gfortran. Maybe Rimvydas can comment on this. We could clarify the documentation an reject e.g. variables using: diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc index ff64588b9a8..75c04ad7ece 100644 --- a/gcc/fortran/trans-decl.cc +++ b/gcc/fortran/trans-decl.cc @@ -814,6 +814,13 @@ gfc_finish_var_decl (tree decl, gfc_symbol * sym) && (TREE_STATIC (decl) || DECL_EXTERNAL (decl))) set_decl_tls_model (decl, decl_default_tls_model (decl)); + if ((sym->attr.ext_attr & (1 << EXT_ATTR_WEAK)) + && sym->attr.flavor != FL_PROCEDURE) +{ + gfc_error ("Symbol %qs at %L has the WEAK attribute but is not a " +"procedure", sym->name, &sym->declared_at); +} + gfc_finish_decl_attrs (decl, &sym->attr); } This would reject code like module m integer :: i, j !GCC$ ATTRIBUTES weak :: j end weak-1.f90:18:17: 18 | integer :: i, j | 1 Error: Symbol 'j' at (1) has the WEAK attribute but is not a procedure Comments and thoughts? Cheers, Harald Grüße Thomas - 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
[PATCH, committed] Fortran: error recovery on invalid assumed size reference [PR104554]
Dear all, I've committed the attached obvious and trivial patch for a NULL pointer dereference on behalf of Steve and after regtesting on x86_64-pc-linux-gnu as r13-6066-ga418129273725fd02e881e6fb5e0877287a1356c Thanks, Harald From a418129273725fd02e881e6fb5e0877287a1356c Mon Sep 17 00:00:00 2001 From: Steve Kargl Date: Wed, 15 Feb 2023 22:20:22 +0100 Subject: [PATCH] Fortran: error recovery on invalid assumed size reference [PR104554] gcc/fortran/ChangeLog: PR fortran/104554 * resolve.cc (check_assumed_size_reference): Avoid NULL pointer dereference. gcc/testsuite/ChangeLog: PR fortran/104554 * gfortran.dg/pr104554.f90: New test. --- gcc/fortran/resolve.cc | 8 +--- gcc/testsuite/gfortran.dg/pr104554.f90 | 11 +++ 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr104554.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 96c34065691..fb0745927ac 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -1670,9 +1670,11 @@ check_assumed_size_reference (gfc_symbol *sym, gfc_expr *e) /* FIXME: The comparison "e->ref->u.ar.type == AR_FULL" is wrong. What should it be? */ - if (e->ref && (e->ref->u.ar.end[e->ref->u.ar.as->rank - 1] == NULL) - && (e->ref->u.ar.as->type == AS_ASSUMED_SIZE) - && (e->ref->u.ar.type == AR_FULL)) + if (e->ref + && e->ref->u.ar.as + && (e->ref->u.ar.end[e->ref->u.ar.as->rank - 1] == NULL) + && (e->ref->u.ar.as->type == AS_ASSUMED_SIZE) + && (e->ref->u.ar.type == AR_FULL)) { gfc_error ("The upper bound in the last dimension must " "appear in the reference to the assumed size " diff --git a/gcc/testsuite/gfortran.dg/pr104554.f90 b/gcc/testsuite/gfortran.dg/pr104554.f90 new file mode 100644 index 000..099f219c85d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr104554.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR fortran/104554 - ICE in check_assumed_size_reference +! Contributed by G.Steinmetz + +program p + type t + integer :: a + end type + class(t) :: x(*) ! { dg-error "Assumed size array" } + x%a = 3 +end -- 2.35.3
[PATCH, committed] Fortran: error recovery on checking procedure argument intent [PR103608]
Dear all, I've committed the attached obvious and trivial patch for another NULL pointer dereference on behalf of Steve and after regtesting on x86_64-pc-linux-gnu as r13-6067-gc75cbeba81e5b4737a9ab7dd28cce650965535a9 Thanks, Harald From c75cbeba81e5b4737a9ab7dd28cce650965535a9 Mon Sep 17 00:00:00 2001 From: Steve Kargl Date: Wed, 15 Feb 2023 22:40:37 +0100 Subject: [PATCH] Fortran: error recovery on checking procedure argument intent [PR103608] gcc/fortran/ChangeLog: PR fortran/103608 * frontend-passes.cc (do_intent): Catch NULL pointer dereference on reference to invalid formal argument. gcc/testsuite/ChangeLog: PR fortran/103608 * gfortran.dg/pr103608.f90: New test. --- gcc/fortran/frontend-passes.cc | 3 ++- gcc/testsuite/gfortran.dg/pr103608.f90 | 14 ++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/pr103608.f90 diff --git a/gcc/fortran/frontend-passes.cc b/gcc/fortran/frontend-passes.cc index 1cbc63016da..02fcb41dbc4 100644 --- a/gcc/fortran/frontend-passes.cc +++ b/gcc/fortran/frontend-passes.cc @@ -3049,7 +3049,8 @@ do_intent (gfc_expr **e) do_sym = dl->ext.iterator->var->symtree->n.sym; if (a->expr && a->expr->symtree - && a->expr->symtree->n.sym == do_sym) + && a->expr->symtree->n.sym == do_sym + && f->sym) { if (f->sym->attr.intent == INTENT_OUT) gfc_error_now ("Variable %qs at %L set to undefined value " diff --git a/gcc/testsuite/gfortran.dg/pr103608.f90 b/gcc/testsuite/gfortran.dg/pr103608.f90 new file mode 100644 index 000..5c37cb78dc6 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr103608.f90 @@ -0,0 +1,14 @@ +! { dg-do compile } +! { dg-options "-w" } +! PR fortran/103608 - ICE in do_intent +! Contributed by G.Steinmetz + +program p + implicit none + integer :: i + integer :: x ! { dg-error "Alternate return specifier" } + x(*) = 0 + do i = 1, 2 + print *, x(i) ! { dg-error "Missing alternate return specifier" } + end do +end -- 2.35.3
[PATCH] Fortran: improve checking of character length specification [PR96025]
Dear all, the attached patch fixes an ICE on invalid (non-integer) specification expressions for character length in function declarations. It appears that the error handling was already in place (mostly) and we need to essentially prevent run-on errors. Regtested on x86_64-pc-linux-gnu. OK for mainline? The PR is marked as a 10/11/12/13 regression, so I would like to backport this as far as it seems reasonable. Thanks, Harald From f581f63e206b54278c27a5c888c2566cb5077f11 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 20 Feb 2023 21:28:09 +0100 Subject: [PATCH] Fortran: improve checking of character length specification [PR96025] gcc/fortran/ChangeLog: PR fortran/96025 * parse.cc (check_function_result_typed): Improve type check of specification expression for character length and return status. (parse_spec): Use status from above. * resolve.cc (resolve_fntype): Prevent use of invalid specification expression for character length. gcc/testsuite/ChangeLog: PR fortran/96025 * gfortran.dg/pr96025.f90: New test. --- gcc/fortran/parse.cc | 23 --- gcc/fortran/resolve.cc| 4 +++- gcc/testsuite/gfortran.dg/pr96025.f90 | 11 +++ 3 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr96025.f90 diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc index f5154d97ae8..47876a3833e 100644 --- a/gcc/fortran/parse.cc +++ b/gcc/fortran/parse.cc @@ -3974,21 +3974,30 @@ match_deferred_characteristics (gfc_typespec * ts) For return types specified in a FUNCTION prefix, the IMPLICIT rules of the scope are not yet parsed so this has to be delayed up to parse_spec. */ -static void +static bool check_function_result_typed (void) { gfc_typespec ts; gcc_assert (gfc_current_state () == COMP_FUNCTION); - if (!gfc_current_ns->proc_name->result) return; + if (!gfc_current_ns->proc_name->result) +return true; ts = gfc_current_ns->proc_name->result->ts; /* Check type-parameters, at the moment only CHARACTER lengths possible. */ /* TODO: Extend when KIND type parameters are implemented. */ if (ts.type == BT_CHARACTER && ts.u.cl && ts.u.cl->length) -gfc_expr_check_typed (ts.u.cl->length, gfc_current_ns, true); +{ + /* Reject invalid type of specification expression for length. */ + if (ts.u.cl->length->ts.type != BT_INTEGER) + return false; + + gfc_expr_check_typed (ts.u.cl->length, gfc_current_ns, true); +} + + return true; } @@ -4097,8 +4106,8 @@ loop: if (verify_now) { - check_function_result_typed (); - function_result_typed = true; + if (check_function_result_typed ()) + function_result_typed = true; } } @@ -4111,8 +4120,8 @@ loop: case ST_IMPLICIT: if (!function_result_typed) { - check_function_result_typed (); - function_result_typed = true; + if (check_function_result_typed ()) + function_result_typed = true; } goto declSt; diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index fb0745927ac..427f901a438 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -17419,7 +17419,9 @@ resolve_fntype (gfc_namespace *ns) } } - if (sym->ts.type == BT_CHARACTER) + if (sym->ts.type == BT_CHARACTER + && sym->ts.u.cl->length + && sym->ts.u.cl->length->ts.type == BT_INTEGER) gfc_traverse_expr (sym->ts.u.cl->length, sym, flag_fn_result_spec, 0); } diff --git a/gcc/testsuite/gfortran.dg/pr96025.f90 b/gcc/testsuite/gfortran.dg/pr96025.f90 new file mode 100644 index 000..ce292bd9664 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr96025.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR fortran/96025 - ICE in expr_check_typed_help +! Contributed by G.Steinmetz + +program p + print *, f() +contains + character(char(1)) function f() ! { dg-error "must be of INTEGER type" } +f = 'f' + end +end -- 2.35.3
Re: [PATCH] Fortran: improve checking of character length specification [PR96025]
Hi Thomas, Am 21.02.23 um 08:19 schrieb Thomas Koenig via Gcc-patches: Hi Harald, the attached patch fixes an ICE on invalid (non-integer) specification expressions for character length in function declarations. It appears that the error handling was already in place (mostly) and we need to essentially prevent run-on errors. Regtested on x86_64-pc-linux-gnu. OK for mainline? As a very minor matter of style, you might want to write function_result_typed = check_function_result_typed (); instead of if (check_function_result_typed ()) function_result_typed = true; I was considering that too, but believed that the logic around these places (a loop and an if) would confuse readers. Thinking again and rechecking, I've changed the patch to follow your suggestion, including a minor style cleanup. Committed as: https://gcc.gnu.org/g:6c1b825b3d6499dfeacf7c79dcf4b56a393ac204 commit r13-6265-g6c1b825b3d6499dfeacf7c79dcf4b56a393ac204 Author: Harald Anlauf Date: Mon Feb 20 21:28:09 2023 +0100 OK either way. The PR is marked as a 10/11/12/13 regression, so I would like to backport this as far as it seems reasonable. Also OK. Thanks for the patch! Thanks for the review! Harald Best regards Thomas
[PATCH] Fortran: reject invalid CHARACTER length of derived type components [PR96024]
Dear all, the attached simple patch detects and rejects CHARACTER components of derived types whose length specification is non-integer. Regtested on x86_64-pc-linux-gnu. OK for mainline? This PR is also marked as a 10/11/12/13 regression, so I would like to backport this as far as it seems reasonable. Thanks, Harald From 0a392415cb5d5486e3e660880c81d6fdbbb47285 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 21 Feb 2023 22:06:33 +0100 Subject: [PATCH] Fortran: reject invalid CHARACTER length of derived type components [PR96024] gcc/fortran/ChangeLog: PR fortran/96024 * resolve.cc (resolve_component): The type of a CHARACTER length expression must be INTEGER. gcc/testsuite/ChangeLog: PR fortran/96024 * gfortran.dg/pr96024.f90: New test. --- gcc/fortran/resolve.cc| 13 + gcc/testsuite/gfortran.dg/pr96024.f90 | 11 +++ 2 files changed, 24 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr96024.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 427f901a438..2780c82c798 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -14892,6 +14892,19 @@ resolve_component (gfc_component *c, gfc_symbol *sym) c->ts.u.cl->length ? &c->ts.u.cl->length->where : &c->loc); return false; } + + if (c->ts.u.cl->length && c->ts.u.cl->length->ts.type != BT_INTEGER) + { + if (!c->ts.u.cl->length->error) + { + gfc_error ("Character length expression of component %qs at %L " + "must be of INTEGER type, found %s", + c->name, &c->ts.u.cl->length->where, + gfc_basic_typename (c->ts.u.cl->length->ts.type)); + c->ts.u.cl->length->error = 1; + } + return false; + } } if (c->ts.type == BT_CHARACTER && c->ts.deferred diff --git a/gcc/testsuite/gfortran.dg/pr96024.f90 b/gcc/testsuite/gfortran.dg/pr96024.f90 new file mode 100644 index 000..2c914a997f2 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr96024.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR fortran/96024 - ICE in mio_name_expr_t +! Contributed by G.Steinmetz + +module m + implicit none + type t + character(char(1)) :: a ! { dg-error "must be of INTEGER type" } + end type + type(t) :: z = t('a') +end -- 2.35.3
[PATCH, committed] Fortran: frontend passes do_subscript leaks gmp memory [PR108924]
Dear all, as reported by Richard - although without a testcase - we leak gmp memory in do_subscript(). The attached patch was derived by inspection of the code pointed at by valgrind and regtested on x86_64-pc-linux-gnu. Committed as obvious as commit r13-6336-g45f406c4f62e516b58dcda20b5a7aa43ff0aa0f3 Author: Harald Anlauf Date: Fri Feb 24 19:56:32 2023 +0100 Thanks, Harald From 45f406c4f62e516b58dcda20b5a7aa43ff0aa0f3 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 24 Feb 2023 19:56:32 +0100 Subject: [PATCH] Fortran: frontend passes do_subscript leaks gmp memory [PR108924] gcc/fortran/ChangeLog: PR fortran/108924 * frontend-passes.cc (do_subscript): Clear used gmp variable. --- gcc/fortran/frontend-passes.cc | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gcc/fortran/frontend-passes.cc b/gcc/fortran/frontend-passes.cc index 02fcb41dbc4..90428982023 100644 --- a/gcc/fortran/frontend-passes.cc +++ b/gcc/fortran/frontend-passes.cc @@ -2883,7 +2883,10 @@ do_subscript (gfc_expr **e) have_do_end = false; if (!have_do_start && !have_do_end) - return 0; + { + mpz_clear (do_step); + return 0; + } /* No warning inside a zero-trip loop. */ if (have_do_start && have_do_end) -- 2.35.3
Re: [Patch] Fortran: Skip bound conv in gfc_conv_gfc_desc_to_cfi_desc with intent(out) ptr [PR108621]
Hi Tobias, Am 24.02.23 um 12:31 schrieb Tobias Burnus: (B) The attached patch: With 'intent(out)' there is no reason to do the conversions. While for nullified pointers the bounds-conversion loop is skipped, it may still be executed for undefined pointers. (Which is usually harmless.) In either case, not generating this code makes sense. OK for mainline? LGTM. I was pondering whether one should keep the testcase closer to the one in the PR, but the essence of the bug and the fix is well represented in the reduced version, and also the tree dump tells the whole story anyway. Regarding GCC 12: I am not really sure as it is no real regression. Besides bogus warnings, there might be an issue for undefined pointers and -fsanitize=undefined, namely if 'ubound - lbound' evaluated on random numbers overflows (such as for ubound = huge(..) and lbound = -huge(..)). But that looks like a rather special case. - Thoughts? I'd rather consider the case of undefined pointers as of practical importance. It's up to you or others to decide whether it should be backported. I would not oppose. Thanks for the patch! Harald 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: Support for WEAK attribute, part 2
Hi Rimvydas, Am 24.02.23 um 06:16 schrieb Rimvydas Jasinskas via Gcc-patches: On Thu, Feb 23, 2023 at 10:53 PM Harald Anlauf wrote: the patch is mostly fine, but there is a minor style issue: + if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK)) + gfc_error ("Symbol %qs at %L has the WEAK attribute but is a %s", + sym->name, &sym->declared_at, sym->attr.dummy + ? "dummy argument" : "local variable"); + It is my understanding that this is not translation-friendly. Please use separate error texts for either case instead. Interesting, I was under the impression this was fixed with OO-inlines around the *.c rename. if this is the case, I must have missed it. > In any case, adjusted in v2 to use: + if (sym->attr.ext_attr & (1 << EXT_ATTR_WEAK)) +{ + if (sym->attr.dummy) +gfc_error ("Symbol %qs at %L has the WEAK attribute but is a " + "dummy argument", sym->name, &sym->declared_at); + else +gfc_error ("Symbol %qs at %L has the WEAK attribute but is a " + "local variable", sym->name, &sym->declared_at); +} This is ok. These testcases are dg-compile and do not go through the "-O0 -O1 -O2 -O3 -Os" options like dg-run. Combining the testcases does not reduce gfortran.sum a lot: I wasn't thinking of gfortran.sum, it's about the total overhead of the testsuite (dejagnu etc.). But thanks for combining the tests! Finally, please do not forget to CC patches to gcc-patches@ so that others can see them. Out of curiosity, what is the purpose of CC patches to gcc-patches too? Attachments are even available in web mailing list too, like in: https://gcc.gnu.org/pipermail/fortran/2023-February/058953.html Well, patches should always go the gcc-patches@, see e.g. https://gcc.gnu.org/gitwrite.html On the other hand, many *Fortran* reviewers will ignore patches there and look at them only when they are sent to fortran@. Thanks for your patch, pushed as r13-6338-gbcbeebc498126c . Harald Regards, Rimvydas
Re: fortran: Reuse associated_dummy memory if previously allocated [PR108923]
Hi Mikael, Am 25.02.23 um 17:35 schrieb Mikael Morin: Hello, Harald found a testcase with memory still leaking despite my previous patch for PR108923. That patch was fixing a leak caused by absence of memory release, the attached patch fixes a leak caused by pointer overwrite. I haven't investigated why sort_actual is called several times( which causes the memory leak) nor tried to avoid that. Theoretically, one could assert that the previous associated_dummy value is the same as the one to be written (it should be the same at each sort_actual invocation), but I have preferred to silently overwrite, and fix just the memory problem. Manually tested on Harald's testcase (predcom-1.f) and ran the full fortran testsuite on x86_64-pc-linux-gnu. OK for master and 12 and 11? LGTM. OK for master and 12-branch. It appears that 11-branch is significantly different in the respective places. get_intrinsic_dummy_arg does not exist there, so this patch seems to not apply there. Or am I missing something? Thanks for the patch! Harald
[PATCH, committed] Fortran: fix memory leak with real to integer conversion warning
Dear all, while checking f951 for memory leaks on testcases that appeared relevant during work on pr108924, I found that the conversion warning triggered by do_subscript_6.f90 uses a code path that forgot to mpfr_clear a used variable. The attached obvious patch fixes this - verified by valgrind. Pushed to mainline as r13-6344-g03c60e525bea13 . Thanks, Harald From 03c60e525bea13c15edd2f64cd582f168fe80bfb Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 25 Feb 2023 19:05:38 +0100 Subject: [PATCH] Fortran: fix memory leak with real to integer conversion warning gcc/fortran/ChangeLog: * arith.cc (gfc_real2int): Clear mpfr variable after use. --- gcc/fortran/arith.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index d0d1c0b03d2..37aeaf1b186 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -2257,6 +2257,7 @@ gfc_real2int (gfc_expr *src, int kind) gfc_typename (&result->ts), &src->where); did_warn = true; } + mpfr_clear (f); } if (!did_warn && warn_conversion_extra) { -- 2.35.3
Re: [patch, libgfortran] Initailize some variable to get rid of nuisance warnings.
Hi Jerry, regarding PACK: since this is a bogus warning as the compiler does not realize that dim >= 1, wouldn't a gcc_assert (dim >= 1); in the right place achieve the same effect, since the first argument must be an array? (It's different for SPREAD, though, where SOURCE may be scalar). Cheers, Harald Am 26.02.23 um 20:52 schrieb Jerry D via Gcc-patches: The attached patch is minor and self explanatory. I assume this should wait for gfortran 14 since no regression involved. Please advise otherwise. Regression tested on x86-64. OK for trunk when the time is right? Regards, Jerry Author: Jerry DeLisle Date: Sat Feb 25 20:30:35 2023 -0800 Fortran: Eliminate nuisance warnings by initializing. Set sstride[0] and mstide[0] to zero, eliminating some warnings. libgfortran/ChangeLog: * generated/pack_c10.c (pack_c10): Regenerated. * generated/pack_c16.c (pack_c16): Regenerated. * generated/pack_c17.c (pack_c17): Regenerated. * generated/pack_c4.c (pack_c4): Regenerated. * generated/pack_c8.c (pack_c8): Regenerated. * generated/pack_i1.c (pack_i1): Regenerated. * generated/pack_i16.c (pack_i16): Regenerated. * generated/pack_i2.c (pack_i2): Regenerated. * generated/pack_i4.c (pack_i4): Regenerated. * generated/pack_i8.c (pack_i8): Regenerated. * generated/pack_r10.c (pack_r10): Regenerated. * generated/pack_r16.c (pack_r16): Regenerated. * generated/pack_r17.c (pack_r17): Regenerated. * generated/pack_r4.c (pack_r4): Regenerated. * generated/pack_r8.c (pack_r8): Regenerated. * generated/spread_c10.c (spread_c10): Regenerated. * generated/spread_c16.c (spread_c16): Regenerated. * generated/spread_c17.c (spread_c17): Regenerated. * generated/spread_c4.c (spread_c4): Regenerated. * generated/spread_c8.c (spread_c8): Regenerated. * generated/spread_i1.c (spread_i1): Regenerated. * generated/spread_i16.c (spread_i16): Regenerated. * generated/spread_i2.c (spread_i2): Regenerated. * generated/spread_i4.c (spread_i4): Regenerated. * generated/spread_i8.c (spread_i8): Regenerated. * generated/spread_r10.c (spread_r10): Regenerated. * generated/spread_r16.c (spread_r16): Regenerated. * generated/spread_r17.c (spread_r17): Regenerated. * generated/spread_r4.c (spread_r4): Regenerated. * generated/spread_r8.c (spread_r8): Regenerated. * intrinsics/execute_command_line.c (execute_command_line_i4), (execute_command_line_i8): Set estat_initial to zero. * intrinsics/pack_generic.c (pack_internal): Set sstride[0] and mstride[0] to zero. * intrinsics/spread_generic.c (spread_internal): Set sstride[0]. * m4/pack.m4: Set sstride[0] and mstride[0]. * m4/spread.m4: Set sstride[0].
[PATCH] Fortran: fix corner case of IBITS intrinsic [PR108937]
Dear all, as found by the reporter, the result of the intrinsic IBITS differed from other compilers (e.g. Intel, NAG) for the corner case that the LEN argument was equal to BIT_SIZE(I), which is explicitly allowed by the standard. We actually had an inconsistency for this case between code generated by the frontend and compile-time simplified expressions. The reporter noticed that this is related to a restriction in gcc that requires that shift widths shall be smaller than the bit sizes, and we already special case this for ISHFT. It makes sense to use the same special casing for IBITS. Attached patch fixes this and regtests on x86_64-pc-linux-gnu. OK for mainline? This issue has been there for ages. Shall this be backported or left in release branches as is? Thanks, Harald From 6844c5ecb271e091a8c913903a79eac932cf5f76 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 27 Feb 2023 21:37:11 +0100 Subject: [PATCH] Fortran: fix corner case of IBITS intrinsic [PR108937] gcc/fortran/ChangeLog: PR fortran/108937 * trans-intrinsic.cc (gfc_conv_intrinsic_ibits): Handle corner case LEN argument of IBITS equal to BITSIZE(I). gcc/testsuite/ChangeLog: PR fortran/108937 * gfortran.dg/ibits_2.f90: New test. --- gcc/fortran/trans-intrinsic.cc| 10 + gcc/testsuite/gfortran.dg/ibits_2.f90 | 32 +++ 2 files changed, 42 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/ibits_2.f90 diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc index 21eeb12ca89..3cce9c0166e 100644 --- a/gcc/fortran/trans-intrinsic.cc +++ b/gcc/fortran/trans-intrinsic.cc @@ -6638,6 +6638,7 @@ gfc_conv_intrinsic_ibits (gfc_se * se, gfc_expr * expr) tree type; tree tmp; tree mask; + tree num_bits, cond; gfc_conv_intrinsic_function_args (se, expr, args, 3); type = TREE_TYPE (args[0]); @@ -6678,8 +6679,17 @@ gfc_conv_intrinsic_ibits (gfc_se * se, gfc_expr * expr) "in intrinsic IBITS", tmp1, tmp2, nbits); } + /* The Fortran standard allows (shift width) LEN <= BIT_SIZE(I), whereas + gcc requires a shift width < BIT_SIZE(I), so we have to catch this + special case. See also gfc_conv_intrinsic_ishft (). */ + num_bits = build_int_cst (TREE_TYPE (args[2]), TYPE_PRECISION (type)); + mask = build_int_cst (type, -1); mask = fold_build2_loc (input_location, LSHIFT_EXPR, type, mask, args[2]); + cond = fold_build2_loc (input_location, GE_EXPR, logical_type_node, args[2], + num_bits); + mask = fold_build3_loc (input_location, COND_EXPR, type, cond, + build_int_cst (type, 0), mask); mask = fold_build1_loc (input_location, BIT_NOT_EXPR, type, mask); tmp = fold_build2_loc (input_location, RSHIFT_EXPR, type, args[0], args[1]); diff --git a/gcc/testsuite/gfortran.dg/ibits_2.f90 b/gcc/testsuite/gfortran.dg/ibits_2.f90 new file mode 100644 index 000..2af5542d764 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/ibits_2.f90 @@ -0,0 +1,32 @@ +! { dg-do run } +! { dg-additional-options "-fcheck=bits" } +! PR fortran/108937 - Intrinsic IBITS(I,POS,LEN) fails when LEN equals +! to BIT_SIZE(I) +! Contributed by saitofuy...@jamstec.go.jp + +program test_bits + implicit none + integer, parameter :: KT = kind (1) + integer, parameter :: lbits = bit_size (0_KT) + integer(kind=KT) :: x, y0, y1 + integer(kind=KT) :: p, l + + x = -1 + p = 0 + do l = 0, lbits + y0 = ibits (x, p, l) + y1 = ibits_1(x, p, l) + if (y0 /= y1) then +print *, l, y0, y1 +stop 1+l + end if + end do +contains + elemental integer(kind=KT) function ibits_1(I, POS, LEN) result(n) +!! IBITS(I, POS, LEN) = (I >> POS) & ~((~0) << LEN) +implicit none +integer(kind=KT),intent(in) :: I +integer, intent(in) :: POS, LEN +n = IAND (ISHFT(I, - POS), NOT(ISHFT(-1_KT, LEN))) + end function ibits_1 +end program test_bits -- 2.35.3
[PATCH] Fortran: fix CLASS attribute handling [PR106856]
Dear all, the attached patch fixes a long-standing issue with CLASS attributes when a declaration is scattered over multiple statements. The major part ("draft") of the patch is by Tobias, which I took up before it started to bit-rot too much, see PR. It is mainly about a proper updating and book-keeping of symbol attributes. While debugging the draft patch, I fixed a few disturbing memleaks in class.cc that showed up when looking at intermediate fallout. This patch also addresses issues reported in a few other PRs: pr53951, pr101101, pr104229, pr107380. These are mostly duplicates at some level. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 4600577e3ecceb2525618685f47c8a979cf9d244 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 2 Mar 2023 22:37:14 +0100 Subject: [PATCH] Fortran: fix CLASS attribute handling [PR106856] gcc/fortran/ChangeLog: PR fortran/106856 * class.cc (gfc_build_class_symbol): Handle update of attributes of existing class container. (gfc_find_derived_vtab): Fix several memory leaks. * decl.cc (attr_decl1): Manage update of symbol attributes from CLASS attributes. * primary.cc (gfc_variable_attr): OPTIONAL shall not be taken or updated from the class container. gcc/testsuite/ChangeLog: PR fortran/106856 * gfortran.dg/interface_41.f90: Remove dg-pattern from valid testcase. * gfortran.dg/class_74.f90: New test. * gfortran.dg/class_75.f90: New test. Co-authored-by: Tobias Burnus --- gcc/fortran/class.cc | 23 +++-- gcc/fortran/decl.cc| 59 +++--- gcc/fortran/primary.cc | 1 - gcc/testsuite/gfortran.dg/class_74.f90 | 41 +++ gcc/testsuite/gfortran.dg/class_75.f90 | 24 + gcc/testsuite/gfortran.dg/interface_41.f90 | 2 +- 6 files changed, 115 insertions(+), 35 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/class_74.f90 create mode 100644 gcc/testsuite/gfortran.dg/class_75.f90 diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc index ae653e74437..2eebdd4a3bb 100644 --- a/gcc/fortran/class.cc +++ b/gcc/fortran/class.cc @@ -638,6 +638,7 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, { char tname[GFC_MAX_SYMBOL_LEN+1]; char *name; + gfc_typespec *orig_ts = ts; gfc_symbol *fclass; gfc_symbol *vtab; gfc_component *c; @@ -646,9 +647,21 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, gcc_assert (as); - if (attr->class_ok) -/* Class container has already been built. */ + /* Class container has already been built with same name. */ + if (attr->class_ok + && ts->u.derived->components->attr.dimension >= attr->dimension + && ts->u.derived->components->attr.codimension >= attr->codimension + && ts->u.derived->components->attr.class_pointer >= attr->pointer + && ts->u.derived->components->attr.allocatable >= attr->allocatable) return true; + if (attr->class_ok) +{ + attr->dimension |= ts->u.derived->components->attr.dimension; + attr->codimension |= ts->u.derived->components->attr.codimension; + attr->pointer |= ts->u.derived->components->attr.class_pointer; + attr->allocatable |= ts->u.derived->components->attr.allocatable; + ts = &ts->u.derived->components->ts; +} attr->class_ok = attr->dummy || attr->pointer || attr->allocatable || attr->select_type_temporary || attr->associate_var; @@ -790,7 +803,7 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, } fclass->attr.is_class = 1; - ts->u.derived = fclass; + orig_ts->u.derived = fclass; attr->allocatable = attr->pointer = attr->dimension = attr->codimension = 0; (*as) = NULL; free (name); @@ -2344,6 +2357,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) vtab->attr.vtab = 1; vtab->attr.access = ACCESS_PUBLIC; gfc_set_sym_referenced (vtab); + free (name); name = xasprintf ("__vtype_%s", tname); gfc_find_symbol (name, ns, 0, &vtype); @@ -2447,6 +2461,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) else { /* Construct default initialization variable. */ + free (name); name = xasprintf ("__def_init_%s", tname); gfc_get_symbol (name, ns, &def_init); def_init->attr.target = 1; @@ -2480,6 +2495,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) ns->contained = sub_ns; sub_ns->resolved = 1; /* Set up procedure symbol. */ + free (name); name = xasprintf ("__copy_%s", tname); gfc_get_symbol (name, sub_ns, ©); sub_ns->proc_name = copy; @@ -2558,6 +2574,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) ns->contained = sub_ns; sub_ns->resolved = 1; /* Set up procedure symbol. */ + free (name); name = xasprintf ("__deallocate_%s", tname); gfc_get_symbol (name, sub_ns, &dealloc); sub_ns->proc_name = dealloc; diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index eec0314cf4
Re: [PATCH] Fortran: fix CLASS attribute handling [PR106856]
Hi Steve, Am 03.03.23 um 20:57 schrieb Steve Kargl via Gcc-patches: On Thu, Mar 02, 2023 at 11:03:48PM +0100, Harald Anlauf via Fortran wrote: - if (attr->class_ok) -/* Class container has already been built. */ + /* Class container has already been built with same name. */ + if (attr->class_ok + && ts->u.derived->components->attr.dimension >= attr->dimension + && ts->u.derived->components->attr.codimension >= attr->codimension + && ts->u.derived->components->attr.class_pointer >= attr->pointer + && ts->u.derived->components->attr.allocatable >= attr->allocatable) I suppose I'm a bit confused here. dimension, codimension, pointer and allocatable are 1-bit bitfields in the attr struct. These can have the values 0 and 1, so the above conditionals are always true. thanks for looking into it. The above part is from the original draft. I thought I could generate testcases that allow to exercise this part, and found a new case that is not covered by the patch and still ICEs: subroutine bar (x) class(*):: x dimension :: x(:) allocatable :: x end :-( We'll need to revisit the logic... The rest of the patch looks reasonable. If Tobias has no objections or comments, it's ok to commit once the above is explained. Thanks, Harald
Re: [PATCH, v2] Fortran: fix CLASS attribute handling [PR106856]
Hi Mikael! Am 04.03.23 um 14:56 schrieb Mikael Morin: I have found the time finally. It's not as bad as it seemed. See below. diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index eec0314cf4c..72d8c6f1c14 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc + sym->attr.pointer = CLASS_DATA(sym)->attr.class_pointer; + sym->attr.allocatable = CLASS_DATA(sym)->attr.allocatable; + sym->attr.dimension = CLASS_DATA(sym)->attr.dimension; + sym->attr.codimension = CLASS_DATA(sym)->attr.codimension; + if (as && CLASS_DATA (sym)->as) + sym->as = gfc_copy_array_spec (CLASS_DATA (sym)->as); Here the condition on 'CLASS_DATA(sym)->as' makes obviously sense, but I don't see why there is also a condition on 'as'. For example, if the array spec has been previously set on the class container's first component, and there is no array spec information in the current statement (i.e. as == NULL), sym->as will remain NULL, and a non-array class container will be built in gfc_build_class_symbol below. Very good catch! Indeed, this fixes the testcase variations. @@ -8807,6 +8785,27 @@ attr_decl1 (void) goto cleanup; } + if (sym->ts.type == BT_CLASS && sym->ts.u.derived->attr.is_class + && !as && !current_attr.pointer && !current_attr.allocatable + && !current_attr.external) + { + sym->attr.pointer = 0; + sym->attr.allocatable = 0; + sym->attr.dimension = 0; + sym->attr.codimension = 0; + gfc_free_array_spec (sym->as); sym->as should probably be reset to NULL here. Done. Maybe both calls to gfc_free_array_spec here and to gfc_copy_array_spec above can be avoided by doing a simple pointer copy? I tried that, but this produced a crash with a double-free. The attached revised version uses the above proven changes, and extends the new testcase class_74.f90 by variations of the failures remaining with version 1 so that different codepaths are tested. Regtested again on x86_64-pc-linux-gnu. Any further comments? Thanks for your very helpful review! Harald From 70cba7da18023282546b9a5d80e976fc3744d732 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 5 Oct 2022 22:25:14 +0200 Subject: [PATCH] Fortran: reject procedures and procedure pointers as IO element [PR107074] gcc/fortran/ChangeLog: PR fortran/107074 * resolve.cc (resolve_transfer): A procedure, type-bound procedure or a procedure pointer cannot be an element of an IO list. * simplify.cc (gfc_simplify_merge): Do not try to reset array lower bound for scalars. gcc/testsuite/ChangeLog: PR fortran/107074 * gfortran.dg/pr107074.f90: New test. * gfortran.dg/pr107074b.f90: New test. --- gcc/fortran/resolve.cc | 31 + gcc/fortran/simplify.cc | 3 ++- gcc/testsuite/gfortran.dg/pr107074.f90 | 11 + gcc/testsuite/gfortran.dg/pr107074b.f90 | 18 ++ 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/pr107074.f90 create mode 100644 gcc/testsuite/gfortran.dg/pr107074b.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index d133bc2d034..d9d101775f6 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -10137,6 +10137,37 @@ resolve_transfer (gfc_code *code) "an assumed-size array", &code->loc); return; } + + /* Check for procedures and procedure pointers. Fortran 2018 has: + + C1233 (R1217) An expression that is an output-item shall not have a + value that is a procedure pointer. + + There does not appear any reason to allow procedure pointers for + input, so we disallow them generally, and we reject procedures. */ + + if (exp->expr_type == EXPR_VARIABLE) +{ + /* Check for type-bound procedures. */ + for (ref = exp->ref; ref; ref = ref->next) + if (ref->type == REF_COMPONENT + && ref->u.c.component->attr.flavor == FL_PROCEDURE) + break; + + /* Procedure or procedure pointer? */ + if (exp->ts.type == BT_PROCEDURE + || (ref && ref->u.c.component->attr.flavor == FL_PROCEDURE)) + { + if (exp->symtree->n.sym->attr.proc_pointer + || (ref && ref->u.c.component->attr.proc_pointer)) + gfc_error ("Data transfer element at %L cannot be a procedure " + "pointer", &code->loc); + else + gfc_error ("Data transfer element at %L cannot be a procedure", + &code->loc); + return; + } +} } diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index 6ac92cf9db8..f0482d349af 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -4915,7 +4915,8 @@ gfc_simplify_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask) { result = gfc_copy_expr (mask->value.logical ? tsource : fsource); /* Parenthesis is needed to get lower bounds of 1. */ - result = gfc_get_parentheses (result); + if (result->rank) + result = gfc_get_parentheses (result); gfc_sim
Re: [PATCH, v2] Fortran: fix CLASS attribute handling [PR106856]
Hi Mikael! Am 04.03.23 um 14:56 schrieb Mikael Morin: I have found the time finally. It's not as bad as it seemed. See below. diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index eec0314cf4c..72d8c6f1c14 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc + sym->attr.pointer = CLASS_DATA(sym)->attr.class_pointer; + sym->attr.allocatable = CLASS_DATA(sym)->attr.allocatable; + sym->attr.dimension = CLASS_DATA(sym)->attr.dimension; + sym->attr.codimension = CLASS_DATA(sym)->attr.codimension; + if (as && CLASS_DATA (sym)->as) + sym->as = gfc_copy_array_spec (CLASS_DATA (sym)->as); Here the condition on 'CLASS_DATA(sym)->as' makes obviously sense, but I don't see why there is also a condition on 'as'. For example, if the array spec has been previously set on the class container's first component, and there is no array spec information in the current statement (i.e. as == NULL), sym->as will remain NULL, and a non-array class container will be built in gfc_build_class_symbol below. Very good catch! Indeed, this fixes the testcase variations. @@ -8807,6 +8785,27 @@ attr_decl1 (void) goto cleanup; } + if (sym->ts.type == BT_CLASS && sym->ts.u.derived->attr.is_class + && !as && !current_attr.pointer && !current_attr.allocatable + && !current_attr.external) + { + sym->attr.pointer = 0; + sym->attr.allocatable = 0; + sym->attr.dimension = 0; + sym->attr.codimension = 0; + gfc_free_array_spec (sym->as); sym->as should probably be reset to NULL here. Done. Maybe both calls to gfc_free_array_spec here and to gfc_copy_array_spec above can be avoided by doing a simple pointer copy? I tried that, but this produced a crash with a double-free. The attached revised version uses the above proven changes, and extends the new testcase class_74.f90 by variations of the failures remaining with version 1 so that different codepaths are tested. Regtested again on x86_64-pc-linux-gnu. Any further comments? Thanks for your very helpful review! Harald From 70cba7da18023282546b9a5d80e976fc3744d732 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 5 Oct 2022 22:25:14 +0200 Subject: [PATCH] Fortran: reject procedures and procedure pointers as IO element [PR107074] gcc/fortran/ChangeLog: PR fortran/107074 * resolve.cc (resolve_transfer): A procedure, type-bound procedure or a procedure pointer cannot be an element of an IO list. * simplify.cc (gfc_simplify_merge): Do not try to reset array lower bound for scalars. gcc/testsuite/ChangeLog: PR fortran/107074 * gfortran.dg/pr107074.f90: New test. * gfortran.dg/pr107074b.f90: New test. --- gcc/fortran/resolve.cc | 31 + gcc/fortran/simplify.cc | 3 ++- gcc/testsuite/gfortran.dg/pr107074.f90 | 11 + gcc/testsuite/gfortran.dg/pr107074b.f90 | 18 ++ 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/pr107074.f90 create mode 100644 gcc/testsuite/gfortran.dg/pr107074b.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index d133bc2d034..d9d101775f6 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -10137,6 +10137,37 @@ resolve_transfer (gfc_code *code) "an assumed-size array", &code->loc); return; } + + /* Check for procedures and procedure pointers. Fortran 2018 has: + + C1233 (R1217) An expression that is an output-item shall not have a + value that is a procedure pointer. + + There does not appear any reason to allow procedure pointers for + input, so we disallow them generally, and we reject procedures. */ + + if (exp->expr_type == EXPR_VARIABLE) +{ + /* Check for type-bound procedures. */ + for (ref = exp->ref; ref; ref = ref->next) + if (ref->type == REF_COMPONENT + && ref->u.c.component->attr.flavor == FL_PROCEDURE) + break; + + /* Procedure or procedure pointer? */ + if (exp->ts.type == BT_PROCEDURE + || (ref && ref->u.c.component->attr.flavor == FL_PROCEDURE)) + { + if (exp->symtree->n.sym->attr.proc_pointer + || (ref && ref->u.c.component->attr.proc_pointer)) + gfc_error ("Data transfer element at %L cannot be a procedure " + "pointer", &code->loc); + else + gfc_error ("Data transfer element at %L cannot be a procedure", + &code->loc); + return; + } +} } diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index 6ac92cf9db8..f0482d349af 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -4915,7 +4915,8 @@ gfc_simplify_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask) { result = gfc_copy_expr (mask->value.logical ? tsource : fsource); /* Parenthesis is needed to get lower bounds of 1. */ - result = gfc_get_parentheses (result); + if (result->rank) + result = gfc_get_parentheses (result); gfc_simplify
Re: [PATCH, v2] Fortran: fix CLASS attribute handling [PR106856]
Sorry, attached the wrong patch. Here's the correct one. Harald Am 04.03.23 um 17:02 schrieb Harald Anlauf via Gcc-patches: The attached revised version uses the above proven changes, and extends the new testcase class_74.f90 by variations of the failures remaining with version 1 so that different codepaths are tested. Regtested again on x86_64-pc-linux-gnu. Any further comments? Thanks for your very helpful review! Harald From 0b7e9ea9c83ca6d6c0aae638be09fbcb8e42c682 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 2 Mar 2023 22:37:14 +0100 Subject: [PATCH] Fortran: fix CLASS attribute handling [PR106856] gcc/fortran/ChangeLog: PR fortran/106856 * class.cc (gfc_build_class_symbol): Handle update of attributes of existing class container. (gfc_find_derived_vtab): Fix several memory leaks. (find_intrinsic_vtab): Ditto. * decl.cc (attr_decl1): Manage update of symbol attributes from CLASS attributes. * primary.cc (gfc_variable_attr): OPTIONAL shall not be taken or updated from the class container. gcc/testsuite/ChangeLog: PR fortran/106856 * gfortran.dg/interface_41.f90: Remove dg-pattern from valid testcase. * gfortran.dg/class_74.f90: New test. * gfortran.dg/class_75.f90: New test. Co-authored-by: Tobias Burnus --- gcc/fortran/class.cc | 25 +++- gcc/fortran/decl.cc| 60 +- gcc/fortran/primary.cc | 1 - gcc/testsuite/gfortran.dg/class_74.f90 | 130 + gcc/testsuite/gfortran.dg/class_75.f90 | 24 gcc/testsuite/gfortran.dg/interface_41.f90 | 2 +- 6 files changed, 207 insertions(+), 35 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/class_74.f90 create mode 100644 gcc/testsuite/gfortran.dg/class_75.f90 diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc index ae653e74437..52235ab83e3 100644 --- a/gcc/fortran/class.cc +++ b/gcc/fortran/class.cc @@ -638,6 +638,7 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, { char tname[GFC_MAX_SYMBOL_LEN+1]; char *name; + gfc_typespec *orig_ts = ts; gfc_symbol *fclass; gfc_symbol *vtab; gfc_component *c; @@ -646,9 +647,21 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, gcc_assert (as); - if (attr->class_ok) -/* Class container has already been built. */ + /* Class container has already been built with same name. */ + if (attr->class_ok + && ts->u.derived->components->attr.dimension >= attr->dimension + && ts->u.derived->components->attr.codimension >= attr->codimension + && ts->u.derived->components->attr.class_pointer >= attr->pointer + && ts->u.derived->components->attr.allocatable >= attr->allocatable) return true; + if (attr->class_ok) +{ + attr->dimension |= ts->u.derived->components->attr.dimension; + attr->codimension |= ts->u.derived->components->attr.codimension; + attr->pointer |= ts->u.derived->components->attr.class_pointer; + attr->allocatable |= ts->u.derived->components->attr.allocatable; + ts = &ts->u.derived->components->ts; +} attr->class_ok = attr->dummy || attr->pointer || attr->allocatable || attr->select_type_temporary || attr->associate_var; @@ -790,7 +803,7 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, } fclass->attr.is_class = 1; - ts->u.derived = fclass; + orig_ts->u.derived = fclass; attr->allocatable = attr->pointer = attr->dimension = attr->codimension = 0; (*as) = NULL; free (name); @@ -2344,6 +2357,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) vtab->attr.vtab = 1; vtab->attr.access = ACCESS_PUBLIC; gfc_set_sym_referenced (vtab); + free (name); name = xasprintf ("__vtype_%s", tname); gfc_find_symbol (name, ns, 0, &vtype); @@ -2447,6 +2461,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) else { /* Construct default initialization variable. */ + free (name); name = xasprintf ("__def_init_%s", tname); gfc_get_symbol (name, ns, &def_init); def_init->attr.target = 1; @@ -2480,6 +2495,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) ns->contained = sub_ns; sub_ns->resolved = 1; /* Set up procedure symbol. */ + free (name); name = xasprintf ("__copy_%s", tname); gfc_get_symbol (name, sub_ns, ©); sub_ns->proc_name = copy; @@ -2558,6 +2574,7 @@ gfc_find_derived_vtab (gfc_symbol *derived) ns->contained = sub_ns; sub_ns->resolved = 1; /* Set up procedure symbol. */ + free (name); name = xasprintf ("__deallocate_%s", tname); gfc_get_symbol (name, sub_ns, &dealloc); sub_ns->proc_name = dealloc;
Re: [PATCH, v2] Fortran: fix CLASS attribute handling [PR106856]
Hi Mikael, Am 04.03.23 um 18:09 schrieb Mikael Morin: There was a comment about the old_symbol thing at the end of my previous message: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613354.html I think Tobias might be the better person to answer this. But when playing with variations of that else-branch, I always hit an issue with class_74.f90, where the class variables are not dummy arguments but local variables. E.g. take the following reduced testcase: subroutine foo class(*) :: y dimension :: y(:,:) pointer :: y end subroutine foo So when we see the dimension but haven't seen the pointer (or allocatable) declaration, we appear to generate an error with bad consequences (ICE). If this is a resolution issue, maybe it can be fixed differently, but likely needs digging deeper. With the patch as-is at least I do not see a memory leak in that context. Cheers, Harald
[PATCH, v3] Fortran: fix CLASS attribute handling [PR106856]
Hi Mikael, Am 04.03.23 um 23:29 schrieb Mikael Morin: Le 04/03/2023 à 22:20, Harald Anlauf a écrit : Hi Mikael, Am 04.03.23 um 18:09 schrieb Mikael Morin: There was a comment about the old_symbol thing at the end of my previous message: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613354.html I think Tobias might be the better person to answer this. But when playing with variations of that else-branch, I always hit an issue with class_74.f90, where the class variables are not dummy arguments but local variables. E.g. take the following reduced testcase: subroutine foo class(*) :: y dimension :: y(:,:) pointer :: y end subroutine foo So when we see the dimension but haven't seen the pointer (or allocatable) declaration, we appear to generate an error with bad consequences (ICE). If this is a resolution issue, maybe it can be fixed differently, but likely needs digging deeper. With the patch as-is at least I do not see a memory leak in that context. One of my suggestions was to fix it as attached. It is probably more clear with an actual patch to look at. It seems to work on your example and class_74 as well. This fix is great. I've included it in the revised patch. It seems to also fix some valgrind errors on this example: subroutine foo pointer :: y dimension :: y(:,:) class(*) :: y end subroutine foo I'm fine with that fix if it works for you. I've added this variant to class_74.f90, so it won't break without noticing. I suggest waiting for next stage 1, but it's your call, you have the green light from Steve anyway. I've chosen to push patch v3 (attached) after a further round of regtesting as r13-6497-g6aa1f40a326374 . Thanks for your work. Many thanks for your very helpful review! Harald From 6aa1f40a3263741d964ef4716e85a0df5cec83b6 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 2 Mar 2023 22:37:14 +0100 Subject: [PATCH] Fortran: fix CLASS attribute handling [PR106856] gcc/fortran/ChangeLog: PR fortran/106856 * class.cc (gfc_build_class_symbol): Handle update of attributes of existing class container. (gfc_find_derived_vtab): Fix several memory leaks. (find_intrinsic_vtab): Ditto. * decl.cc (attr_decl1): Manage update of symbol attributes from CLASS attributes. * primary.cc (gfc_variable_attr): OPTIONAL shall not be taken or updated from the class container. * symbol.cc (free_old_symbol): Adjust management of symbol versions to not prematurely free array specs while working on the declation of CLASS variables. gcc/testsuite/ChangeLog: PR fortran/106856 * gfortran.dg/interface_41.f90: Remove dg-pattern from valid testcase. * gfortran.dg/class_74.f90: New test. * gfortran.dg/class_75.f90: New test. Co-authored-by: Tobias Burnus --- gcc/fortran/class.cc | 25 +++- gcc/fortran/decl.cc| 56 gcc/fortran/primary.cc | 1 - gcc/fortran/symbol.cc | 6 +- gcc/testsuite/gfortran.dg/class_74.f90 | 151 + gcc/testsuite/gfortran.dg/class_75.f90 | 24 gcc/testsuite/gfortran.dg/interface_41.f90 | 2 +- 7 files changed, 229 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/class_74.f90 create mode 100644 gcc/testsuite/gfortran.dg/class_75.f90 diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc index ae653e74437..52235ab83e3 100644 --- a/gcc/fortran/class.cc +++ b/gcc/fortran/class.cc @@ -638,6 +638,7 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, { char tname[GFC_MAX_SYMBOL_LEN+1]; char *name; + gfc_typespec *orig_ts = ts; gfc_symbol *fclass; gfc_symbol *vtab; gfc_component *c; @@ -646,9 +647,21 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, gcc_assert (as); - if (attr->class_ok) -/* Class container has already been built. */ + /* Class container has already been built with same name. */ + if (attr->class_ok + && ts->u.derived->components->attr.dimension >= attr->dimension + && ts->u.derived->components->attr.codimension >= attr->codimension + && ts->u.derived->components->attr.class_pointer >= attr->pointer + && ts->u.derived->components->attr.allocatable >= attr->allocatable) return true; + if (attr->class_ok) +{ + attr->dimension |= ts->u.derived->components->attr.dimension; + attr->codimension |= ts->u.derived->components->attr.codimension; + attr->pointer |= ts->u.derived->components->attr.class_pointer; + attr->allocatable |= ts->u.derived->components->attr.allocatable; + ts = &ts->u.derived->components->ts; +} attr->class_ok = attr->dummy || attr->pointer || attr->allocatable || attr->select_type_temporary || attr->associate_var; @@ -790,7 +803,7 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr, } fclass->attr.is_class = 1; - ts->u.derived = fclass; + orig
[PATCH] Fortran: fix ICE with bind(c) in block data [PR104332]
Dear all, the attached almost obvious patch fixes a NULL pointer dereference in a check of a symbol with the bind(c) attribute. Regtested on x86_64-pc-linux-gnu. OK for mainline? This PR is marked as 10/11/12/13 regression, thus it should qualify for a backport. It's simple enough anyway. Thanks, Harald From ef96d7d360c088d68e3b405401bdb8b589d562f2 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 9 Mar 2023 18:59:08 +0100 Subject: [PATCH] Fortran: fix ICE with bind(c) in block data [PR104332] gcc/fortran/ChangeLog: PR fortran/104332 * resolve.cc (resolve_symbol): Avoid NULL pointer dereference while checking a symbol with the BIND(C) attribute. gcc/testsuite/ChangeLog: PR fortran/104332 * gfortran.dg/bind_c_usage_34.f90: New test. --- gcc/fortran/resolve.cc| 4 ++-- gcc/testsuite/gfortran.dg/bind_c_usage_34.f90 | 21 +++ 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/bind_c_usage_34.f90 diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 2780c82c798..46585879ddc 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -15933,8 +15933,8 @@ resolve_symbol (gfc_symbol *sym) /* First, make sure the variable is declared at the module-level scope (J3/04-007, Section 15.3). */ - if (sym->ns->proc_name->attr.flavor != FL_MODULE && - sym->attr.in_common == 0) + if (!(sym->ns->proc_name && sym->ns->proc_name->attr.flavor == FL_MODULE) + && !sym->attr.in_common) { gfc_error ("Variable %qs at %L cannot be BIND(C) because it " "is neither a COMMON block nor declared at the " diff --git a/gcc/testsuite/gfortran.dg/bind_c_usage_34.f90 b/gcc/testsuite/gfortran.dg/bind_c_usage_34.f90 new file mode 100644 index 000..40c8e9363cf --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bind_c_usage_34.f90 @@ -0,0 +1,21 @@ +! { dg-do compile } +! PR fortran/104332 - ICE with bind(c) in block data +! Contributed by G. Steinmetz + +block data + bind(c) :: a ! { dg-error "cannot be BIND\\(C\\)" } +end + +block data aa + real, bind(c) :: a ! { dg-error "cannot be BIND\\(C\\)" } +end + +block data bb + real:: a ! { dg-error "cannot be BIND\\(C\\)" } + bind(c) :: a +end + +block data cc + common /a/ x + bind(c) :: /a/ +end -- 2.35.3
[PATCH, pushed] Fortran: fix bounds check for copying of class expressions [PR106945]
Dear all, I've committed the attached patch to mainline as obvious after regtesting on x86_64-pc-linux-gnu. https://gcc.gnu.org/g:2cf5f485e0351bb1faf46196a99e524688f3966e commit r13-6605-g2cf5f485e0351bb1faf46196a99e524688f3966e Author: Harald Anlauf Date: Sat Mar 11 15:37:37 2023 +0100 Fortran: fix bounds check for copying of class expressions [PR106945] In the bounds check for copying of class expressions, the number of elements determined from a descriptor, returned as type gfc_array_index_type (i.e. a signed type), should be converted to the type of the passed element count, which is of type size_type_node (i.e. unsigned), for use in comparisons. gcc/fortran/ChangeLog: PR fortran/106945 * trans-expr.cc (gfc_copy_class_to_class): Convert element counts in bounds check to common type for comparison. gcc/testsuite/ChangeLog: PR fortran/106945 * gfortran.dg/pr106945.f90: New test. The PR is marked as a 10/11/12/13 regression, and given its simplicity, it should qualify for backports. Thanks, Harald From 2cf5f485e0351bb1faf46196a99e524688f3966e Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 11 Mar 2023 15:37:37 +0100 Subject: [PATCH] Fortran: fix bounds check for copying of class expressions [PR106945] In the bounds check for copying of class expressions, the number of elements determined from a descriptor, returned as type gfc_array_index_type (i.e. a signed type), should be converted to the type of the passed element count, which is of type size_type_node (i.e. unsigned), for use in comparisons. gcc/fortran/ChangeLog: PR fortran/106945 * trans-expr.cc (gfc_copy_class_to_class): Convert element counts in bounds check to common type for comparison. gcc/testsuite/ChangeLog: PR fortran/106945 * gfortran.dg/pr106945.f90: New test. --- gcc/fortran/trans-expr.cc | 1 + gcc/testsuite/gfortran.dg/pr106945.f90 | 11 +++ 2 files changed, 12 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr106945.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 045c8b00b90..dcd39f46776 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1531,6 +1531,7 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited) name = (const char *)(DECL_NAME (to)->identifier.id.str); from_len = gfc_conv_descriptor_size (from_data, 1); + from_len = fold_convert (TREE_TYPE (orig_nelems), from_len); tmp = fold_build2_loc (input_location, NE_EXPR, logical_type_node, from_len, orig_nelems); msg = xasprintf ("Array bound mismatch for dimension %d " diff --git a/gcc/testsuite/gfortran.dg/pr106945.f90 b/gcc/testsuite/gfortran.dg/pr106945.f90 new file mode 100644 index 000..e760ca7d27f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr106945.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! { dg-options "-fcoarray=single -fcheck=bounds -ftrapv" } +! PR fortran/106945 +! Contributed by G. Steinmetz + +module m + implicit none + type t + class(*), allocatable :: a[:] + end type +end -- 2.35.3
[PATCH] Fortran: rank checking with explicit-/assumed-size arrays and CLASS [PR58331]
Dear all, the attached patch, which is based on a draft by Tobias, fixes an old rejects-valid issue with rank checking for CLASS arrays by using the proper array spec of CLASS variables. The testcase covers only non-coarray cases, as playing with coarray variants hit pre-exisiting issues in gfortran that are very likely unrelated to the interface checks. I consider this rather as post 13-release stuff. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 4453686ae4e70c14a0898c6687db912fa84ece9f Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 14 Mar 2023 20:23:06 +0100 Subject: [PATCH] Fortran: rank checking with explicit-/assumed-size arrays and CLASS [PR58331] gcc/fortran/ChangeLog: PR fortran/58331 * interface.cc (compare_parameter): Adjust check of array dummy arguments to handle the case of CLASS variables. gcc/testsuite/ChangeLog: PR fortran/58331 * gfortran.dg/class_dummy_10.f90: New test. Co-authored-by: Tobias Burnus --- gcc/fortran/interface.cc | 27 +++--- gcc/testsuite/gfortran.dg/class_dummy_10.f90 | 56 2 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/class_dummy_10.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index dafe41753b7..1d0f8bb5915 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -2349,6 +2349,7 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, char err[200]; gfc_component *ppc; bool codimension = false; + gfc_array_spec *formal_as; /* If the formal arg has type BT_VOID, it's to one of the iso_c_binding procs c_f_pointer or c_f_procpointer, and we need to accept most @@ -2540,6 +2541,9 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, return false; } + formal_as = formal->ts.type == BT_CLASS ? CLASS_DATA (formal)->as + : formal->as; + if (codimension && formal->attr.allocatable) { gfc_ref *last = NULL; @@ -2650,10 +2654,10 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1) return true; - rank_check = where != NULL && !is_elemental && formal->as - && (formal->as->type == AS_ASSUMED_SHAPE - || formal->as->type == AS_DEFERRED) - && actual->expr_type != EXPR_NULL; + rank_check = where != NULL && !is_elemental && formal_as +&& (formal_as->type == AS_ASSUMED_SHAPE + || formal_as->type == AS_DEFERRED) +&& actual->expr_type != EXPR_NULL; /* Skip rank checks for NO_ARG_CHECK. */ if (formal->attr.ext_attr & (1 << EXT_ATTR_NO_ARG_CHECK)) @@ -2662,14 +2666,20 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, /* Scalar & coindexed, see: F2008, Section 12.5.2.4. */ if (rank_check || ranks_must_agree || (formal->attr.pointer && actual->expr_type != EXPR_NULL) - || (actual->rank != 0 && !(is_elemental || formal->attr.dimension)) + || (actual->rank != 0 + && !(is_elemental || formal->attr.dimension + || (formal->ts.type == BT_CLASS + && CLASS_DATA (formal)->attr.dimension))) || (actual->rank == 0 && ((formal->ts.type == BT_CLASS && CLASS_DATA (formal)->as->type == AS_ASSUMED_SHAPE) || (formal->ts.type != BT_CLASS && formal->as->type == AS_ASSUMED_SHAPE)) && actual->expr_type != EXPR_NULL) - || (actual->rank == 0 && formal->attr.dimension + || (actual->rank == 0 + && (formal->attr.dimension + || (formal->ts.type == BT_CLASS + && CLASS_DATA (formal)->attr.dimension)) && gfc_is_coindexed (actual)) /* Assumed-rank actual argument; F2018 C838. */ || actual->rank == -1) @@ -2690,7 +2700,10 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, } return false; } - else if (actual->rank != 0 && (is_elemental || formal->attr.dimension)) + else if (actual->rank != 0 + && (is_elemental || formal->attr.dimension + || (formal->ts.type == BT_CLASS + && CLASS_DATA (formal)->attr.dimension))) return true; /* At this point, we are considering a scalar passed to an array. This diff --git a/gcc/testsuite/gfortran.dg/class_dummy_10.f90 b/gcc/testsuite/gfortran.dg/class_dummy_10.f90 new file mode 100644 index 000..cee5d4d82b2 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/class_dummy_10.f90 @@ -0,0 +1,56 @@ +! { dg-do compile } +! PR fortran/58331 - rank checking for CLASS dummy arguments + +module mymod + implicit none +contains + subroutine mysub(a, n) +integer, intent(in) :: n +class(*), intent(in) :: a(n) + +select type(a) +type is(integer) + print *,'a is integer' + print *, "n=", n, ' a=', a +class default + print *,'a is unsupported type' + stop 1 +end select + end + + ! Assumed rank + subroutine sub_ar (a) +class(*), intent(in) :: a(..) +print *, rank (a), size (a), ":", shape (a) + end + + ! As
Re: [PATCH] Fortran: rank checking with explicit-/assumed-size arrays and CLASS [PR58331]
Hi Tobias, Am 15.03.23 um 10:10 schrieb Tobias Burnus: Hi Harald, On 14.03.23 20:38, Harald Anlauf wrote: The testcase covers only non-coarray cases, as playing with coarray variants hit pre-exisiting issues in gfortran that are very likely unrelated to the interface checks. I concur (but would not rule out additional interface issues). More testing seems to mostly uncover issues later on in trans*.cc, e.g. when passing type to class. I'll open a PR on this as a followup. I consider this rather as post 13-release stuff. In any case, the coarray issue can be fixed separately. And I think post-GCC-13 makes sense. Good. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks – LGTM! + formal_as = formal->ts.type == BT_CLASS ? CLASS_DATA (formal)->as + : formal->as; + (Jakub remarks for such code that some editor (emacs?), he does not use, mis--auto-indent such a code - and proposes to add a parentheses around the right-hand side of the assignment.) Ah, adding parentheses helps! I've reformatted this block accordingly. Pushed as: https://gcc.gnu.org/g:901edd99b44976b3c2b13a7d525d9e315540186a * * * I also wonder whether we need some run-time testcase. The interface check now works and I also tend to write dg-do-compile testcases, but given what can go wrong with all the array descriptor, class etc handling, we may want to ensure it works at run time. – Thoughts? If you comment out the lines with dg-error, the code compiles and seems to run fine here. I've even found cases where passing array sections works correctly here and with current Intel it does not ;-) I'd prefer to postpone more elaborate run-time tests until we have more non-ICEing related code. Thanks, Harald (That's independent of the patch it and could be done as follow up, if it deemed reasonable. The included testcase is surely compile-only as it has dg-error checks.) 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
[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
[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: 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: 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] PR fortran/70070 - ICE on initializing character data beyond min/max bound
Dear all, the attached patch is pretty much self-explaining: check for bounds violation when initializing a substring in a data statement and treat the resulting error. If more detailed information should be emitted with the error message, I'm open for suggestions. Regtested on x86_64-pc-linux-gnu. OK for master? Thanks, Harald PR fortran/70070 - ICE on initializing character data beyond min/max bound Check for initialization of substrings beyond bounds in DATA statements. gcc/fortran/ChangeLog: PR fortran/70070 * data.c (create_character_initializer): Check substring indices against bounds. (gfc_assign_data_value): Catch error returned from create_character_initializer. gcc/testsuite/ChangeLog: PR fortran/70070 * gfortran.dg/pr70070.f90: New test. diff --git a/gcc/fortran/data.c b/gcc/fortran/data.c index 1313b335c86..d9f0b45da9b 100644 --- a/gcc/fortran/data.c +++ b/gcc/fortran/data.c @@ -183,6 +183,13 @@ create_character_initializer (gfc_expr *init, gfc_typespec *ts, } } + if (start < 0 || end > init->value.character.length) +{ + gfc_error ("Invalid substring in DATA statement at %L", + &ref->u.ss.start->where); + return NULL; +} + if (rvalue->ts.type == BT_HOLLERITH) { for (size_t i = 0; i < (size_t) len; i++) @@ -576,6 +583,8 @@ gfc_assign_data_value (gfc_expr *lvalue, gfc_expr *rvalue, mpz_t index, if (lvalue->ts.u.cl->length == NULL && !(ref && ref->u.ss.length != NULL)) return false; expr = create_character_initializer (init, last_ts, ref, rvalue); + if (!expr) + return false; } else { diff --git a/gcc/testsuite/gfortran.dg/pr70070.f90 b/gcc/testsuite/gfortran.dg/pr70070.f90 new file mode 100644 index 000..c79cd229552 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr70070.f90 @@ -0,0 +1,8 @@ +! { dg-do compile } +! PR fortran/70070 - ICE on initializing character data beyond min/max bound + +program p + character(1) :: a, b + data (a(i:i),i=0,0) /1*'#'/ ! { dg-error "Invalid substring" } + data (b(i:i),i=1,2) /2*'#'/ ! { dg-error "Invalid substring" } +end
Re: [PATCH] PR fortran/70070 - ICE on initializing character data beyond min/max bound
Hi Thomas, > Gesendet: Montag, 25. Januar 2021 um 19:58 Uhr > Von: "Thomas Koenig" > a.f90:3:10: > > 3 | print a(0:3) >| 1 > Error: Substring start index at (1) is less than one > a.f90:4:10: > > 4 | print a(1:4) >| 1 > Error: Substring end index at (1) exceeds the string length > > Could you maybe just re-use these? this is done in the attached patch. Committed and pushed to master. > OK with adjusted error message. Thanks for the patch! Thanks for the review! Harald pr70070.patch-v2 Description: Binary data
[PATCH] [8/9/10/11 Regression] [OOP] PR fortran/86470 - ICE with OpenMP
Dear all, the fix for this ICE is obvious: make gfc_call_malloc behave as documented. Apparently the special case in question was not exercised in the testsuite. Regtested on x86_64-pc-linux-gnu. OK for master / backports? Should the testcase be moved to the gomp/ subdirectory? Thanks, Harald PR fortran/86470 - ICE with OpenMP, class(*) allocatable gfc_call_malloc should malloc an area of size 1 if no size given. gcc/fortran/ChangeLog: PR fortran/86470 * trans.c (gfc_call_malloc): Allocate area of size 1 if passed size is NULL (as documented). gcc/testsuite/ChangeLog: PR fortran/86470 * gfortran.dg/pr86470.f90: New test. diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index a2376917635..ab53fc5f441 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -689,6 +689,9 @@ gfc_call_malloc (stmtblock_t * block, tree type, tree size) /* Call malloc. */ gfc_start_block (&block2); + if (size == NULL_TREE) +size = build_int_cst (size_type_node, 1); + size = fold_convert (size_type_node, size); size = fold_build2_loc (input_location, MAX_EXPR, size_type_node, size, build_int_cst (size_type_node, 1)); diff --git a/gcc/testsuite/gfortran.dg/pr86470.f90 b/gcc/testsuite/gfortran.dg/pr86470.f90 new file mode 100644 index 000..4021e5d655c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr86470.f90 @@ -0,0 +1,13 @@ +! { dg-do compile } +! { dg-options "-fopenmp" } +! PR fortran/86470 - ICE with OpenMP, class(*) + +program p + implicit none + class(*), allocatable :: val +!$OMP PARALLEL private(val) + allocate(integer::val) + val = 1 + deallocate(val) +!$OMP END PARALLEL +end
Re: [PATCH] [8/9/10/11 Regression] [OOP] PR fortran/86470 - ICE with OpenMP
Hi Thomas, > > Should the testcase be moved to the gomp/ subdirectory? > Yes. It's a compile-time test, and it will then only be run > if the the compiler can do OpenMP. > > You will not need the > > +! { dg-options "-fopenmp" } > > line, then. Adjusted and committed as r11-6950-g33a7a93218b1393d0135e3c4a9ad9ced87808f5e > Thanks for the patch! Thanks, Harald
[PATCH] PR fortran/99147 - Sanitizer detects heap-use-after-free in gfc_add_flavor
Dear all, the PR reports an issue detected with an ASAN instrumented compiler, which can also be verified with valgrind. It appears that the state of gfc_new_block could be such that it should not be dereferenced. Reversing the order of condition evaluation helped. I failed to find out why this should happen, but then other places in the code put dereferences of gfc_new_block behind other checks. Simple things like initializing gfc_new_block with NULL in decl.c did not help. Regtested on x86_64-pc-linux-gnu. No testcase added since the issue can be found only with an instrumented compiler or valgrind. I consider the patch to be obvious and trivial, but post it here in case somebody wants to dig deeper. OK for master? Thanks, Harald PR fortran/99147 - Sanitizer detects heap-use-after-free in gfc_add_flavor Reverse order of conditions to avoid invalid read. gcc/fortran/ChangeLog: * symbol.c (gfc_add_flavor): Reverse order of conditions. diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c index 3b988d1be22..e982374d9d1 100644 --- a/gcc/fortran/symbol.c +++ b/gcc/fortran/symbol.c @@ -1772,8 +1772,8 @@ gfc_add_flavor (symbol_attribute *attr, sym_flavor f, const char *name, /* Copying a procedure dummy argument for a module procedure in a submodule results in the flavor being copied and would result in an error without this. */ - if (gfc_new_block && gfc_new_block->abr_modproc_decl - && attr->flavor == f && f == FL_PROCEDURE) + if (attr->flavor == f && f == FL_PROCEDURE + && gfc_new_block && gfc_new_block->abr_modproc_decl) return true; if (attr->flavor != FL_UNKNOWN)
[PATCH] PR fortran/99169 - [9/10/11 Regression] Segfault when passing allocatable scalar into intent(out) dummy argument
Dear all, we should not clobber the pointer in case of an allocatable scalar being an intent(out) dummy argument to a procedure. Regtested on x86_64-pc-linux-gnu. OK for master? Since this is a regression, also for backports to 10/9? Thanks, Harald PR fortran/99169 - Do not clobber allocatable intent(out) dummy argument gcc/fortran/ChangeLog: * trans-expr.c (gfc_conv_procedure_call): Do not add clobber to allocatable intent(out) argument. gcc/testsuite/ChangeLog: * gfortran.dg/intent_optimize_3.f90: New test. diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index 103cb31c664..cab58cd1bba 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -6077,6 +6079,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && !fsym->attr.allocatable && !fsym->attr.pointer && !e->symtree->n.sym->attr.dimension && !e->symtree->n.sym->attr.pointer + && !e->symtree->n.sym->attr.allocatable /* See PR 41453. */ && !e->symtree->n.sym->attr.dummy /* FIXME - PR 87395 and PR 41453 */ diff --git a/gcc/testsuite/gfortran.dg/intent_optimize_3.f90 b/gcc/testsuite/gfortran.dg/intent_optimize_3.f90 new file mode 100644 index 000..6ecd722da76 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/intent_optimize_3.f90 @@ -0,0 +1,16 @@ +! { dg-do run } +! { dg-options "-O2" } +! PR99169 - Segfault passing allocatable scalar into intent(out) dummy argument + +program p + implicit none + integer, allocatable :: i + allocate (i) + call set (i) + if (i /= 5) stop 1 +contains + subroutine set (i) +integer, intent(out) :: i +i = 5 + end subroutine set +end program p