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

[Vladimir Kozlov]

On Wed, 19 Mar 2025 00:03:19 GMT, Dean Long <dlong 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.
>
> src/hotspot/share/code/codeBlob.cpp line 156:
> 
>> 154:   } else {
>> 155:     // We need unique and valid not null address
>> 156:     _mutable_data = blob_end();
> 
> It makes me a little nervous pointing this value to real data.  When RelocIterator computes `_current = nm->relocation_begin() - 1`, it should never read or write from that address, but how can we guarantee that?  Any non-null address that is guarateed unmapped would do, or a special protetected page like `bad_page` here: https://github.com/openjdk/jdk/blob/8e530633a9d99d7ce585cafd5573cb89212feee7/src/hotspot/share/runtime/safepointMechanism.cpp#L66.  If using protected memory seems like overkill, then I suggest using a static.  Something like this:
> 
>    static union {
>     relocInfo _dummy[1];
>   } _empty[2];
>   [...]
>   _mutable_data = _empty+1;
> 
> However, I think this is not the first time we have run into this issue with RelocIterator.  Maybe it's time that we rewrote it to avoid this situation?

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)

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

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