8231634: SA stack walking fails with "illegal bci"

Reviewed-by: amenkov, sspitsyn

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ConstMethod.java

@@ -194,6 +194,17 @@ public class ConstMethod extends Metadata {
 
   // bytecode accessors
 
+  /** See if address is in the Method's bytecodes */
+  public boolean isAddressInMethod(Address bcp) {
+    Address bytecodeStart = getAddress().addOffsetTo(bytecodeOffset);
+    Address bytecodeEnd = bytecodeStart.addOffsetTo(getCodeSize() - 1);
+    if (bcp.greaterThanOrEqual(bytecodeStart) && bcp.lessThanOrEqual(bytecodeEnd)) {
+      return true;
+    } else {
+      return false;
+    }
+  }
+
   /** Get a bytecode or breakpoint at the given bci */
   public int getBytecodeOrBPAt(int bci) {
     return getAddress().getJByteAt(bytecodeOffset + bci) & 0xFF;
@@ -296,7 +307,8 @@ public class ConstMethod extends Metadata {
     }
 
     if (Assert.ASSERTS_ENABLED) {
-      Assert.that(bci == 0 || 0 <= bci && bci < getCodeSize(), "illegal bci");
+        Assert.that(0 <= bci && bci < getCodeSize(),
+                    "illegal bci(" + bci + ") codeSize(" + getCodeSize() + ")");
     }
     int bestBCI  =  0;
     int bestLine = -1;

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ThreadStackTrace.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -47,7 +47,7 @@ public class ThreadStackTrace {
 
     public void dumpStack(int maxDepth) {
         if (!thread.isJavaThread()) {
-            System.out.println("dumpStack: not java Thread returning");
+            System.out.println("dumpStack: not java Thread.");
             return;
         }
         try {

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/x86/X86Frame.java

@@ -444,17 +444,23 @@ public class X86Frame extends Frame {
     // FIXME: this is not atomic with respect to GC and is unsuitable
     // for use in a non-debugging, or reflective, system. Need to
     // figure out how to express this.
-    Address bcp = addressOfInterpreterFrameBCX().getAddressAt(0);
-
-    // If we are in the top level frame then the bcp  may have been set for us. If so then let it
-    // take priority. If we are in a top level interpreter frame, the bcp is live in R13 (on x86)
-    // and not saved in the BCX stack slot.
-    if (live_bcp != null) {
-        bcp = live_bcp;
-    }
 
     Address methodHandle = addressOfInterpreterFrameMethod().getAddressAt(0);
     Method method = (Method)Metadata.instantiateWrapperFor(methodHandle);
+    Address bcp = addressOfInterpreterFrameBCX().getAddressAt(0);
+
+    // If we are in the top level frame then the bcp may have been set for us. If so then let it
+    // take priority. If we are in a top level interpreter frame, the bcp is live in R13 (on x86_64)
+    // and not saved in the BCX stack slot.
+    if (live_bcp != null) {
+        // Only use live_bcp if it points within the Method's bytecodes. Sometimes R13 is used
+        // for scratch purposes and is not a valid BCP. If it is not valid, then we stick with
+        // the bcp stored in the frame, which R13 should have been flushed to.
+        if (method.getConstMethod().isAddressInMethod(live_bcp)) {
+            bcp = live_bcp;
+        }
+    }
+
     return bcpToBci(bcp, method);
   }
 

test/jdk/ProblemList.txt

@@ -937,7 +937,6 @@ java/util/ServiceLoader/ReloadTest.java                     8242935 generic-all
 # svc_tools
 
 sun/tools/jhsdb/BasicLauncherTest.java                          8211767 linux-ppc64,linux-ppc64le
-sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java              8231634 generic-all
 
 
 ############################################################################