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

[Axel Boldt-Christmas]

On Mon, 10 Jun 2024 13:15:17 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> We shouldn't specify a wrong array length which causes undefined behavior. Using a "flexible array member".
>
> I thought flexible array members were a C only thing. 
> 
> I did something along the lines of this when I was experimenting with UBsan. Not sure if it is any better, but it does not use language extensions. Not sure if it is ok to look beyond the object through a `intptr_t*`. But at least it is not through a `intptr_t[1]`. 
> 
> 
> diff --git a/src/hotspot/share/cds/cppVtables.cpp b/src/hotspot/share/cds/cppVtables.cpp
> index c339ce9c0de..55332dc484e 100644
> --- a/src/hotspot/share/cds/cppVtables.cpp
> +++ b/src/hotspot/share/cds/cppVtables.cpp
> @@ -66,19 +66,19 @@
>  
>  class CppVtableInfo {
>    intptr_t _vtable_size;
> -  intptr_t _cloned_vtable[1];
> +  intptr_t _cloned_vtable;
>  public:
>    static int num_slots(int vtable_size) {
>      return 1 + vtable_size; // Need to add the space occupied by _vtable_size;
>    }
>    int vtable_size()           { return int(uintx(_vtable_size)); }
>    void set_vtable_size(int n) { _vtable_size = intptr_t(n); }
> -  intptr_t* cloned_vtable()   { return &_cloned_vtable[0]; }
> -  void zero()                 { memset(_cloned_vtable, 0, sizeof(intptr_t) * vtable_size()); }
> +  intptr_t* cloned_vtable()   { return &_cloned_vtable; }
> +  void zero()                 { memset(&_cloned_vtable, 0, sizeof(intptr_t) * vtable_size()); }
>    // Returns the address of the next CppVtableInfo that can be placed immediately after this CppVtableInfo
>    static size_t byte_size(int vtable_size) {
>      CppVtableInfo i;
> -    return pointer_delta(&i._cloned_vtable[vtable_size], &i, sizeof(u1));
> +    return pointer_delta(&i.cloned_vtable()[vtable_size], &i, sizeof(u1));
>    }
>  };

> @xmas92: Thanks! I have implemented a similar emulation for "flexible array members". Not sure which one is better.

I like yours because it does not look beyond the object through a pointer into the object. It instead creates a pointer beyond the object and uses that. As an attached storage. Just like always, need to be careful with alignment and padding when adding storage beyond the objects representation. But everything is `intptr_t` here so it should all good.

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

PR Comment: https://git.openjdk.org/jdk/pull/19623#issuecomment-2158425186