Fix debug info for substituted methods

substratevm/mx.substratevm/suite.py

@@ -587,7 +587,7 @@
             "sourceDirs": ["src"],
             "dependencies": [
                 "mx:JUNIT_TOOL",
-                "sdk:GRAAL_SDK",
+                "SVM",
             ],
             "checkstyle": "com.oracle.svm.core",
             "workingSets": "SVM",
@@ -1321,7 +1321,7 @@
           ],
           "distDependencies": [
             "mx:JUNIT_TOOL",
-            "sdk:GRAAL_SDK",
+            "SVM",
             "SVM_CONFIGURE",
           ],
           "testDistribution" : True,

substratevm/mx.substratevm/testhello.py

@@ -263,6 +263,13 @@ def test():
                           r"%s = 1000"%(wildcard_pattern))
         checker.check(exec_string, skip_fails=False)
 
+    # set a break point at hello.Hello$DefaultGreeter::greet
+    # expect "Breakpoint 2 at 0x[0-9a-f]+: file hello/Target_Hello_DefaultGreeter.java, line [0-9]+."
+    exec_string = execute("break hello.Hello$DefaultGreeter::greet")
+    rexp = r"Breakpoint 2 at %s: file hello/Target_hello_Hello_DefaultGreeter\.java, line %s\."%(address_pattern, digits_pattern)
+    checker = Checker("break on substituted method", rexp)
+    checker.check(exec_string, skip_fails=False)
+
     # look up PrintStream.println methods
     # expect "All functions matching regular expression "java.io.PrintStream.println":"
     # expect ""
@@ -284,7 +291,7 @@ def test():
     # set a break point at PrintStream.println(String)
     # expect "Breakpoint 2 at 0x[0-9a-f]+: java.base/java/io/PrintStream.java, line [0-9]+."
     exec_string = execute("break java.io.PrintStream::println(java.lang.String *)")
-    rexp = r"Breakpoint 2 at %s: file .*java/io/PrintStream\.java, line %s\."%(address_pattern, digits_pattern)
+    rexp = r"Breakpoint 3 at %s: file .*java/io/PrintStream\.java, line %s\."%(address_pattern, digits_pattern)
     checker = Checker('break println', rexp)
     checker.check(exec_string, skip_fails=False)
 
@@ -400,7 +407,8 @@ def test():
         print(checker)
         sys.exit(1)
 
-    # continue to next breakpoint
+    # continue to next 2 breakpoints
+    execute("continue")
     execute("continue")
 
     # run backtrace to check we are in java.io.PrintStream::println(java.lang.String)

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java

@@ -302,7 +302,7 @@ protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBa
          * substitution
          */
         FileEntry methodFileEntry = debugInfoBase.ensureFileEntry(fileName, filePath, cachePath);
-        return new MethodEntry(methodFileEntry, methodName, this, resultType, paramTypeArray, paramNameArray, modifiers, debugMethodInfo.isDeoptTarget());
+        return new MethodEntry(methodFileEntry, methodName, this, resultType, paramTypeArray, paramNameArray, modifiers, debugMethodInfo.isDeoptTarget(), debugMethodInfo.isSubstitutionMethod());
     }
 
     @Override
