diff mbox series

[fortran] PR fortran/90350 - ubound ICE on assumed size array even though explicit bound is specified

Message ID c5f27682-07ac-5ffc-d8ab-d4cb72e6414c@gmail.com
State New
Headers show
Series [fortran] PR fortran/90350 - ubound ICE on assumed size array even though explicit bound is specified | expand

Commit Message

José Rui Faustino de Sousa April 19, 2020, 4:04 p.m. UTC 
Hi all!

Proposed patch to Bug 90350 - ubound ICE on assumed size array even 
though explicit bound is specified

Patch tested only on x86_64-pc-linux-gnu.

Bumped into the same problem.

Probably a better fix would be to add an extra step to the reference 
chain reflecting that array-section are explicit-shape arrays not 
whatever that was sectioned. But, although this pattern of problem shows 
up in the code in other places, it may be more trouble than it is worth...

Thank you very much.

Best regards,
José Rui

2020-4-19  José Rui Faustino de Sousa  <jrfsousa@gmail.com>

  PR fortran/90350
  * simplify.c (simplify_bound): In the case of assumed-size arrays check
  if the reference is to a full array.

2020-4-19  José Rui Faustino de Sousa  <jrfsousa@gmail.com>

  PR fortran/90350
  * PR90350.f90: New test.

Comments

Thomas Koenig April 19, 2020, 4:31 p.m. UTC | #1 
Hi Jose,

first, thanks for coming on board!

A question: Do you have a copyright assignment yet?  This patch is
probably short enough that it can be accepted without it, but if
you're planning to contribute more (which I certainly hope) then
it would make sense to do this.

Regarding your patch, I have one question: What will happen
with the test case

program artificial
implicit none
integer :: arr(-10:10)
    call asub(arr,size(arr))
end program artificial
subroutine asub(arr,n)
integer,intent(in) :: arr(*)
integer,intent(in) :: n
    write(*,*)'UPPER=',ubound(arr(3:))
    write(*,*)'LOWER=',lbound(arr(3:))
    write(*,*)'SIZE=',size(arr(3:))
end subroutine asub

? In other words, maybe a check on the upper bound
of the last dimension would be better?

Regards

	Thomas
José Rui Faustino de Sousa April 19, 2020, 6:03 p.m. UTC | #2 
Hi Thomas!

 > ? In other words, maybe a check on the upper bound
 > of the last dimension would be better?
 >

You mean enforcing:

C928 (R921) The second subscript shall not be omitted from a 
subscript-triplet in the last dimension of an assumed-size array.

right?

If I have correctly understood the way things are done this is a more 
general test which is already done at resolve.c around line 4690.

One could just duplicate the test to be extra safe.

 > A question: Do you have a copyright assignment yet?
 >

Yes, I have already done that.

Best regards,
José Rui
Thomas Koenig April 20, 2020, 9:58 a.m. UTC | #3 
Am 19.04.20 um 20:03 schrieb José Rui Faustino de Sousa via Fortran:
> Hi Thomas!
> 
>  > ? In other words, maybe a check on the upper bound
>  > of the last dimension would be better?
>  >
> 
> You mean enforcing:
> 
> C928 (R921) The second subscript shall not be omitted from a 
> subscript-triplet in the last dimension of an assumed-size array.
> 
> right?
> 
> If I have correctly understood the way things are done this is a more 
> general test which is already done at resolve.c around line 4690.

I just checked that it works, so I think this is fine.  It looks
like assumed_size_refs_4.f90 already checks for that, so that is
fine.

Other than that, a couple of points:

The style you used for your patch,

+      if (upper
+         && type == AR_FULL
+         && as
+         && as->type == AS_ASSUMED_SIZE)

doesn't conform to the GNU style guidelines. This should go on one
line (as long as it fits).

Also, I could not apply your patch via copy & paste from the text of
the e-mail. Could you send patches as an attachment in the future?
Due do some unfortunate limitations of the current mailing list
software, it is probably best if you send it as a *.txt file.

So, OK for trunk with the cosmetic corrections above.

Thanks a lot for the patch, and welcome aboard!

>  > A question: Do you have a copyright assignment yet?
>  >
> 
> Yes, I have already done that.

Excellent.

Best regards

	Thomas
diff mbox series

Patch

diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index d5703e3..4818368 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4157,6 +4157,7 @@  simplify_bound (gfc_expr *array, gfc_expr *dim, 
gfc_expr *kind, int upper)
  {
    gfc_ref *ref;
    gfc_array_spec *as;
+  ar_type type = AR_UNKNOWN;
    int d;

    if (array->ts.type == BT_CLASS)
@@ -4180,6 +4181,7 @@  simplify_bound (gfc_expr *array, gfc_expr *dim, 
gfc_expr *kind, int upper)
        switch (ref->type)
         {
         case REF_ARRAY:
+         type = ref->u.ar.type;
           switch (ref->u.ar.type)
             {
             case AR_ELEMENT:
@@ -4233,7 +4235,10 @@  simplify_bound (gfc_expr *array, gfc_expr *dim, 
gfc_expr *kind, int upper)
        int k;

        /* UBOUND(ARRAY) is not valid for an assumed-size array.  */
-      if (upper && as && as->type == AS_ASSUMED_SIZE)
+      if (upper
+         && type == AR_FULL
+         && as
+         && as->type == AS_ASSUMED_SIZE)
         {
           /* An error message will be emitted in
              check_assumed_size_reference (resolve.c).  */
diff --git a/gcc/testsuite/gfortran.dg/PR90350.f90 
b/gcc/testsuite/gfortran.dg/PR90350.f90
new file mode 100644
index 0000000..2e2cf10
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/PR90350.f90
@@ -0,0 +1,19 @@ 
+! { dg-do compile }
+!
+! Test the fix for PR90350
+!
+! Contributed by  <urbanjost@comcast.net>
+!
+
+program artificial
+implicit none
+integer :: arr(-10:10)
+   call asub(arr,size(arr))
+end program artificial
+subroutine asub(arr,n)
+integer,intent(in) :: arr(*)
+integer,intent(in) :: n
+   write(*,*)'UPPER=',ubound(arr(:n))
+   write(*,*)'LOWER=',lbound(arr(:n))
+   write(*,*)'SIZE=',size(arr(:n))
+end subroutine asub