[GR-67238] Provide layered image build variant of debuginfotests.

PullRequest: graal/21435

Author: dominikmascherbauer

substratevm/mx.substratevm/mx_substratevm.py

@@ -1120,10 +1120,6 @@
                     "jdk.internal.misc",
                     "sun.security.jca",
                 ],
-                "jdk.internal.vm.ci" : [
-                    "jdk.vm.ci.code",
-                    "jdk.vm.ci.meta",
-                ],
             },
             "checkstyle": "com.oracle.svm.test",
             "checkstyleVersion" : "10.21.0",
@@ -1137,19 +1133,46 @@
             "jacoco" : "exclude",
         },
 
-        "com.oracle.svm.test.missing.classes": {
+        "com.oracle.svm.test.debug": {
             "subDir": "src",
             "sourceDirs": ["src"],
             "dependencies": [
+                "sdk:NATIVEIMAGE",
+                "SVM",
             ],
+            "requiresConcealed": {
+                "jdk.internal.vm.ci": [
+                    "jdk.vm.ci.code",
+                    "jdk.vm.ci.meta",
+                ],
+            },
+            "checkstyle": "com.oracle.svm.test",
+            "workingSets": "SVM",
             "annotationProcessors": [
+                "compiler:GRAAL_PROCESSOR",
+                "SVM_PROCESSOR",
+            ],
+            "javaCompliance": "21+",
+            "spotbugs": "false",
+            "jacoco": "exclude",
+            "testProject": True,
+        },
+
+        "com.oracle.svm.test.debug.missing.classes": {
+            "subDir": "src",
+            "sourceDirs": ["src"],
+            "dependencies": [
             ],
             "checkstyle": "com.oracle.svm.test",
-            "javaCompliance" : "21+",
             "workingSets": "SVM",
+            "annotationProcessors": [
+                "compiler:GRAAL_PROCESSOR",
+                "SVM_PROCESSOR",
+            ],
+            "javaCompliance": "21+",
             "spotbugs": "false",
+            "jacoco": "exclude",
             "testProject": True,
-            "jacoco" : "exclude",
         },
 
         "com.oracle.svm.with.space.test": {
@@ -2437,27 +2460,19 @@
             "com.oracle.svm.test",
             "com.oracle.svm.configure.test",
             "com.oracle.svm.graal.test",
+            "com.oracle.svm.test.debug",
+            "com.oracle.svm.test.debug.missing.classes",
             "SVM_TEST_RESOURCE_WITH_SPACE",
           ],
           "distDependencies": [
             "mx:JUNIT_TOOL",
             "sdk:NATIVEIMAGE",
             "SVM",
             "SVM_CONFIGURE",
-            "SVM_TEST_MISSING_CLASSES",
           ],
           "testDistribution" : True,
         },
 
