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

[Kim Barrett]

On Fri, 14 Jun 2024 17:20:31 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> We shouldn't specify a wrong array length which causes undefined behavior. Using a "flexible array member".
>
> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use _cloned_vtable[1] and address computation.

Changes requested by kbarrett (Reviewer).

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.

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.

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

PR Review: https://git.openjdk.org/jdk/pull/19623#pullrequestreview-2119651230
PR Review Comment: https://git.openjdk.org/jdk/pull/19623#discussion_r1640577021
PR Review Comment: https://git.openjdk.org/jdk/pull/19623#discussion_r1640577366