Skip to content

Commit

Permalink
Update the value of ACC_VALUE and remove j.l.IdentityObject
Browse files Browse the repository at this point in the history 
1. Update the value of ACC_VALUE
2. remove ACC_PERMITS_VALUE
3. Add ACC_IDENTITY
4. remove IdentityObject
5. Update test code
6. Re-include PackageAnnotation tests which were excluded because of 
the injected j.l.IdentityObject.
7. OpenJDK ASM code is not updated for ACC_IDENTITY yet, it does not
allow ACC_IDENTITY to be set. Exclude tests testGCRetransform and
test_EXTENDED_HCR which failed on access flags check of ACC_IDENTITY

issue eclipse-openj9#15081, eclipse-openj9#12878

Signed-off-by: Hang Shao <[email protected]>
  • Loading branch information
@hangshao0
hangshao0 committed May 26, 2022
1 parent e929d6b commit 13a8dfb
Show file tree
Hide file tree
Showing 13 changed files with 105 additions and 146 deletions.
27 changes: 0 additions & 27 deletions jcl/src/java.base/share/classes/java/lang/IdentityObject.java
View file

This file was deleted.

62 changes: 24 additions & 38 deletions runtime/bcutil/ClassFileOracle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
#include "j9protos.h"

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
#define VALUE_TYPES_MAJOR_VERSION 55
/* major version 63 is Java 19 */
#define VALUE_TYPES_MAJOR_VERSION 63
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */

