Hi Yasumasa,
On 19/09/2017 12:55 PM, Yasumasa Suenaga wrote:
> Thanks Chris, Robbin,
>> I'm waiting reviewer(s) for this change.
Reviewed.
This simply reverts the change of 8185102.
Thanks,
David
-----
>> Yasumasa
>>> 2017/09/19 午前7:14 "Chris Plummer" <chris.plummer at oracle.com> <mailto:chris.plummer at oracle.com>>:
>> Hi Yasumasa,
>> Ok, I see now that CIntegerField is just an interface, so it's up to
> a class to implement getValue() to fetch the field. I'm a bit
> unclear on how that part works, but from responses by others, it
> seems this is ok.
>> I've run all the tests I can find that use jstack or jhsdb, and the
> assert was not triggered. Probably need to have a NMethod on the
> stack to trigger the code you are fixing.
>> thanks,
>> Chris
>>> On 9/17/17 1:13 AM, Yasumasa Suenaga wrote:
>> Hi Chris,
>> I've tested this issue on Fedora 26 x86_64.
> I think we can sue CIntegerField at this point because
> CIntegerField is not specialized for various int size [1].
> In fact, CIntegerField had been used at this point [2], and HSDB
> worked fine.
>>> Thanks,
>> Yasumasa
>>> [1]
>http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29> <http://hg.openjdk.java.net/jdk10/master/file/fd36993f7bf5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/CIntegerField.java#l29>
> [2] http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3> <http://hg.openjdk.java.net/jdk10/master/rev/cbfdbefc6ea3>
>>> On 2017/09/17 3:58, Chris Plummer wrote:
>> Hi Yasumasa,
>> Is this on a 32-bit system? I don't see how you could
> otherwise call getCIntegerField() on a long type. jlong is
> always 64-bit and long is (generally) 32-bit on 32-bit
> systems, and 64-bit on 64-bit systems, at least that seems
> to be the case with linux.
>> From what I can see, _stack_traversal_mark is now the only
> long type in vmStructs.cpp. I don't know that we have a
> mechanism to safely fetch it on both 32-bit and 64-bit systems.
>> _stack_traversal_mark seems to be a long because _traversals
> is also a long.
>> static long _traversals; //
> Stack scan count, also sweep ID.
>> This too might be considered a bug. I'm not sure why you
> would want the size of this field to vary between 32-bit and
> 64-bit systems (adding compiler-dev to help answer that).
>> So, while I would agree that your fix is generally in the
> right direction, I think we first need to revisit the use of
> long for these fields. If they can be changed to an int,
> then your fix is correct (pending the changes to int). If
> not, then maybe we need getCLongField() support.
>> And lastly, we really should have a test to detect this bug.
> Maybe we already do, and it is failing but is going
> unnoticed for some reason. I'll try to look into that some
> more on Monday.
>> thanks,
>> Chris
>> On 9/16/17 5:20 AM, Yasumasa Suenaga wrote:
>> Hi all,
>> I tried to get thread dump via jstack command on CLHSDB.
> But it was failed as below:
>> ```
> Caused by: sun.jvm.hotspot.types.WrongTypeException:
> field "_stack_traversal_mark" in type nmethod is not of
> type jlong, but instead of type long
> at
> jdk.hotspot.agent/sun.jvm.hotspot.types.basic.BasicType.getField(BasicType.java:206)
> at
> jdk.hotspot.agent/sun.jvm.hotspot.types.basic.BasicType.getField(BasicType.java:212)
> at
> jdk.hotspot.agent/sun.jvm.hotspot.types.basic.BasicType.getJLongField(BasicType.java:249)
> at
> jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod.initialize(NMethod.java:108)
> at
> jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod.access$000(NMethod.java:35)
> at
> jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod$1.update(NMethod.java:81)
>> at
> jdk.hotspot.agent/sun.jvm.hotspot.runtime.VM.registerVMInitializedObserver(VM.java:451)
> at
> jdk.hotspot.agent/sun.jvm.hotspot.code.NMethod.<clinit>(NMethod.java:79)
> ... 23 more
> ```
>> I think this exception is caused by JDK-8186837.
> This changeset has changed the type of
> `nmethod::_stack_traversal_mark` to `long` from `jlong`.
>> SA should follow this change.
>> I uploaded a webrev for this issue. This webrev is
> generated from consolidated repo (jdk10/master).
> Could you review it?
>>http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/> <http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.00/>
>>> I cannot access JPRT. So I need reviewer.
>>> Thanks,
>> Yasumasa
>>>>>>