C++ PATCH: Fix non-virtual destructor bug in new ABI

[Mark Mitchell <mark at codesourcery dot com>]

There was a design bug in the new ABI that I found because it was
related to a GNATS report.  It's important to fix ABI bugs before the
release because after the release we're going to be pretty much stuck
with them.

In the new ABI, we build three variants of a destructor: one that does
not destroy virtual bases, one that does destroy virtual bases, and
one that destroys virtual bases and also calls `operator delete'.
However, it's not safe to build the latter for a non-virtual
destructor because in that case `operator delete' need not be public,
or even unambiguous.  So, the ABI has been changed to eliminate the
deleting destructor variant in this case.

Tested on i686-pc-linux-gnu.

Applied on both the mainline and the branch.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

2001-02-13  Mark Mitchell  <mark@codesourcery.com>

	* cp-tree.h (CLASSTYPE_DESTRUCTORS): Fix typo in comment.
	* call.c (build_op_delete_call): Simplify to remove duplicate
	code.
	* class.c (clone_function_decl): Don't build the deleting variant
	of a non-virtual destructor.
	* decl.c (finish_destructor_body): Don't call delete if this is a
	non-virtual destructor.
	* init.c (build_delete): Explicitly call `operator delete' when
	deleting an object with a non-virtual destructor.
	
Index: cp/call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.255
diff -c -p -r1.255 call.c
*** call.c	2001/02/12 09:58:16	1.255
--- call.c	2001/02/14 06:26:00
*************** build_op_delete_call (code, addr, size, 
*** 3547,3552 ****
--- 3547,3553 ----
       int flags;
  {
    tree fn, fns, fnname, fntype, argtypes, args, type;
+   int pass;
  
    if (addr == error_mark_node)
      return error_mark_node;
*************** build_op_delete_call (code, addr, size, 
*** 3595,3642 ****
        args = NULL_TREE;
      }
  
-   argtypes = tree_cons (NULL_TREE, ptr_type_node, argtypes);
-   fntype = build_function_type (void_type_node, argtypes);
- 
    /* Strip const and volatile from addr.  */
    addr = cp_convert (ptr_type_node, addr);
- 
-   fn = instantiate_type (fntype, fns, itf_no_attributes);
  
!   if (fn != error_mark_node)
      {
!       if (TREE_CODE (fns) == TREE_LIST)
! 	/* Member functions.  */
! 	enforce_access (type, fn);
!       return build_function_call (fn, tree_cons (NULL_TREE, addr, args));
      }
  
    /* If we are doing placement delete we do nothing if we don't find a
       matching op delete.  */
    if (placement)
-     return NULL_TREE;
- 
-   /* Normal delete; now try to find a match including the size argument.  */
-   argtypes = tree_cons (NULL_TREE, ptr_type_node,
- 			tree_cons (NULL_TREE, sizetype, void_list_node));
-   fntype = build_function_type (void_type_node, argtypes);
- 
-   fn = instantiate_type (fntype, fns, itf_no_attributes);
- 
-   if (fn != error_mark_node)
-     {
-       if (BASELINK_P (fns))
- 	/* Member functions.  */
- 	enforce_access (type, fn);
-       return build_function_call
- 	(fn, tree_cons (NULL_TREE, addr,
- 			build_tree_list (NULL_TREE, size)));
-     }
- 
-   /* finish_function passes LOOKUP_SPECULATIVELY if we're in a
-      destructor, in which case the error should be deferred
-      until someone actually tries to delete one of these.  */
-   if (flags & LOOKUP_SPECULATIVELY)
      return NULL_TREE;
  
    cp_error ("no suitable `operator delete' for `%T'", type);
--- 3596,3640 ----
        args = NULL_TREE;
      }
  
    /* Strip const and volatile from addr.  */
    addr = cp_convert (ptr_type_node, addr);
  
!   /* We make two tries at finding a matching `operator delete'.  On
!      the first pass, we look for an one-operator (or placement)
!      operator delete.  If we're not doing placement delete, then on
!      the second pass we look for a two-argument delete.  */
!   for (pass = 0; pass < (placement ? 1 : 2); ++pass) 
      {
!       if (pass == 0)
! 	argtypes = tree_cons (NULL_TREE, ptr_type_node, argtypes);
!       else 
! 	/* Normal delete; now try to find a match including the size
! 	   argument.  */
! 	argtypes = tree_cons (NULL_TREE, ptr_type_node,
! 			      tree_cons (NULL_TREE, sizetype, 
! 					 void_list_node));
! 
!       fntype = build_function_type (void_type_node, argtypes);
!       fn = instantiate_type (fntype, fns, itf_no_attributes);
! 
!       if (fn != error_mark_node)
! 	{
! 	  /* Member functions.  */
! 	  if (BASELINK_P (fns))
! 	    enforce_access (type, fn);
! 
! 	  if (pass == 0)
! 	    args = tree_cons (NULL_TREE, addr, args);
! 	  else
! 	    args = tree_cons (NULL_TREE, addr, 
! 			      build_tree_list (NULL_TREE, size));
! 	  return build_function_call (fn, args);
! 	}
      }
  
    /* If we are doing placement delete we do nothing if we don't find a
       matching op delete.  */
    if (placement)
      return NULL_TREE;
  
    cp_error ("no suitable `operator delete' for `%T'", type);
