RFR(S): 8038624:interpretedVFrame::expressions() must respect InterpreterOopMap for liveness

[Markus Grönlund]

Hi Coleen,

 

Deoptimization does not use this code for anything critical - deoptimization is based on compiledVFrame::expressions(), not InterpretedVFrame::expressions().

 

After deopting the compiled frame to an interpreter frame, there is an debug section in vframeArrayElement::unpack_on_stack (#ifndef PRODUCT which will call InterpretedVFrame::expressions() if you have set the TraceDeoptimization && Verbose), which is used for outputting debug information only.

 

Thanks

Markus

 

From: Coleen Phillimore 
Sent: den 2 april 2014 22:57
To: hotspot-runtime-dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(S): 8038624:interpretedVFrame::expressions() must respect InterpreterOopMap for liveness

 


Markus,

This is really disturbing because deoptimization uses this code to layout the interpreter frame (unpack_on_stack).  Bugs in deoptimization can be very subtle but we would see some bugs if this was wrong, I think.
Is this a recent failure and there was a change in some other area that made this crash?
I don't know why interpreter_frame_stack_size wouldn't be the same as oopmap.expression_stack_size() unless it doesn't include the pushed arguments?

There's some odd code I've never understood for deoptimization in interpreter/interpreter.cpp.   I think you should have someone from the compiler group (added) who understands deoptimization review your change.

Coleen

On 4/2/14 10:29 AM, Markus Grönlund wrote:

Greetings,

 

Kindly asking for reviews for the following change:

 

Bug(s): http://bugs.openjdk.java.net/browse/JDK-8038624 

https://bugs.openjdk.java.net/browse/JDK-8038344 

 

Webrev: HYPERLINK "http://cr.openjdk.java.net/%7Emgronlun/8038624/webrev01/"http://cr.openjdk.java.net/~mgronlun/8038624/webrev01/ 

 

Problem description:

 

An InterpreterOopMap for a particular bci position does not include expression/operand stack liveness info in the oop_mask/bit_mask if the bci is a call instruction, i.e. for the invoke* instructions (invokevirtual, invokespecial, invokestatic, invokedynamic, invokeinterface).

 

This leads to a discrepancy between what is actually on the expression/operand stack (given via fr().interpreter_frame_expression_stack_size()) and what is given in the liveness oop_mask/bit_mask (given via InterpreterOopMap) at a particular bci.

The code in interpretedVFrame::expressions() is currently based on information given from fr().interpreter_frame_expression_stack_size(), and will index into the retrieved oop_mask/bit_mask based on this information (expression slot nr + _max_locals). These indexes either: 

1. Fetches a 0 (since no live info at that position in the mask) if the index is low enough to still be inside the bit_mask word boundary. It will then proceed to treat the expression slot (which might be a real reference) as a T_INT (0 is a value, 1 is a reference) 

2. Indexes out of bounds for the oop_map/bit_mask (see https://bugs.openjdk.java.net/browse/JDK-8038344 ), and picks up information outside that is not related to a liveness bit mask. If that position happens to yield a 1, but the real expression slot is a value ("v"), the system can assert "(obj->is_oop()) failed: not an oop: 0x00000001"

 

Tested by running:

 

nsk/jdi/*

 

 

Other info:

 

I dislike having to create a new StackValueCollection even though I know the length is 0 and it will not be actively used. However, this pattern of always creating and returning empty objects is prevalent in this piece of code and is not easily detangled.

 

 

Thanks in advance

Markus

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140404/ee56e2b1/attachment-0001.html>