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
2012-06-25 Florian Weimer <fwei...@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 <fwei...@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);
+}