RFR: 8333639: ubsan: cppVtables.cpp:81:55: runtime error: index 14 out of bounds for type 'long int [1]' [v3]

[Martin Doerr]

On Sat, 15 Jun 2024 01:20:06 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use _cloned_vtable[1] and address computation.
>
> src/hotspot/share/cds/cppVtables.cpp line 79:
> 
>> 77:   intptr_t* cloned_vtable()   { return (intptr_t*)((char*)this + offset_of(CppVtableInfo, _cloned_vtable)); }
>> 78:   void zero()                 { memset(cloned_vtable(), 0, sizeof(intptr_t) * vtable_size()); }
>> 79:   // Returns the address of the next CppVtableInfo that can be placed immediately after this CppVtableInfo
> 
> The description of this function is wrong, as it returns an offset rather than
> an address.

It returns a pointer which is computed by base + offset. I've factored out the offset computation.

> src/hotspot/share/cds/cppVtables.cpp line 82:
> 
>> 80:   static size_t byte_size(int vtable_size) {
>> 81:     CppVtableInfo i;
>> 82:     return pointer_delta(&i.cloned_vtable()[vtable_size], &i, sizeof(u1));
> 
> Rather than making a dummy CppVTableInfo and doing pointer arithmetic, better
> would be something like
> 
> offset_of(CppVtableInfo, _cloned_vtable) + (sizeof(intptr_t) * vtable_size)
> 
> It might be that some of the subexpressions of that should be broken out into helper
> functions that can also be used in `clone_vtable()` and `zero()`.
> 
> Also, the really paranoid might `align_up` that to `alignof(CppVtableInfo)`. Currently that's a nop.  Up to you.

Right. The pointer_delta variant was not so nice.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19623#discussion_r1640972138
PR Review Comment: https://git.openjdk.org/jdk/pull/19623#discussion_r1640972956