RFR: 8352426: RelocIterator should correctly handle nullptr address of relocation data

[Dean Long]

On Mon, 24 Mar 2025 17:04:04 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

> This is a follow-up to the recent #24102 patch. It addresses an issue where RelocIterator may receive a nullptr as the relocation table address. This change can also serve as an independent fix for JDK-8352112.
> 
> RelocIterator::initialize() and RelocIterator::next() perform decrement/increment operations on an internal relocaction pointer.
> If nm->relocation_begin() returns nullptr, this results in undefined behavior, as pointer arithmetic on nullptr is prohibited by the C++ Standard.
> 
> Instead of introducing a null-check (which would add overhead in RelocIterator::next(), a performance-sensitive path), we initialize _current with a dummy static variable. This pointer is never dereferenced, so its actual value is not important - it just serves to avoid undefined behavior.
> 
> RelocIterator::RelocIterator constructor can initialize _current pointer as well. However, in that place we have an assert to ensure that nullptr value is not allowed, and it seems we do not need to apply dummy value there.
> 
> Testing:
> 
> The fix has been verified against the failure in JDK-8352112. The issue no longer reproduces with this patch, regardless of whether the original fix from #24102 is applied.

Looks good.  I wasn't try to suggest that we need to rewrite RelocIterator now to support C++ iterators, but that that kind of interface could avoid the "nullptr - 1" issue, because then the iterator's begin() and end() functions would both return nullptr, and iteration would end because begin() == end().

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

Marked as reviewed by dlong (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24203#pullrequestreview-2714816398