Determining the size of C++ vtables

[Thomas Stüfe]

Hi Ioi,

On Wed, Mar 1, 2017 at 3:37 AM, Ioi Lam <ioi.lam at oracle.com> wrote:

>
>
> On 2/24/17 11:52 PM, Thomas Stüfe wrote:
>
> Hi ioi,
>
> On Fri, Feb 24, 2017 at 7:50 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>
>>
>>
>> On 2/23/17 11:17 PM, Thomas Stüfe wrote:
>>
>> Hi Ioi,
>>
>> After reading through the original mailthread referenced in the bug
>> report, the only reason the "just copy enough (200) slots from the vtable
>> in libjvm.so to the vtable in the image" approach does not work for you is
>> that you are afraid of hitting unmapped memory? If I understand it
>> correctly.
>>
>> If that is the case, why not copy 200 slots with SafeFetch and stop at
>> the first error? That should not incur much additional costs as long as you
>> do not hit an unmapped area, in which case you'd stop anyway.
>>
>> Oh, that's a good simplification.I hadn't thought about using SafeFetch
>> :-(
>>
>> One issue with it is the 200 slots are too big (ConstantPool needs just
>> 10 slots), so finding the exact size is a little more efficient and saves a
>> few KBs.
>>
>>
> How many vtables are you copying? Does it really matter?
>
> I am copying only 8 vtables, so it doesn't really matter.
>
> As I said, I think your approach is quite elegant, but not terribly
> portable. We already make some assumptions (that vtables are organized as
> one continuous block of function pointers) and you add some more
> assumptions about the order of the methods in the vtable.
>
> I think even if you can make sure that it works now with our set of
> platforms and compilers, this adds the risk that a future compiler update
> may change the vtable layout and break the code. E.g. a vtable dumped with
> a VM compilers by an older compiler could not be loaded with the newly
> compiled VM.
>
> This shouldn't be a problem with usual compiler upgrades:
>
> You can load a CDS image only with the VM that generates it (we have a VM
> version string check), so if you have upgraded the compiler (or enabled
> RTTI and rebuilt the VM), so you have larger vtables, you won't be able to
> load any old CDS image anyway.
>
> However, if your compiler completely changes the way it implements vtables
> (e.g., adding one level of indirection to vptr), then, things will break,
> but the existing CDS vtable code also has the same problem.
>
> Also, you set the bar a bit higher for future ports, because you add
> requirements for the C++ compiler.
>
> But I may be too cautious here and I am waiting for others to chime in.
>
> As a side note, it would make sense to write a tiny C++ test which copies
> vtables around using your technique and then checks if they still work;
> such a test we could use on out platforms to check if our compilers can
> cope. Such a test would also be a good sanity check to add to the VM
> initialization if CDS is active.
>
> I've taken your suggestion to use SafeFetchN instead of trying to figure
> out the vtable size. That simplifies the product-build a bit.
>
> I put my vtable sizing code in debug builds. It's not strictly portable,
> but it probably fail quickly on an esoteric c++ compiler so you can figure
> out what's wrong. It will also assert if you have added too many new
> virtual method to overflow the current limit of 150.
>
> Please reply your comments to the RFR e-mail thread.
>
>
Thank you for taking my suggestions, I will take a look at that RFR.

Thomas


> Thanks
> - Ioi
>
>
> Kind Regards, Thomas
>
>
>
>> Thanks
>> - Ioi
>>
>>
>> That way you make yourself a bit less dependent on compiler internals. I
>> agree that your approach is more elegant, though.
>>
>> Kind Regards, Thomas
>>
>> On Fri, Feb 24, 2017 at 4:55 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>
>>>
>>>
>>> On 2/23/17 7:47 PM, Ioi Lam wrote:
>>>
>>>> Hi,
>>>>
>>>> I am working on https://bugs.openjdk.java.net/browse/JDK-8005165
>>>> (Remove
>>>> CPU-dependent code in self-patching vtables), I need a way find out the
>>>> size
>>>> of a C++ vtable. I ended up doing this:
>>>>
>>>>
>>>> // Objects of the Metadata types (such as Klass and ConstantPool) have
>>>> C++ vtables.
>>>> // (In GCC this is the field <Type>::_vptr, i.e., first word in the
>>>> object.)
>>>> //
>>>> // Addresses of the vtables and the methods may be different across JVM
>>>> runs,
>>>> // if libjvm.so is dynamically loaded at a different base address.
>>>> //
>>>> // To ensure that the Metadata objects in the CDS archive always have
>>>> the correct vtable:
>>>> //
>>>> // + at dump time:  we redirect the _vptr to point to our own vtables
>>>> inside
>>>> //                  the CDS image
>>>> // + at run time:   we clone the actual contents of the vtables from
>>>> libjvm.so
>>>> //                  into our own tables.
>>>> //
>>>> // To determine the size of the vtable for each type, we use the
>>>> following
>>>> // trick by declaring 2 subclasses:
>>>> //
>>>> //   class CppVtabTesterA: public InstanceKlass {
>>>> //          virtual int   last_virtual_method() {return 1;}
>>>> //   };
>>>> //   class CppVtabTesterB: public InstanceKlass {
>>>> //          virtual void* last_virtual_method() {return NULL};
>>>> //   };
>>>> //
>>>> // CppVtabTesterA and CppVtabTesterB's vtables have the following
>>>> properties:
>>>> // - Their size (N+1) is exactly one more than the size of
>>>> InstanceKlass's vtable (N)
>>>> // - The first N entries have are exactly the same as in
>>>> InstanceKlass's vtable.
>>>> // - Their last entry is different.
>>>> //
>>>> // So to determine the value of N, we just walk CppVtabTesterA and
>>>> CppVtabTesterB's tables
>>>> // and find the first entry that's different
>>>>
>>>>
>>>> Could anyone comment if this is acceptable? I know it's not 100%
>>>> portable (C++ doesn't
>>>> specify where to find the vtable, or what's inside), but my assumptions
>>>> is the same as
>>>> the existing code. I.e., _vptr is a pointer located at offset 0 of the
>>>> object, and it
>>>> points to a one-dimensional array.
>>>>
>>>> So at least it's not any worse than before?
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>> By the way, I first tried having only a single "tester" class and walk
>>> the vtable to look for &last_virtual_method, but the C++ compiler told me
>>> that taking the address of a non-static function is not allowed ..... so I
>>> ended up creating two tester classes and checking their differences.
>>>
>>>
>>>
>>>
>>
>>
>
>