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

[Stefan Karlsson]

On Wed, 29 May 2024 22:04:46 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> > > val needs an unsigned type to avoid undefined behavior because of signed integer overflow. I'd use uintptr_t.
> > 
> > 
> > Makes sense to use something unsigned. Any good place(s) where to put those templates? For now I would just simply put them into relocInfo.hpp (we can used them if we need to reuse them somewhere else) .
> 
> I would suggest `utilities/globalDefinitions.hpp` somewhere near ` pointer_delta*()`

I'm not fully convinced that this is good idea.

While reading this patch, it is not clear to me that it is correct to hide the warning that ubsan has found. Maybe it is, but I don't see any explanation here showing why it is OK to subtract or add against null here.

This is one reason why I'm reluctant to see these functions getting put into globalDefinitions.hpp. I think that there's a risk that people will start to use these functions without making a full analysis to see if there really is a bug that needs to be solved, if there are some code quality improvements that could be done to get rid of the null pointer, or if it is in fact something that we just want to silence the warning for.

If you really want to go ahead and add these functions, I would like to see them get more descriptive names that explain why they are used instead of plain ++ and --. For example: `add/sub_to_ptr_maybe_null`.

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

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