RFR(S) 8161598,,Kitchensink fails: assert(nm->insts_contains(original_pc)) failed: original PC must be in nmethod/CompiledMethod

[Daniel D. Daugherty]

This is very nicely done and a very good find!
I think this will make AsyncGetCallTrace() on X86/X64
more stable.

Dan

On 8/16/16 10:00 AM, dean.long at oracle.com wrote:
> Thanks Coleen.
>
> dl
>
>
> On 8/16/16 7:52 AM, Coleen Phillimore wrote:
>>
>> I think this looks good.
>>
>> Coleen
>>
>>
>> On 8/15/16 1:57 PM, dean.long at oracle.com wrote:
>>> Thanks Fred.
>>>
>>> Still waiting for a Reviewer.
>>>
>>> dl
>>>
>>>
>>> On 8/15/16 6:26 AM, Frederic Parain wrote:
>>>> Thank you,
>>>>
>>>> Looks good to me.
>>>>
>>>> Fred
>>>>
>>>> On 08/12/2016 04:19 PM, dean.long at oracle.com wrote:
>>>>> Sure:
>>>>>
>>>>> http://cr.openjdk.java.net/~dlong/8161598/webrev.1/
>>>>>
>>>>> dl
>>>>>
>>>>>
>>>>> On 8/12/16 6:46 AM, Frederic Parain wrote:
>>>>>> Dean,
>>>>>>
>>>>>> In file macroAssembler_x86.cpp, could it be possible to
>>>>>> get rid of the clear_pc argument? It seems completely
>>>>>> useless now.
>>>>>>
>>>>>> Fred
>>>>>>
>>>>>>
>>>>>> On 08/09/2016 01:39 PM, dean.long at oracle.com wrote:
>>>>>>> Ping.
>>>>>>>
>>>>>>> dl
>>>>>>>
>>>>>>>
>>>>>>> On 8/4/16 3:28 PM, dean.long at oracle.com wrote:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8161598
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~dlong/8161598/webrev/
>>>>>>>>
>>>>>>>> Sorry, this issue is Confidential.  The problem is similar to 
>>>>>>>> 8029441,
>>>>>>>> where we suspend a thread and use 
>>>>>>>> pd_get_top_frame_for_profiling() to
>>>>>>>> get the top frame for stack walking.  The problem is "last Java 
>>>>>>>> frame"
>>>>>>>> anchor frames on x86. In lots of places we do not store 
>>>>>>>> last_Java_pc.
>>>>>>>> This is OK in the synchronous stack walk case done by the current
>>>>>>>> thread.  But in the asynchronous case, there are small windows 
>>>>>>>> where
>>>>>>>> it's not always safe to get PC from sp[-1].
>>>>>>>>
>>>>>>>> The solution is not to treat x86 anchor frames as "always 
>>>>>>>> walkable".
>>>>>>>> Instead, we follow the example of sparc and make them walking by
>>>>>>>> filling in last_Java_pc when it's safe.
>>>>>>>>
>>>>>>>> I went for the minimal fix, resetting clear_pc to true in
>>>>>>>> reset_last_Java_frame() but not changing the API and all the 
>>>>>>>> callers.
>>>>>>>> I can fix this if reviewers feel strongly about it.
>>>>>>>>
>>>>>>>> dl
>>>>>>>>
>>>>>>>
>>>>>
>>>
>>
>
>