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]

[C++ PATCH] ctor vtable vcall offsets


Hi,
I've installed the attached patch to both the mainline and 3.0 branch.
We were not generating correct constructor vtable vcall offsets for
class heirarchies with nearly-empty primary virtual bases. If you
called a virtual function in a ctor/dtor that had been overridden from a
virtual base, bad things might have happened, now they don't.


built & tested on i686-pc-linux-gnu, approved by Mark.

nathan
-- 
Dr Nathan Sidwell   ::   http://www.codesourcery.com   ::   CodeSourcery LLC
         'But that's a lie.' - 'Yes it is. What's your point?'
nathan@codesourcery.com : http://www.cs.bris.ac.uk/~nathan/ : nathan@acm.org
2001-02-27  Nathan Sidwell  <nathan@codesourcery.com>

	Fix ctor vtable vcall offsets.
	* class.c (struct vtbl_init_data_s): Add rtti_binfo member.
	(build_rtt_vtbl_entries): Lose RTTI_BINFO parameter.
	(get_matching_base): Remove.
	(get_original_base): New function.
	(build_vtbl_initializer): Initialize vid.rtti_binfo. 
	Use a virtual thunk for a ctor vtable with an index
	(add_vcall_offset_vtbl_entries_1): Check if binfo has lost a
	primary base within a constructor vtable. Only set
	BV_VCALL_INDEX when not a constructor vtable. Adjust vcall offset
	when primary base has been lost.
	* cp-tree.h (BINFO_VIRTUALS): Remove ambiguity from comment.

2001-02-27  Nathan Sidwell  <nathan@codesourcery.com>

	* g++.old-deja/g++.abi/vtable3.h: Check vcall offsets too.

