diff mbox series

PR fortran/99205 - [10/11 Regression] Out of memory with undefined character length

Message ID trinity-f0b06340-8653-42d5-b5bf-fcfb5e2ce01e-1615319100120@3c-app-gmx-bs50
State New
Headers show
Series PR fortran/99205 - [10/11 Regression] Out of memory with undefined character length | expand

Commit Message

Harald Anlauf March 9, 2021, 7:45 p.m. UTC
Dear all,

character variables with undefined length are not allowed as
objects in DATA statements.  We better reject them.

Regtested on x86_64-pc-linux-gnu.  OK for master / backport?

Thanks,
Harald


PR fortran/99205 - Out of memory with undefined character length

A variable that is a data statement object shall be a designator,
thus a character variable shall have a constant length.

gcc/fortran/ChangeLog:

	PR fortran/99205
	* data.c (gfc_assign_data_value): Reject non-constant character
	length for lvalue.

gcc/testsuite/ChangeLog:

	PR fortran/99205
	* gfortran.dg/pr99205.f90: New test.

Comments

Tobias Burnus March 10, 2021, 10:06 a.m. UTC | #1
Dear Harald, dear all,

On 09.03.21 20:45, Harald Anlauf via Fortran wrote:
> character variables with undefined length are not allowed as
> objects in DATA statements.  We better reject them.
>
> Regtested on x86_64-pc-linux-gnu.  OK for master / backport?
>
> PR fortran/99205 - Out of memory with undefined character length
>
> A variable that is a data statement object shall be a designator,
> thus a character variable shall have a constant length.

This comment is wrong: A designator does not imply this – nor is
F2018:C875 violated, not even the substring starting/ending point
const expr bit.


The reason that it is not permitted is the "automatic data object"
(see *...* highlighted parts in the quote):

C876   (R839) A *variable* whose *designator* appears as a
*data-stmt-object* or a data-i-do-object hall not be a
dummy argument, accessed by use or host association,
in a named common block unless the DATA statement is
in a block data program unit, in blank common, a function name,
a function result name, an *automatic* *data* *object,*
or an allocatable variable.

With the definition: "3.11 automatic data object
nondummy data object with a type parameter or array bound that
depends on the value of aspecification-expr that is not
a constant expression (8.3)"


In the following variant of the program, the invalid
variable declaration of 'c' itself is avoided by using
a block:

integer :: ll
ll = 4
block
   character(ll) :: c(2), cc(2)
   character(ll) :: c2(2), cc2(2)
   data c /'a', 'b'/
   data c2(:)(1:1) /'a', 'b'/
   common /block/ cc, cc2
end block
end

You may consider to update your testcase or to
augment your testcase.

The patch itself is OK, but you really need to
update the commit description.

Tobias

>
> gcc/fortran/ChangeLog:
>
>       PR fortran/99205
>       * data.c (gfc_assign_data_value): Reject non-constant character
>       length for lvalue.
>
> gcc/testsuite/ChangeLog:
>
>       PR fortran/99205
>       * gfortran.dg/pr99205.f90: New test.
>
>
> pr99205.patch
>
> diff --git a/gcc/fortran/data.c b/gcc/fortran/data.c
> index 25e97930169..71e2552025d 100644
> --- a/gcc/fortran/data.c
> +++ b/gcc/fortran/data.c
> @@ -595,6 +595,9 @@ gfc_assign_data_value (gfc_expr *lvalue, gfc_expr *rvalue, mpz_t index,
>         /* An initializer has to be constant.  */
>         if (lvalue->ts.u.cl->length == NULL && !(ref && ref->u.ss.length != NULL))
>       return false;
> +      if (lvalue->ts.u.cl->length
> +       && lvalue->ts.u.cl->length->expr_type != EXPR_CONSTANT)
> +     return false;
>         expr = create_character_initializer (init, last_ts, ref, rvalue);
>         if (!expr)
>       return false;
> diff --git a/gcc/testsuite/gfortran.dg/pr99205.f90 b/gcc/testsuite/gfortran.dg/pr99205.f90
> new file mode 100644
> index 00000000000..ed0782ce8a0
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr99205.f90
> @@ -0,0 +1,11 @@
> +! { dg-do compile }
> +! PR fortran/99205 - Out of memory with undefined character length
> +! { dg-options "-w" }
> +
> +program p
> +  character(l) :: c(2) ! { dg-error "must have constant character length" }
> +  data c /'a', 'b'/
> +  common c
> +end
> +
> +! { dg-error "cannot appear in the expression at" " " { target *-*-* } 6 }
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Harald Anlauf March 10, 2021, 7:43 p.m. UTC | #2
Dear Tobias,

thanks for your comments.

> > A variable that is a data statement object shall be a designator,
> > thus a character variable shall have a constant length.
> 
> This comment is wrong: A designator does not imply this – nor is
> F2018:C875 violated, not even the substring starting/ending point
> const expr bit.

OK, I will think of a better wording.

> In the following variant of the program, the invalid
> variable declaration of 'c' itself is avoided by using
> a block:
> 
> integer :: ll
> ll = 4
> block
>    character(ll) :: c(2), cc(2)
>    character(ll) :: c2(2), cc2(2)
>    data c /'a', 'b'/
>    data c2(:)(1:1) /'a', 'b'/
>    common /block/ cc, cc2
> end block
> end

No, this example is invalid, see

C1107(R1107) A block-specification-part shall not contain a COMMON, EQUIVALENCE, INTENT, NAMELIST,
OPTIONAL, statement function, or VALUE statement.

Harald
Tobias Burnus March 10, 2021, 8:18 p.m. UTC | #3
Dear Harald,

On 10.03.21 20:43, Harald Anlauf wrote:
>> In the following variant of the program, the invalid
>> variable declaration of 'c' itself is avoided by using
>> a block:
>>
>> integer :: ll
>> ll = 4
>> block
>>     character(ll) :: c(2), cc(2)
>>     character(ll) :: c2(2), cc2(2)
>>     data c /'a', 'b'/
>>     data c2(:)(1:1) /'a', 'b'/
>>     common /block/ cc, cc2
>> end block
>> end
> No, this example is invalid

Of course this example is invalid – I only wrote that it avoids the
issues with the declaration of 'C' itself not that the code as a whole
is valid.

Regarding:

> C1107(R1107) A block-specification-part shall not contain a COMMON, EQUIVALENCE, INTENT, NAMELIST,
> OPTIONAL, statement function, or VALUE statement.

That could be mended by removing the 'common' line. Especially as I
forgot to actually use 'cc'/'cc2' in a data statement ... Or by using a
host-associated 'll' instead of a block. But that still will have the
automatic issue for common, hence, removing common is probably best. One
the other hand:

In any case, there are hundreds of ways to write invalid code – chose
one you like for the testcase :-)

Thanks for this patch – and all the belated patch work!

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Harald Anlauf March 10, 2021, 9:26 p.m. UTC | #4
Dear Tobias, all,

> The reason that it is not permitted is the "automatic data object"
> (see *...* highlighted parts in the quote):
>
> C876   (R839) A *variable* whose *designator* appears as a
> *data-stmt-object* or a data-i-do-object hall not be a
> dummy argument, accessed by use or host association,
> in a named common block unless the DATA statement is
> in a block data program unit, in blank common, a function name,
> a function result name, an *automatic* *data* *object,*
> or an allocatable variable.
>
> With the definition: "3.11 automatic data object
> nondummy data object with a type parameter or array bound that
> depends on the value of aspecification-expr that is not
> a constant expression (8.3)"

inspired by Tobias' (although invalid) code example I found another
testcase which lead to trouble during error recovery due to a NULL
pointer dereference.  Here's the updated changelog for the updated
patch (attached).  I also renamed the first testcase so that they
fit better to the existing scheme.

Again regtested on x86_64-pc-linux-gnu.  Now OK for mainline / 10?

Thanks,
Harald


PR fortran/99205 - Out of memory with undefined character length

A character variable appearing as a data statement object cannot
be automatic, thus it shall have constant length.

gcc/fortran/ChangeLog:

	PR fortran/99205
	* data.c (gfc_assign_data_value): Reject non-constant character
	length for lvalue.
	* trans-array.c (gfc_conv_array_initializer): Restrict loop to
	elements which are defined to avoid NULL pointer dereference.

gcc/testsuite/ChangeLog:

	PR fortran/99205
	* gfortran.dg/data_char_4.f90: New test.
	* gfortran.dg/data_char_5.f90: New test.
Tobias Burnus March 10, 2021, 9:52 p.m. UTC | #5
Dear Harald, dear all,

On 10.03.21 22:26, Harald Anlauf wrote:
> [...] I found another testcase which lead to trouble during error
> recovery due to a NULL pointer dereference.

I was a bit surprised that the crash only occurs in trans*.c;
however, the error 'non-constant initialization expression'
is only issued by gfc_conv_constant, called via gfc_get_symbol_decl.

Thus, when the error is shown we are already deep in trans*.c land.

>    Here's the updated changelog for the updated
> patch (attached).  I also renamed the first testcase so that they
> fit better to the existing scheme.
>
> Again regtested on x86_64-pc-linux-gnu.  Now OK for mainline / 10?

LGTM for mainline and after a bit of waiting for GCC10.

Thanks,

Tobias

> PR fortran/99205 - Out of memory with undefined character length
>
> A character variable appearing as a data statement object cannot
> be automatic, thus it shall have constant length.
>
> gcc/fortran/ChangeLog:
>
>       PR fortran/99205
>       * data.c (gfc_assign_data_value): Reject non-constant character
>       length for lvalue.
>       * trans-array.c (gfc_conv_array_initializer): Restrict loop to
>       elements which are defined to avoid NULL pointer dereference.
>
> gcc/testsuite/ChangeLog:
>
>       PR fortran/99205
>       * gfortran.dg/data_char_4.f90: New test.
>       * gfortran.dg/data_char_5.f90: New test.
>
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
diff mbox series

Patch

diff --git a/gcc/fortran/data.c b/gcc/fortran/data.c
index 25e97930169..71e2552025d 100644
--- a/gcc/fortran/data.c
+++ b/gcc/fortran/data.c
@@ -595,6 +595,9 @@  gfc_assign_data_value (gfc_expr *lvalue, gfc_expr *rvalue, mpz_t index,
       /* An initializer has to be constant.  */
       if (lvalue->ts.u.cl->length == NULL && !(ref && ref->u.ss.length != NULL))
 	return false;
+      if (lvalue->ts.u.cl->length
+	  && lvalue->ts.u.cl->length->expr_type != EXPR_CONSTANT)
+	return false;
       expr = create_character_initializer (init, last_ts, ref, rvalue);
       if (!expr)
 	return false;
diff --git a/gcc/testsuite/gfortran.dg/pr99205.f90 b/gcc/testsuite/gfortran.dg/pr99205.f90
new file mode 100644
index 00000000000..ed0782ce8a0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr99205.f90
@@ -0,0 +1,11 @@ 
+! { dg-do compile }
+! PR fortran/99205 - Out of memory with undefined character length
+! { dg-options "-w" }
+
+program p
+  character(l) :: c(2) ! { dg-error "must have constant character length" }
+  data c /'a', 'b'/
+  common c
+end
+
+! { dg-error "cannot appear in the expression at" " " { target *-*-* } 6 }