Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8147039

Incorrect locals and operands in compiled frames

    Details

    • Subcomponent:
    • Resolved In Build:
      b120
    • Verification:
      Verified

      Description

      java.lang.LiveStackFrame is an experimental API to access locals, operands and monitors of a live stack frame.

      LiveStackFrame::getLocals returns an array of the local slots of a live stack frame, similar for getStack and getMonitors methods.

      The VM implementation uses javaVFrame::locals(), javaVFrame::expressions() and jjavaVFrame::monitors() methods.

      It's observed that the returned StackValueCollection for locals and operands are different when running in interpreted frame and compiled frames and also different for C1 and C2.

      1. for long, double local, it occupies two slots. -Xint mode, the value of a long/double local variable obtained from a pair of two local slots matches the expected value.

      The value is incorrect on a compiled frame.

      2. The ordering of locals does not match the index in the bytecode

        Issue Links

          Activity

          Hide
          mchung Mandy Chung added a comment - - edited
          There is a bug in the stackwalk code to get javaVFrame during stack walking using vframeStream:

            static javaVFrame* cast(vframe* vf) {
              assert(vf == NULL || vf->is_java_frame(), "must be java frame");
              return (javaVFrame*) vf;
            }

          For C1 compiled frame, the javaVFrame it returns is out of sync (already advanced to the next java frame). Roland suggests to reimplement stackwalk.cpp not to use vframeStream but javaVFrames and use java_sender() to walk the frames. It'd be good for vframeStream to support locals/operands.

          The reason C2 appears to work okay as the compiled method is deoptimized right away.
          Show
          mchung Mandy Chung added a comment - - edited There is a bug in the stackwalk code to get javaVFrame during stack walking using vframeStream:   static javaVFrame* cast(vframe* vf) {     assert(vf == NULL || vf->is_java_frame(), "must be java frame");     return (javaVFrame*) vf;   } For C1 compiled frame, the javaVFrame it returns is out of sync (already advanced to the next java frame). Roland suggests to reimplement stackwalk.cpp not to use vframeStream but javaVFrames and use java_sender() to walk the frames. It'd be good for vframeStream to support locals/operands. The reason C2 appears to work okay as the compiled method is deoptimized right away.
          Hide
          roland Roland Westrelin added a comment -
          Here is my analysis when I looked at this problem in december:

          I think what you observe is due to a bug in stackwalk.cpp: you don’t see the good number of locals because the locals that the code in stackwalk.cpp collects are for the wrong frame. That’s something you can see with the following assert:

          diff --git a/src/share/vm/prims/stackwalk.cpp b/src/share/vm/prims/stackwalk.cpp
          --- a/src/share/vm/prims/stackwalk.cpp
          +++ b/src/share/vm/prims/stackwalk.cpp
          @@ -133,6 +133,8 @@
              // fill in StackFrameInfo and initialize MemberName
              if (live_frame_info(mode)) {
                Handle stackFrame(frames_array->obj_at(index));
          + javaVFrame* vf = vfst.java_frame();
          + assert(vf->method() == method && vf->bci() == bci, "out of sync");
                fill_live_stackframe(stackFrame, method, bci, vfst.java_frame(), CHECK_0);
              } else if (need_method_info(mode)) {
                Handle stackFrame(frames_array->obj_at(index));

          In that case method:bci is LocalsAndOperands::test:28 and vf->method():vf->bci() is java.lang.StackWalker::walk:10. The explanation is that the current frame is for compiled method LocalsAndOperands::test which inlines java.lang.StackWalker::walk. vfst.method() was moved forward so it skipped over java.lang.StackWalker::walk but when you call vfst.java_frame(), you get a compiledVFrame for the top scope of the current frame that is java.lang.StackWalker::walk. What you get from java_frame() is out of sync with the vframeStream and I don’t think there’s a way to keep them in sync. So I think stackwalk.cpp has to be reimplemented to not use vframeStream but javaVFrames and use java_sender() to walk the frames.

          If you do then I won’t fix https://bugs.openjdk.java.net/browse/JDK-8144851 but you’ll have to remember to apply a similar change: set update_map to true in the RegisterMap that you use.

          The reason C2 appears to work ok, I think, is become the compiled method for LocalsAndOperands::test is deoptimized right away so you’re not really testing a scenario with a compiled method.
          Show
          roland Roland Westrelin added a comment - Here is my analysis when I looked at this problem in december: I think what you observe is due to a bug in stackwalk.cpp: you don’t see the good number of locals because the locals that the code in stackwalk.cpp collects are for the wrong frame. That’s something you can see with the following assert: diff --git a/src/share/vm/prims/stackwalk.cpp b/src/share/vm/prims/stackwalk.cpp --- a/src/share/vm/prims/stackwalk.cpp +++ b/src/share/vm/prims/stackwalk.cpp @@ -133,6 +133,8 @@     // fill in StackFrameInfo and initialize MemberName     if (live_frame_info(mode)) {       Handle stackFrame(frames_array->obj_at(index)); + javaVFrame* vf = vfst.java_frame(); + assert(vf->method() == method && vf->bci() == bci, "out of sync");       fill_live_stackframe(stackFrame, method, bci, vfst.java_frame(), CHECK_0);     } else if (need_method_info(mode)) {       Handle stackFrame(frames_array->obj_at(index)); In that case method:bci is LocalsAndOperands::test:28 and vf->method():vf->bci() is java.lang.StackWalker::walk:10. The explanation is that the current frame is for compiled method LocalsAndOperands::test which inlines java.lang.StackWalker::walk. vfst.method() was moved forward so it skipped over java.lang.StackWalker::walk but when you call vfst.java_frame(), you get a compiledVFrame for the top scope of the current frame that is java.lang.StackWalker::walk. What you get from java_frame() is out of sync with the vframeStream and I don’t think there’s a way to keep them in sync. So I think stackwalk.cpp has to be reimplemented to not use vframeStream but javaVFrames and use java_sender() to walk the frames. If you do then I won’t fix https://bugs.openjdk.java.net/browse/JDK-8144851 but you’ll have to remember to apply a similar change: set update_map to true in the RegisterMap that you use. The reason C2 appears to work ok, I think, is become the compiled method for LocalsAndOperands::test is deoptimized right away so you’re not really testing a scenario with a compiled method.
          Hide
          mchung Mandy Chung added a comment -
          Roland - thanks for the analysis. As we discussed previously, I agree it's a bug in stackwalk and vframeStream::java_frame() that I added.

          I'm looking for vframeStream support to provide the capability to get locals, operands, monitors from a vframe, if it makes sense.
          Show
          mchung Mandy Chung added a comment - Roland - thanks for the analysis. As we discussed previously, I agree it's a bug in stackwalk and vframeStream::java_frame() that I added. I'm looking for vframeStream support to provide the capability to get locals, operands, monitors from a vframe, if it makes sense.
          Hide
          vlivanov Vladimir Ivanov added a comment -
          Mandy, what Roland suggests is to avoid vframeStream and reimplement stack walking using javaVFrame/vframe::java_sender, e.g.:
            if (thread->has_last_Java_frame()) {
              RegisterMap rm(thread);
              for (javaVFrame* vf = thread->last_java_vframe(&rm); vf != NULL; vf = vf->java_sender()) {
          ... (grap locals, operands, monitors)
              }
            }
          Show
          vlivanov Vladimir Ivanov added a comment - Mandy, what Roland suggests is to avoid vframeStream and reimplement stack walking using javaVFrame/vframe::java_sender, e.g.:   if (thread->has_last_Java_frame()) {     RegisterMap rm(thread);     for (javaVFrame* vf = thread->last_java_vframe(&rm); vf != NULL; vf = vf->java_sender()) { ... (grap locals, operands, monitors)     }   }
          Hide
          twisti Christian Thalinger added a comment -
          Will this bug be fixed in 9?
          Show
          twisti Christian Thalinger added a comment - Will this bug be fixed in 9?
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/hs/jdk/rev/d2f46fdfc3ca
          User: bchristi
          Date: 2016-05-06 01:14:31 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs/jdk/rev/d2f46fdfc3ca User: bchristi Date: 2016-05-06 01:14:31 +0000
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/6174ad93770c
          User: bchristi
          Date: 2016-05-07 06:52:00 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/6174ad93770c User: bchristi Date: 2016-05-07 06:52:00 +0000
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/d2f46fdfc3ca
          User: lana
          Date: 2016-05-25 17:36:43 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/d2f46fdfc3ca User: lana Date: 2016-05-25 17:36:43 +0000
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/6174ad93770c
          User: lana
          Date: 2016-05-25 17:36:48 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/6174ad93770c User: lana Date: 2016-05-25 17:36:48 +0000

            People

            • Assignee:
              bchristi Brent Christian
              Reporter:
              mchung Mandy Chung
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: