Save unset field NAS in ROM class and DDR support #22998

theresa-m · 2025-11-21T21:00:59Z

record name and signature SRP instead of just the name in the rom class so ClassFileWriter can recreate the class file.
print early larval frame with cfdump -d support
print early larval frame with ddr dumpromclass
print early larval frame with ddr dumpromclasslinear

related: #21884

theresa-m · 2025-11-21T21:05:37Z

DDR and cfdump output with these changes:

Sample class

import jdk.internal.vm.annotation.Strict;
 
class Test {
        public static void main(String[] args) {
EarlyLarvalStackMap e = new EarlyLarvalStackMap(true);
        }
 
        static class EarlyLarvalStackMap {
    //   StackMapTable: number_of_entries = 2
    //     frame_type = 12 /* same */
    //     frame_type = 246 /* early_larval */
    //       number of unset_fields = 2
    //         unset_field = #NameAndType j:I
    //         unset_field = #NameAndType o:Ljava/lang/Object;
    //           offset_delta = 4
    //           locals = [ this, int ]
    //           stack = []
                @Strict int i;
                @Strict int j;
                @Strict Object o;
                EarlyLarvalStackMap(boolean b) {
                        if (b) {
                                i = 1;
                        } else {
                                i = 2;
                        }
                        j = 0;
                        o = new Object();
                        super();
                }
        }
}

cfdump, -d Test$EarlyLarvalStackMap.class

The output is the same when I generate a .j9class with cfdump -t and run cfdump -d on that.

        StackMapTable:
          Stackmaps (2):
            pc: 12 same
            early_larval:
              unset fields: 2
                NAS: 8, Name: 9 -> j, Signature: 6 -> I
                NAS: 17, Name: 18 -> o, Signature: 19 -> Ljava/lang/Object;
              base frame: pc: 17 same

DDR, !dumpromclass

  StackMapTable
    Stackmaps(2):
      pc: 12 same
      early_larval:
        unset_fields: 2
          j:I
          o:Ljava/lang/Object;
        base: pc: 17 same

DDR, !dumpromclasslinear ,4

