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

[Leonard Chan via Phabricator via libcxx-commits]

leonardchan 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
----------------
ldionne wrote:
> 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.
The latter, where it's turned on.


================
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;
+
----------------
ldionne wrote:
> 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?
Thanks for catching this. It took me some time to check and assert what these values should be. Updated to just load 32-bit ints instead of `ptrdiff_t`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77606