Merge pull request eclipse-openj9#20399 from theresa-m/fix_jtreg_Stri…

…ctFields Fix Valhalla Jtreg StrictFields test
babsingh · Oct 25, 2024 · 6544c13 · 6544c13
2 parents 35db7ee + 4715350
commit 6544c13
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 24 deletions.
diff --git a/runtime/bcutil/cfreader.c b/runtime/bcutil/cfreader.c
@@ -1708,35 +1708,24 @@ checkFields(J9PortLibrary* portLib, J9CfrClassFile * classfile, U_8 * segment, U
 
 #if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
 		if (J9_IS_CLASSFILE_VALUETYPE(classfile)) {
-			if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_ABSTRACT)) {
-				if (J9_ARE_NO_BITS_SET(value, CFR_ACC_STATIC)) {
-					errorCode = J9NLS_CFR_ERR_MISSING_ACC_STATIC_ON_ABSTRACT_IDENTITYLESS_CLASS_FIELD__ID;
-					goto _errorFound;
-				}
-			} else {
-				if (J9_ARE_NO_BITS_SET(value, CFR_ACC_STATIC | CFR_ACC_FINAL)) {
-					errorCode = J9NLS_CFR_ERR_VALUE_CLASS_FIELD_NOT_STATIC_OR_FINAL__ID;
-					goto _errorFound;
-				}
-			}
-
 			/* Each field of a value class must have exactly one of its ACC_STATIC or ACC_STRICT flags set. */
 			if (J9_ARE_NO_BITS_SET(value, CFR_ACC_STRICT | CFR_ACC_STATIC)) {
 				errorCode = J9NLS_CFR_ERR_VALUE_CLASS_FIELD_NOT_STATIC_OR_STRICT__ID;
 				goto _errorFound;
 			}
 		}
 
-		/* A field must not have set both ACC_STRICT and ACC_STATIC. */
-		if (J9_ARE_ALL_BITS_SET(value, CFR_ACC_STRICT | CFR_ACC_STATIC)) {
-			errorCode = J9NLS_CFR_ERR_FIELD_CANT_BE_STRICT_AND_STATIC__ID;
-			goto _errorFound;
-		}
-
-		/* A field that has set ACC_STRICT must also have set ACC_FINAL. */
-		if (J9_ARE_ALL_BITS_SET(value, CFR_ACC_STRICT) && J9_ARE_NO_BITS_SET(value, CFR_ACC_FINAL)) {
-			errorCode = J9NLS_CFR_ERR_STRICT_FIELD_MUST_BE_FINAL__ID;
-			goto _errorFound;
+		if (J9ROMFIELD_IS_STRICT(classfile, value)) {
+			/* A field must not have set both ACC_STRICT and ACC_STATIC. */
+			if (J9_ARE_ALL_BITS_SET(value, CFR_ACC_STATIC)) {
+				errorCode = J9NLS_CFR_ERR_FIELD_CANT_BE_STRICT_AND_STATIC__ID;
+				goto _errorFound;
+			}
+			/* A field that has set ACC_STRICT must also have set ACC_FINAL. */
+			if (J9_ARE_NO_BITS_SET(value, CFR_ACC_FINAL)) {
+				errorCode = J9NLS_CFR_ERR_STRICT_FIELD_MUST_BE_FINAL__ID;
+				goto _errorFound;
+			}
 		}
 #endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
 

diff --git a/runtime/bcverify/vrfyhelp.c b/runtime/bcverify/vrfyhelp.c
@@ -906,7 +906,7 @@ isFieldAccessCompatible(J9BytecodeVerificationData *verifyData, J9ROMFieldRef *f
 		J9BranchTargetStack *liveStack = (J9BranchTargetStack *)verifyData->liveStack;
 		J9ROMFieldShape *field = findFieldFromCurrentRomClass(romClass, fieldRef);
 #if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
-		IDATA isStrictField = (NULL != field) && J9_ARE_ALL_BITS_SET(field->modifiers, J9AccStrict);
+		IDATA isStrictField = (NULL != field) && J9ROMFIELD_IS_STRICT(romClass, field->modifiers);
 #endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
 		if (J9_ARE_ALL_BITS_SET(receiver, BCV_SPECIAL_INIT)) {
 #if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)

diff --git a/runtime/oti/j9.h b/runtime/oti/j9.h
@@ -352,6 +352,7 @@ static const struct { \
 #define J9_VALUETYPE_FLATTENED_SIZE(clazz) (J9CLASS_HAS_4BYTE_PREPADDING((clazz)) ? ((clazz)->totalInstanceSize - sizeof(U_32)) : (clazz)->totalInstanceSize)
 #define J9_IS_J9ARRAYCLASS_NULL_RESTRICTED(clazz) J9_ARE_ALL_BITS_SET((clazz)->classFlags, J9ClassArrayIsNullRestricted)
 #define J9CLASS_GET_NULLRESTRICTED_ARRAY(clazz) (J9_IS_J9CLASS_VALUETYPE(clazz) ? (clazz)->nullRestrictedArrayClass : NULL)
+#define J9ROMFIELD_IS_STRICT(romClassOrClassfile, fieldModifiers) (J9_IS_CLASSFILE_OR_ROMCLASS_VALUETYPE_VERSION(romClassOrClassfile) && J9_ARE_ALL_BITS_SET(fieldModifiers, J9AccStrict))
 #else /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
 #define J9CLASS_UNPADDED_INSTANCE_SIZE(clazz) ((clazz)->totalInstanceSize)
 #define J9_IS_J9CLASS_ALLOW_DEFAULT_VALUE(clazz) FALSE
@@ -362,6 +363,7 @@ static const struct { \
 #define J9_VALUETYPE_FLATTENED_SIZE(clazz)((UDATA) 0) /* It is not possible for this macro to be used since we always check J9_IS_J9CLASS_FLATTENED before ever using it. */
 #define J9_IS_J9ARRAYCLASS_NULL_RESTRICTED(clazz) FALSE
 #define J9CLASS_GET_NULLRESTRICTED_ARRAY(clazz) NULL
+#define J9ROMFIELD_IS_STRICT(romClassOrClassfile, fieldModifiers) FALSE
 #endif /* defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES) */
 #define IS_CLASS_SIGNATURE(firstChar) ('L' == (firstChar))
 

diff --git a/test/functional/Valhalla/src_qtypes/org/openj9/test/lworld/ValueTypeTests.java b/test/functional/Valhalla/src_qtypes/org/openj9/test/lworld/ValueTypeTests.java
@@ -2904,7 +2904,7 @@ static public void testAccStrictFieldMustHaveAccFinal() {
 	}
 
 	/* Each field of a value class must have exactly one of its ACC_STATIC or ACC_STRICT flags set */
-	@Test(expectedExceptions = java.lang.ClassFormatError.class, expectedExceptionsMessageRegExp = ".*Fields of value classes must have either ACC_STATIC or ACC_FINAL flags set.*")
+	@Test(expectedExceptions = java.lang.ClassFormatError.class, expectedExceptionsMessageRegExp = ".*Value class fields must have either ACC_STATIC or ACC_STRICT set.*")
 	static public void testValueClassFieldMustHaveAccStaticOrAccStrict() {
 		ValueTypeGenerator.generateTestValueClassFieldMustHaveAccStaticOrAccStrict();
 	}