8239014: -XX:-UseEmptySlotsInSupers sometime fails to reproduce the l…

…ayout of the old code

Reviewed-by: dholmes, coleenp

src/hotspot/share/classfile/fieldLayoutBuilder.cpp

@@ -134,13 +134,12 @@ void FieldLayout::initialize_instance_layout(const InstanceKlass* super_klass) {
     _start = _blocks;
     insert(first_empty_block(), new LayoutRawBlock(LayoutRawBlock::RESERVED, instanceOopDesc::base_offset_in_bytes()));
   } else {
-    reconstruct_layout(super_klass);
+    bool has_fields = reconstruct_layout(super_klass);
     fill_holes(super_klass);
-    if (UseEmptySlotsInSupers && !super_klass->has_contended_annotations()) {
-      _start = _blocks; // Setting _start to _blocks instead of _last would allow subclasses
-                        // to allocate fields in empty slots of their super classes
+    if ((UseEmptySlotsInSupers && !super_klass->has_contended_annotations()) || !has_fields) {
+      _start = _blocks;  // start allocating fields from the first empty block
     } else {
-      _start = _last;
+      _start = _last;    // append fields at the end of the reconstructed layout
     }
   }
 }
@@ -294,13 +293,15 @@ LayoutRawBlock* FieldLayout::insert_field_block(LayoutRawBlock* slot, LayoutRawB
   return block;
 }
 
-void FieldLayout::reconstruct_layout(const InstanceKlass* ik) {
+bool FieldLayout::reconstruct_layout(const InstanceKlass* ik) {
+  bool has_instance_fields = false;
   GrowableArray<LayoutRawBlock*>* all_fields = new GrowableArray<LayoutRawBlock*>(32);
   while (ik != NULL) {
     for (AllFieldStream fs(ik->fields(), ik->constants()); !fs.done(); fs.next()) {
       BasicType type = Signature::basic_type(fs.signature());
       // distinction between static and non-static fields is missing
       if (fs.access_flags().is_static()) continue;
+      has_instance_fields = true;
       int size = type2aelembytes(type);
       // INHERITED blocks are marked as non-reference because oop_maps are handled by their holder class
       LayoutRawBlock* block = new LayoutRawBlock(fs.index(), LayoutRawBlock::INHERITED, size, size, false);
@@ -322,6 +323,7 @@ void FieldLayout::reconstruct_layout(const InstanceKlass* ik) {
     _last = b;
   }
   _start = _blocks;
+  return has_instance_fields;
 }
 
 // Called during the reconstruction of a layout, after fields from super

src/hotspot/share/classfile/fieldLayoutBuilder.hpp

@@ -192,7 +192,7 @@ class FieldLayout : public ResourceObj {
   void add_field_at_offset(LayoutRawBlock* blocks, int offset, LayoutRawBlock* start = NULL);
   void add_contiguously(GrowableArray<LayoutRawBlock*>* list, LayoutRawBlock* start = NULL);
   LayoutRawBlock* insert_field_block(LayoutRawBlock* slot, LayoutRawBlock* block);
-  void reconstruct_layout(const InstanceKlass* ik);
+  bool reconstruct_layout(const InstanceKlass* ik);
   void fill_holes(const InstanceKlass* ik);
   LayoutRawBlock* insert(LayoutRawBlock* slot, LayoutRawBlock* block);
   void remove(LayoutRawBlock* block);

test/hotspot/jtreg/runtime/FieldLayout/OldLayoutCheck.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2020, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8239014
+ * @summary -XX:-UseEmptySlotsInSupers sometime fails to reproduce the layout of the old code
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.management
+ * @requires vm.bits == "64"
+ * @run main/othervm -XX:+UseCompressedClassPointers -XX:-UseEmptySlotsInSupers OldLayoutCheck
+ */
+
+/*
+ * @test
+ * @requires vm.bits == "32"
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.management
+ * @run main/othervm -XX:-UseEmptySlotsInSupers OldLayoutCheck
+ */
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Comparator;
+import jdk.internal.misc.Unsafe;
+
+import jdk.test.lib.Asserts;
+
+public class OldLayoutCheck {
+
+    static class LIClass {
+        public long l;
+        public int i;
+    }
+
+    static public void main(String[] args) {
+        Unsafe unsafe = Unsafe.getUnsafe();
+        Class c = LIClass.class;
+        Field[] fields = c.getFields();
+        for (int i = 0; i < fields.length; i++) {
+            long offset = unsafe.objectFieldOffset(fields[i]);
+            if (fields[i].getType() == int.class) {
+                Asserts.assertEquals(offset, 12L, "Misplaced int field");
+            } else if (fields[i].getType() == long.class) {
+                Asserts.assertEquals(offset, 16L, "Misplaced long field");
+            } else {
+                Asserts.fail("Unexpected field type");
+            }
+        }
+    }
+}