@@ -357,10 +357,11 @@ public MethodEntry getMethodEntry(DebugMethodInfo debugMethodInfo, DebugInfoBase
         String methodName = debugInfoBase.uniqueDebugString(debugMethodInfo.name());
         String paramSignature = debugMethodInfo.paramSignature();
         String returnTypeName = debugMethodInfo.valueType();
+        boolean substitutionMethod = debugMethodInfo.isSubstitutionMethod();
         ListIterator<MethodEntry> methodIterator = methods.listIterator();
         while (methodIterator.hasNext()) {
             MethodEntry methodEntry = methodIterator.next();
-            int comparisonResult = methodEntry.compareTo(methodName, paramSignature, returnTypeName);
+            int comparisonResult = methodEntry.compareTo(methodName, paramSignature, returnTypeName, substitutionMethod);
             if (comparisonResult == 0) {
                 return methodEntry;
             } else if (comparisonResult > 0) {

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/MethodEntry.java

@@ -30,14 +30,18 @@ public class MethodEntry extends MemberEntry implements Comparable<MethodEntry>
     final TypeEntry[] paramTypes;
     final String[] paramNames;
     final boolean isDeoptTarget;
+    final boolean isSubstitution;
 
-    public MethodEntry(FileEntry fileEntry, String methodName, ClassEntry ownerType, TypeEntry valueType, TypeEntry[] paramTypes, String[] paramNames, int modifiers, boolean isDeoptTarget) {
+    public MethodEntry(FileEntry fileEntry, String methodName, ClassEntry ownerType, TypeEntry valueType,
+                    TypeEntry[] paramTypes, String[] paramNames, int modifiers, boolean isDeoptTarget,
+                    boolean isSubstitutionMethod) {
         super(fileEntry, methodName, ownerType, valueType, modifiers);
         assert ((paramTypes == null && paramNames == null) ||
                         (paramTypes != null && paramNames != null && paramTypes.length == paramNames.length));
         this.paramTypes = paramTypes;
         this.paramNames = paramNames;
         this.isDeoptTarget = isDeoptTarget;
+        this.isSubstitution = isSubstitutionMethod;
     }
 
     public String methodName() {
@@ -74,7 +78,7 @@ public String getParamName(int idx) {
         return paramNames[idx];
     }
 
-    public int compareTo(String methodName, String paramSignature, String returnTypeName) {
+    public int compareTo(String methodName, String paramSignature, String returnTypeName, boolean isSubstitutionMethod) {
         int nameComparison = memberName.compareTo(methodName);
         if (nameComparison != 0) {
             return nameComparison;
@@ -100,6 +104,9 @@ public int compareTo(String methodName, String paramSignature, String returnType
                 return paraComparison;
             }
         }
+        if (this.isSubstitution != isSubstitutionMethod) {
+            return this.isSubstitution ? 1 : -1;
+        }
         return 0;
     }
 
@@ -128,6 +135,9 @@ public int compareTo(MethodEntry other) {
                 return paramComparison;
             }
         }
+        if (this.isSubstitution != other.isSubstitution) {
+            return this.isSubstitution ? 1 : -1;
+        }
         return 0;
     }
 }

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debuginfo/DebugInfoProvider.java

@@ -225,6 +225,11 @@ interface DebugMethodInfo extends DebugMemberInfo {
          * @return true if this method has been compiled in as a deoptimization target
          */
         boolean isDeoptTarget();
+
+        /**
+         * @return true if this method substitutes another method
+         */
+        boolean isSubstitutionMethod();
     }
 
     /**

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfInfoSectionImpl.java

@@ -682,7 +682,7 @@ private int writeMethodDeclaration(DebugContext context, ClassEntry classEntry,
         String name = uniqueDebugString(range.getMethodName());
         log(context, "  [0x%08x]     name 0x%x (%s)", pos, debugStringIndex(name), name);
         pos = writeAttrStrp(name, buffer, pos);
-        int fileIdx = classEntry.localFilesIdx();
+        int fileIdx = classEntry.localFilesIdx(range.getFileEntry());
         log(context, "  [0x%08x]     file 0x%x (%s)", pos, fileIdx, range.getFileEntry().getFullName());
         pos = writeAttrData2((short) fileIdx, buffer, pos);
         String returnTypeName = range.getMethodReturnTypeName();

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfLineSectionImpl.java

@@ -436,7 +436,6 @@ private int writeLineNumberTable(DebugContext context, ClassEntry classEntry, by
         String primaryClassName = classEntry.getTypeName();
         String primaryFileName = classEntry.getFileName();
         String file = primaryFileName;
-        int fileIdx = 1;
         log(context, "  [0x%08x] primary class %s", pos, primaryClassName);
         log(context, "  [0x%08x] primary file %s", pos, primaryFileName);
         for (PrimaryEntry primaryEntry : classEntry.getPrimaryEntries()) {
@@ -464,6 +463,7 @@ private int writeLineNumberTable(DebugContext context, ClassEntry classEntry, by
             /*
              * Initialize and write a row for the start of the primary method.
              */
+            int fileIdx = classEntry.localFilesIdx(primaryRange.getFileEntry());
             pos = writeSetFileOp(context, file, fileIdx, buffer, pos);
             pos = writeSetBasicBlockOp(context, buffer, pos);
             /*

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.java

@@ -45,9 +45,7 @@
 import org.graalvm.compiler.graph.NodeSourcePosition;
 import org.graalvm.nativeimage.ImageSingletons;
 
-import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider;
 import com.oracle.graal.pointsto.infrastructure.WrappedJavaMethod;
-import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.objectfile.debuginfo.DebugInfoProvider;
 import com.oracle.svm.core.StaticFieldsSupport;
 import com.oracle.svm.core.SubstrateOptions;
@@ -197,6 +195,20 @@ protected static ResolvedJavaType getJavaType(HostedType hostedType, boolean wan
         return hostedType.getWrapped().getWrapped();
     }
 
+    private static ResolvedJavaType getJavaType(ResolvedJavaMethod method, boolean wantOriginal) {
+        if (method instanceof HostedMethod) {
+            return getJavaType((HostedMethod) method, wantOriginal);
+        }
+        ResolvedJavaMethod wrappedMethod = method;
+        while (wrappedMethod instanceof WrappedJavaMethod) {
+            wrappedMethod = ((WrappedJavaMethod) wrappedMethod).getWrapped();
+        }
+        if (wrappedMethod instanceof SubstitutionMethod) {
+            return ((SubstitutionMethod) wrappedMethod).getAnnotated().getDeclaringClass();
+        }
+        return wrappedMethod.getDeclaringClass();
+    }
+
     protected static ResolvedJavaType getJavaType(HostedMethod hostedMethod, boolean wantOriginal) {
         if (wantOriginal) {
             /* Check for wholesale replacement of the original class. */
@@ -216,6 +228,9 @@ protected static ResolvedJavaType getJavaType(HostedMethod hostedMethod, boolean
             return hostedType.getWrapped().getWrapped();
         }
         ResolvedJavaMethod javaMethod = hostedMethod.getWrapped().getWrapped();
+        if (javaMethod instanceof SubstitutionMethod) {
+            return ((SubstitutionMethod) javaMethod).getAnnotated().getDeclaringClass();
+        }
         return javaMethod.getDeclaringClass();
     }
 
@@ -267,10 +282,9 @@ private abstract class NativeImageDebugFileInfo implements DebugFileInfo {
         @SuppressWarnings("try")
         NativeImageDebugFileInfo(HostedType hostedType) {
             ResolvedJavaType javaType = getJavaType(hostedType, false);
-            Class<?> clazz = hostedType.getJavaClass();
             SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class);
             try (DebugContext.Scope s = debugContext.scope("DebugFileInfo", hostedType)) {
-                fullFilePath = sourceManager.findAndCacheSource(javaType, clazz, debugContext);
+                fullFilePath = sourceManager.findAndCacheSource(javaType, debugContext);
             } catch (Throwable e) {
                 throw debugContext.handle(e);
             }
@@ -280,10 +294,9 @@ private abstract class NativeImageDebugFileInfo implements DebugFileInfo {
         NativeImageDebugFileInfo(HostedMethod hostedMethod) {
             ResolvedJavaType javaType = getJavaType(hostedMethod, false);
             HostedType hostedType = hostedMethod.getDeclaringClass();
-            Class<?> clazz = hostedType.getJavaClass();
             SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class);
             try (DebugContext.Scope s = debugContext.scope("DebugFileInfo", hostedType)) {
-                fullFilePath = sourceManager.findAndCacheSource(javaType, clazz, debugContext);
+                fullFilePath = sourceManager.findAndCacheSource(javaType, debugContext);
             } catch (Throwable e) {
                 throw debugContext.handle(e);
             }
@@ -293,10 +306,9 @@ private abstract class NativeImageDebugFileInfo implements DebugFileInfo {
         NativeImageDebugFileInfo(HostedField hostedField) {
             ResolvedJavaType javaType = getJavaType(hostedField, false);
             HostedType hostedType = hostedField.getDeclaringClass();
-            Class<?> clazz = hostedType.getJavaClass();
             SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class);
             try (DebugContext.Scope s = debugContext.scope("DebugFileInfo", hostedType)) {
-                fullFilePath = sourceManager.findAndCacheSource(javaType, clazz, debugContext);
+                fullFilePath = sourceManager.findAndCacheSource(javaType, debugContext);
             } catch (Throwable e) {
                 throw debugContext.handle(e);
             }
@@ -726,6 +738,11 @@ public boolean isDeoptTarget() {
                 return hostedMethod.isDeoptTarget();
             }
 
+            @Override
+            public boolean isSubstitutionMethod() {
+                return hostedMethod.getWrapped().getWrapped() instanceof SubstitutionMethod;
+            }
+
             @Override
             public int modifiers() {
                 return hostedMethod.getModifiers();
@@ -987,6 +1004,11 @@ public boolean isDeoptTarget() {
             return hostedMethod.isDeoptTarget();
         }
 
+        @Override
+        public boolean isSubstitutionMethod() {
+            return hostedMethod.getWrapped().getWrapped() instanceof SubstitutionMethod;
+        }
+
         @Override
         public List<String> paramTypes() {
             Signature signature = hostedMethod.getSignature();
@@ -1134,6 +1156,14 @@ public boolean isDeoptTarget() {
             return name().endsWith(HostedMethod.METHOD_NAME_DEOPT_SUFFIX);
         }
 
+        @Override
+        public boolean isSubstitutionMethod() {
+            if (method instanceof HostedMethod) {
+                return ((HostedMethod) method).getWrapped().getWrapped() instanceof SubstitutionMethod;
+            }
+            return method instanceof SubstitutionMethod;
+        }
+
         @Override
         public int addressLo() {
             return lo;
@@ -1184,25 +1214,10 @@ public int modifiers() {
 
         @SuppressWarnings("try")
         private void computeFullFilePath() {
-            ResolvedJavaType declaringClass = method.getDeclaringClass();
-            Class<?> clazz = null;
-            if (declaringClass instanceof OriginalClassProvider) {
-                clazz = ((OriginalClassProvider) declaringClass).getJavaClass();
-            }
-            /*
-             * HostedType wraps an AnalysisType and both HostedType and AnalysisType punt calls to
-             * getSourceFilename to the wrapped class so for consistency we need to do the path
-             * lookup relative to the doubly unwrapped HostedType or singly unwrapped AnalysisType.
-             */
-            if (declaringClass instanceof HostedType) {
-                declaringClass = ((HostedType) declaringClass).getWrapped();
-            }
-            if (declaringClass instanceof AnalysisType) {
-                declaringClass = ((AnalysisType) declaringClass).getWrapped();
-            }
+            ResolvedJavaType declaringClass = getJavaType(method, false);
             SourceManager sourceManager = ImageSingletons.lookup(SourceManager.class);
             try (DebugContext.Scope s = debugContext.scope("DebugCodeInfo", declaringClass)) {
-                fullFilePath = sourceManager.findAndCacheSource(declaringClass, clazz, debugContext);
+                fullFilePath = sourceManager.findAndCacheSource(declaringClass, debugContext);
             } catch (Throwable e) {
                 throw debugContext.handle(e);
             }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/sources/SourceManager.java

@@ -26,6 +26,8 @@
 
 package com.oracle.svm.hosted.image.sources;
 
+import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider;
+import com.oracle.svm.hosted.c.GraalAccess;
 import jdk.vm.ci.meta.ResolvedJavaType;
 import org.graalvm.compiler.debug.DebugContext;
 
@@ -46,12 +48,11 @@ public class SourceManager {
      * the source.
      * 
      * @param resolvedType the Java type whose source file should be located and cached
-     * @param clazz the Java class associated with the resolved type
      * @param debugContext context for logging details of any lookup failure
      * @return a path identifying the location of a successfully cached file for inclusion in the
      *         generated debug info or null if a source file cannot be found or cached.
      */
-    public Path findAndCacheSource(ResolvedJavaType resolvedType, Class<?> clazz, DebugContext debugContext) {
+    public Path findAndCacheSource(ResolvedJavaType resolvedType, DebugContext debugContext) {
         /* short circuit if we have already seen this type */
         Path path = verifiedPaths.get(resolvedType);
         if (path != null) {
@@ -68,6 +69,7 @@ public Path findAndCacheSource(ResolvedJavaType resolvedType, Class<?> clazz, De
              */
             if (resolvedType.isInstanceClass() || resolvedType.isInterface()) {
                 String packageName = computePackageName(resolvedType);
+                Class<?> clazz = OriginalClassProvider.getJavaClass(GraalAccess.getOriginalSnippetReflection(), resolvedType);
                 path = locateSource(fileName, packageName, clazz);
                 if (path == null) {
                     // as a last ditch effort derive path from the Java class name

substratevm/src/com.oracle.svm.test/src/hello/Target_hello_Hello_DefaultGreeter.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2020, Red Hat Inc. 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package hello;
+
+import com.oracle.svm.core.annotate.Substitute;
+import com.oracle.svm.core.annotate.TargetClass;
+
+@TargetClass(value = Hello.DefaultGreeter.class)
+final class Target_hello_Hello_DefaultGreeter {
+    @Substitute
+    public void greet() {
+        // Checkstyle: stop
+        System.out.println("Hello, substituted world!");
+        // Checkstyle: resume
+    }
+}