[libc++abi] Handle null pointer-to-object [PR64593]

[iains]

Summary

This addresses cases (currently failing) where we throw a null pointer-to-object and is a proposed fix for https://github.com/llvm/llvm-project/issues/64953

We are trying to satisfy the following bullet from the C++ ABI 15.3:

• the handler is of type cv1 T* cv2 and E is a pointer type that can be converted to the type of the handler by either or both of:
  
  o a standard pointer conversion (4.10 [conv.ptr]) not involving conversions to private or protected or ambiguous classes.
  
  o a qualification conversion.

The existing implementation assesses the ambiguity of bases by computing the offsets to them; ambiguous cases are then when the same base appears at different offsets. This includes indirecting through the vtables to find the offsets to virtual bases.

When the thrown pointer points to a real object, this is quite efficient since if the base is found, not ambiguous and on a public path, the offset is needed to return the adjusted pointer (and the indirections are not particularly expensive to compute).

However, when we throw a null pointer-to-object, this scheme is no longer applicable (and the code currently bypasses the relevant computations, leading to the incorrect catches).

---

The solution proposed here takes a composite approach:

1. When the pointer-to-object points to a real instance (well, at least, it is non-null), we use the existing scheme.

2. When the pointer-to-object is null:
   • We note that there is no real object.
   • When we are processing non-virtual bases, we continue to compute the offsets, but for a notional dummy object based at 0. This is OK, since we never need to access the object content for non-virtual bases.
   • When we are processing a path with one or more virtual bases, we remember a cookie corresponding to the inner-most virtual base found so far (and set the notional offset to 0). Offsets to inner non-virtual bases are then computed as normal.

A base is then ambiguous iff:

• There is a recorded virtual base cookie and that is different from the current one or..
• The non-virtual base offsets differ.

When a handler for a pointer succeeds in catching a base pointer for a thrown null pointer-to-object, we still return a nullptr (so the adjustment to the pointer is not required and need not be computed).

Since we noted that there was no object when starting the search for ambiguous bases, we know that we can skip the pointer adjustment.

[iains]

added handling for virtual bases and a testcase

[iains]

update the testcase to remove a case that triggers a GCC warning.

[iains]

Some background since it is relevant to the testing done so far.

@urnathan : I think the strategy outlined in the patch header is necessary - I would welcome your opinion on whether it is sufficient.

There is clearly scope for lots of fallout from changes in this library ... so some notes on testing so far:

Mostly, I work on macOS - where the installed system C++abi lib is libc++abi.
I am working on a patch to allow GCC's libstdc++ to sit on top of libc++abi (to try and resolve some of the issues we face when multiple C++ and C++abi libraries get linked into a single executable).

Testing this combination on the GCC testsuite reveals a number of cases where c++abi has different behaviour (or possibly bugs) w.r.t. to supc++. The cases here are completely reproducible in stand-alone tests and reported in the referenced issue.

The patch does resolve around 85 failing cases where the the common factor is a thrown null pointer-to-object.
The test case passes natively on arm64-apple-darwin21 and on x86_64-apple-darwin21 (via rosetta 2) - I plan to test more widely if there is interest in pursuing this.

[philnik]

Since we are moving to GitHub PRs and there hasn't been any activity here yet, could you move this patch over?

[iains]

In D158769#4632875, @philnik wrote:

Since we are moving to GitHub PRs and there hasn't been any activity here yet, could you move this patch over?

OK, will do.
I am still trying to get a handle on the new process (especially w.r.t draft patches, stacked ones, stuff already partially reviewed in Phab and not very likely to be completed by Oct... and rebasing). However, this one is stand-alone :)

[Mordante]

In D158769#4636145, @iains wrote:

In D158769#4632875, @philnik wrote:

Since we are moving to GitHub PRs and there hasn't been any activity here yet, could you move this patch over?

OK, will do.
I am still trying to get a handle on the new process (especially w.r.t draft patches, stacked ones, stuff already partially reviewed in Phab and not very likely to be completed by Oct... and rebasing). However, this one is stand-alone :)

