RFR(S): JDK-8203481 Incorrect constraint for unextended_sp in frame:safe_for_sender

[Daniel D. Daugherty]

On 6/14/18 7:36 AM, Dmitry Samersoff wrote:
> David,
>
> When we constructing frame [1] we get interpreter_frame_sender_sp from
> fp()[unextended_sp_offset = -1] and store it as _unextended_sp.
>
> Previously, interpreter stored caller_sp there.
>
> But it's possible that caller_sp is below or above sp of current frame.

That last sentence is not making sense to me.

At least on X64, that last sentence doesn't make sense to me.


So I'm in an interpreter frame and I have a caller SP value saved
in my frame. I'm about to make a call (create a new frame) and you
are saying that the current sp in my current frame might be below
or above the saved caller SP value. I expect it to be below where
below means cur SP <= saved caller SP.

Dan



>
> One of such cases is a top level frame when we iterate frames from
> JVM_GetStackAccessControlContext. Iteration of recursive frames from JFR
> FullStackTrace is the other case.
>
>
> 1.
>
> frame frame::sender_for_interpreter_frame(RegisterMap* map) const {
>    // SP is the raw SP from the sender after adapter or interpreter
>    // extension.
>    intptr_t* sender_sp = this->sender_sp();
>
>    // This is the sp before any possible extension (adapter/locals).
>    intptr_t* unextended_sp = interpreter_frame_sender_sp();
>
> ...
>
> // sender_sp
> intptr_t* frame::interpreter_frame_sender_sp() const {
>    assert(is_interpreted_frame(), "interpreted frame expected");
>    return (intptr_t*) at(interpreter_frame_sender_sp_offset);
> }
>
> -Dmitry\S
>
>
>
> On 11.06.2018 00:05, David Holmes wrote:
>> Hi Dmitry,
>>
>> I have to admit I still can't connect all the pieces here. When we are
>> operating in frame::safe_for_sender, what has set _unextended_sp? Can
>> you walk through a complete example? I can't quite connect Andrew's
>> example to the original problem.
>>
>> It still seems to me that the current frame should have:
>>
>> sp <= unextended_sp <= stack_base()
>>
>> ??
>>
>> Thanks,
>> David
>>
>> On 11/06/2018 2:54 AM, Dmitry Samersoff wrote:
>>> Hello Everybody,
>>>
>>> Please, review updated webrev:
>>>
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8203481/webrev.02
>>>
>>> CR link:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8203481
>>>
>>> -Dmitry
>>>
>>> On 05/21/2018 04:44 PM, Dmitry Samersoff wrote:
>>>> Hello Everybody,
>>>>
>>>> Please review small fix
>>>>
>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8203481/webrev.01/
>>>>
>>>> CR:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8203481
>>>>
>>>> Testing:
>>>>
>>>> jfr tests that depends to safe_for_sender functionality
>>>>
>>>> ./jdk/jdk/jfr/api/consumer/TestRecordedFullStackTrace.java
>>>> ./jdk/jdk/jfr/event/profiling/TestFullStackTrace.java
>>>>
>>>> fails on AARCH64.
>>>>
>>>> These tests passed after the fix.
>>>>
>>>>
>>>
>