In light of JDK-8215355 we should check all uses of Thread::stack_base() to watch for range tests that should be exclusive but are inclusive, and vice-versa.

Review comments from JDK-8215355

On 18/11/2019 9:58 pm, Thomas Stüfe wrote:
> There might be more cases like this, e.g.
>
> frame_x86.cpp frame::is_interpreted_frame_valid():
>
> if (locals > thread->stack_base() || locals < (address) fp()) return false;
>
> Many of them could use Thread::in_usable_stack(), I assume.

On 19/11/2019 3:36 am, Daniel D. Daugherty wrote:
> src/hotspot/share/runtime/thread.cpp
>
> Not your issue, but these two lines feel strange/wrong:
>
> L1008: // Allow non Java threads to call this without stack_base
> L1009: if (_stack_base == NULL) return true;
>
> When _stack_base is NULL, any 'adr' is in the caller's stack? The
> comment is not helping understand why this is so...
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaThread.java
>
> Again, not your issue, but these four lines are questionable:
>
> L383 Address sp = lastSPDbg();
> L384 Address stackBase = getStackBase();
> L385 // Be robust
> L386 if (sp == null) return false;
>
> I can see why a NULL sp would cause a "false" return since obviously
> something is a amiss in the frame. However, the C++ code doesn't make
> this check so why does the SA code?
>
> And this code doesn't check stackBase == NULL so it's not matching
> the C++ code either.

> So we have Thread::is_in_usable_stack(), Thread::on_local_stack() and
> Thread::is_in_stack()? I haven't compared all three side by side, but
> there might be some cleanup work that can be done here (in a different
> bug).