Index: cp/class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.358
diff -c -p -r1.358 class.c
*** class.c	2001/02/12 09:58:17	1.358
--- class.c	2001/02/14 06:26:05
*************** clone_function_decl (fn, update_method_v
*** 4319,4328 ****
  	 version.  We clone the deleting version first because that
  	 means it will go second on the TYPE_METHODS list -- and that
  	 corresponds to the correct layout order in the virtual
! 	 function table.  */
!       clone = build_clone (fn, deleting_dtor_identifier);
!       if (update_method_vec_p)
! 	add_method (DECL_CONTEXT (clone), clone, /*error_p=*/0);
        clone = build_clone (fn, complete_dtor_identifier);
        if (update_method_vec_p)
  	add_method (DECL_CONTEXT (clone), clone, /*error_p=*/0);
--- 4319,4334 ----
  	 version.  We clone the deleting version first because that
  	 means it will go second on the TYPE_METHODS list -- and that
  	 corresponds to the correct layout order in the virtual
! 	 function table.  
! 
!          For a non-virtual destructor, we do not build a deleting
! 	 destructor.  */
!       if (DECL_VIRTUAL_P (fn))
! 	{
! 	  clone = build_clone (fn, deleting_dtor_identifier);
! 	  if (update_method_vec_p)
! 	    add_method (DECL_CONTEXT (clone), clone, /*error_p=*/0);
! 	}
        clone = build_clone (fn, complete_dtor_identifier);
        if (update_method_vec_p)
  	add_method (DECL_CONTEXT (clone), clone, /*error_p=*/0);
Index: cp/cp-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.572
diff -c -p -r1.572 cp-tree.h
*** cp-tree.h	2001/02/12 09:58:17	1.572
--- cp-tree.h	2001/02/14 06:26:09
*************** struct lang_type
*** 1456,1462 ****
  #define CLASSTYPE_CONSTRUCTORS(NODE) \
    (TREE_VEC_ELT (CLASSTYPE_METHOD_VEC (NODE), CLASSTYPE_CONSTRUCTOR_SLOT))
  
! /* A FUNCTION_DECL for the destructor for NODE.  These are te
     destructors that take an in-charge parameter.  */
  #define CLASSTYPE_DESTRUCTORS(NODE) \
    (TREE_VEC_ELT (CLASSTYPE_METHOD_VEC (NODE), CLASSTYPE_DESTRUCTOR_SLOT))
--- 1456,1462 ----
  #define CLASSTYPE_CONSTRUCTORS(NODE) \
    (TREE_VEC_ELT (CLASSTYPE_METHOD_VEC (NODE), CLASSTYPE_CONSTRUCTOR_SLOT))
  
! /* A FUNCTION_DECL for the destructor for NODE.  These are the
     destructors that take an in-charge parameter.  */
  #define CLASSTYPE_DESTRUCTORS(NODE) \
    (TREE_VEC_ELT (CLASSTYPE_METHOD_VEC (NODE), CLASSTYPE_DESTRUCTOR_SLOT))
Index: cp/decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.747
diff -c -p -r1.747 decl.c
*** decl.c	2001/02/12 09:58:17	1.747
--- decl.c	2001/02/14 06:26:17
*************** static void
*** 13730,13738 ****
  finish_destructor_body ()
  {
    tree compound_stmt;
-   tree virtual_size;
    tree exprstmt;
-   tree if_stmt;
  
    /* Create a block to contain all the extra code.  */
    compound_stmt = begin_compound_stmt (/*has_no_scope=*/0);
--- 13730,13736 ----
*************** finish_destructor_body ()
*** 13814,13844 ****
  	}
      }
  
!   virtual_size = c_sizeof (current_class_type);
  
!   /* At the end, call delete if that's what's requested.  */
  
