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

[Vladimir Kozlov]

On Thu, 20 Mar 2025 10:46:10 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> How about this?:
>> 
>> +++ b/src/hotspot/share/code/relocInfo.cpp
>> @@ -117,6 +117,8 @@ void relocInfo::change_reloc_info_for_address(RelocIterator *itr, address pc, re
>>  // Implementation of RelocIterator
>>  
>> +static relocInfo dummy_reloc[2];
>> +
>>  void RelocIterator::initialize(nmethod* nm, address begin, address limit) {
>>    initialize_misc();
>>  
>> @@ -127,8 +129,14 @@ void RelocIterator::initialize(nmethod* nm, address begin, address limit) {
>>    guarantee(nm != nullptr, "must be able to deduce nmethod from other arguments");
>>  
>>    _code    = nm;
>> -  _current = nm->relocation_begin() - 1;
>> -  _end     = nm->relocation_end();
>> +  // Check for no relocations case and use dummy data to avoid referencing wrong data.
>> +  if (nm->relocation_size() == 0) {
>> +    _current = dummy_reloc;
>> +    _end     = dummy_reloc + 1;
>> +  } else {
>> +    _current = nm->relocation_begin() - 1;
>> +    _end     = nm->relocation_end();
>> +  }
>>    _addr    = nm->content_begin();
>>  
>>    // Initialize code sections.
>> 
>> 
>> I filed RFE: [JDK-8352426](https://bugs.openjdk.org/browse/JDK-8352426)
>
> We can just add nullptr checks before pointer arithmetic in relocInfo:
> 
> diff --git a/src/hotspot/share/code/relocInfo.cpp b/src/hotspot/share/code/relocInfo.cpp
> index 7aae32759dd..c694f21e5ca 100644
> --- a/src/hotspot/share/code/relocInfo.cpp
> +++ b/src/hotspot/share/code/relocInfo.cpp
> @@ -127,7 +127,8 @@ void RelocIterator::initialize(nmethod* nm, address begin, address limit) {
>    guarantee(nm != nullptr, "must be able to deduce nmethod from other arguments");
> 
>    _code    = nm;
> -  _current = nm->relocation_begin() - 1;
> +  _current = nm->relocation_begin();
> +  if (_current != nullptr) { _current--; }
>    _end     = nm->relocation_end();
>    _addr    = nm->content_begin();
> 
> diff --git a/src/hotspot/share/code/relocInfo.hpp b/src/hotspot/share/code/relocInfo.hpp
> index 25cca49e50b..b440e713493 100644
> --- a/src/hotspot/share/code/relocInfo.hpp
> +++ b/src/hotspot/share/code/relocInfo.hpp
> @@ -603,7 +603,7 @@ class RelocIterator : public StackObj {
> 
>    // get next reloc info, return !eos
>    bool next() {
> -    _current++;
> +    if (_current != nullptr) { _current++; }
>      assert(_current <= _end, "must not overrun relocInfo");
>      if (_current == _end) {
>        set_has_current(false);

I think we should not add additional check to `next()` - it is performance critical.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24102#discussion_r2006127495