[Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

Hi all,

as the PR (and the attached testcase) shows, gfortran doesn't handle 
noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is 
mishandled (segfault while building the "tree" as expr == NULL).

As Jerry has dug up in the standard (F2008, R610 alias F2018, R908), 
substrings are only permitted on scalars (or array elements). – 
[gfortran has some partial support for noncontiguous memory as actual 
argument the now rejected test cases show.]

In words of the standard (here: F2018):

9.4.1  Substrings

A substring is a contiguous portion of a character string (7.4.4).

R908   substring   is   parent-string  ( substring-range )

R909   parent-string
             is   scalar-variable-name
             or  array-element
             or  coindexed-named-object
             or  scalar-structure-component
             or  scalar-constant

The patch adds a check to the variable resolving – it comes too late to 
print a nice error message for "string(:)(:)"; that statement gets 
matched unsuccessfully and at the end the buffered error message of 
match_variable (unexpected character "(") gets printed. – One could 
reject it earlier but the code doesn't seem to be that nice. (See PR for 
a variant.)

Build and regtested on x86_64-linux.
OK for the trunk?

Cheers

Tobias


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: array_substring.diff --]
[-- Type: text/x-patch; name="array_substring.diff", Size: 4992 bytes --]

2018-10-08  Tobias Burnus <burnus@net-b.de>

	PR fortran/83522
	* resolve.c (resolve_ref): Reject nonscalar
	substring references.

        PR fortran/83522
	* gfortran.dg/actual_array_substr_1.f90: Add dg-error,
	change to dg-do compile.
	* gfortran.dg/actual_array_substr_2.f90: Ditto.
	* gfortran.dg/array_initializer_1.f90: Use array
	element not size-one section.
	* gfortran.dg/array_substring.f90: New.

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 87e65df5f4e..4ec881e5d78 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -5129,6 +5129,13 @@ resolve_ref (gfc_expr *expr)
 	  break;
 
 	case REF_SUBSTRING:
+	  /* F2008, R610 alias F2018, R908.  */
+	  if (current_part_dimension || seen_part_dimension)
+	    {
+	      gfc_error ("Substring reference of nonscalar not permitted at %L",
+			 &expr->where);
+	      return false;
+	    }
 	  break;
 	}
 
diff --git a/gcc/testsuite/gfortran.dg/actual_array_substr_1.f90 b/gcc/testsuite/gfortran.dg/actual_array_substr_1.f90
index 7c10c7020eb..d992f45f465 100644
--- a/gcc/testsuite/gfortran.dg/actual_array_substr_1.f90
+++ b/gcc/testsuite/gfortran.dg/actual_array_substr_1.f90
@@ -1,15 +1,17 @@
-! { dg-do run }
+! { dg-do compile }
 ! Test fix of PR28118, in which a substring reference to an
 ! actual argument with an array reference would cause a segfault.
 !
+! Revised for PR fortran/83522
+!
 ! Contributed by Paul Thomas  <pault@gcc.gnu.org>
 !
 program gfcbug33
   character(12) :: a(2)
   a(1) = "abcdefghijkl"
   a(2) = "mnopqrstuvwx"
-  call foo ((a(2:1:-1)(6:)))
-  call bar ((a(:)(7:11)))
+  call foo ((a(2:1:-1)(6:))) ! { dg-error "Substring reference of nonscalar not permitted" }
+  call bar ((a(:)(7:11))) ! { dg-error "Substring reference of nonscalar not permitted" }
 contains
   subroutine foo (chr)
     character(7) :: chr(:)
diff --git a/gcc/testsuite/gfortran.dg/actual_array_substr_2.f90 b/gcc/testsuite/gfortran.dg/actual_array_substr_2.f90
index fed51342ee0..9bcd19c3fb0 100644
--- a/gcc/testsuite/gfortran.dg/actual_array_substr_2.f90
+++ b/gcc/testsuite/gfortran.dg/actual_array_substr_2.f90
@@ -1,10 +1,12 @@
-! { dg-do run }
+! { dg-do compile }
 ! Tests the fix for pr28174, in which the fix for pr28118 was
 ! corrupting the character lengths of arrays that shared a
 ! character length structure.  In addition, in developing the
 ! fix, it was noted that intent(out/inout) arguments were not
 ! getting written back to the calling scope.
 !
