C++ PATCH: throwing (T *)NULL, and ambiguous bases

Nathan Sidwell nathan@acm.org
Tue Jun 8 11:51:00 GMT 1999


Hi,
I've analysed the remaining failing testcases Robert Lipe (eh990323-[234].C)
recently added and fell into a bit of a hole. This generated another 8
testcases of my own (attached), which we also fail. The attached patch fixes
all these and Robert's remaining cases (hurrah!). The patch affects both throw
and dynamic_cast and is confined to the C++ runtime support.

The core of the problem is in __class_type_info's dcast method, it has the
following flaws

1) it returns NULL to indicate no base was found
2) it returns NULL to indicate that an ambiguous base was found
3) if needs_public is true, it only looks in the public bases
4) if passed a NULL object ptr, it proceeds to adjust it, thereby generating a
non-null ptr.
5) if passed a NULL object ptr with a virtual base it proceeds to indirect (and
segfaults).

1 and 2 conspire such that (eg) a tripply ambiguous base will be accepted if
two of the ambiguities were down one branch of the class DAG -- this returns
NULL, and is taken to mean no base. Thus the third base is found and considered
unambiguous.
3 conspires to make access and ambiguity non-orthogonal (contrary to 11/3). It
needs to look for an unambiguous public base, which means find an unambiguous
base and check that it is public.
4 and 5 prevent throwing (T *)NULL, which is permitted by the standard.

dcast needs to return more information than NULL/non-NULL. I have fixed this by
moving dcast's functionality to do_dcast (which is virtual) and having a
non-virtual dcast stub to dispatch to do_dcast and validate return values. The
new dcast has an additional pointer parameter `bool *ok' which is filled in to
indicate that a valid conversion has/has not been found. The existing return
value of the adjusted pointer can also be used to detect valid conversions,
provided NULL is never supplied as an object to convert.

do_dcast has two additional parameters, `type_info **basetype' and `bool
*is_public'. The latter is filled in with the access information to the located
base type. The former indicates what has been found. The return type is the
adjusted pointer (which will be NULL on failure, or if given a NULL object
pointer). BASETYPE will have the following values,
- NULL if no conversion has been found.
- ambig_base, if an ambiguous base has been found
- valid_base, if a non-ambiguous base has been found via a non-virtual path
- otherwise it is the __type_info for the most derrived non-virtual descendant
of the base within the virtual path containing the unambiguous base.
(ambig_base and valid_base are enumeration constants.)

__class_type_info::do_dcast makes use of these return values to detect
ambiguity. It needs the final case in order to detect whether a base from a
NULL ptr is ambiguous or not. With a non-NULL ptr it can just compare adjusted
pointers and be done with it. For the NULL case, all the located bases will be
NULL, so that doesn't work. However, if we know the type at which the virtual
DAG started above the located base, we can determine ambiguity. If these are
the same, it is unambiguous, otherwise it is ambiguous.

The testcases nathan24.C-nathan31.C all deal with thowing and catching various
class heirarchies. testcase nathan32.C deals with a dynamic_cast which we
currently fail to detect as ambiguous.

__throw_type_match_rtti is adjusted so we can accept a NULL pointer. In that
case we return (void *)1.
__cplus_type_matcher is adjusted to never accidentally return a NULL pointer
when we mean OK. We return the INFO pointer not the object pointer. Also check
__throw_type_match's return value to deal with the NULL pointer wart.

The above mechanism relies on (void *)1 and (void *)2 being distinguishable
from all possible object pointers. I avoided changing function signatures to
remain API compatible across the library's external interface. The changed
function, dcast, is (I believe) called only from within libgcc2, of which it is
a part. The additional out-of-band return from __throw_type_match_rtti is a
case which we currently fail.

ok to install? (Naturally I'll change the names of the test cases
appropriately).

nathan
-- 
Dr Nathan Sidwell :: Computer Science Department :: Bristol University
        I have seen the death of PhotoShop -- it is called GIMP
nathan@acm.org  http://www.cs.bris.ac.uk/~nathan/  nathan@cs.bris.ac.uk
1999-06-08  Nathan Sidwell  <nathan@acm.org>

	* tinfo.h (valid_base, ambig_base): New enumeration flags.
	(dcast): Add OK parameter.
	(do_dcast): Move functionality from dcast. Alter parameters.
	* tinfo.cc (dcast): Dispatch to d_dcast.
	(do_dcast): Move old dcast functionality here and adjust.
	Reimplement arbitrary class searching to deal with NULL
	conversions and correctly detect ambiguity.
	* tinfo2.cc (__throw_type_match_rtti): Deal with accepting a
	NULL pointer.
	* exception.cc (__cplus_type_matcher): Likewise.

