[GR-68616] Fix DynamicAccessDetectionPhase listing wrong source method when working with inlined callers and avoid duplicates

PullRequest: graal/21810

Author: jormundur00

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/DynamicAccessDetectionFeature.java

@@ -55,8 +55,8 @@
 import java.util.Set;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -71,6 +71,8 @@
 @AutomaticallyRegisteredFeature
 public final class DynamicAccessDetectionFeature implements InternalFeature {
 
+    // We use a ConcurrentSkipListMap, as opposed to a ConcurrentHashMap, to maintain
+    // order of methods by access kind.
     public record MethodsByAccessKind(Map<DynamicAccessDetectionPhase.DynamicAccessKind, CallLocationsByMethod> methodsByAccessKind) {
         MethodsByAccessKind() {
             this(new ConcurrentSkipListMap<>());
@@ -85,7 +87,9 @@ public CallLocationsByMethod getCallLocationsByMethod(DynamicAccessDetectionPhas
         }
     }
 
-    public record CallLocationsByMethod(Map<String, ConcurrentLinkedQueue<String>> callLocationsByMethod) {
+    // We use a ConcurrentSkipListSet, as opposed to a wrapped ConcurrentHashMap, to maintain
+    // order of call locations by method.
+    public record CallLocationsByMethod(Map<String, ConcurrentSkipListSet<String>> callLocationsByMethod) {
         CallLocationsByMethod() {
             this(new ConcurrentSkipListMap<>());
         }
@@ -94,8 +98,8 @@ public Set<String> getMethods() {
             return callLocationsByMethod.keySet();
         }
 
-        public ConcurrentLinkedQueue<String> getMethodCallLocations(String methodName) {
-            return callLocationsByMethod.getOrDefault(methodName, new ConcurrentLinkedQueue<>());
+        public ConcurrentSkipListSet<String> getMethodCallLocations(String methodName) {
+            return callLocationsByMethod.getOrDefault(methodName, new ConcurrentSkipListSet<>());
         }
     }
 
@@ -136,7 +140,7 @@ public static DynamicAccessDetectionFeature instance() {
     public void addCall(String entry, DynamicAccessDetectionPhase.DynamicAccessKind accessKind, String call, String callLocation) {
         MethodsByAccessKind entryContent = callsBySourceEntry.computeIfAbsent(entry, k -> new MethodsByAccessKind());
         CallLocationsByMethod methodCallLocations = entryContent.methodsByAccessKind().computeIfAbsent(accessKind, k -> new CallLocationsByMethod());
-        ConcurrentLinkedQueue<String> callLocations = methodCallLocations.callLocationsByMethod().computeIfAbsent(call, k -> new ConcurrentLinkedQueue<>());
+        ConcurrentSkipListSet<String> callLocations = methodCallLocations.callLocationsByMethod().computeIfAbsent(call, k -> new ConcurrentSkipListSet<>());
         callLocations.add(callLocation);
     }
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/DynamicAccessDetectionPhase.java

@@ -231,7 +231,7 @@ protected void run(StructuredGraph graph, CoreProviders context) {
             MethodInfo methodInfo = getMethodInfo(callTarget.targetMethod());
 
             if (methodInfo != null && sourceEntry != null) {
-                NodeSourcePosition nspToShow = getRootSourcePosition(callTarget.getNodeSourcePosition());
+                NodeSourcePosition nspToShow = callTarget.getNodeSourcePosition();
                 if (nspToShow != null && !dynamicAccessDetectionFeature.containsFoldEntry(nspToShow.getBCI(), nspToShow.getMethod())) {
                     String callLocation = nspToShow.getMethod().asStackTraceElement(nspToShow.getBCI()).toString();
                     dynamicAccessDetectionFeature.addCall(sourceEntry, methodInfo.accessKind(), methodInfo.signature(), callLocation);
@@ -317,14 +317,6 @@ private static String getSourceEntry(AnalysisType callerClass) {
         }
     }
 
-    private static NodeSourcePosition getRootSourcePosition(NodeSourcePosition nodeSourcePosition) {
-        NodeSourcePosition rootNodeSourcePosition = nodeSourcePosition;
-        while (rootNodeSourcePosition != null && rootNodeSourcePosition.getCaller() != null) {
-            rootNodeSourcePosition = rootNodeSourcePosition.getCaller();
-        }
-        return rootNodeSourcePosition;
-    }
-
     public static void clearMethodSignatures() {
         reflectionMethodSignatures.clear();
         resourceMethodSignatures.clear();