RFR: 8332955: ubsan: runningCounters.cpp:48:61: runtime error: member call on null pointer of type 'struct VirtualSpaceList'

[Thomas Stuefe]

On Wed, 29 May 2024 11:06:57 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>>> It seems unfortunate to have to pollute the code with these checks just because we crash during startup. Could this instead be fixed by not calling into `MemTracker::error_report` if we crash too early?
>> 
>> We had those nullptr checks anyway in the getters using VirtualSpaceList::vslist_class(); so it is probably safer to do the same for VirtualSpaceList::vslist_nonclass() .
>> Even if the callers are adjusted and the ubsan detected issue is gone for the moment, it can always happen that the calling code is reorganized a bit in future and we member-function-call again into a nullptr.
>
>> > It seems unfortunate to have to pollute the code with these checks just because we crash during startup. Could this instead be fixed by not calling into `MemTracker::error_report` if we crash too early?
>> 
>> We had those nullptr checks anyway in the getters using VirtualSpaceList::vslist_class(); so it is probably safer to do the same for VirtualSpaceList::vslist_nonclass() .
> 
> The difference is that the class metaspace is optional whereas the nonclass is not. I don't think that null-checks for the class space is a good argument that we should have them for the nonclass metaspace, which never should be null except in the extreme case when we crash before even having fully started the JVM.
> 
> I think I've made my point w.r.t. this, and I'll let Thomas decide if he still thinks that it is nicer to add these null-checks.

@stefank Yes, that was my original thought too. Non-class is mandatory, Class is optional. See discussion in JBS issue.

But making NMT reporting dependent on Metaspace initialization (different subsystem) is a bit awkward, too. And who knows who else wants to query these counters in the future. And when. Returning 0 seemed the simplest way to make ubsan happy.

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

PR Comment: https://git.openjdk.org/jdk/pull/19435#issuecomment-2137265369