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]

[PATCH] Fix 19891


Hi,

this patch fixes 19891, a palandromic regression with covariancy.  It
triggers a case where we would recursively call dfs_walk_once, which is
a no-no.  It must have started when I replaced a dfs_walk with
a dfs_walk_once call for vtable construction.  As well as the recursive
call in update_vtable_entry_for_fn, the call of build_class_member_access_expr
in build_simple_base_path causes a recursive call to check for the
legality of the member access.  In this particular case such a check is
unneeded, because we know the access is valid.  I copied the relevant parts
of building a member access into build_simple_base_path --- refactoring
into a worker function did not seem profitable.

It is unfortunately lame that we have to redo the base path search
in update_vtable_entry_for_fn, and lame that we have to use
unary_complex_lvalue too.  But those are more involved issues.

booted & tested on i686-pc-linux-gnu, installed.

nathan
--
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

2005-02-11  Nathan Sidwell  <nathan@codesourcery.com>

	PR c++/19891
	* class.c (build_simple_base_path): Build the component_ref
	directly.
	(update_vtable_entry_for_fn): Walk the covariant's binfo chain
	rather than using lookup_base.
	* search.c (dfs_walk_once): Add non-recursive assert check.
	* typeck.c (build_class_member_access_expr): It is possible for
	the member type to be both const and volatile.

