[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: JDK-8187597: WrongTypeException is occurred at CLHSDB jstack after JDK-8186837
From: David Holmes <david.holmes () oracle ! com>
Date: 2017-09-26 10:55:05
Message-ID: 21df4ebd-823b-cf13-b9f7-24b31be35f0a () oracle ! com
[Download RAW message or body]
Hi Yasumasa,
On 26/09/2017 7:31 PM, Yasumasa Suenaga wrote:
> 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?
On its way, but please create the final changeset ready for your
sponsor(s) to directly import in the future.
Thanks,
David
>
> Thanks,
>
> Yasumasa
>
>
> 2017-09-21 9:31 GMT+09:00 David Holmes <david.holmes@oracle.com>:
> > On 21/09/2017 9:57 AM, Yasumasa Suenaga wrote:
> > >
> > > 2017/09/21 午前8:35 "David Holmes" <david.holmes@oracle.com
> > > <mailto:david.holmes@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@oracle.com
> > > <mailto:david.holmes@oracle.com>
> > > <mailto:david.holmes@oracle.com
> > >
> > > <mailto:david.holmes@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@oracle.com
> > > <mailto:chris.plummer@oracle.com>
> > > <mailto:chris.plummer@oracle.com
> > > <mailto:chris.plummer@oracle.com>>
> > > <mailto:chris.plummer@oracle.com
> > > <mailto:chris.plummer@oracle.com>
> > > <mailto:chris.plummer@oracle.com
> > > <mailto:chris.plummer@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
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic