Thanks David, BTW, can I push this change after jdk10/master is opened? I cannot access JPRT.
Yasumasa 2017/09/19 εεΎ0:08 "David Holmes" <[email protected]>: > 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" <[email protected] <mailto: >> [email protected]>>: >> >> 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.hots >> pot.types.basic.BasicType.getField(BasicType.java:206) >> at >> jdk.hotspot.agent/sun.jvm.hots >> pot.types.basic.BasicType.getField(BasicType.java:212) >> at >> jdk.hotspot.agent/sun.jvm.hots >> pot.types.basic.BasicType.getJLongField(BasicType.java:249) >> at >> jdk.hotspot.agent/sun.jvm.hots >> pot.code.NMethod.initialize(NMethod.java:108) >> at >> jdk.hotspot.agent/sun.jvm.hots >> pot.code.NMethod.access$000(NMethod.java:35) >> at >> jdk.hotspot.agent/sun.jvm.hots >> pot.code.NMethod$1.update(NMethod.java:81) >> >> at >> jdk.hotspot.agent/sun.jvm.hots >> pot.runtime.VM.registerVMInitializedObserver(VM.java:451) >> at >> jdk.hotspot.agent/sun.jvm.hots >> pot.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/~ys >> uenaga/JDK-8187597/webrev.00/ >> <http://cr.openjdk.java.net/~y >> suenaga/JDK-8187597/webrev.00/> >> >> >> I cannot access JPRT. So I need reviewer. >> >> >> Thanks, >> >> Yasumasa >> >> >> >> >> >> >>
