[GR-33516] Updated fix for inline debuginfo to fix issue 2701

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/InvokeWithExceptionNode.java

@@ -248,6 +248,8 @@ public InvokeNode replaceWithInvoke() {
         AbstractBeginNode oldException = this.exceptionEdge;
         graph().replaceSplitWithFixed(this, newInvoke, this.next());
         GraphUtil.killCFG(oldException);
+        // copy across any original node source position
+        newInvoke.setNodeSourcePosition(getNodeSourcePosition());
         return newInvoke;
     }
 

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

@@ -30,7 +30,6 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
-import java.util.ListIterator;
 import java.util.Map;
 
 import org.graalvm.compiler.debug.DebugContext;
@@ -63,6 +62,10 @@ public class ClassEntry extends StructureTypeEntry {
      * Details of methods located in this instance.
      */
     protected List<MethodEntry> methods;
+    /**
+     * An index of all currently known methods keyed by the unique local symbol name of the method.
+     */
+    private Map<String, MethodEntry> methodsIndex;
     /**
      * A list recording details of all primary ranges included in this class sorted by ascending
      * address range.
@@ -98,6 +101,7 @@ public ClassEntry(String className, FileEntry fileEntry, int size) {
         this.interfaces = new ArrayList<>();
         this.fileEntry = fileEntry;
         this.methods = new ArrayList<>();
+        this.methodsIndex = new HashMap<>();
         this.primaryEntries = new ArrayList<>();
         this.primaryIndex = new HashMap<>();
         this.localFiles = new ArrayList<>();
@@ -137,9 +141,7 @@ public void addDebugInfo(DebugInfoBase debugInfoBase, DebugTypeInfo debugTypeInf
         /* Add details of fields and field types */
         debugInstanceTypeInfo.fieldInfoProvider().forEach(debugFieldInfo -> this.processField(debugFieldInfo, debugInfoBase, debugContext));
         /* Add details of methods and method types */
-        debugInstanceTypeInfo.methodInfoProvider().forEach(methodFieldInfo -> this.methods.add(this.processMethod(methodFieldInfo, debugInfoBase, debugContext, false)));
-        /* Sort methods to improve lookup speed */
-        this.methods.sort(MethodEntry::compareTo);
+        debugInstanceTypeInfo.methodInfoProvider().forEach(debugMethodInfo -> this.processMethod(debugMethodInfo, debugInfoBase, debugContext));
     }
 
     public void indexPrimary(Range primary, List<DebugFrameSizeChange> frameSizeInfos, int frameSize) {
@@ -168,13 +170,19 @@ public void indexSubRange(Range subrange) {
         /* We should already have seen the primary range. */
         assert primaryEntry != null;
         assert primaryEntry.getClassEntry() == this;
-        primaryEntry.addSubRange(subrange);
         FileEntry subFileEntry = subrange.getFileEntry();
         if (subFileEntry != null) {
             indexLocalFileEntry(subFileEntry);
         }
     }
 
+    private void indexMethodEntry(MethodEntry methodEntry) {
+        String methodName = methodEntry.getSymbolName();
+        assert methodsIndex.get(methodName) == null : methodName;
+        methods.add(methodEntry);
+        methodsIndex.put(methodName, methodEntry);
+    }
+
     private void indexLocalFileEntry(FileEntry localFileEntry) {
         if (localFilesIndex.get(localFileEntry) == null) {
             localFiles.add(localFileEntry);
@@ -273,8 +281,8 @@ private void processInterface(String interfaceName, DebugInfoBase debugInfoBase,
         interfaceClassEntry.addImplementor(this, debugContext);
     }
 
-    protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext, boolean fromRangeInfo) {
-        String methodName = debugInfoBase.uniqueDebugString(debugMethodInfo.name());
+    protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
+        String methodName = debugMethodInfo.name();
         String resultTypeName = TypeEntry.canonicalize(debugMethodInfo.valueType());
         int modifiers = debugMethodInfo.modifiers();
         List<String> paramTypes = debugMethodInfo.paramTypes();
@@ -297,8 +305,11 @@ protected MethodEntry processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBa
          * substitution
          */
         FileEntry methodFileEntry = debugInfoBase.ensureFileEntry(debugMethodInfo);
-        return new MethodEntry(methodFileEntry, debugMethodInfo.symbolNameForMethod(), methodName, this, resultType,
-                        paramTypeArray, paramNameArray, modifiers, debugMethodInfo.isDeoptTarget(), fromRangeInfo);
+        MethodEntry methodEntry = new MethodEntry(debugInfoBase, debugMethodInfo, methodFileEntry, methodName,
+                        this, resultType, paramTypeArray, paramNameArray);
+        indexMethodEntry(methodEntry);
+
+        return methodEntry;
     }
 
     @Override
@@ -340,35 +351,19 @@ public ClassEntry getSuperClass() {
     }
 
     public MethodEntry ensureMethodEntryForDebugRangeInfo(DebugRangeInfo debugRangeInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
-        assert listIsSorted(methods);
-        ListIterator<MethodEntry> methodIterator = methods.listIterator();
-        String methodName = debugInfoBase.uniqueDebugString(debugRangeInfo.name());
-        String paramSignature = debugRangeInfo.paramSignature();
-        String returnTypeName = debugRangeInfo.valueType();
-        while (methodIterator.hasNext()) {
-            MethodEntry methodEntry = methodIterator.next();
-            int comparisonResult = methodEntry.compareTo(methodName, paramSignature, returnTypeName);
-            if (comparisonResult == 0) {
-                methodEntry.setInRangeAndUpdateFileEntry(debugInfoBase, debugRangeInfo);
-                if (methodEntry.fileEntry != null) {
-                    /* Ensure that the methodEntry's fileEntry is present in the localsFileIndex */
-                    indexLocalFileEntry(methodEntry.fileEntry);
-                }
-                return methodEntry;
-            } else if (comparisonResult > 0) {
-                methodIterator.previous();
-                break;
+
+        MethodEntry methodEntry = methodsIndex.get(debugRangeInfo.symbolNameForMethod());
+        if (methodEntry == null) {
+            methodEntry = processMethod(debugRangeInfo, debugInfoBase, debugContext);
+        } else {
+            methodEntry.updateRangeInfo(debugInfoBase, debugRangeInfo);
+            /* Ensure that the methodEntry's fileEntry is present in the localsFileIndex */
+            FileEntry methodFileEntry = methodEntry.fileEntry;
+            if (methodFileEntry != null) {
+                indexLocalFileEntry(methodFileEntry);
             }
         }
-        MethodEntry newMethodEntry = processMethod(debugRangeInfo, debugInfoBase, debugContext, true);
-        methodIterator.add(newMethodEntry);
-        return newMethodEntry;
-    }
-
-    private static boolean listIsSorted(List<MethodEntry> list) {
-        List<MethodEntry> copy = new ArrayList<>(list);
-        copy.sort(MethodEntry::compareTo);
-        return list.equals(copy);
+        return methodEntry;
     }
 
     public List<MethodEntry> getMethods() {

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

@@ -35,6 +35,7 @@
 import java.util.Map;
 
 import com.oracle.objectfile.debuginfo.DebugInfoProvider.DebugFileInfo;
+import jdk.vm.ci.meta.ResolvedJavaType;
 import org.graalvm.compiler.debug.DebugContext;
 
 import com.oracle.objectfile.debuginfo.DebugInfoProvider;
@@ -103,7 +104,7 @@ public abstract class DebugInfoBase {
     /**
      * index of already seen classes.
      */
-    private Map<String, ClassEntry> primaryClassesIndex = new HashMap<>();
+    private Map<ResolvedJavaType, ClassEntry> primaryClassesIndex = new HashMap<>();
     /**
      * Index of files which contain primary or secondary ranges.
      */
@@ -238,38 +239,24 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
              */
             String fileName = debugCodeInfo.fileName();
             Path filePath = debugCodeInfo.filePath();
-            String className = TypeEntry.canonicalize(debugCodeInfo.ownerType());
+            ResolvedJavaType ownerType = debugCodeInfo.ownerType();
             String methodName = debugCodeInfo.name();
             int lo = debugCodeInfo.addressLo();
             int hi = debugCodeInfo.addressHi();
             int primaryLine = debugCodeInfo.line();
 
             /* Search for a method defining this primary range. */
-            ClassEntry classEntry = ensureClassEntry(className);
+            ClassEntry classEntry = ensureClassEntry(ownerType);
             MethodEntry methodEntry = classEntry.ensureMethodEntryForDebugRangeInfo(debugCodeInfo, this, debugContext);
             Range primaryRange = new Range(stringTable, methodEntry, lo, hi, primaryLine);
-            debugContext.log(DebugContext.INFO_LEVEL, "PrimaryRange %s.%s %s %s:%d [0x%x, 0x%x]", className, methodName, filePath, fileName, primaryLine, lo, hi);
+            debugContext.log(DebugContext.INFO_LEVEL, "PrimaryRange %s.%s %s %s:%d [0x%x, 0x%x]", ownerType.toJavaName(), methodName, filePath, fileName, primaryLine, lo, hi);
             classEntry.indexPrimary(primaryRange, debugCodeInfo.getFrameSizeChanges(), debugCodeInfo.getFrameSize());
-            debugCodeInfo.lineInfoProvider().forEach(debugLineInfo -> {
-                String fileNameAtLine = debugLineInfo.fileName();
-                Path filePathAtLine = debugLineInfo.filePath();
-                String classNameAtLine = TypeEntry.canonicalize(debugLineInfo.ownerType());
-                String methodNameAtLine = debugLineInfo.name();
-                int loAtLine = lo + debugLineInfo.addressLo();
-                int hiAtLine = lo + debugLineInfo.addressHi();
-                int line = debugLineInfo.line();
-                /*
-                 * Record all subranges even if they have no line or file so we at least get a
-                 * symbol for them and don't see a break in the address range.
-                 */
-                ClassEntry subClassEntry = ensureClassEntry(classNameAtLine);
-                MethodEntry subMethodEntry = subClassEntry.ensureMethodEntryForDebugRangeInfo(debugLineInfo, this, debugContext);
-                Range subRange = new Range(stringTable, subMethodEntry, loAtLine, hiAtLine, line, primaryRange);
-                classEntry.indexSubRange(subRange);
-                try (DebugContext.Scope s = debugContext.scope("Subranges")) {
-                    debugContext.log(DebugContext.VERBOSE_LEVEL, "SubRange %s.%s %s %s:%d 0x%x, 0x%x]", classNameAtLine, methodNameAtLine, filePathAtLine, fileNameAtLine, line, loAtLine, hiAtLine);
-                }
-            });
+            /*
+             * Record all subranges even if they have no line or file so we at least get a symbol
+             * for them and don't see a break in the address range.
+             */
+            debugCodeInfo.lineInfoProvider().forEach(debugLineInfo -> recursivelyAddSubRanges(debugLineInfo, primaryRange, classEntry, debugContext));
+            primaryRange.mergeSubranges(debugContext);
         }));
 
         debugInfoProvider.dataInfoProvider().forEach(debugDataInfo -> debugDataInfo.debugContext((debugContext) -> {
@@ -350,17 +337,60 @@ ClassEntry lookupClassEntry(String typeName) {
         return (ClassEntry) typeEntry;
     }
 
-    private ClassEntry ensureClassEntry(String className) {
+    /**
+     * Recursively creates subranges based on DebugLineInfo including, and appropriately linking,
+     * nested inline subranges.
+     *
+     * @param lineInfo
+     * @param primaryRange
+     * @param classEntry
+     * @param debugContext
+     * @return the subrange for {@code lineInfo} linked with all its caller subranges up to the
+     *         primaryRange
+     */
+    @SuppressWarnings("try")
+    private Range recursivelyAddSubRanges(DebugInfoProvider.DebugLineInfo lineInfo, Range primaryRange, ClassEntry classEntry, DebugContext debugContext) {
+        if (lineInfo == null) {
+            return primaryRange;
+        }
+        /*
+         * We still insert subranges for the primary method but they don't actually count as inline.
+         * we only need a range so that subranges for inline code can refer to the top level line
+         * number
+         */
+        boolean isInline = lineInfo.getCaller() != null;
+        assert (isInline || (lineInfo.name().equals(primaryRange.getMethodName()) && TypeEntry.canonicalize(lineInfo.ownerType().toJavaName()).equals(primaryRange.getClassName())));
+
+        Range caller = recursivelyAddSubRanges(lineInfo.getCaller(), primaryRange, classEntry, debugContext);
+        final String fileName = lineInfo.fileName();
+        final Path filePath = lineInfo.filePath();
+        final ResolvedJavaType ownerType = lineInfo.ownerType();
+        final String methodName = lineInfo.name();
+        final int lo = primaryRange.getLo() + lineInfo.addressLo();
+        final int hi = primaryRange.getLo() + lineInfo.addressHi();
+        final int line = lineInfo.line();
+        ClassEntry subRangeClassEntry = ensureClassEntry(ownerType);
+        MethodEntry subRangeMethodEntry = subRangeClassEntry.ensureMethodEntryForDebugRangeInfo(lineInfo, this, debugContext);
+        Range subRange = new Range(stringTable, subRangeMethodEntry, lo, hi, line, primaryRange, isInline, caller);
+        classEntry.indexSubRange(subRange);
+        try (DebugContext.Scope s = debugContext.scope("Subranges")) {
+            debugContext.log(DebugContext.VERBOSE_LEVEL, "SubRange %s.%s %s %s:%d 0x%x, 0x%x]",
+                            ownerType.toJavaName(), methodName, filePath, fileName, line, lo, hi);
+        }
+        return subRange;
+    }
+
+    private ClassEntry ensureClassEntry(ResolvedJavaType type) {
         /* See if we already have an entry. */
-        ClassEntry classEntry = primaryClassesIndex.get(className);
+        ClassEntry classEntry = primaryClassesIndex.get(type);
         if (classEntry == null) {
-            TypeEntry typeEntry = typesIndex.get(className);
+            TypeEntry typeEntry = typesIndex.get(TypeEntry.canonicalize(type.toJavaName()));
             assert (typeEntry != null && typeEntry.isClass());
             classEntry = (ClassEntry) typeEntry;
             primaryClasses.add(classEntry);
-            primaryClassesIndex.put(className, classEntry);
+            primaryClassesIndex.put(type, classEntry);
         }
-        assert (classEntry.getTypeName().equals(className));
+        assert (classEntry.getTypeName().equals(TypeEntry.canonicalize(type.toJavaName())));
         return classEntry;
     }