!   /* FDIS sez: At the point of definition of a virtual destructor
!      (including an implicit definition), non-placement operator delete
!      shall be looked up in the scope of the destructor's class and if
!      found shall be accessible and unambiguous.
! 
!      This is somewhat unclear, but I take it to mean that if the class
!      only defines placement deletes we don't do anything here.  So we
!      pass LOOKUP_SPECULATIVELY; delete_sanity will complain for us if
!      they ever try to delete one of these.  */
!   exprstmt = build_op_delete_call
!     (DELETE_EXPR, current_class_ptr, virtual_size,
!      LOOKUP_NORMAL | LOOKUP_SPECULATIVELY, NULL_TREE);
! 
!   if_stmt = begin_if_stmt ();
!   finish_if_stmt_cond (build (BIT_AND_EXPR, integer_type_node,
! 			      current_in_charge_parm,
! 			      integer_one_node),
! 		       if_stmt);
!   finish_expr_stmt (exprstmt);
!   finish_then_clause (if_stmt);
!   finish_if_stmt ();
  
    /* Close the block we started above.  */
    finish_compound_stmt (/*has_no_scope=*/0, compound_stmt);
--- 13812,13842 ----
  	}
      }
  
!   /* In a virtual destructor, we must call delete.  */
!   if (DECL_VIRTUAL_P (current_function_decl))
!     {
!       tree if_stmt;
!       tree virtual_size = c_sizeof (current_class_type);
  
!       /* [class.dtor]
  
! 	 At the point of definition of a virtual destructor (including
! 	 an implicit definition), non-placement operator delete shall
! 	 be looked up in the scope of the destructor's class and if
! 	 found shall be accessible and unambiguous.  */
!       exprstmt = build_op_delete_call
! 	(DELETE_EXPR, current_class_ptr, virtual_size,
! 	 LOOKUP_NORMAL | LOOKUP_SPECULATIVELY, NULL_TREE);
! 
!       if_stmt = begin_if_stmt ();
!       finish_if_stmt_cond (build (BIT_AND_EXPR, integer_type_node,
! 				  current_in_charge_parm,
! 				  integer_one_node),
! 			   if_stmt);
!       finish_expr_stmt (exprstmt);
!       finish_then_clause (if_stmt);
!       finish_if_stmt ();
!     }
  
    /* Close the block we started above.  */
    finish_compound_stmt (/*has_no_scope=*/0, compound_stmt);
Index: cp/init.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/init.c,v
retrieving revision 1.232
diff -c -p -r1.232 init.c
*** init.c	2001/02/12 09:58:18	1.232
--- init.c	2001/02/14 06:26:23
*************** build_delete (type, addr, auto_delete, f
*** 3235,3246 ****
--- 3235,3264 ----
        tree do_delete = NULL_TREE;
        tree ifexp;
  
+       /* For `::delete x', we must not use the deleting destructor
+ 	 since then we would not be sure to get the global `operator
+ 	 delete'.  */
        if (use_global_delete && auto_delete == sfk_deleting_destructor)
  	{
  	  /* Delete the object. */
  	  do_delete = build_builtin_delete_call (addr);
  	  /* Otherwise, treat this like a complete object destructor
  	     call.  */
+ 	  auto_delete = sfk_complete_destructor;
+ 	}
+       /* If the destructor is non-virtual, there is no deleting
+ 	 variant.  Instead, we must explicitly call the appropriate
+ 	 `operator delete' here.  */
+       else if (!DECL_VIRTUAL_P (CLASSTYPE_DESTRUCTORS (type))
+ 	       && auto_delete == sfk_deleting_destructor)
+ 	{
+ 	  /* Buidl the call.  */
+ 	  do_delete = build_op_delete_call (DELETE_EXPR,
+ 					    addr,
+ 					    c_sizeof_nowarn (type),
+ 					    LOOKUP_NORMAL,
+ 					    NULL_TREE);
+ 	  /* Call the complete object destructor.  */
  	  auto_delete = sfk_complete_destructor;
  	}
  
Index: testsuite/g++.old-deja/g++.oliva/delete1.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.oliva/delete1.C,v
retrieving revision 1.2
diff -c -p -r1.2 delete1.C
*** delete1.C	1999/09/04 15:09:04	1.2
--- delete1.C	2001/02/14 06:26:28
***************
*** 1,6 ****
  // Build don't link:
  
! // Copyright (C) 1999 Free Software Foundation
  
  // by Alexandre Oliva <oliva@dcc.unicamp.br>
  // simplified from bug report by K. Haley <khaley@bigfoot.com>
--- 1,6 ----
  // Build don't link:
  
! // Copyright (C) 1999, 2001 Free Software Foundation
  
  // by Alexandre Oliva <oliva@dcc.unicamp.br>
  // simplified from bug report by K. Haley <khaley@bigfoot.com>
*************** struct bar : foo {
*** 21,27 ****
      delete this; // ERROR - delete is private
      // An implicit invocation of delete is emitted in destructors, but
      // it should only be checked in virtual destructors
!   } // gets bogus error - not virtual - XFAIL *-*-*
  } bar_;
  
  struct baz : foo {
--- 21,27 ----
      delete this; // ERROR - delete is private
      // An implicit invocation of delete is emitted in destructors, but
      // it should only be checked in virtual destructors
!   } // gets bogus error - not virtual
  } bar_;
  
  struct baz : foo {