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

[Zoltán Majó]

Hi Rickard,


On 01/20/2017 10:17 AM, Rickard Bäckman wrote:
> On 01/19, Vladimir Kozlov wrote:
>> Looks good to me too.
>>
>> I am wonder if we have other places where we need to use
>> insts_contains_inclusive().
> Probably. It could also be problematic if we ever did something like
> stub_contains(pc). Which would have been true in this case.
> I checked the code and we don't have any such cases.

thank you for checking the code! I did not find such cases either.

> Another solution would be to always insert at least 1 hlt
> instruction after a call that isn't supposed to return.
> Or at least always if the call is the last instruction in the last block to be
> emitted.

Yes, that would work. Do you think we should file an enhancement for that?

Best regards,


Zoltan

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