/* The array entries must be in same order as the enums in ClassFileOracle.hpp */
Expand Down Expand Up @@ -212,8 +213,8 @@ ClassFileOracle::ClassFileOracle(BufferManager *bufferManager, J9CfrClassFile *c
_needsStaticConstantInit(false),
_isRecord(false),
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
_hasIdentityInterface(false),
_isIdentityInterfaceNeeded(false),
_hasIdentityFlagSet(false),
_isIdentityFlagNeeded(false),
_isValueType(false),
_hasNonStaticSynchronizedMethod(false),
_hasNonStaticFields(false),
Expand Down Expand Up @@ -260,6 +261,17 @@ ClassFileOracle::ClassFileOracle(BufferManager *bufferManager, J9CfrClassFile *c
*/
_hasNonEmptyConstructor = true;
}

if (J9_ARE_ALL_BITS_SET(_classFile->accessFlags, CFR_ACC_VALUE_TYPE | CFR_ACC_IDENTITY)) {
_buildResult = InvalidValueType;
} else {
if (J9_ARE_ALL_BITS_SET(_classFile->accessFlags, CFR_ACC_VALUE_TYPE)) {
_isValueType = true;
}
if (J9_ARE_ALL_BITS_SET(_classFile->accessFlags, CFR_ACC_IDENTITY)) {
_hasIdentityFlagSet = true;
}
}
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */

/* analyze class file */
Expand Down Expand Up @@ -290,7 +302,7 @@ ClassFileOracle::ClassFileOracle(BufferManager *bufferManager, J9CfrClassFile *c

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
if (OK == _buildResult) {
checkAndRecordIsIdentityInterfaceNeeded();
checkAndRecordIsIdentityFlagNeeded();
}
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
if (OK == _buildResult) {
Expand Down Expand Up @@ -723,9 +735,6 @@ class ClassFileOracle::InterfaceVisitor : public ClassFileOracle::ConstantPoolIn
InterfaceVisitor(ClassFileOracle *classFileOracle, ConstantPoolMap *constantPoolMap) :
_classFileOracle(classFileOracle),
_constantPoolMap(constantPoolMap),
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
_wasIdentityInterfaceSeen(false),
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */
_wasCloneableSeen(false),
_wasSerializableSeen(false)
{
Expand All @@ -745,28 +754,16 @@ class ClassFileOracle::InterfaceVisitor : public ClassFileOracle::ConstantPoolIn
_wasSerializableSeen = true;
}
#undef SERIALIZABLE_NAME

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
if( _classFileOracle->isUTF8AtIndexEqualToString(cpIndex, IDENTITY_OBJECT_NAME, sizeof(IDENTITY_OBJECT_NAME)) ) {
_wasIdentityInterfaceSeen = true;
}
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */
}

bool wasCloneableSeen() const { return _wasCloneableSeen; }
bool wasSerializableSeen() const { return _wasSerializableSeen; }
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
bool wasIdentityInterfaceSeen() const { return _wasIdentityInterfaceSeen; }
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */

private:
ClassFileOracle *_classFileOracle;
ConstantPoolMap *_constantPoolMap;
bool _wasCloneableSeen;
bool _wasSerializableSeen;
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
bool _wasIdentityInterfaceSeen;
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */
};

void
Expand All @@ -782,32 +779,23 @@ ClassFileOracle::walkInterfaces()
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */
_isCloneable = interfaceVisitor.wasCloneableSeen();
_isSerializable = interfaceVisitor.wasSerializableSeen();
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
if (J9_ARE_ALL_BITS_SET(_classFile->accessFlags, CFR_ACC_VALUE_TYPE)) {
_isValueType = true;
}
_hasIdentityInterface = interfaceVisitor.wasIdentityInterfaceSeen();
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */
}

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
void
ClassFileOracle::checkAndRecordIsIdentityInterfaceNeeded()
ClassFileOracle::checkAndRecordIsIdentityFlagNeeded()
{
if (isValueType()) {
if (_hasIdentityInterface
|| J9_ARE_ANY_BITS_SET(_classFile->accessFlags, CFR_ACC_ABSTRACT | CFR_ACC_INTERFACE)
|| _hasNonStaticSynchronizedMethod
) {
if (_hasNonStaticSynchronizedMethod) {
_buildResult = InvalidValueType;
}
} else {
if (!_hasIdentityInterface
if (!_hasIdentityFlagSet
&& (getSuperClassNameIndex() != 0) /* j.l.Object has no superClass */
) {
if (J9_ARE_NO_BITS_SET(_classFile->accessFlags, CFR_ACC_ABSTRACT | CFR_ACC_INTERFACE)) {
/* For concrete classes, IdentityInterface is needed. */
_isIdentityInterfaceNeeded = true;
_isIdentityFlagNeeded = true;
} else {
/**
* For abstract classes, IdentityInterface is needed only when it has non-static fields,
Expand All @@ -817,14 +805,12 @@ ClassFileOracle::checkAndRecordIsIdentityInterfaceNeeded()
|| (_hasNonStaticSynchronizedMethod)
|| (_hasNonEmptyConstructor)
) {
_isIdentityInterfaceNeeded = true;
_isIdentityFlagNeeded = true;
}
}
}
}
if (J9_ARE_ALL_BITS_SET(_classFile->accessFlags, CFR_ACC_PERMITS_VALUE)) {
if (_hasIdentityInterface || _isIdentityInterfaceNeeded) {
_buildResult = InvalidValueType;
if (_isIdentityFlagNeeded) {
_classFile->accessFlags |= CFR_ACC_IDENTITY;
}
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions runtime/bcutil/ClassFileOracle.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1014,8 +1014,8 @@ class RecordComponentIterator
U_16 getPermittedSubclassesClassCount() const { return _isSealed ? _permittedSubclassesAttribute->numberOfClasses : 0; }
bool isValueBased() const { return _isClassValueBased; }
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
bool needsIdentityInterface() const { return _isIdentityInterfaceNeeded; }
bool hasIdentityInterface() const { return _hasIdentityInterface; }
bool needsIdentityFlag() const { return _isIdentityFlagNeeded; }
bool hasIdentityFlagSet() const { return _hasIdentityFlagSet; }
bool isValueType() const { return _isValueType; }
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */

Expand Down Expand Up @@ -1119,8 +1119,8 @@ class RecordComponentIterator
bool _isSealed;
bool _isClassValueBased;
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
bool _hasIdentityInterface;
bool _isIdentityInterfaceNeeded;
bool _hasIdentityFlagSet;
bool _isIdentityFlagNeeded;
bool _isValueType;
bool _hasNonStaticSynchronizedMethod;
bool _hasNonStaticFields;
Expand Down Expand Up @@ -1150,7 +1150,7 @@ class RecordComponentIterator
void checkHiddenClass();
void walkInterfaces();
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
void checkAndRecordIsIdentityInterfaceNeeded();
void checkAndRecordIsIdentityFlagNeeded();
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
void walkMethods();
void walkRecordComponents(J9CfrAttributeRecord *attrib);
Expand Down
18 changes: 3 additions & 15 deletions runtime/bcutil/ROMClassBuilder.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2001, 2021 IBM Corp. and others
* Copyright (c) 2001, 2022 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -254,14 +254,11 @@ BuildResult
ROMClassBuilder::injectInterfaces(ClassFileOracle *classFileOracle)
{
U_32 numOfInterfaces = 0;
if (classFileOracle->needsIdentityInterface()) {
J9_DECLARE_CONSTANT_UTF8(identityInterface, IDENTITY_OBJECT_NAME);
_interfaceInjectionInfo.interfaces[numOfInterfaces++] = (J9UTF8 *) &identityInterface;
}
_interfaceInjectionInfo.numOfInterfaces = numOfInterfaces;
return OK;
}
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */

BuildResult
ROMClassBuilder::handleAnonClassName(J9CfrClassFile *classfile, bool *isLambda, ROMClassCreationContext *context)
{
Expand Down Expand Up @@ -542,7 +539,6 @@ ROMClassBuilder::prepareAndLaydown( BufferManager *bufferManager, ClassFileParse
#else /* J9VM_OPT_VALHALLA_VALUE_TYPES */
SRPKeyProducer srpKeyProducer(&classFileOracle);
#endif /* J9VM_OPT_VALHALLA_VALUE_TYPES */

/*
* The ROMClassWriter must be constructed before the SRPOffsetTable because it generates additional SRP keys.
* There must be no SRP keys generated after the SRPOffsetTable is initialized.
Expand Down Expand Up @@ -1175,7 +1171,7 @@ ROMClassBuilder::finishPrepareAndLaydown(
* + UNUSED
*
* + UNUSED
* + J9AccClassHasIdentity
* + UNUSED
* + J9AccClassIsValueBased
* + J9AccClassHiddenOptionNestmate
*
Expand Down Expand Up @@ -1263,14 +1259,6 @@ ROMClassBuilder::computeExtraModifiers(ClassFileOracle *classFileOracle, ROMClas
modifiers |= J9AccClassIsValueBased;
}

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
if (classFileOracle->hasIdentityInterface()
|| classFileOracle->needsIdentityInterface()
) {
modifiers |= J9AccClassHasIdentity;
}
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */

U_32 classNameindex = classFileOracle->getClassNameIndex();

#define WEAK_NAME "java/lang/ref/WeakReference"
Expand Down
50 changes: 19 additions & 31 deletions runtime/bcutil/cfreader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1637,16 +1637,16 @@ checkFields(J9PortLibrary* portLib, J9CfrClassFile * classfile, U_8 * segment, U

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_VALUE_TYPE)) {
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;
}
}
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_PERMITS_VALUE)) {
/* if CFR_ACC_PERMITS_VALUE is set, we already know that CFR_ACC_ABSTRACT must be set as well */
if (J9_ARE_NO_BITS_SET(value, CFR_ACC_STATIC)) {
errorCode = J9NLS_CFR_ERR_MISSING_ACC_STATIC_ON_ABSTRACT_CLASS_FIELD__ID;
goto _errorFound;
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;
}
}
}
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
Expand Down Expand Up @@ -1902,12 +1902,10 @@ checkMethods(J9PortLibrary* portLib, J9CfrClassFile* classfile, U_8* segment, U_
}

#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_VALUE_TYPE) ||
J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_PERMITS_VALUE))
{
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_VALUE_TYPE)) {
if (J9_ARE_ALL_BITS_SET(value, CFR_ACC_SYNCHRONIZED)) {
if (J9_ARE_NO_BITS_SET(value, CFR_ACC_STATIC)) {
errorCode = J9NLS_CFR_ERR_NON_STATIC_SYNCHRONIZED_VALUE_TYPE_METHOD__ID;
errorCode = J9NLS_CFR_ERR_NON_STATIC_SYNCHRONIZED_VALUE_TYPE_METHOD_V1__ID;
goto _errorFound;
}
}
Expand Down Expand Up @@ -2939,7 +2937,7 @@ j9bcutil_readClassFileBytes(J9PortLibrary *portLib,
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
/* Currently value type is built on JDK19, so compare with JDK19 for now. Eventually it may need to compare to JDK20. */
if ((flags & BCT_MajorClassFileVersionMask) < BCT_JavaMajorVersionShifted(19)) {
classfile->accessFlags &= ~(CFR_ACC_VALUE_TYPE | CFR_ACC_PRIMITIVE_VALUE_TYPE | CFR_ACC_PERMITS_VALUE);
classfile->accessFlags &= ~(CFR_ACC_VALUE_TYPE | CFR_ACC_PRIMITIVE_VALUE_TYPE | CFR_ACC_IDENTITY);
}
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_PRIMITIVE_VALUE_TYPE)) {
if (J9_ARE_NO_BITS_SET(classfile->accessFlags, CFR_ACC_VALUE_TYPE)) {
Expand All @@ -2949,30 +2947,20 @@ j9bcutil_readClassFileBytes(J9PortLibrary *portLib,
}
}
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_VALUE_TYPE)) {
if (J9_ARE_NO_BITS_SET(classfile->accessFlags, CFR_ACC_FINAL)) {
errorCode = J9NLS_CFR_ERR_FINAL_FLAG_MISSING_ON_VALUE_CLASS__ID;
if (J9_ARE_NO_BITS_SET(classfile->accessFlags, CFR_ACC_ABSTRACT | CFR_ACC_FINAL)) {
errorCode = J9NLS_CFR_ERR_FINAL_ABSTRACT_FLAG_MISSING_ON_VALUE_CLASS__ID;
offset = index - data - 2;
goto _errorFound;
}
if (J9_ARE_ANY_BITS_SET(classfile->accessFlags, CFR_ACC_ABSTRACT | CFR_ACC_PERMITS_VALUE | CFR_ACC_ENUM | CFR_ACC_INTERFACE | CFR_ACC_MODULE)) {
if (J9_ARE_ANY_BITS_SET(classfile->accessFlags, CFR_ACC_IDENTITY | CFR_ACC_ENUM | CFR_ACC_MODULE)) {
errorCode = J9NLS_CFR_ERR_INCORRECT_FLAG_FOUND_ON_VALUE_CLASS__ID;
offset = index - data - 2;
goto _errorFound;
}
}
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_PERMITS_VALUE)) {
if (J9_ARE_NO_BITS_SET(classfile->accessFlags, CFR_ACC_ABSTRACT)) {
errorCode = J9NLS_CFR_ERR_PERMITS_VALUE_ON_NON_ABSTRACT_CLASS__ID;
offset = index - data - 2;
goto _errorFound;
}
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_FINAL)) {
errorCode = J9NLS_CFR_ERR_PERMITS_VALUE_ON_FINAL_CLASS__ID;
offset = index - data - 2;
goto _errorFound;
}
if (J9_ARE_ALL_BITS_SET(classfile->accessFlags, CFR_ACC_INTERFACE)) {
errorCode = J9NLS_CFR_ERR_PERMITS_VALUE_ON_INTERFACE__ID;
if (J9_IS_CLASSFILE_PRIMITIVE_VALUETYPE(classfile)) {
if (J9_ARE_ANY_BITS_SET(classfile->accessFlags, CFR_ACC_ABSTRACT | CFR_ACC_INTERFACE)) {
errorCode = J9NLS_CFR_ERR_INCORRECT_FLAG_FOUND_ON_PRMITIVE_VALUE_CLASS__ID;
offset = index - data - 2;
goto _errorFound;
}
Expand Down
28 changes: 28 additions & 0 deletions runtime/nls/cfre/cfrerr.nls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1668,3 +1668,31 @@ J9NLS_CFR_ERR_INIT_ON_VALUE_CLASS.explanation=Please consult the Java Virtual Ma
J9NLS_CFR_ERR_INIT_ON_VALUE_CLASS.system_action=The JVM will throw a verification or class loading-related exception such as java.lang.ClassFormatError.
J9NLS_CFR_ERR_INIT_ON_VALUE_CLASS.user_response=Contact the provider of the class file for a corrected version.
# END NON-TRANSLATABLE

J9NLS_CFR_ERR_MISSING_ACC_STATIC_ON_ABSTRACT_IDENTITYLESS_CLASS_FIELD=In an abstract non-identity class, each field must be declared static.
# START NON-TRANSLATABLE
J9NLS_CFR_ERR_MISSING_ACC_STATIC_ON_ABSTRACT_IDENTITYLESS_CLASS_FIELD.explanation=Please consult the Java Virtual Machine Specification for a detailed explanation.
J9NLS_CFR_ERR_MISSING_ACC_STATIC_ON_ABSTRACT_IDENTITYLESS_CLASS_FIELD.system_action=The JVM will throw a verification or class loading-related exception such as java.lang.ClassFormatError.
J9NLS_CFR_ERR_MISSING_ACC_STATIC_ON_ABSTRACT_IDENTITYLESS_CLASS_FIELD.user_response=Contact the provider of the class file for a corrected version.
# END NON-TRANSLATABLE

J9NLS_CFR_ERR_NON_STATIC_SYNCHRONIZED_VALUE_TYPE_METHOD_V1=In value classes a synchronized method must be static
# START NON-TRANSLATABLE
J9NLS_CFR_ERR_NON_STATIC_SYNCHRONIZED_VALUE_TYPE_METHOD_V1.explanation=Please consult the Java Virtual Machine Specification for a detailed explanation.
J9NLS_CFR_ERR_NON_STATIC_SYNCHRONIZED_VALUE_TYPE_METHOD_V1.system_action=The JVM will throw a verification or class loading-related exception such as java.lang.ClassFormatError.
J9NLS_CFR_ERR_NON_STATIC_SYNCHRONIZED_VALUE_TYPE_METHOD_V1.user_response=Contact the provider of the class file for a corrected version.
# END NON-TRANSLATABLE

J9NLS_CFR_ERR_FINAL_ABSTRACT_FLAG_MISSING_ON_VALUE_CLASS=Access flag ACC_FINAL or ACC_ABSTRACT is missing on the value class.
# START NON-TRANSLATABLE
J9NLS_CFR_ERR_FINAL_ABSTRACT_FLAG_MISSING_ON_VALUE_CLASS.explanation=Please consult the Java Virtual Machine Specification for a detailed explanation.
J9NLS_CFR_ERR_FINAL_ABSTRACT_FLAG_MISSING_ON_VALUE_CLASS.system_action=The JVM will throw a verification or class loading-related exception such as java.lang.ClassFormatError.
J9NLS_CFR_ERR_FINAL_ABSTRACT_FLAG_MISSING_ON_VALUE_CLASS.user_response=Contact the provider of the class file for a corrected version.
# END NON-TRANSLATABLE

J9NLS_CFR_ERR_INCORRECT_FLAG_FOUND_ON_PRMITIVE_VALUE_CLASS=Incorrect access flag found on the primitive value class.
# START NON-TRANSLATABLE
J9NLS_CFR_ERR_INCORRECT_FLAG_FOUND_ON_PRMITIVE_VALUE_CLASS.explanation=Please consult the Java Virtual Machine Specification for a detailed explanation.
J9NLS_CFR_ERR_INCORRECT_FLAG_FOUND_ON_PRMITIVE_VALUE_CLASS.system_action=The JVM will throw a verification or class loading-related exception such as java.lang.ClassFormatError.
J9NLS_CFR_ERR_INCORRECT_FLAG_FOUND_ON_PRMITIVE_VALUE_CLASS.user_response=Contact the provider of the class file for a corrected version.
# END NON-TRANSLATABLE
Loading

0 comments on commit 13a8dfb

Please sign in to comment.