Jason Merrill <ja...@redhat.com> writes:
> If we're currently initializing a subobject, then we don't want to
> initialize any of our virtual base fields unless they are primary to
> the current type.
Indeed. I think I understand better now.
> I'm also rather nervous about using is_*_base_of tests given that a
> class can have multiple indirect bases of a particular type. Whether
> or not there is a virtual base T of U isn't important; what is
> important is whether the current field represents a virtual base.
I have updated is_virtual_base_of to make it test if the current field
represents a virtual base. Is there a simpler way to detect that?
> In the C++ testcase most testcases return non-zero to indicate
> failure. The main reason for this is to avoid having to deal with
> declaring abort.
Ah okay. I did notice that C++ tests were using the non-zero return
convention but I didn't know why. In the patch below I took the liberty
to use abort() nonetheless because as we need to include <new> for the
placement new operator, I thought just adding another header would be
understandable.
Jakub Jelinek <ja...@redhat.com> writes:
> Or, in the simplify_aggr_init case where the created tree is handed
> immediately to the gimplifier, there is IMHO no point in initializing
> all fields to zero when the gimplifier throws that immediately away
> again.
>From what I understand, zero-initializing doesn't necessarily mean
setting all fields to zero, because, e.g, for member pointers fields the
ABI states that a NULL pointer is represented as -1.
So this is the patch I am currently testing.
--
Dodji
>From 6a757e998cb6b09883ec366c8c8939a70a215600 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <do...@redhat.com>
Date: Fri, 11 Mar 2011 15:24:00 +0100
Subject: [PATCH] [PATCH] PR c++/PR48035
gcc/cp/
* cp-tree.h (is_virtual_base_of): Declare new function.
* class.c (is_virtual_base_of): Define it.
* init.c (build_zero_init_1): Extract from from build_zero_init.
Don't initialize non-primary virtual bases of sub-objects.
(build_zero_init_1): Use build_zero_init_1.
gcc/testsuite/
* g++.dg/inherit/virtual8.C: New test.
---
gcc/cp/class.c | 21 ++++++++++
gcc/cp/cp-tree.h | 1 +
gcc/cp/init.c | 66 ++++++++++++++++++++++---------
gcc/testsuite/g++.dg/inherit/virtual8.C | 52 ++++++++++++++++++++++++
4 files changed, 121 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/inherit/virtual8.C
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 1325260..4c6c9d5 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -7008,6 +7008,27 @@ get_primary_binfo (tree binfo)
return copied_binfo (primary_base, binfo);
}
+/* Returns TRUE if BASE is a direct virtual base of TYPE, FALSE
+ otherwise. */
+
+bool
+is_virtual_base_of (tree base, tree type)
+{
+ tree binfo;
+
+ for (binfo = TREE_CHAIN (TYPE_BINFO (type));
+ binfo;
+ binfo = TREE_CHAIN (binfo))
+ {
+ if (!BINFO_VIRTUAL_P (binfo))
+ continue;
+
+ if (same_type_p (BINFO_TYPE (binfo), base))
+ return binfo;
+ }
+ return NULL_TREE;
+}
+
/* If INDENTED_P is zero, indent to INDENT. Return nonzero. */
static int
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 4b49046..558c38b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4727,6 +4727,7 @@ extern void fixup_attribute_variants (tree);
extern tree* decl_cloned_function_p (const_tree, bool);
extern void clone_function_decl (tree, int);
extern void adjust_clone_args (tree);
+extern bool is_virtual_base_of (tree, tree);
/* in cvt.c */
extern tree convert_to_reference (tree, tree, int, int, tree);
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 56f66fa..440db79 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -130,20 +130,17 @@ initialize_vtbl_ptrs (tree addr)
dfs_walk_once (TYPE_BINFO (type), dfs_initialize_vtbl_ptrs, NULL, list);
}
-/* Return an expression for the zero-initialization of an object with
- type T. This expression will either be a constant (in the case
- that T is a scalar), or a CONSTRUCTOR (in the case that T is an
- aggregate), or NULL (in the case that T does not require
- initialization). In either case, the value can be used as
- DECL_INITIAL for a decl of the indicated TYPE; it is a valid static
- initializer. If NELTS is non-NULL, and TYPE is an ARRAY_TYPE, NELTS
- is the number of elements in the array. If STATIC_STORAGE_P is
- TRUE, initializers are only generated for entities for which
- zero-initialization does not simply mean filling the storage with
- zero bytes. */
+/* A subroutine of build_zero_init.
+
+ The parameters are the same as for build_zero_init. If
+ IN_SUBOBJECT is TRUE, that means we are currently initializing a
+ subobject. In that case, we don't initialize virtual bases unless
+ the virtual base is a primary base of TYPE. */
-tree
-build_zero_init (tree type, tree nelts, bool static_storage_p)
+static tree
+build_zero_init_1 (tree type, tree nelts,
+ bool static_storage_p,
+ bool in_subjobject)
{
tree init = NULL_TREE;
@@ -188,15 +185,25 @@ build_zero_init (tree type, tree nelts, bool
static_storage_p)
if (TREE_CODE (field) != FIELD_DECL)
continue;
+ /* If we are initializing a subobject then skip non-primary
+ virtual bases. */
+ if (in_subjobject
+ && is_virtual_base_of (TREE_TYPE (field), type)
+ && CLASSTYPE_PRIMARY_BINFO (type)
+ && !same_type_p (BINFO_TYPE (CLASSTYPE_PRIMARY_BINFO (type)),
+ TREE_TYPE (field)))
+ continue;
+
/* Note that for class types there will be FIELD_DECLs
corresponding to base classes as well. Thus, iterating
over TYPE_FIELDs will result in correct initialization of
all of the subobjects. */
if (!static_storage_p || !zero_init_p (TREE_TYPE (field)))
{
- tree value = build_zero_init (TREE_TYPE (field),
- /*nelts=*/NULL_TREE,
- static_storage_p);
+ tree value = build_zero_init_1 (TREE_TYPE (field),
+ /*nelts=*/NULL_TREE,
+ static_storage_p,
+ /*in_subobject=*/true);
if (value)
CONSTRUCTOR_APPEND_ELT(v, field, value);
}
@@ -244,9 +251,10 @@ build_zero_init (tree type, tree nelts, bool
static_storage_p)
ce->index = build2 (RANGE_EXPR, sizetype, size_zero_node,
max_index);
- ce->value = build_zero_init (TREE_TYPE (type),
- /*nelts=*/NULL_TREE,
- static_storage_p);
+ ce->value = build_zero_init_1 (TREE_TYPE (type),
+ /*nelts=*/NULL_TREE,
+ static_storage_p,
+ false);
}
/* Build a constructor to contain the initializations. */
@@ -264,6 +272,26 @@ build_zero_init (tree type, tree nelts, bool
static_storage_p)
return init;
}
+/* Return an expression for the zero-initialization of an object with
+ type T. This expression will either be a constant (in the case
+ that T is a scalar), or a CONSTRUCTOR (in the case that T is an
+ aggregate), or NULL (in the case that T does not require
+ initialization). In either case, the value can be used as
+ DECL_INITIAL for a decl of the indicated TYPE; it is a valid static
+ initializer. If NELTS is non-NULL, and TYPE is an ARRAY_TYPE, NELTS
+ is the number of elements in the array. If STATIC_STORAGE_P is
+ TRUE, initializers are only generated for entities for which
+ zero-initialization does not simply mean filling the storage with
+ zero bytes. */
+
+tree
+build_zero_init (tree type, tree nelts, bool static_storage_p)
+{
+ return build_zero_init_1 (type, nelts,
+ static_storage_p,
+ /*in_subobject=*/false);
+}
+
/* Return a suitable initializer for value-initializing an object of type
TYPE, as described in [dcl.init]. */
diff --git a/gcc/testsuite/g++.dg/inherit/virtual8.C
b/gcc/testsuite/g++.dg/inherit/virtual8.C
new file mode 100644
index 0000000..62bdc3b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/inherit/virtual8.C
@@ -0,0 +1,52 @@
+// Origin PR c++/PR48035
+// { dg-do run }
+
+#include <new>
+#include <cstring>
+#include <cstdlib>
+
+struct A
+{
+ virtual void foo (void) {}
+ virtual ~A () {}
+};
+
+struct B : public A
+{
+ virtual ~B () {}
+};
+
+struct C
+{
+ virtual ~C ()
+ {
+ }
+ int c;
+};
+
+struct D : public virtual B, public C
+{
+ virtual ~D () {}
+};
+
+struct E : public virtual D
+{
+ virtual ~E ()
+ {
+ }
+};
+
+int
+main ()
+{
+ char *v = new char[sizeof (E) + 16];
+ memset (v, 0x55, sizeof (E) + 16);
+ E *e = new (v) E ();
+ e->~E ();
+
+ for (unsigned i = sizeof (E); i < sizeof (E) + 16; ++i)
+ if (v[i] != 0x55)
+ abort();
+ delete[] v;
+}
+
--
1.7.3.4
