This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR c++/PR48035


Jason Merrill <jason@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 <jakub@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 <dodji@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]