RFR: 8354954: Typed static memory for late initialization of static class members in Hotspot

[Johan Sjölen]

On Thu, 17 Apr 2025 07:23:40 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Your implementation has a lot of UB, I believe a conformant implementation should look like this:

That implementation won't work for any non-trivial types. Is my code actually UB? This should be no different to a placement-new allocation of memory returned by `malloc`, or from `mmap`, or from `void*` received from anywhere else in the program, as far as I can see. We depend on having well-aligned and well-sized data being able to be used as memory for constructing non-trivial datatypes a lot in Hotspot.

See this Godbolt for your code: https://godbolt.org/z/8Ez7ae7es
And mine: https://godbolt.org/z/WanfEsb3q

At least UBSAN doesn't complain. GCC's static analyzer does, but I think it's a false positive (it believes that `free` is called on the pointer to `s`, but it's clearly not).

> P.S: I saw where your confusion comes from now :) You need an anonymous union member, you also need a constructor and a destructor for `S`, like this:
> 
>     struct S {
>         union {
>             F _f;
>         }
> 
>         S() {}
>         ~S() {}
>     }
> 
> The reason is that an anonymous union acts as if the members of the union are the members of the enclosing class, which means that the class constructor will construct the members of the union and the class destructor will destroy them. If you are insisted on named union you will need to provide the constructor and destructor to the union type:
> 
>     struct S {
>         union U {
>             F _f;
> 
>             U() {}
>             ~U() {}
>         };
>         U u;
>     }
> 
> https://godbolt.org/z/xdTKTjWE7

@merykitty,

Is this the correct implementation??

```c++
#include <new>

// A memory area with adequate size and alignment for storage of a T.
template<typename T>
class alignas(alignof(T)) StaticArea {
    union {
        T _t;
    };

public:
  StaticArea() {

  }
  ~StaticArea() {}

  T* operator->() {
        return as();
  }

    T* as() {
        return std::launder(reinterpret_cast<T*>(this));
    }

  template <class... Ts>
  void init(Ts&... args) {
    new (as()) T(args...);
  }
};

> src/hotspot/share/utilities/staticArea.hpp line 58:
> 
>> 56: 
>> 57:   T* as() {
>> 58:     return reinterpret_cast<T*>(this);
> 
> I don't think this is legal. Normally the placement new returns a pointer and you have to use that pointer to access the newly initialized object. Reinterpreting the pointer passed into the placement new needs `std::launder` to be conformant.

Then I don't see how we can do this, as we really want to avoid having to store a pointer.

> src/hotspot/share/utilities/staticArea.hpp line 62:
> 
>> 60:   T* operator->() {
>> 61:     assert(!is_death_pattern(), "Potential access to uninitialized memory");
>> 62:     return as();
> 
> This is pretty contradictory as you need the content here to be an `uint32_t` for the `is_death_pattern` call to be valid, but you need it to be a `T` so that the following operation can be executed. I can easily see the compiler getting upset with this aliasing.

We don't compile with strict aliasing enabled, so I think this is fine, even if it goes againt the spec.

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

PR Comment: https://git.openjdk.org/jdk/pull/24689#issuecomment-2812289873
PR Comment: https://git.openjdk.org/jdk/pull/24689#issuecomment-2812391324
PR Review Comment: https://git.openjdk.org/jdk/pull/24689#discussion_r2048621048
PR Review Comment: https://git.openjdk.org/jdk/pull/24689#discussion_r2048627969