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:
> 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...
> 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