Skip to content

Commit

Permalink
Merge pull request #17816 from hangshao0/main
Browse files Browse the repository at this point in the history
Do not cache nesthost of hidden nestmate classes if extension enabled
  • Loading branch information
keithc-ca authored Jul 27, 2023
2 parents ecfd805 + bac839b commit 61dadcc
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 52 deletions.
15 changes: 10 additions & 5 deletions runtime/codert_vm/cnathelp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3204,6 +3204,8 @@ old_slow_jitNewInstanceImplAccessCheck(J9VMThread *currentThread)
J9InternalVMFunctions *vmFuncs = currentThread->javaVM->internalVMFunctions;
void *oldPC = buildJITResolveFrame(currentThread, J9_SSF_JIT_RESOLVE_RUNTIME_HELPER, parmCount);
IDATA checkResult = vmFuncs->checkVisibility(currentThread, callerClass, thisClass, thisClass->romClass->modifiers, J9_LOOK_REFLECT_CALL);
thisClass = VM_VMHelpers::currentClass(thisClass);
callerClass = VM_VMHelpers::currentClass(callerClass);
if (checkResult < J9_VISIBILITY_ALLOWED) {
illegalAccess:
if (VM_VMHelpers::immediateAsyncPending(currentThread)) {
Expand Down Expand Up @@ -3240,17 +3242,20 @@ old_slow_jitNewInstanceImplAccessCheck(J9VMThread *currentThread)
* unless the two classes are in the same nest (JDK11 and beyond).
*/
#if JAVA_SPEC_VERSION >= 11
if (NULL == thisClass->nestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, thisClass, 0)) {
J9Class *thisClassNestHost = thisClass->nestHost;
J9Class *callerClassNestHost = NULL;
if (NULL == thisClassNestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, thisClass, 0, &thisClassNestHost)) {
goto illegalAccess;
}
}
if (NULL == callerClass->nestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, callerClass, 0)) {
callerClassNestHost = callerClass->nestHost;
if (NULL == callerClassNestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, callerClass, 0, &callerClassNestHost)) {
goto illegalAccess;
}
}
if (thisClass->nestHost != callerClass->nestHost)
if (thisClassNestHost != callerClassNestHost)
#endif /* JAVA_SPEC_VERSION >= 11 */
{
if (thisClass != callerClass) {
Expand Down
14 changes: 8 additions & 6 deletions runtime/j9vm/java11vmi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1613,20 +1613,22 @@ JVM_AreNestMates(JNIEnv *env, jclass jClassOne, jclass jClassTwo)
} else {
J9Class *clazzOne = J9VM_J9CLASS_FROM_HEAPCLASS(currentThread, clazzObjectOne);
J9Class *clazzTwo = J9VM_J9CLASS_FROM_HEAPCLASS(currentThread, clazzObjectTwo);
J9Class *clazzOneNestHost = clazzOne->nestHost;
J9Class *clazzTwoNestHost = NULL;

if (NULL == clazzOne->nestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, clazzOne, J9_LOOK_NO_THROW)) {
if (NULL == clazzOneNestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, clazzOne, J9_LOOK_NO_THROW, &clazzOneNestHost)) {
goto done;
}
}

if (NULL == clazzTwo->nestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, clazzTwo, J9_LOOK_NO_THROW)) {
clazzTwoNestHost = clazzTwo->nestHost;
if (NULL == clazzTwoNestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, clazzTwo, J9_LOOK_NO_THROW, &clazzTwoNestHost)) {
goto done;
}
}

if (clazzOne->nestHost == clazzTwo->nestHost) {
if (clazzOneNestHost == clazzTwoNestHost) {
result = JNI_TRUE;
}
}
Expand Down
17 changes: 8 additions & 9 deletions runtime/jcl/common/java_lang_Class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1836,9 +1836,7 @@ Java_java_lang_Class_getNestHostImpl(JNIEnv *env, jobject recv)
J9Class *nestHost = clazz->nestHost;

