diff mbox series

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

Message ID 90ec17e4-779f-4847-873e-78567681cf6a@gmail.com
State New
Headers show
Series Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294) | expand

Commit Message

Martin Sebor Feb. 12, 2019, 1:13 a.m. UTC 
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

Comments

Rainer Orth Feb. 12, 2019, 11:43 p.m. UTC | #1 
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
Martin Sebor Feb. 13, 2019, 7:51 p.m. UTC | #2 
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
Eric Botcazou Feb. 15, 2019, 7:24 a.m. UTC | #3 
> 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...
Martin Sebor Feb. 15, 2019, 9:28 p.m. UTC | #4 
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
Martin Sebor Feb. 15, 2019, 10:13 p.m. UTC | #5 
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
Eric Botcazou Feb. 15, 2019, 10:46 p.m. UTC | #6 
> 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?
Martin Sebor Feb. 15, 2019, 10:55 p.m. UTC | #7 
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
Richard Biener Feb. 18, 2019, 10:26 a.m. UTC | #8 
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
diff mbox series

Patch

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