RFR: 8352112: [ubsan] hotspot/share/code/relocInfo.cpp:130:37: runtime error: applying non-zero offset 18446744073709551614 to null pointer

[Boris Ulasevich]

On Tue, 18 Mar 2025 18:01:58 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> Before [JDK-8343789](https://bugs.openjdk.org/browse/JDK-8343789) `relocation_begin()` was never null even when there was no relocations - it pointed to the beginning of constant or code section in such case. It was used by relocation code to simplify code and avoid null checks.
> With that fix `relocation_begin()` points to address in `CodeBlob::_mutable_data` field which could be `nullptr` if there is no relocation and metadata.
> 
> There easy fix is to avoid `nullptr` in `CodeBlob::_mutable_data`. We could do that similar to what we do for `nmethod::_immutable_data`: [nmethod.cpp#L1514](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/nmethod.cpp#L1514).
> 
> Tested tier1-4, stress, xcomp. Verified with failed tests listed in bug report.

Thanks for fixing that!

I managed to reproduce the issue on a fastdebug build on linux-x64 with the --enable-ubsan configuration option. With this change, all UBSAN "applying non-zero offset to null pointer" errors are resolved.

I have some doubts I'd like to discuss.

[1]
This change seems to be a workaround. Setting pointers to nullptr is a standard practice when no meaningful value is available. The RelocationHandler performs pointer arithmetic on the address without checking its validity. Shouldn't the issue be addressed in RelocationHandler instead?

[2]
If we stick to setting reasonable value to _mutable_data, I have concerns about the chosen value. Why blob_end? We can't even be sure it's within the CodeCache range. Wouldn't it be better to set _mutable_data = header_begin()?

Also, it seems not good that _mutable_data is initialized with different values — once in the member initializer list and then again in the method body. Can we set a default value in the member initializer list instead?

Also, I think that we should update the _mutable_data initial value in the second CodeBlob constructor as well. Otherwise CodeBlob::purge can call std::free with nullptr for non-nmethod CodeBlobs (std::free safely handles nullptr, but it's better to avoid relying on it)

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

PR Comment: https://git.openjdk.org/jdk/pull/24100#issuecomment-2734652066