diff mbox

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

Message ID 4FC8B116.6000607@redhat.com
State New
Headers show

Commit Message

Florian Weimer June 1, 2012, 12:09 p.m. UTC 
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.

Comments

Jason Merrill June 1, 2012, 3:37 p.m. UTC | #1 
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
Florian Weimer June 1, 2012, 3:40 p.m. UTC | #2 
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.
Jason Merrill June 1, 2012, 4:19 p.m. UTC | #3 
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
diff mbox

Patch

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))