8153912: Reconsider StackFrame::getFileName and StackFrame::getLineNumber

Add StackFrame::getByteCodeIndex method. Revised getFileName and getLineNumber method.

Reviewed-by: dfuchs, bchristi

jdk/src/java.base/share/classes/java/lang/StackFrameInfo.java

@@ -29,21 +29,20 @@ import jdk.internal.misc.SharedSecrets;
 
 import static java.lang.StackWalker.Option.*;
 import java.lang.StackWalker.StackFrame;
-import java.lang.reflect.Module;
 import java.util.Optional;
 import java.util.OptionalInt;
 
 class StackFrameInfo implements StackFrame {
-    private final static JavaLangInvokeAccess jlInvokeAccess =
+    private final static JavaLangInvokeAccess JLIA =
         SharedSecrets.getJavaLangInvokeAccess();
 
     // Footprint improvement: MemberName::clazz can replace
     // StackFrameInfo::declaringClass.
 
-    final StackWalker walker;
+    private final StackWalker walker;
-    final Class<?> declaringClass;
+    private final Class<?> declaringClass;
-    final Object memberName;
+    private final Object memberName;
-    final short bci;
+    private final short bci;
     private volatile StackTraceElement ste;
 
     /*
@@ -54,9 +53,17 @@ class StackFrameInfo implements StackFrame {
         this.walker = walker;
         this.declaringClass = null;
         this.bci = -1;
-        this.memberName = jlInvokeAccess.newMemberName();
+        this.memberName = JLIA.newMemberName();
     }
 
+    // package-private called by StackStreamFactory to skip
+    // the capability check
+    Class<?> declaringClass() {
+        return declaringClass;
+    }
+
+    // ----- implementation of StackFrame methods
+
     @Override
     public String getClassName() {
         return declaringClass.getName();
@@ -70,31 +77,39 @@ class StackFrameInfo implements StackFrame {
 
     @Override
     public String getMethodName() {
-        return jlInvokeAccess.getName(memberName);
+        return JLIA.getName(memberName);
     }
 
     @Override
-    public final Optional<String> getFileName() {
+    public int getByteCodeIndex() {
-        StackTraceElement ste = toStackTraceElement();
+        return bci;
-        return ste.getFileName() != null ? Optional.of(ste.getFileName()) : Optional.empty();
     }
 
     @Override
-    public final OptionalInt getLineNumber() {
+    public String getFileName() {
-        StackTraceElement ste = toStackTraceElement();
+        if (isNativeMethod())
-        return ste.getLineNumber() > 0 ? OptionalInt.of(ste.getLineNumber()) : OptionalInt.empty();
+            return null;
+
+        return toStackTraceElement().getFileName();
     }
 
     @Override
-    public final boolean isNativeMethod() {
+    public int getLineNumber() {
-        StackTraceElement ste = toStackTraceElement();
+        if (isNativeMethod())
-        return ste.isNativeMethod();
+            return -2;
+
+        return toStackTraceElement().getLineNumber();
+    }
+
+
+    @Override
+    public boolean isNativeMethod() {
+        return JLIA.isNative(memberName);
     }
 
     @Override
     public String toString() {
-        StackTraceElement ste = toStackTraceElement();
+        return toStackTraceElement().toString();
-        return ste.toString();
     }
 
     /**

jdk/src/java.base/share/classes/java/lang/StackStreamFactory.java

@@ -492,7 +492,7 @@ final class StackStreamFactory {
 
             @Override
             final Class<?> at(int index) {
-                return stackFrames[index].declaringClass;
+                return stackFrames[index].declaringClass();
             }
         }
 
@@ -761,7 +761,7 @@ final class StackStreamFactory {
 
             @Override
             final Class<?> at(int index) {
-                return stackFrames[index].declaringClass;
+                return stackFrames[index].declaringClass();
             }
         }
 

jdk/src/java.base/share/classes/java/lang/StackWalker.java

@@ -126,6 +126,20 @@ public final class StackWalker {
          */
         public Class<?> getDeclaringClass();
 
+        /**
+         * Returns the index to the code array of the {@code Code} attribute
+         * containing the execution point represented by this stack frame.
+         * The code array gives the actual bytes of Java Virtual Machine code
+         * that implement the method.
+         *
+         * @return the index to the code array of the {@code Code} attribute
+         *         containing the execution point represented by this stack frame,
+         *         or a negative number if the method is native.
+         *
+         * @jvms 4.7.3 The {@code Code} Attribute
+         */
+        public int getByteCodeIndex();
+
         /**
          * Returns the name of the source file containing the execution point
          * represented by this stack frame.  Generally, this corresponds
@@ -135,12 +149,12 @@ public final class StackWalker {
          * other than a file, such as an entry in a source repository.
          *
          * @return the name of the file containing the execution point
-         *         represented by this stack frame, or empty {@code Optional}
+         *         represented by this stack frame, or {@code null} if
-         *         is unavailable.
+         *         this information is unavailable.
          *
          * @jvms 4.7.10 The {@code SourceFile} Attribute
          */
-        public Optional<String> getFileName();
+        public String getFileName();
 
         /**
          * Returns the line number of the source line containing the execution
@@ -150,12 +164,12 @@ public final class StackWalker {
          * Specification</cite>.
          *
          * @return the line number of the source line containing the execution
-         *         point represented by this stack frame, or empty
+         *         point represented by this stack frame, or a negative number if
-         *         {@code Optional} if this information is unavailable.
+         *         this information is unavailable.
          *
          * @jvms 4.7.12 The {@code LineNumberTable} Attribute
          */
-        public OptionalInt getLineNumber();
+        public int getLineNumber();
 
         /**
          * Returns {@code true} if the method containing the execution point

jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java

@@ -25,6 +25,8 @@
 
 package java.lang.invoke;
 
+import jdk.internal.misc.JavaLangInvokeAccess;
+import jdk.internal.misc.SharedSecrets;
 import sun.invoke.util.BytecodeDescriptor;
 import sun.invoke.util.VerifyAccess;
 
@@ -1143,15 +1145,25 @@ import java.util.Objects;
     }
 
     static {
-        // Allow privileged classes outside of java.lang
+        // StackFrameInfo stores Member and this provides the shared secrets
-        jdk.internal.misc.SharedSecrets.setJavaLangInvokeAccess(new jdk.internal.misc.JavaLangInvokeAccess() {
+        // for stack walker to access MemberName information.
+        SharedSecrets.setJavaLangInvokeAccess(new JavaLangInvokeAccess() {
+            @Override
             public Object newMemberName() {
                 return new MemberName();
             }
+
+            @Override
             public String getName(Object mname) {
                 MemberName memberName = (MemberName)mname;
                 return memberName.getName();
             }
+
+            @Override
+            public boolean isNative(Object mname) {
+                MemberName memberName = (MemberName)mname;
+                return memberName.isNative();
+            }
         });
     }
 }

jdk/src/java.base/share/classes/jdk/internal/misc/JavaLangInvokeAccess.java

@@ -35,4 +35,9 @@ public interface JavaLangInvokeAccess {
      * Returns the name for the given MemberName
      */
     String getName(Object mname);
+
+    /**
+     * Returns {@code true} if the given MemberName is a native method
+     */
+    boolean isNative(Object mname);
 }

