[RFA] fix c++/17386

[Richard Henderson <rth at redhat dot com>]

As discussed in the PR, the problem here is that the vptr field is
accessed via two different FIELD_DECLs:

  obj.<D1575>._vptr.A = &_ZTV1A[2];
  obj._vptr.A = &_ZTV1C[2];

The aliasing code is built to assume that no structure fields 
overlap.  Only unions are allowed to have overlapping fields.
So it determines that these two memory accesses do not alias,
which leads to the observable miscompilation in the scheduler.

The following adjusts build_vfield_ref such that we always
reference the vptr through the primary base class that owns
the vptr.  So "obj.<D1575>._vptr.A" in this case.

Couple of concerns:

  (1) The ??? in make_new_vtable was correct.  The replacement I used
      passes the same data.  I left things as-is because it seems to work.

  (2) I wonder if the whole warn_invalid_offsetof thing in typeck.c
      that I patched can be deleted.  Or moved elsewhere.  Or something.
      Not with this patch though...

  (3) Why in the world was TYPE_VFIELD being duplicated in the first place?
  
Bootstrapped and tested on i686-linux.

Ok?


r~


	* call.c (build_vfield_ref): Move...
	* class.c (build_vfield_ref): ... here.  Convert datum to the
	primary base containing the vptr.
	(make_new_vtable): Simplify build_primary_vtable arguments.
	(finish_struct_1): Do not duplicate TYPE_VFIELD.
	* typeck.c (build_class_member_access_expr): Don't warn for
	null object access to base fields.

Index: call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.504
diff -u -p -r1.504 call.c
--- call.c	30 Aug 2004 15:28:37 -0000	1.504
+++ call.c	10 Sep 2004 20:33:41 -0000
@@ -191,23 +191,6 @@ static bool magic_varargs_p (tree);
 static tree build_temp (tree, tree, int, void (**)(const char *, ...));
 static void check_constructor_callable (tree, tree);
 
-tree
-build_vfield_ref (tree datum, tree type)
-{
-  if (datum == error_mark_node)
-    return error_mark_node;
-
-  if (TREE_CODE (TREE_TYPE (datum)) == REFERENCE_TYPE)
-    datum = convert_from_reference (datum);
-
-  if (TYPE_BASE_CONVS_MAY_REQUIRE_CODE_P (type)
-      && !same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (datum), type))
-    datum = convert_to_base (datum, type, /*check_access=*/false);
-
-  return build3 (COMPONENT_REF, TREE_TYPE (TYPE_VFIELD (type)),
-		 datum, TYPE_VFIELD (type), NULL_TREE);
-}
-
 /* Returns nonzero iff the destructor name specified in NAME
    (a BIT_NOT_EXPR) matches BASETYPE.  The operand of NAME can take many
    forms...  */
Index: class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.669
diff -u -p -r1.669 class.c
--- class.c	10 Sep 2004 11:12:10 -0000	1.669
+++ class.c	10 Sep 2004 20:33:42 -0000
@@ -483,6 +483,38 @@ convert_to_base_statically (tree expr, t
 }
 
 
+tree
+build_vfield_ref (tree datum, tree type)
+{
+  tree vfield, vcontext;
+
+  if (datum == error_mark_node)
+    return error_mark_node;
+
+  if (TREE_CODE (TREE_TYPE (datum)) == REFERENCE_TYPE)
+    datum = convert_from_reference (datum);
+
+  /* First, convert to the requested type.  */
+  if (!same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (datum), type))
+    datum = convert_to_base (datum, type, /*check_access=*/false);
+
+  /* Second, the requested type may not be the owner of its own vptr.
+     If not, convert to the base class that owns it.  We cannot use
+     convert_to_base here, because VCONTEXT may appear more than once
+     in the inheritence hierarchy of TYPE, and thus direct conversion
+     between the types may be ambiguous.  Following the path back up
+     one step at a time via primary bases avoids the problem.  */
+  vfield = TYPE_VFIELD (type);
+  vcontext = DECL_CONTEXT (vfield);
+  while (!same_type_ignoring_top_level_qualifiers_p (vcontext, type))
+    {
+      datum = build_simple_base_path (datum, CLASSTYPE_PRIMARY_BINFO (type));
+      type = TREE_TYPE (datum);
+    }
+
+  return build3 (COMPONENT_REF, TREE_TYPE (vfield), datum, vfield, NULL_TREE);
+}
+
 /* Given an object INSTANCE, return an expression which yields the
    vtable element corresponding to INDEX.  There are many special
    cases for INSTANCE which we take care of here, mainly to avoid
@@ -783,10 +815,7 @@ make_new_vtable (tree t, tree binfo)
     /* In this case, it is *type*'s vtable we are modifying.  We start
        with the approximation that its vtable is that of the
        immediate base class.  */
