diff mbox

C++ PATCH for c++/51331 (wrong-code with virtual bases and zero-initialization)

Message ID 4EEBC6DF.8060401@redhat.com
State New
Headers show

Commit Message

Jason Merrill Dec. 16, 2011, 10:31 p.m. UTC 
For PR 48035, Jakub factored build_zero_init_1 out of build_zero_init to 
avoid clobbering the area occupied by virtual bases in the complete 
type, but not part of a base subobject.  For 50618, I changed 
build_aggr_init_1 to use it.  This testcase shows another case of the 
same problem: in this case, we're dealing with a virtual base of a 
virtual base.  Building the smaller CONSTRUCTOR doesn't help in this 
case because an empty CONSTRUCTOR means zero-initializing the whole 
object.  The reason the 48035/50618 patches work is that in 
expand_assignment, when we're dealing with a COMPONENT_REF to a 
particular base field, we use the DECL_SIZE of the field to know how 
much space to zero out.  In this case we don't have a COMPONENT_REF, so 
we still zero out the full size of the type.

This patch fixes the PR by building a COMPONENT_REF in this case as 
well.  A proper fix depends on 22488.

Tested x86_64-pc-linux-gnu, applying to trunk and 4.6.
diff mbox

Patch

commit e3c92cd26597fea326d06a17e029d5b2c72e0e6c
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Dec 16 13:54:35 2011 -0500

    	PR c++/51331
    	* class.c (convert_to_base_statically): Just call
    	build_simple_base_path.
    	(build_simple_base_path): Check field offset.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 3cb76de..c96f7bf 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -471,7 +471,14 @@  build_simple_base_path (tree expr, tree binfo)
     /* Is this the base field created by build_base_field?  */
     if (TREE_CODE (field) == FIELD_DECL
 	&& DECL_FIELD_IS_BASE (field)
-	&& TREE_TYPE (field) == type)
+	&& TREE_TYPE (field) == type
+	/* If we're looking for a field in the most-derived class,
+	   also check the field offset; we can have two base fields
+	   of the same type if one is an indirect virtual base and one
+	   is a direct non-virtual base.  */
+	&& (BINFO_INHERITANCE_CHAIN (d_binfo)
+	    || tree_int_cst_equal (byte_position (field),
+				   BINFO_OFFSET (binfo))))
       {
 	/* We don't use build_class_member_access_expr here, as that
 	   has unnecessary checks, and more importantly results in
@@ -546,6 +553,10 @@  convert_to_base_statically (tree expr, tree base)
   expr_type = TREE_TYPE (expr);
   if (!SAME_BINFO_TYPE_P (BINFO_TYPE (base), expr_type))
     {
+      /* If this is a non-empty base, use a COMPONENT_REF.  */
+      if (!is_empty_class (BINFO_TYPE (base)))
+	return build_simple_base_path (expr, base);
+
       /* We use fold_build2 and fold_convert below to simplify the trees
 	 provided to the optimizers.  It is not safe to call these functions
 	 when processing a template because they do not handle C++-specific
diff --git a/gcc/testsuite/g++.dg/init/value10.C b/gcc/testsuite/g++.dg/init/value10.C
new file mode 100644
index 0000000..2066410
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/value10.C
@@ -0,0 +1,27 @@ 
+// PR c++/51331
+// { dg-do run }
+
+struct A {
+  A(): x(10) {}
+  virtual ~A() {}
+
+  int x;
+};
+
+struct B: public virtual A {
+};
+
+struct C: public virtual A {
+};
+
+struct D: public B, virtual public C {
+  D(): B(), C() {}  // note an explicit call to C() which is auto-generated
+};
+
+int main() {
+  D* d = new D();
+
+  // Crashes here with the following message:
+  // *** glibc detected *** ./test: free(): invalid next size (fast)
+  delete d;
+}