UBSan: Teach isDerivedFromAtOffset and findBaseAtOffset about vbases

[majnemer]

Summary

When the virtual_mask is set, offset_flags >> __offset_shift yields
an offset into the vtable. Dereferencing this vtable slot gets us the
vbase offset.

Adjust a test case to verify that this, in fact, works.

[samsonov]

LGTM. Thanks for doing this!

lib/ubsan/ubsan_type_hash.cc

143 ↗

(On Diff #15673)

Can this be

sptr VTable = *reinterpret_cast<sptr *>(Object);
OffsetHere = *reinterpret_cast<sptr *>(VTable + OffsetHere);

?

[majnemer]

Closed by commit rL221445 (authored by @majnemer).

compiler-rt/trunk/lib/ubsan/ubsan_type_hash.cc

 }
 
 /// A cache of recently-checked hashes. Mini hash table with "random" evictions.
 __ubsan::HashValue
 __ubsan::__ubsan_vptr_type_cache[__ubsan::VptrTypeCacheSize];
 
 /// \brief Determine whether \p Derived has a \p Base base class subobject at
 /// offset \p Offset.
-static bool isDerivedFromAtOffset(const abi::__class_type_info *Derived,
+static bool isDerivedFromAtOffset(sptr Object,
+                                  const abi::__class_type_info *Derived,
                                   const abi::__class_type_info *Base,
                                   sptr Offset) {
   if (Derived->__type_name == Base->__type_name)
     return Offset == 0;
 
   if (const abi::__si_class_type_info *SI =
         dynamic_cast<const abi::__si_class_type_info*>(Derived))
-    return isDerivedFromAtOffset(SI->__base_type, Base, Offset);
+    return isDerivedFromAtOffset(Object, SI->__base_type, Base, Offset);
 
   const abi::__vmi_class_type_info *VTI =
     dynamic_cast<const abi::__vmi_class_type_info*>(Derived);
   if (!VTI)
     // No base class subobjects.
     return false;
 
   // Look for a base class which is derived from \p Base at the right offset.
   for (unsigned int base = 0; base != VTI->base_count; ++base) {
     // FIXME: Curtail the recursion if this base can't possibly contain the
     //        given offset.
     sptr OffsetHere = VTI->base_info[base].__offset_flags >>
                       abi::__base_class_type_info::__offset_shift;
     if (VTI->base_info[base].__offset_flags &
-          abi::__base_class_type_info::__virtual_mask)
-      // For now, just punt on virtual bases and say 'yes'.
-      // FIXME: OffsetHere is the offset in the vtable of the virtual base
-      //        offset. Read the vbase offset out of the vtable and use it.
-      return true;
-    if (isDerivedFromAtOffset(VTI->base_info[base].__base_type,
-                              Base, Offset - OffsetHere))
+          abi::__base_class_type_info::__virtual_mask) {
+      sptr VTable = *reinterpret_cast<const sptr *>(Object);
+      OffsetHere = *reinterpret_cast<const sptr *>(VTable + OffsetHere);
+    }
+    if (isDerivedFromAtOffset(Object + OffsetHere,
+                              VTI->base_info[base].__base_type, Base,
+                              Offset - OffsetHere))
       return true;
   }
 
   return false;
 }
 
 /// \brief Find the derived-most dynamic base class of \p Derived at offset
 /// \p Offset.
-static const abi::__class_type_info *findBaseAtOffset(
-    const abi::__class_type_info *Derived, sptr Offset) {
+static const abi::__class_type_info *
+findBaseAtOffset(sptr Object, const abi::__class_type_info *Derived,
+                 sptr Offset) {
   if (!Offset)
     return Derived;
 
   if (const abi::__si_class_type_info *SI =
         dynamic_cast<const abi::__si_class_type_info*>(Derived))
-    return findBaseAtOffset(SI->__base_type, Offset);
+    return findBaseAtOffset(Object, SI->__base_type, Offset);
 
   const abi::__vmi_class_type_info *VTI =
     dynamic_cast<const abi::__vmi_class_type_info*>(Derived);
   if (!VTI)
     // No base class subobjects.
     return 0;
 
   for (unsigned int base = 0; base != VTI->base_count; ++base) {
     sptr OffsetHere = VTI->base_info[base].__offset_flags >>
                       abi::__base_class_type_info::__offset_shift;
     if (VTI->base_info[base].__offset_flags &
-          abi::__base_class_type_info::__virtual_mask)
-      // FIXME: Can't handle virtual bases yet.
-      continue;
-    if (const abi::__class_type_info *Base =
-          findBaseAtOffset(VTI->base_info[base].__base_type,
-                           Offset - OffsetHere))
+          abi::__base_class_type_info::__virtual_mask) {
+      sptr VTable = *reinterpret_cast<const sptr *>(Object);
+      OffsetHere = *reinterpret_cast<const sptr *>(VTable + OffsetHere);
+    }
+    if (const abi::__class_type_info *Base = findBaseAtOffset(
+            Object + OffsetHere, VTI->base_info[base].__base_type,
       return Base;
   }
 
   return 0;
 }
 
 namespace {
 
bool __ubsan::checkDynamicType(void *Object, void *Type, HashValue Hash) {
 
   // Check that this is actually a type_info object for a class type.
   abi::__class_type_info *Derived =
     dynamic_cast<abi::__class_type_info*>(Vtable->TypeInfo);
   if (!Derived)
     return false;
 
   abi::__class_type_info *Base = (abi::__class_type_info*)Type;
-  if (!isDerivedFromAtOffset(Derived, Base, -Vtable->Offset))
+  if (!isDerivedFromAtOffset(reinterpret_cast<sptr>(Object), Derived, Base,
+                             -Vtable->Offset))
     return false;
 
   // Success. Cache this result.
   __ubsan_vptr_type_cache[Hash % VptrTypeCacheSize] = Hash;
   *Bucket = Hash;
   return true;
 }
 
 __ubsan::DynamicTypeInfo __ubsan::getDynamicTypeInfo(void *Object) {
   VtablePrefix *Vtable = getVtablePrefix(Object);
   if (!Vtable)
     return DynamicTypeInfo(0, 0, 0);
   const abi::__class_type_info *ObjectType = findBaseAtOffset(
-    static_cast<const abi::__class_type_info*>(Vtable->TypeInfo),
+      reinterpret_cast<sptr>(Object),
+      static_cast<const abi::__class_type_info *>(Vtable->TypeInfo),
       -Vtable->Offset);
   return DynamicTypeInfo(Vtable->TypeInfo->__type_name, -Vtable->Offset,
                          ObjectType ? ObjectType->__type_name : "<unknown>");
 }

compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr.cpp

 
 struct T : S {
   T() : b(0) {}
   int b;
   int g() { return 0; }
   virtual int v() { return 1; }
 };
 
-struct U : S, T { virtual int v() { return 2; } };
+struct X {};
+struct U : S, T, virtual X { virtual int v() { return 2; } };
 
 struct V : S {};
 
 // Make p global so that lsan does not complain.
 T *p = 0;
 
 int access_p(T *p, char type);