-    /* ??? This actually passes TYPE_BINFO (t), not the primary base binfo,
-       since we've updated DECL_CONTEXT (TYPE_VFIELD (t)) by now.  */
-    return build_primary_vtable (TYPE_BINFO (DECL_CONTEXT (TYPE_VFIELD (t))),
-				 t);
+    return build_primary_vtable (binfo, t);
   else
     /* This is our very own copy of `basetype' to play with.  Later,
        we will fill in all the virtual functions that override the
@@ -4927,7 +4956,6 @@ finish_struct_1 (tree t)
   /* A TREE_LIST.  The TREE_VALUE of each node is a FUNCTION_DECL.  */
   tree virtuals = NULL_TREE;
   int n_fields = 0;
-  tree vfield;
 
   if (COMPLETE_TYPE_P (t))
     {
@@ -4981,25 +5009,6 @@ finish_struct_1 (tree t)
        needs a mode.  */
     compute_record_mode (CLASSTYPE_AS_BASE (t));
 
-  /* Make sure that we get our own copy of the vfield FIELD_DECL.  */
-  vfield = TYPE_VFIELD (t);
-  if (vfield && CLASSTYPE_HAS_PRIMARY_BASE_P (t))
-    {
-      tree primary = CLASSTYPE_PRIMARY_BINFO (t);
-
-      gcc_assert (same_type_p (DECL_FIELD_CONTEXT (vfield),
-			       BINFO_TYPE (primary)));
-      /* The vtable better be at the start.  */
-      gcc_assert (integer_zerop (DECL_FIELD_OFFSET (vfield)));
-      gcc_assert (integer_zerop (BINFO_OFFSET (primary)));
-      
-      vfield = copy_decl (vfield);
-      DECL_FIELD_CONTEXT (vfield) = t;
-      TYPE_VFIELD (t) = vfield;
-    }
-  else
-    gcc_assert (!vfield || DECL_FIELD_CONTEXT (vfield) == t);
-
   virtuals = modify_all_vtables (t, nreverse (virtuals));
 
   /* If necessary, create the primary vtable for this class.  */
Index: typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck.c,v
retrieving revision 1.574
diff -u -p -r1.574 typeck.c
--- typeck.c	10 Sep 2004 06:40:15 -0000	1.574
+++ typeck.c	10 Sep 2004 20:33:42 -0000
@@ -1711,9 +1711,14 @@ build_class_member_access_expr (tree obj
 	 give the right answer.  Note that we complain whether or not they
 	 actually used the offsetof macro, since there's no way to know at this
 	 point.  So we just give a warning, instead of a pedwarn.  */
+      /* Do not produce this warning for base class field references, because
+	 we know for a fact that didn't come from offsetof.  This does occur
+	 in various testsuite cases where a null object is passed where a
+	 vtable access is required.  */
       if (null_object_p && warn_invalid_offsetof
 	  && CLASSTYPE_NON_POD_P (object_type)
-	  && ! skip_evaluation)
+	  && !DECL_FIELD_IS_BASE (member)
+	  && !skip_evaluation)
 	{
 	  warning ("invalid access to non-static data member `%D' of NULL object", 
 		   member);