jdk/test/java/lang/StackWalker/EmbeddedStackWalkTest.java

@@ -140,8 +140,8 @@ public class EmbeddedStackWalkTest {
                 s.limit(BIG_LOOP)
                  .filter(f -> c.getName().equals(f.getClassName()) && mn.equals(f.getMethodName()))
                  .forEach(f -> {
-                    assertEquals(f.getFileName().get(), fileName);
+                    assertEquals(f.getFileName(), fileName);
-                    int line = f.getLineNumber().getAsInt();
+                    int line = f.getLineNumber();
                     assertTrue(line >= BEGIN_LINE && line <= END_LINE);
 
                     StackTraceElement st = f.toStackTraceElement();

jdk/test/java/lang/StackWalker/StackRecorderUtil.java

@@ -100,8 +100,8 @@ public class StackRecorderUtil implements Iterable<StackRecorderUtil.TestFrame>
             }
             if (!Objects.equals(ste.getClassName(), sf.getClassName())
                 || !Objects.equals(ste.getMethodName(), sf.getMethodName())
-                || !Objects.equals(ste.getFileName(), sf.getFileName().orElse(null))
+                || !Objects.equals(ste.getFileName(), sf.getFileName())
-                || !Objects.equals(ste.getLineNumber(), sf.getLineNumber().orElse(-1))
+                || !Objects.equals(ste.getLineNumber(), sf.getLineNumber())
                 || !Objects.equals(ste.isNativeMethod(), sf.isNativeMethod())) {
                 throw new RuntimeException("StackFrame and StackTraceElement differ: " +
                         "sf=" + sf + ", ste=" + ste);

jdk/test/java/lang/StackWalker/TestBCI.java

@@ -0,0 +1,190 @@
+/*
+ * Copyright (c) 2016, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8140450
+ * @summary Basic test for the StackWalker::getByteCodeIndex method
+ * @modules jdk.jdeps/com.sun.tools.classfile
+ * @run main TestBCI
+ */
+
+import com.sun.tools.classfile.Attribute;
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.Code_attribute;
+import com.sun.tools.classfile.ConstantPoolException;
+import com.sun.tools.classfile.Descriptor;
+import com.sun.tools.classfile.LineNumberTable_attribute;
+import com.sun.tools.classfile.Method;
+
+import java.lang.StackWalker.StackFrame;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import static java.lang.StackWalker.Option.RETAIN_CLASS_REFERENCE;
+
+public class TestBCI {
+    public static void main(String... args) throws Exception {
+        TestBCI test = new TestBCI(Walker.class);
+        System.out.println("Line number table:");
+        test.methods.values().stream()
+            .sorted(Comparator.comparing(MethodInfo::name).reversed())
+            .forEach(System.out::println);
+
+        // walk the stack
+        test.walk();
+    }
+
+    private final Map<String, MethodInfo> methods;
+    private final Class<?> clazz;
+    TestBCI(Class<?> c) throws ConstantPoolException, IOException {
+        Map<String, MethodInfo> methods;
+        String filename = c.getName().replace('.', '/') + ".class";
+        try (InputStream in = c.getResourceAsStream(filename)) {
+            ClassFile cf = ClassFile.read(in);
+            methods = Arrays.stream(cf.methods)
+                .map(m -> new MethodInfo(cf, m))
+                .collect(Collectors.toMap(MethodInfo::name, Function.identity()));
+        }
+        this.clazz = c;
+        this.methods = methods;
+    }
+
+    void walk() {
+        Walker walker = new Walker();
+        walker.m1();
+    }
+
+    void verify(StackFrame frame) {
+        if (frame.getDeclaringClass() != clazz)
+            return;
+
+        int bci = frame.getByteCodeIndex();
+        int lineNumber = frame.getLineNumber();
+        System.out.format("%s.%s bci %d (%s:%d)%n",
+                          frame.getClassName(), frame.getMethodName(), bci,
+                          frame.getFileName(), lineNumber);
+
+        MethodInfo method = methods.get(frame.getMethodName());
+        SortedSet<Integer> values = method.findLineNumbers(bci).get();
+        if (!values.contains(lineNumber)) {
+            throw new RuntimeException("line number for bci: " + bci + " "
+                + lineNumber + " not matched line number table: " + values);
+        }
+    }
+
+    /*
+     * BCIs in the execution stack when StackWalker::forEach is invoked
+     * will cover BCI range in the line number table.
+     */
+    class Walker {
+        final StackWalker walker = StackWalker.getInstance(RETAIN_CLASS_REFERENCE);
+        void m1() {
+            int i = (int)Math.random()+2;
+            m2(i*2);
+        }
+
+        void m2(int i) {
+            i++;
+            m3(i);
+        }
+
+        void m3(int i) {
+            i++; m4(i++);
+        }
+
+        int m4(int i) {
+            walker.forEach(TestBCI.this::verify);
+            return i;
+        }
+    }
+
+    static class MethodInfo {
+        final Method method;
+        final String name;
+        final String paramTypes;
+        final String returnType;
+        final Map<Integer, SortedSet<Integer>> bciToLineNumbers = new HashMap<>();
+        MethodInfo(ClassFile cf, Method m) {
+            this.method = m;
+
+            String name;
+            String paramTypes;
+            String returnType;
+            LineNumberTable_attribute.Entry[] lineNumberTable;
+            try {
+                // method name
+                name = m.getName(cf.constant_pool);
+                // signature
+                paramTypes = m.descriptor.getParameterTypes(cf.constant_pool);
+                returnType = m.descriptor.getReturnType(cf.constant_pool);
+                Code_attribute codeAttr = (Code_attribute)
+                    m.attributes.get(Attribute.Code);
+                lineNumberTable = ((LineNumberTable_attribute)
+                    codeAttr.attributes.get(Attribute.LineNumberTable)).line_number_table;
+            } catch (ConstantPoolException|Descriptor.InvalidDescriptor e) {
+                throw new RuntimeException(e);
+            }
+            this.name = name;
+            this.paramTypes = paramTypes;
+            this.returnType = returnType;
+            Arrays.stream(lineNumberTable).forEach(entry ->
+                bciToLineNumbers.computeIfAbsent(entry.start_pc, _n -> new TreeSet<>())
+                    .add(entry.line_number));
+        }
+
+        String name() {
+            return name;
+        }
+
+        Optional<SortedSet<Integer>> findLineNumbers(int value) {
+            return bciToLineNumbers.entrySet().stream()
+                    .sorted(Map.Entry.comparingByKey(Comparator.reverseOrder()))
+                    .filter(e -> e.getKey().intValue() <= value)
+                    .map(Map.Entry::getValue)
+                    .findFirst();
+        }
+
+        @Override
+        public String toString() {
+            StringBuilder sb = new StringBuilder();
+            sb.append(name);
+            sb.append(paramTypes).append(returnType).append(" ");
+            bciToLineNumbers.entrySet().stream()
+                .sorted(Map.Entry.comparingByKey())
+                .forEach(entry -> sb.append("bci:").append(entry.getKey()).append(" ")
+                                    .append(entry.getValue()).append(" "));
+            return sb.toString();
+        }
+    }
+
+}