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

[Rickard Bäckman]

On 01/20, Zoltán Majó wrote:
> 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?

I think it is a good idea since it will avoid any situation where there
is confusion about whether it is right to use stub_begin() >= or
stub_begin() > as well as inst_end() <= and inst_end() <

/R

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