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

[Kim Barrett]

On Sat, 15 Jun 2024 10:01:25 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:
> 
>   Factor out cloned_vtable_offs().

Some more nits or pre-existing issues.

src/hotspot/share/cds/cppVtables.cpp line 70:

> 68:   intptr_t _vtable_size;
> 69:   intptr_t _cloned_vtable[1]; // Pseudo flexible array member.
> 70:   static size_t cloned_vtable_offs() { return offset_of(CppVtableInfo, _cloned_vtable); }

I'd really prefer spelling out "offset" rather than saving two characters with the "offs" abbreviation.

src/hotspot/share/cds/cppVtables.cpp line 73:

> 71: public:
> 72:   static int num_slots(int vtable_size) {
> 73:     return 1 + vtable_size; // Need to add the space occupied by _vtable_size;

Pre-existing: Maybe this ought to be `byte_size() / sizeof(intptr_t)` or something like that? And the
name `num_slots` seems confusing for what this is doing.  Do we actually need both `byte_size`
and `num_slots`?

src/hotspot/share/cds/cppVtables.cpp line 75:

> 73:     return 1 + vtable_size; // Need to add the space occupied by _vtable_size;
> 74:   }
> 75:   int vtable_size()           { return int(uintx(_vtable_size)); }

There's a bunch of pre-existing weirdness around the type of _vtable_size.  (I think _every_ use involves a
conversion.)  Doing anything about that doesn't really belong in this change, but consider a followup cleanup.

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

PR Review: https://git.openjdk.org/jdk/pull/19623#pullrequestreview-2120702112
PR Review Comment: https://git.openjdk.org/jdk/pull/19623#discussion_r1641344772
PR Review Comment: https://git.openjdk.org/jdk/pull/19623#discussion_r1641350527
PR Review Comment: https://git.openjdk.org/jdk/pull/19623#discussion_r1641347531