[9] RFR(S): 8156659: assert(CodeCache::find_blob_unsafe(_pc) == _cb) failed: inconsistent

[dean.long at oracle.com]

Looks good to me to.  You might want to change "function" to "method" 
but no big deal.

dl


On 8/17/16 6:54 AM, Tobias Hartmann wrote:
> Hi Nils,
>
> looks good to me!
>
> Best regards,
> Tobias
>
> On 17.08.2016 15:30, Nils Eliasson wrote:
>> Thanks Tobias!
>>
>> I'll push when I have your and Deans ok on the updated webrev.
>>
>> http://cr.openjdk.java.net/~neliasso/8156659/webrev.07
>>
>> Test results have been added to the bug.
>>
>> Regards,
>> Nils
>>
>> On 2016-08-16 14:38, Tobias Hartmann wrote:
>>> Hi Nils,
>>>
>>> thanks for taking care of the issue! I missed that the constructor fetches the PC from the wrong position and, as you mentioned, with Dean's fix for 8029441 we don't need to create a fully initialized frame anyway. Your fix looks good to me (maybe add a comment, as Dean suggested).
>>>
>>> Best regards,
>>> Tobias
>>>
>>> On 15.08.2016 17:28, Nils Eliasson wrote:
>>>> Hi,
>>>>
>>>> After further investigation here is a new patch that needs your review. This code was suggested by Dean Long.
>>>>
>>>> Background:
>>>> This bug (JDK-8156659) was caused by the fix (JDK-8150821) which fixed a bug introduced by "JEP 270: Reserved Stack Areas for Critical Sections".
>>>>
>>>> JDK-8150821 changed the get_frame_at_stack_banging_point to create a fully initialized frame with both an SP and a FP. This frame constructor fetches the PC from the wrong stack position. The programs traps at the stack bang before the SAVE so the calling PC is still in O7. The frame tries to fetch the PC from I7 where it would have ended up after the SAVE.
>>>>
>>>> The original code that was fixed works, but has two problems. It first creates a frame with the wrong PC (the PC of the stack bang, but the SP of the caller), then gets the parent frame and iterates from there. The top most frame will be ignored and not checked properly.
>>>>
>>>> This patch uses the SP from the trap context and the contents of O7 for PC. That is the SP and the PC for the caller of the method that traps on the stack bang.
>>>>
>>>> Testing:
>>>> Reproducer
>>>> Nightly on sparc (will be completed before push)
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156659
>>>> Webrev: http://cr.openjdk.java.net/~neliasso/8156659/webrev.04/
>>>>
>>>> Regards,
>>>> Nils Eliasson
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 2016-06-17 14:26, Vladimir Ivanov wrote:
>>>>> Why aren't other platforms affected?
>>>>>
>>>>> hotspot/src/cpu/x86/vm/frame_x86.cpp:
>>>>>
>>>>> frame frame::sender(RegisterMap* map) const {
>>>>> ...
>>>>>     assert(_cb == CodeCache::find_blob(pc()),"Must be the same");
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>>
>>>>> On 6/17/16 1:52 PM, Nils Eliasson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review,
>>>>>>
>>>>>> Summary:
>>>>>> Test is provoking stack overflow errors and is running with
>>>>>> deoptimzeALot. The recently added method
>>>>>> "look_for_reserved_stack_annotated_method(...)" will traverse the stack
>>>>>> and encounter an deoptimized frame that triggers
>>>>>> "assert(CodeCache::find_blob_unsafe(_pc) == _cb, "inconsistent");". It
>>>>>> fails on deoptimzed frames where the pc is patched and need to be
>>>>>> changed to use raw_pc() to work correctly.
>>>>>>
>>>>>> Testing:
>>>>>> Running hotspot_all
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156659
>>>>>> Webrev: http://cr.openjdk.java.net/~neliasso/8156659/webrev.02/
>>>>>>
>>>>>> Regards,
>>>>>> Nils Eliasson