* [PATCH] Fortran : Two further previously missed ICEs PR53298
@ 2020-09-14 7:22 Mark Eggleston
2020-09-16 7:02 ` Andre Vehreschild
2020-10-13 7:26 ` Mark Eggleston
0 siblings, 2 replies; 5+ messages in thread
From: Mark Eggleston @ 2020-09-14 7:22 UTC (permalink / raw)
To: gcc-patches, fortran
[-- Attachment #1: Type: text/plain, Size: 2227 bytes --]
Second attempt this time with patch attached.
For review.
Fixes the two ICEs reported in PR that remained after the previous fix.
There is a side affect that is manifested in the tree dumps. Instead of
__builtin_free (ptr2->dat.data);
we get
__builtin_free ((void *) ptr2->dat.data);
I do not know the cause of this but from what I can tell the newly
inserted cast is harmless. All the examples I've seen so have the cast
except where the parameter is declared as void *. In the tree dumps ptr2
is declared as struct testtype2 *, I do not know where the type is
declared so I don't know whether data is declared void * (I expect it is).
Is it worth the effort to determine how to remove the extra (void *)?
[PATCH] Fortran : Two further previously missed ICEs PR53298
There were 3 ICEs with different call stacks in the comments of this
PR. A previous commit fixed only one of those ICEs.
The ICEs fixed here are in trans-array.c and trans-expr.c.
The first ICE occurred when the array reference is not AR_ELEMENT
gfc_conv_scalarized_array_ref is called with se and ar, if se->ss is
NULL the ICE occurs. If se->ss is NULL there is nothing to do before
the return.
The second ICE occurs in code that did not match its comments. Fixing
the code to match the comments fixes the ICE. A side affect is that
the in the tree dumps for finalize_35.f90 and finalize_36.f90 contain
"__builtin_free ((void *) ptr2->dat.data);", the "(void *)" was
previously omitted. The cast is harmless.
2020-09-11 Mark Eggleston <markeggleston@gcc.gnu.org>
gcc/fortran/
PR fortran/53298
* trans-array.c (gfc_conv_array_ref): In the body of the if
statement only execute the code before the reurn is se->ss is
set.
* trans-expr.c (gfc_conv_component_ref): Change the if
expression to match the comments.
2020-09-04 Mark Eggleston <markeggleston@gcc.gnu.org>
gcc/testsuite/
PR fortran/53298
* gfortran.dg/finalize_35.f90: Handle extra (void *).
* gfortran.dg/finalize_36.f90: Handle extra (void *).
* gfortran.dg/pr53298_2.f90: New test.
* gfortran.dg/pr53298_3.f90: New test.
--
https://www.codethink.co.uk/privacy.html
[-- Attachment #2: 0001-Fortran-Two-further-previously-missed-ICEs-PR53298.patch --]
[-- Type: text/x-patch, Size: 5706 bytes --]
From b88c7fdae5e76e5d47086f124357cee7159bc6fd Mon Sep 17 00:00:00 2001
From: Mark Eggleston <markeggleston@gcc.gnu.org>
Date: Tue, 8 Sep 2020 14:28:18 +0100
Subject: [PATCH] Fortran : Two further previously missed ICEs PR53298
There were 3 ICEs with different call stacks in the comments of this
PR. A previous commit fixed only one of those ICEs.
The ICEs fixed here are in trans-array.c and trans-expr.c.
The first ICE occurred when the array reference is not AR_ELEMENT
gfc_conv_scalarized_array_ref is called with se and ar, if se->ss is
NULL the ICE occurs. If se->ss is NULL there is nothing to do before
the return.
The second ICE occurs in code that did not match its comments. Fixing
the code to match the comments fixes the ICE. A side affect is that
the in the tree dumps for finalize_35.f90 and finalize_36.f90 contain
"__builtin_free ((void *) ptr2->dat.data);", the "(void *)" was
previously omitted. The cast is harmless.
2020-09-11 Mark Eggleston <markeggleston@gcc.gnu.org>
gcc/fortran/
PR fortran/53298
* trans-array.c (gfc_conv_array_ref): In the body of the if
statement only execute the code before the reurn is se->ss is
set.
* trans-expr.c (gfc_conv_component_ref): Change the if
expression to match the comments.
2020-09-04 Mark Eggleston <markeggleston@gcc.gnu.org>
gcc/testsuite/
PR fortran/53298
* gfortran.dg/finalize_35.f90: Handle extra (void *).
* gfortran.dg/finalize_36.f90: Handle extra (void *).
* gfortran.dg/pr53298_2.f90: New test.
* gfortran.dg/pr53298_3.f90: New test.
---
gcc/fortran/trans-array.c | 7 +++++--
gcc/fortran/trans-expr.c | 4 ++--
gcc/testsuite/gfortran.dg/finalize_35.f90 | 2 +-
gcc/testsuite/gfortran.dg/finalize_36.f90 | 2 +-
gcc/testsuite/gfortran.dg/pr53298_2.f90 | 16 ++++++++++++++++
gcc/testsuite/gfortran.dg/pr53298_3.f90 | 31 +++++++++++++++++++++++++++++++
6 files changed, 56 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr53298_2.f90
create mode 100644 gcc/testsuite/gfortran.dg/pr53298_3.f90
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 6566c47d4ae..06268739515 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -3638,8 +3638,11 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr,
/* Handle scalarized references separately. */
if (ar->type != AR_ELEMENT)
{
- gfc_conv_scalarized_array_ref (se, ar);
- gfc_advance_se_ss_chain (se);
+ if (se->ss)
+ {
+ gfc_conv_scalarized_array_ref (se, ar);
+ gfc_advance_se_ss_chain (se);
+ }
return;
}
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 36ff9b5cbc6..193553ace0b 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -2474,8 +2474,8 @@ gfc_conv_component_ref (gfc_se * se, gfc_ref * ref)
RECORD_TYPE within a UNION_TYPE) always use the given FIELD_DECL. */
if (context != TREE_TYPE (decl)
- && !( TREE_CODE (TREE_TYPE (field)) == UNION_TYPE /* Field is union */
- || TREE_CODE (context) == UNION_TYPE)) /* Field is map */
+ && ( TREE_CODE (context) == UNION_TYPE /* Field is union */
+ || TREE_CODE (context) == MAP_TYPE)) /* Field is map */
{
tree f2 = c->norestrict_decl;
if (!f2 || DECL_FIELD_CONTEXT (f2) != TREE_TYPE (decl))
diff --git a/gcc/testsuite/gfortran.dg/finalize_35.f90 b/gcc/testsuite/gfortran.dg/finalize_35.f90
index 66435c43ecc..6d7cf581b6c 100644
--- a/gcc/testsuite/gfortran.dg/finalize_35.f90
+++ b/gcc/testsuite/gfortran.dg/finalize_35.f90
@@ -45,4 +45,4 @@ program run
end do
end program run
-! { dg-final { scan-tree-dump-times "__builtin_free\\ \\(ptr2" 2 "original" } }
+! { dg-final { scan-tree-dump-times "__builtin_free...void....ptr2" 2 "original" } }
diff --git a/gcc/testsuite/gfortran.dg/finalize_36.f90 b/gcc/testsuite/gfortran.dg/finalize_36.f90
index 432f5472aeb..fdb80441e63 100644
--- a/gcc/testsuite/gfortran.dg/finalize_36.f90
+++ b/gcc/testsuite/gfortran.dg/finalize_36.f90
@@ -36,4 +36,4 @@
call Leaker()
end program
-! { dg-final { scan-tree-dump-times "__builtin_free\\ \\(ptr2" 4 "original" } }
+! { dg-final { scan-tree-dump-times "__builtin_free...void....ptr2" 4 "original" } }
diff --git a/gcc/testsuite/gfortran.dg/pr53298_2.f90 b/gcc/testsuite/gfortran.dg/pr53298_2.f90
new file mode 100644
index 00000000000..741fc1a8b43
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr53298_2.f90
@@ -0,0 +1,16 @@
+! { dg-do run }
+
+program pr53298
+ character(len=5) :: str(3)
+ str = [ "abcde", "12345", "ABCDE" ]
+ call f(str(:))
+contains
+ subroutine f(x)
+ character(*) :: x(:)
+ character(12) :: buffer
+ if (size(x(:)(1:)).ne.3) stop 1
+ write(buffer,'(3A4)') x(:)(2:)
+ if (buffer.ne."bcde2345BCDE") stop 2
+ end subroutine f
+end
+
diff --git a/gcc/testsuite/gfortran.dg/pr53298_3.f90 b/gcc/testsuite/gfortran.dg/pr53298_3.f90
new file mode 100644
index 00000000000..b3a191b7ab4
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr53298_3.f90
@@ -0,0 +1,31 @@
+! { dg-do run }
+
+module m53298
+ integer, parameter :: lench=24
+
+ type :: t1
+ character(len=lench) :: c="foo"
+ end type t1
+ type(t1) :: a(1)
+
+ type :: t2
+ character(len=lench) :: d = "bar"
+ end type t2
+ type(t2), parameter :: b = t2("bar")
+
+contains
+ subroutine proc()
+ integer :: l
+ l = len_trim(a(1)%c)
+ if (a(1)%c(1:l).ne."foo") stop 1
+ if (b%d(1:l).ne."bar") stop 2
+ a(1) = t1("bar")
+ if (a(1)%c(1:l).ne.b%d(1:l)) stop 3
+ end subroutine proc
+end module m53298
+
+program pr53298
+ use m53298
+
+ call proc
+end program pr53298
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fortran : Two further previously missed ICEs PR53298
2020-09-14 7:22 [PATCH] Fortran : Two further previously missed ICEs PR53298 Mark Eggleston
@ 2020-09-16 7:02 ` Andre Vehreschild
2020-09-29 13:53 ` Mark Eggleston
2020-10-13 7:26 ` Mark Eggleston
1 sibling, 1 reply; 5+ messages in thread
From: Andre Vehreschild @ 2020-09-16 7:02 UTC (permalink / raw)
To: Mark Eggleston; +Cc: gcc-patches, fortran
Hi Mark,
a few remarks:
[...]
> [PATCH] Fortran : Two further previously missed ICEs PR53298
>
> There were 3 ICEs with different call stacks in the comments of this
> PR. A previous commit fixed only one of those ICEs.
>
> The ICEs fixed here are in trans-array.c and trans-expr.c.
>
> The first ICE occurred when the array reference is not AR_ELEMENT
> gfc_conv_scalarized_array_ref is called with se and ar, if se->ss is
> NULL the ICE occurs. If se->ss is NULL there is nothing to do before
> the return.
>
> The second ICE occurs in code that did not match its comments. Fixing
> the code to match the comments fixes the ICE. A side affect is that
> the in the tree dumps for finalize_35.f90 and finalize_36.f90 contain
^^^
Spurious "the" found.
[...]
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 6566c47d4ae..06268739515 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -3638,8 +3638,11 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref *
ar, gfc_expr *expr, /* Handle scalarized references separately. */
if (ar->type != AR_ELEMENT)
{
- gfc_conv_scalarized_array_ref (se, ar);
- gfc_advance_se_ss_chain (se);
+ if (se->ss)
+ {
+ gfc_conv_scalarized_array_ref (se, ar);
+ gfc_advance_se_ss_chain (se);
+ }
Why is this only in element ref needed and not every else? When I tried
to fix ICEs this way, I was usually asked if was fixing symptom and not
the error.
return;
}
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 36ff9b5cbc6..193553ace0b 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -2474,8 +2474,8 @@ gfc_conv_component_ref (gfc_se * se, gfc_ref *
ref) RECORD_TYPE within a UNION_TYPE) always use the given FIELD_DECL.
*/
if (context != TREE_TYPE (decl)
- && !( TREE_CODE (TREE_TYPE (field)) == UNION_TYPE /* Field is
union */
- || TREE_CODE (context) == UNION_TYPE)) /* Field is
map */
+ && ( TREE_CODE (context) == UNION_TYPE /* Field is union */
+ || TREE_CODE (context) == MAP_TYPE)) /* Field is map */
{
tree f2 = c->norestrict_decl;
if (!f2 || DECL_FIELD_CONTEXT (f2) != TREE_TYPE (decl))
(Sorry for the line breaks).
I can't help it, but the old code looked so dubious that I wonder why
it worked in the first place. Have you tried with a mapped type?
Regards,
Andre
--
Andre Vehreschild * vehre@gmx.de
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fortran : Two further previously missed ICEs PR53298
2020-09-16 7:02 ` Andre Vehreschild
@ 2020-09-29 13:53 ` Mark Eggleston
0 siblings, 0 replies; 5+ messages in thread
From: Mark Eggleston @ 2020-09-29 13:53 UTC (permalink / raw)
To: Andre Vehreschild; +Cc: gcc-patches, fortran
On 16/09/2020 08:02, Andre Vehreschild wrote:
> Hi Mark,
>
> a few remarks:
>
> [...]
>
>> [PATCH] Fortran : Two further previously missed ICEs PR53298
>>
>> There were 3 ICEs with different call stacks in the comments of this
>> PR. A previous commit fixed only one of those ICEs.
>>
>> The ICEs fixed here are in trans-array.c and trans-expr.c.
>>
>> The first ICE occurred when the array reference is not AR_ELEMENT
>> gfc_conv_scalarized_array_ref is called with se and ar, if se->ss is
>> NULL the ICE occurs. If se->ss is NULL there is nothing to do before
>> the return.
>>
>> The second ICE occurs in code that did not match its comments. Fixing
>> the code to match the comments fixes the ICE. A side affect is that
>> the in the tree dumps for finalize_35.f90 and finalize_36.f90 contain
> ^^^
> Spurious "the" found.
wording has been updated.
>
> [...]
>
> diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
> index 6566c47d4ae..06268739515 100644
> --- a/gcc/fortran/trans-array.c
> +++ b/gcc/fortran/trans-array.c
> @@ -3638,8 +3638,11 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref *
> ar, gfc_expr *expr, /* Handle scalarized references separately. */
> if (ar->type != AR_ELEMENT)
> {
> - gfc_conv_scalarized_array_ref (se, ar);
> - gfc_advance_se_ss_chain (se);
> + if (se->ss)
> + {
> + gfc_conv_scalarized_array_ref (se, ar);
> + gfc_advance_se_ss_chain (se);
> + }
>
> Why is this only in element ref needed and not every else? When I tried
> to fix ICEs this way, I was usually asked if was fixing symptom and not
> the error.
This is for references that aren't elements. As I understand it se
corresponds to the upper bound, e.g. for a(1:) the upper bound may not
be known at this point so se is NULL resulting in a crash due to it
being de-referenced in gfc_conv_scalarized_array_ref.
>
> return;
> }
>
> diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
> index 36ff9b5cbc6..193553ace0b 100644
> --- a/gcc/fortran/trans-expr.c
> +++ b/gcc/fortran/trans-expr.c
> @@ -2474,8 +2474,8 @@ gfc_conv_component_ref (gfc_se * se, gfc_ref *
> ref) RECORD_TYPE within a UNION_TYPE) always use the given FIELD_DECL.
> */
> if (context != TREE_TYPE (decl)
> - && !( TREE_CODE (TREE_TYPE (field)) == UNION_TYPE /* Field is
> union */
> - || TREE_CODE (context) == UNION_TYPE)) /* Field is
> map */
> + && ( TREE_CODE (context) == UNION_TYPE /* Field is union */
> + || TREE_CODE (context) == MAP_TYPE)) /* Field is map */
> {
> tree f2 = c->norestrict_decl;
> if (!f2 || DECL_FIELD_CONTEXT (f2) != TREE_TYPE (decl))
>
> (Sorry for the line breaks).
>
> I can't help it, but the old code looked so dubious that I wonder why
> it worked in the first place. Have you tried with a mapped type?
structure /st/
union
map
character(5) a, b
end map
map
character(10) c
end map
end union
end structure
record /st/ r
r.c = "abcde12345"
write(*,*) r.a(2:), r.b(3:), r.c(6:)
end
outputs (as expected) with and without the patch
bcde34512345
I can add this as an extra test case if required.
>
> Regards,
> Andre
>
>
--
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fortran : Two further previously missed ICEs PR53298
2020-09-14 7:22 [PATCH] Fortran : Two further previously missed ICEs PR53298 Mark Eggleston
2020-09-16 7:02 ` Andre Vehreschild
@ 2020-10-13 7:26 ` Mark Eggleston
1 sibling, 0 replies; 5+ messages in thread
From: Mark Eggleston @ 2020-10-13 7:26 UTC (permalink / raw)
To: gcc-patches, fortran
**ping**
see https://gcc.gnu.org/pipermail/gcc-patches/2020-September/554034.html
and https://gcc.gnu.org/pipermail/gcc-patches/2020-September/555072.html
OK for master?
On 14/09/2020 08:22, Mark Eggleston wrote:
> Second attempt this time with patch attached.
>
> For review.
>
> Fixes the two ICEs reported in PR that remained after the previous fix.
>
> There is a side affect that is manifested in the tree dumps. Instead of
>
> __builtin_free (ptr2->dat.data);
>
> we get
>
> __builtin_free ((void *) ptr2->dat.data);
>
> I do not know the cause of this but from what I can tell the newly
> inserted cast is harmless. All the examples I've seen so have the
> cast except where the parameter is declared as void *. In the tree
> dumps ptr2 is declared as struct testtype2 *, I do not know where the
> type is declared so I don't know whether data is declared void * (I
> expect it is).
>
> Is it worth the effort to determine how to remove the extra (void *)?
>
> [PATCH] Fortran : Two further previously missed ICEs PR53298
>
> There were 3 ICEs with different call stacks in the comments of this
> PR. A previous commit fixed only one of those ICEs.
>
> The ICEs fixed here are in trans-array.c and trans-expr.c.
>
> The first ICE occurred when the array reference is not AR_ELEMENT
> gfc_conv_scalarized_array_ref is called with se and ar, if se->ss is
> NULL the ICE occurs. If se->ss is NULL there is nothing to do before
> the return.
>
> The second ICE occurs in code that did not match its comments. Fixing
> the code to match the comments fixes the ICE. A side affect is that
> the in the tree dumps for finalize_35.f90 and finalize_36.f90 contain
> "__builtin_free ((void *) ptr2->dat.data);", the "(void *)" was
> previously omitted. The cast is harmless.
>
> 2020-09-11 Mark Eggleston <markeggleston@gcc.gnu.org>
>
> gcc/fortran/
>
> PR fortran/53298
> * trans-array.c (gfc_conv_array_ref): In the body of the if
> statement only execute the code before the reurn is se->ss is
> set.
> * trans-expr.c (gfc_conv_component_ref): Change the if
> expression to match the comments.
>
> 2020-09-04 Mark Eggleston <markeggleston@gcc.gnu.org>
>
> gcc/testsuite/
>
> PR fortran/53298
> * gfortran.dg/finalize_35.f90: Handle extra (void *).
> * gfortran.dg/finalize_36.f90: Handle extra (void *).
> * gfortran.dg/pr53298_2.f90: New test.
> * gfortran.dg/pr53298_3.f90: New test.
>
--
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Fortran : Two further previously missed ICEs PR53298
@ 2020-09-11 7:52 Mark Eggleston
0 siblings, 0 replies; 5+ messages in thread
From: Mark Eggleston @ 2020-09-11 7:52 UTC (permalink / raw)
To: gcc-patches, fortran
For review.
Fixes the two ICEs reported in PR that remained after the previous fix.
There is a side affect that is manifested in the tree dumps. Instead of
__builtin_free (ptr2->dat.data);
we get
__builtin_free ((void *) ptr2->dat.data);
I do not know the cause of this but from what I can tell the newly
inserted cast is harmless. All the examples I've seen so have the cast
except where the parameter is declared as void *. In the tree dumps ptr2
is declared as struct testtype2 *, I do not know where the type is
declared so I don't know whether data is declared void * (I expect it is).
Is it worth the effort to determine how to remove the extra (void *)?
[PATCH] Fortran : Two further previously missed ICEs PR53298
There were 3 ICEs with different call stacks in the comments of this
PR. A previous commit fixed only one of those ICEs.
The ICEs fixed here are in trans-array.c and trans-expr.c.
The first ICE occurred when the array reference is not AR_ELEMENT
gfc_conv_scalarized_array_ref is called with se and ar, if se->ss is
NULL the ICE occurs. If se->ss is NULL there is nothing to do before
the return.
The second ICE occurs in code that did not match its comments. Fixing
the code to match the comments fixes the ICE. A side affect is that
the in the tree dumps for finalize_35.f90 and finalize_36.f90 contain
"__builtin_free ((void *) ptr2->dat.data);", the "(void *)" was
previously omitted. The cast is harmless.
2020-09-11 Mark Eggleston <markeggleston@gcc.gnu.org>
gcc/fortran/
PR fortran/53298
* trans-array.c (gfc_conv_array_ref): In the body of the if
statement only execute the code before the reurn is se->ss is
set.
* trans-expr.c (gfc_conv_component_ref): Change the if
expression to match the comments.
2020-09-04 Mark Eggleston <markeggleston@gcc.gnu.org>
gcc/testsuite/
PR fortran/53298
* gfortran.dg/finalize_35.f90: Handle extra (void *).
* gfortran.dg/finalize_36.f90: Handle extra (void *).
* gfortran.dg/pr53298_2.f90: New test.
* gfortran.dg/pr53298_3.f90: New test.
--
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-13 7:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 7:22 [PATCH] Fortran : Two further previously missed ICEs PR53298 Mark Eggleston
2020-09-16 7:02 ` Andre Vehreschild
2020-09-29 13:53 ` Mark Eggleston
2020-10-13 7:26 ` Mark Eggleston
-- strict thread matches above, loose matches on Subject: below --
2020-09-11 7:52 Mark Eggleston
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).