Request for reviews (XS): 7106944: assert(_pc == *pc_addr) failed may be too strong

[Vladimir Kozlov]

Should we then check that the frame is deoptimized already and bailout after 
revoke_biases_of_monitors() call? I surprise we did not hit any problems before 
by deoptimizing frame twice.

Vladimir

Tom Rodriguez wrote:
> On Nov 2, 2011, at 10:22 AM, Vladimir Kozlov wrote:
> 
>> Yes, it looks good. But, Christian, can you show (call stack?) how we endup deoptimizing twice? How we enter deoptimization code twice?
> 
> I think it's biased locking revocation.  Here's the assertion point:
> 
> V  [libjvm.so+0x2701328]  void VMError::report(outputStream*)+0x8c8
> V  [libjvm.so+0x2702469]  void VMError::report_and_die()+0x4fd
> V  [libjvm.so+0xe869bf]  void report_vm_error(const char*,int,const char*,const char*)+0x55f
> V  [libjvm.so+0xff8666]  void frame::patch_pc(Thread*,unsigned char*)+0xe6
> V  [libjvm.so+0xfe6e8d]  void frame::deoptimize(JavaThread*)+0x361
> V  [libjvm.so+0xed9622]  void Deoptimization::deoptimize_frame(JavaThread*,long*)+0x55e
> V  [libjvm.so+0xa34674]  void deopt_caller()+0x154
> V  [libjvm.so+0xa35974]  void Runtime1::new_type_array(JavaThread*,klassOopDesc*,int)+0x3c4
> 
> deopt_caller has a check for the caller already being deopted:
> 
> static bool caller_is_deopted() {
>   JavaThread* thread = JavaThread::current();
>   RegisterMap reg_map(thread, false);
>   frame runtime_frame = thread->last_frame();
>   frame caller_frame = runtime_frame.sender(&reg_map);
>   assert(caller_frame.is_compiled_frame(), "must be compiled");
>   return caller_frame.is_deoptimized_frame();
> }
> 
> // Stress deoptimization                                                                                                                                
> static void deopt_caller() {
>   if ( !caller_is_deopted()) {
>     JavaThread* thread = JavaThread::current();
>     RegisterMap reg_map(thread, false);
>     frame runtime_frame = thread->last_frame();
>     frame caller_frame = runtime_frame.sender(&reg_map);
>     Deoptimization::deoptimize_frame(thread, caller_frame.id());
>     assert(caller_is_deopted(), "Must be deoptimized");
>   }
> }
> 
> but down in deoptimize we may revoke biases at a safepoint:
> 
>   if (UseBiasedLocking) {
>     revoke_biases_of_monitors(thread, fr, map);
>   }
>   deoptimize_single_frame(thread, fr);
> 
> This allows a deopt to occur during the safepoint, after the initial guard.
> 
> tom
> 
>> Thanks,
>> Vladimir
>>
>> On 11/2/11 10:01 AM, Tom Rodriguez wrote:
>>> Looks good.
>>>
>>> tom
>>>
>>> On Nov 2, 2011, at 8:53 AM, Christian Thalinger wrote:
>>>
>>>> http://cr.openjdk.java.net/~twisti/7106944/
>>>>
>>>> 7106944: assert(_pc == *pc_addr) failed may be too strong
>>>> Reviewed-by:
>>>>
>>>> Sometimes we are already deoptimizing a frame when we request another
>>>> deoptimization of the same frame.  This makes the assert to fail.
>>>>
>>>> The fix is to add a check to the assert that the return address is the
>>>> same that we are going to patch.
>>>>
>>>> src/cpu/x86/vm/frame_x86.cpp
>>>>
>