RFR(S): 8038624:interpretedVFrame::expressions() must respect InterpreterOopMap for liveness
Markus Grönlund
markus.gronlund at oracle.com
Wed Apr 2 18:41:31 UTC 2014
Hi Yumin,
Thanks for taking a look.
For 1,
Thanks for pointing out the default initialization step. I have updated the code to use it.
For 2,
>From what I have seen so far, the bci liveness information in the bit_mask for the expression stack is, for all bytecodes (except invoke*), reflecting the current top-of-stack state (tos) *before* the seeked bci is executed. For these, the fr().interpreter_frame_expression_stack_size() should == length.
For invoke* instructions, the bci liveness information in the bit_mask reflects the current top-of-stack state *after* the seeked bci is executed. Here, fr().interpreter_frame_expression_stack_size() will most likely be > length.
This means indexing into via fr().interpreter_frame_expression_stack_at(i) should be safe ( index variable will index subset or equal to real expression stack len -1)
I have updated the webrev to check this invariant in an assertion as well.
Updated webrev: http://cr.openjdk.java.net/~mgronlun/8038624/webrev02/
Thanks
Markus
From: Yumin Qi
Sent: den 2 april 2014 19:33
To: Markus Grönlund; serviceability-dev at openjdk.net; hotspot-runtime-dev
Subject: Re: RFR(S): 8038624:interpretedVFrame::expressions() must respect InterpreterOopMap for liveness
Markus,
Excellent finding and fix. I have question:
324 StackValueCollection* interpretedVFrame::expressions() const {
325
326 int length = 0;
327 InterpreterOopMap oop_mask; <<<< ------------------ default initialize will set _expression_stack_size
to 0.
328
329 if (!method()->is_native()) {
330 // Get oopmap describing oops and int for current bci
331 if (TraceDeoptimization && Verbose) {
332 methodHandle m_h(method());
333 OopMapCache::compute_one_oop_map(m_h, bci(), &oop_mask);
334 } else {
335 method()->mask_for(bci(), &oop_mask);
336 }
337
338 length = oop_mask.expression_stack_size(); <<<<<-------------------------- 1)
339 }
340
341 StackValueCollection* result = new StackValueCollection(length);
342
343 if (0 == length) {
344 return result;
345 }
346
347 int nof_locals = method()->max_locals();
348
349 // handle expressions
350 for(int i=0; i < length; i++) { <<<<------------------------------------------------------2)
351 // Find stack location
352 intptr_t *addr = fr().interpreter_frame_expression_stack_at(i); <<<<------------------- 2)
353
354 // Depending on oop/int put it in the right package
355 StackValue *sv;
356 if (oop_mask.is_oop(i + nof_locals)) {
357 // oop value
358 Handle h(*(oop *)addr);
359 sv = new StackValue(h);
360 } else {
361 // integer
362 sv = new StackValue(*addr);
363 }
364 assert(sv != NULL, "sanity check");
365 result->add(sv);
366 }
367 return result;
368 }
For 1) and 2), the length may not be consistent, this is from your analysis from JIRA, what if
interpreter_frame_expression_stack_size() is different from 'length'? Is there a possibility out of bound here?
Thanks
Yumin
On 4/2/2014 7: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/20140402/eedb49c5/attachment.html>
More information about the hotspot-runtime-dev
mailing list