In D158769#4636145, @iains wrote:

In D158769#4632875, @philnik wrote:

Since we are moving to GitHub PRs and there hasn't been any activity here yet, could you move this patch over?

OK, will do.
I am still trying to get a handle on the new process (especially w.r.t draft patches, stacked ones, stuff already partially reviewed in Phab and not very likely to be completed by Oct... and rebasing). However, this one is stand-alone :)

Thanks can you abandon this review after moving it to GitHub?

[ldionne]

This is now https://github.com/llvm/llvm-project/pull/65902, let's abandon here please.

Nevermind me, I got confused.

[iains]

sorry I did not get to moving this yet - but will try during this week.

[iains]

this is now
https://github.com/llvm/llvm-project/pull/68076

libcxxabi/src/private_typeinfo.h

// Data that helps stop the search before the entire tree is searched:
     // communicates to a dst_type node that (static_ptr, static_type) was found
     //    above it.
     bool found_our_static_ptr;
     // communicates to a dst_type node that a static_type was found
     //    above it, but it wasn't (static_ptr, static_type)
     bool found_any_static_type;
     // Set whenever a search can be stopped
     bool search_done;
+
+    // Data that modifies the search mechanism.
+
+    // There is no object (seen when we throw a null pointer to object).
+    bool have_object;
 };
 
 // Has no base class
 class _LIBCXXABI_TYPE_VIS __class_type_info : public __shim_type_info {
 public:
   _LIBCXXABI_HIDDEN virtual ~__class_type_info();
 
   _LIBCXXABI_HIDDEN void process_static_type_above_dst(__dynamic_cast_info *,
                                                        const void *,
                                                        const void *, int) const;
   _LIBCXXABI_HIDDEN void process_static_type_below_dst(__dynamic_cast_info *,
                                                        const void *, int) const;
-  _LIBCXXABI_HIDDEN void process_found_base_class(__dynamic_cast_info *, void *,
-                                                  int) const;
+  _LIBCXXABI_HIDDEN void process_found_base_class(__dynamic_cast_info*, void*, const void*, int) const;
   _LIBCXXABI_HIDDEN virtual void search_above_dst(__dynamic_cast_info *,
                                                   const void *, const void *,
                                                   int, bool) const;
   _LIBCXXABI_HIDDEN virtual void
   search_below_dst(__dynamic_cast_info *, const void *, int, bool) const;
   _LIBCXXABI_HIDDEN virtual bool can_catch(const __shim_type_info *,
                                            void *&) const;
-  _LIBCXXABI_HIDDEN virtual void
-  has_unambiguous_public_base(__dynamic_cast_info *, void *, int) const;
+  _LIBCXXABI_HIDDEN virtual void has_unambiguous_public_base(__dynamic_cast_info*, void*, const void*, int) const;
 };
 
 // Has one non-virtual public base class at offset zero
 class _LIBCXXABI_TYPE_VIS __si_class_type_info : public __class_type_info {
 public:
   const __class_type_info *__base_type;
 
   _LIBCXXABI_HIDDEN virtual ~__si_class_type_info();
 
   _LIBCXXABI_HIDDEN virtual void search_above_dst(__dynamic_cast_info *,
                                                   const void *, const void *,
                                                   int, bool) const;
   _LIBCXXABI_HIDDEN virtual void
   search_below_dst(__dynamic_cast_info *, const void *, int, bool) const;
-  _LIBCXXABI_HIDDEN virtual void
-  has_unambiguous_public_base(__dynamic_cast_info *, void *, int) const;
+  _LIBCXXABI_HIDDEN virtual void has_unambiguous_public_base(__dynamic_cast_info*, void*, const void*, int) const;
 };
 
 struct _LIBCXXABI_HIDDEN __base_class_type_info
 {
 public:
     const __class_type_info* __base_type;
     long __offset_flags;
 
     enum __offset_flags_masks
     {
         __virtual_mask = 0x1,
         __public_mask  = 0x2, // base is public
         __offset_shift = 8
     };
 
     void search_above_dst(__dynamic_cast_info*, const void*, const void*, int, bool) const;
     void search_below_dst(__dynamic_cast_info*, const void*, int, bool) const;
-    void has_unambiguous_public_base(__dynamic_cast_info*, void*, int) const;
+    void has_unambiguous_public_base(__dynamic_cast_info*, void*, const void*, int) const;
 };
 
 // Has one or more base classes
 class _LIBCXXABI_TYPE_VIS __vmi_class_type_info : public __class_type_info {
 public:
   unsigned int __flags;
   unsigned int __base_count;
   __base_class_type_info __base_info[1];
 
   enum __flags_masks {
     __non_diamond_repeat_mask = 0x1, // has two or more distinct base class
                                      //    objects of the same type
     __diamond_shaped_mask = 0x2      // has base class object with two or
                                      //    more derived objects
   };
 
   _LIBCXXABI_HIDDEN virtual ~__vmi_class_type_info();
 
   _LIBCXXABI_HIDDEN virtual void search_above_dst(__dynamic_cast_info *,
                                                   const void *, const void *,
                                                   int, bool) const;
   _LIBCXXABI_HIDDEN virtual void
   search_below_dst(__dynamic_cast_info *, const void *, int, bool) const;
-  _LIBCXXABI_HIDDEN virtual void
-  has_unambiguous_public_base(__dynamic_cast_info *, void *, int) const;
+  _LIBCXXABI_HIDDEN virtual void has_unambiguous_public_base(__dynamic_cast_info*, void*, const void*, int) const;
 };
 
 class _LIBCXXABI_TYPE_VIS __pbase_type_info : public __shim_type_info {
 public:
   unsigned int __flags;
   const __shim_type_info *__pointee;
 
   enum __masks {

libcxxabi/src/private_typeinfo.cpp

 // type_info* will work for typeids from the same compiled file but fail
 // for typeids from a DLL and an executable. Among other things, exceptions
 // are not caught by handlers since can_catch() returns false.
 //
 // Defining _LIBCXXABI_FORGIVING_DYNAMIC_CAST does not help since can_catch() calls
 // is_equal() with use_strcmp=false so the string names are not compared.
 
 #include <cstdint>
+#include <cassert>
 #include <string.h>
 
 #ifdef _LIBCXXABI_FORGIVING_DYNAMIC_CAST
 #include "abort_message.h"
 #include <sys/syslog.h>
 #include <atomic>
 #endif
 
__class_type_info::can_catch(const __shim_type_info* thrown_type,
     // bullet 1
     if (is_equal(this, thrown_type, false))
         return true;
     const __class_type_info* thrown_class_type =
         dynamic_cast<const __class_type_info*>(thrown_type);
     if (thrown_class_type == 0)
         return false;
     // bullet 2
-    __dynamic_cast_info info = {thrown_class_type, 0, this, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,};
+    assert (adjustedPtr && "catching a class without an object?");
+    __dynamic_cast_info info = {thrown_class_type, 0, this,
+                                -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, true};
     info.number_of_dst_type = 1;
-    thrown_class_type->has_unambiguous_public_base(&info, adjustedPtr, public_path);
+    thrown_class_type->has_unambiguous_public_base(&info, adjustedPtr, nullptr, public_path);
     if (info.path_dst_ptr_to_static_ptr == public_path)
     {
         adjustedPtr = const_cast<void*>(info.dst_ptr_leading_to_static_ptr);
         return true;
     }
     return false;
 }
 
 #ifdef __clang__
 #pragma clang diagnostic pop
 #endif
 
+// When we have an object to inspect - we just pass the pointer to the sub-
+// object that matched the static_type we just checked.  If that is different
+// from any previously recorded pointer to that object type, then we have
+// an ambiguous case.
+
+// When we have no object to inspect, we need to account for virtual bases
+// explicitly.
+// virtualBase is the pointer to the name of the innermost virtual base
+// type, or nullptr if there is no virtual base on the path so far.
+// adjustedPtr points to the subobject we just found.
+// If virtualBase != any previously recorded (including the case of nullptr
+// representing an already-found static sub-object) then we have an ambiguous
+// case.  Assuming that the virtualBase values agree; if then we have a
+// different offset (adjustedPtr) from any previously recorded, this indicates
+// an ambiguous case within the virtual base.
+
 void
 __class_type_info::process_found_base_class(__dynamic_cast_info* info,
                                                void* adjustedPtr,
+                                               const void* virtualBase,
                                                int path_below) const
 {
-    if (info->dst_ptr_leading_to_static_ptr == 0)
+    if (info->number_to_static_ptr == 0)
     {
         // First time here
         info->dst_ptr_leading_to_static_ptr = adjustedPtr;
         info->path_dst_ptr_to_static_ptr = path_below;
+        // re-purpose this pointer.
+        info->dst_ptr_not_leading_to_static_ptr = virtualBase;
         info->number_to_static_ptr = 1;
     }
-    else if (info->dst_ptr_leading_to_static_ptr == adjustedPtr)
+    else if (info->dst_ptr_not_leading_to_static_ptr == virtualBase &&
+             info->dst_ptr_leading_to_static_ptr == adjustedPtr)
     {
         // We've been here before.  Update path to "most public"
         if (info->path_dst_ptr_to_static_ptr == not_public_path)
             info->path_dst_ptr_to_static_ptr = path_below;
     }
     else
     {
         // We've detected an ambiguous cast from (thrown_class_type, adjustedPtr)
         //   to a static_type
         info->number_to_static_ptr += 1;
         info->path_dst_ptr_to_static_ptr = not_public_path;
         info->search_done = true;
     }
 }
 
 void
 __class_type_info::has_unambiguous_public_base(__dynamic_cast_info* info,
                                                void* adjustedPtr,
+                                               const void* virtualBase,
                                                int path_below) const
 {
     if (is_equal(this, info->static_type, false))
-        process_found_base_class(info, adjustedPtr, path_below);
+        process_found_base_class(info, adjustedPtr, virtualBase, path_below);
 }
 
 void
 __si_class_type_info::has_unambiguous_public_base(__dynamic_cast_info* info,
                                                   void* adjustedPtr,
+                                                  const void* virtualBase,
                                                   int path_below) const
 {
     if (is_equal(this, info->static_type, false))
-        process_found_base_class(info, adjustedPtr, path_below);
+        process_found_base_class(info, adjustedPtr, virtualBase, path_below);
     else
-        __base_type->has_unambiguous_public_base(info, adjustedPtr, path_below);
+        __base_type->has_unambiguous_public_base(info, adjustedPtr, virtualBase, path_below);
 }
 
 void
 __base_class_type_info::has_unambiguous_public_base(__dynamic_cast_info* info,
                                                     void* adjustedPtr,
+                                                    const void* virtualBase,
                                                     int path_below) const
 {
+    bool is_virtual = __offset_flags & __virtual_mask;
     ptrdiff_t offset_to_base = 0;
-    if (adjustedPtr != nullptr)
+    if (info->have_object)
     {
+        /* We have an object to inspect, we can look through its vtables to
+           find the layout.  */
         offset_to_base = __offset_flags >> __offset_shift;
-        if (__offset_flags & __virtual_mask)
+        if (is_virtual)
         {
             const char* vtable = *static_cast<const char*const*>(adjustedPtr);
             offset_to_base = update_offset_to_base(vtable, offset_to_base);
         }
+    } else if (! is_virtual) {
+        /* We have no object - so we cannot use it for determining layout when
+           we have a virtual base (since we cannot indirect through the vtable
+           to find the actual object offset).  However, for non-virtual bases,
+           we can pretend to have an object based at '0' */
+        offset_to_base = __offset_flags >> __offset_shift;
+    } else {
+      // no object to inspect, and the next base is virtual.
+      // we want to update virtualBase to the new innermost virtual base.
+      // using the pointer to the typeinfo name as a key.
+      virtualBase = static_cast<const void*>(__base_type->name ());
+      // .. and reset the pointer.
+      adjustedPtr = nullptr;
     }
     __base_type->has_unambiguous_public_base(
             info,
             static_cast<char*>(adjustedPtr) + offset_to_base,
+            virtualBase,
             (__offset_flags & __public_mask) ? path_below : not_public_path);
 }
 
 void
 __vmi_class_type_info::has_unambiguous_public_base(__dynamic_cast_info* info,
                                                    void* adjustedPtr,
+                                                   const void* virtualBase,
                                                    int path_below) const
 {
     if (is_equal(this, info->static_type, false))
-        process_found_base_class(info, adjustedPtr, path_below);
+        process_found_base_class(info, adjustedPtr, virtualBase, path_below);
     else
     {
         typedef const __base_class_type_info* Iter;
         const Iter e = __base_info + __base_count;
         Iter p = __base_info;
-        p->has_unambiguous_public_base(info, adjustedPtr, path_below);
+        p->has_unambiguous_public_base(info, adjustedPtr, virtualBase, path_below);
         if (++p < e)
         {
             do
             {
-                p->has_unambiguous_public_base(info, adjustedPtr, path_below);
+                p->has_unambiguous_public_base(info, adjustedPtr, virtualBase, path_below);
                 if (info->search_done)
                     break;
             } while (++p < e);
         }
     }
 }
 
 // Handles bullet 1 for both pointers and member pointers
__pointer_type_info::can_catch(const __shim_type_info* thrown_type,
     const __class_type_info* catch_class_type =
         dynamic_cast<const __class_type_info*>(__pointee);
     if (catch_class_type == 0)
         return false;
     const __class_type_info* thrown_class_type =
         dynamic_cast<const __class_type_info*>(thrown_pointer_type->__pointee);
     if (thrown_class_type == 0)
         return false;
-    __dynamic_cast_info info = {thrown_class_type, 0, catch_class_type, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,};
+    bool have_object = adjustedPtr != nullptr;
+    __dynamic_cast_info info = {thrown_class_type, 0, catch_class_type, -1,
+                                0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+              have_object };
     info.number_of_dst_type = 1;
-    thrown_class_type->has_unambiguous_public_base(&info, adjustedPtr, public_path);
+    thrown_class_type->has_unambiguous_public_base(&info, adjustedPtr, nullptr, public_path);
     if (info.path_dst_ptr_to_static_ptr == public_path)
     {
-        if (adjustedPtr != NULL)
+        // In the case of a thrown null pointer, we have no object but we might
+        // well have computed the offset to where a public sub-object would be.
+        // However, we do not want to return that offset to the user; we still
+        // want them to catch a null ptr.
+        if (have_object)
             adjustedPtr = const_cast<void*>(info.dst_ptr_leading_to_static_ptr);
+        else
+            adjustedPtr = nullptr;
         return true;
     }
     return false;
 }
 
 bool __pointer_type_info::can_catch_nested(
     const __shim_type_info* thrown_type) const
 {

libcxxabi/test/catch_null_pointer_to_object_pr64953.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  This test case checks specifically the cases under bullet 3.3:
+//
+//  C++ ABI 15.3:
+//  A handler is a match for an exception object of type E if
+//     *  The handler is of type cv T or cv T& and E and T are the same type
+//        (ignoring the top-level cv-qualifiers), or
+//     *  the handler is of type cv T or cv T& and T is an unambiguous base
+//        class of E, or
+//  >  *  the handler is of type cv1 T* cv2 and E is a pointer type that can   <
+//  >     be converted to the type of the handler by either or both of         <
+//  >       o  a standard pointer conversion (4.10 [conv.ptr]) not involving   <
+//  >          conversions to private or protected or ambiguous classes        <
+//  >       o  a qualification conversion                                      <
+//     *  the handler is a pointer or pointer to member type and E is
+//        std::nullptr_t
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: no-exceptions
+
+#include <exception>
+#include <stdlib.h>
+#include <assert.h>
+#include <stdio.h>
+
+struct Base {
+  int b;
+};
+struct Base2 {
+  int b;
+};
+struct Derived1 : Base {
+  int b;
+};
+struct Derived2 : Base {
+  int b;
+};
+struct Derived3 : Base2 {
+  int b;
+};
+struct Private : private Base {
+  int b;
+};
+struct Protected : protected Base {
+  int b;
+};
+struct Virtual1 : virtual Base {
+  int b;
+};
+struct Virtual2 : virtual Base {
+  int b;
+};
+
+struct Ambiguous1 : Derived1, Derived2 {
+  int b;
+};
+struct Ambiguous2 : Derived1, Private {
+  int b;
+};
+struct Ambiguous3 : Derived1, Protected {
+  int b;
+};
+struct Ambiguous4 : Derived1, Virtual2 {
+  int b;
+};
+
+struct NoPublic1 : Private, Base2 {
+  int b;
+};
+struct NoPublic2 : Protected, Base2 {
+  int b;
+};
+
+struct Catchable1 : Derived3, Derived1 {
+  int b;
+};
+struct Catchable2 : Virtual1, Virtual2 {
+  int b;
+};
+struct Catchable3 : virtual Base, Virtual2 {
+  int b;
+};
+
+// Check that, when we have a null pointer-to-object that we catch a nullptr.
+template <typename T // Handler type
+          ,
+          typename E // Thrown exception type
+          >
+void assert_catches() {
+  try {
+    throw static_cast<E>(0);
+    printf("%s\n", __PRETTY_FUNCTION__);
+    assert(false && "Statements after throw must be unreachable");
+  } catch (T t) {
+    assert(t == nullptr);
+    return;
+  } catch (...) {
+    printf("%s\n", __PRETTY_FUNCTION__);
+    assert(false && "Should not have entered catch-all");
+  }
+
+  printf("%s\n", __PRETTY_FUNCTION__);
+  assert(false && "The catch should have returned");
+}
+
+template <typename T // Handler type
+          ,
+          typename E // Thrown exception type
+          >
+void assert_cannot_catch() {
+  try {
+    throw static_cast<E>(0);
+    printf("%s\n", __PRETTY_FUNCTION__);
+    assert(false && "Statements after throw must be unreachable");
+  } catch (T t) {
+    printf("%s\n", __PRETTY_FUNCTION__);
+    assert(false && "Should not have entered the catch");
+  } catch (...) {
+    assert(true);
+    return;
+  }
+
+  printf("%s\n", __PRETTY_FUNCTION__);
+  assert(false && "The catch-all should have returned");
+}
+
+// Check that when we have a pointer-to-actual-object we, in fact, get the
+// adjusted pointer to the base class.
+template <typename T // Handler type
+          ,
+          typename O // Object type
+          >
+void assert_catches_bp() {
+  O* o = new (O);
+  try {
+    throw o;
+    printf("%s\n", __PRETTY_FUNCTION__);
+    assert(false && "Statements after throw must be unreachable");
+  } catch (T t) {
+    assert(t == static_cast<T>(o));
+    //__builtin_printf("o = %p t = %p\n", o, t);
+    return;
+  } catch (...) {
+    printf("%s\n", __PRETTY_FUNCTION__);
+    assert(false && "Should not have entered catch-all");
+  }
+
+  printf("%s\n", __PRETTY_FUNCTION__);
+  assert(false && "The catch should have returned");
+}
+
+void f1() {
+  assert_catches<Base*, Catchable1*>();
+  assert_catches<Base*, Catchable2*>();
+  assert_catches<Base*, Catchable3*>();
+}
+
+void f2() {
+  assert_cannot_catch<Base*, Ambiguous1*>();
+  assert_cannot_catch<Base*, Ambiguous2*>();
+  assert_cannot_catch<Base*, Ambiguous3*>();
+  assert_cannot_catch<Base*, Ambiguous4*>();
+  assert_cannot_catch<Base*, NoPublic1*>();
+  assert_cannot_catch<Base*, NoPublic2*>();
+}
+
+void f3() {
+  assert_catches_bp<Base*, Catchable1>();
+  assert_catches_bp<Base*, Catchable2>();
+  assert_catches_bp<Base*, Catchable3>();
+}
+
+int main(int, char**) {
+  f1();
+  f2();
+  f3();
+  return 0;
+}