From bac839b1e50b8fba68832342dbffb8cd201d5d6a Mon Sep 17 00:00:00 2001 From: Hang Shao Date: Thu, 20 Jul 2023 13:53:40 -0400 Subject: [PATCH] Do not cache nesthost of hidden nestmate classes if extension enabled Hidden nest member is not in the nestmember attribute of the nest host. If extensions are enabled and the nest host is redefined, the nest host of hidden nest member is not updated. Do not cache nest host into class->nestHost in this case so that the nest host will always be found in the class table for hidden nest members. Signed-off-by: Hang Shao --- runtime/codert_vm/cnathelp.cpp | 15 +++-- runtime/j9vm/java11vmi.c | 14 +++-- runtime/jcl/common/java_lang_Class.cpp | 17 +++--- runtime/oti/j9nonbuilder.h | 2 +- runtime/oti/vm_api.h | 9 +-- runtime/vm/j9vm.tdf | 4 +- runtime/vm/visible.c | 79 +++++++++++++++++--------- 7 files changed, 88 insertions(+), 52 deletions(-) diff --git a/runtime/codert_vm/cnathelp.cpp b/runtime/codert_vm/cnathelp.cpp index fba11ceef0e..20f4c940e1b 100644 --- a/runtime/codert_vm/cnathelp.cpp +++ b/runtime/codert_vm/cnathelp.cpp @@ -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)) { @@ -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) { diff --git a/runtime/j9vm/java11vmi.c b/runtime/j9vm/java11vmi.c index 5dfd5bd1bc8..b0f064c29a3 100644 --- a/runtime/j9vm/java11vmi.c +++ b/runtime/j9vm/java11vmi.c @@ -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; } } diff --git a/runtime/jcl/common/java_lang_Class.cpp b/runtime/jcl/common/java_lang_Class.cpp index 0ba447784b5..1bdbc50f631 100644 --- a/runtime/jcl/common/java_lang_Class.cpp +++ b/runtime/jcl/common/java_lang_Class.cpp @@ -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 */ @@ -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; @@ -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; } diff --git a/runtime/oti/j9nonbuilder.h b/runtime/oti/j9nonbuilder.h index eb6fe3f9a13..70cb94f2422 100644 --- a/runtime/oti/j9nonbuilder.h +++ b/runtime/oti/j9nonbuilder.h @@ -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); diff --git a/runtime/oti/vm_api.h b/runtime/oti/vm_api.h index 6f2263b5017..d227d08b1fc 100644 --- a/runtime/oti/vm_api.h +++ b/runtime/oti/vm_api.h @@ -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 diff --git a/runtime/vm/j9vm.tdf b/runtime/vm/j9vm.tdf index 362b88cd881..bb4c849ab1c 100644 --- a/runtime/vm/j9vm.tdf +++ b/runtime/vm/j9vm.tdf @@ -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)" @@ -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" diff --git a/runtime/vm/visible.c b/runtime/vm/visible.c index d61765fc18f..6d01b91898e 100644 --- a/runtime/vm/visible.c +++ b/runtime/vm/visible.c @@ -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 @@ -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); @@ -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) ) { @@ -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); + } } } @@ -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: @@ -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;