RFR 8159284: bigapps/Jetty - assert(jfa->last_Java_sp() > sp()) failed with tracing code in use

[Coleen Phillimore]

Thank you, Fred.

Coleen

On 8/3/16 9:15 AM, Frederic Parain wrote:
> Looks good to me.
> Thank you for the renaming.
>
> Fred
>
> On 08/02/2016 06:19 PM, Coleen Phillimore wrote:
>>
>>
>> On 8/2/16 6:08 PM, Coleen Phillimore wrote:
>>>
>>> Hi, I have another webrev with the name change of
>>> is_entry_frame_valid(), which passes JPRT.
>>
>> http://cr.openjdk.java.net/~coleenp/8159284.02/webrev/index.html
>>
>>>
>>> thanks,
>>> Coleen
>>>
>>>
>>> On 8/2/16 2:28 PM, Frederic Parain wrote:
>>>>
>>>>
>>>> On 08/02/2016 02:22 PM, Coleen Phillimore wrote:
>>>>>
>>>>> Fred, Thank you for reviewing this.
>>>>>
>>>>> On 8/2/16 2:02 PM, Frederic Parain wrote:
>>>>>> Coleen,
>>>>>>
>>>>>> src/cpu/aarch64/vm/frame_aarch64.cpp
>>>>>>
>>>>>>   line 116: return entry_frame_is_safe(thread);
>>>>>>   line 207: return sender.is_entry_frame_safe(thread);
>>>>>>
>>>>>>   Method names are different, is it a typo?
>>>>>>
>>>>>
>>>>> Yes, it is a typo.  Thank you for catching it.
>>>>>
>>>>>> src/cpu/sparc/vm/frame_sparc.cpp
>>>>>>
>>>>>>   No comments
>>>>>>
>>>>>> src/cpu/x86/vm/frame_x86.cpp
>>>>>>
>>>>>>   No comments
>>>>>>
>>>>>> src/share/vm/runtime/frame.cpp
>>>>>>
>>>>>>   No comments
>>>>>>
>>>>>> src/share/vm/runtime/frame.hpp
>>>>>>
>>>>>>   I would expect the new method name to be is_entry_frame_safe()
>>>>>>   as most tester methods begin with "is_" but I this is just a
>>>>>>   personal opinion.
>>>>>
>>>>> I named it entry_frame_is_safe() because I thought it looked less
>>>>> redundant and more distinct in the code fragments where they 
>>>>> occurred:
>>>>>
>>>>>     // Entry frame checks
>>>>>     if (is_entry_frame()) {
>>>>>       // an entry frame must have a valid fp.
>>>>>
>>>>>       if (!fp_safe) return false;
>>>>>
>>>>>       return entry_frame_is_safe(thread);
>>>>>     }
>>>>>
>>>>> is_entry_frame_safe() looks like is_entry_frame() or implies it's a
>>>>> "safe" call to is_entry_frame.
>>>>>
>>>>> I guess it is the standard to start with 'is' for these sorts of
>>>>> tests.   How about is_safe_entry_frame() ?
>>>>
>>>> Sounds good.
>>>>
>>>> Fred
>>>>
>>>>> thanks,
>>>>> Coleen
>>>>>
>>>>>>
>>>>>> Fred
>>>>>>
>>>>>> On 08/01/2016 07:38 PM, Coleen Phillimore wrote:
>>>>>>> Summary: Test condition in assert in frame::safe_for_sender() for
>>>>>>> entry
>>>>>>> frames and return false.
>>>>>>>
>>>>>>> This bug is for a confidential part of the project that needs more
>>>>>>> robustness checks to verify that frame::sender() can be called.
>>>>>>>
>>>>>>> Also refactored into frame::entry_frame_is_safe() because these
>>>>>>> platforms had the same code for entry frames to determine whether
>>>>>>> it is
>>>>>>> still safe to trust this frame to call sender(). These platforms 
>>>>>>> had
>>>>>>> also the same assert in sender_for_entry_frame() that the sampling
>>>>>>> code
>>>>>>> hit.
>>>>>>>
>>>>>>> Tested with our nightly tests on all platforms and bigapps/Jetty.
>>>>>>>
>>>>>>> open webrev at 
>>>>>>> http://cr.openjdk.java.net/~coleenp/8159284.01/webrev
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>
>>>
>>