RFR(XS) 8029441: assert(!((nmethod*)_cb)->is_deopt_pc(_pc)) failed: invariant broken

[dean.long at oracle.com]

Thanks Dan!

Does a compiler person want to look this over before I push?

dl


On 7/29/16 8:38 AM, Daniel D. Daugherty wrote:
> On 7/28/16 12:53 PM, dean.long at oracle.com wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8029441
>>
>> http://cr.openjdk.java.net/~dlong/8029441/webrev/
>>
>> This assert happens if we are unlucky enough to be at the start of 
>> the deopt handler when another thread is calling 
>> JavaThread::pd_get_top_frame_for_profiling() on the first thread. In 
>> this case, the frame isn't going to be patched, so the assert is not 
>> helping.  I removed the assert, but added a new assert to patch_pc() 
>> to make sure we don't try to patch an unpatchable frame.
>
> Dusting off my stack sampling cobwebs...
>
> src/cpu/sparc/vm/frame_sparc.cpp
>     So frame::init() has an assert about the current compiled PC
>     not being a deopt PC. The sampling thread runs into this assert()
>     in JavaThread::pd_get_top_frame() when it's trying to construct
>     the top frame.
>
>     The target thread happens to be just starting a deopt so that
>     first frame's PC is a deopt PC. Ouch! Very nice catch of race
>     that's probably been there a very long time.
>
>     Update: Not very long in it's current form:
>
>     11069: assert(!((CompiledMethod*)_cb)->is_deopt_pc(_pc), 
> "invariant broken");
>
>     $ hg log -r 11069
>     changeset:   11069:69d081845165
>     user:        rbackman
>     date:        Tue Apr 26 10:28:51 2016 +0200
>     summary:     8151956: Support non-continuous CodeBlobs in HotSpot
>
>     But very old in it's previous form:
>
>     0:     assert(!((nmethod*)_cb)->is_deopt_pc(_pc), "invariant 
> broken");
>
>     So back to SCCS history:
>
>     1.168.1.1 assert(!((nmethod*)_cb)->is_deopt_pc(_pc), "invariant 
> broken");
>
>     $ sp -r1.168.1.1 src/cpu/sparc/vm/frame_sparc.cpp
>     src/cpu/sparc/vm/SCCS/s.frame_sparc.cpp:
>
>     D 1.168.1.1 06/12/07 10:06:37 sgoldman 298 297 00087/00035/00497
>     MRs:
>     COMMENTS:
>     6463133 - patchless deopt. Automatically convert frames so that if 
> they
>     are deoptimized users can only see the original pc. Cache the 
> codeblob
>     that matches a frame's pc.
>
>     So the earlier form of the assert dates back to Steve G's work on 
> deopt.
>
>
>     As for the new assert() in frame::patch_pc(), it looks fine to me
>     but the proof is in the (long term) testing!
>
>     Again, nice catch!
>
> Dan
>
>
>>
>> Tested with "hs-nightly-runtime", "hs-tier0-comp", and all the 
>> individual tests where this crash was happening.
>>
>> dl
>>
>