+! Revised for PR fortran/83522
+!
 ! Based on the testscase by Harald Anlauf  <anlauf@gmx.de>
 !
 program pr28174
@@ -20,7 +22,7 @@ program pr28174
   n = m - 4
 
 ! Make sure that variable substring references work.
-  call foo (a(:)(m:m+5), c(:)(n:m+2), d(:)(5:9))
+  call foo (a(:)(m:m+5), c(:)(n:m+2), d(:)(5:9)) ! { dg-error "Substring reference of nonscalar not permitted" }
   if (any (a .ne. teststring)) STOP 1
   if (any (b .ne. teststring)) STOP 2
   if (any (c .ne. (/"ab456789#hij", &
@@ -37,8 +39,7 @@ contains
 ! This next is not required by the standard but tests the
 ! functioning of the gfortran implementation.
 !   if (all (x(:)(3:7) .eq. y)) STOP 5
-    x = foostring (:)(5 : 4 + len (x))
-    y = foostring (:)(3 : 2 + len (y))
+    x = foostring (:)(5 : 4 + len (x)) ! { dg-error "Substring reference of nonscalar not permitted" }
+    y = foostring (:)(3 : 2 + len (y)) ! { dg-error "Substring reference of nonscalar not permitted" }
   end subroutine foo
 end program pr28174
-
diff --git a/gcc/testsuite/gfortran.dg/array_initializer_1.f90 b/gcc/testsuite/gfortran.dg/array_initializer_1.f90
index d4f5e940c10..28c9272f3ac 100644
--- a/gcc/testsuite/gfortran.dg/array_initializer_1.f90
+++ b/gcc/testsuite/gfortran.dg/array_initializer_1.f90
@@ -18,7 +18,7 @@
 
   character(4), parameter :: chr(4) = (/"abcd", "efgh", "ijkl", "mnop"/)
   character(4), parameter :: chrs = chr(ii)(2:3)//chr(2)(ii-3:ii-2) 
-  character(4), parameter :: chrt(2) = (/chr(2:2)(2:3), chr(ii-1)(3:ii)/)
+  character(4), parameter :: chrt(2) = (/chr(2)(2:3), chr(ii-1)(3:ii)/)
   character(2), parameter :: chrx(2) = (/(chr(i)(i:i+1), i=2,3)/)
 
   if (any (y .ne. (/5., 6., 15., 16./))) STOP 1
diff --git a/gcc/testsuite/gfortran.dg/array_substring.f90 b/gcc/testsuite/gfortran.dg/array_substring.f90
new file mode 100644
index 00000000000..ea80556cfc1
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/array_substring.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+!
+! PR fortran/83522
+!
+! Contributed by urbanjost and Jerry DeLisle
+!
+program testit
+  character(len=:),allocatable :: strings(:)
+  integer :: i
+  strings=[character(len=2) :: 'AA','BB']
+  write(*,*)strings(:)(:)  ! { dg-error "Substring reference of nonscalar not permitted" }
+  !strings(:)(:) ! Parse error: "Invalid character in name"
+  strings(:)(:) = 'x'   ! { dg-error "Substring reference of nonscalar not permitted" }
+  do i=1, size(strings)
+    write(*,*)strings(i)(:)  ! This is valid and works
+  end do
+end program testit

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Thomas Koenig]

Hi Tobias,

nice to hear from you again!

> Build and regtested on x86_64-linux.
> OK for the trunk?

OK. Thanks for the patch!

Regards

	Thomas

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Paul Richard Thomas]

This seems to have caused errors in the testsuite. I guess that the
problem is that the testcases are invalid :-(

From David Edelsohn:
Error: Substring reference of nonscalar not permitted at (1)

arrayio_11.f90
arrayio_12.f90
associate_23.f90
data_char_1.f90
deferred_character_2.f90
deferred_character_22.f90
deferred_character_23.f90
deferred_character_8.f90
deferred_type_component_3.f90

Cheers

Paul

On Mon, 8 Oct 2018 at 23:16, Thomas Koenig <tkoenig@netcologne.de> wrote:
>
> Hi Tobias,
>
> nice to hear from you again!
>
> > Build and regtested on x86_64-linux.
> > OK for the trunk?
>
> OK. Thanks for the patch!
>
> Regards
>
>         Thomas



--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Jakub Jelinek]

On Mon, Oct 08, 2018 at 09:44:57PM +0200, Tobias Burnus wrote:
> as the PR (and the attached testcase) shows, gfortran doesn't handle
> noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is
> mishandled (segfault while building the "tree" as expr == NULL).
> 
> As Jerry has dug up in the standard (F2008, R610 alias F2018, R908),
> substrings are only permitted on scalars (or array elements). – [gfortran
> has some partial support for noncontiguous memory as actual argument the now
> rejected test cases show.]
> 
> In words of the standard (here: F2018):
> 
> 9.4.1  Substrings
> 
> A substring is a contiguous portion of a character string (7.4.4).
> 
> R908   substring   is   parent-string  ( substring-range )
> 
> R909   parent-string
>             is   scalar-variable-name
>             or  array-element
>             or  coindexed-named-object
>             or  scalar-structure-component
>             or  scalar-constant
> 
> The patch adds a check to the variable resolving – it comes too late to
> print a nice error message for "string(:)(:)"; that statement gets matched
> unsuccessfully and at the end the buffered error message of match_variable
> (unexpected character "(") gets printed. – One could reject it earlier but
> the code doesn't seem to be that nice. (See PR for a variant.)
> 
> Build and regtested on x86_64-linux.
> OK for the trunk?

As I wrote in the PR, I believe it is incorrect and should be reverted.
While for
  character(kind=1,length=4) :: f(4,5)
e.g.
  f(:,:)(1:2)
is not matching the substring non-terminal, it is IMNSHO matching the
array-section non-terminal:
R918   array-section
            is data-ref [ ( substring-range ) ]
            or complex-part-designator
C925   (R918) Exactly one part-ref shall have nonzero rank, and either the final
	      part-ref shall have a section-subscript-list with nonzero rank, another
	      part-ref shall have nonzero rank, or the complex-part-designator
	      shall be an array.
C926   (R918) If a substring-range appears, the rightmost part-name shall be of type
	      character.
and in that case:

An array element is a scalar. An array section is an array. If a substring-range
appears in an array-section, each element is the designated substring of the
corresponding element of the array section.

applies.

	Jakub

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[David Edelsohn]

Apparently this change also breaks CPU SPEC (465.tonto?).

Is this really a correct change?  Even if the testcases really are
wrong, it seems that this change is going to break a lot of code in
the wild.  This seems like a very bad choice, even if it pedantically
is correct.

Thanks, David

On Wed, Oct 10, 2018 at 7:12 PM Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
>
> This seems to have caused errors in the testsuite. I guess that the
> problem is that the testcases are invalid :-(
>
> From David Edelsohn:
> Error: Substring reference of nonscalar not permitted at (1)
>
> arrayio_11.f90
> arrayio_12.f90
> associate_23.f90
> data_char_1.f90
> deferred_character_2.f90
> deferred_character_22.f90
> deferred_character_23.f90
> deferred_character_8.f90
> deferred_type_component_3.f90
>
> Cheers
>
> Paul
>
> On Mon, 8 Oct 2018 at 23:16, Thomas Koenig <tkoenig@netcologne.de> wrote:
> >
> > Hi Tobias,
> >
> > nice to hear from you again!
> >
> > > Build and regtested on x86_64-linux.
> > > OK for the trunk?
> >
> > OK. Thanks for the patch!
> >
> > Regards
> >
> >         Thomas
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[David Edelsohn]

Another data point: I tried some of the testcases with IBM XL Fortran
compiler and it didn't complain or fail.  I have no idea if this
behavior is an IBM extension or how other Fortran compilers behave.

GNU Fortran probably should be compatible with other Fortran
compilers, even if the behavior is not pedantically conformant with
the standard.

Thanks, David

On Wed, Oct 10, 2018 at 7:12 PM Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
>
> This seems to have caused errors in the testsuite. I guess that the
> problem is that the testcases are invalid :-(
>
> From David Edelsohn:
> Error: Substring reference of nonscalar not permitted at (1)
>
> arrayio_11.f90
> arrayio_12.f90
> associate_23.f90
> data_char_1.f90
> deferred_character_2.f90
> deferred_character_22.f90
> deferred_character_23.f90
> deferred_character_8.f90
> deferred_type_component_3.f90
>
> Cheers
>
> Paul
>
> On Mon, 8 Oct 2018 at 23:16, Thomas Koenig <tkoenig@netcologne.de> wrote:
> >
> > Hi Tobias,
> >
> > nice to hear from you again!
> >
> > > Build and regtested on x86_64-linux.
> > > OK for the trunk?
> >
> > OK. Thanks for the patch!
> >
> > Regards
> >
> >         Thomas
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Paul Richard Thomas]

The section from the standard in comment #2 is being misinterpreted.
R609 and R610 concern substrings in the context of section 6.4, which
concerns SCALARS!

Section 6.5 concerns arrays and you will find there:

R618 array-section is data-ref [ ( substring-range ) ] or
complex-part-designator

C624 (R618) Exactly one part-ref shall have nonzero rank, and either
the final part-ref shall have a section-
subscript-list with nonzero rank, another part-ref shall have nonzero
rank, or the complex-part-designator
shall be an array.

Further lines appear later in which in which substrings of array
sections are explicitly mentioned: eg.
If a substring-range appears in an array-section, each element is the
designated substring of the corresponding element of the array
section. The value of a subscript in an array element shall be within
the bounds for its dimension.

Thus the patch is contrary to the standard and must be removed as soon
as possible.

Paul


On Thu, 11 Oct 2018 at 15:20, David Edelsohn <dje.gcc@gmail.com> wrote:
>
> Another data point: I tried some of the testcases with IBM XL Fortran
> compiler and it didn't complain or fail.  I have no idea if this
> behavior is an IBM extension or how other Fortran compilers behave.
>
> GNU Fortran probably should be compatible with other Fortran
> compilers, even if the behavior is not pedantically conformant with
> the standard.
>
> Thanks, David
>
> On Wed, Oct 10, 2018 at 7:12 PM Paul Richard Thomas
> <paul.richard.thomas@gmail.com> wrote:
> >
> > This seems to have caused errors in the testsuite. I guess that the
> > problem is that the testcases are invalid :-(
> >
> > From David Edelsohn:
> > Error: Substring reference of nonscalar not permitted at (1)
> >
> > arrayio_11.f90
> > arrayio_12.f90
> > associate_23.f90
> > data_char_1.f90
> > deferred_character_2.f90
> > deferred_character_22.f90
> > deferred_character_23.f90
> > deferred_character_8.f90
> > deferred_type_component_3.f90
> >
> > Cheers
> >
> > Paul
> >
> > On Mon, 8 Oct 2018 at 23:16, Thomas Koenig <tkoenig@netcologne.de> wrote:
> > >
> > > Hi Tobias,
> > >
> > > nice to hear from you again!
> > >
> > > > Build and regtested on x86_64-linux.
> > > > OK for the trunk?
> > >
> > > OK. Thanks for the patch!
> > >
> > > Regards
> > >
> > >         Thomas
> >
> >
> >
> > --
> > "If you can't explain it simply, you don't understand it well enough"
> > - Albert Einstein



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Tobias Burnus]

In light of the somehow missed test-suite fails and the discussion about 
rejecting too much, I have reverted the patch with commit r83522.

Tobias

Am 11.10.18 um 12:46 schrieb Jakub Jelinek:
> On Mon, Oct 08, 2018 at 09:44:57PM +0200, Tobias Burnus wrote:
>> as the PR (and the attached testcase) shows, gfortran doesn't handle
>> noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is
>> mishandled (segfault while building the "tree" as expr == NULL).
>>
>> As Jerry has dug up in the standard (F2008, R610 alias F2018, R908),
>> substrings are only permitted on scalars (or array elements). – [gfortran
>> has some partial support for noncontiguous memory as actual argument the now
>> rejected test cases show.]
>>
>> In words of the standard (here: F2018):
>>
>> 9.4.1  Substrings
>>
>> A substring is a contiguous portion of a character string (7.4.4).
>>
>> R908   substring   is   parent-string  ( substring-range )
>>
>> R909   parent-string
>>              is   scalar-variable-name
>>              or  array-element
>>              or  coindexed-named-object
>>              or  scalar-structure-component
>>              or  scalar-constant
>>
>> The patch adds a check to the variable resolving – it comes too late to
>> print a nice error message for "string(:)(:)"; that statement gets matched
>> unsuccessfully and at the end the buffered error message of match_variable
>> (unexpected character "(") gets printed. – One could reject it earlier but
>> the code doesn't seem to be that nice. (See PR for a variant.)
>>
>> Build and regtested on x86_64-linux.
>> OK for the trunk?
> As I wrote in the PR, I believe it is incorrect and should be reverted.
> While for
>    character(kind=1,length=4) :: f(4,5)
> e.g.
>    f(:,:)(1:2)
> is not matching the substring non-terminal, it is IMNSHO matching the
> array-section non-terminal:
> R918   array-section
>              is data-ref [ ( substring-range ) ]
>              or complex-part-designator
> C925   (R918) Exactly one part-ref shall have nonzero rank, and either the final
> 	      part-ref shall have a section-subscript-list with nonzero rank, another
> 	      part-ref shall have nonzero rank, or the complex-part-designator
> 	      shall be an array.
> C926   (R918) If a substring-range appears, the rightmost part-name shall be of type
> 	      character.
> and in that case:
>
> An array element is a scalar. An array section is an array. If a substring-range
> appears in an array-section, each element is the designated substring of the
> corresponding element of the array section.
>
> applies.
>
> 	Jakub

Re: [Patch, Fortran] PR fortran/83522 – reject array-valued substrings

[Paul Richard Thomas]

Hi Tobias,

I am sorry that your return has proven to be a bit bloody. Thanks for
reverting it. I am afraid that Jerry missed the context completely.

Regards

Paul

On Thu, 11 Oct 2018 at 20:37, Tobias Burnus <burnus@net-b.de> wrote:
>
> In light of the somehow missed test-suite fails and the discussion about
> rejecting too much, I have reverted the patch with commit r83522.
>
> Tobias
>
> Am 11.10.18 um 12:46 schrieb Jakub Jelinek:
> > On Mon, Oct 08, 2018 at 09:44:57PM +0200, Tobias Burnus wrote:
> >> as the PR (and the attached testcase) shows, gfortran doesn't handle
> >> noncontiguous access with substrings. Namely, "str_array(:)(1:5)" is
> >> mishandled (segfault while building the "tree" as expr == NULL).
> >>
> >> As Jerry has dug up in the standard (F2008, R610 alias F2018, R908),
> >> substrings are only permitted on scalars (or array elements). – [gfortran
> >> has some partial support for noncontiguous memory as actual argument the now
> >> rejected test cases show.]
> >>
> >> In words of the standard (here: F2018):
> >>
> >> 9.4.1  Substrings
> >>
> >> A substring is a contiguous portion of a character string (7.4.4).
> >>
> >> R908   substring   is   parent-string  ( substring-range )
> >>
> >> R909   parent-string
> >>              is   scalar-variable-name
> >>              or  array-element
> >>              or  coindexed-named-object
> >>              or  scalar-structure-component
> >>              or  scalar-constant
> >>
> >> The patch adds a check to the variable resolving – it comes too late to
> >> print a nice error message for "string(:)(:)"; that statement gets matched
> >> unsuccessfully and at the end the buffered error message of match_variable
> >> (unexpected character "(") gets printed. – One could reject it earlier but
> >> the code doesn't seem to be that nice. (See PR for a variant.)
> >>
> >> Build and regtested on x86_64-linux.
> >> OK for the trunk?
> > As I wrote in the PR, I believe it is incorrect and should be reverted.
> > While for
> >    character(kind=1,length=4) :: f(4,5)
> > e.g.
> >    f(:,:)(1:2)
> > is not matching the substring non-terminal, it is IMNSHO matching the
> > array-section non-terminal:
> > R918   array-section
> >              is data-ref [ ( substring-range ) ]
> >              or complex-part-designator
> > C925   (R918) Exactly one part-ref shall have nonzero rank, and either the final
> >             part-ref shall have a section-subscript-list with nonzero rank, another
> >             part-ref shall have nonzero rank, or the complex-part-designator
> >             shall be an array.
> > C926   (R918) If a substring-range appears, the rightmost part-name shall be of type
> >             character.
> > and in that case:
> >
> > An array element is a scalar. An array section is an array. If a substring-range
> > appears in an array-section, each element is the designated substring of the
> > corresponding element of the array section.
> >
> > applies.
> >
> >       Jakub



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein