[PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Martin Sebor]

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

The attached patch removes the assumption introduced earlier today
in my fix for bug 87996 that the valid_constant_size_p argument is
a constant expression.  I couldn't come up with a C/C++ test case
where this isn't true but apparently it can happen in Ada which I
inadvertently didn't build.  I still haven't figured out what
I have to do to build it on my Fedora 29 machine so I tested
this change by hand (besides bootstrapping w/o Ada).

The first set of instructions Google gives me don't seem to do
it:

   https://fedoraproject.org/wiki/Features/Ada_developer_tools

and neither does dnf install gcc-gnat as explained on our Wiki:

   https://gcc.gnu.org/wiki/GNAT

If someone knows the magic chant I would be grateful (it might
be helpful to also update the Wiki page -- the last change to
it was made in 2012; I volunteer to do that).

Martin

[-- Attachment #2: gcc-89294.diff --]
[-- Type: text/x-patch, Size: 1879 bytes --]

PR middle-end/89294 - ICE in valid_constant_size_p

gcc/c-family/ChangeLog:

	PR middle-end/89294
	* c-common.c (invalid_array_size_error): Handle cst_size_not_constant.

gcc/ChangeLog:

	PR middle-end/89294
	* tree.c (valid_constant_size_p): Avoid assuming size is a constant
	expression.
	* tree.h (cst_size_error): Add the cst_size_not_constant enumerator.

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 268783)
+++ gcc/c-family/c-common.c	(working copy)
@@ -8241,6 +8241,13 @@ invalid_array_size_error (location_t loc, cst_size
   tree maxsize = max_object_size ();
   switch (error)
     {
+    case cst_size_not_constant:
+      if (name)
+	error_at (loc, "size of array %qE is not a constant expression",
+		  name);
+      else
+	error_at (loc, "size of array is not a constant expression");
+      break;
     case cst_size_negative:
       if (name)
 	error_at (loc, "size %qE of array %qE is negative",
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 268783)
+++ gcc/tree.c	(working copy)
@@ -7521,8 +7521,14 @@ valid_constant_size_p (const_tree size, cst_size_e
   if (!perr)
     perr = &error;
 
-  if (TREE_OVERFLOW (size))
+  if (TREE_CODE (size) != INTEGER_CST)
     {
+      *perr = cst_size_not_constant;
+      return false;
+    }
+
+  if (TREE_OVERFLOW_P (size))
+    {
       *perr = cst_size_overflow;
       return false;
     }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 268783)
+++ gcc/tree.h	(working copy)
@@ -4352,6 +4352,7 @@ extern tree excess_precision_type (tree);
    is not a valid size.  */
 enum cst_size_error {
   cst_size_ok,
+  cst_size_not_constant,
   cst_size_negative,
   cst_size_too_big,
   cst_size_overflow

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Rainer Orth]

Hi Martin,

> The attached patch removes the assumption introduced earlier today
> in my fix for bug 87996 that the valid_constant_size_p argument is
> a constant expression.  I couldn't come up with a C/C++ test case
> where this isn't true but apparently it can happen in Ada which I
> inadvertently didn't build.  I still haven't figured out what
> I have to do to build it on my Fedora 29 machine so I tested
> this change by hand (besides bootstrapping w/o Ada).

I've just completed a i386-pc-solaris2.11 bootstrap with your patch and
the failures are gone.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Martin Sebor]

On 2/12/19 4:43 PM, Rainer Orth wrote:
> Hi Martin,
> 
>> The attached patch removes the assumption introduced earlier today
>> in my fix for bug 87996 that the valid_constant_size_p argument is
>> a constant expression.  I couldn't come up with a C/C++ test case
>> where this isn't true but apparently it can happen in Ada which I
>> inadvertently didn't build.  I still haven't figured out what
>> I have to do to build it on my Fedora 29 machine so I tested
>> this change by hand (besides bootstrapping w/o Ada).
> 
> I've just completed a i386-pc-solaris2.11 bootstrap with your patch and
> the failures are gone.

Thanks for the confirmation!  After reinstalling Ada I'm now able
to build it again, even with --enable-languages=all.  Not sure why
that didn't build it the first time.

Martin

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Eric Botcazou]

> The attached patch removes the assumption introduced earlier today
> in my fix for bug 87996 that the valid_constant_size_p argument is
> a constant expression.  I couldn't come up with a C/C++ test case
> where this isn't true but apparently it can happen in Ada which I
> inadvertently didn't build.

Can we do something here?  Our internal testers have been down for 3 days 
because of this blunder...

-- 
Eric Botcazou

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Martin Sebor]

On 2/15/19 12:24 AM, Eric Botcazou wrote:
>> The attached patch removes the assumption introduced earlier today
>> in my fix for bug 87996 that the valid_constant_size_p argument is
>> a constant expression.  I couldn't come up with a C/C++ test case
>> where this isn't true but apparently it can happen in Ada which I
>> inadvertently didn't build.
> 
> Can we do something here?  Our internal testers have been down for 3 days
> because of this blunder...

I'm ready to commit the patch once it's approved, and have been since
the day the problem was reported.


Martin

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Martin Sebor]

Ping: https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00857.html

Jason, since you approved the original patch, can you please also
review this one?  Due to the Ada test breakage there seems to be
some anxiety about getting the problem corrected soon.

Thanks
Martin

On 2/11/19 6:13 PM, Martin Sebor wrote:
> The attached patch removes the assumption introduced earlier today
> in my fix for bug 87996 that the valid_constant_size_p argument is
> a constant expression.  I couldn't come up with a C/C++ test case
> where this isn't true but apparently it can happen in Ada which I
> inadvertently didn't build.  I still haven't figured out what
> I have to do to build it on my Fedora 29 machine so I tested
> this change by hand (besides bootstrapping w/o Ada).
> 
> The first set of instructions Google gives me don't seem to do
> it:
> 
>    https://fedoraproject.org/wiki/Features/Ada_developer_tools
> 
> and neither does dnf install gcc-gnat as explained on our Wiki:
> 
>    https://gcc.gnu.org/wiki/GNAT
> 
> If someone knows the magic chant I would be grateful (it might
> be helpful to also update the Wiki page -- the last change to
> it was made in 2012; I volunteer to do that).
> 
> Martin

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Eric Botcazou]

> I'm ready to commit the patch once it's approved, and have been since
> the day the problem was reported.

Maybe CCing whoever approved the previous patch would help?

-- 
Eric Botcazou

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Martin Sebor]

On 2/15/19 3:46 PM, Eric Botcazou wrote:
>> I'm ready to commit the patch once it's approved, and have been since
>> the day the problem was reported.
> 
> Maybe CCing whoever approved the previous patch would help?

I just pinged the patch a few minutes ago and CC'd Jason.  Sorry
about any trouble this has caused.

Martin

Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

[Richard Biener]

On Tue, Feb 12, 2019 at 2:13 AM Martin Sebor <msebor@gmail.com> wrote:
>
> The attached patch removes the assumption introduced earlier today
> in my fix for bug 87996 that the valid_constant_size_p argument is
> a constant expression.  I couldn't come up with a C/C++ test case
> where this isn't true but apparently it can happen in Ada which I
> inadvertently didn't build.  I still haven't figured out what
> I have to do to build it on my Fedora 29 machine so I tested
> this change by hand (besides bootstrapping w/o Ada).

OK.

> The first set of instructions Google gives me don't seem to do
> it:
>
>    https://fedoraproject.org/wiki/Features/Ada_developer_tools
>
> and neither does dnf install gcc-gnat as explained on our Wiki:
>
>    https://gcc.gnu.org/wiki/GNAT
>
> If someone knows the magic chant I would be grateful (it might
> be helpful to also update the Wiki page -- the last change to
> it was made in 2012; I volunteer to do that).
>
> Martin