[C++] Reject variably modified types in operator new

[C++] Reject variably modified types in operator new

[Florian Weimer]

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

This patch flags operator new on variably modified types as an error.
If this is acceptable, this will simplify the implementation of the
C++11 requirement to throw std::bad_array_new_length instead of
allocating a memory region which is too short.

Okay for trunk?  Or should I guard this with -fpermissive?

2012-05-29  Florian Weimer  <fweimer@redhat.com>

	* init.c (build_new): Reject variably modified types.


2012-05-29  Florian Weimer  <fweimer@redhat.com>

	* g++.dg/init/new33.C: New.

-- 
Florian Weimer / Red Hat Product Security Team

[-- Attachment #2: operator-new-vla.diff --]
[-- Type: text/x-patch, Size: 1077 bytes --]

Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 187951)
+++ gcc/cp/init.c	(working copy)
@@ -2844,6 +2844,13 @@
   if (!complete_type_or_maybe_complain (type, NULL_TREE, complain))
     return error_mark_node;
 
+  if (variably_modified_type_p (type, NULL_TREE))
+    {
+      if (complain & tf_error)
+	error ("new cannot be applied to a variably modified type");
+      return error_mark_node;
+    }
+
   rval = build_new_1 (placement, type, nelts, init, use_global_new, complain);
   if (rval == error_mark_node)
     return error_mark_node;
Index: gcc/testsuite/g++.dg/init/new33.C
===================================================================
--- gcc/testsuite/g++.dg/init/new33.C	(revision 0)
+++ gcc/testsuite/g++.dg/init/new33.C	(revision 0)
@@ -0,0 +1,10 @@
+// { dg-do compile }
+
+int
+main (int argc, char **argv)
+{
+  typedef char A[argc];
+  new A; // { dg-error "variably modified" }
+  new A[0]; // { dg-error "variably modified" }
+  new A[5]; // { dg-error "variably modified" }
+}

Re: [C++] Reject variably modified types in operator new

[Gabriel Dos Reis]

On Tue, May 29, 2012 at 11:00 AM, Florian Weimer <fweimer@redhat.com> wrote:
> This patch flags operator new on variably modified types as an error.
> If this is acceptable, this will simplify the implementation of the
> C++11 requirement to throw std::bad_array_new_length instead of
> allocating a memory region which is too short.
>
> Okay for trunk?  Or should I guard this with -fpermissive?

I must say that ideally this should go in.  However, this having
been accepted in previous releases, I think people would like
one release of deprecation.  So my suggestion is:
   -- make it an error unless -fpermissive.
   -- if -fpermissive, make it unconditionally deprecated.
   -- schedule for entire removal in 4.9.

>
> 2012-05-29  Florian Weimer  <fweimer@redhat.com>
>
>        * init.c (build_new): Reject variably modified types.
>
>
> 2012-05-29  Florian Weimer  <fweimer@redhat.com>
>
>        * g++.dg/init/new33.C: New.
>
> --
> Florian Weimer / Red Hat Product Security Team

Re: [C++] Reject variably modified types in operator new

[Florian Weimer]

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

On 05/29/2012 06:41 PM, Gabriel Dos Reis wrote:
> On Tue, May 29, 2012 at 11:00 AM, Florian Weimer<fweimer@redhat.com>  wrote:
>> This patch flags operator new on variably modified types as an error.
>> If this is acceptable, this will simplify the implementation of the
>> C++11 requirement to throw std::bad_array_new_length instead of
>> allocating a memory region which is too short.
>>
>> Okay for trunk?  Or should I guard this with -fpermissive?
>
> I must say that ideally this should go in.  However, this having
> been accepted in previous releases, I think people would like
> one release of deprecation.  So my suggestion is:
>     -- make it an error unless -fpermissive.
>     -- if -fpermissive, make it unconditionally deprecated.
>     -- schedule for entire removal in 4.9.

On the other hand, it is such an obscure feature that it is rather 
unlikely that it has any users.  The usual C++ conformance fixes and 
libstdc++ header reorganizations cause much more pain, and no 
depreciation is required for them.

Perhaps we can get away here without depreciation, too?

I wrote a few tests for operator new[] (attached), and it does seem to 
work correctly as required.  I secretly hoped it was broken, but no luck 
there.

-- 
Florian Weimer / Red Hat Product Security Team

