diff mbox series

[Fortran,PR77518,(coarray),v1] Fix ICE in sizeof(coarray)

Message ID 20240718151618.7a3e24ae@gmx.de
State New
Headers show
Series [Fortran,PR77518,(coarray),v1] Fix ICE in sizeof(coarray) | expand

Commit Message

Andre Vehreschild July 18, 2024, 1:16 p.m. UTC 
Hi all,

the attached patch fixes an ICE when the object supplied to sizeof() is
a coarray of class type. This is fixed by taking the class object from
the se's class_container.

Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?

Regards,
	Andre
--
Andre Vehreschild * Email: vehre ad gcc dot gnu dot org

Comments

Paul Richard Thomas July 18, 2024, 5:16 p.m. UTC | #1 
Hi Andre,

While I realise that this is not your doing, should we not
check DECL_LANG_SPECIFIC ()) before touching GFC_DECL_SAVED_DESCRIPTOR? Or
is access guaranteed by the REF_COMPONENT check?

A micro-nit line 12 s/User/Use/

Apart from this, it looks to be eminently obvious. OK for mainline.

Paul


On Thu, 18 Jul 2024 at 14:16, Andre Vehreschild <vehre@gmx.de> wrote:

> Hi all,
>
> the attached patch fixes an ICE when the object supplied to sizeof() is
> a coarray of class type. This is fixed by taking the class object from
> the se's class_container.
>
> Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
>
> Regards,
>         Andre
> --
> Andre Vehreschild * Email: vehre ad gcc dot gnu dot org
>
Andre Vehreschild July 19, 2024, 11:26 a.m. UTC | #2 
Hi Paul,

thanks for the review.

> While I realise that this is not your doing, should we not
> check DECL_LANG_SPECIFIC ()) before touching GFC_DECL_SAVED_DESCRIPTOR?

I like that idea. I have added it. But what should we do when
DECL_LANG_SPECIFIC is not set? I have chosen to add a gcc_unreachable(), but
that will trigger an ICE in the future should the prerequisites not be met.

> Or is access guaranteed by the REF_COMPONENT check?

Well, as we have seen, it is not. At least that is not guaranteed to my
knowledge.

When you don't like this solution solution, then I will dig deeper to figure
what is going on and how to resolve it.

> A micro-nit line 12 s/User/Use/

Ups, thanks, fixed.

Not merging yet, therefore updated patch attached.

- Andre

>
> Apart from this, it looks to be eminently obvious. OK for mainline.
>
> Paul
>
>
> On Thu, 18 Jul 2024 at 14:16, Andre Vehreschild <vehre@gmx.de> wrote:
>
> > Hi all,
> >
> > the attached patch fixes an ICE when the object supplied to sizeof() is
> > a coarray of class type. This is fixed by taking the class object from
> > the se's class_container.
> >
> > Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
> >
> > Regards,
> >         Andre
> > --
> > Andre Vehreschild * Email: vehre ad gcc dot gnu dot org
> >


--
Andre Vehreschild * Email: vehre ad gmx dot de
Andre Vehreschild Aug. 9, 2024, 2:30 p.m. UTC | #3 
Ping!

@Paul, you already had a look at this patch, but I made some changes. Or they
ok?

- Andre

On Fri, 19 Jul 2024 13:26:21 +0200
Andre Vehreschild <vehre@gmx.de> wrote:

> Hi Paul,
>
> thanks for the review.
>
> > While I realise that this is not your doing, should we not
> > check DECL_LANG_SPECIFIC ()) before touching GFC_DECL_SAVED_DESCRIPTOR?
>
> I like that idea. I have added it. But what should we do when
> DECL_LANG_SPECIFIC is not set? I have chosen to add a gcc_unreachable(), but
> that will trigger an ICE in the future should the prerequisites not be met.
>
> > Or is access guaranteed by the REF_COMPONENT check?
>
> Well, as we have seen, it is not. At least that is not guaranteed to my
> knowledge.
>
> When you don't like this solution solution, then I will dig deeper to figure
> what is going on and how to resolve it.
>
> > A micro-nit line 12 s/User/Use/
>
> Ups, thanks, fixed.
>
> Not merging yet, therefore updated patch attached.
>
> - Andre
>
> >
> > Apart from this, it looks to be eminently obvious. OK for mainline.
> >
> > Paul
> >
> >
> > On Thu, 18 Jul 2024 at 14:16, Andre Vehreschild <vehre@gmx.de> wrote:
> >
> > > Hi all,
> > >
> > > the attached patch fixes an ICE when the object supplied to sizeof() is
> > > a coarray of class type. This is fixed by taking the class object from
> > > the se's class_container.
> > >
> > > Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
> > >
> > > Regards,
> > >         Andre
> > > --
> > > Andre Vehreschild * Email: vehre ad gcc dot gnu dot org
> > >
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de


--
Andre Vehreschild * Email: vehre ad gmx dot de
Andre Vehreschild Aug. 20, 2024, 12:35 p.m. UTC | #4 
Hi all,

pinging this patch.

Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?

Regards,
	Andre

On Fri, 9 Aug 2024 16:30:52 +0200
Andre Vehreschild <vehre@gmx.de> wrote:

