[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 Neil Carlson changed: What|Removed |Added CC||neil.n.carlson at gmail dot com --- Comment #12 from Neil Carlson --- The commit r265171 that fixed this issue also introduced a regression in 8.2 and 9, and certainly the 7 branch too if it was back-ported to it. See PR89174 for the example, which had worked correctly for 7, 8, and 9, but which now segfaults on at least 8 and 9. I suppose it's too late revert that patch ...
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #13 from Paul Thomas --- (In reply to Neil Carlson from comment #12) > The commit r265171 that fixed this issue also introduced a regression in 8.2 > and 9, and certainly the 7 branch too if it was back-ported to it. See > PR89174 for the example, which had worked correctly for 7, 8, and 9, but > which now segfaults on at least 8 and 9. I suppose it's too late revert that > patch ... Hi Neil, As I said in comment #8, I decided that backporting was not going to work because there were too many antecedent patches. I just checked through the ChangeLog for 8-branch and there is no sign of anybody else attempting the fix. I'll take a look at 89174. Thanks Paul
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 Thomas Koenig changed: What|Removed |Added Status|NEW |RESOLVED CC||tkoenig at gcc dot gnu.org Resolution|--- |FIXED --- Comment #14 from Thomas Koenig --- (In reply to Paul Thomas from comment #13) > As I said in comment #8, I decided that backporting was not going to work > because there were too many antecedent patches. So, let's close this one as FIXED. We cannot backport all fixes to all branches, or else all gfortran versions would be the same :-)
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #10 from Antony Lewis --- In the latest 9.0 trunk I still see what looks like a similar ICE error (though I have not tried to isolate it again). See https://travis-ci.org/cmbant/forutils/builds/483365115 when running test script in https://github.com/cmbant/forutils/tree/gcc_bug1 . (code is reported invalid in gcc6, but SEG faults in 7,8,9).
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #11 from Antony Lewis --- I posted remaining ICE in 9.0.0 20190119 as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89069
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 Dominique d'Humieres changed: What|Removed |Added Priority|P3 |P4 Status|UNCONFIRMED |NEW Last reconfirmed||2018-10-09 Ever confirmed|0 |1 --- Comment #1 from Dominique d'Humieres --- Confirmed from 4.8 up to trunk (9.0). An instrumented compiler gives ../../work/gcc/fortran/resolve.c:8883:21: runtime error: member access within null pointer of type 'struct gfc_component'
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566
--- Comment #2 from Tobias Burnus ---
Created attachment 44831
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44831&action=edit
Partial draft patch
If I apply the attached patch (no idea whether it really makes sense, CLASS is
quite confusing!), it converts to gimple but then fails because of the
following.
For some odd reasons the gimplfier does not like that f951 assigns a value to
the digit 0 (last but one line of the "finally") ...
[Otherwise, the code looks fine to me.]
struct __class__STAR_1_0p * point;
struct __class__STAR_1_0p class.0;
class.0._data = __tmp_class_object_array_pointer->_data->p._data;
class.0._vptr = __tmp_class_object_array_pointer->_data->p._vptr;
...
point = &class.0;
...
finally
{
__tmp_class_object_array_pointer->_data->p._data = class.0._data;
__tmp_class_object_array_pointer->_data->p._vptr = class.0._vptr;
0 = (unsigned long) class.0._len;
__tmp_class_object_array_pointer->_data->p._len = class.0._len;
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #3 from Tobias Burnus --- @Paul: Some guidance is welcome! (In reply to Tobias Burnus from comment #2) > For some odd reasons the gimplfier does not like that f951 assigns a value > to the digit 0 (last but one line of the "finally") ... That's in trans-expr.c's gfc_conv_class_to_class(): if (UNLIMITED_POLY (e)) tmp = gfc_class_len_get (tmp); else if (e->ts.type == BT_CHARACTER) tmp = slen; else tmp = build_zero_cst (size_type_node); gfc_add_modify (&parmse->pre, ctree, fold_convert (TREE_TYPE (ctree), tmp)); OK so far – now comes the finally part: if (!elemental && full_array && copyback) gfc_add_modify (&parmse->post, tmp, fold_convert (TREE_TYPE (tmp), ctree)); And here we have assigned (if the "else" branch for tmp branch was taken) a value to the build_zero_cst()! Actually, I wonder whether the _len = 0 is needed. The current code is: class.0._len = 0; class.0._data = __tmp_class_object_array_pointer->_data->p._data; class.0._vptr = __tmp_class_object_array_pointer->_data->p._vptr; class.0._len = __tmp_class_object_array_pointer->_data->p._len; i.e. we set _len twice. And also in finally, we have it also twice: 0 = (unsigned long) class.0._len; __tmp_class_object_array_pointer->_data->p._len = class.0._len; As band-aid the following works (on top of the patch from attachment 44831) --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -1133,3 +1133,3 @@ gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, gfc_typespec class_ts, references, where the dynamic type cannot change. */ - if (!elemental && full_array && copyback) + if (!elemental && full_array && copyback && !INTEGER_CST_P (tmp)) gfc_add_modify (&parmse->post, tmp,
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #4 from paul.richard.thomas at gmail dot com --- Hi Tobias, You are grappling with exactly the error that I am grappling with in backporting my deferred character patches to 8-branch. The problem is the following and it is specific to deferred character components: 1) The string length of these components is stored in a hidden component, which gfc_conv_expr and gfc_conv_expr_descriptor successfully return as the string length. 2) Unfortunately, we are forced in the field decl for these components to make them zero length (NULL fails for reasons that I simply do not understand). This means that the ts.u.backend_decl is also constant zero and so must not be assigned to. 3) What I have been doing is to avoid assigning to the backend_decl by applying a guard that it is not integer_zerop and instead, looking for TREE_CODE (se->string_length) == COMPONENT_REF and assigning to that. I have to run Jan to the eye hospital this morning for an injection (the final one). When I get home, I will take a look at your partial patch and try to identify the spot where this happens. It's good to have you back. Thomas and I were getting rather lonely! Regards Paul On Fri, 12 Oct 2018 at 22:07, burnus at gcc dot gnu.org wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 > > --- Comment #3 from Tobias Burnus --- > @Paul: Some guidance is welcome! > > (In reply to Tobias Burnus from comment #2) > > For some odd reasons the gimplfier does not like that f951 assigns a value > > to the digit 0 (last but one line of the "finally") ... > > That's in trans-expr.c's gfc_conv_class_to_class(): > > if (UNLIMITED_POLY (e)) > tmp = gfc_class_len_get (tmp); > else if (e->ts.type == BT_CHARACTER) > tmp = slen; > else > tmp = build_zero_cst (size_type_node); > gfc_add_modify (&parmse->pre, ctree, > fold_convert (TREE_TYPE (ctree), tmp)); > > OK so far – now comes the finally part: > > if (!elemental && full_array && copyback) > gfc_add_modify (&parmse->post, tmp, > fold_convert (TREE_TYPE (tmp), ctree)); > > And here we have assigned (if the "else" branch for tmp branch was taken) a > value to the build_zero_cst()! > > > Actually, I wonder whether the _len = 0 is needed. The current code is: > > class.0._len = 0; > class.0._data = > __tmp_class_object_array_pointer->_data->p._data; > class.0._vptr = > __tmp_class_object_array_pointer->_data->p._vptr; > class.0._len = > __tmp_class_object_array_pointer->_data->p._len; > > i.e. we set _len twice. And also in finally, we have it also twice: > > 0 = (unsigned long) class.0._len; > __tmp_class_object_array_pointer->_data->p._len = class.0._len; > > As band-aid the following works (on top of the patch from attachment 44831) > > --- a/gcc/fortran/trans-expr.c > +++ b/gcc/fortran/trans-expr.c > @@ -1133,3 +1133,3 @@ gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, > gfc_typespec class_ts, > references, where the dynamic type cannot change. */ > - if (!elemental && full_array && copyback) > + if (!elemental && full_array && copyback && !INTEGER_CST_P (tmp)) > gfc_add_modify (&parmse->post, tmp, > > -- > You are receiving this mail because: > You are on the CC list for the bug.
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #5 from paul.richard.thomas at gmail dot com --- Sorry, forget that last. I got out on the wrong side of the bed I think. I will take a proper look later. Cheers Paul On Sat, 13 Oct 2018 at 07:45, paul.richard.thomas at gmail dot com wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 > > --- Comment #4 from paul.richard.thomas at gmail dot com at gmail dot com> --- > Hi Tobias, > > You are grappling with exactly the error that I am grappling with in > backporting my deferred character patches to 8-branch. The problem is > the following and it is specific to deferred character components: > > 1) The string length of these components is stored in a hidden > component, which gfc_conv_expr and gfc_conv_expr_descriptor > successfully return as the string length. > 2) Unfortunately, we are forced in the field decl for these components > to make them zero length (NULL fails for reasons that I simply do not > understand). This means that the ts.u.backend_decl is also constant > zero and so must not be assigned to. > 3) What I have been doing is to avoid assigning to the backend_decl by > applying a guard that it is not integer_zerop and instead, looking for > TREE_CODE (se->string_length) == COMPONENT_REF and assigning to that. > > I have to run Jan to the eye hospital this morning for an injection > (the final one). When I get home, I will take a look at your partial > patch and try to identify the spot where this happens. > > It's good to have you back. Thomas and I were getting rather lonely! > > Regards > > Paul > > On Fri, 12 Oct 2018 at 22:07, burnus at gcc dot gnu.org > wrote: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 > > > > --- Comment #3 from Tobias Burnus --- > > @Paul: Some guidance is welcome! > > > > (In reply to Tobias Burnus from comment #2) > > > For some odd reasons the gimplfier does not like that f951 assigns a value > > > to the digit 0 (last but one line of the "finally") ... > > > > That's in trans-expr.c's gfc_conv_class_to_class(): > > > > if (UNLIMITED_POLY (e)) > > tmp = gfc_class_len_get (tmp); > > else if (e->ts.type == BT_CHARACTER) > > tmp = slen; > > else > > tmp = build_zero_cst (size_type_node); > > gfc_add_modify (&parmse->pre, ctree, > > fold_convert (TREE_TYPE (ctree), tmp)); > > > > OK so far – now comes the finally part: > > > > if (!elemental && full_array && copyback) > > gfc_add_modify (&parmse->post, tmp, > > fold_convert (TREE_TYPE (tmp), ctree)); > > > > And here we have assigned (if the "else" branch for tmp branch was taken) a > > value to the build_zero_cst()! > > > > > > Actually, I wonder whether the _len = 0 is needed. The current code is: > > > > class.0._len = 0; > > class.0._data = > > __tmp_class_object_array_pointer->_data->p._data; > > class.0._vptr = > > __tmp_class_object_array_pointer->_data->p._vptr; > > class.0._len = > > __tmp_class_object_array_pointer->_data->p._len; > > > > i.e. we set _len twice. And also in finally, we have it also twice: > > > > 0 = (unsigned long) class.0._len; > > __tmp_class_object_array_pointer->_data->p._len = class.0._len; > > > > As band-aid the following works (on top of the patch from attachment 44831) > > > > --- a/gcc/fortran/trans-expr.c > > +++ b/gcc/fortran/trans-expr.c > > @@ -1133,3 +1133,3 @@ gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, > > gfc_typespec class_ts, > > references, where the dynamic type cannot change. */ > > - if (!elemental && full_array && copyback) > > + if (!elemental && full_array && copyback && !INTEGER_CST_P (tmp)) > > gfc_add_modify (&parmse->post, tmp, > > > > -- > > You are receiving this mail because: > > You are on the CC list for the bug. > > -- > You are receiving this mail because: > You are on the CC list for the bug.
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #6 from Paul Thomas --- Created attachment 44835 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44835&action=edit Fix for the PR Hi Tobias, The problem that you found occurs in trans-expr.c (gfc_conv_class_to_class). Once found, the fix was trivial (See the attachment) and the testcase below compiles and executes correctly. The call to gfc_conv_class_to_class is made at trans-stmt.c:1822. The argument 'copy_back' is set true. However, the copyback is made to the select type temporary, rather than to 'Pt'. Therefore, the assignment works but pointing to a new target does not. Setting 'copy_back' to false regtests OK but I suspect that it should break the associate construct for some cases. That said, to my surprise, this causes an ICE: call AddArray contains subroutine AddArray() type Object_array_pointer class(*), pointer :: p(:) => null() end type Object_array_pointer type (Object_array_pointer) :: obj character(3), target :: tgt1(2) = ['one','two'] character(5), target :: tgt2(2) = ['three','four '] obj%p => tgt1 associate (point => obj%p) end associate end subroutine AddArray end However, your patch in resolve.c caused a good number of regressions and so I put that right too. Over to you! Paul call AddArray contains subroutine AddArray() type Object_array_pointer class(*), pointer :: p(:) => null() end type Object_array_pointer class(*), pointer :: Pt => null() character(3), target :: tgt1(2) = ['one','two'] allocate (Pt, source = Object_array_pointer ()) select type (Pt) type is (object_array_pointer) Pt%p => tgt1 end select select type (Pt) class is (object_array_pointer) select type (Point=> Pt%P) type is (character(*)) if (any (Point .ne. tgt1)) stop 1 Point = ['abc','efg'] end select end select select type (Pt) class is (object_array_pointer) select type (Point=> Pt%P) type is (character(*)) if (any (Point .ne. ['abc','efg'])) stop 2 end select end select end subroutine AddArray end
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #7 from Paul Thomas --- Author: pault Date: Mon Oct 15 16:31:15 2018 New Revision: 265171 URL: https://gcc.gnu.org/viewcvs?rev=265171&root=gcc&view=rev Log: 2018-10-15 Paul Thomas Tobias Burnus PR fortran/87566 * resolve.c (resolve_assoc_var): Add missing array spec for class associate names. (resolve_select_type): Handle case where last typed component of the selector has a different type to the expression. * trans-expr.c (gfc_find_and_cut_at_last_class_ref): Replace call to gfc_expr_to_initialize with call to gfc_copy_expr. (gfc_conv_class_to_class): Guard assignment to 'len' field against case where zero constant is supplied. 2018-10-15 Paul Thomas Tobias Burnus PR fortran/87566 * gfortran.dg/select_type_44.f90: New test. * gfortran.dg/associate_42.f90: New test. Added: trunk/gcc/testsuite/gfortran.dg/associate_42.f90 trunk/gcc/testsuite/gfortran.dg/select_type_44.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/resolve.c trunk/gcc/fortran/trans-expr.c trunk/gcc/testsuite/ChangeLog
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #8 from Paul Thomas --- Fixed on trunk. I am going back on my original intention to backport the recent patches to 8-branch. Or, rather, I will do them one at a time if at all. The trouble is that an omnibus patch doesn't work for all the new testcases because of some earlier patches applied to trunk. I will keep this open in the hope that a backport will work but do not hold your breath. Best regards Paul
[Bug fortran/87566] ICE with class(*) and select
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87566 --- Comment #9 from Paul Thomas --- If I don't take it, I will lose it! Cheers Paul
