RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with "assert(fr().interpreter_frame_expression_stack_size() >= length) failed: error in expression stack!"

[Coleen Phillimore]

Markus,

I reviewed this.  This looks good.  I don't think there is a way to 
refactor in order to resolve the duplication between the GC code and 
this code but I verified that they look the same.  I appreciate the 
consolidation between InterpreterVFrame::expressions() and locals() though.

Do you have to worry about Serguei's dead locals if oop_mask.is_dead() 
or does that just get a null address?

Thanks,
Coleen

On 6/25/14, 7:58 AM, Markus Grönlund wrote:
> Greetings,
>
>   
>
> Kindly looking for reviews for the following change:
>
>   
>
> Bug: http://bugs.openjdk.java.net/browse/JDK-8039905
>
> Webrev: http://cr.openjdk.java.net/~mgronlun/8039905/webrev01/
>
>   
>
> Description:
>
>   
>
> JVMTI inspection code for following references makes use of a VM_HeapWalkOperation in order to follow-up root sets.
>
>   
>
> In bug:
>
>   
>
> https://bugs.openjdk.java.net/browse/JDK-8038624 - "interpretedVFrame::expressions() must respect InterpreterOopMap for liveness", it was found that the interpretedVFrame code had a discrepancy between basing length information from both asking the interpreter frame (which saw live expression slots for calls instructions) as well as the oop map (which did not).
>
>   
>
> The liveness decisions for a particular BCI should be based on what the oopmap gives, and that was done as of that bug (8038624).
>
>   
>
> In that process, I added an assert in an attempt to validate certain assumptions, and this assert has triggered during nightly testing in some cases.
>
>   
>
> I have therefore inspected the GC code which follow-up roots from an interpreted frame (please see frame::oops_interpreted_do() and InterpreterFrameClosure::do_offset() (in frame.cpp) for reference), and reworked  InterpretedVFrame so that inspections for the locals and expression slots are done in the same way as the GC code (especially in regards to taking decisions on the InterpreterOopMap).
>
>   
>
> I needed to use InterpreterOopMap from a const context, and this is why I have "constified" this class where needed, as well as making "number_of_entries()" a const public accessor (to easily reach oop_mask length info).
>
>   
>
> Testing completed:
>
>   
>
> nsk/jdi* tests (especially the problematic ones reported for 8039905)
>
> compiler/6507107/HeapWalkingTest
>
>   
>
> Thanks
>
> Markus