> Ping!
>
> @Paul, you already had a look at this patch, but I made some changes. Or they
> ok?
>
> - Andre
>
> On Fri, 19 Jul 2024 13:26:21 +0200
> Andre Vehreschild <vehre@gmx.de> wrote:
>
> > Hi Paul,
> >
> > thanks for the review.
> >
> > > While I realise that this is not your doing, should we not
> > > check DECL_LANG_SPECIFIC ()) before touching GFC_DECL_SAVED_DESCRIPTOR?
> >
> > I like that idea. I have added it. But what should we do when
> > DECL_LANG_SPECIFIC is not set? I have chosen to add a gcc_unreachable(), but
> > that will trigger an ICE in the future should the prerequisites not be met.
> >
> > > Or is access guaranteed by the REF_COMPONENT check?
> >
> > Well, as we have seen, it is not. At least that is not guaranteed to my
> > knowledge.
> >
> > When you don't like this solution solution, then I will dig deeper to figure
> > what is going on and how to resolve it.
> >
> > > A micro-nit line 12 s/User/Use/
> >
> > Ups, thanks, fixed.
> >
> > Not merging yet, therefore updated patch attached.
> >
> > - Andre
> >
> > >
> > > Apart from this, it looks to be eminently obvious. OK for mainline.
> > >
> > > Paul
> > >
> > >
> > > On Thu, 18 Jul 2024 at 14:16, Andre Vehreschild <vehre@gmx.de> wrote:
> > >
> > > > Hi all,
> > > >
> > > > the attached patch fixes an ICE when the object supplied to sizeof() is
> > > > a coarray of class type. This is fixed by taking the class object from
> > > > the se's class_container.
> > > >
> > > > Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
> > > >
> > > > Regards,
> > > >         Andre
> > > > --
> > > > Andre Vehreschild * Email: vehre ad gcc dot gnu dot org
> > > >
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de


--
Andre Vehreschild * Email: vehre ad gmx dot de
Jerry D Aug. 20, 2024, 4:16 p.m. UTC | #5 
On 8/20/24 5:35 AM, Andre Vehreschild wrote:
> Hi all,
> 
> pinging this patch.
> 
> Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
> 
> Regards,
> 	Andre
> 

Your approach looks reasonable so I think OK to push.

Thanks,

Jerry
Andre Vehreschild Aug. 21, 2024, 7:42 a.m. UTC | #6 
Hi Jerry,

thank you for the review. Committed as gcc-15-3062-g515730fd65a

Thanks again,
	Andre

On Tue, 20 Aug 2024 09:16:50 -0700
Jerry D <jvdelisle2@gmail.com> wrote:

> On 8/20/24 5:35 AM, Andre Vehreschild wrote:
> > Hi all,
> >
> > pinging this patch.
> >
> > Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?
> >
> > Regards,
> > 	Andre
> >
>
> Your approach looks reasonable so I think OK to push.
>
> Thanks,
>
> Jerry
>


--
Andre Vehreschild * Email: vehre ad gmx dot de
diff mbox series

Patch

From 0699756fd047873d267c15b226f11b799edd8d94 Mon Sep 17 00:00:00 2001
From: Andre Vehreschild <vehre@gcc.gnu.org>
Date: Thu, 18 Jul 2024 14:53:31 +0200
Subject: [PATCH] Fortran: Fix ICE in sizeof(coarray) [PR77518]

Use se's class_container where present in sizeof().

	PR fortran/77518

gcc/fortran/ChangeLog:

	* trans-intrinsic.cc (gfc_conv_intrinsic_sizeof): User
	class_container of se when set.

gcc/testsuite/ChangeLog:

	* gfortran.dg/coarray/sizeof_1.f90: New test.
---
 gcc/fortran/trans-intrinsic.cc                |  7 +++--
 .../gfortran.dg/coarray/sizeof_1.f90          | 27 +++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/coarray/sizeof_1.f90

diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index d58bea30101c..9e03615e7275 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -8163,8 +8163,11 @@  gfc_conv_intrinsic_sizeof (gfc_se *se, gfc_expr *expr)
 		   && arg->ref && arg->ref->type == REF_COMPONENT))
 	/* The scalarizer added an additional temp.  To get the class' vptr
 	   one has to look at the original backend_decl.  */
-	byte_size = gfc_class_vtab_size_get (
-	      GFC_DECL_SAVED_DESCRIPTOR (arg->symtree->n.sym->backend_decl));
+	if (argse.class_container)
+	  byte_size = gfc_class_vtab_size_get (argse.class_container);
+	else
+	  byte_size = gfc_class_vtab_size_get (
+	    GFC_DECL_SAVED_DESCRIPTOR (arg->symtree->n.sym->backend_decl));
       else
 	gcc_unreachable ();
     }
diff --git a/gcc/testsuite/gfortran.dg/coarray/sizeof_1.f90 b/gcc/testsuite/gfortran.dg/coarray/sizeof_1.f90
new file mode 100644
index 000000000000..b26f84164068
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/sizeof_1.f90
@@ -0,0 +1,27 @@ 
+!{ dg-do run }
+
+! Check that pr77518 is fixed.
+! Based on code by Gerhard Steinmetz  <gerhard.steinmetz.fortran@t-online.de>
+
+program coarray_sizeof_1
+  type t
+  end type
+  type t2
+    integer :: v = 42
+  end type
+  type t3
+    type(t2) :: s
+    integer :: n = 1
+  end type
+
+  class(t), allocatable :: z[:]
+  class(t2), allocatable :: z2[:]
+  class(t3), allocatable :: z3[:]
+
+  if (sizeof(z) /= 0) stop 1
+  if (sizeof(z2) /= sizeof(integer)) stop 2
+  allocate(z3[*])
+  if (sizeof(z3) /= sizeof(z2) + sizeof(integer)) stop 3
+  if (sizeof(z3%s) /= sizeof(z2)) stop 4
+end
+
--
2.45.2