8296956: [JVMCI] HotSpotResolvedJavaFieldImpl.getIndex returns wrong value

Reviewed-by: thartmann, never

Author: Doug Simon

src/hotspot/share/runtime/fieldDescriptor.hpp

@@ -62,7 +62,7 @@ class fieldDescriptor {
 
   AccessFlags access_flags()      const    { return _access_flags; }
   oop loader()                    const;
-  // Offset (in words) of field from start of instanceOop / Klass*
+  // Offset (in bytes) of field from start of instanceOop / Klass*
   inline int offset()             const;
   Symbol* generic_signature()     const;
   int index()                     const    { return _index; }

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java

@@ -256,11 +256,23 @@ private static int rawIndexToConstantPoolCacheIndex(int rawIndex, int opcode) {
         if (opcode == Bytecodes.INVOKEDYNAMIC) {
             index = rawIndex;
             // See: ConstantPool::is_invokedynamic_index
-            assert index < 0 : "not an invokedynamic constant pool index " + index;
+            if (index >= 0) {
+                throw new IllegalArgumentException("not an invokedynamic constant pool index " + index);
+            }
         } else {
-            assert opcode == Bytecodes.GETFIELD || opcode == Bytecodes.PUTFIELD || opcode == Bytecodes.GETSTATIC || opcode == Bytecodes.PUTSTATIC || opcode == Bytecodes.INVOKEINTERFACE ||
-                            opcode == Bytecodes.INVOKEVIRTUAL || opcode == Bytecodes.INVOKESPECIAL || opcode == Bytecodes.INVOKESTATIC : "unexpected invoke opcode " + opcode;
-            index = rawIndex + config().constantPoolCpCacheIndexTag;
+            if (opcode == Bytecodes.GETFIELD ||
+                            opcode == Bytecodes.PUTFIELD ||
+                            opcode == Bytecodes.GETSTATIC ||
+                            opcode == Bytecodes.PUTSTATIC ||
+                            opcode == Bytecodes.INVOKEINTERFACE ||
+                            opcode == Bytecodes.INVOKEVIRTUAL ||
+                            opcode == Bytecodes.INVOKESPECIAL ||
+                            opcode == Bytecodes.INVOKESTATIC) {
+                index = rawIndex + config().constantPoolCpCacheIndexTag;
+            } else {
+                throw new IllegalArgumentException("unexpected opcode " + opcode);
+
+            }
         }
         return index;
     }
@@ -292,7 +304,9 @@ private static boolean isInvokedynamicIndex(int index) {
      * See {@code ConstantPool::decode_invokedynamic_index}.
      */
     private static int decodeInvokedynamicIndex(int i) {
-        assert isInvokedynamicIndex(i) : i;
+        if (!isInvokedynamicIndex(i)) {
+            throw new IllegalArgumentException("not an invokedynamic index: " + i);
+        }
         return ~i;
     }
 
@@ -315,7 +329,7 @@ public long getMetadataHandle() {
      * @return constant pool tag
      */
     private JvmConstant getTagAt(int index) {
-        assert checkBounds(index);
+        checkBounds(index);
         HotSpotVMConfig config = config();
         final long metaspaceConstantPoolTags = UNSAFE.getAddress(getConstantPoolPointer() + config.constantPoolTagsOffset);
         final int tag = UNSAFE.getByteVolatile(null, metaspaceConstantPoolTags + config.arrayU1DataOffset + index);
@@ -332,7 +346,7 @@ private JvmConstant getTagAt(int index) {
      * @return constant pool entry
      */
     long getEntryAt(int index) {
-        assert checkBounds(index);
+        checkBounds(index);
         int offset = index * runtime().getHostJVMCIBackend().getTarget().wordSize;
         return UNSAFE.getAddress(getConstantPoolPointer() + config().constantPoolSize + offset);
     }
@@ -344,7 +358,7 @@ long getEntryAt(int index) {
      * @return integer constant pool entry at index
      */
     private int getIntAt(int index) {
-        assert checkTag(index, constants.jvmInteger);
+        checkTag(index, constants.jvmInteger);
         int offset = index * runtime().getHostJVMCIBackend().getTarget().wordSize;
         return UNSAFE.getInt(getConstantPoolPointer() + config().constantPoolSize + offset);
     }
@@ -356,7 +370,7 @@ private int getIntAt(int index) {
      * @return long constant pool entry
      */
     private long getLongAt(int index) {
-        assert checkTag(index, constants.jvmLong);
+        checkTag(index, constants.jvmLong);
         int offset = index * runtime().getHostJVMCIBackend().getTarget().wordSize;
         return UNSAFE.getLong(getConstantPoolPointer() + config().constantPoolSize + offset);
     }
@@ -368,7 +382,7 @@ private long getLongAt(int index) {
      * @return float constant pool entry
      */
     private float getFloatAt(int index) {
-        assert checkTag(index, constants.jvmFloat);
+        checkTag(index, constants.jvmFloat);
         int offset = index * runtime().getHostJVMCIBackend().getTarget().wordSize;
         return UNSAFE.getFloat(getConstantPoolPointer() + config().constantPoolSize + offset);
     }
@@ -380,7 +394,7 @@ private float getFloatAt(int index) {
      * @return float constant pool entry
      */
     private double getDoubleAt(int index) {
-        assert checkTag(index, constants.jvmDouble);
+        checkTag(index, constants.jvmDouble);
         int offset = index * runtime().getHostJVMCIBackend().getTarget().wordSize;
         return UNSAFE.getDouble(getConstantPoolPointer() + config().constantPoolSize + offset);
     }
@@ -392,7 +406,7 @@ private double getDoubleAt(int index) {
      * @return {@code JVM_CONSTANT_NameAndType} constant pool entry
      */
     private int getNameAndTypeAt(int index) {
-        assert checkTag(index, constants.jvmNameAndType);
+        checkTag(index, constants.jvmNameAndType);
         int offset = index * runtime().getHostJVMCIBackend().getTarget().wordSize;
         return UNSAFE.getInt(getConstantPoolPointer() + config().constantPoolSize + offset);
     }
@@ -474,7 +488,7 @@ private int getKlassRefIndexAt(int index) {
      * @return klass reference index
      */
     private int getUncachedKlassRefIndexAt(int index) {
-        assert checkTagIsFieldOrMethod(index);
+        checkTagIsFieldOrMethod(index);
         int offset = index * runtime().getHostJVMCIBackend().getTarget().wordSize;
         final int refIndex = UNSAFE.getInt(getConstantPoolPointer() + config().constantPoolSize + offset);
         // klass ref index is in the low 16-bits.
@@ -485,24 +499,26 @@ private int getUncachedKlassRefIndexAt(int index) {
      * Checks that the constant pool index {@code index} is in the bounds of the constant pool.
      *
      * @param index constant pool index
-     * @throws AssertionError if the check fails
+     * @throws IndexOutOfBoundsException if the check fails
      */
-    private boolean checkBounds(int index) {
-        assert 0 <= index && index < length() : "index " + index + " not between 0 and " + length();
-        return true;
+    private void checkBounds(int index) {
+        if (index < 1 || index >= length()) {
+            throw new IndexOutOfBoundsException("index " + index + " not between 1 and " + length());
+        }
     }
 
     /**
      * Checks that the constant pool tag at index {@code index} is equal to {@code tag}.
      *
      * @param index constant pool index
      * @param tag expected tag
-     * @throws AssertionError if the check fails
+     * @throws IllegalArgumentException if the check fails
      */
-    private boolean checkTag(int index, JvmConstant tag) {
+    private void checkTag(int index, JvmConstant tag) {
         final JvmConstant tagAt = getTagAt(index);
-        assert tagAt == tag : "constant pool tag at index " + index + " is " + tagAt + " but expected " + tag;
-        return true;
+        if (tagAt != tag) {
+            throw new IllegalArgumentException("constant pool tag at index " + index + " is " + tagAt + " but expected " + tag);
+        }
     }
 
     /**
@@ -511,12 +527,13 @@ private boolean checkTag(int index, JvmConstant tag) {
      * {@link JvmConstants#jvmInterfaceMethodref}.
      *
      * @param index constant pool index
-     * @throws AssertionError if the check fails
+     * @throws IllegalArgumentException if the check fails
      */
-    private boolean checkTagIsFieldOrMethod(int index) {
+    private void checkTagIsFieldOrMethod(int index) {
         final JvmConstant tagAt = getTagAt(index);
-        assert tagAt == constants.jvmFieldref || tagAt == constants.jvmMethodref || tagAt == constants.jvmInterfaceMethodref : tagAt;
-        return true;
+        if (tagAt != constants.jvmFieldref && tagAt != constants.jvmMethodref && tagAt != constants.jvmInterfaceMethodref) {
+            throw new IllegalArgumentException("constant pool tag at index " + index + " is " + tagAt);
+        }
     }
 
     @Override
@@ -642,7 +659,6 @@ JavaConstant getStaticFieldConstantValue(int cpi) {
 
     @Override
     public Object lookupConstant(int cpi) {
-        assert cpi != 0;
         final JvmConstant tag = getTagAt(cpi);
         switch (tag.name) {
             case "Integer":
@@ -679,7 +695,7 @@ public Object lookupConstant(int cpi) {
 
     @Override
     public String lookupUtf8(int cpi) {
-        assert checkTag(cpi, constants.jvmUtf8);
+        checkTag(cpi, constants.jvmUtf8);
         return compilerToVM().getSymbol(getEntryAt(cpi));
     }
 
@@ -690,7 +706,10 @@ public Signature lookupSignature(int cpi) {
 
     @Override
     public JavaConstant lookupAppendix(int cpi, int opcode) {
-        assert Bytecodes.isInvoke(opcode);
+        if (!Bytecodes.isInvoke(opcode)) {
+            throw new IllegalArgumentException("expected an invoke bytecode at " + cpi + ", got " + opcode);
+        }
+
         final int index = rawIndexToConstantPoolCacheIndex(cpi, opcode);
         return compilerToVM().lookupAppendixInPool(this, index);
     }
@@ -822,10 +841,14 @@ public JavaField lookupField(int cpi, ResolvedJavaMethod method, int opcode) {
     public int rawIndexToConstantPoolIndex(int rawIndex, int opcode) {
         int index;
         if (isInvokedynamicIndex(rawIndex)) {
-            assert opcode == Bytecodes.INVOKEDYNAMIC;
+            if (opcode != Bytecodes.INVOKEDYNAMIC) {
+                throw new IllegalArgumentException("expected INVOKEDYNAMIC at " + rawIndex + ", got " + opcode);
+            }
             index = decodeInvokedynamicIndex(rawIndex) + config().constantPoolCpCacheIndexTag;
         } else {
-            assert opcode != Bytecodes.INVOKEDYNAMIC;
+            if (opcode == Bytecodes.INVOKEDYNAMIC) {
+                throw new IllegalArgumentException("unexpected INVOKEDYNAMIC at " + rawIndex);
+            }
             index = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);
         }
         return compilerToVM().constantPoolRemapInstructionOperandFromCache(this, index);
@@ -898,7 +921,7 @@ public void loadReferencedType(int cpi, int opcode, boolean initialize) {
                 if (tag == constants.jvmMethodref) {
                     if (Bytecodes.isInvokeHandleAlias(opcode) && isSignaturePolymorphicHolder(type)) {
                         final int methodRefCacheIndex = rawIndexToConstantPoolCacheIndex(cpi, opcode);
-                        assert checkTag(compilerToVM().constantPoolRemapInstructionOperandFromCache(this, methodRefCacheIndex), constants.jvmMethodref);
+                        checkTag(compilerToVM().constantPoolRemapInstructionOperandFromCache(this, methodRefCacheIndex), constants.jvmMethodref);
                         compilerToVM().resolveInvokeHandleInPool(this, methodRefCacheIndex);
                     }
                 }
@@ -950,7 +973,7 @@ static boolean isSignaturePolymorphicHolder(final ResolvedJavaType type) {
     public boolean isResolvedDynamicInvoke(int cpi, int opcode) {
         if (Bytecodes.isInvokeHandleAlias(opcode)) {
             final int methodRefCacheIndex = rawIndexToConstantPoolCacheIndex(cpi, opcode);
-            assert checkTag(compilerToVM().constantPoolRemapInstructionOperandFromCache(this, methodRefCacheIndex), constants.jvmMethodref);
+            checkTag(compilerToVM().constantPoolRemapInstructionOperandFromCache(this, methodRefCacheIndex), constants.jvmMethodref);
             int op = compilerToVM().isResolvedInvokeHandleInPool(this, methodRefCacheIndex);
             return op == opcode;
         }

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaFieldImpl.java

@@ -22,10 +22,10 @@
  */
 package jdk.vm.ci.hotspot;
 
+import static jdk.internal.misc.Unsafe.ADDRESS_SIZE;
 import static jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime;
 import static jdk.vm.ci.hotspot.HotSpotVMConfig.config;
 import static jdk.vm.ci.hotspot.UnsafeAccess.UNSAFE;
-import static jdk.internal.misc.Unsafe.ADDRESS_SIZE;
 
 import java.lang.annotation.Annotation;
 
@@ -45,28 +45,26 @@ class HotSpotResolvedJavaFieldImpl implements HotSpotResolvedJavaField {
     private JavaType type;
 
     /**
-     * Value of {@code fieldDescriptor::access_flags()}.
+     * Offset (in bytes) of field from start of its storage container (i.e. {@code instanceOop} or
+     * {@code Klass*}).
      */
     private final int offset;
 
     /**
      * Value of {@code fieldDescriptor::index()}.
      */
-    private final short index;
+    private final int index;
 
     /**
      * This value contains all flags as stored in the VM including internal ones.
      */
     private final int modifiers;
 
-    HotSpotResolvedJavaFieldImpl(HotSpotResolvedObjectTypeImpl holder, JavaType type, long offset, int modifiers, int index) {
+    HotSpotResolvedJavaFieldImpl(HotSpotResolvedObjectTypeImpl holder, JavaType type, int offset, int modifiers, int index) {
         this.holder = holder;
         this.type = type;
-        this.index = (short) index;
-        assert this.index == index;
-        assert offset != -1;
-        assert offset == (int) offset : "offset larger than int";
-        this.offset = (int) offset;
+        this.index = index;
+        this.offset = offset;
         this.modifiers = modifiers;
     }
 
@@ -143,6 +141,10 @@ public JavaType getType() {
 
     }
 
+    /**
+     * Gets the offset (in bytes) of field from start of its storage container (i.e.
+     * {@code instanceOop} or {@code Klass*}).
+     */
     @Override
     public int getOffset() {
         return offset;

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java

@@ -576,7 +576,7 @@ public int getVtableLength() {
         return result;
     }
 
-    HotSpotResolvedJavaField createField(JavaType type, long offset, int rawFlags, int index) {
+    HotSpotResolvedJavaField createField(JavaType type, int offset, int rawFlags, int index) {
         return new HotSpotResolvedJavaFieldImpl(this, type, offset, rawFlags, index);
     }
 

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ResolvedJavaField.java

@@ -40,6 +40,10 @@ public interface ResolvedJavaField extends JavaField, ModifiersProvider, Annotat
     @Override
     int getModifiers();
 
+    /**
+     * Returns the offset of the field relative to the base of its storage container (e.g.,
+     * {@code instanceOop} for an instance field or {@code Klass*} for a static field on HotSpot).
+     */
     int getOffset();
 
     default boolean isFinal() {

test/hotspot/jtreg/compiler/jvmci/common/patches/jdk.internal.vm.ci/jdk/vm/ci/hotspot/HotSpotResolvedJavaFieldHelper.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2022, 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.
+ */
+
+package jdk.vm.ci.hotspot;
+
+import jdk.vm.ci.meta.JavaType;
+import jdk.vm.ci.meta.ResolvedJavaField;
+
+public class HotSpotResolvedJavaFieldHelper {
+    public static ResolvedJavaField createField(HotSpotResolvedObjectTypeImpl holder, JavaType type, int offset, int modifiers, int index) {
+        return new HotSpotResolvedJavaFieldImpl(holder, type, offset, modifiers, index);
+    }
+
+    public static int getIndex(ResolvedJavaField field) {
+        return ((HotSpotResolvedJavaFieldImpl) field).getIndex();
+    }
+}