RFR: 8300821: UB: Applying non-zero offset to non-null pointer 0xfffffffffffffffe produced null pointer

[Tobias Hartmann]

On Fri, 3 Mar 2023 14:46:51 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

> "UndefinedBehaviorSanitizer" (https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html) in Xcode running on `java --version` discovered  an Undefined Behavior. The reason is in the `next()` method https://github.com/openjdk/jdk/blob/040f5b55bd03bcc2209ece6eebf223ba1fabf824/src/hotspot/share/asm/codeBuffer.cpp#L798 
> 
> In ``RelocIterator::next()`` we get a nullpointer after `_current++`
> https://github.com/openjdk/jdk/blob/040f5b55bd03bcc2209ece6eebf223ba1fabf824/src/hotspot/share/code/relocInfo.hpp#L612
> But this is actually expected: In the constructor of the iterator `RelocIterator::RelocIterator` we have 
> ```c++
> _current = cs->locs_start()-1;
> _end     = cs->locs_end();
> 
> and in our case locs_start() and locs_end() are `null` - so `_current` is `null`-1. After `_current++` both `_end` and `_current` are `null`. Just after `_current++` we then check if `_current == _end` and return `false` (there is no next reloc info)
> 
> ## Solution
> We want to be able to turn on "UndefinedBehaviorSanitizer" and don't have false positives. So we add a check 
> `cs->has_locs()` and only create the iterator if we have reloc info. 
> 
> Also added a sanity check in `RelocIterator::RelocIterator` that checks that either both `_current` and `_end` are null or both are not null.

Looks good to me.

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

Marked as reviewed by thartmann (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12854