RFR: 8331731: ubsan: relocInfo.cpp:155:30: runtime error: applying non-zero offset 18446744073709551614 to null pointer [v2]

[Stefan Karlsson]

On Thu, 30 May 2024 12:38:44 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

> We're not hiding a warning. Using pointer addition with `nullptr` or result `nullptr` is undefined behavior. So, the current implementation is not guaranteed to do what we expect. It only works because compilers seem to be merciful.

I think you are reading too much into the words I used. Ubsan is warning/complaining/error that the code is problematic.

> However, casting `nullptr` to `uintptr_t` is guaranteed to be 0. So, switching to unsigned integer arithmetics avoids this problem.

I fully understand the suggested patch.

> The `RelocIterator` code is designed to work with such values.

This is the crux of my complaint. Is this a good design given that the usage of null isn't apparent on first read? Or was this only something that grew into existence after a while? And would it make more sense for maintainability to not be doing this? I think it is important to think about those questions when trying to handle all these ubsan failures.

FWIW, I (and Axel) took a step back and asked why do we think it is necessary to support null pointers here and I think you can see the musing around in Axel's response above.

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

PR Comment: https://git.openjdk.org/jdk/pull/19424#issuecomment-2139999240