On 19/09/2017 1:19 PM, Yasumasa Suenaga wrote:
Thanks David,

BTW, can I push this change after jdk10/master is opened?
I cannot access JPRT.

I think we'd probably prefer this to go into jdk10/hs - once it is open - and for that you need a sponsor.

Thanks,
David


Yasumasa


2017/09/19 午後0:08 "David Holmes" <david.hol...@oracle.com <mailto:david.hol...@oracle.com>>:

    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.plum...@oracle.com
        <mailto:chris.plum...@oracle.com>
        <mailto:chris.plum...@oracle.com
        <mailto:chris.plum...@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>
<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>
<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/>
<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






Reply via email to