Hello,
In the example of the patch below, the zero-initialization of the
instance of E runs past the size of the object.
That's because build_zero_init recursively tries to initialize all the
sub-objects of 'e' without handling cases where 'e' could have
sub-objects for virtual direct or indirect primary bases of E, that
would come after a sub-object for the primary base of E.
More specifically, the layout of 'e' is (I left the vptrs out
for clarity):
+subobject<B> <-- comes first b/c B is the primary base of E
+subobject<A>
+subobject<Implementation> <-- this one doesn't include any
+subobject<C> subjobject of B b/c B is already
included above.
And the code currently generated tries to zero-initialize
subobject<Implementation>.subobject<B> even though it is not present.
The patch below teaches build_zero_init to consider that after a
subobject for a primary base the object has no subobject for virtual
bases that are direct or indirect primary bases.
Tested on x86_64-unknown-linux-gnu and i686-unknown-linux-gnu against
trunk.
PS: Thanks to Jakub for coming up with the placement new idea that eases
the writing of a deja-gnu test for this PR, and for bootstrapping the
patch on his fast iron on i686 and x86_64 for all languages.
--
Dodji
>From b52987810a313657202fc7ecae6b503311146302 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <do...@redhat.com>
Date: Thu, 10 Mar 2011 14:10:05 +0100
Subject: [PATCH] PR c++/PR48035
gcc/cp/
* cp-tree.h (is_primary_base_of, is_virtual_base_of): Declare new
functions.
* class.c (is_base_of, is_virtual_base_of, is_primary_base_of):
Define new functions.
* init.c (build_zero_init_1): Extract from from build_zero_init.
Handle sub-objects for virtual primary bases allocated after a
sub-object of a primary base.
(build_zero_init_1): Use build_zero_init_1.
gcc/testsuite/
* g++.dg/inherit/virtual8.C: New test.
---
gcc/cp/class.c | 74 ++++++++++++++++++++++++++
gcc/cp/cp-tree.h | 2 +
gcc/cp/init.c | 86 +++++++++++++++++++++++-------
gcc/testsuite/g++.dg/inherit/virtual8.C | 52 +++++++++++++++++++
4 files changed, 194 insertions(+), 20 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..b811e8f 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -7008,6 +7008,80 @@ get_primary_binfo (tree binfo)
return copied_binfo (primary_base, binfo);
}
+/* This is a subroutine of is_virtual_base_of.
+
+ Returns TRUE if BASE is a direct or indirect base class of TYPE,
+ FALSE otherwise. */
+
+static bool
+is_base_of (tree base, tree type)
+{
+ int i;
+ tree binfo;
+
+ for (i = 0; BINFO_BASE_ITERATE (TYPE_BINFO (type), i, binfo); ++i)
+ {
+ if (same_type_p (BINFO_TYPE (binfo), base)
+ || is_base_of (base, BINFO_TYPE (binfo)))
+ return true;
+ }
+ return false;
+}
+
+/* Returns TRUE if BASE is a direct or indirect virtual base class of
+ TYPE, FALSE otherwise. */
+
+bool
+is_virtual_base_of (tree base, tree type)
+{
+ int i;
+ tree binfo;
+
+ for (i = 0; BINFO_BASE_ITERATE (TYPE_BINFO (type), i, binfo); ++i)
+ {
+ if (!BINFO_VIRTUAL_P (binfo))
+ continue;
+
+ if (same_type_p (BINFO_TYPE (binfo), base))
+ return true;
+
+ if (is_base_of (base, BINFO_TYPE (binfo)))
+ return true;
+ }
+ return false;
+}
+
+/* Returns TRUE if BASE is a direct primary base class of TYPE. If
+ INDIRECT_P is TRUE, then the function returns TRUE if BASE is a
+ direct or indirect base class of TYPE. Returns FALSE
+ otherwise. */
+
+bool
+is_primary_base_of (tree base, tree type, bool indirect_p)
+{
+ int i;
+ tree binfo;
+
+ if (!CLASS_TYPE_P (type)
+ || !CLASS_TYPE_P (base))
+ return false;
+
+ if (CLASSTYPE_HAS_PRIMARY_BASE_P (type)
+ && same_type_p (base,
+ BINFO_TYPE (get_primary_binfo (TYPE_BINFO (type)))))
+ return true;
+
+ if (!indirect_p)
+ return false;
+
+ for (i = 0; BINFO_BASE_ITERATE (TYPE_BINFO (type), i, binfo); ++i)
+ {
+ if (is_primary_base_of (base, BINFO_TYPE (binfo), true))
+ return true;
+ }
+ return false;
+}
+
/* 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..cba5ddb 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4727,6 +4727,8 @@ 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_primary_base_of (tree, tree, bool);
+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..7beb94f 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -130,20 +130,22 @@ 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.
-tree
-build_zero_init (tree type, tree nelts, bool static_storage_p)
+ The parameters are the same as for build_zero_init. If
+ CURRENT_OBJECT_TYPE is different from NULL_TREE, it means that we
+ are currently initializing sub-objects of an object of type
+ CURRENT_OBJECT_TYPE and we have already initialized the sub-object
+ for the primary base of CURRENT_OBJECT_TYPE. In that case, this
+ function will avoid initializing sub-objects for virtual direct or
+ indirect primary bases as per the C++ ABI specficication
+ [2.4.III/"Virtual Base Allocation"]. */
+
+static tree
+build_zero_init_1 (tree type,
+ tree nelts,
+ bool static_storage_p,
+ tree current_object_type)
{
tree init = NULL_TREE;
@@ -188,17 +190,42 @@ build_zero_init (tree type, tree nelts, bool
static_storage_p)
if (TREE_CODE (field) != FIELD_DECL)
continue;
+ /* If we are initializing a sub-object of
+ CURRENT_OBJECT_TYPE [for which a primary base class
+ sub-object has already been initialized] then we must NOT
+ initialize any sub-object from a virtual base that is a
+ direct or indirect primary base of
+ CURRENT_OBJECT_TYPE. */
+ if (current_object_type
+ && is_virtual_base_of (TREE_TYPE (field), current_object_type)
+ && is_primary_base_of (TREE_TYPE (field), current_object_type,
+ /*indirect_p=*/true))
+ 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,
+ current_object_type);
if (value)
CONSTRUCTOR_APPEND_ELT(v, field, value);
+
+ /* Dectect the case where, while initializing an object
+ of type TYPE, we have just initialized a sub-object
+ of TYPE for a primary base. That sub-object would
+ then be FIELD. In that case, we must be remember
+ what object we are initializing so that we can apply
+ the rules of sub-objects allocation for virtual
+ bases. */
+ if (current_object_type == NULL_TREE
+ && is_primary_base_of (TREE_TYPE (field), type,
+ /*indirect_p=*/false))
+ current_object_type = type;
}
/* For unions, only the first field is initialized. */
@@ -244,9 +271,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,
+ NULL_TREE);
}
/* Build a constructor to contain the initializations. */
@@ -261,7 +289,25 @@ build_zero_init (tree type, tree nelts, bool
static_storage_p)
if (init)
TREE_CONSTANT (init) = 1;
- return init;
+ 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, NULL_TREE);
}
/* Return a suitable initializer for value-initializing an object of type
diff --git a/gcc/testsuite/g++.dg/inherit/virtual8.C
b/gcc/testsuite/g++.dg/inherit/virtual8.C
new file mode 100644
index 0000000..28626f4
--- /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>
+
+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)
+ return 1;
+
+ delete[] v;
+ return 0;
+}
--
1.7.3.4
