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