* C++ PATCH for c++/48446 (ICE with VLA)
@ 2011-04-14 14:52 Jason Merrill
2011-05-02 18:38 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Jason Merrill @ 2011-04-14 14:52 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 541 bytes --]
The issue here was that with a complex expression in DECL_SIZE for z,
when wrap_cleanups tried to protect the initializer of m it walked into
the sizeof and thus the bounds expression, which it really shouldn't be
messing with.
This patch avoids this issue by saving bounds values which have side
effects into a local automatic variable (since VLAs can only appear in
automatic variables). We stick with the SAVE_EXPR approach for bounds
without side-effects to avoid breaking vla9.C.
Tested x86_64-pc-linux-gnu, applying to trunk.
[-- Attachment #2: 48446.patch --]
[-- Type: text/plain, Size: 3135 bytes --]
commit e491243ab5dce11b38cae78911f107652fa0048b
Author: Jason Merrill <jason@redhat.com>
Date: Fri Apr 8 17:43:42 2011 -0400
PR c++/48446
* decl.c (compute_array_index_type): Use get_temp_regvar instead
of variable_size.
* init.c (get_temp_regvar): No longer static.
* cp-tree.h: Declare it.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 15c1974..3ca44c2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4964,6 +4964,7 @@ extern tree build_offset_ref (tree, tree, bool);
extern tree build_new (VEC(tree,gc) **, tree, tree,
VEC(tree,gc) **, int,
tsubst_flags_t);
+extern tree get_temp_regvar (tree, tree);
extern tree build_vec_init (tree, tree, tree, bool, int,
tsubst_flags_t);
extern tree build_delete (tree, tree,
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 7dea9b7..2c0f80f 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7710,8 +7710,16 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain)
processing_template_decl = saved_processing_template_decl;
if (!TREE_CONSTANT (itype))
- /* A variable sized array. */
- itype = variable_size (itype);
+ {
+ /* A variable sized array. */
+ if (TREE_SIDE_EFFECTS (itype))
+ /* Use get_temp_regvar rather than variable_size here so that
+ people walking expressions that use a variable of this type
+ don't walk into this expression. */
+ itype = get_temp_regvar (TREE_TYPE (itype), itype);
+ else
+ itype = variable_size (itype);
+ }
/* Make sure that there was no overflow when creating to a signed
index type. (For example, on a 32-bit machine, an array with
size 2^32 - 1 is too big.) */
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 3131690..32afa03 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -45,7 +45,6 @@ static void expand_virtual_init (tree, tree);
static tree sort_mem_initializers (tree, tree);
static tree initializing_context (tree);
static void expand_cleanup_for_base (tree, tree);
-static tree get_temp_regvar (tree, tree);
static tree dfs_initialize_vtbl_ptrs (tree, void *);
static tree build_dtor_call (tree, special_function_kind, int);
static tree build_field_list (tree, tree, int *);
@@ -2875,7 +2874,7 @@ create_temporary_var (tree type)
things when it comes time to do final cleanups (which take place
"outside" the binding contour of the function). */
-static tree
+tree
get_temp_regvar (tree type, tree init)
{
tree decl;
diff --git a/gcc/testsuite/g++.dg/ext/vla10.C b/gcc/testsuite/g++.dg/ext/vla10.C
new file mode 100644
index 0000000..17cdb2f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/vla10.C
@@ -0,0 +1,32 @@
+// PR c++/48446
+// { dg-options "" }
+
+template<typename T>
+struct A
+{
+ ~A ();
+ T *operator-> () const;
+};
+
+struct B
+{
+ typedef A <B> P;
+ static P foo (int);
+};
+
+struct C
+{
+ typedef A<C> P;
+ static const int c = 80;
+};
+
+C::P bar ();
+
+void
+baz ()
+{
+ char z[bar ()->c];
+ {
+ B::P m = B::foo (sizeof (z));
+ }
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: C++ PATCH for c++/48446 (ICE with VLA)
2011-04-14 14:52 C++ PATCH for c++/48446 (ICE with VLA) Jason Merrill
@ 2011-05-02 18:38 ` Jason Merrill
2011-05-06 22:51 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Jason Merrill @ 2011-05-02 18:38 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 709 bytes --]
On 04/14/2011 10:52 AM, Jason Merrill wrote:
> This patch avoids this issue by saving bounds values which have side
> effects into a local automatic variable (since VLAs can only appear in
> automatic variables). We stick with the SAVE_EXPR approach for bounds
> without side-effects to avoid breaking vla9.C.
I finally found the code in grokdeclarator that I thought I remembered
while working on this bug but couldn't find, and the comment there led
me to put together a testcase broken by this change. So I'm reverting
the change to compute_array_index_type in favor of a more aggressive
version of the precalculation that grokdeclarator already does.
Tested x86_64-pc-linux-gnu, applying to trunk.
[-- Attachment #2: 48446-2.patch --]
[-- Type: text/plain, Size: 4207 bytes --]
commit dc6c2d45b04636be3430ca7722a5f648001659d2
Author: Jason Merrill <jason@redhat.com>
Date: Fri Apr 29 11:56:33 2011 -0400
PR c++/48446
* decl.c (stabilize_save_expr_r, stabilize_vla_size): New.
(compute_array_index_type): Revert earlier 48446 changes.
(grokdeclarator): Use stabilize_vla_size.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0a2e1dd..f9dd6de 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7576,6 +7576,38 @@ check_static_variable_definition (tree decl, tree type)
return 0;
}
+/* *expr_p is part of the TYPE_SIZE of a variably-sized array. If any
+ SAVE_EXPRs in *expr_p wrap expressions with side-effects, break those
+ expressions out into temporary variables so that walk_tree doesn't
+ step into them (c++/15764). */
+
+static tree
+stabilize_save_expr_r (tree *expr_p, int *walk_subtrees, void *data)
+{
+ struct pointer_set_t *pset = (struct pointer_set_t *)data;
+ tree expr = *expr_p;
+ if (TREE_CODE (expr) == SAVE_EXPR)
+ {
+ tree op = TREE_OPERAND (expr, 0);
+ cp_walk_tree (&op, stabilize_save_expr_r, data, pset);
+ if (TREE_SIDE_EFFECTS (op))
+ TREE_OPERAND (expr, 0) = get_temp_regvar (TREE_TYPE (op), op);
+ }
+ else if (!EXPR_P (expr))
+ *walk_subtrees = 0;
+ return NULL;
+}
+
+/* Entry point for the above. */
+
+static void
+stabilize_vla_size (tree size)
+{
+ struct pointer_set_t *pset = pointer_set_create ();
+ /* Break out any function calls into temporary variables. */
+ cp_walk_tree (&size, stabilize_save_expr_r, pset, pset);
+}
+
/* Given the SIZE (i.e., number of elements) in an array, compute an
appropriate index type for the array. If non-NULL, NAME is the
name of the thing being declared. */
@@ -7769,16 +7801,8 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain)
processing_template_decl = saved_processing_template_decl;
if (!TREE_CONSTANT (itype))
- {
- /* A variable sized array. */
- if (TREE_SIDE_EFFECTS (itype))
- /* Use get_temp_regvar rather than variable_size here so that
- people walking expressions that use a variable of this type
- don't walk into this expression. */
- itype = get_temp_regvar (TREE_TYPE (itype), itype);
- else
- itype = variable_size (itype);
- }
+ /* A variable sized array. */
+ itype = variable_size (itype);
/* Make sure that there was no overflow when creating to a signed
index type. (For example, on a 32-bit machine, an array with
size 2^32 - 1 is too big.) */
@@ -9051,7 +9075,12 @@ grokdeclarator (const cp_declarator *declarator,
&& (decl_context == NORMAL || decl_context == FIELD)
&& at_function_scope_p ()
&& variably_modified_type_p (type, NULL_TREE))
- finish_expr_stmt (TYPE_SIZE (type));
+ {
+ /* First break out any side-effects. */
+ stabilize_vla_size (TYPE_SIZE (type));
+ /* And then force evaluation of the SAVE_EXPR. */
+ finish_expr_stmt (TYPE_SIZE (type));
+ }
if (declarator->kind == cdk_reference)
{
@@ -9126,6 +9155,14 @@ grokdeclarator (const cp_declarator *declarator,
}
}
+ /* We need to stabilize side-effects in VLA sizes for regular array
+ declarations too, not just pointers to arrays. */
+ if (type != error_mark_node && !TYPE_NAME (type)
+ && (decl_context == NORMAL || decl_context == FIELD)
+ && at_function_scope_p ()
+ && variably_modified_type_p (type, NULL_TREE))
+ stabilize_vla_size (TYPE_SIZE (type));
+
/* A `constexpr' specifier used in an object declaration declares
the object as `const'. */
if (constexpr_p && innermost_code != cdk_function)
diff --git a/gcc/testsuite/c-c++-common/vla-1.c b/gcc/testsuite/c-c++-common/vla-1.c
new file mode 100644
index 0000000..401c4e0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/vla-1.c
@@ -0,0 +1,21 @@
+/* Test that changes to a variable are reflected in a VLA later in the
+ expression. */
+/* { dg-options "" } */
+
+#ifdef __cplusplus
+extern "C"
+#endif
+void abort();
+
+int i = 4;
+int f()
+{
+ return i;
+}
+
+int main()
+{
+ if (i+=2, sizeof(*(int(*)[f()])0) != 6*sizeof(int))
+ abort();
+ return 0;
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: C++ PATCH for c++/48446 (ICE with VLA)
2011-05-02 18:38 ` Jason Merrill
@ 2011-05-06 22:51 ` Jason Merrill
0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2011-05-06 22:51 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 209 bytes --]
I noticed a minor tweak I could make to speed this up and figure I might
as well, even though it shouldn't be a significant component of compile
time.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.6.
[-- Attachment #2: 48446-3.patch --]
[-- Type: text/plain, Size: 737 bytes --]
commit 4c3e6de3e988799dac490b6eb2b762674b5bb9f8
Author: Jason Merrill <jason@redhat.com>
Date: Thu May 5 17:57:50 2011 -0400
* decl.c (stabilize_save_expr_r): Set *walk_subtrees as
appropriate.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index c5184e0..b5d4cc2 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7615,8 +7615,9 @@ stabilize_save_expr_r (tree *expr_p, int *walk_subtrees, void *data)
cp_walk_tree (&op, stabilize_save_expr_r, data, pset);
if (TREE_SIDE_EFFECTS (op))
TREE_OPERAND (expr, 0) = get_temp_regvar (TREE_TYPE (op), op);
+ *walk_subtrees = 0;
}
- else if (!EXPR_P (expr))
+ else if (!EXPR_P (expr) || !TREE_SIDE_EFFECTS (expr))
*walk_subtrees = 0;
return NULL;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-05-06 21:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-14 14:52 C++ PATCH for c++/48446 (ICE with VLA) Jason Merrill
2011-05-02 18:38 ` Jason Merrill
2011-05-06 22:51 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).