Index: egcs/gcc/cp/exception.cc
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/cp/exception.cc,v
retrieving revision 1.23
diff -c -3 -p -r1.23 exception.cc
*** exception.cc	1998/12/16 21:15:22	1.23
--- exception.cc	1999/06/08 17:46:11
*************** __eh_free (void *p)
*** 168,173 ****
--- 168,180 ----
  
  typedef void * (* rtimetype) (void);
  
+ /* Check a thrown type against what we're prepared to accept. Return
+    non-NULL if we should catch it. Return NULL if we don't want it.
+    If we do take it, adjust any object pointer to the expected type.
+    Remember C++ allows you to throw (T *)NULL. __throw_type_match_rtti
+    catches that and returns (void *)1, we need to spot that. We also need
+    to make sure we never return NULL, when we want the exception.  */
+ 
  extern "C" void *
  __cplus_type_matcher (cp_eh_info *info, rtimetype match_info, 
                                   exception_descriptor *exception_table)
*************** __cplus_type_matcher (cp_eh_info *info, 
*** 180,186 ****
      return NULL;
  
    if (match_info == CATCH_ALL_TYPE)
!     return info->value;
  
    /* we don't worry about version info yet, there is only one version! */
    
--- 187,193 ----
      return NULL;
  
    if (match_info == CATCH_ALL_TYPE)
!     return info;
  
    /* we don't worry about version info yet, there is only one version! */
    
*************** __cplus_type_matcher (cp_eh_info *info, 
*** 188,195 ****
    ret = __throw_type_match_rtti (match_type, info->type, info->original_value);
    /* change value of exception */
    if (ret)
!     info->value = ret;
!   return ret;
  }
  
  
--- 195,205 ----
    ret = __throw_type_match_rtti (match_type, info->type, info->original_value);
    /* change value of exception */
    if (ret)
!     {
!       info->value = ret == (void *)1 ? NULL : ret;
!       return info;
!     }
!   return NULL;
  }
  
  
