[9] RFR (S): 8172844: Assert fails in deoptimization due to original PC at the end of code section

[Vladimir Kozlov]

Looks good to me too.

I am wonder if we have other places where we need to use 
insts_contains_inclusive().

Thanks,
Vladimir

On 1/19/17 5:36 AM, Zoltán Majó wrote:
> Hi Rickard,
>
>
> On 01/19/2017 02:22 PM, Rickard Bäckman wrote:
>> Looks good to me.
>
> thank you for the review and for the suggestions.
>
>>
>> Some clean up if you don't mind.
>>
>> frame_aarch64.inline.hpp:
>>
>> 85     assert(((CompiledMethod*)_cb)->insts_contains(_pc), "original
>> PC must be in CompiledMethod");
>> should use _cb->as_compiled_method() instead of the cast.
>>
>> frame_x86.inline.hpp:
>> same as above.
>
> That makes sense. Here is the updated webrev:
> http://cr.openjdk.java.net/~zmajo/8172844/webrev.01/
>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>
>>
>> Thanks
>> /R
>>
>> On 01/19, Zoltán Majó wrote:
>>> Hi,
>>>
>>>
>>> please review the fix for 8172844.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8172844
>>> http://cr.openjdk.java.net/~zmajo/8172844/webrev.00/
>>>
>>> The assert checking the "original pc" (i.e., the pc that was on the
>>> stack before the stack location was overwritten to point to the
>>> deoptimization handler) is too strict. If the last instruction of
>>> the (to-be-)deoptimized method is a call, the pc will point to the
>>> instruction following the call. That instruction is outside of the
>>> code section (as the call is the last instruction in the code
>>> section).
>>>
>>> I suggest we relax the assert to accept the original pc being at the
>>> end of the code section as well. I tested the change with JPRT and
>>> locally with the reproducer, the failure did not appear. RBT is in
>>> progress.
>>>
>>> Thank you!
>>>
>>> Best regards,
>>>
>>>
>>> Zoltan
>>>
>