RFR: 8289925: Shared code shouldn't reference the platform specific method frame::interpreter_frame_last_sp() [v2]

[Richard Reingruber]

On Fri, 26 Aug 2022 08:10:00 GMT, Dean Long <dlong at openjdk.org> wrote:

> > The interpreter allocates a new frame reserving space for the maximum expression stack. For a call it is trimmed to the current size of the stack. Consequently unextended_sp < sp is possible if max. stack is large.
> 
> OK, if I'm not mistaken, aarch64 is doing the same thing.

I have had a look at aarch64 now. There the interpreter also pushes a new frame
reserving space for the maximum expression stack as on ppc64 but the frame is
_not_ trimmed to the current size of the expression stack as on ppc64. So on
aarch64 unextended_sp < sp seems to be impossible.

> So that could mean that code like Continuation::is_frame_in_continuation()
> that uses unextended_sp is already broken on some platforms. For example, if
> an interpreted frame in the carrier thread had a very large max_stack, then
> is_frame_in_continuation() could return true when it should return false.

It is likely not yet broken but for ppc64 it will not work. The usage of
unextended_sp in shared code is problematic because there is no shared
specification for it.

Looking at the callers of is_sp_in_continuation() we find the the following is passed as parameter `sp`:

1. frame::interpreter_frame_last_sp()
2. frame::unextended_sp()
3. frame::sp() - 2
4. frame::sp()

Where 3. is a special case for continuation entry frames (see
Continuation::get_continuation_entry_for_entry_frame()). -2 is needed because
is_sp_in_continuation() would return false for the sp of the entry frame.

I'd like to change the callers of is_sp_in_continuation()

```c++
static inline bool is_sp_in_continuation(const ContinuationEntry* entry, intptr_t* const sp) {
  return entry->entry_sp() > sp;
}


to pass the actual sp. This is correct because the following is true on all
platforms:

```c++
a.sp() > E->entry_sp() > b.sp() > c.sp()


where `a`, `b`, `c` are stack frames in call order and `E` is a ContinuationEntry.
`a` is the caller frame of the continuation entry frame that corresponds to `E`.

is_sp_in_continuation() will then return true for `b.sp()` and `c.sp()` and
false for `a.sp()`. At least on ppc64 is_sp_in_continuation() can return true
for `a.unextended_sp()` because of the frame trimming described above. That's
why it should not be used. frame::interpreter_frame_last_sp() should not be used
as it is not declared and specified as a shared method.

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

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