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

[Axel Boldt-Christmas]

On Mon, 17 Jun 2024 10:34:39 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:
> 
>   Cleanup.

This approach seems alright to me.

However I am not sure I understood what was the problem with the initial solution where you make the metadata and the payload disjoint. Nor why the following statement no longer applies.

> The current code takes the address of the array member and uses that as the
base of an array access. So it's effectively doing obj._buffer[i] for i > 0.
And that is UB, and ubsan rightly complains.

All of these implementations are now reinterpreting a field of type `T[1]` as a `T[]`. It is unclear to me why using `offset_of` changes anything.

_Side note:_
The thing I liked with the disjoin approach is that you create two objects, one with the `metadata` (the length in this case) and one which is the payload allocated with and created next to the metadata. As a form of attached storage. Using something like this:
```c++
T* payload() {  return reinterpret_cast<T*>(align_up(reinterpret_cast<char*>(this + 1), align_of(T))); }

Which for `CppVtableInfo` boiled down to `&_vtable_size + 1`.

It would be nicer to have a more explicit lifetime for these, such that you allocated the memory, get the `char* metadata` and `char* payload` addresses and then create the objects `new (metadata) MetadataT(...)` followed by `new (payload) PayloadT(...)`.

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

Marked as reviewed by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19623#pullrequestreview-2124553819