if (NULL == nestHost) {
if (J9_VISIBILITY_ALLOWED == vmFuncs->loadAndVerifyNestHost(currentThread, clazz, J9_LOOK_NO_THROW)) {
nestHost = clazz->nestHost;
} else {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, clazz, J9_LOOK_NO_THROW, &nestHost)) {
/* If there is a failure loading or accessing the nest host, or if this class or interface does
* not specify a nest, then it is considered to belong to its own nest and this is returned as
* the host */
Expand Down Expand Up @@ -1882,10 +1880,9 @@ Java_java_lang_Class_getNestMembersImpl(JNIEnv *env, jobject recv)
J9Class *nestHost = clazz->nestHost;

if (NULL == nestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, clazz, 0)) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, clazz, 0, &nestHost)) {
goto _done;
}
nestHost = clazz->nestHost;
}
romHostClass = nestHost->romClass;
nestMemberCount = romHostClass->nestMemberCount;
Expand Down Expand Up @@ -1920,18 +1917,20 @@ Java_java_lang_Class_getNestMembersImpl(JNIEnv *env, jobject recv)
PUSH_OBJECT_IN_SPECIAL_FRAME(currentThread, resultObject);
J9Class *nestMember = vmFuncs->internalFindClassUTF8(currentThread, J9UTF8_DATA(nestMemberName), J9UTF8_LENGTH(nestMemberName), classLoader, J9_FINDCLASS_FLAG_THROW_ON_FAIL);
resultObject = POP_OBJECT_IN_SPECIAL_FRAME(currentThread);

