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

[Boris Ulasevich]

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.

This change addresses the bug reported in JDK-8352426 by ensuring that a nullptr relocation table does not result in undefined behavior.

RelocIterator isn’t a simple iterator - it encapsulates a variety of functions beyond just iteration. Reworking the API to support a range-based for loop would require a significant redesign of its interface and behavior. In my opinion, such a rework is beyond the scope of JDK-8352426. If the goal is to modernize the RelocIterator API, including support for range-based for loops, we should either explicitly reformulate JDK-8352426 to include that broader scope, or better yet, create a separate JBS issue to track that as an independent refactoring. What do you think?

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

PR Comment: https://git.openjdk.org/jdk/pull/24203#issuecomment-2751364575