Index: cp/class.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cp/class.c,v
retrieving revision 1.364
diff -c -3 -p -r1.364 class.c
*** class.c	2001/02/21 14:42:23	1.364
--- class.c	2001/02/27 10:31:51
*************** typedef struct vtbl_init_data_s
*** 66,73 ****
  {
    /* The base for which we're building initializers.  */
    tree binfo;
!   /* The binfo for the most-derived type.  */
    tree derived;
    /* The negative-index vtable initializers built up so far.  These
       are in order from least negative index to most negative index.  */
    tree inits;
--- 66,76 ----
  {
    /* The base for which we're building initializers.  */
    tree binfo;
!   /* The type of the most-derived type.  */
    tree derived;
+   /* The binfo for the dynamic type. This will be TYPE_BINFO (derived),
+      unless ctor_vtbl_p is true.  */
+   tree rtti_binfo;
    /* The negative-index vtable initializers built up so far.  These
       are in order from least negative index to most negative index.  */
    tree inits;
*************** static void accumulate_vtbl_inits PARAMS
*** 182,188 ****
  static tree dfs_accumulate_vtbl_inits PARAMS ((tree, tree, tree, tree,
  					       tree));
  static void set_vindex PARAMS ((tree, int *));
! static void build_rtti_vtbl_entries PARAMS ((tree, tree, vtbl_init_data *));
  static void build_vcall_and_vbase_vtbl_entries PARAMS ((tree, 
  							vtbl_init_data *));
  static void force_canonical_binfo_r PARAMS ((tree, tree, tree, tree));
--- 185,191 ----
  static tree dfs_accumulate_vtbl_inits PARAMS ((tree, tree, tree, tree,
  					       tree));
  static void set_vindex PARAMS ((tree, int *));
! static void build_rtti_vtbl_entries PARAMS ((tree, vtbl_init_data *));
  static void build_vcall_and_vbase_vtbl_entries PARAMS ((tree, 
  							vtbl_init_data *));
  static void force_canonical_binfo_r PARAMS ((tree, tree, tree, tree));
*************** static tree *build_vtt_inits PARAMS ((tr
*** 199,205 ****
  static tree dfs_build_secondary_vptr_vtt_inits PARAMS ((tree, void *));
  static tree dfs_ctor_vtable_bases_queue_p PARAMS ((tree, void *data));
  static tree dfs_fixup_binfo_vtbls PARAMS ((tree, void *));
! static tree get_matching_base PARAMS ((tree, tree));
  static tree dfs_get_primary_binfo PARAMS ((tree, void*));
  static int record_subobject_offset PARAMS ((tree, tree, splay_tree));
  static int check_subobject_offset PARAMS ((tree, tree, splay_tree));
--- 202,208 ----
  static tree dfs_build_secondary_vptr_vtt_inits PARAMS ((tree, void *));
  static tree dfs_ctor_vtable_bases_queue_p PARAMS ((tree, void *data));
  static tree dfs_fixup_binfo_vtbls PARAMS ((tree, void *));
! static tree get_original_base PARAMS ((tree, tree));
  static tree dfs_get_primary_binfo PARAMS ((tree, void*));
  static int record_subobject_offset PARAMS ((tree, tree, splay_tree));
  static int check_subobject_offset PARAMS ((tree, tree, splay_tree));
*************** build_vtt (t)
*** 6830,6861 ****
    initialize_array (vtt, inits);
  }
  
! /* The type corresponding to BINFO is a base class of T, but BINFO is
!    in the base class hierarchy of a class derived from T.  Return the
!    base, in T's hierarchy, that corresponds to BINFO.  */
  
  static tree
! get_matching_base (binfo, t)
       tree binfo;
-      tree t;
  {
    tree derived;
!   int i;
! 
!   if (same_type_p (BINFO_TYPE (binfo), t))
      return binfo;
! 
!   if (TREE_VIA_VIRTUAL (binfo))
!     return binfo_for_vbase (BINFO_TYPE (binfo), t);
! 
!   derived = get_matching_base (BINFO_INHERITANCE_CHAIN (binfo), t);
!   for (i = 0; i < BINFO_N_BASETYPES (derived); ++i)
!     if (same_type_p (BINFO_TYPE (BINFO_BASETYPE (derived, i)),
! 		     BINFO_TYPE (binfo)))
!       return BINFO_BASETYPE (derived, i);
! 
!   my_friendly_abort (20000628);
!   return NULL_TREE;
  }
  
  /* Recursively build the VTT-initializer for BINFO (which is in the
--- 6833,6863 ----
    initialize_array (vtt, inits);
  }
  
! /* The type corresponding to BASE_BINFO is a base of the type of BINFO, but
!    from within some heirarchy which is inherited from the type of BINFO.
!    Return BASE_BINFO's equivalent binfo from the hierarchy dominated by
!    BINFO.  */
  
  static tree
! get_original_base (base_binfo, binfo)
!      tree base_binfo;
       tree binfo;
  {
    tree derived;
!   int ix;
!   
!   if (same_type_p (BINFO_TYPE (base_binfo), BINFO_TYPE (binfo)))
      return binfo;
!   if (TREE_VIA_VIRTUAL (base_binfo))
!     return binfo_for_vbase (BINFO_TYPE (base_binfo), BINFO_TYPE (binfo));
!   derived = get_original_base (BINFO_INHERITANCE_CHAIN (base_binfo), binfo);
!   
!   for (ix = 0; ix != BINFO_N_BASETYPES (derived); ix++)
!     if (same_type_p (BINFO_TYPE (base_binfo),
!                      BINFO_TYPE (BINFO_BASETYPE (derived, ix))))
!       return BINFO_BASETYPE (derived, ix);
!   my_friendly_abort (20010223);
!   return NULL;
  }
  
  /* Recursively build the VTT-initializer for BINFO (which is in the
*************** build_vtbl_initializer (binfo, orig_binf
*** 7347,7352 ****
--- 7349,7355 ----
    memset (&vid, 0, sizeof (vid));
    vid.binfo = binfo;
    vid.derived = t;
+   vid.rtti_binfo = rtti_binfo;
    vid.last_init = &vid.inits;
    vid.primary_vtbl_p = (binfo == TYPE_BINFO (t));
    vid.ctor_vtbl_p = !same_type_p (BINFO_TYPE (rtti_binfo), t);
*************** build_vtbl_initializer (binfo, orig_binf
*** 7354,7360 ****
    vid.index = ssize_int (-3);
  
    /* Add entries to the vtable for RTTI.  */
!   build_rtti_vtbl_entries (binfo, rtti_binfo, &vid);
  
    /* Create an array for keeping track of the functions we've
       processed.  When we see multiple functions with the same
--- 7357,7363 ----
    vid.index = ssize_int (-3);
  
    /* Add entries to the vtable for RTTI.  */
!   build_rtti_vtbl_entries (binfo, &vid);
  
    /* Create an array for keeping track of the functions we've
       processed.  When we see multiple functions with the same
*************** build_vtbl_initializer (binfo, orig_binf
*** 7384,7390 ****
        tree fn;
        tree pfn;
        tree init;
! 
        /* Pull the offset for `this', and the function to call, out of
  	 the list.  */
        delta = BV_DELTA (v);
--- 7387,7394 ----
        tree fn;
        tree pfn;
        tree init;
!       int generate_with_vtable_p = BV_GENERATE_THUNK_WITH_VTABLE_P (v);
!       
        /* Pull the offset for `this', and the function to call, out of
  	 the list.  */
        delta = BV_DELTA (v);
*************** build_vtbl_initializer (binfo, orig_binf
*** 7394,7401 ****
  	  vcall_index = BV_VCALL_INDEX (v);
  	  my_friendly_assert (vcall_index != NULL_TREE, 20000621);
  	}
        else
! 	vcall_index = NULL_TREE;
  
        fn = BV_FN (v);
        my_friendly_assert (TREE_CODE (delta) == INTEGER_CST, 19990727);
--- 7398,7414 ----
  	  vcall_index = BV_VCALL_INDEX (v);
  	  my_friendly_assert (vcall_index != NULL_TREE, 20000621);
  	}
+       else if (vid.ctor_vtbl_p && BV_VCALL_INDEX (v))
+         {
+           /* In the original, we did not need to use the vcall index, even
+              though there was one, but in a ctor vtable things might be
+              different (a primary virtual base might have moved). Be
+              conservative and use a vcall adjusting thunk.  */
+ 	  vcall_index = BV_VCALL_INDEX (v);
+           generate_with_vtable_p = 1;
+         }
        else
!         vcall_index = NULL_TREE;
  
        fn = BV_FN (v);
        my_friendly_assert (TREE_CODE (delta) == INTEGER_CST, 19990727);
*************** build_vtbl_initializer (binfo, orig_binf
*** 7413,7419 ****
        TREE_CONSTANT (pfn) = 1;
        /* Enter it in the vtable.  */
        init = build_vtable_entry (delta, vcall_index, pfn,
! 				 BV_GENERATE_THUNK_WITH_VTABLE_P (v));
        /* And add it to the chain of initializers.  */
        vfun_inits = tree_cons (NULL_TREE, init, vfun_inits);
      }
--- 7426,7432 ----
        TREE_CONSTANT (pfn) = 1;
        /* Enter it in the vtable.  */
        init = build_vtable_entry (delta, vcall_index, pfn,
! 				 generate_with_vtable_p);
        /* And add it to the chain of initializers.  */
        vfun_inits = tree_cons (NULL_TREE, init, vfun_inits);
      }
*************** add_vcall_offset_vtbl_entries_1 (binfo, 
*** 7624,7629 ****
--- 7637,7643 ----
    tree base_virtuals;
    tree orig_virtuals;
    tree binfo_inits;
+   int lost_primary = 0;
    /* If BINFO is a primary base, this is the least derived class of
       BINFO that is not a primary base.  */
    tree non_primary_binfo;
*************** add_vcall_offset_vtbl_entries_1 (binfo, 
*** 7645,7650 ****
--- 7659,7679 ----
  	 care about its vtable offsets.  */
        if (TREE_VIA_VIRTUAL (non_primary_binfo))
  	{
+ 	  if (vid->ctor_vtbl_p)
+ 	    {
+     	      tree probe;
+ 	  
+ 	      for (probe = vid->binfo;
+ 	           probe != non_primary_binfo;
+ 	           probe = get_primary_binfo (probe))
+ 	        {
+                   if (BINFO_LOST_PRIMARY_P (probe))
+                     {
+                       lost_primary = 1;
+                       break;
+                     }
+ 	        }
+             }
  	  non_primary_binfo = vid->binfo;
  	  break;
  	}
*************** add_vcall_offset_vtbl_entries_1 (binfo, 
*** 7655,7660 ****
--- 7684,7695 ----
        non_primary_binfo = b;
      }
  
+   if (vid->ctor_vtbl_p)
+     /* For a ctor vtable we need the equivalent binfo within the hierarchy
+        where rtti_binfo is the most derived type.  */
+     non_primary_binfo = get_original_base
+           (non_primary_binfo, TYPE_BINFO (BINFO_TYPE (vid->rtti_binfo)));
+ 
    /* Make entries for the rest of the virtuals.  */
    for (base_virtuals = BINFO_VIRTUALS (binfo),
  	 derived_virtuals = BINFO_VIRTUALS (non_primary_binfo),
*************** add_vcall_offset_vtbl_entries_1 (binfo, 
*** 7669,7674 ****
--- 7704,7710 ----
        tree base;
        tree base_binfo;
        size_t i;
+       tree vcall_offset;
  
        /* Find the declaration that originally caused this function to
  	 be present.  */
*************** add_vcall_offset_vtbl_entries_1 (binfo, 
*** 7699,7706 ****
  	      || (DECL_DESTRUCTOR_P (BV_FN (derived_entry))
  		  && DECL_DESTRUCTOR_P (fn)))
  	    {
! 	      BV_VCALL_INDEX (derived_virtuals) 
! 		= BV_VCALL_INDEX (derived_entry);
  	      break;
  	    }
  	}
--- 7735,7743 ----
  	      || (DECL_DESTRUCTOR_P (BV_FN (derived_entry))
  		  && DECL_DESTRUCTOR_P (fn)))
  	    {
! 	      if (!vid->ctor_vtbl_p)
!   	        BV_VCALL_INDEX (derived_virtuals) 
! 		  = BV_VCALL_INDEX (derived_entry);
  	      break;
  	    }
  	}
*************** add_vcall_offset_vtbl_entries_1 (binfo, 
*** 7713,7724 ****
        base_binfo = get_binfo (base, vid->derived, /*protect=*/0);
  
        /* Compute the vcall offset.  */
!       *vid->last_init 
! 	= (build_tree_list 
! 	   (NULL_TREE,
! 	    fold (build1 (NOP_EXPR, vtable_entry_type,
! 			  size_diffop (BINFO_OFFSET (base_binfo),
! 				       BINFO_OFFSET (vid->vbase))))));
        vid->last_init = &TREE_CHAIN (*vid->last_init);
  
        /* Keep track of the vtable index where this vcall offset can be
--- 7750,7765 ----
        base_binfo = get_binfo (base, vid->derived, /*protect=*/0);
  
        /* Compute the vcall offset.  */
!       vcall_offset = BINFO_OFFSET (vid->vbase);
!       if (lost_primary)
!         vcall_offset = size_binop (PLUS_EXPR, vcall_offset,
!                                    BINFO_OFFSET (vid->binfo));
!       vcall_offset = size_diffop (BINFO_OFFSET (base_binfo),
! 		                  vcall_offset);
!       vcall_offset = fold (build1 (NOP_EXPR, vtable_entry_type,
!     			           vcall_offset));
!       
!       *vid->last_init = build_tree_list (NULL_TREE, vcall_offset);
        vid->last_init = &TREE_CHAIN (*vid->last_init);
  
        /* Keep track of the vtable index where this vcall offset can be
*************** add_vcall_offset_vtbl_entries_1 (binfo, 
*** 7738,7749 ****
  
  /* Return vtbl initializers for the RTTI entries coresponding to the
     BINFO's vtable.  The RTTI entries should indicate the object given
!    by RTTI_BINFO.  */
  
  static void
! build_rtti_vtbl_entries (binfo, rtti_binfo, vid)
       tree binfo;
-      tree rtti_binfo;
       vtbl_init_data *vid;
  {
    tree b;
--- 7779,7789 ----
  
  /* Return vtbl initializers for the RTTI entries coresponding to the
     BINFO's vtable.  The RTTI entries should indicate the object given
!    by VID->rtti_binfo.  */
  
  static void
! build_rtti_vtbl_entries (binfo, vid)
       tree binfo;
       vtbl_init_data *vid;
  {
    tree b;
*************** build_rtti_vtbl_entries (binfo, rtti_bin
*** 7754,7760 ****
    tree init;
  
    basetype = BINFO_TYPE (binfo);
!   t = BINFO_TYPE (rtti_binfo);
  
    /* For a COM object there is no RTTI entry.  */
    if (CLASSTYPE_COM_INTERFACE (basetype))
--- 7794,7800 ----
    tree init;
  
    basetype = BINFO_TYPE (binfo);
!   t = BINFO_TYPE (vid->rtti_binfo);
  
    /* For a COM object there is no RTTI entry.  */
    if (CLASSTYPE_COM_INTERFACE (basetype))
*************** build_rtti_vtbl_entries (binfo, rtti_bin
*** 7772,7778 ****
        my_friendly_assert (BINFO_PRIMARY_BASE_OF (primary_base) == b, 20010127);
        b = primary_base;
      }
!   offset = size_diffop (BINFO_OFFSET (rtti_binfo), BINFO_OFFSET (b));
  
    /* The second entry is the address of the typeinfo object.  */
    if (flag_rtti)
--- 7812,7818 ----
        my_friendly_assert (BINFO_PRIMARY_BASE_OF (primary_base) == b, 20010127);
        b = primary_base;
      }
!   offset = size_diffop (BINFO_OFFSET (vid->rtti_binfo), BINFO_OFFSET (b));
  
    /* The second entry is the address of the typeinfo object.  */
    if (flag_rtti)
*************** build_rtti_vtbl_entries (binfo, rtti_bin
*** 7802,7808 ****
  
  /* Build an entry in the virtual function table.  DELTA is the offset
     for the `this' pointer.  VCALL_INDEX is the vtable index containing
!    the vcall offset; zero if none.  ENTRY is the virtual function
     table entry itself.  It's TREE_TYPE must be VFUNC_PTR_TYPE_NODE,
     but it may not actually be a virtual function table pointer.  (For
     example, it might be the address of the RTTI object, under the new
--- 7842,7848 ----
  
  /* Build an entry in the virtual function table.  DELTA is the offset
     for the `this' pointer.  VCALL_INDEX is the vtable index containing
!    the vcall offset; NULL_TREE if none.  ENTRY is the virtual function
     table entry itself.  It's TREE_TYPE must be VFUNC_PTR_TYPE_NODE,
     but it may not actually be a virtual function table pointer.  (For
     example, it might be the address of the RTTI object, under the new
Index: cp/cp-tree.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cp/cp-tree.h,v
retrieving revision 1.581
diff -c -3 -p -r1.581 cp-tree.h
*** cp-tree.h	2001/02/22 11:46:32	1.581
--- cp-tree.h	2001/02/27 10:31:54
*************** Boston, MA 02111-1307, USA.  */
*** 136,142 ****
       does not have a BV_FN; it is just an offset.
  
       The BV_OVERRIDING_BASE is the binfo for the final overrider for
!      this function.  (This binfo's BINFO_TYPE will always be the same
       as the DECL_CLASS_CONTEXT for the function.)
  
     BINFO_VTABLE
--- 136,142 ----
       does not have a BV_FN; it is just an offset.
  
       The BV_OVERRIDING_BASE is the binfo for the final overrider for
!      this function.  (That binfo's BINFO_TYPE will always be the same
       as the DECL_CLASS_CONTEXT for the function.)
  
     BINFO_VTABLE
Index: testsuite/g++.old-deja/g++.abi/vtable3.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/testsuite/g++.old-deja/g++.abi/vtable3.h,v
retrieving revision 1.1
diff -c -3 -p -r1.1 vtable3.h
*** vtable3.h	2001/02/05 11:45:16	1.1
--- vtable3.h	2001/02/27 10:31:54
***************
*** 7,17 ****
  #include <typeinfo>
  #include <stdio.h>
  
- // XXX. vcall offsets are still broken, remove this define to re-enable
- // testing when fixed.
- #define NO_VCALL_TEST
- 
  int fail;
  
  template <typename BASE, typename DERIVED>
  int Test (DERIVED *d, int expect)
--- 7,14 ----
  #include <typeinfo>
  #include <stdio.h>
  
  int fail;
+ struct A;
  
  template <typename BASE, typename DERIVED>
  int Test (DERIVED *d, int expect)
*************** int Test (DERIVED *d, int expect)
*** 19,24 ****
--- 16,22 ----
    BASE *b = static_cast <BASE *> (d);
    void *full_b = dynamic_cast <void *> (b);
    void *full_d = dynamic_cast <void *> (d);
+   A *ap = static_cast <A *> (b);
    
    if (full_b != full_d)
      {
*************** int Test (DERIVED *d, int expect)
*** 37,44 ****
                typeid (BASE).name (), typeid (DERIVED).name ());
        return 1;
      }
! #ifndef NO_VCALL_TEST
!   b->Baz (static_cast <void *> (b));
    
    int res = b->Foo (static_cast <void *> (d));
    
--- 35,42 ----
                typeid (BASE).name (), typeid (DERIVED).name ());
        return 1;
      }
! 
!   b->Baz (static_cast <void *> (ap));
    
    int res = b->Foo (static_cast <void *> (d));
    
*************** int Test (DERIVED *d, int expect)
*** 49,55 ****
                typeid (BASE).name (), res, expect);
        return 1;
      }
! #endif
    return 0;
  }
  
--- 47,53 ----
                typeid (BASE).name (), res, expect);
        return 1;
      }
! 
    return 0;
  }
  

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