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]

C++ PATCH for c++/50618 (wrong-code with virtual bases)


When an object is value-initialized, if the type doesn't have a user-provided default constructor, the object is zero-initialized first, and then the synthesized constructor is called. The problem in this PR was that when value-initializing a base in a constructor we were zero-initializing virtual bases of that base even though they had already been initialized properly. The fix is to specify to build_zero_init_1 that we only want to clear the as-base portion of the type.

On the trunk I also tidied up the logic; on release branches I made the minimal change. For the 4.4 branch I also needed to backport the fix for 48035.

Tested x86_64-pc-linux-gnu, applying to 4.4, 4.5, 4.6 and trunk.
commit ae7159a0bd2822557c503cd85d911e0390aaf55d
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Oct 13 12:59:31 2011 -0400

    	PR c++/50618
    	* init.c (expand_aggr_init_1): Don't zero-initialize virtual
    	bases of a base subobject.

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 7897fff..8a5bece 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1588,27 +1588,26 @@ expand_aggr_init_1 (tree binfo, tree true_exp, tree exp, tree init, int flags,
      that's value-initialization.  */
   if (init == void_type_node)
     {
-      /* If there's a user-provided constructor, we just call that.  */
-      if (type_has_user_provided_constructor (type))
-	/* Fall through.  */;
-      /* If there isn't, but we still need to call the constructor,
-	 zero out the object first.  */
-      else if (type_build_ctor_call (type))
+      /* If no user-provided ctor, we need to zero out the object.  */
+      if (!type_has_user_provided_constructor (type))
 	{
-	  init = build_zero_init (type, NULL_TREE, /*static_storage_p=*/false);
+	  tree field_size = NULL_TREE;
+	  if (exp != true_exp
+	      && CLASSTYPE_AS_BASE (type) != type)
+	    /* Don't clobber already initialized virtual bases.  */
+	    field_size = TYPE_SIZE (CLASSTYPE_AS_BASE (type));
+	  init = build_zero_init_1 (type, NULL_TREE, /*static_storage_p=*/false,
+				    field_size);
 	  init = build2 (INIT_EXPR, type, exp, init);
 	  finish_expr_stmt (init);
-	  /* And then call the constructor.  */
 	}
+
       /* If we don't need to mess with the constructor at all,
-	 then just zero out the object and we're done.  */
-      else
-	{
-	  init = build2 (INIT_EXPR, type, exp,
-			 build_value_init_noctor (type, complain));
-	  finish_expr_stmt (init);
-	  return;
-	}
+	 then we're done.  */
+      if (! type_build_ctor_call (type))
+	return;
+
+      /* Otherwise fall through and call the constructor.  */
       init = NULL_TREE;
     }
 
diff --git a/gcc/testsuite/g++.dg/init/vbase1.C b/gcc/testsuite/g++.dg/init/vbase1.C
new file mode 100644
index 0000000..bbfd58f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/vbase1.C
@@ -0,0 +1,39 @@
+// PR c++/50618
+// { dg-do run }
+
+struct Base
+{
+    const int text;
+    Base():text(1) {}
+    Base(int aText)
+    : text(aText) {}
+};
+struct SubA : public virtual Base
+{
+protected:
+  int x;
+public:
+  SubA(int aX)
+  : x(aX) {}
+};
+class SubB : public virtual Base
+{};
+struct Diamond : public SubA, public SubB
+{
+    Diamond(int text)
+    : Base(text), SubA(5), SubB() {}
+
+    void printText()
+    {
+        if(text != 2)
+          __builtin_abort();
+        if(x!=5)
+          __builtin_abort();
+    }
+};
+
+int main(int, char**)
+{
+    Diamond x(2);
+    x.printText();
+}
commit fda61bbc8fd29b1df3dbeb576e0dcf806b2fcdf5
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Oct 13 13:11:31 2011 -0400

    	PR c++/50618
    	* init.c (expand_aggr_init_1): Don't zero-initialize virtual
    	bases of a base subobject.

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index c4bd635..f85a30b 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1561,7 +1561,12 @@ expand_aggr_init_1 (tree binfo, tree true_exp, tree exp, tree init, int flags,
 	 zero out the object first.  */
       else if (TYPE_NEEDS_CONSTRUCTING (type))
 	{
-	  init = build_zero_init (type, NULL_TREE, /*static_storage_p=*/false);
+	  tree field_size = NULL_TREE;
+	  if (exp != true_exp && CLASSTYPE_AS_BASE (type) != type)
+	    /* Don't clobber already initialized virtual bases.  */
+	    field_size = TYPE_SIZE (CLASSTYPE_AS_BASE (type));
+	  init = build_zero_init_1 (type, NULL_TREE, /*static_storage_p=*/false,
+				    field_size);
 	  init = build2 (INIT_EXPR, type, exp, init);
 	  finish_expr_stmt (init);
 	  /* And then call the constructor.  */
diff --git a/gcc/testsuite/g++.dg/init/vbase1.C b/gcc/testsuite/g++.dg/init/vbase1.C
new file mode 100644
index 0000000..bbfd58f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/vbase1.C
@@ -0,0 +1,39 @@
+// PR c++/50618
+// { dg-do run }
+
+struct Base
+{
+    const int text;
+    Base():text(1) {}
+    Base(int aText)
+    : text(aText) {}
+};
+struct SubA : public virtual Base
+{
+protected:
+  int x;
+public:
+  SubA(int aX)
+  : x(aX) {}
+};
+class SubB : public virtual Base
+{};
+struct Diamond : public SubA, public SubB
+{
+    Diamond(int text)
+    : Base(text), SubA(5), SubB() {}
+
+    void printText()
+    {
+        if(text != 2)
+          __builtin_abort();
+        if(x!=5)
+          __builtin_abort();
+    }
+};
+
+int main(int, char**)
+{
+    Diamond x(2);
+    x.printText();
+}

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