if (NULL == nestMember) {
/* If internalFindClassUTF8 fails to find the nest member, it sets
* a NoClassDefFoundError
*/
goto _done;
} else if (NULL == nestMember->nestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, nestMember, 0)) {
}
nestMember = VM_VMHelpers::currentClass(nestMember);
J9Class *memberNestHost = nestMember->nestHost;
if (NULL == memberNestHost) {
if (J9_VISIBILITY_ALLOWED != vmFuncs->loadAndVerifyNestHost(currentThread, nestMember, 0, &memberNestHost)) {
goto _done;
}
}
if (nestMember->nestHost != nestHost) {
if (memberNestHost != nestHost) {
vmFuncs->setNestmatesError(currentThread, nestMember, nestHost, J9_VISIBILITY_NEST_MEMBER_NOT_CLAIMED_ERROR);
goto _done;
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/oti/j9nonbuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -4962,7 +4962,7 @@ typedef struct J9InternalVMFunctions {
void ( *throwNativeOOMError)(JNIEnv *env, U_32 moduleName, U_32 messageNumber);
void ( *throwNewJavaIoIOException)(JNIEnv *env, const char *message);
#if JAVA_SPEC_VERSION >= 11
UDATA ( *loadAndVerifyNestHost)(struct J9VMThread *vmThread, struct J9Class *clazz, UDATA options);
UDATA ( *loadAndVerifyNestHost)(struct J9VMThread *vmThread, struct J9Class *clazz, UDATA options, J9Class **nestHostFound);
void ( *setNestmatesError)(struct J9VMThread *vmThread, struct J9Class *nestMember, struct J9Class *nestHost, IDATA errorCode);
#endif /* JAVA_SPEC_VERSION >= 11 */
BOOLEAN ( *areValueTypesEnabled)(struct J9JavaVM *vm);
Expand Down
9 changes: 5 additions & 4 deletions runtime/oti/vm_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -3657,16 +3657,17 @@ trace(J9VMThread *vmStruct);
* to the same runtime package as the class. This function requires
* VMAccess
*
* @param vmThread handle to vmThread
* @param clazz the class to search the nest host
* @param options lookup options
* @param[in] vmThread handle to vmThread
* @param[in] clazz the class to search the nest host
* @param[in] options lookup options
* @param[out] nestHostFound the nest host of clazz
*
* @return J9_VISIBILITY_ALLOWED if success,
* J9_VISIBILITY_NEST_HOST_LOADING_FAILURE_ERROR if failed to load nesthost,
* J9_VISIBILITY_NEST_HOST_DIFFERENT_PACKAGE_ERROR if nesthost is not in the same runtime package
*/
UDATA
loadAndVerifyNestHost(J9VMThread *vmThread, J9Class *clazz, UDATA options);
loadAndVerifyNestHost(J9VMThread *vmThread, J9Class *clazz, UDATA options, J9Class **nestHostFound);

/**
* Sets the nestmates error based on the errorCode
Expand Down
4 changes: 3 additions & 1 deletion runtime/vm/j9vm.tdf
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ TraceEvent=Trc_VM_objectMonitorEnterBlocking_gotFlatLockInLoop Overhead=1 Level=
TraceExit=Trc_VM_objectMonitorEnterBlocking_Exit Overhead=1 Level=4 Template="objectMonitorEnterBlocking -- result = %p"

TraceEntry=Trc_VM_checkVisibility_Entry Overhead=1 Level=3 Template="checkVisibility from %p to %p modifiers=%zx"
TraceException=Trc_VM_checkVisibility_Failed Overhead=1 Level=1 Template="checkVisibility from %p (%.*s) to %p (%.*s) modifiers=%zx failed"
TraceException=Trc_VM_checkVisibility_Failed Obsolete Overhead=1 Level=1 Template="checkVisibility from %p (%.*s) to %p (%.*s) modifiers=%zx failed"
TraceExit=Trc_VM_checkVisibility_Exit Overhead=1 Level=3 Template="checkVisibility result=%x"

TraceEntry=Trc_VM_resolveKnownClass_Entry Overhead=1 Level=1 Template="resolveKnownClass(index=%zu)"
Expand Down Expand Up @@ -928,3 +928,5 @@ TraceEvent=Trc_VM_allocateThunkHeap_allocate_thunk_heap_node_failed NoEnv Overhe

TraceExit=Trc_VM_criu_initHooks_Exit Overhead=1 Level=5 Template="initializeCriuHooks - checkpointState.hookRecords (%p), classIterationRestoreHookRecords (%p), delayedLockingOperationsRecords (%p)"
TraceException=Trc_VM_criu_setSingleThreadModeJVMCRIUException_triggerOneOffJavaDump Overhead=1 Level=1 Template="setCRIUSingleThreadModeJVMCRIUException triggerOneOffDump() returns %d"

TraceException=Trc_VM_checkVisibility_Failed Overhead=1 Level=1 Template="checkVisibility from %p (%.*s) to %p (%.*s) modifiers=%zx lookupOptions=%zx failed"
79 changes: 53 additions & 26 deletions runtime/vm/visible.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,24 +129,26 @@ checkVisibility(J9VMThread *currentThread, J9Class* sourceClass, J9Class* destCl
* host is loaded first and the class being accessed loads its
* nest host after.
*/
if (NULL == sourceClass->nestHost) {
result = loadAndVerifyNestHost(currentThread, sourceClass, lookupOptions);
J9Class *sourceClassNestHost = sourceClass->nestHost;
J9Class *destClassNestHost = NULL;
if (NULL == sourceClassNestHost) {
result = loadAndVerifyNestHost(currentThread, sourceClass, lookupOptions, &sourceClassNestHost);
if (J9_VISIBILITY_ALLOWED != result) {
goto _exit;
}
sourceClass = J9_CURRENT_CLASS(sourceClass);
destClass = J9_CURRENT_CLASS(destClass);
}
if (NULL == destClass->nestHost) {
result = loadAndVerifyNestHost(currentThread, destClass, lookupOptions);
destClassNestHost = destClass->nestHost;
if (NULL == destClassNestHost) {
result = loadAndVerifyNestHost(currentThread, destClass, lookupOptions, &destClassNestHost);
if (J9_VISIBILITY_ALLOWED != result) {
goto _exit;
}
sourceClass = J9_CURRENT_CLASS(sourceClass);
destClass = J9_CURRENT_CLASS(destClass);
}

if (sourceClass->nestHost != destClass->nestHost) {
if (sourceClassNestHost != destClassNestHost) {
result = J9_VISIBILITY_NON_MODULE_ACCESS_ERROR;
}
} else
Expand Down Expand Up @@ -182,11 +184,11 @@ checkVisibility(J9VMThread *currentThread, J9Class* sourceClass, J9Class* destCl
_exit:
#endif /* JAVA_SPEC_VERSION >= 11 */
if (J9_VISIBILITY_NON_MODULE_ACCESS_ERROR == result) {
/* "checkVisibility from %p (%.*s) to %p (%.*s) modifiers=%zx failed" */
/* "checkVisibility from %p (%.*s) to %p (%.*s) modifiers=%zx lookupOptions=%zx failed" */
Trc_VM_checkVisibility_Failed(currentThread,
sourceClass, J9UTF8_LENGTH(J9ROMCLASS_CLASSNAME(sourceClass->romClass)), J9UTF8_DATA(J9ROMCLASS_CLASSNAME(sourceClass->romClass)),
destClass, J9UTF8_LENGTH(J9ROMCLASS_CLASSNAME(destClass->romClass)), J9UTF8_DATA(J9ROMCLASS_CLASSNAME(destClass->romClass)),
modifiers);
modifiers, lookupOptions);
}

Trc_VM_checkVisibility_Exit(currentThread, result);
Expand Down Expand Up @@ -271,29 +273,42 @@ setNestmatesError(J9VMThread *vmThread, J9Class *nestMember, J9Class *nestHost,
/*
* Loads and verifies the nesthost
*
* @param vmThread vmthread token
* @param clazz the j9lass requesting the nesthost
* @param options loading options (e.g. J9_LOOK_NO_THROW)
* @param[in] vmThread vmthread token
* @param[in] clazz the j9lass requesting the nesthost
* @param[in] options loading options (e.g. J9_LOOK_NO_THROW)
* @param[out] nestHostFound the nest host of clazz
* @return J9_VISIBILITY_ALLOWED if nest host is loaded, otherwise
* J9_VISIBILITY_NEST_HOST_LOADING_FAILURE_ERROR
* J9_VISIBILITY_NEST_HOST_DIFFERENT_PACKAGE_ERROR
* J9_VISIBILITY_NEST_MEMBER_NOT_CLAIMED_ERROR
*/
UDATA
loadAndVerifyNestHost(J9VMThread *vmThread, J9Class *clazz, UDATA options)
loadAndVerifyNestHost(J9VMThread *vmThread, J9Class *clazz, UDATA options, J9Class **nestHostFound)
{
UDATA result = J9_VISIBILITY_ALLOWED;
if (NULL == clazz->nestHost) {
J9Class *nestHost = NULL;
J9Class *nestHost = clazz->nestHost;
if (NULL != nestHost) {
*nestHostFound = nestHost;
} else {
J9ROMClass *romClass = clazz->romClass;
J9UTF8 *nestHostName = J9ROMCLASS_NESTHOSTNAME(romClass);
BOOLEAN canRunJavaCode = J9_ARE_NO_BITS_SET(options, J9_LOOK_NO_JAVA);
BOOLEAN throwException = canRunJavaCode && J9_ARE_NO_BITS_SET(options, J9_LOOK_NO_THROW);
BOOLEAN hiddenNestMate = J9ROMCLASS_IS_OPTIONNESTMATE_SET(romClass);
J9Class* curClazz = clazz;
BOOLEAN cacheNestHostInClass = TRUE;
J9Class *curClazz = clazz;

if (hiddenNestMate) {
BOOLEAN isCurClassHiddenNestMate = hiddenNestMate;
if (areExtensionsEnabled(vmThread->javaVM)) {
/*
* Hidden class is not in the nestmember attribute of the nest host. So nest host cannot find hidden class nest members.
* Class redefinition cannot update the nest host of hidden class in fixNestMembers()(hshelp.c).
* So do not set cache nest host in clazz->nestHost, as the host might have been redefined.
* Always using the class name to get the nest host from the class table.
*/
cacheNestHostInClass = FALSE;
}
while ((isCurClassHiddenNestMate)
&& (curClazz != curClazz->hostClass)
) {
Expand All @@ -302,15 +317,21 @@ loadAndVerifyNestHost(J9VMThread *vmThread, J9Class *clazz, UDATA options)
isCurClassHiddenNestMate = J9ROMCLASS_IS_OPTIONNESTMATE_SET(curClazz->romClass);
nestHostName = J9ROMCLASS_NESTHOSTNAME(curClazz->romClass);
}

if (NULL != curClazz->nestHost) {
/*
* hidden class defined with ClassOption.NESTMATE has the same nest host as its hostClass (curClazz).
* The nest host of curClass is already found, return directly.
*/
clazz->nestHost = curClazz->nestHost;
result = J9_VISIBILITY_ALLOWED;
goto done;
if (cacheNestHostInClass) {
if (NULL != curClazz->nestHost) {
/*
* hidden class defined with ClassOption.NESTMATE has the same nest host as its hostClass (curClazz).
* The nest host of curClass is already found, return directly.
*/
*nestHostFound = curClazz->nestHost;
clazz->nestHost = *nestHostFound;
result = J9_VISIBILITY_ALLOWED;
goto done;
}
} else {
if (NULL == nestHostName) {
nestHostName = J9ROMCLASS_CLASSNAME(curClazz->romClass);
}
}
}

Expand Down Expand Up @@ -360,7 +381,10 @@ loadAndVerifyNestHost(J9VMThread *vmThread, J9Class *clazz, UDATA options)

/* If a problem occurred in nest host verification then the nest host value is invalid */
if (J9_VISIBILITY_ALLOWED == result) {
clazz->nestHost = nestHost;
*nestHostFound = nestHost;
if (cacheNestHostInClass) {
clazz->nestHost = nestHost;
}
} else if ((JAVA_SPEC_VERSION >= 15) && canRunJavaCode) {
/**
* JVM spec updated in Java 15:
Expand All @@ -371,7 +395,10 @@ loadAndVerifyNestHost(J9VMThread *vmThread, J9Class *clazz, UDATA options)
* which tries to find the nest host in loaded classes only. It is possible that nest host is not loaded yet.
* So set clazz->nestHost only when canRunJavaCode is TRUE.
*/
clazz->nestHost = curClazz;
*nestHostFound = curClazz;
if (cacheNestHostInClass) {
clazz->nestHost = curClazz;
}
vmThread->currentException = NULL;
vmThread->privateFlags &= ~(UDATA)J9_PRIVATE_FLAGS_REPORT_EXCEPTION_THROW;
result = J9_VISIBILITY_ALLOWED;
Expand Down

0 comments on commit 61dadcc

Please sign in to comment.