hg: valhalla/valhalla/hotspot: Fix and cleanup in vwithfield

[Karen Kinnear]

Frederic,

Thank you for the explanation. Got it - I had missed the original value type passed
on the stack by reference needs to be popped off.

thank you,
Karen

> On Jun 7, 2017, at 5:16 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
> 
> Karen,
> 
> The integer returned is not the size of the new value type, it’s a stack
> adjustment value.
> 
> The vwithfield runtime entry returns a new value type in thread->vm_result
> and an integer which is used by the assembly code to adjust the stack
> pointer (the vwithfield bytecode must consume the new field value and the
> original value type passed on the stack before pushing the new value).
> We are doing this because we cannot manipulate the expression stack from
> the runtime and the assembly code doesn’t know the size of
> the new field value (one slot or two slot depending on the field type).
> 
> The bug was that the stack adjustment returned was missing the size of
> the original value type passed on the stack, leading to a “shift” of operands
> on the expression stack because the original value type was never popped.
> This issue caused some GC issues (because of the shift, there was a
> mismatch between the oopmap computed and the real location of references),
> and could eventually caused some stack overflow issues.
> 
> Fred
> 
> 
>> On Jun 7, 2017, at 17:04, Karen Kinnear <karen.kinnear at oracle.com> wrote:
>> 
>> Frederic,
>> 
>> Looks good. Thank you for the cleanup.
>> 
>> One question please - why do you add type2size(T_VALUETYPE) * stackElementSize
>> when you return the size of the new value type result?
>> 
>> thanks,
>> Karen
>> 
>>> On Jun 7, 2017, at 3:41 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
>>> 
>>> Fixing a bug in vwithfield leading to buggy stack management.
>>> Thanks to Mr Simms for having spotted and identify the issue.
>>> I took the opportunity to do a clean up of the vwithfield method,
>>> removing raw pointer arithmetic on the expression stack, using
>>> the frame API instead.
>>> 
>>> Fred
>>> 
>>> 
>>>> On Jun 7, 2017, at 15:39, frederic.parain at oracle.com wrote:
>>>> 
>>>> Changeset: b03e14ea3e9b
>>>> Author:    fparain
>>>> Date:      2017-06-07 15:38 -0400
>>>> URL:       http://hg.openjdk.java.net/valhalla/valhalla/hotspot/rev/b03e14ea3e9b
>>>> 
>>>> Fix and cleanup in vwithfield
>>>> 
>>>> ! src/share/vm/interpreter/interpreterRuntime.cpp
>>>> 
>>> 
>> 
>