8374554: [lworld] Field layout assert with UseCompactObjectHeaders

Reviewed-by: coleenp

Author: fparain

src/hotspot/share/classfile/fieldLayoutBuilder.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2026, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1064,29 +1064,17 @@ void FieldLayoutBuilder::compute_inline_class_layout() {
 
   assert(_layout->start()->block_kind() == LayoutRawBlock::RESERVED, "Unexpected");
 
-  if (_layout->super_has_fields() && !_is_abstract_value) {  // non-static field layout
-    if (!_has_nonstatic_fields) {
-      assert(_is_abstract_value, "Concrete value types have at least one field");
-      // Nothing to do
-    } else {
-      // decide which alignment to use, then set first allowed field offset
-
-      assert(_layout->super_alignment() >= _payload_alignment, "Incompatible alignment");
-      assert(_layout->super_alignment() % _payload_alignment == 0, "Incompatible alignment");
-
-      if (_payload_alignment < _layout->super_alignment()) {
-        int new_alignment = _payload_alignment > _layout->super_min_align_required() ? _payload_alignment : _layout->super_min_align_required();
-        assert(new_alignment % _payload_alignment == 0, "Must be");
-        assert(new_alignment % _layout->super_min_align_required() == 0, "Must be");
-        _payload_alignment = new_alignment;
-      }
-      _layout->set_start(_layout->first_field_block());
-    }
-  } else {
+  if (!_layout->super_has_fields()) {
+    // No inherited fields, the layout must be empty except for the RESERVED block
+    // PADDING is inserted if needed to ensure the correct alignment of the payload.
     if (_is_abstract_value && _has_nonstatic_fields) {
+      // non-static fields of the abstract class must be laid out without knowning
+      // the alignment constraints of the fields of the sub-classes, so the worst
+      // case scenario is assumed, which is currently the alignment of T_LONG.
+      // PADDING is added if needed to ensure the payload will respect this alignment.
       _payload_alignment = type2aelembytes(BasicType::T_LONG);
     }
-    assert(_layout->start()->next_block()->block_kind() == LayoutRawBlock::EMPTY || !UseCompressedClassPointers, "Unexpected");
+    assert(_layout->start()->next_block()->block_kind() == LayoutRawBlock::EMPTY, "Unexpected");
     LayoutRawBlock* first_empty = _layout->start()->next_block();
     if (first_empty->offset() % _payload_alignment != 0) {
       LayoutRawBlock* padding = new LayoutRawBlock(LayoutRawBlock::PADDING, _payload_alignment - (first_empty->offset() % _payload_alignment));
@@ -1096,6 +1084,29 @@ void FieldLayoutBuilder::compute_inline_class_layout() {
       }
       _layout->set_start(padding);
     }
+  } else { // the class has inherited some fields from its super(s)
+    if (!_is_abstract_value) {
+      // This is the step where the layout of the final concrete value class' layout
+      // is computed. Super abstract value classes might have been too conservative
+      // regarding alignment constraints, but now that the full set of non-static fields is
+      // known, compute which alignment to use, then set first allowed field offset
+
+      assert(_has_nonstatic_fields, "Concrete value classes must have at least one field");
+      if (_payload_alignment == -1) { // current class declares no local nonstatic fields
+        _payload_alignment = _layout->super_min_align_required();
+      }
+
+      assert(_layout->super_alignment() >= _payload_alignment, "Incompatible alignment");
+      assert(_layout->super_alignment() % _payload_alignment == 0, "Incompatible alignment");
+
+      if (_payload_alignment < _layout->super_alignment()) {
+        int new_alignment = _payload_alignment > _layout->super_min_align_required() ? _payload_alignment : _layout->super_min_align_required();
+        assert(new_alignment % _payload_alignment == 0, "Must be");
+        assert(new_alignment % _layout->super_min_align_required() == 0, "Must be");
+        _payload_alignment = new_alignment;
+      }
+      _layout->set_start(_layout->first_field_block());
+    }
   }
 
   _layout->add(_root_group->big_primitive_fields());

test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/FieldAlignmentTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024, 2026, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -22,36 +22,69 @@
  */
 
 /*
- * @test id=Oops32
+ * @test id=32
  * @requires vm.bits == 32
  * @requires vm.flagless
  * @library /test/lib
  * @modules java.base/jdk.internal.vm.annotation
  * @enablePreview
  * @compile FieldLayoutAnalyzer.java FieldAlignmentTest.java
- * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 0
+ * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 32
  */
 
-  /*
- * @test id=CompressedOops
+/*
+ * @test id=64_COOP_CCP_NCOH
+ * @requires vm.bits == 64
+ * @requires vm.flagless
+ * @library /test/lib
+ * @modules java.base/jdk.internal.vm.annotation
+ * @enablePreview
+ * @compile FieldLayoutAnalyzer.java FieldAlignmentTest.java
+ * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 64_COOP_CCP_NCOH
+ */
+
+/*
+ * @test id=64_NCOOP_CCP_NCOH
  * @requires vm.bits == 64
  * @requires vm.flagless
  * @library /test/lib
  * @modules java.base/jdk.internal.vm.annotation
  * @enablePreview
  * @compile FieldLayoutAnalyzer.java FieldAlignmentTest.java
- * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 1
+ * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 64_NCOOP_CCP_NCOH
  */
 
-  /*
- * @test id=NoCompressedOops
+/*
+ * @test id=64_NCOOP_NCCP_NCOH
  * @requires vm.bits == 64
  * @requires vm.flagless
  * @library /test/lib
  * @modules java.base/jdk.internal.vm.annotation
  * @enablePreview
  * @compile FieldLayoutAnalyzer.java FieldAlignmentTest.java
- * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 2
+ * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 64_NCOOP_NCCP_NCOH
+ */
+
+/*
+ * @test id=64_COOP_CCP_COH
+ * @requires vm.bits == 64
+ * @requires vm.flagless
+ * @library /test/lib
+ * @modules java.base/jdk.internal.vm.annotation
+ * @enablePreview
+ * @compile FieldLayoutAnalyzer.java FieldAlignmentTest.java
+ * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 64_COOP_CCP_COH
+ */
+
+/*
+ * @test id=64_NCOOP_CCP_COH
+ * @requires vm.bits == 64
+ * @requires vm.flagless
+ * @library /test/lib
+ * @modules java.base/jdk.internal.vm.annotation
+ * @enablePreview
+ * @compile FieldLayoutAnalyzer.java FieldAlignmentTest.java
+ * @run main runtime.valhalla.inlinetypes.field_layout.FieldAlignmentTest 64_NCOOP_CCP_COH
  */
 
  package runtime.valhalla.inlinetypes.field_layout;
@@ -152,7 +185,10 @@ void generateTestRunner() throws Exception {
     jdk.test.lib.helpers.ClassFileInstaller.writeClassToDisk(className, byteCode);
   }
 
-  static ProcessBuilder exec(String compressedOopsArg, String... args) throws Exception {
+  static ProcessBuilder exec(String compressedOopsArg,
+                             String compressedKlassPointersArg,
+                             String compactObjectHeader,
+                             String... args) throws Exception {
     List<String> argsList = new ArrayList<>();
     Collections.addAll(argsList, "--enable-preview");
     Collections.addAll(argsList, "-XX:+UnlockDiagnosticVMOptions");
@@ -161,6 +197,12 @@ static ProcessBuilder exec(String compressedOopsArg, String... args) throws Exce
     if (compressedOopsArg != null) {
       Collections.addAll(argsList, compressedOopsArg);
     }
+    if (compressedKlassPointersArg != null) {
+      Collections.addAll(argsList, compressedKlassPointersArg);
+    }
+    if (compactObjectHeader != null) {
+      Collections.addAll(argsList, compactObjectHeader);
+    }
     Collections.addAll(argsList, "-Xmx256m");
     Collections.addAll(argsList, "-cp", System.getProperty("java.class.path") + System.getProperty("path.separator") +".");
     Collections.addAll(argsList, args);
@@ -169,14 +211,40 @@ static ProcessBuilder exec(String compressedOopsArg, String... args) throws Exce
 
   public static void main(String[] args) throws Exception {
     String compressedOopsArg;
+    String compressedKlassPointersArg;
+    String compactObjectHeaderArg;
 
     switch(args[0]) {
-      case "0": compressedOopsArg = null;
-                break;
-      case "1": compressedOopsArg = "-XX:+UseCompressedOops";
-                break;
-      case "2": compressedOopsArg = "-XX:-UseCompressedOops";
-                break;
+      case "32":
+        compressedOopsArg = null;
+        compressedKlassPointersArg = null;
+        compactObjectHeaderArg = null;
+        break;
+      case "64_COOP_CCP_NCOH":
+        compressedOopsArg = "-XX:+UseCompressedOops";
+        compressedKlassPointersArg =  "-XX:+UseCompressedClassPointers";
+        compactObjectHeaderArg = "-XX:-UseCompactObjectHeaders";
+        break;
+      case "64_NCOOP_CCP_NCOH":
+        compressedOopsArg = "-XX:-UseCompressedOops";
+        compressedKlassPointersArg = "-XX:+UseCompressedClassPointers";
+        compactObjectHeaderArg = "-XX:-UseCompactObjectHeaders";
+        break;
+      case "64_NCOOP_NCCP_NCOH":
+        compressedOopsArg = "-XX:-UseCompressedOops";
+        compressedKlassPointersArg = "-XX:-UseCompressedClassPointers";
+        compactObjectHeaderArg = "-XX:-UseCompactObjectHeaders";
+        break;
+      case "64_COOP_CCP_COH":
+        compressedOopsArg = "-XX:+UseCompressedOops";
+        compressedKlassPointersArg = "-XX:+UseCompressedClassPointers";
+        compactObjectHeaderArg = "-XX:+UseCompactObjectHeaders";
+        break;
+      case "64_NCOOP_CCP_COH":
+        compressedOopsArg = "-XX:-UseCompressedOops";
+        compressedKlassPointersArg = "-XX:+UseCompressedClassPointers";
+        compactObjectHeaderArg = "-XX:+UseCompactObjectHeaders";
+        break;
       default: throw new RuntimeException("Unrecognized configuration");
     }
 
@@ -186,7 +254,7 @@ public static void main(String[] args) throws Exception {
     fat.generateTestRunner();
 
     // Execute the test runner in charge of loading all test classes
-    ProcessBuilder pb = exec(compressedOopsArg, "TestRunner");
+    ProcessBuilder pb = exec(compressedOopsArg, compressedKlassPointersArg, compactObjectHeaderArg, "TestRunner");
     OutputAnalyzer out = new OutputAnalyzer(pb.start());
 
     if (out.getExitValue() != 0) {

test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/FieldLayoutAnalyzer.java

@@ -78,11 +78,17 @@ static BlockType parseType(String s) {
     }
   }
 
-  static enum LayoutKind {
-    NON_FLAT,
-    NULL_FREE_NON_ATOMIC_FLAT,
-    NULL_FREE_ATOMIC_FLAT,
-    NULLABLE_ATOMIC_FLAT;
+  static public enum LayoutKind {
+    NON_FLAT(false),
+    NULL_FREE_NON_ATOMIC_FLAT(false),
+    NULL_FREE_ATOMIC_FLAT(false),
+    NULLABLE_ATOMIC_FLAT(true);
+
+    private final boolean hasNullMarker;
+
+    private LayoutKind(boolean hasNullMarker) {
+      this.hasNullMarker = hasNullMarker;
+    }
 
     static LayoutKind parseLayoutKind(String s) {
       switch(s) {
@@ -94,6 +100,10 @@ static LayoutKind parseLayoutKind(String s) {
           throw new RuntimeException("Unknown layout kind: " + s);
       }
     }
+
+    boolean hasNullMarker() {
+      return this.hasNullMarker;
+    }
   }
 
   static public record FieldBlock (int offset,
@@ -124,6 +134,7 @@ void print(PrintStream out) {
     }
 
     boolean isFlat() { return type == BlockType.FLAT; } // Warning: always return false for inherited fields, even flat ones
+    boolean hasNullMarker() { return layoutKind.hasNullMarker(); }
 
     static FieldBlock parseField(String line) {
       String[] fieldLine = line.split("\\s+");
@@ -243,7 +254,7 @@ int getAlignment(LayoutKind layoutKind) {
     void processField(String line, boolean isStatic) {
       FieldBlock block = FieldBlock.parseField(line);
       if (isStatic) {
-        Asserts.assertTrue(block.type != BlockType.INHERITED); // static fields cannotbe inherited
+        Asserts.assertTrue(block.type != BlockType.INHERITED); // static fields cannot be inherited
         staticFields.add(block);
       } else {
         nonStaticFields.add(block);