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

[Zoltán Majó]

Hi Vladimir,


On 01/19/2017 08:33 PM, Vladimir Kozlov wrote:
> Looks good to me too.

thank you for the review!

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

I was wondering, too. I found only the frame::frame(...) method to need 
insts_contains_inclusive() (in addition to the 
frame::verify_deopt_original_pc(...) method for which the assert was 
triggered). The patch changes both places.

Thank you!

Best regards,


Zoltan

>
> 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
>>>>
>>