RFR: 8163014: Mysterious/wrong value for "long" frame local variable on 64-bit

Max Ockner max.ockner at oracle.com
Mon Sep 12 16:06:43 UTC 2016


New webrev: http://cr.openjdk.java.net/~mockner/8163014.hotspot.04/
  Comments below.
-Max
On 9/9/2016 1:54 PM, dean.long at oracle.com wrote:
>
>   614   movq(Address(rsp, 0), r);
>   615   movptr(Address(rsp, Interpreter::expr_offset_in_bytes(1)), NULL_WORD );
> I wish these two looked more similar.  I guess movq is the same as 
> movprt, and
> Interpreter::expr_offset_in_bytes(0)
> could be used at 614?

Yes. I have made this change.
>
>   329   str(zr, pre(esp, -wordSize));
>   330   str(r, pre(esp, -wordsize));
>
> Do you want to use a single stp here?
>
The current patch is a working code suggestion from Andrew Dinn.  If it 
is important to change, I will see if I can build on aarch64 before 
submitting.

> dl
>
> On 9/9/16 9:51 AM, Max Ockner wrote:
>> Oops. Here is version 2: 
>> http://cr.openjdk.java.net/~mockner/8163014.hotspot.03/
>>
>> Max
>>
>> On 9/8/2016 12:26 PM, Max Ockner wrote:
>>> Here is version 2 of this fix.
>>>  - Added copyright to LocalLongHelper.java
>>>  - Switched LocalLongTest.java to run with -Xint instead of -Xcomp, 
>>> and cleaned up some copypaste garbage.
>>>  - changed the test directory name from locallong to LocalLong.
>>>
>>> Reran test with -Xint.
>>>
>>> Thanks,
>>> Max
>>>
>>> On 9/7/2016 9:47 AM, Coleen Phillimore wrote:
>>>>
>>>> Max,
>>>>
>>>> http://cr.openjdk.java.net/~mockner/8163014.01/test/runtime/locallong/LocalLongHelper.java.html 
>>>>
>>>>
>>>> There's a lot in this test, but I can't think of a different way to 
>>>> inspect the stack to find out if the long unused slot is zeroed. 
>>>> This file is missing a copyright, and I think the directory should 
>>>> be LocalLong (I think our new convention of test directory names is 
>>>> capitalized).  I think we should keep the test in the hotspot 
>>>> repository even though it uses the StackWalk API because otherwise 
>>>> it won't be run with hotspot changes on an ad-hoc or nightly basis.
>>>>
>>>> http://cr.openjdk.java.net/~mockner/8163014.01/test/runtime/locallong/LocalLongTest.java.html 
>>>>
>>>>
>>>> I thought the idea was to run this with -Xint not -Xcomp?
>>>>
>>>> The interpreter changes look good.
>>>>
>>>> Coleen
>>>>
>>>> On 9/6/16 5:33 PM, Max Ockner wrote:
>>>>> Hello,
>>>>> Please review this multi-platform fix for the stack walk API.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8163014
>>>>> Webrev: http://cr.openjdk.java.net/~mockner/8163014.01/
>>>>>
>>>>> In 64 bits, long values can fit into a single slot, but two slots 
>>>>> are still used. The high slot contains garbage. This normally 
>>>>> wouldn't matter since it is never read from but the stack walk API 
>>>>> expects to display useful information. This is an issue when 
>>>>> displaying longs from local variables, so this means we can kill 
>>>>> any garbage off by zeroing it when it is pushed to the stack in 
>>>>> the previous frame. This solution zeroes the high byte of a long 
>>>>> value when it is being pushed to the stack (in push_l).
>>>>>
>>>>> This applies to x86, aarch64, and sparc. This change does not 
>>>>> apply to ppc, though the bug almost certainly does affect it. I 
>>>>> have left ppc untouched since I don't have access to the hardware 
>>>>> required to reproduce the bug and test the fix.
>>>>>
>>>>> I have adapted the reproducer from the bug into a test. It is 
>>>>> curently sitting in runtime/locallong, but I believe there must be 
>>>>> a better place for it.
>>>>>
>>>>> Thanks,
>>>>> Max
>>>>
>>>
>>
>



More information about the hotspot-runtime-dev mailing list