=== Section Start: stackMap (20 bytes)                                                    ===
0x0000000130B4832C-0x0000000130B48330 [           0x00000014 stackMapSize                 ] 
0x0000000130B48330-0x0000000130B48332 [               0x0200 stackMapFrameCount           ] 
0x0000000130B48332-0x0000000130B48333 [                 0x0C stackMapFrameType            ] 
0x0000000130B48333-0x0000000130B48334 [                 0xF6 stackMapFrameType            ] 
0x0000000130B48334-0x0000000130B48336 [               0x0200 numberOfUnsetFields          ] 
0x0000000130B48336-0x0000000130B4833A [           0xFFFFFF8E cpFieldNAS                   ]  -> 0x0000000130B482C4
0x0000000130B4833A-0x0000000130B4833E [           0xFFFFFF9A cpFieldNAS                   ]  -> 0x0000000130B482D4
0x0000000130B4833E-0x0000000130B4833F [                 0x04 baseFrameType                ] 
0x0000000130B4833F-0x0000000130B48340 [                      Padding

keithc-ca · 2025-11-21T22:39:38Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/RomClassWalker.java

-
-		for (; frameCount > 0; frameCount--) {
+		boolean walkBaseFrame = false;
+		while (frameCount > 0) {


~~This was better expressed as a for loop; please undo that change.~~
Now I see why, but I don't like it. Why don't CFR_STACKMAP_EARLY_LARVAL > frameType count as frames?

In the jvm spec: Tags in the range [128-245] are reserved for future use.

Ok, this is part of that future usage, but that doesn't answer my question.

The vm throws an assertion if it finds one of these values in a class file, they will not be used in a rom class.

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/commands/J9BCUtil.java

keithc-ca · 2025-11-21T22:44:18Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/commands/J9BCUtil.java

+	{
+		int unsetFieldCount = new U16(slotData.at(0)).leftShift(8).bitOr(slotData.at(1)).intValue();
+		slotData = slotData.add(2);
+		out.println("        unset_fields: " + unsetFieldCount);


Please use format() instead (throughout the new code), e.g. here

out.format(" unset_fields: %d%n", unsetFieldCount);

keithc-ca

The literal MethodIsClinit referenced in ClassFileOracle.cpp is missing from romphase.h and the corresponding strings in ROMClassCreationContext.cpp are out-of-sync.

keithc-ca · 2025-11-24T19:24:41Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/commands/J9BCUtil.java

-				} else if (mapType < 247) {
-					out.println("      UNKNOWN FRAME TAG: (" + mapType + ")\n");
+				} else if (mapType < CFR_STACKMAP_EARLY_LARVAL) {
+					out.println("UNKNOWN FRAME TAG: (" + mapType + ")\n");


If two newlines are actually desired here, this should use format() with two occcurrences of %n.

keithc-ca · 2025-11-24T19:25:36Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/commands/J9BCUtil.java

+					dumpBaseFrame = false;
+					out.print("        base: ");
+				} else {
+					mapPC++;


Please use += 1 here and on line 1226.

keithc-ca · 2025-11-24T19:28:25Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/commands/J9BCUtil.java


+	static U8Pointer dumpUnsetFields(PrintStream out, J9ROMClassPointer classfile, U8Pointer slotData) throws CorruptDataException
+	{
+		int unsetFieldCount = new U16(slotData.at(0)).leftShift(8).bitOr(slotData.at(1)).intValue();


Why does this use bitOr() instead of add() like most other places in this file?

I reused the logic to read 16 bits from the only other place that uses bitOr. I can change them both so its more consistent.

I see now, and converts the value to I32 so bitOr is needed in line 1151 but not here.

keithc-ca · 2025-11-24T19:30:38Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/RomClassWalker.java

 				cursor = cursor.add(length);
-			} else if (CFR_STACKMAP_SAME_LOCALS_1_STACK_EXTENDED > frameType) { /* 128..246 */
+			} else if (CFR_STACKMAP_EARLY_LARVAL > frameType) { /* 128..245 */
 				/* Reserved frame types - no extra data */


While you're here, please add the missing period.

keithc-ca · 2025-11-24T19:35:06Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/RomClassWalker.java

 					cursor = cursor.add(length);
 				}
 			}
+			frameCount--;


Please use -= 1.

keithc-ca · 2025-11-24T21:00:55Z

runtime/cfdumper/main.c

 }

+#if defined(J9VM_OPT_VALHALLA_STRICT_FIELDS)
+static U_8 *dumpUnsetFields(J9CfrClassFile *classfile, U_8 *slotData,  U_32 tabLevel)


Formatting:

static U_8 * dumpUnsetFields(J9CfrClassFile *classfile, U_8 *slotData, U_32 tabLevel)

keithc-ca · 2025-11-24T21:01:38Z

runtime/cfdumper/main.c

+		U_16 nameAndSignatureIndex = (slotData[0] << 8) + slotData[1];
+		slotData += 2;
+		U_16 nameIndex = classfile->constantPool[nameAndSignatureIndex].slot1;
+		U_16 signatureIndex = classfile->constantPool[nameAndSignatureIndex].slot2;


This is not legal C code: all declarations must be at the beginning of a block:

U_16 nameAndSignatureIndex = (slotData[0] << 8) + slotData[1]; U_16 nameIndex = classfile->constantPool[nameAndSignatureIndex].slot1; U_16 signatureIndex = classfile->constantPool[nameAndSignatureIndex].slot2; slotData += 2; for (i = 0; i < tabLevel; i++) { j9tty_printf(PORTLIB, " "); }

keithc-ca · 2025-11-24T21:04:45Z

runtime/cfdumper/main.c

+		slotData += 2;
+		U_16 nameIndex = classfile->constantPool[nameAndSignatureIndex].slot1;
+		U_16 signatureIndex = classfile->constantPool[nameAndSignatureIndex].slot2;
+		j9tty_printf(PORTLIB, "NAS: %i, Name: %i -> %s, Signature: %i -> %s\n",


Please don't capitalize "Name" and "Signature" here.

keithc-ca · 2025-11-24T21:05:09Z

runtime/oti/bcverify.h

@@ -55,8 +55,8 @@ typedef struct J9BranchTargetStack {

 #if defined(J9VM_OPT_VALHALLA_STRICT_FIELDS)
 typedef struct J9StrictFieldEntry {
-	 /* nameutf8 is the only field used to determine equality. */
-	J9UTF8* nameutf8;
+	 /* nas is the only field used to determine equality. */


Please correct the indentation here.

keithc-ca · 2025-11-24T21:09:11Z

runtime/oti/j9nonbuilder.h

@@ -2141,7 +2141,7 @@ typedef struct J9TranslationBufferSet {
 typedef struct J9EarlyLarvalFrame {
 	IDATA baseFramePC; /* The only field used to determine equality. */
 	U_16 numberOfUnsetFields;
-	U_16 *unsetFieldCpList;
+	J9ROMNameAndSignature **unsetFieldNASList;


Do the entries need to be pointers here? Why not just pairs of U_16?

I wanted to keep the NAS instead of just the name and signature constant pool indices so that the ClassFileWriter can recreate the class file from the rom class.
NAS doesn't get stored in the rom class directly like NameAndType in a class file so I used the pointer instead.

keithc-ca · 2025-11-24T21:14:13Z

Please update the commit, the title and description of this pull request to capitalize acronyms "NAS", "ROM" and "DDR.

- record name and signature SRP instead of just the name in the rom class so ClassFileWriter can recreate the class file. - print early larval frame with cfdump -d support - print early larval frame with ddr dumpromclass - print early larval frame with ddr dumpromclasslinear Signed-off-by: Theresa Mammarella <[email protected]>

hangshao0 · 2025-11-28T15:33:14Z

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/commands/J9BCUtil.java

-					out.println("      UNKNOWN FRAME TAG: (" + mapType + ")\n");
+				} else if (mapType < CFR_STACKMAP_EARLY_LARVAL) {
+					out.format("UNKNOWN FRAME TAG: (" + mapType + ")%n%n");
+				} else if (mapType == CFR_STACKMAP_EARLY_LARVAL) {


Do we need a class version check here (and for the new code dealing with unsetfields) ? As for Pre-Valhalla class, it should still be treated as UNKNOWN FRAME TAG

theresa-m added comp:vm project:valhalla Used to track Project Valhalla related work labels Nov 21, 2025

theresa-m requested review from hangshao0 and keithc-ca November 21, 2025 21:05

keithc-ca reviewed Nov 21, 2025

View reviewed changes

debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/commands/J9BCUtil.java Outdated Show resolved Hide resolved

keithc-ca reviewed Nov 21, 2025

View reviewed changes

theresa-m force-pushed the strictfields_8 branch 2 times, most recently from 32d71d2 to 2ec7371 Compare November 24, 2025 18:32

theresa-m requested a review from keithc-ca November 24, 2025 18:32

keithc-ca requested changes Nov 24, 2025

View reviewed changes

theresa-m changed the title ~~Save unset field nas in rom class and ddr support~~ Save unset field NAS in ROM class and DDR support Nov 27, 2025

theresa-m force-pushed the strictfields_8 branch from 2ec7371 to 817bd87 Compare November 28, 2025 15:18

hangshao0 reviewed Nov 28, 2025

View reviewed changes

Save unset field NAS in ROM class and DDR support #22998

Are you sure you want to change the base?

Save unset field NAS in ROM class and DDR support #22998

Conversation

theresa-m commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theresa-m commented Nov 21, 2025

DDR and cfdump output with these changes:

Sample class

cfdump, -d Test$EarlyLarvalStackMap.class

DDR, !dumpromclass

DDR, !dumpromclasslinear ,4

Uh oh!

keithc-ca Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keithc-ca left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keithc-ca commented Nov 24, 2025

Uh oh!

hangshao0 Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

theresa-m commented Nov 21, 2025 •

edited

Loading

keithc-ca Nov 21, 2025 •

edited

Loading

hangshao0 Nov 28, 2025 •

edited

Loading