[-- Attachment #2: new34.C --]
[-- Type: text/x-c++src, Size: 2207 bytes --]

// Testcase for invocation of constructors/destructors in operator new[].
// { dg-do run }

#include <stdlib.h>

struct E {
  virtual ~E() { }
};

struct S {
  S();
  ~S();
};

static int count;
static int max;
static int throwAfter = -1;
static S *pS;

S::S()
{
  if (throwAfter >= 0 && count >= throwAfter)
    throw E();
  if (pS)
    {
      ++pS;
      if (this != pS)
	abort();
    }
  else
    pS = this;
  ++count;
  max = count;
}

S::~S()
{
  if (count > 1)
    {
      if (this != pS)
	abort();
      --pS;
    }
  else
    pS = 0;
  --count;
}

void __attribute__((noinline)) doit(int n)
{
  {
    S *s = new S[n];
    if (count != n)
      abort();
    if (pS != s + n - 1)
      abort();
    delete [] s;
    if (count != 0)
      abort();
  }
  typedef S A[n];
  {
    S *s = new A;
    if (count != n)
      abort();
    if (pS != s + n - 1)
      abort();
    delete [] s;
    if (count != 0)
      abort();
  }
  throwAfter = 2;
  max = 0;
  try
    {
      new S[n];
      abort();
    }
  catch (E)
    {
      if (max != 2)
	abort();
    }
  max = 0;
  try
    {
      new A;
      abort();
    }
  catch (E)
    {
      if (max != 2)
	abort();
    }
  throwAfter = -1;
}

int main()
{
  {
    S s;
    if (count != 1)
      abort();
    if (pS != &s)
      abort();
  }
  if (count != 0)
    abort();
  {
    S *s = new S;
    if (count != 1)
      abort();
    if (pS != s)
      abort();
    delete s;
    if (count != 0)
      abort();
  }
  {
    S *s = new S[1];
    if (count != 1)
      abort();
    if (pS != s)
      abort();
    delete [] s;
    if (count != 0)
      abort();
  }
  {
    S *s = new S[5];
    if (count != 5)
      abort();
    if (pS != s + 4)
      abort();
    delete [] s;
    if (count != 0)
      abort();
  }
  typedef S A[5];
  {
    S *s = new A;
    if (count != 5)
      abort();
    if (pS != s + 4)
      abort();
    delete [] s;
    if (count != 0)
      abort();
  }
  throwAfter = 2;
  max = 0;
  try
    {
      new S[5];
      abort();
    }
  catch (E)
    {
      if (max != 2)
	abort();
    }
  max = 0;
  try
    {
      new A;
      abort();
    }
  catch (E)
    {
      if (max != 2)
	abort();
    }
  throwAfter = -1;
  doit(5);
}

Re: [C++] Reject variably modified types in operator new

[Gabriel Dos Reis]

On Wed, May 30, 2012 at 3:47 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 05/29/2012 06:41 PM, Gabriel Dos Reis wrote:
>>
>> On Tue, May 29, 2012 at 11:00 AM, Florian Weimer<fweimer@redhat.com>
>>  wrote:
>>>
>>> This patch flags operator new on variably modified types as an error.
>>> If this is acceptable, this will simplify the implementation of the
>>> C++11 requirement to throw std::bad_array_new_length instead of
>>> allocating a memory region which is too short.
>>>
>>> Okay for trunk?  Or should I guard this with -fpermissive?
>>
>>
>> I must say that ideally this should go in.  However, this having
>> been accepted in previous releases, I think people would like
>> one release of deprecation.  So my suggestion is:
>>    -- make it an error unless -fpermissive.
>>    -- if -fpermissive, make it unconditionally deprecated.
>>    -- schedule for entire removal in 4.9.
>
>
> On the other hand, it is such an obscure feature that it is rather unlikely
> that it has any users.  The usual C++ conformance fixes and libstdc++ header
> reorganizations cause much more pain, and no depreciation is required for
> them.
>
> Perhaps we can get away here without depreciation, too?

That is a good point.  Jason, what do you think?

-- Gaby

Re: [C++] Reject variably modified types in operator new

[Mike Stump]

On May 30, 2012, at 9:15 AM, Gabriel Dos Reis wrote:
>> On the other hand, it is such an obscure feature that it is rather unlikely
>> that it has any users.  The usual C++ conformance fixes and libstdc++ header
>> reorganizations cause much more pain, and no depreciation is required for
>> them.
>> 
>> Perhaps we can get away here without depreciation, too?
> 
> That is a good point.  Jason, what do you think?

My take, -fpermissive is when 100s of projects make extensive use of the feature and you don't want to kill them all.  :-)  For really obscure corners, better to just fix bugs and refine semantics and not worry too much about it, life is too short.  If there are tons of user reports of, you guys broke this...  it can always be added in a .1 release later, if not caught before the .0 release.

Re: [C++] Reject variably modified types in operator new

[Jason Merrill]

On 05/29/2012 12:00 PM, Florian Weimer wrote:
> This patch flags operator new on variably modified types as an error.
> If this is acceptable, this will simplify the implementation of the
> C++11 requirement to throw std::bad_array_new_length instead of
> allocating a memory region which is too short.

Hmm.  I'm somewhat reluctant to outlaw a pattern that has an obvious 
meaning.  On the other hand, it is an extension that is mostly there for 
C compatibility, which would not be affected by this restriction.  So I 
guess the change is OK, but please add a comment about the motivation.

Jason

Re: [C++] Reject variably modified types in operator new

[Florian Weimer]

On 05/29/2012 06:00 PM, Florian Weimer wrote:
> This patch flags operator new on variably modified types as an error.
> If this is acceptable, this will simplify the implementation of the
> C++11 requirement to throw std::bad_array_new_length instead of
> allocating a memory region which is too short.

It turns out that the patch is not good enough.  Apparently, people write

   new (T[n])

instead of

   new T[n]

from time to time.  This is ill-formed (for variable n), and is 
currently accepted silently.  We even have test suite coverage for this.

I'll try to warn about this case and make the transformation to the 
proper operator new[] call.

-- 
Florian Weimer / Red Hat Product Security Team

Re: [C++] Reject variably modified types in operator new

[Florian Weimer]

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

On 06/01/2012 11:00 AM, Florian Weimer wrote:

> I'll try to warn about this case and make the transformation to the
> proper operator new[] call.

Here's the version.  I've added a warning for the ill-formed code.

The only remaining glitch is in g++.dg/cpp0x/regress/debug-debug7.C, 
specifically (b is not a constant):

   int (*x)[b] = new int[a][b];	// { dg-error "not usable" }

The new warning I've added fires on this line, too.  How can I check for 
the pending error and suppress the warning?

2012-06-01  Florian Weimer  <fweimer@redhat.com>

     * init.c (build_new): Reject variably modified types.

2012-06-01  Florian Weimer  <fweimer@redhat.com>

     * g++.dg/init/new35.C: New.
     * g++.dg/init/new36.C: New.
     * g++.dg/ext/vla5.C: New warning.
     * g++.dg/ext/vla8.C: New warning.

-- 
Florian Weimer / Red Hat Product Security Team

[-- Attachment #2: operator-new-vla.diff --]
[-- Type: text/x-patch, Size: 4837 bytes --]

Index: gcc/testsuite/g++.dg/ext/vla5.C
===================================================================
--- gcc/testsuite/g++.dg/ext/vla5.C	(revision 188104)
+++ gcc/testsuite/g++.dg/ext/vla5.C	(working copy)
@@ -6,5 +6,5 @@
 void
 test (int a)
 {
-  new (char[a]);
+  new (char[a]); // { dg-warning "variable array length" }
 }
Index: gcc/testsuite/g++.dg/ext/vla8.C
===================================================================
--- gcc/testsuite/g++.dg/ext/vla8.C	(revision 188104)
+++ gcc/testsuite/g++.dg/ext/vla8.C	(working copy)
@@ -8,7 +8,7 @@
 
   AvlTreeIter()
   {
-    new (void* [Num()]);
+    new (void* [Num()]); // { dg-warning "variable array length" }
   }
 };
 
Index: gcc/testsuite/g++.dg/init/new35.C
===================================================================
--- gcc/testsuite/g++.dg/init/new35.C	(revision 0)
+++ gcc/testsuite/g++.dg/init/new35.C	(revision 0)
@@ -0,0 +1,13 @@
+// { dg-do compile }
+// { dg-options "-Wno-vla" }
+
+int
+main (int argc, char **argv)
+{
+  typedef char A[argc];
+  new A;
+  new A[0]; // { dg-error "variably modified" }
+  new A[5]; // { dg-error "variably modified" }
+  new (A[0]); // { dg-error "variably modified" }
+  new (A[5]); // { dg-error "variably modified" }
+}
Index: gcc/testsuite/g++.dg/init/new36.C
===================================================================
--- gcc/testsuite/g++.dg/init/new36.C	(revision 0)
+++ gcc/testsuite/g++.dg/init/new36.C	(revision 0)
@@ -0,0 +1,153 @@
+// Testcase for invocation of constructors/destructors in operator new[].
+// { dg-do run }
+
+#include <stdlib.h>
+
+struct E {
+  virtual ~E() { }
+};
+
+struct S {
+  S();
+  ~S();
+};
+
+static int count;
+static int max;
+static int throwAfter = -1;
+static S *pS;
+
+S::S()
+{
+  if (throwAfter >= 0 && count >= throwAfter)
+    throw E();
+  if (pS)
+    {
+      ++pS;
+      if (this != pS)
+	abort();
+    }
+  else
+    pS = this;
+  ++count;
+  max = count;
+}
+
+S::~S()
+{
+  if (count > 1)
+    {
+      if (this != pS)
+	abort();
+      --pS;
+    }
+  else
+    pS = 0;
+  --count;
+}
+
+void __attribute__((noinline)) doit(int n)
+{
+  {
+    S *s = new S[n];
+    if (count != n)
+      abort();
+    if (pS != s + n - 1)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  throwAfter = 2;
+  max = 0;
+  try
+    {
+      new S[n];
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  throwAfter = -1;
+}
+
+int main()
+{
+  {
+    S s;
+    if (count != 1)
+      abort();
+    if (pS != &s)
+      abort();
+  }
+  if (count != 0)
+    abort();
+  {
+    S *s = new S;
+    if (count != 1)
+      abort();
+    if (pS != s)
+      abort();
+    delete s;
+    if (count != 0)
+      abort();
+  }
+  {
+    S *s = new S[1];
+    if (count != 1)
+      abort();
+    if (pS != s)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  {
+    S *s = new S[5];
+    if (count != 5)
+      abort();
+    if (pS != s + 4)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  typedef S A[5];
+  {
+    S *s = new A;
+    if (count != 5)
+      abort();
+    if (pS != s + 4)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  throwAfter = 2;
+  max = 0;
+  try
+    {
+      new S[5];
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  max = 0;
+  try
+    {
+      new A;
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  throwAfter = -1;
+  doit(5);
+}
Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 188104)
+++ gcc/cp/init.c	(working copy)
@@ -2805,6 +2805,34 @@
       make_args_non_dependent (*init);
     }
 
+  /* If the type is variably modified (a GNU extension for C
+     compatibility), we could end up with a variable-times-variable
+     multiplication at run time, complicating overflow checking.  */
+  if (variably_modified_type_p (type, NULL_TREE))
+    {
+      /* Try to transform new (T[n]) to new T[n], and accept the
+	 result if T is not variably modified. */
+      bool good = false;
+      if (nelts == NULL_TREE && TREE_CODE (type) == ARRAY_TYPE)
+	{
+	  tree inner_type = TREE_TYPE (type);
+	  if (!variably_modified_type_p (inner_type, NULL_TREE))
+	    {
+	      pedwarn (input_location, OPT_Wvla,
+		       "ISO C++ forbids variable array length in parenthesized type-id");
+	      nelts = array_type_nelts_top (type);
+	      type = inner_type;
+	      good = true;
+	    }
+	}
+      if (!good)
+	{
+	  if (complain & tf_error)
+	    error ("new cannot be applied to a variably modified type");
+	  return error_mark_node;
+	}
+    }
+
   if (nelts)
     {
       if (!build_expr_type_conversion (WANT_INT | WANT_ENUM, nelts, false))

Re: [C++] Reject variably modified types in operator new

[Alexander Monakov]

As someone who wrote code that does such allocations, I'm surprised that this
is a GNU extension, and is rejected even by the C++11 standard.  How is one
supposed to perform allocations of two-dimensional arrays when inner dimension
is given as function argument?

I'm relatively inexperienced, but let me disagree about the assessment of this
feature as "obscure".  I would expect that some existing programs that
perform, say, CFD calculations on two- or three-dimensional regular grids,
or, better yet, do image processing, would allocate grid data in exactly that
manner, and would be broken by this change:

void foo(int gridSizeH, int gridSizeV)
{
  typedef double gridRow[gridSizeH];
  
  gridRow *grid = new gridRow[gridSizeV];

  ...
}

Therefore I suggest a deprecation period with -fpermissive.

Thanks.

Alexander

Re: [C++] Reject variably modified types in operator new

[Jason Merrill]

On 06/01/2012 08:09 AM, Florian Weimer wrote:
> The only remaining glitch is in g++.dg/cpp0x/regress/debug-debug7.C,
> specifically (b is not a constant):
>
>    int (*x)[b] = new int[a][b];    // { dg-error "not usable" }
>
> The new warning I've added fires on this line, too.  How can I check for
> the pending error and suppress the warning?

Warning only if the array is a typedef might do the trick.

Jason

Re: [C++] Reject variably modified types in operator new

[Florian Weimer]

On 06/01/2012 05:37 PM, Jason Merrill wrote:
> On 06/01/2012 08:09 AM, Florian Weimer wrote:
>> The only remaining glitch is in g++.dg/cpp0x/regress/debug-debug7.C,
>> specifically (b is not a constant):
>>
>> int (*x)[b] = new int[a][b]; // { dg-error "not usable" }
>>
>> The new warning I've added fires on this line, too. How can I check for
>> the pending error and suppress the warning?
>
> Warning only if the array is a typedef might do the trick.

I'm afraid not.  We really want to warn for

   new (char[n])

as well.

I'm puzzled why build_new is even invoked after detecting that there is 
a non-constant expression.


-- 
Florian Weimer / Red Hat Product Security Team

Re: [C++] Reject variably modified types in operator new

[Jason Merrill]

On 06/01/2012 11:40 AM, Florian Weimer wrote:
> I'm puzzled why build_new is even invoked after detecting that there is
> a non-constant expression.

I'd accept a patch to change that.

Jason

Re: [C++] Reject variably modified types in operator new

[Florian Weimer]

On 06/01/2012 06:19 PM, Jason Merrill wrote:
> On 06/01/2012 11:40 AM, Florian Weimer wrote:
>> I'm puzzled why build_new is even invoked after detecting that there is
>> a non-constant expression.
>
> I'd accept a patch to change that.

I don't really now what I'm doing here.  But I noticed that in 
cp_parser_constant_expression, a failure in 
require_potential_rvalue_constant_expression is not reported to the caller.

This changes error reporting, and a few test cases need adjustment.  In 
some, reporting is better, in others, it's slightly worse.  I need to 
make a second pass over the changes to make sure that they are alright.

Does this change make any sense at all?

Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 188104)
+++ gcc/cp/parser.c	(working copy)
@@ -7701,8 +7701,9 @@ cp_parser_constant_expression (cp_parser* parser,
  	 separately in e.g. cp_parser_template_argument.  */
        bool is_const = potential_rvalue_constant_expression (expression);
        parser->non_integral_constant_expression_p = !is_const;
-      if (!is_const && !allow_non_constant_p)
-	require_potential_rvalue_constant_expression (expression);
+      if (!is_const && !allow_non_constant_p
+	  && !require_potential_rvalue_constant_expression (expression))
+	expression = error_mark_node;
      }
    if (allow_non_constant_p)
      *non_constant_p = parser->non_integral_constant_expression_p;


-- 
Florian Weimer / Red Hat Product Security Team

Re: [C++] Reject variably modified types in operator new

[Jason Merrill]

On 06/01/2012 03:55 PM, Florian Weimer wrote:
> On 06/01/2012 06:19 PM, Jason Merrill wrote:
>> On 06/01/2012 11:40 AM, Florian Weimer wrote:
>>> I'm puzzled why build_new is even invoked after detecting that there is
>>> a non-constant expression.
>>
>> I'd accept a patch to change that.
>
> I don't really now what I'm doing here. But I noticed that in
> cp_parser_constant_expression, a failure in
> require_potential_rvalue_constant_expression is not reported to the caller.
>
> This changes error reporting, and a few test cases need adjustment. In
> some, reporting is better, in others, it's slightly worse. I need to
> make a second pass over the changes to make sure that they are alright.

Oh, right.  I think I tried this change once and then decided that 
things were better without it, but I don't remember why.  The change 
looks reasonable by itself; what changes are you seeing in error reporting.

Jason

Re: [C++] Reject variably modified types in operator new

[Florian Weimer]

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

On 06/01/2012 02:09 PM, Florian Weimer wrote:
> On 06/01/2012 11:00 AM, Florian Weimer wrote:
>
>> I'll try to warn about this case and make the transformation to the
>> proper operator new[] call.
>
> Here's the version. I've added a warning for the ill-formed code.
>
> The only remaining glitch is in g++.dg/cpp0x/regress/debug-debug7.C,
> specifically (b is not a constant):
>
> int (*x)[b] = new int[a][b]; // { dg-error "not usable" }
>
> The new warning I've added fires on this line, too. How can I check for
> the pending error and suppress the warning?

As discussed in the other thread with Jason and Gaby, I have changed 
cp/parser.c to accept non-constant values and produce the error in 
build_new_1.  This is arguably the right place because this check must 
be performed after template instantiation.  (I will submit a follow-up 
patch for the remaining type check in the parser once this is accepted.)

testsuite/g++.dg/cpp0x/regress/debug-debug7.C remains problematic.  The 
diagnostic at parse time was definitely better, but I see no way to keep 
that.  I have reworded the diagnostics because the code no longer has 
access to the original syntax, so it cannot tell whether the operator 
new call involved a type-id in parens.

Bootstrapped and tested on x86_64-linux-gnu, with no new regressions.

-- 
Florian Weimer / Red Hat Product Security Team

[-- Attachment #2: operator-new-vla.diff --]
[-- Type: text/x-patch, Size: 9364 bytes --]

2012-06-11  Florian Weimer  <fweimer@redhat.com>

	* init.c (build_new_1): Warn about (T[N]) for variable N, and
	reject T[M][N].

	* parser.c (cp_parser_direct_new_declarator): Accept non-constant
	expressions.  Handled now in build_new_1.

2012-06-11  Florian Weimer  <fweimer@redhat.com>

	* g++.dg/init/new35.C: New.
	* g++.dg/init/new36.C: New.
	* g++.dg/ext/vla5.C: New warning.
	* g++.dg/ext/vla8.C: New warning.
	* g++.dg/cpp0x/regress/debug-debug7.C: Update diagnostics.

Index: cp/init.c
===================================================================
--- cp/init.c	(revision 188384)
+++ cp/init.c	(working copy)
@@ -2175,6 +2175,7 @@
   tree pointer_type;
   tree non_const_pointer_type;
   tree outer_nelts = NULL_TREE;
+  bool outer_nelts_from_type = false;
   tree alloc_call, alloc_expr;
   /* The address returned by the call to "operator new".  This node is
      a VAR_DECL and is therefore reusable.  */
@@ -2209,10 +2210,14 @@
     }
   else if (TREE_CODE (type) == ARRAY_TYPE)
     {
+      /* Transforms new (T[N]) to new T[N].  The former is a GNU
+	 extension.  (This also covers new T where T is a VLA
+	 typedef.)  */
       array_p = true;
       nelts = array_type_nelts_top (type);
       outer_nelts = nelts;
       type = TREE_TYPE (type);
+      outer_nelts_from_type = true;
     }
 
   /* If our base type is an array, then make sure we know how many elements
@@ -2220,11 +2225,40 @@
   for (elt_type = type;
        TREE_CODE (elt_type) == ARRAY_TYPE;
        elt_type = TREE_TYPE (elt_type))
-    nelts = cp_build_binary_op (input_location,
-				MULT_EXPR, nelts,
-				array_type_nelts_top (elt_type),
-				complain);
+    {
+      tree inner_nelts = array_type_nelts_top (elt_type);
+      tree inner_nelts_cst = maybe_constant_value (inner_nelts);
+      if (!TREE_CONSTANT (inner_nelts_cst))
+	{
+	  if (complain & tf_error)
+	    error_at (EXPR_LOC_OR_HERE (inner_nelts),
+		      "array size in operator new must be constant");
+	  nelts = error_mark_node;
+	}
+      if (nelts != error_mark_node)
+	nelts = cp_build_binary_op (input_location,
+				    MULT_EXPR, nelts,
+				    inner_nelts_cst,
+				    complain);
+    }
 
+  if (variably_modified_type_p (elt_type, NULL_TREE) && (complain & tf_error))
+    {
+      error ("variably modified type not allowed in operator new");
+      return error_mark_node;
+    }
+
+  if (nelts == error_mark_node)
+    return error_mark_node;
+
+  /* Warn if we performed the (T[N]) to T[N] transformation and N is
+     variable.  */
+  if (outer_nelts_from_type
+      && !TREE_CONSTANT (maybe_constant_value (outer_nelts))
+      && (complain & tf_warning_or_error))
+    pedwarn(EXPR_LOC_OR_HERE (outer_nelts), OPT_Wvla,
+	    "ISO C++ does not support variable-length array types");
+
   if (TREE_CODE (elt_type) == VOID_TYPE)
     {
       if (complain & tf_error)
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 188384)
+++ cp/parser.c	(working copy)
@@ -6845,41 +6845,34 @@
   while (true)
     {
       tree expression;
+      cp_token *token;
 
       /* Look for the opening `['.  */
       cp_parser_require (parser, CPP_OPEN_SQUARE, RT_OPEN_SQUARE);
-      /* The first expression is not required to be constant.  */
-      if (!declarator)
+
+      token = cp_lexer_peek_token (parser->lexer);
+      expression = cp_parser_expression (parser, /*cast_p=*/false, NULL);
+      /* The standard requires that the expression have integral
+	 type.  DR 74 adds enumeration types.  We believe that the
+	 real intent is that these expressions be handled like the
+	 expression in a `switch' condition, which also allows
+	 classes with a single conversion to integral or
+	 enumeration type.  */
+      if (!processing_template_decl)
 	{
-	  cp_token *token = cp_lexer_peek_token (parser->lexer);
-	  expression = cp_parser_expression (parser, /*cast_p=*/false, NULL);
-	  /* The standard requires that the expression have integral
-	     type.  DR 74 adds enumeration types.  We believe that the
-	     real intent is that these expressions be handled like the
-	     expression in a `switch' condition, which also allows
-	     classes with a single conversion to integral or
-	     enumeration type.  */
-	  if (!processing_template_decl)
+	  expression
+	    = build_expr_type_conversion (WANT_INT | WANT_ENUM,
+					  expression,
+					  /*complain=*/true);
+	  if (!expression)
 	    {
-	      expression
-		= build_expr_type_conversion (WANT_INT | WANT_ENUM,
-					      expression,
-					      /*complain=*/true);
-	      if (!expression)
-		{
-		  error_at (token->location,
-			    "expression in new-declarator must have integral "
-			    "or enumeration type");
-		  expression = error_mark_node;
-		}
+	      error_at (token->location,
+			"expression in new-declarator must have integral "
+			"or enumeration type");
+	      expression = error_mark_node;
 	    }
 	}
-      /* But all the other expressions must be.  */
-      else
-	expression
-	  = cp_parser_constant_expression (parser,
-					   /*allow_non_constant=*/false,
-					   NULL);
+
       /* Look for the closing `]'.  */
       cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
 
Index: testsuite/g++.dg/cpp0x/regress/debug-debug7.C
===================================================================
--- testsuite/g++.dg/cpp0x/regress/debug-debug7.C	(revision 188384)
+++ testsuite/g++.dg/cpp0x/regress/debug-debug7.C	(working copy)
@@ -7,8 +7,8 @@
 main() {
 
   int a = 4;
-  int b = 5;			// { dg-message "not const" }
-  int (*x)[b] = new int[a][b];	// { dg-error "not usable" }
+  int b = 5;
+  int (*x)[b] = new int[a][b]; // { dg-error "array size.*must be constant" }
 
   x[2][1] = 7;
 
Index: testsuite/g++.dg/ext/vla5.C
===================================================================
--- testsuite/g++.dg/ext/vla5.C	(revision 188384)
+++ testsuite/g++.dg/ext/vla5.C	(working copy)
@@ -6,5 +6,5 @@
 void
 test (int a)
 {
-  new (char[a]);
+  new (char[a]); // { dg-warning "variable-length array" }
 }
Index: testsuite/g++.dg/ext/vla8.C
===================================================================
--- testsuite/g++.dg/ext/vla8.C	(revision 188384)
+++ testsuite/g++.dg/ext/vla8.C	(working copy)
@@ -8,8 +8,8 @@
 
   AvlTreeIter()
   {
-    new (void* [Num()]);
+    new (void* [Num()]); // { dg-warning "variable-length array" }
   }
 };
 
-AvlTreeIter<int> a;
+AvlTreeIter<int> a; // { dg-message "from here" }
Index: testsuite/g++.dg/init/new35.C
===================================================================
--- testsuite/g++.dg/init/new35.C	(revision 0)
+++ testsuite/g++.dg/init/new35.C	(revision 0)
@@ -0,0 +1,13 @@
+// { dg-do compile }
+// { dg-options "" }
+
+int
+main (int argc, char **argv)
+{
+  typedef char A[argc];
+  new A; // { dg-warning "variable-length array types" }
+  new A[0]; // { dg-error "must be constant" }
+  new A[5]; // { dg-error "must be constant" }
+  new (A[0]); // { dg-error "must be constant" }
+  new (A[5]); // { dg-error "must be constant" }
+}
Index: testsuite/g++.dg/init/new36.C
===================================================================
--- testsuite/g++.dg/init/new36.C	(revision 0)
+++ testsuite/g++.dg/init/new36.C	(revision 0)
@@ -0,0 +1,153 @@
+// Testcase for invocation of constructors/destructors in operator new[].
+// { dg-do run }
+
+#include <stdlib.h>
+
+struct E {
+  virtual ~E() { }
+};
+
+struct S {
+  S();
+  ~S();
+};
+
+static int count;
+static int max;
+static int throwAfter = -1;
+static S *pS;
+
+S::S()
+{
+  if (throwAfter >= 0 && count >= throwAfter)
+    throw E();
+  if (pS)
+    {
+      ++pS;
+      if (this != pS)
+	abort();
+    }
+  else
+    pS = this;
+  ++count;
+  max = count;
+}
+
+S::~S()
+{
+  if (count > 1)
+    {
+      if (this != pS)
+	abort();
+      --pS;
+    }
+  else
+    pS = 0;
+  --count;
+}
+
+void __attribute__((noinline)) doit(int n)
+{
+  {
+    S *s = new S[n];
+    if (count != n)
+      abort();
+    if (pS != s + n - 1)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  throwAfter = 2;
+  max = 0;
+  try
+    {
+      new S[n];
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  throwAfter = -1;
+}
+
+int main()
+{
+  {
+    S s;
+    if (count != 1)
+      abort();
+    if (pS != &s)
+      abort();
+  }
+  if (count != 0)
+    abort();
+  {
+    S *s = new S;
+    if (count != 1)
+      abort();
+    if (pS != s)
+      abort();
+    delete s;
+    if (count != 0)
+      abort();
+  }
+  {
+    S *s = new S[1];
+    if (count != 1)
+      abort();
+    if (pS != s)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  {
+    S *s = new S[5];
+    if (count != 5)
+      abort();
+    if (pS != s + 4)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  typedef S A[5];
+  {
+    S *s = new A;
+    if (count != 5)
+      abort();
+    if (pS != s + 4)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  throwAfter = 2;
+  max = 0;
+  try
+    {
+      new S[5];
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  max = 0;
+  try
+    {
+      new A;
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  throwAfter = -1;
+  doit(5);
+}

Re: [C++] Reject variably modified types in operator new

[Jason Merrill]

On 06/11/2012 12:11 PM, Florian Weimer wrote:
> +      tree inner_nelts_cst = maybe_constant_value (inner_nelts);
> +      if (!TREE_CONSTANT (inner_nelts_cst))
> +	{
> +	  if (complain & tf_error)
> +	    error_at (EXPR_LOC_OR_HERE (inner_nelts),
> +		      "array size in operator new must be constant");

Please use cxx_constant_value to give a more specific error about what 
is non-constant.

> +  /* Warn if we performed the (T[N]) to T[N] transformation and N is
> +     variable.  */
> +  if (outer_nelts_from_type
> +      && !TREE_CONSTANT (maybe_constant_value (outer_nelts))
> +      && (complain & tf_warning_or_error))
> +    pedwarn(EXPR_LOC_OR_HERE (outer_nelts), OPT_Wvla,
> +	    "ISO C++ does not support variable-length array types");

Here, if we aren't complaining we should return error_mark_node; we 
always need to act pedantic in SFINAE context.

Jason

Re: [C++] Reject variably modified types in operator new

[Florian Weimer]

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

On 06/25/2012 05:25 AM, Jason Merrill wrote:
> On 06/11/2012 12:11 PM, Florian Weimer wrote:
>> + tree inner_nelts_cst = maybe_constant_value (inner_nelts);
>> + if (!TREE_CONSTANT (inner_nelts_cst))
>> + {
>> + if (complain & tf_error)
>> + error_at (EXPR_LOC_OR_HERE (inner_nelts),
>> + "array size in operator new must be constant");
>
> Please use cxx_constant_value to give a more specific error about what
> is non-constant.

Thanks for your review.

I tried this, but for this example program,

/* 1 */ void f(int n, int m)
/* 2 */ {
/* 3 */   typedef char T[n];
/* 4 */   new T[m];
/* 5 */ }

GCC reports these errors:

/tmp/t.C: In function ‘void f(int, int)’:
/tmp/t.C:4:18: error: array size in operator new must be constant
    /* 4 */   new T[m];
                     ^
/tmp/t.C:4:18: error: ‘n’ is not a constant expression

The message should point to the typedef, but instead, it references the 
line with operator new (which doesn't even contain the variable n).

For the non-VLA typedef case, it is an improvement.  But I would like to 
leave in both errors, as in the attached patch.

If you have suggestions how to improve cxx_constant_value error 
reporting, I can look into that in a separate patch.

>> + /* Warn if we performed the (T[N]) to T[N] transformation and N is
>> + variable. */
>> + if (outer_nelts_from_type
>> + && !TREE_CONSTANT (maybe_constant_value (outer_nelts))
>> + && (complain & tf_warning_or_error))
>> + pedwarn(EXPR_LOC_OR_HERE (outer_nelts), OPT_Wvla,
>> + "ISO C++ does not support variable-length array types");
>
> Here, if we aren't complaining we should return error_mark_node; we
> always need to act pedantic in SFINAE context.

The new version should do that.

I've added a new test case about error reporting in templates (where we 
previously accepted ill-formed code such as new long[n][T::n] with n, 
T::n both variables).

Bootstrapped and tested on x86_64-unknown-linux-gnu, with no new 
regressions.  OK for trunk?

-- 
Florian Weimer / Red Hat Product Security Team



[-- Attachment #2: operator-new-vla.diff --]
[-- Type: text/x-patch, Size: 11036 bytes --]

2012-06-25  Florian Weimer  <fweimer@redhat.com>

	* init.c (build_new_1): Warn about (T[N]) for variable N, and
	reject T[M][N].

	* parser.c (cp_parser_direct_new_declarator): Accept non-constant
	expressions.  Handled now in build_new_1.

2012-06-25  Florian Weimer  <fweimer@redhat.com>

	* g++.dg/init/new35.C: New.
	* g++.dg/init/new36.C: New.
	* g++.dg/init/new37.C: New.
	* g++.dg/ext/vla5.C: New warning.
	* g++.dg/ext/vla8.C: New warning.
	* g++.dg/cpp0x/regress/debug-debug7.C: Update diagnostics.

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 419c13f..5a81643 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -2175,6 +2175,7 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts,
   tree pointer_type;
   tree non_const_pointer_type;
   tree outer_nelts = NULL_TREE;
+  bool outer_nelts_from_type = false;
   tree alloc_call, alloc_expr;
   /* The address returned by the call to "operator new".  This node is
      a VAR_DECL and is therefore reusable.  */
@@ -2209,10 +2210,14 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts,
     }
   else if (TREE_CODE (type) == ARRAY_TYPE)
     {
+      /* Transforms new (T[N]) to new T[N].  The former is a GNU
+	 extension for variable N.  (This also covers new T where T is
+	 a VLA typedef.)  */
       array_p = true;
       nelts = array_type_nelts_top (type);
       outer_nelts = nelts;
       type = TREE_TYPE (type);
+      outer_nelts_from_type = true;
     }
 
   /* If our base type is an array, then make sure we know how many elements
@@ -2220,10 +2225,46 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts,
   for (elt_type = type;
        TREE_CODE (elt_type) == ARRAY_TYPE;
        elt_type = TREE_TYPE (elt_type))
-    nelts = cp_build_binary_op (input_location,
-				MULT_EXPR, nelts,
-				array_type_nelts_top (elt_type),
-				complain);
+    {
+      tree inner_nelts = array_type_nelts_top (elt_type);
+      tree inner_nelts_cst = maybe_constant_value (inner_nelts);
+      if (!TREE_CONSTANT (inner_nelts_cst))
+	{
+	  if (complain & tf_error)
+	    {
+	      error_at (EXPR_LOC_OR_HERE (inner_nelts),
+			"array size in operator new must be constant");
+	      cxx_constant_value(inner_nelts);
+	    }
+	  nelts = error_mark_node;
+	}
+      if (nelts != error_mark_node)
+	nelts = cp_build_binary_op (input_location,
+				    MULT_EXPR, nelts,
+				    inner_nelts_cst,
+				    complain);
+    }
+
+  if (variably_modified_type_p (elt_type, NULL_TREE) && (complain & tf_error))
+    {
+      error ("variably modified type not allowed in operator new");
+      return error_mark_node;
+    }
+
+  if (nelts == error_mark_node)
+    return error_mark_node;
+
+  /* Warn if we performed the (T[N]) to T[N] transformation and N is
+     variable.  */
+  if (outer_nelts_from_type
+      && !TREE_CONSTANT (maybe_constant_value (outer_nelts)))
+    {
+      if (complain & tf_warning_or_error)
+	pedwarn(EXPR_LOC_OR_HERE (outer_nelts), OPT_Wvla,
+		"ISO C++ does not support variable-length array types");
+      else
+	return error_mark_node;
+    }
 
   if (TREE_CODE (elt_type) == VOID_TYPE)
     {
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6bc6877c..46f1401 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6883,41 +6883,34 @@ cp_parser_direct_new_declarator (cp_parser* parser)
   while (true)
     {
       tree expression;
+      cp_token *token;
 
       /* Look for the opening `['.  */
       cp_parser_require (parser, CPP_OPEN_SQUARE, RT_OPEN_SQUARE);
-      /* The first expression is not required to be constant.  */
-      if (!declarator)
+
+      token = cp_lexer_peek_token (parser->lexer);
+      expression = cp_parser_expression (parser, /*cast_p=*/false, NULL);
+      /* The standard requires that the expression have integral
+	 type.  DR 74 adds enumeration types.  We believe that the
+	 real intent is that these expressions be handled like the
+	 expression in a `switch' condition, which also allows
+	 classes with a single conversion to integral or
+	 enumeration type.  */
+      if (!processing_template_decl)
 	{
-	  cp_token *token = cp_lexer_peek_token (parser->lexer);
-	  expression = cp_parser_expression (parser, /*cast_p=*/false, NULL);
-	  /* The standard requires that the expression have integral
-	     type.  DR 74 adds enumeration types.  We believe that the
-	     real intent is that these expressions be handled like the
-	     expression in a `switch' condition, which also allows
-	     classes with a single conversion to integral or
-	     enumeration type.  */
-	  if (!processing_template_decl)
+	  expression
+	    = build_expr_type_conversion (WANT_INT | WANT_ENUM,
+					  expression,
+					  /*complain=*/true);
+	  if (!expression)
 	    {
-	      expression
-		= build_expr_type_conversion (WANT_INT | WANT_ENUM,
-					      expression,
-					      /*complain=*/true);
-	      if (!expression)
-		{
-		  error_at (token->location,
-			    "expression in new-declarator must have integral "
-			    "or enumeration type");
-		  expression = error_mark_node;
-		}
+	      error_at (token->location,
+			"expression in new-declarator must have integral "
+			"or enumeration type");
+	      expression = error_mark_node;
 	    }
 	}
-      /* But all the other expressions must be.  */
-      else
-	expression
-	  = cp_parser_constant_expression (parser,
-					   /*allow_non_constant=*/false,
-					   NULL);
+
       /* Look for the closing `]'.  */
       cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/regress/debug-debug7.C b/gcc/testsuite/g++.dg/cpp0x/regress/debug-debug7.C
index ea8f1eb..d3f14f4 100644
--- a/gcc/testsuite/g++.dg/cpp0x/regress/debug-debug7.C
+++ b/gcc/testsuite/g++.dg/cpp0x/regress/debug-debug7.C
@@ -7,8 +7,8 @@ int
 main() {
 
   int a = 4;
-  int b = 5;			// { dg-message "not const" }
-  int (*x)[b] = new int[a][b];	// { dg-error "not usable" }
+  int b = 5;
+  int (*x)[b] = new int[a][b]; // { dg-error "array size.*must be constant|usable in a constant" }
 
   x[2][1] = 7;
 
diff --git a/gcc/testsuite/g++.dg/ext/vla5.C b/gcc/testsuite/g++.dg/ext/vla5.C
index 021d484..2457e34 100644
--- a/gcc/testsuite/g++.dg/ext/vla5.C
+++ b/gcc/testsuite/g++.dg/ext/vla5.C
@@ -6,5 +6,5 @@
 void
 test (int a)
 {
-  new (char[a]);
+  new (char[a]); // { dg-warning "variable-length array" }
 }
diff --git a/gcc/testsuite/g++.dg/ext/vla8.C b/gcc/testsuite/g++.dg/ext/vla8.C
index 7b7428d..1c6000f 100644
--- a/gcc/testsuite/g++.dg/ext/vla8.C
+++ b/gcc/testsuite/g++.dg/ext/vla8.C
@@ -8,8 +8,8 @@ struct AvlTreeIter
 
   AvlTreeIter()
   {
-    new (void* [Num()]);
+    new (void* [Num()]); // { dg-warning "variable-length array" }
   }
 };
 
-AvlTreeIter<int> a;
+AvlTreeIter<int> a; // { dg-message "from here" }
diff --git a/gcc/testsuite/g++.dg/init/new35.C b/gcc/testsuite/g++.dg/init/new35.C
new file mode 100644
index 0000000..c5f79aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/new35.C
@@ -0,0 +1,13 @@
+// { dg-do compile }
+// { dg-options "" }
+
+int
+main (int argc, char **argv)
+{
+  typedef char A[argc];
+  new A; // { dg-warning "variable-length array types|not a constant" }
+  new A[0]; // { dg-error "must be constant|not a constant" }
+  new A[5]; // { dg-error "must be constant|not a constant" }
+  new (A[0]); // { dg-error "must be constant|not a constant" }
+  new (A[5]); // { dg-error "must be constant|not a constant" }
+}
diff --git a/gcc/testsuite/g++.dg/init/new36.C b/gcc/testsuite/g++.dg/init/new36.C
new file mode 100644
index 0000000..c9b7af2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/new36.C
@@ -0,0 +1,153 @@
+// Testcase for invocation of constructors/destructors in operator new[].
+// { dg-do run }
+
+#include <stdlib.h>
+
+struct E {
+  virtual ~E() { }
+};
+
+struct S {
+  S();
+  ~S();
+};
+
+static int count;
+static int max;
+static int throwAfter = -1;
+static S *pS;
+
+S::S()
+{
+  if (throwAfter >= 0 && count >= throwAfter)
+    throw E();
+  if (pS)
+    {
+      ++pS;
+      if (this != pS)
+	abort();
+    }
+  else
+    pS = this;
+  ++count;
+  max = count;
+}
+
+S::~S()
+{
+  if (count > 1)
+    {
+      if (this != pS)
+	abort();
+      --pS;
+    }
+  else
+    pS = 0;
+  --count;
+}
+
+void __attribute__((noinline)) doit(int n)
+{
+  {
+    S *s = new S[n];
+    if (count != n)
+      abort();
+    if (pS != s + n - 1)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  throwAfter = 2;
+  max = 0;
+  try
+    {
+      new S[n];
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  throwAfter = -1;
+}
+
+int main()
+{
+  {
+    S s;
+    if (count != 1)
+      abort();
+    if (pS != &s)
+      abort();
+  }
+  if (count != 0)
+    abort();
+  {
+    S *s = new S;
+    if (count != 1)
+      abort();
+    if (pS != s)
+      abort();
+    delete s;
+    if (count != 0)
+      abort();
+  }
+  {
+    S *s = new S[1];
+    if (count != 1)
+      abort();
+    if (pS != s)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  {
+    S *s = new S[5];
+    if (count != 5)
+      abort();
+    if (pS != s + 4)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  typedef S A[5];
+  {
+    S *s = new A;
+    if (count != 5)
+      abort();
+    if (pS != s + 4)
+      abort();
+    delete [] s;
+    if (count != 0)
+      abort();
+  }
+  throwAfter = 2;
+  max = 0;
+  try
+    {
+      new S[5];
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  max = 0;
+  try
+    {
+      new A;
+      abort();
+    }
+  catch (E)
+    {
+      if (max != 2)
+	abort();
+    }
+  throwAfter = -1;
+  doit(5);
+}
diff --git a/gcc/testsuite/g++.dg/init/new37.C b/gcc/testsuite/g++.dg/init/new37.C
new file mode 100644
index 0000000..82ca18b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/new37.C
@@ -0,0 +1,63 @@
+// { dg-do compile }
+
+void
+nonconst(int n)
+{
+  new (long[n][n]); // { dg-error "variable length|array size|not a constant" }
+  new long[n][n]; // { dg-error "variable length|array size|not a constant" }
+}
+
+template <typename T>
+void *
+callnew(int n)
+{
+  return new long[n][T::n];
+}
+
+template <typename T>
+void *
+callnew_fail_1(int n)
+{
+  return new long[n][T::n]; // { dg-error "variable length|array size|usable in a constant" }
+}
+
+template <typename T>
+void *
+callnew_fail_2()
+{
+  return new long[T::n]; // { dg-error "size in array new" }
+}
+
+template <typename T>
+void *
+callnew_fail_3()
+{
+  return new T[2][T::n]; // { dg-error "size of array has non-integral type" }
+}
+
+struct T1 {
+  static int n;
+};
+
+struct T2 {
+  static const double n = 2; // { dg-error "non-integral type" }
+};
+
+struct T3 {
+  static const int n = 2;
+};
+
+struct T4 {
+  enum { n = 3 };
+};
+
+void
+test_callnew(int n)
+{
+  new long[0.2]; // { dg-error "integral or enumeration type" }
+  callnew_fail_1<T1>(n);
+  callnew_fail_2<T2>();
+  callnew_fail_3<T2>();
+  callnew<T3>(n);
+  callnew<T4>(n);
+}

Re: [C++] Reject variably modified types in operator new

[Jason Merrill]

On 06/25/2012 08:17 AM, Florian Weimer wrote:
> The message should point to the typedef, but instead, it references the
> line with operator new (which doesn't even contain the variable n).

Yep, this is another example of how we don't track/use expression 
locations well enough.

> For the non-VLA typedef case, it is an improvement. But I would like to
> leave in both errors, as in the attached patch.

OK.

> If you have suggestions how to improve cxx_constant_value error
> reporting, I can look into that in a separate patch.

One way would be to pass around a location_t loc in the cxx_eval_* 
functions, starting with input_location, but then at the top of 
cxx_eval_constant_expression have

if (EXPR_HAS_LOCATION (t))
   loc = EXPR_LOCATION (t);

and use error_at (loc in all error messages.

And perhaps also change array_type_nelts_top to give the PLUS_EXPR (if 
any) the location of the array typedef (if any).

> Bootstrapped and tested on x86_64-unknown-linux-gnu, with no new
> regressions. OK for trunk?

Yes.

Jason