RFR: JDK-8187597: WrongTypeException is occurred at CLHSDB jstack after JDK-8186837

Yasumasa Suenaga yasuenag at gmail.com
Tue Sep 26 09:31:00 UTC 2017


Hi David,

> You will need to rebase all your patches before they can be sponsored.

I uploaded webrev for jdk10/hs:

  http://cr.openjdk.java.net/~ysuenaga/JDK-8187597/webrev.01/


Could you push it?


Thanks,

Yasumasa


2017-09-21 9:31 GMT+09:00 David Holmes <david.holmes at oracle.com>:
> On 21/09/2017 9:57 AM, Yasumasa Suenaga wrote:
>>
>> 2017/09/21 午前8:35 "David Holmes" <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.com>>:
>>
>>     The opening announcement was somewhat premature. They created
>>     jdk10/hs but we're not quite ready to start accepting changes yet.
>>
>>
>> Where can I get the opening announcement for jdk10/hs?
>
>
> hotspot-dev
>
>> I will send review request after that.
>
>
> You will need to rebase all your patches before they can be sponsored.
>
> Thanks,
> David
>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>>     David
>>
>>
>>     On 21/09/2017 8:44 AM, Yasumasa Suenaga wrote:
>>
>>         Hi David,
>>
>>         jdk10/hs has been opened [1].
>>         Could you push this change?
>>
>>
>>         Thanks,
>>
>>         Yasumasa
>>
>>
>>         [1]
>>
>> http://mail.openjdk.java.net/pipermail/jdk10-dev/2017-September/000499.html
>>
>> <http://mail.openjdk.java.net/pipermail/jdk10-dev/2017-September/000499.html>
>>
>>
>>         On 2017/09/19 12:31, David Holmes wrote:
>>
>>             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.holmes at oracle.com
>>                 <mailto:david.holmes at oracle.com>
>>                 <mailto:david.holmes at oracle.com
>>
>>                 <mailto:david.holmes at 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.plummer at oracle.com
>> <mailto:chris.plummer at oracle.com>
>>                          <mailto:chris.plummer at oracle.com
>>                 <mailto:chris.plummer at oracle.com>>
>>                          <mailto:chris.plummer at oracle.com
>>                 <mailto:chris.plummer at oracle.com>
>>                          <mailto: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>
>>
>>
>> <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>
>>
>>
>> <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>>
>>
>> <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/>>
>>
>> <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
>>
>>
>>
>>
>>
>>
>>
>


More information about the hotspot-compiler-dev mailing list