From 6a561bac686f645d3b263369a2401260d1ec144d Mon Sep 17 00:00:00 2001 From: Jason Feng Date: Thu, 19 Sep 2024 15:25:58 -0400 Subject: [PATCH] -agentpath: libraries require no path/name decoration The agent library specified via -agentpath: has an absolute path/name, it should not be decorated again during the agent library loading; For -agentpath: option in the restore option file, compare the actual platform-dependent library name; Added isCheckpointAllowed_VM()/isDebugOnRestoreEnabled_VM(), and refactored recent CRIU debug support related usages. Signed-off-by: Jason Feng --- runtime/jvmti/jvmtiCapability.c | 5 +-- runtime/jvmti/jvmtiHook.c | 2 +- runtime/jvmti/jvmtiStartup.c | 68 +++++++++++++++++++++++---------- runtime/oti/j9nonbuilder.h | 2 + runtime/oti/vm_api.h | 27 +++++++++++++ runtime/vm/CRIUHelpers.cpp | 22 ++++++++--- runtime/vm/intfunc.c | 2 + runtime/vm/jvminit.c | 2 +- runtime/vm/lookuphelper.c | 2 +- runtime/vm/vmhook.c | 2 +- 10 files changed, 101 insertions(+), 33 deletions(-) diff --git a/runtime/jvmti/jvmtiCapability.c b/runtime/jvmti/jvmtiCapability.c index 3de11be2643..df8999672b9 100644 --- a/runtime/jvmti/jvmtiCapability.c +++ b/runtime/jvmti/jvmtiCapability.c @@ -548,10 +548,9 @@ mapCapabilitiesToEvents(J9JVMTIEnv * j9env, jvmtiCapabilities * capabilities, J9 #if defined(J9VM_OPT_CRIU_SUPPORT) J9JavaVM * vm = j9env->vm; - J9VMThread *mainThread = vm->mainThread; J9InternalVMFunctions *vmFuncs = vm->internalVMFunctions; - BOOLEAN skipHookReserve = vmFuncs->isCheckpointAllowed(mainThread) - && vmFuncs->isDebugOnRestoreEnabled(mainThread); + BOOLEAN skipHookReserve = vmFuncs->isCheckpointAllowed_VM(vm) + && vmFuncs->isDebugOnRestoreEnabled_VM(vm); /* Skip J9HookReserve for the events required by JDWP agent pre-checkpoint when DebugOnRestore is enabled, * these events will be registered post-restore if a JDWP agent is specified in the restore option file, * otherwise they are going to be unregistered by J9HookUnregister() which only clears J9HOOK_FLAG_HOOKED, diff --git a/runtime/jvmti/jvmtiHook.c b/runtime/jvmti/jvmtiHook.c index bc083f0155d..ceae1eace87 100644 --- a/runtime/jvmti/jvmtiHook.c +++ b/runtime/jvmti/jvmtiHook.c @@ -2047,7 +2047,7 @@ hookGlobalEvents(J9JVMTIData * jvmtiData) } #if defined(J9VM_OPT_CRIU_SUPPORT) - if (vm->internalVMFunctions->isDebugOnRestoreEnabled(vm->mainThread)) { + if (vm->internalVMFunctions->isDebugOnRestoreEnabled_VM(vm)) { if ((*vmHook)->J9HookRegisterWithCallSite(vmHook, J9HOOK_TAG_AGENT_ID | J9HOOK_VM_PREPARING_FOR_RESTORE, jvmtiHookVMRestoreCRIUInit, OMR_GET_CALLSITE(), jvmtiData, J9HOOK_AGENTID_FIRST)) { return 1; } diff --git a/runtime/jvmti/jvmtiStartup.c b/runtime/jvmti/jvmtiStartup.c index 2d50b521f63..56fe7eb9520 100644 --- a/runtime/jvmti/jvmtiStartup.c +++ b/runtime/jvmti/jvmtiStartup.c @@ -57,7 +57,8 @@ static J9JVMTIAgentLibrary* findAgentLibrary(J9JavaVM *vm, const char *libraryAn static jint issueAgentOnLoadAttach(J9JavaVM *vm, J9JVMTIAgentLibrary *agentLibrary, const char *options, char *loadFunctionName, BOOLEAN *foundLoadFn); I_32 JNICALL loadAgentLibraryOnAttach(struct J9JavaVM *vm, const char *library, const char *options, UDATA decorate); static BOOLEAN isAgentLibraryLoaded(J9JavaVM *vm, const char *library, BOOLEAN decorate); -static jint createAgentLibraryWithOption(J9JavaVM *vm, J9VMInitArgs *argsList, IDATA agentIndex, J9JVMTIAgentLibrary **agentLibrary, BOOLEAN isXrunjdwp, BOOLEAN *isJDWPagent); +static jint createAgentLibraryWithOption(J9JavaVM *vm, J9VMInitArgs *argsList, IDATA agentIndex, J9JVMTIAgentLibrary **agentLibrary, BOOLEAN isXrunjdwp, UDATA decorate, BOOLEAN *isJDWPagent); +static BOOLEAN processAgentLibraryFromArgsList(J9JavaVM *vm, J9VMInitArgs *argsList, BOOLEAN loadLibrary, BOOLEAN isXrunjdwp, BOOLEAN isAgentPath); #define INSTRUMENT_LIBRARY "instrument" @@ -121,12 +122,15 @@ parseLibraryAndOptions(char *libraryAndOptions, UDATA *libraryLength, char **opt * @param[in] agentIndex the option index for the agent library * @param[in/out] agentLibrary environment for the agent * @param[in] isXrunjdwp indicate if the agent option is MAPOPT_XRUNJDWP + * @param[in] decorate change the library path/name * @param[in/out] isJDWPagent indicate if DebugOnRestore is enabled and the agent is JDWP * * @return JNI_OK if succeeded, otherwise JNI_ERR/JNI_ENOMEM */ static jint -createAgentLibraryWithOption(J9JavaVM *vm, J9VMInitArgs *argsList, IDATA agentIndex, J9JVMTIAgentLibrary **agentLibrary, BOOLEAN isXrunjdwp, BOOLEAN *isJDWPagent) +createAgentLibraryWithOption( + J9JavaVM *vm, J9VMInitArgs *argsList, IDATA agentIndex, J9JVMTIAgentLibrary **agentLibrary, + BOOLEAN isXrunjdwp, UDATA decorate, BOOLEAN *isJDWPagent) { jint result = JNI_OK; char optionsBuf[OPTIONSBUFF_LEN]; @@ -162,15 +166,26 @@ createAgentLibraryWithOption(J9JavaVM *vm, J9VMInitArgs *argsList, IDATA agentIn UDATA optionsLength = 0; parseLibraryAndOptions(optionsPtr, &libraryLength, &options, &optionsLength); - result = createAgentLibrary(vm, optionsPtr, libraryLength, options, optionsLength, TRUE, agentLibrary); + result = createAgentLibrary(vm, optionsPtr, libraryLength, options, optionsLength, decorate, agentLibrary); Trc_JVMTI_createAgentLibraryWithOption_agentlib_result(optionsPtr, libraryLength, options, optionsLength, *agentLibrary, result); } #if defined(J9VM_OPT_CRIU_SUPPORT) - if ((JNI_OK == result) - && (isXrunjdwp || (0 == strncmp(JDWP_AGENT, optionsPtr, libraryLength))) - && (vm->internalVMFunctions->isDebugOnRestoreEnabled(vm->mainThread)) - ){ - *isJDWPagent = TRUE; + if ((JNI_OK == result) && vm->internalVMFunctions->isDebugOnRestoreEnabled_VM(vm)){ + /* 1. If isXrunjdwp is TRUE, the agent option is MAPOPT_XRUNJDWP which implies a JDWP agent. + * 2. If decorate is TRUE, the agent option is VMOPT_AGENTLIB_COLON, + * just compare the platform independent library name. + * 3. If decorate is FALSE, the agent option is VMOPT_AGENTPATH_COLON, + * compare the actual platform-specific library name with libjdwp.so. + * CRIU only supports Linux OS flavours. + */ +#define LIB_JDWP "libjdwp.so" + if (isXrunjdwp + || (decorate && (0 == strncmp(JDWP_AGENT, optionsPtr, libraryLength))) + || (!decorate && (0 == strncmp(LIB_JDWP, optionsPtr + libraryLength - LITERAL_STRLEN(LIB_JDWP), LITERAL_STRLEN(LIB_JDWP)))) + ) { + *isJDWPagent = TRUE; + } +#undef LIB_JDWP } #endif /* defined(J9VM_OPT_CRIU_SUPPORT) */ #undef JDWP_AGENT @@ -185,25 +200,36 @@ createAgentLibraryWithOption(J9JavaVM *vm, J9VMInitArgs *argsList, IDATA agentIn /** * Create an agent library from an option index. + * Three agent options are expected: VMOPT_AGENTLIB_COLON, VMOPT_AGENTPATH_COLON or MAPOPT_XRUNJDWP. * * @param[in] vm Java VM * @param[in] argsList a J9VMInitArgs - * @param[in] agentColon the agent option, VMOPT_AGENTLIB_COLON, VMOPT_AGENTPATH_COLON or MAPOPT_XRUNJDWP * @param[in] loadLibrary indicate if the agent library created to be loaded or not * @param[in] isXrunjdwp indicate if the agent option is MAPOPT_XRUNJDWP + * @param[in] isAgentPath indicate if the agent option is VMOPT_AGENTPATH_COLON * * @return TRUE if succeeded, otherwise FALSE */ static BOOLEAN -processAgentLibraryFromArgsList(J9JavaVM *vm, J9VMInitArgs *argsList, const char *agentColon, BOOLEAN loadLibrary, BOOLEAN isXrunjdwp) +processAgentLibraryFromArgsList(J9JavaVM *vm, J9VMInitArgs *argsList, BOOLEAN loadLibrary, BOOLEAN isXrunjdwp, BOOLEAN isAgentPath) { - IDATA agentIndex = FIND_AND_CONSUME_ARG_FORWARD(argsList, STARTSWITH_MATCH, agentColon, NULL); + IDATA agentIndex = 0; BOOLEAN result = TRUE; + const char *agentColon = NULL; + if (isXrunjdwp) { + agentColon = MAPOPT_XRUNJDWP; + } else if (isAgentPath) { + agentColon = VMOPT_AGENTPATH_COLON; + } else { + /* only three options are expected */ + agentColon = VMOPT_AGENTLIB_COLON; + } + agentIndex = FIND_AND_CONSUME_ARG_FORWARD(argsList, STARTSWITH_MATCH, agentColon, NULL); while (agentIndex >= 0) { J9JVMTIAgentLibrary *agentLibrary = NULL; BOOLEAN isJDWPagent = FALSE; - if (JNI_OK != createAgentLibraryWithOption(vm, argsList, agentIndex, &agentLibrary, isXrunjdwp, &isJDWPagent)) { + if (JNI_OK != createAgentLibraryWithOption(vm, argsList, agentIndex, &agentLibrary, isXrunjdwp, !isAgentPath, &isJDWPagent)) { result = FALSE; break; } @@ -294,9 +320,9 @@ criuRestoreInitializeLib(J9JavaVM *vm, J9JVMTIEnv *j9env) { J9VMInitArgs *criuRestoreArgsList = vm->checkpointState.restoreArgsList; - processAgentLibraryFromArgsList(vm, criuRestoreArgsList, VMOPT_AGENTLIB_COLON, FALSE, FALSE); - processAgentLibraryFromArgsList(vm, criuRestoreArgsList, VMOPT_AGENTPATH_COLON, FALSE, FALSE); - processAgentLibraryFromArgsList(vm, criuRestoreArgsList, MAPOPT_XRUNJDWP, FALSE, TRUE); + processAgentLibraryFromArgsList(vm, criuRestoreArgsList, FALSE, FALSE, FALSE); + processAgentLibraryFromArgsList(vm, criuRestoreArgsList, FALSE, FALSE, TRUE); + processAgentLibraryFromArgsList(vm, criuRestoreArgsList, FALSE, TRUE, FALSE); if (J9_ARE_NO_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_IS_JDWP_ENABLED)) { J9JVMTIData * jvmtiData = vm->jvmtiData; @@ -311,9 +337,9 @@ criuRestoreStartAgent(J9JavaVM *vm) { J9VMInitArgs *criuRestoreArgsList = vm->checkpointState.restoreArgsList; - processAgentLibraryFromArgsList(vm, criuRestoreArgsList, VMOPT_AGENTLIB_COLON, TRUE, FALSE); - processAgentLibraryFromArgsList(vm, criuRestoreArgsList, VMOPT_AGENTPATH_COLON, TRUE, FALSE); - processAgentLibraryFromArgsList(vm, criuRestoreArgsList, MAPOPT_XRUNJDWP, TRUE, TRUE); + processAgentLibraryFromArgsList(vm, criuRestoreArgsList, TRUE, FALSE, FALSE); + processAgentLibraryFromArgsList(vm, criuRestoreArgsList, TRUE, FALSE, TRUE); + processAgentLibraryFromArgsList(vm, criuRestoreArgsList, TRUE, TRUE, FALSE); } #endif /* defined(J9VM_OPT_CRIU_SUPPORT) */ @@ -327,10 +353,10 @@ IDATA J9VMDllMain(J9JavaVM *vm, IDATA stage, void *reserved) if (JNI_OK != initializeJVMTI(vm)) { goto _error; } - if (FALSE == processAgentLibraryFromArgsList(vm, vm->vmArgsArray, VMOPT_AGENTLIB_COLON, FALSE, FALSE)) { + if (FALSE == processAgentLibraryFromArgsList(vm, vm->vmArgsArray, FALSE, FALSE, FALSE)) { goto _error; } - if (FALSE == processAgentLibraryFromArgsList(vm, vm->vmArgsArray, VMOPT_AGENTPATH_COLON, FALSE, FALSE)) { + if (FALSE == processAgentLibraryFromArgsList(vm, vm->vmArgsArray, FALSE, FALSE, TRUE)) { goto _error; } /* -Xrun libraries that have an Agent_OnLoad are treated like -agentlib: */ @@ -381,7 +407,7 @@ IDATA J9VMDllMain(J9JavaVM *vm, IDATA stage, void *reserved) * Adding capabilities is required before checkpoint if JIT is enabled. * Following code can be removed when JIT allows capabilities to be added after restore. */ - if (vm->internalVMFunctions->isDebugOnRestoreEnabled(vm->mainThread)) { + if (vm->internalVMFunctions->isDebugOnRestoreEnabled_VM(vm)) { Trc_JVMTI_criuAddCapabilities_invoked(); /* ignore the failure, it won't cause a problem if JDWP is not enabled later */ criuAddCapabilities(vm, NULL != vm->jitConfig); diff --git a/runtime/oti/j9nonbuilder.h b/runtime/oti/j9nonbuilder.h index b8134c1afb2..0098b68b11f 100644 --- a/runtime/oti/j9nonbuilder.h +++ b/runtime/oti/j9nonbuilder.h @@ -5157,9 +5157,11 @@ typedef struct J9InternalVMFunctions { BOOLEAN (*isCRIUSupportEnabled)(struct J9VMThread *currentThread); BOOLEAN (*enableCRIUSecProvider)(struct J9VMThread *currentThread); BOOLEAN (*isCheckpointAllowed)(struct J9VMThread *currentThread); + BOOLEAN (*isCheckpointAllowed_VM)(struct J9JavaVM *vm); BOOLEAN (*isNonPortableRestoreMode)(struct J9VMThread *currentThread); BOOLEAN (*isJVMInPortableRestoreMode)(struct J9VMThread *currentThread); BOOLEAN (*isDebugOnRestoreEnabled)(struct J9VMThread *currentThread); + BOOLEAN (*isDebugOnRestoreEnabled_VM)(struct J9JavaVM *vm); void (*setRequiredGhostFileLimit)(struct J9VMThread *currentThread, U_32 ghostFileLimit); BOOLEAN (*runInternalJVMCheckpointHooks)(struct J9VMThread *currentThread, const char **nlsMsgFormat); BOOLEAN (*runInternalJVMRestoreHooks)(struct J9VMThread *currentThread, const char **nlsMsgFormat); diff --git a/runtime/oti/vm_api.h b/runtime/oti/vm_api.h index 0f4067dfa81..7a267f0ccae 100644 --- a/runtime/oti/vm_api.h +++ b/runtime/oti/vm_api.h @@ -563,6 +563,19 @@ enableCRIUSecProvider(J9VMThread *currentThread); BOOLEAN isCheckpointAllowed(J9VMThread *currentThread); +/** + * @brief Queries if checkpointing is permitted. Note, when + * -XX:+CRIURestoreNonPortableMode option is specified checkpointing + * will not be permitted after the JVM has been restored from a checkpoint + * (checkpoint once mode). + * Same as isCheckpointAllowed() except for the incoming parameter. + * + * @param currentThread vmthread token + * @return TRUE if permitted, FALSE otherwise + */ +BOOLEAN +isCheckpointAllowed_VM(J9JavaVM *vm); + /** * @brief Queries if non-portable restore mode (specified via * -XX:+CRIURestoreNonPortableMode) is enabled. If so, checkpointing will @@ -597,6 +610,20 @@ isJVMInPortableRestoreMode(J9VMThread *currentThread); BOOLEAN isDebugOnRestoreEnabled(J9VMThread *currentThread); +/** + * @brief Queries if debug on restore (specified via + * -XX:+DebugOnRestore) is supported. If so, the JVM + * will run in FSD mode pre-checkpoint and will transition out + * FSD mode on restore (unless debug is specified post restore). + * Same as isDebugOnRestoreEnabled() except for the incoming parameter. + * + * @param vm javaVM token + * + * @return TRUE if debug on restore is enabled, FALSE otherwise + */ +BOOLEAN +isDebugOnRestoreEnabled_VM(J9JavaVM *vm); + /** * @brief Sets the maximum size for the CRIU ghost files. * If the new limit is smaller or equal to the previous limit, diff --git a/runtime/vm/CRIUHelpers.cpp b/runtime/vm/CRIUHelpers.cpp index 4b4eaf3a125..16462855149 100644 --- a/runtime/vm/CRIUHelpers.cpp +++ b/runtime/vm/CRIUHelpers.cpp @@ -145,11 +145,17 @@ isCRIUSupportEnabled(J9VMThread *currentThread) BOOLEAN isCheckpointAllowed(J9VMThread *currentThread) +{ + return isCheckpointAllowed_VM(currentThread->javaVM); +} + +BOOLEAN +isCheckpointAllowed_VM(J9JavaVM *vm) { BOOLEAN result = FALSE; - if (isCRaCorCRIUSupportEnabled(currentThread)) { - result = J9_ARE_ALL_BITS_SET(currentThread->javaVM->checkpointState.flags, J9VM_CRIU_IS_CHECKPOINT_ALLOWED); + if (isCRaCorCRIUSupportEnabled_VM(vm)) { + result = J9_ARE_ALL_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_IS_CHECKPOINT_ALLOWED); } return result; @@ -182,9 +188,15 @@ isJVMInPortableRestoreMode(J9VMThread *currentThread) BOOLEAN isDebugOnRestoreEnabled(J9VMThread *currentThread) { - return J9_ARE_NO_BITS_SET(currentThread->javaVM->checkpointState.flags, J9VM_CRIU_IS_JDWP_ENABLED) - && J9_ARE_ALL_BITS_SET(currentThread->javaVM->checkpointState.flags, J9VM_CRIU_SUPPORT_DEBUG_ON_RESTORE) - && isCRaCorCRIUSupportEnabled(currentThread); + return isDebugOnRestoreEnabled_VM(currentThread->javaVM); +} + +BOOLEAN +isDebugOnRestoreEnabled_VM(J9JavaVM *vm) +{ + return J9_ARE_NO_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_IS_JDWP_ENABLED) + && J9_ARE_ALL_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_SUPPORT_DEBUG_ON_RESTORE) + && isCRaCorCRIUSupportEnabled_VM(vm); } void diff --git a/runtime/vm/intfunc.c b/runtime/vm/intfunc.c index a68b2b98c02..245b36a4be9 100644 --- a/runtime/vm/intfunc.c +++ b/runtime/vm/intfunc.c @@ -413,9 +413,11 @@ J9InternalVMFunctions J9InternalFunctions = { isCRIUSupportEnabled, enableCRIUSecProvider, isCheckpointAllowed, + isCheckpointAllowed_VM, isNonPortableRestoreMode, isJVMInPortableRestoreMode, isDebugOnRestoreEnabled, + isDebugOnRestoreEnabled_VM, setRequiredGhostFileLimit, runInternalJVMCheckpointHooks, runInternalJVMRestoreHooks, diff --git a/runtime/vm/jvminit.c b/runtime/vm/jvminit.c index 1cbf6310ba7..c464ed52ef5 100644 --- a/runtime/vm/jvminit.c +++ b/runtime/vm/jvminit.c @@ -2986,7 +2986,7 @@ VMInitStages(J9JavaVM *vm, IDATA stage, void* reserved) } #endif /* defined(J9VM_INTERP_ATOMIC_FREE_JNI_USES_FLUSH) */ #if defined(J9VM_OPT_CRIU_SUPPORT) - if (isDebugOnRestoreEnabled(vm->mainThread)) { + if (isDebugOnRestoreEnabled_VM(vm)) { Trc_VM_VMInitStages_isDebugOnRestoreEnabled(); /* enable jvmtiCapabilities.can_get_source_debug_extension */ vm->requiredDebugAttributes |= J9VM_DEBUG_ATTRIBUTE_SOURCE_DEBUG_EXTENSION; diff --git a/runtime/vm/lookuphelper.c b/runtime/vm/lookuphelper.c index 789e68bef8c..7823ec12981 100644 --- a/runtime/vm/lookuphelper.c +++ b/runtime/vm/lookuphelper.c @@ -35,7 +35,7 @@ mustReportEnterStepOrBreakpoint(J9JavaVM *vm) UDATA hookedOrReserved = 0; #if defined(J9VM_OPT_CRIU_SUPPORT) - if (isDebugOnRestoreEnabled(vm->mainThread)) { + if (isDebugOnRestoreEnabled_VM(vm)) { hookedOrReserved = J9_EVENT_IS_HOOKED_OR_RESERVED(vm->hookInterface, J9HOOK_VM_METHOD_ENTER) || J9_EVENT_IS_HOOKED_OR_RESERVED(vm->hookInterface, J9HOOK_VM_METHOD_RETURN) || J9_EVENT_IS_HOOKED_OR_RESERVED(vm->hookInterface, J9HOOK_VM_SINGLE_STEP) diff --git a/runtime/vm/vmhook.c b/runtime/vm/vmhook.c index 489053eafad..2c2be7cfdd4 100644 --- a/runtime/vm/vmhook.c +++ b/runtime/vm/vmhook.c @@ -144,7 +144,7 @@ hookAboutToBootstrapEvent(J9HookInterface **hook, UDATA eventNum, void *voidEven } #if defined(J9VM_OPT_CRIU_SUPPORT) - if (isDebugOnRestoreEnabled(vmThread)) { + if (isDebugOnRestoreEnabled_VM(vm)) { debugModeRequested = J9_EVENT_IS_HOOKED_OR_RESERVED(vm->hookInterface, J9HOOK_VM_METHOD_ENTER) || J9_EVENT_IS_HOOKED_OR_RESERVED(vm->hookInterface, J9HOOK_VM_METHOD_RETURN) || J9_EVENT_IS_HOOKED_OR_RESERVED(vm->hookInterface, J9HOOK_VM_FRAME_POP)