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