Index: cp/class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.703
diff -c -3 -p -r1.703 class.c
*** cp/class.c	9 Feb 2005 02:53:35 -0000	1.703
--- cp/class.c	11 Feb 2005 16:35:32 -0000
*************** build_simple_base_path (tree expr, tree 
*** 408,414 ****
--- 408,425 ----
  
    if (d_binfo == NULL_TREE)
      {
+       tree temp;
+       
        gcc_assert (TYPE_MAIN_VARIANT (TREE_TYPE (expr)) == type);
+       
+       /* Transform `(a, b).x' into `(*(a, &b)).x', `(a ? b : c).x'
+      	 into `(*(a ?  &b : &c)).x', and so on.  A COND_EXPR is only
+      	 an lvalue in the frontend; only _DECLs and _REFs are lvalues
+      	 in the backend.  */
+       temp = unary_complex_lvalue (ADDR_EXPR, expr);
+       if (temp)
+ 	expr = build_indirect_ref (temp, NULL);
+ 
        return expr;
      }
  
*************** build_simple_base_path (tree expr, tree 
*** 421,428 ****
      if (TREE_CODE (field) == FIELD_DECL
  	&& DECL_FIELD_IS_BASE (field)
  	&& TREE_TYPE (field) == type)
!       return build_class_member_access_expr (expr, field,
! 					     NULL_TREE, false);
  
    /* Didn't find the base field?!?  */
    gcc_unreachable ();
--- 432,458 ----
      if (TREE_CODE (field) == FIELD_DECL
  	&& DECL_FIELD_IS_BASE (field)
  	&& TREE_TYPE (field) == type)
!       {
! 	/* We don't use build_class_member_access_expr here, as that
! 	   has unnecessary checks, and more importantly results in
! 	   recursive calls to dfs_walk_once.  */
! 	int type_quals = cp_type_quals (TREE_TYPE (expr));
! 
! 	expr = build3 (COMPONENT_REF,
! 		       cp_build_qualified_type (type, type_quals),
! 		       expr, field, NULL_TREE);
! 	expr = fold_if_not_in_template (expr);
! 	
! 	/* Mark the expression const or volatile, as appropriate.
! 	   Even though we've dealt with the type above, we still have
! 	   to mark the expression itself.  */
! 	if (type_quals & TYPE_QUAL_CONST)
! 	  TREE_READONLY (expr) = 1;
! 	if (type_quals & TYPE_QUAL_VOLATILE)
! 	  TREE_THIS_VOLATILE (expr) = 1;
! 	
! 	return expr;
!       }
  
    /* Didn't find the base field?!?  */
    gcc_unreachable ();
*************** update_vtable_entry_for_fn (tree t, tree
*** 1996,2001 ****
--- 2026,2034 ----
           also be converting to the return type of FN, we have to
           combine the two conversions here.  */
        tree fixed_offset, virtual_offset;
+ 
+       over_return = TREE_TYPE (over_return);
+       base_return = TREE_TYPE (base_return);
        
        if (DECL_THUNK_P (fn))
  	{
*************** update_vtable_entry_for_fn (tree t, tree
*** 2011,2042 ****
  	   overriding function. We will want the vbase offset from
  	   there.  */
  	virtual_offset = binfo_for_vbase (BINFO_TYPE (virtual_offset),
! 					  TREE_TYPE (over_return));
!       else if (!same_type_p (TREE_TYPE (over_return),
! 			     TREE_TYPE (base_return)))
  	{
  	  /* There was no existing virtual thunk (which takes
! 	     precedence).  */
! 	  tree thunk_binfo;
! 	  base_kind kind;
! 	  
! 	  thunk_binfo = lookup_base (TREE_TYPE (over_return),
! 				     TREE_TYPE (base_return),
! 				     ba_check | ba_quiet, &kind);
  
! 	  if (thunk_binfo && (kind == bk_via_virtual
! 			      || !BINFO_OFFSET_ZEROP (thunk_binfo)))
  	    {
  	      tree offset = convert (ssizetype, BINFO_OFFSET (thunk_binfo));
  
! 	      if (kind == bk_via_virtual)
  		{
! 		  /* We convert via virtual base. Find the virtual
! 		     base and adjust the fixed offset to be from there.  */
! 		  while (!BINFO_VIRTUAL_P (thunk_binfo))
! 		    thunk_binfo = BINFO_INHERITANCE_CHAIN (thunk_binfo);
! 
! 		  virtual_offset = thunk_binfo;
  		  offset = size_diffop
  		    (offset, convert
  		     (ssizetype, BINFO_OFFSET (virtual_offset)));
--- 2044,2090 ----
  	   overriding function. We will want the vbase offset from
  	   there.  */
  	virtual_offset = binfo_for_vbase (BINFO_TYPE (virtual_offset),
! 					  over_return);
!       else if (!same_type_ignoring_top_level_qualifiers_p
! 	       (over_return, base_return))
  	{
  	  /* There was no existing virtual thunk (which takes
! 	     precedence).  So find the binfo of the base function's
! 	     return type within the overriding function's return type.
! 	     We cannot call lookup base here, because we're inside a
! 	     dfs_walk, and will therefore clobber the BINFO_MARKED
! 	     flags.  Fortunately we know the covariancy is valid (it
! 	     has already been checked), so we can just iterate along
! 	     the binfos, which have been chained in inheritance graph
! 	     order.  Of course it is lame that we have to repeat the
! 	     search here anyway -- we should really be caching pieces
! 	     of the vtable and avoiding this repeated work.  */
! 	  tree thunk_binfo, base_binfo;
! 
! 	  /* Find the base binfo within the overriding function's
! 	     return type.  */
! 	  for (base_binfo = TYPE_BINFO (base_return),
! 	       thunk_binfo = TYPE_BINFO (over_return);
! 	       !SAME_BINFO_TYPE_P (BINFO_TYPE (thunk_binfo),
! 				   BINFO_TYPE (base_binfo));
! 	       thunk_binfo = TREE_CHAIN (thunk_binfo))
! 	    continue;
  
! 	  /* See if virtual inheritance is involved.  */
! 	  for (virtual_offset = thunk_binfo;
! 	       virtual_offset;
! 	       virtual_offset = BINFO_INHERITANCE_CHAIN (virtual_offset))
! 	    if (BINFO_VIRTUAL_P (virtual_offset))
! 	      break;
! 	  
! 	  if (virtual_offset || !BINFO_OFFSET_ZEROP (thunk_binfo))
  	    {
  	      tree offset = convert (ssizetype, BINFO_OFFSET (thunk_binfo));
  
! 	      if (virtual_offset)
  		{
! 		  /* We convert via virtual base.  Adjust the fixed
! 		     offset to be from there.  */
  		  offset = size_diffop
  		    (offset, convert
  		     (ssizetype, BINFO_OFFSET (virtual_offset)));
Index: cp/search.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/search.c,v
retrieving revision 1.344
diff -c -3 -p -r1.344 search.c
*** cp/search.c	9 Feb 2005 02:53:40 -0000	1.344
--- cp/search.c	11 Feb 2005 16:35:37 -0000
*************** tree
*** 1639,1647 ****
--- 1639,1650 ----
  dfs_walk_once (tree binfo, tree (*pre_fn) (tree, void *),
  	       tree (*post_fn) (tree, void *), void *data)
  {
+   static int active = 0;  /* We must not be called recursively. */
    tree rval;
  
    gcc_assert (pre_fn || post_fn);
+   gcc_assert (!active);
+   active++;
    
    if (!CLASSTYPE_DIAMOND_SHAPED_P (BINFO_TYPE (binfo)))
      /* We are not diamond shaped, and therefore cannot encounter the
*************** dfs_walk_once (tree binfo, tree (*pre_fn
*** 1666,1671 ****
--- 1669,1677 ----
        else
  	dfs_unmark_r (binfo);
      }
+ 
+   active--;
+   
    return rval;
  }
  
Index: cp/typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck.c,v
retrieving revision 1.613
diff -c -3 -p -r1.613 typeck.c
*** cp/typeck.c	9 Feb 2005 02:53:41 -0000	1.613
--- cp/typeck.c	11 Feb 2005 16:35:51 -0000
*************** build_class_member_access_expr (tree obj
*** 1750,1756 ****
  	 expression itself.  */
        if (type_quals & TYPE_QUAL_CONST)
  	TREE_READONLY (result) = 1;
!       else if (type_quals & TYPE_QUAL_VOLATILE)
  	TREE_THIS_VOLATILE (result) = 1;
      }
    else if (BASELINK_P (member))
--- 1750,1756 ----
  	 expression itself.  */
        if (type_quals & TYPE_QUAL_CONST)
  	TREE_READONLY (result) = 1;
!       if (type_quals & TYPE_QUAL_VOLATILE)
  	TREE_THIS_VOLATILE (result) = 1;
      }
    else if (BASELINK_P (member))
// { dg-do run  }

// Copyright (C) 2005 Free Software Foundation, Inc.
// Contributed by Nathan Sidwell 11 Feb 2005 <nathan@codesourcery.com>

// Origin: bredelin@ucla.edu
// Bug 19891: Incorrect covariant vtables

struct Model {
  bool full_tree;
  virtual Model* clone() const =0;
  virtual const char *name() const =0;
  virtual ~Model() {}
};

struct R: virtual public Model {
  virtual R* clone() const =0;
};
struct A: virtual public Model {
  virtual A* clone() const=0;
};
struct RA: public R, public A {
  virtual RA* clone() const=0;
};

static const char *string = "EQU";

struct EQU: public RA {
  virtual EQU* clone() const {return new EQU(*this);}
  const char *name() const {return string;}
};

int main() {
  Model* M1 = new EQU();
  Model* M2 = M1->clone();
  Model* M3 = M2->clone();

  if (M1->name () != string)
    return 1;
  if (M2->name () != string)
    return 2;
  if (M3->name () != string)
    return 3;
  
  return 0;
}

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