-        "SVM_TEST_MISSING_CLASSES" : {
-            "subDir": "src",
-            "relpath" : True,
-            "dependencies": [
-                "com.oracle.svm.test.missing.classes"
-            ],
-            "testDistribution": True,
-        },
-
         # Special test distribution used for testing inclusion of resources from jar files with a space in their name.
         # The space in the distribution name is intentional.
         "SVM_TESTS WITH SPACE" : {

substratevm/mx.substratevm/suite.py

@@ -48,18 +48,20 @@ public record Local(LocalEntry localEntry, int line) {
     private boolean isInlined;
     private final boolean isOverride;
     private final boolean isConstructor;
+    private final boolean isCompiledInPriorLayer;
     private final int vtableOffset;
     private final String symbolName;
 
     public MethodEntry(FileEntry fileEntry, int line, String methodName, StructureTypeEntry ownerType,
                     TypeEntry valueType, int modifiers, List<LocalEntry> params, String symbolName, boolean isDeopt, boolean isOverride, boolean isConstructor,
-                    int vtableOffset, int lastParamSlot) {
+                    boolean isCompiledInPriorLayer, int vtableOffset, int lastParamSlot) {
         super(fileEntry, line, methodName, ownerType, valueType, modifiers);
         this.params = params;
         this.symbolName = symbolName;
         this.isDeopt = isDeopt;
         this.isOverride = isOverride;
         this.isConstructor = isConstructor;
+        this.isCompiledInPriorLayer = isCompiledInPriorLayer;
         this.vtableOffset = vtableOffset;
         this.lastParamSlot = lastParamSlot;
         this.locals = new ArrayList<>();
@@ -208,6 +210,10 @@ public boolean isConstructor() {
         return isConstructor;
     }
 
+    public boolean isCompiledInPriorLayer() {
+        return isCompiledInPriorLayer;
+    }
+
     public boolean isVirtual() {
         return vtableOffset >= 0;
     }

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

@@ -605,7 +605,7 @@ private int writeInstanceClasses(DebugContext context, byte[] buffer, int p) {
              * declaration in the owner type CU. Other information is already written to the
              * corresponding type units.
              */
-            if (classEntry.getMethods().stream().anyMatch(m -> m.isInRange() || m.isInlined()) ||
+            if (classEntry.getMethods().stream().anyMatch(m -> m.isInRange() || m.isInlined() || m.isCompiledInPriorLayer()) ||
                             classEntry.getFields().stream().anyMatch(DwarfInfoSectionImpl::isManifestedStaticField)) {
                 setCUIndex(classEntry, pos);
                 pos = writeInstanceClassInfo(context, classEntry, buffer, pos);
@@ -1279,7 +1279,7 @@ private int writeField(DebugContext context, StructureTypeEntry entry, FieldEntr
     private int writeSkeletonMethodDeclarations(DebugContext context, ClassEntry classEntry, byte[] buffer, int p) {
         int pos = p;
         for (MethodEntry method : classEntry.getMethods()) {
-            if (method.isInRange() || method.isInlined()) {
+            if (method.isInRange() || method.isInlined() || method.isCompiledInPriorLayer()) {
                 /*
                  * Declare all methods whether or not they have been compiled or inlined.
                  */
@@ -1357,7 +1357,7 @@ private int writeSkeletonMethodParameterDeclaration(DebugContext context, LocalE
     private int writeMethodDeclarations(DebugContext context, ClassEntry classEntry, byte[] buffer, int p) {
         int pos = p;
         for (MethodEntry method : classEntry.getMethods()) {
-            if (method.isInRange() || method.isInlined()) {
+            if (method.isInRange() || method.isInlined() || method.isCompiledInPriorLayer()) {
                 /*
                  * Declare all methods whether they have been compiled or inlined.
                  */

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

@@ -508,6 +508,11 @@ private int writeCompiledMethodLineInfo(DebugContext context, ClassEntry classEn
                             pos = writeAdvancePCOp(context, addressDelta, buffer, pos);
                         }
                     }
+                    /*
+                     * Add a row to the line number table and reset some flags. This makes sure a
+                     * debugger can stop here and properly process line and address. Special opcodes
+                     * have a similar effect as the copy operation, hence it is only used here.
+                     */
                     pos = writeCopyOp(context, buffer, pos);
                 }
             }

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

@@ -733,10 +733,11 @@ private MethodEntry createMethodEntry(SharedMethod method) {
         boolean isOverride = isOverride(method);
         boolean isDeopt = method.isDeoptTarget();
         boolean isConstructor = method.isConstructor();
+        boolean isCompiledInPriorLayer = isCompiledInPriorLayer(method);
 
         return new MethodEntry(fileEntry, line, methodName, ownerType,
                         valueType, modifiers, params, symbolName, isDeopt, isOverride, isConstructor,
-                        vTableOffset, lastParamSlot);
+                        isCompiledInPriorLayer, vTableOffset, lastParamSlot);
     }
 
     /**
@@ -1010,6 +1011,10 @@ public boolean isOverride(@SuppressWarnings("unused") SharedMethod method) {
         return false;
     }
 
+    public boolean isCompiledInPriorLayer(@SuppressWarnings("unused") SharedMethod method) {
+        return false;
+    }
+
     public boolean isVirtual(@SuppressWarnings("unused") SharedMethod method) {
         return false;
     }
@@ -1217,7 +1222,7 @@ protected LocalEntry lookupLocalEntry(String name, TypeEntry typeEntry, int slot
     /**
      * Lookup a {@code LocalValueEntry}. This can either be a register, stack, constant value. This
      * allows to reuse the same entries for multiple ranges.
-     * 
+     *
      * @param localValueEntry the {@code LocalValueEntry} to lookup
      * @return the {@code LocalValueEntry} from the lookup map
      */
@@ -1403,7 +1408,7 @@ private Map<LocalEntry, LocalValueEntry> initSyntheticInfoList(ParamLocationProd
                 debug.log(DebugContext.DETAILED_LEVEL, "local[%d] %s type %s slot %d", paramIdx + 1, param.name(), param.type().getTypeName(), param.slot());
                 debug.log(DebugContext.DETAILED_LEVEL, "  =>  %s", value);
             }
-            LocalValueEntry localValueEntry = createLocalValueEntry(value, PRE_EXTEND_FRAME_SIZE);
+            LocalValueEntry localValueEntry = createLocalValueEntry(value, PRE_EXTEND_FRAME_SIZE, true);
             if (localValueEntry != null) {
                 localValueInfos.put(param, localValueEntry);
             }
@@ -1565,7 +1570,13 @@ public Range process(CompilationResultFrameTree.FrameNode node, CallRange caller
             LineNumberTable lineNumberTable = method.getLineNumberTable();
             int line = lineNumberTable == null ? -1 : lineNumberTable.getLineNumber(pos.getBCI());
 
-            Map<LocalEntry, LocalValueEntry> localValueInfos = initLocalInfoList(pos, methodEntry, frameSize);
+            /*
+             * Locals are stored in both leaf ranges and call ranges. However, for call ranges we do
+             * not want to store register values as registers may be overwritten in callee ranges.
+             * Thus, a debugger is still able to process stack values and constants for stack
+             * traces.
+             */
+            Map<LocalEntry, LocalValueEntry> localValueInfos = initLocalInfoList(pos, methodEntry, frameSize, isLeaf);
             Range locationInfo = Range.createSubrange(primary, methodEntry, localValueInfos, node.getStartPos(), node.getEndPos() + 1, line, callerInfo, isLeaf);
 
             if (debug.isLogEnabled()) {
@@ -1585,9 +1596,10 @@ public Range process(CompilationResultFrameTree.FrameNode node, CallRange caller
      * @param pos the bytecode position of the location info
      * @param methodEntry the {@code MethodEntry} corresponding to the bytecode position
      * @param frameSize the current frame size
+     * @param isLeaf whether the range to create this value for is a leaf range
      * @return a mapping from {@code LocalEntry} to {@code LocalValueEntry}
      */
-    protected Map<LocalEntry, LocalValueEntry> initLocalInfoList(BytecodePosition pos, MethodEntry methodEntry, int frameSize) {
+    protected Map<LocalEntry, LocalValueEntry> initLocalInfoList(BytecodePosition pos, MethodEntry methodEntry, int frameSize, boolean isLeaf) {
         Map<LocalEntry, LocalValueEntry> localInfos = new HashMap<>();
 
         if (pos instanceof BytecodeFrame frame && frame.numLocals > 0) {
@@ -1688,7 +1700,7 @@ protected Map<LocalEntry, LocalValueEntry> initLocalInfoList(BytecodePosition po
                      * upfront from the local variable table, the LocalEntry already exists.
                      */
                     LocalEntry localEntry = methodEntry.getOrAddLocalEntry(lookupLocalEntry(name, typeEntry, slot), line);
-                    LocalValueEntry localValueEntry = createLocalValueEntry(value, frameSize);
+                    LocalValueEntry localValueEntry = createLocalValueEntry(value, frameSize, isLeaf);
                     if (localEntry != null && localValueEntry != null) {
                         localInfos.put(localEntry, localValueEntry);
                     }
@@ -1703,16 +1715,24 @@ protected Map<LocalEntry, LocalValueEntry> initLocalInfoList(BytecodePosition po
 
     /**
      * Creates a {@code LocalValueEntry} for a given {@code JavaValue}. This processes register
-     * values, stack values, primitive constants and constant in the heap.
+     * values, stack values, primitive constants and constant in the heap. Register values are only
+     * added for leaf frames, as register may be overwritten within callee frames of a call range.
+     * But, stack values and constants also work for call ranges.
      * 
      * @param value the given {@code JavaValue}
      * @param frameSize the frame size for stack values
+     * @param isLeaf whether the range to create this value for is a leaf range
      * @return the {@code LocalValueEntry} or {@code null} if the value can't be processed
      */
-    private LocalValueEntry createLocalValueEntry(JavaValue value, int frameSize) {
+    private LocalValueEntry createLocalValueEntry(JavaValue value, int frameSize, boolean isLeaf) {
         switch (value) {
             case RegisterValue registerValue -> {
-                return lookupLocalValueEntry(new RegisterValueEntry(registerValue.getRegister().number));
+                /*
+                 * We only want to create register entries for leaf nodes. Thus, they are only valid
+                 * within a leaf range but not over a whole call range where the register value is
+                 * potentially overwritten.
+                 */
+                return isLeaf ? lookupLocalValueEntry(new RegisterValueEntry(registerValue.getRegister().number)) : null;
             }
             case StackSlot stackValue -> {
                 int stackSlot = frameSize == 0 ? stackValue.getRawOffset() : stackValue.getOffset(frameSize);
@@ -1827,7 +1847,7 @@ private Range createEmbeddedParentLocationInfo(PrimaryRange primary, Compilation
         LineNumberTable lineNumberTable = method.getLineNumberTable();
         int line = lineNumberTable == null ? -1 : lineNumberTable.getLineNumber(pos.getBCI());
 
-        Map<LocalEntry, LocalValueEntry> localValueInfos = initLocalInfoList(pos, methodEntry, frameSize);
+        Map<LocalEntry, LocalValueEntry> localValueInfos = initLocalInfoList(pos, methodEntry, frameSize, true);
         Range locationInfo = Range.createSubrange(primary, methodEntry, localValueInfos, startPos, endPos, line, callerLocation, true);
 
         if (debug.isLogEnabled()) {
@@ -1881,7 +1901,7 @@ private static boolean skipPos(BytecodePosition pos) {
      * at native image build-time while still having comparable performance (for most of the index
      * maps). The most performance critical maps that see more traffic will use the less memory
      * efficient ConcurrentHashMaps instead.
-     * 
+     *
      * @param map the {@code EconomicMap} to perform {@code computeIfAbsent} on
      * @param key the key to look for
      * @param mappingFunction the function producing the value for a given key

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/debug/SharedDebugInfoProvider.java

@@ -71,6 +71,8 @@
 import com.oracle.svm.core.debug.SubstrateDebugTypeEntrySupport;
 import com.oracle.svm.core.graal.meta.RuntimeConfiguration;
 import com.oracle.svm.core.image.ImageHeapPartition;
+import com.oracle.svm.core.imagelayer.DynamicImageLayerInfo;
+import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
 import com.oracle.svm.core.meta.SharedMethod;
 import com.oracle.svm.core.meta.SharedType;
 import com.oracle.svm.core.util.VMError;
@@ -120,6 +122,8 @@ class NativeImageDebugInfoProvider extends SharedDebugInfoProvider {
     private final int primitiveStartOffset;
     private final int referenceStartOffset;
     private final Set<HostedMethod> allOverrides;
+    private final boolean buildingImageLayer;
+    private final int layerNumber;
 
     NativeImageDebugInfoProvider(DebugContext debug, NativeImageCodeCache codeCache, NativeImageHeap heap, NativeLibraries nativeLibs, HostedMetaAccess metaAccess,
                     RuntimeConfiguration runtimeConfiguration) {
@@ -141,6 +145,9 @@ class NativeImageDebugInfoProvider extends SharedDebugInfoProvider {
                         .flatMap(m -> Arrays.stream(m.getImplementations())
                                         .filter(Predicate.not(m::equals)))
                         .collect(Collectors.toSet());
+
+        buildingImageLayer = ImageLayerBuildingSupport.buildingImageLayer();
+        layerNumber = DynamicImageLayerInfo.getCurrentLayerNumber();
     }
 
     @SuppressWarnings("unused")
@@ -496,6 +503,11 @@ public boolean isOverride(SharedMethod method) {
         return method instanceof HostedMethod && allOverrides.contains(method);
     }
 
+    @Override
+    public boolean isCompiledInPriorLayer(SharedMethod method) {
+        return method instanceof HostedMethod hostedMethod && hostedMethod.isCompiledInPriorLayer();
+    }
+
     @Override
     public boolean isVirtual(SharedMethod method) {
         return method instanceof HostedMethod hostedMethod && hostedMethod.hasVTableIndex();
@@ -582,8 +594,14 @@ private void processFieldEntries(HostedType type, StructureTypeEntry structureTy
         }
 
         for (ResolvedJavaField field : type.getStaticFields()) {
-            assert field instanceof HostedField;
-            structureTypeEntry.addField(createFieldEntry((HostedField) field, structureTypeEntry));
+            HostedField hField = (HostedField) field;
+            /*
+             * If we are in a layered build only add debug info for a static field if it is
+             * installed in the current layer.
+             */
+            if (!buildingImageLayer || (hField.hasInstalledLayerNum() && layerNumber == hField.getInstalledLayerNum())) {
+                structureTypeEntry.addField(createFieldEntry(hField, structureTypeEntry));
+            }
         }
     }
 

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

@@ -22,7 +22,7 @@
  * or visit www.oracle.com if you need additional information or have any
  * questions.
  */
-package com.oracle.svm.test.missing.classes;
+package com.oracle.svm.test.debug.missing.classes;
 
 public class TestClass {
     public static final Object CONSTANT_OBJECT_FIELD = 42;

substratevm/src/com.oracle.svm.test.missing.classes/src/com/oracle/svm/test/missing/classes/TestClass.java renamed to substratevm/src/com.oracle.svm.test.debug.missing.classes/src/com/oracle/svm/test/debug/missing/classes/TestClass.java

@@ -36,11 +36,10 @@
 import org.graalvm.nativeimage.c.type.CTypeConversion.CCharPointerHolder;
 import org.graalvm.word.PointerBase;
 import org.graalvm.word.SignedWord;
+import org.graalvm.word.WordBase;
 
 import com.oracle.svm.core.NeverInline;
 
-import jdk.graal.compiler.api.directives.GraalDirectives;
-
 @CContext(CInterfaceDebugTestDirectives.class)
 public class CStructTests {
     @CPointerTo(nameOfCType = "int")
@@ -238,10 +237,20 @@ private static void testMixedArguments(String m1, short s, SimpleStruct ss1, lon
         System.out.println("You find " + m1);
         System.out.println("You find " + m2);
         System.out.println("You find " + m3);
-        GraalDirectives.blackhole(s);
-        GraalDirectives.blackhole(ss1);
-        GraalDirectives.blackhole(l);
-        GraalDirectives.blackhole(ss2);
+        blackhole(s);
+        blackhole(ss1);
+        blackhole(l);
+        blackhole(ss2);
+    }
+
+    @NeverInline("For testing.")
+    @SuppressWarnings("unused")
+    static void blackhole(Object value) {
+    }
+
+    @NeverInline("For testing.")
+    @SuppressWarnings("unused")
+    static void blackhole(WordBase value) {
     }
 
     public static void main(String[] args) {