[libcxx-commits] [PATCH] D77606: [libcxxabi] Add macro for changing functions to support the relative vtables ABI

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxxabi/src/private_typeinfo.cpp:618
     // Get (dynamic_ptr, dynamic_type) from static_ptr
-    void **vtable = *static_cast<void ** const *>(static_ptr);
-    ptrdiff_t offset_to_derived = reinterpret_cast<ptrdiff_t>(vtable[-2]);
+#if __has_feature(cxx_abi_relative_vtable)
+    // The vtable address will point to the first virtual function, which is 8
----------------
Just to be clear, is this enabled whenever the compiler *supports* the feature, or whenever the feature is actually turned on?

The former would be incorrect, because any sufficiently recent Clang would take that code path. The latter is what we want.


================
Comment at: libcxxabi/src/private_typeinfo.cpp:623
+    uint32_t offset_to_derived = vtable[-2];
     const void* dynamic_ptr = static_cast<const char*>(static_ptr) + offset_to_derived;
+
----------------
Are you certain this is the only place where code needs changing? For example we seem to extract the vtable in `__base_class_type_info::search_above_dst` and other functions -- don't they need to be changed too?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77606/new/

https://reviews.llvm.org/D77606