Re: [patch,Fortran] PR 92050 - fix ICE with -fcheck=all
On Mon, Nov 25, 2019 at 03:34:25PM +0100, Tobias Burnus wrote: > > well, the question is what counts as regression. In any case, I have now > committed that patch as r278689. > Regression is fairly easy to define. Standard conforming code that compiled and executed correctly (for some definition of correct) prior to a patch, which no longer compiles and/or executes, is a regression. Yes, I know, there are the latent bugs that pop up, which appear to be regressions. :( Thanks for your recent patches. -- Steve
Re: [patch,Fortran] PR 92050 - fix ICE with -fcheck=all
Hi Steve, well, the question is what counts as regression. In any case, I have now committed that patch as r278689. Cheers, Tobias
Re: [patch,Fortran] PR 92050 - fix ICE with -fcheck=all
Just my $0.02.
Backporting a patch from trunk that fixes a regression is
always encouraged. It is up to the person doing the backport
to determine the level of effort. If it exceeds some threshold
the person can choose to not backport.
For patches that fix a bug, which is not a regresssion, but
allows gfortran to accept valid Fortran or reject invalid Fortran,
I think that this is up to the discretion of the person doing the
backport. For example, I backported several patches that fixed
free-source code parsing issues with attribute statements (i.e.,
gfortran accepted "publica" as if it were "public a").
For new features, I think backporting is okay if the new feature
is isolated behind an option.
--
steve
On Fri, Nov 22, 2019 at 01:36:03PM +0100, Tobias Burnus wrote:
> Hi all,
>
> I was asked to backport this fix to the GCC 9 branch – given that
> -fcheck=bounds is a common option and it fails with code like
> genecode.org. Given that ICEs with -fcheck are a regression and that the
> patch is not that invasive, I am inclined to accept it.
>
> Comments/(dis)approvals?
>
> Tobias
>
> On 10/11/19 12:17 PM, Tobias Burnus wrote:
> > Checking produces extra code (unsurprisingly); this code needs to end
> > up in the program…
> >
> > Bootstrapped on x86-64_gnu-linux. OK for the trunk?
> >
> > Tobias
> >
> >
> > pr92050.diff
> >
> > diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
> > index 965ab7786a1..65238ff623d 100644
> > --- a/gcc/fortran/trans-expr.c
> > +++ b/gcc/fortran/trans-expr.c
> > @@ -7031,8 +7031,11 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol *
> > sym,
> > gfc_allocate_lang_decl (result);
> > GFC_DECL_SAVED_DESCRIPTOR (result) = parmse.expr;
> > gfc_free_expr (class_expr);
> > - gcc_assert (parmse.pre.head == NULL_TREE
> > - && parmse.post.head == NULL_TREE);
> > + /* -fcheck= can add diagnostic code, which has to be placed before
> > +the call. */
> > + if (parmse.pre.head != NULL)
> > + gfc_add_expr_to_block (>pre, parmse.pre.head);
> > + gcc_assert (parmse.post.head == NULL_TREE);
> > }
> >
> > /* Follow the function call with the argument post block. */
> > diff --git a/gcc/testsuite/gfortran.dg/pr92050.f90
> > b/gcc/testsuite/gfortran.dg/pr92050.f90
> > new file mode 100644
> > index 000..64193878d8f
> > --- /dev/null
> > +++ b/gcc/testsuite/gfortran.dg/pr92050.f90
> > @@ -0,0 +1,53 @@
> > +! { dg-do run }
> > +! { dg-options "-fcheck=all" }
> > +! { dg-shouldfail "above upper bound" }
> > +!
> > +! PR fortran/92050
> > +!
> > +!
> > +module buggy
> > + implicit none (type, external)
> > +
> > + type :: par
> > + contains
> > +procedure, public :: fun => fun_par
> > + end type par
> > +
> > + type comp
> > +class(par), allocatable :: p
> > + end type comp
> > +
> > + type foo
> > +type(comp), allocatable :: m(:)
> > + end type foo
> > +
> > +contains
> > +
> > + function fun_par(this)
> > +class(par) :: this
> > +integer:: fun_par(1)
> > +fun_par = 42
> > + end function fun_par
> > +
> > + subroutine update_foo(this)
> > +class(foo) :: this
> > +write(*,*) this%m(1)%p%fun()
> > + end subroutine update_foo
> > +
> > + subroutine bad_update_foo(this)
> > +class(foo) :: this
> > +write(*,*) this%m(2)%p%fun()
> > + end subroutine bad_update_foo
> > +end module buggy
> > +
> > +program main
> > + use buggy
> > + implicit none (type, external)
> > + type(foo) :: x
> > + allocate(x%m(1))
> > + allocate(x%m(1)%p)
> > + call update_foo(x)
> > + call bad_update_foo(x)
> > +end program main
> > +
> > +! { dg-output "At line 39 of file .*pr92050.f90.*Fortran runtime error:
> > Index '2' of dimension 1 of array 'this%m' above upper bound of 1" }
--
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow
Re: [patch,Fortran] PR 92050 - fix ICE with -fcheck=all
Hi all,
I was asked to backport this fix to the GCC 9 branch – given that
-fcheck=bounds is a common option and it fails with code like
genecode.org. Given that ICEs with -fcheck are a regression and that the
patch is not that invasive, I am inclined to accept it.
Comments/(dis)approvals?
Tobias
On 10/11/19 12:17 PM, Tobias Burnus wrote:
Checking produces extra code (unsurprisingly); this code needs to end
up in the program…
Bootstrapped on x86-64_gnu-linux. OK for the trunk?
Tobias
pr92050.diff
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 965ab7786a1..65238ff623d 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -7031,8 +7031,11 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
gfc_allocate_lang_decl (result);
GFC_DECL_SAVED_DESCRIPTOR (result) = parmse.expr;
gfc_free_expr (class_expr);
- gcc_assert (parmse.pre.head == NULL_TREE
- && parmse.post.head == NULL_TREE);
+ /* -fcheck= can add diagnostic code, which has to be placed before
+the call. */
+ if (parmse.pre.head != NULL)
+ gfc_add_expr_to_block (>pre, parmse.pre.head);
+ gcc_assert (parmse.post.head == NULL_TREE);
}
/* Follow the function call with the argument post block. */
diff --git a/gcc/testsuite/gfortran.dg/pr92050.f90
b/gcc/testsuite/gfortran.dg/pr92050.f90
new file mode 100644
index 000..64193878d8f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr92050.f90
@@ -0,0 +1,53 @@
+! { dg-do run }
+! { dg-options "-fcheck=all" }
+! { dg-shouldfail "above upper bound" }
+!
+! PR fortran/92050
+!
+!
+module buggy
+ implicit none (type, external)
+
+ type :: par
+ contains
+procedure, public :: fun => fun_par
+ end type par
+
+ type comp
+class(par), allocatable :: p
+ end type comp
+
+ type foo
+type(comp), allocatable :: m(:)
+ end type foo
+
+contains
+
+ function fun_par(this)
+class(par) :: this
+integer:: fun_par(1)
+fun_par = 42
+ end function fun_par
+
+ subroutine update_foo(this)
+class(foo) :: this
+write(*,*) this%m(1)%p%fun()
+ end subroutine update_foo
+
+ subroutine bad_update_foo(this)
+class(foo) :: this
+write(*,*) this%m(2)%p%fun()
+ end subroutine bad_update_foo
+end module buggy
+
+program main
+ use buggy
+ implicit none (type, external)
+ type(foo) :: x
+ allocate(x%m(1))
+ allocate(x%m(1)%p)
+ call update_foo(x)
+ call bad_update_foo(x)
+end program main
+
+! { dg-output "At line 39 of file .*pr92050.f90.*Fortran runtime error: Index '2'
of dimension 1 of array 'this%m' above upper bound of 1" }
Re: [patch,Fortran] PR 92050 - fix ICE with -fcheck=all
On Fri, Oct 11, 2019 at 12:17:49PM +0200, Tobias Burnus wrote: > Checking produces extra code (unsurprisingly); this code needs to end up > in the program… > > Bootstrapped on x86-64_gnu-linux. OK for the trunk? > OK. -- Steve
[patch,Fortran] PR 92050 - fix ICE with -fcheck=all
Checking produces extra code (unsurprisingly); this code needs to end up
in the program…
Bootstrapped on x86-64_gnu-linux. OK for the trunk?
Tobias
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 965ab7786a1..65238ff623d 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -7031,8 +7031,11 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
gfc_allocate_lang_decl (result);
GFC_DECL_SAVED_DESCRIPTOR (result) = parmse.expr;
gfc_free_expr (class_expr);
- gcc_assert (parmse.pre.head == NULL_TREE
- && parmse.post.head == NULL_TREE);
+ /* -fcheck= can add diagnostic code, which has to be placed before
+ the call. */
+ if (parmse.pre.head != NULL)
+ gfc_add_expr_to_block (>pre, parmse.pre.head);
+ gcc_assert (parmse.post.head == NULL_TREE);
}
/* Follow the function call with the argument post block. */
diff --git a/gcc/testsuite/gfortran.dg/pr92050.f90 b/gcc/testsuite/gfortran.dg/pr92050.f90
new file mode 100644
index 000..64193878d8f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr92050.f90
@@ -0,0 +1,53 @@
+! { dg-do run }
+! { dg-options "-fcheck=all" }
+! { dg-shouldfail "above upper bound" }
+!
+! PR fortran/92050
+!
+!
+module buggy
+ implicit none (type, external)
+
+ type :: par
+ contains
+procedure, public :: fun => fun_par
+ end type par
+
+ type comp
+class(par), allocatable :: p
+ end type comp
+
+ type foo
+type(comp), allocatable :: m(:)
+ end type foo
+
+contains
+
+ function fun_par(this)
+class(par) :: this
+integer:: fun_par(1)
+fun_par = 42
+ end function fun_par
+
+ subroutine update_foo(this)
+class(foo) :: this
+write(*,*) this%m(1)%p%fun()
+ end subroutine update_foo
+
+ subroutine bad_update_foo(this)
+class(foo) :: this
+write(*,*) this%m(2)%p%fun()
+ end subroutine bad_update_foo
+end module buggy
+
+program main
+ use buggy
+ implicit none (type, external)
+ type(foo) :: x
+ allocate(x%m(1))
+ allocate(x%m(1)%p)
+ call update_foo(x)
+ call bad_update_foo(x)
+end program main
+
+! { dg-output "At line 39 of file .*pr92050.f90.*Fortran runtime error: Index '2' of dimension 1 of array 'this%m' above upper bound of 1" }