Index: egcs/gcc/cp/tinfo.cc
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/cp/tinfo.cc,v
retrieving revision 1.10
diff -c -3 -p -r1.10 tinfo.cc
*** tinfo.cc	1999/05/02 22:44:24	1.10
--- tinfo.cc	1999/06/08 17:46:14
*************** __rtti_user (void *addr, const char *nam
*** 63,105 ****
  { new (addr) __user_type_info (name); }
  
  // dynamic_cast helper methods.
- // Returns a pointer to the desired sub-object or 0.
  
  void * __user_type_info::
! dcast (const type_info& to, int, void *addr, const type_info *, void *) const
! { return (*this == to) ? addr : 0; }
  
  void * __si_type_info::
! dcast (const type_info& to, int require_public, void *addr,
!        const type_info *sub, void *subptr) const
  {
    if (*this == to)
!     return addr;
!   return base.dcast (to, require_public, addr, sub, subptr);
  }
  
! void* __class_type_info::
! dcast (const type_info& desired, int is_public, void *objptr,
!        const type_info *sub, void *subptr) const
  {
    if (*this == desired)
!     return objptr;
  
    void *match_found = 0;
    for (size_t i = 0; i < n_bases; i++)
      {
!       if (is_public && base_list[i].access != PUBLIC)
! 	continue;
! 
!       void *p = (char *)objptr + base_list[i].offset;
!       if (base_list[i].is_virtual)
! 	p = *(void **)p;
!       p = base_list[i].base->dcast (desired, is_public, p, sub, subptr);
        if (p)
  	{
! 	  if (match_found == 0)
! 	    match_found = p;
! 	  else if (match_found != p)
  	    {
  	      if (sub)
  		{
--- 63,179 ----
  { new (addr) __user_type_info (name); }
  
  // dynamic_cast helper methods.
  
+ void *__user_type_info::
+ dcast (const type_info &target, int need_public, void *objptr,
+        const type_info *sub = 0, void *subptr = 0, bool *ok = 0) const
+ {
+   const std::type_info *type;
+   bool is_public;
+   
+   void *adj = do_dcast(target, objptr, sub, subptr, &type, &is_public);
+   bool is_ok = (type && type != (const void *)ambig_base
+                 && (is_public || !need_public));
+   if (!is_ok)
+     adj = NULL;
+   if (ok)
+     *ok = is_ok;
+   return adj;
+ }
+ 
  void * __user_type_info::
! do_dcast (const type_info& to, void *addr,
!           const type_info *, void *,
!           const type_info **basetype, bool *is_public) const
! {
!   *is_public = true;
!   if (*this == to)
!     {
!       *basetype = (const type_info *)(const void *)valid_base;
!       return addr;
!     }
!   *basetype = 0;
!   return 0;
! }
  
  void * __si_type_info::
! do_dcast (const type_info& to, void *addr,
!           const type_info *sub, void *subptr,
!           const type_info **basetype, bool *is_public) const
  {
+   *is_public = true;
    if (*this == to)
!     {
!       *basetype = (const type_info *)(const void *)valid_base;
!       return addr;
!     }
!   return base.do_dcast (to, addr, sub, subptr, basetype, is_public);
  }
  
! void * __class_type_info::
! do_dcast (const type_info& desired, void *objptr,
!           const type_info *sub, void *subptr,
!           const type_info **basetype, bool *is_public) const
  {
+   *is_public = true;
    if (*this == desired)
!     {
!       *basetype = (const type_info *)(const void *)valid_base;
!       return objptr;
!     }
  
    void *match_found = 0;
+   const type_info *found_type = 0;
+   bool public_p = false;
+   
    for (size_t i = 0; i < n_bases; i++)
      {
!       void *p = objptr;
        if (p)
+         {
+           p = (char *)p + base_list[i].offset;
+           if (base_list[i].is_virtual)
+   	    p = *(void **)p;
+         }
+       bool base_pub;
+       const type_info *base_flag;
+       p = base_list[i].base->do_dcast (desired, p, sub, subptr,
+                                        &base_flag, &base_pub);
+       if (base_flag)
  	{
!           if (base_flag == (const void *)ambig_base)
!             {
!               found_type = base_flag;
!               match_found = NULL;
!               break;
!             }
!       
!           if (base_flag == (const void *)valid_base && base_list[i].is_virtual)
!             base_flag = base_list[i].base;
!           if (base_list[i].access != PUBLIC)
!             base_pub = false;
!       
! 	  if (!found_type)
! 	    {
! 	      match_found = p;
! 	      found_type = base_flag;
! 	      public_p = base_pub;
! 	    }
! 	  else if (match_found == p)
! 	    {
! 	      // found at same address -- if p is NULL, we need to be carefull
!               // For non-NULL p, this must be ok.
! 	      if (!p && (base_flag == (const void *)valid_base
! 	                 || found_type == (const void *)valid_base
! 	                 || !(*base_flag == *found_type)))
! 	        {
!                   found_type = (const type_info *)(const void *)ambig_base;
!                   match_found = 0;
!                   break;
! 	        }
! 	      public_p |= base_pub;
! 	    }
! 	  else
  	    {
  	      if (sub)
  		{
*************** dcast (const type_info& desired, int is_
*** 108,116 ****
  
  		  const __user_type_info &d =
  		    static_cast <const __user_type_info &> (desired);
  
! 		  void *os = d.dcast (*sub, 1, match_found);
! 		  void *ns = d.dcast (*sub, 1, p);
  
  		  if (os == ns)
  		    /* ambiguous -- subptr is a virtual base */;
--- 182,192 ----
  
  		  const __user_type_info &d =
  		    static_cast <const __user_type_info &> (desired);
+ 	          bool pub;
+ 	          const type_info *ot;
  
! 		  void *os = d.do_dcast (*sub, match_found, NULL, NULL, &ot, &pub);
! 		  void *ns = d.do_dcast (*sub, p, NULL, NULL, &ot, &pub);
  
  		  if (os == ns)
  		    /* ambiguous -- subptr is a virtual base */;
*************** dcast (const type_info& desired, int is_
*** 119,134 ****
--- 195,215 ----
  		  else if (ns == subptr)
  		    {
  		      match_found = p;
+ 	              public_p = base_pub;
+ 	              found_type = base_flag;
  		      continue;
  		    }
  		}
  
  	      // base found at two different pointers,
  	      // conversion is not unique
+ 	      *basetype = (const type_info *)(const void *)ambig_base;
  	      return 0;
  	    }
  	}
      }
  
+   *basetype = found_type;
+   *is_public = public_p;
    return match_found;
  }
Index: egcs/gcc/cp/tinfo.h
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/cp/tinfo.h,v
retrieving revision 1.5
diff -c -3 -p -r1.5 tinfo.h
*** tinfo.h	1999/04/02 15:35:57	1.5
--- tinfo.h	1999/06/08 17:46:14
***************
*** 8,19 ****
  // type_info for a class with no base classes (or an enum).
  
  struct __user_type_info : public std::type_info {
    __user_type_info (const char *n) : type_info (n) {}
  
!   // If our type can be converted to the desired type, 
!   // return the pointer, adjusted accordingly; else return 0.
!   virtual void* dcast (const type_info &, int, void *,
! 		       const type_info * = 0, void * = 0) const;
  };
  
  // type_info for a class with one public, nonvirtual base class.
--- 8,44 ----
  // type_info for a class with no base classes (or an enum).
  
  struct __user_type_info : public std::type_info {
+   public:
    __user_type_info (const char *n) : type_info (n) {}
  
!   protected:
!   // Indicate type of base found -- see do_dcast
!   enum
!   {
!     valid_base = 1,
!     ambig_base = 2
!   };
!     
!   public:
!   // If our type can be converted to the desired type, return the pointer,
!   // adjusted accordingly, otherwise return NULL. Set OK, if non-NULL, to
!   // indicate that we found a valid convertion.  OBJPTR can be NULL, in which
!   // case NULL will be returned and OK must be used to detect that this is
!   // valid.
!   void* dcast (const type_info &target, int need_public, void *objptr,
! 	       const type_info *sub = 0, void *subptr = 0, bool *ok = 0) const;
!   
!   public:
!   // Worker function for dcast. BASETYPE is used to indicate,
!   //   an ambiguity is found (ambig_base)
!   //   found in a virtual base (__user_type_info of vbase)
!   //   found as non-virtual base (valid_base)
!   //   not found (NULL)
!   // IS_PUBLIC is used to indicate we found it with public access.
!   // Returns adjusted pointer.
!   virtual void* do_dcast (const type_info &target, void *objptr,
! 		          const type_info *sub, void *subptr,
! 	                  const type_info **basetype, bool *is_public) const;
  };
  
  // type_info for a class with one public, nonvirtual base class.
*************** public:
*** 25,32 ****
    __si_type_info (const char *n, const __user_type_info &b)
      : __user_type_info (n), base (b) { }
  
!   virtual void *dcast (const type_info &, int, void *,
! 		       const type_info * = 0, void * = 0) const;
  };
  
  // type_info for a general class.
--- 50,58 ----
    __si_type_info (const char *n, const __user_type_info &b)
      : __user_type_info (n), base (b) { }
  
!   virtual void *do_dcast (const type_info &target, void *objptr,
! 		          const type_info *sub, void *subptr,
! 	                  const type_info **basetype, bool *is_public) const;
  };
  
  // type_info for a general class.
*************** struct __class_type_info : public __user
*** 49,55 ****
    __class_type_info (const char *name, const base_info *bl, size_t bn)
      : __user_type_info (name), base_list (bl), n_bases (bn) {}
  
!   // This is a little complex.
!   virtual void* dcast (const type_info &, int, void *,
! 		       const type_info * = 0, void * = 0) const;
  };
--- 75,82 ----
    __class_type_info (const char *name, const base_info *bl, size_t bn)
      : __user_type_info (name), base_list (bl), n_bases (bn) {}
  
!   // This is a little complex (to say the least).
!   virtual void* do_dcast (const type_info &target, void *objptr,
! 		          const type_info *sub, void *subptr,
! 	                  const type_info **basetype, bool *is_public) const;
  };
Index: egcs/gcc/cp/tinfo2.cc
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/cp/tinfo2.cc,v
retrieving revision 1.10
diff -c -3 -p -r1.10 tinfo2.cc
*** tinfo2.cc	1998/12/16 21:16:15	1.10
--- tinfo2.cc	1999/06/08 17:46:15
*************** struct __array_type_info : public type_i
*** 91,99 ****
  // Entry points for the compiler.
  
  /* Low level match routine used by compiler to match types of catch
!    variables and thrown objects.  */
  
! extern "C" void*
  __throw_type_match_rtti (const void *catch_type_r, const void *throw_type_r,
  			 void *objptr)
  {
--- 91,103 ----
  // Entry points for the compiler.
  
  /* Low level match routine used by compiler to match types of catch
!    variables and thrown objects.  Return NULL on no match and adjusted object
!    pointer for a match.  Because OBJPTR can be NULL, (when (T *)NULL was
!    thrown), it would be possible for us to return NULL on success.  But
!    that would be taken as not matching.  In that case, we return the special
!    value (void *)1, which should be checked for.  */
  
! extern "C" void *
  __throw_type_match_rtti (const void *catch_type_r, const void *throw_type_r,
  			 void *objptr)
  {
*************** __throw_type_match_rtti (const void *cat
*** 101,107 ****
    const type_info &throw_type = *(const type_info *)throw_type_r;
    
    if (catch_type == throw_type)
!     return objptr;
    
  #if 0
    printf ("We want to match a %s against a %s!\n",
--- 105,115 ----
    const type_info &throw_type = *(const type_info *)throw_type_r;
    
    if (catch_type == throw_type)
!     {
!       if (!objptr)
!         objptr = (void *)1;
!       return objptr;
!     }
    
  #if 0
    printf ("We want to match a %s against a %s!\n",
*************** __throw_type_match_rtti (const void *cat
*** 109,120 ****
  #endif
  
    void *new_objptr = 0;
  
    if (const __user_type_info *p
        = dynamic_cast <const __user_type_info *> (&throw_type))
      {
!       /* The 1 skips conversions to private bases. */
!       new_objptr = p->dcast (catch_type, 1, objptr);
      }
    else if (const __pointer_type_info *fr =
  	   dynamic_cast <const __pointer_type_info *> (&throw_type))
--- 117,129 ----
  #endif
  
    void *new_objptr = 0;
+   bool match = false;
  
    if (const __user_type_info *p
        = dynamic_cast <const __user_type_info *> (&throw_type))
      {
!       /* The 1 inhibits conversions to private bases. */
!       new_objptr = p->dcast (catch_type, 1, objptr, NULL, NULL, &match);
      }
    else if (const __pointer_type_info *fr =
  	   dynamic_cast <const __pointer_type_info *> (&throw_type))
*************** __throw_type_match_rtti (const void *cat
*** 153,167 ****
  	goto fail;
  
        if (*subto == *subfr)
! 	new_objptr = objptr;
        else if (*subto == typeid (void)
  	       && dynamic_cast <const __func_type_info *> (subfr) == 0)
! 	new_objptr = objptr;
        else if (const __user_type_info *p
  	       = dynamic_cast <const __user_type_info *> (subfr))
  	{
! 	  /* The 1 skips conversions to private bases. */
! 	  new_objptr = p->dcast (*subto, 1, objptr);
  	}
        else if (const __pointer_type_info *pfr
  	       = dynamic_cast <const __pointer_type_info *> (subfr))
--- 162,182 ----
  	goto fail;
  
        if (*subto == *subfr)
!         {
! 	  new_objptr = objptr;
! 	  match = true;
! 	}
        else if (*subto == typeid (void)
  	       && dynamic_cast <const __func_type_info *> (subfr) == 0)
!         {
!   	  new_objptr = objptr;
!           match = true;
! 	}
        else if (const __user_type_info *p
  	       = dynamic_cast <const __user_type_info *> (subfr))
  	{
! 	  /* The 1 inhibits conversions to private bases. */
! 	  new_objptr = p->dcast (*subto, 1, objptr, NULL, NULL, &match);
  	}
        else if (const __pointer_type_info *pfr
  	       = dynamic_cast <const __pointer_type_info *> (subfr))
*************** __throw_type_match_rtti (const void *cat
*** 212,217 ****
--- 227,233 ----
  	      if (*subto == *subfr)
  		{
  		  new_objptr = objptr;
+ 	          match = true;
  		  break;
  		}
  
*************** __throw_type_match_rtti (const void *cat
*** 225,236 ****
  	    }
  	}
      }
!  fail:
  
  #if 0
!   if (new_objptr)
      printf ("It converts, delta is %d\n", new_objptr-objptr);
  #endif
    return new_objptr;
  }
  
--- 241,256 ----
  	    }
  	}
      }
!   
  
  #if 0
!   if (match)
      printf ("It converts, delta is %d\n", new_objptr-objptr);
  #endif
+   if (match && !new_objptr)
+       new_objptr = (void *)1;
+ 
+   fail:
    return new_objptr;
  }
  


More information about the Gcc-patches mailing list