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

make pd_get_top_frame_for_profiling more robust

    Details

      Description

      Implementations of pd_get_top_frame_for_profiling use frame::safe_for_sender to see if the top frame is in a safe state for stack walking. Safe_for_sender has lots of checks and heuristics, but some of them are not fool-proof, particularly for compiled frames.

      C1/C2 compiled frames have a "frame complete" offset for frame setup that safe_for_sender can use, but no corresponding "frame incomplete" offset for frame tear-down.

      Deoptimization stubs have even more problems. They have no "frame complete" offset, and they have multiple entry points.

      To allow safe_for_sender to work correctly on compiled frames, we would need to map each offset to the frame/stack delta at the location. This would obsolete "frame complete", but would require more meta data about the codeblob. A possible solution would be a new reloc type.

      Alternatively, we could make safe_for_sender more conservative, at the cost of fined-grained profiling, so that it only returns true when the frame is really safe. So instead of blinding declaring deopt stubs safe, it would need to return false if it lacks enough information

        Issue Links

          Activity

          Hide
          zmajo Zoltan Majo (Inactive) added a comment -
          ILW=crash,low frequency of occurrence,no workaround=HLH=P2
          Show
          zmajo Zoltan Majo (Inactive) added a comment - ILW=crash,low frequency of occurrence,no workaround=HLH=P2
          Hide
          zmajo Zoltan Majo (Inactive) added a comment -
          Hi Dean, we need to keep track of this issue, so I'm assigning it to you. We can think later how to take care of it. Zoltan
          Show
          zmajo Zoltan Majo (Inactive) added a comment - Hi Dean, we need to keep track of this issue, so I'm assigning it to you. We can think later how to take care of it. Zoltan
          Hide
          iignatyev Igor Ignatyev added a comment -
          why is this P2 deferred to 10?
          Show
          iignatyev Igor Ignatyev added a comment - why is this P2 deferred to 10?
          Hide
          dlong Dean Long added a comment -
          We don't have a fix, and it probably doesn't deserve to be a P2. The related bugs are P3. The workaround is to not use this kind of profiling.
          Show
          dlong Dean Long added a comment - We don't have a fix, and it probably doesn't deserve to be a P2. The related bugs are P3. The workaround is to not use this kind of profiling.
          Hide
          dlong Dean Long added a comment -
          I don't think we have time to fix this properly for jdk9. The easiest fix would be to ignore top frames that are compiled, but that makes pd_get_top_frame_for_profiling not very useful.
          Show
          dlong Dean Long added a comment - I don't think we have time to fix this properly for jdk9. The easiest fix would be to ignore top frames that are compiled, but that makes pd_get_top_frame_for_profiling not very useful.
          Hide
          dlong Dean Long added a comment -
          An example of what is wrong is the compiled epilogue. We pop the frame, then have a small window before we return where is_frame_complete_at() returns true, the PC is still in compiled code, but the compiled frame is gone. The checks used by safe_for_sender() a susceptible to a false positive if the right junk is on the stack.

          To fix this properly, we need C1/C2/Graal/AOT to create compiled methods with extra metadata so that is_frame_complete_at() [or perhaps a new frame_adjustment_at()] gives correct results for arbitrary locations.
          Show
          dlong Dean Long added a comment - An example of what is wrong is the compiled epilogue. We pop the frame, then have a small window before we return where is_frame_complete_at() returns true, the PC is still in compiled code, but the compiled frame is gone. The checks used by safe_for_sender() a susceptible to a false positive if the right junk is on the stack. To fix this properly, we need C1/C2/Graal/AOT to create compiled methods with extra metadata so that is_frame_complete_at() [or perhaps a new frame_adjustment_at()] gives correct results for arbitrary locations.

            People

            • Assignee:
              dlong Dean Long
              Reporter:
              dlong Dean Long
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated: