review for 7057587: JSR 292 - crash with jruby in test/test_respond_to.rb

[Tom Rodriguez]

On Jun 22, 2011, at 2:06 PM, Vladimir Kozlov wrote:

> Tom Rodriguez wrote:
>> On Jun 22, 2011, at 12:23 PM, Vladimir Kozlov wrote:
>>> Is it possible to add an assert in nmethod::preserve_callee_argument_oops() to verify that invokedynamic has a receiver?
>> Do you mean to verify that the receiver is really a valid oop or something else?
> 
> Yes, check for a valid oop since it will be GC'ed. What confused me is the sentence: "at this point". Does it mean that at some point invokedynamic does not have a valid receiver?

Yes that's confusing.  At a compiled call site the receiver is always there at a point where we might gc.  In other contexts the order of operations might be different.  In the interpreter the implicit receiver is injected after resolution.  I think this might get cleaned up later to be more consistent.

tom

> 
> Thanks,
> Vladimir
> 
>> tom
>>> Otherwise looks good.
>>> 
>>> thanks,
>>> Vladimir
>>> 
>>> Tom Rodriguez wrote:
>>>> http://cr.openjdk.java.net/~never/7057587
>>>> 40 lines changed: 6 ins; 0 del; 34 mod; 4861 unchg
>>>> 7057587: JSR 292 - crash with jruby in test/test_respond_to.rb
>>>> Summary: don't skip receiver when GC'ing compiled invokedynamic callsites
>>>> Reviewed-by:
>>>> When GC'ing at a call site during resolution the arguments to the call
>>>> have to be handled specially.  In most cases the implicit receiver to
>>>> method handle invoke is ignored but in this case it must be treated as
>>>> real so that it is properly GC'ed.  Tested with failing test from
>>>> report.  Also ran jck, regression tests and vm.mlvm tests.
>>>> I also included a fix to the inline level printing.  For method handle
>>>> invokes we don't want to count them against MaxInlineLevel and
>>>> previously this was done by adding a bias to the inline_depth.  This
>>>> screwed up the indenting making the inlining output unreadable.
>>>> Instead we should keep the depth count consistent and adjust the max
>>>> inline level for the subtree.  This is done by keeping that value in
>>>> the InlineTree.  inline_depth was renamed to inline_level to be
>>>> consistent with MaxInlineLevel.