Properly handle static fields and compiled methods from the base layer.

Fix crash in CStructTests.
Make sure params are not optimized out in PrettyPrinterTest.
Only emit register locations for leaf ranges.

Author: dominikmascherbauer

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

@@ -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/elf/dwarf/DwarfInfoSectionImpl.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/DwarfLineSectionImpl.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.svm.core/src/com/oracle/svm/core/debug/SharedDebugInfoProvider.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.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoProvider.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;
@@ -496,6 +498,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();
@@ -583,7 +590,14 @@ private void processFieldEntries(HostedType type, StructureTypeEntry structureTy
 
         for (ResolvedJavaField field : type.getStaticFields()) {
             assert field instanceof HostedField;
-            structureTypeEntry.addField(createFieldEntry((HostedField) field, structureTypeEntry));
+            /*
+             * If we are in a layered build only add debug info for a static field if it is
+             * installed in the current layer.
+             */
+            if (!ImageLayerBuildingSupport.buildingImageLayer() ||
+                            (((HostedField) field).hasInstalledLayerNum() && DynamicImageLayerInfo.getCurrentLayerNumber() == StaticFieldsSupport.getInstalledLayerNum(field))) {
+                structureTypeEntry.addField(createFieldEntry((HostedField) field, structureTypeEntry));
+            }
         }
     }
 

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/CStructTests.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) {

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/PrettyPrinterTest.java

@@ -115,51 +115,54 @@ public static class Holder {
     @NeverInline("For testing purposes")
     static void testPrimitive(byte b, Byte bObj, short s, Short sObj, char c, Character cObj, int i, Integer iObj, long l, Long lObj,
                     float f, Float fObj, double d, Double dObj, boolean x, Boolean xObj) {
-        System.out.print("");
+        blackhole(b, bObj, s, sObj, c, cObj, i, iObj, l, lObj, f, fObj, d, dObj, x, xObj);
     }
 
     @SuppressWarnings("unused")
     @NeverInline("For testing purposes")
     static void testString(String nullStr, String emptyStr, String str, String uStr1, String uStr2, String uStr3, String uStr4, String uStr5, String str0) {
-        System.out.print("");
+        blackhole(nullStr, emptyStr, str, uStr1, uStr2, uStr3, uStr4, uStr5, str0);
     }
 
     @SuppressWarnings("unused")
     @NeverInline("For testing purposes")
     static void testArray(int[] ia, Object[] oa, String[] sa) {
-        System.out.print("");
+        blackhole(ia, oa, sa);
     }
 
     @SuppressWarnings("unused")
     @NeverInline("For testing purposes")
     static void testObject(ExampleClass object, ExampleClass recObject) {
-        System.out.print("");
+        blackhole(object, recObject);
     }
 
     @SuppressWarnings("unused")
     @NeverInline("For testing purposes")
     static void testArrayList(ArrayList<String> strList, List<Object> mixedList, ArrayList<Object> nullList) {
-        System.out.print("");
+        blackhole(strList, mixedList, nullList);
     }
 
     @SuppressWarnings("unused")
     @NeverInline("For testing purposes")
     static void testHashMap(HashMap<String, String> strMap, Map<Object, Object> mixedMap) {
-        System.out.print("");
+        blackhole(strMap, mixedMap);
     }
 
     @SuppressWarnings("unused")
     @NeverInline("For testing purposes")
     static void testLambda(Function<String, String> lambda) {
-        System.out.print("");
+        blackhole(lambda);
     }
 
     @SuppressWarnings("unused")
     @NeverInline("For testing purposes")
     static void testClassType(Class<?> clazz, Holder dyn) {
-        System.out.print("");
-        System.out.print(staticHolder.c);
-        System.out.print(staticHolder.o);
+        blackhole(clazz, dyn, staticHolder.c, staticHolder.o);
+    }
+
+    @NeverInline("For testing.")
+    @SuppressWarnings("unused")
+    static void blackhole(Object... value) {
     }
 
     static ExampleClass setupExampleObject(boolean recursive) {

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/RuntimeCompileDebugInfoTest.java

@@ -219,9 +219,6 @@ public static class RegisterMethodsFeature implements Feature {
             ModuleSupport.accessPackagesToClass(ModuleSupport.Access.EXPORT, RegisterMethodsFeature.class, false,
                             "jdk.internal.vm.ci",
                             "jdk.vm.ci.code");
-            ModuleSupport.accessPackagesToClass(ModuleSupport.Access.EXPORT, RegisterMethodsFeature.class, false,
-                            "jdk.graal.compiler",
-                            "jdk.graal.compiler.api.directives", "jdk.graal.compiler.word");
         }
 
         @Override