6572160: 3/3 Instrumentation.getObjectSize triggers JVM crash in JPLISAssert in shutdown

Summary: Tolerate JVMTI_ERROR_WRONG_PHASE return codes so that JLI methods can be called to the end of VM's life.
Reviewed-by: ohair, sspitsyn

5 files changed, 78 insertions, 0 deletions

diff --git a/src/share/instrument/InvocationAdapter.c b/src/share/instrument/InvocationAdapter.c
index 9221ab58f9..a7747f12e0 100644
--- a/src/share/instrument/InvocationAdapter.c
+++ b/src/share/instrument/InvocationAdapter.c
@@ -626,6 +626,7 @@ appendClassPath( JPLISAgent* agent,
     jvmtiError jvmtierr;
 
     jvmtierr = (*jvmtienv)->AddToSystemClassLoaderSearch(jvmtienv, jarfile);
+    check_phase_ret_1(jvmtierr);
 
     if (jvmtierr == JVMTI_ERROR_NONE) {
         return 0;
@@ -634,6 +635,7 @@ appendClassPath( JPLISAgent* agent,
         jvmtiError err;
 
         err = (*jvmtienv)->GetPhase(jvmtienv, &phase);
+        /* can be called from any phase */
         jplis_assert(err == JVMTI_ERROR_NONE);
 
         if (phase == JVMTI_PHASE_LIVE) {
@@ -805,6 +807,8 @@ appendBootClassPath( JPLISAgent* agent,
 
         /* print warning if boot class path not updated */
         if (jvmtierr != JVMTI_ERROR_NONE) {
+            check_phase_blob_ret(jvmtierr, free(path));
+
             fprintf(stderr, "WARNING: %s not added to bootstrap class loader search: ", path);
             switch (jvmtierr) {
                 case JVMTI_ERROR_ILLEGAL_ARGUMENT :
diff --git a/src/share/instrument/JPLISAgent.c b/src/share/instrument/JPLISAgent.c
index d58305dc41..4b01c88484 100644
--- a/src/share/instrument/JPLISAgent.c
+++ b/src/share/instrument/JPLISAgent.c
@@ -179,6 +179,7 @@ getJPLISEnvironment(jvmtiEnv * jvmtienv) {
     jvmtierror = (*jvmtienv)->GetEnvironmentLocalStorage(
                                             jvmtienv,
                                             (void**)&environment);
+    /* can be called from any phase */
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
 
     if (jvmtierror == JVMTI_ERROR_NONE) {
@@ -230,6 +231,7 @@ createNewJPLISAgent(JavaVM * vm, JPLISAgent **agent_ptr) {
         /* don't leak envs */
         if ( initerror != JPLIS_INIT_ERROR_NONE ) {
             jvmtiError jvmtierror = (*jvmtienv)->DisposeEnvironment(jvmtienv);
+            /* can be called from any phase */
             jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
         }
     }
@@ -277,6 +279,7 @@ initializeJPLISAgent(   JPLISAgent *    agent,
     jvmtierror = (*jvmtienv)->SetEnvironmentLocalStorage(
                                             jvmtienv,
                                             &(agent->mNormalEnvironment));
+    /* can be called from any phase */
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
 
     /* check what capabilities are available */
@@ -284,11 +287,17 @@ initializeJPLISAgent(   JPLISAgent *    agent,
 
     /* check phase - if live phase then we don't need the VMInit event */
     jvmtierror = (*jvmtienv)->GetPhase(jvmtienv, &phase);
+    /* can be called from any phase */
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
     if (phase == JVMTI_PHASE_LIVE) {
         return JPLIS_INIT_ERROR_NONE;
     }
 
+    if (phase != JVMTI_PHASE_ONLOAD) {
+        /* called too early or called too late; either way bail out */
+        return JPLIS_INIT_ERROR_FAILURE;
+    }
+
     /* now turn on the VMInit event */
     if ( jvmtierror == JVMTI_ERROR_NONE ) {
         jvmtiEventCallbacks callbacks;
@@ -298,6 +307,7 @@ initializeJPLISAgent(   JPLISAgent *    agent,
         jvmtierror = (*jvmtienv)->SetEventCallbacks( jvmtienv,
                                                      &callbacks,
                                                      sizeof(callbacks));
+        check_phase_ret_blob(jvmtierror, JPLIS_INIT_ERROR_FAILURE);
         jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
     }
 
@@ -307,6 +317,7 @@ initializeJPLISAgent(   JPLISAgent *    agent,
                                                 JVMTI_ENABLE,
                                                 JVMTI_EVENT_VM_INIT,
                                                 NULL /* all threads */);
+        check_phase_ret_blob(jvmtierror, JPLIS_INIT_ERROR_FAILURE);
         jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
     }
 
@@ -622,6 +633,7 @@ setLivePhaseEventHandlers(  JPLISAgent * agent) {
     jvmtierror = (*jvmtienv)->SetEventCallbacks( jvmtienv,
                                                  &callbacks,
                                                  sizeof(callbacks));
+    check_phase_ret_false(jvmtierror);
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
 
 
@@ -632,6 +644,7 @@ setLivePhaseEventHandlers(  JPLISAgent * agent) {
                                                     JVMTI_DISABLE,
                                                     JVMTI_EVENT_VM_INIT,
                                                     NULL /* all threads */);
+        check_phase_ret_false(jvmtierror);
         jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
     }
 
@@ -642,6 +655,7 @@ setLivePhaseEventHandlers(  JPLISAgent * agent) {
                                                     JVMTI_ENABLE,
                                                     JVMTI_EVENT_CLASS_FILE_LOAD_HOOK,
                                                     NULL /* all threads */);
+        check_phase_ret_false(jvmtierror);
         jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
     }
 
@@ -660,6 +674,7 @@ checkCapabilities(JPLISAgent * agent) {
     memset(&potentialCapabilities, 0, sizeof(potentialCapabilities));
 
     jvmtierror = (*jvmtienv)->GetPotentialCapabilities(jvmtienv, &potentialCapabilities);
+    check_phase_ret(jvmtierror);
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
 
     if ( jvmtierror == JVMTI_ERROR_NONE ) {
@@ -681,9 +696,11 @@ enableNativeMethodPrefixCapability(jvmtiEnv * jvmtienv) {
     jvmtiError          jvmtierror;
 
         jvmtierror = (*jvmtienv)->GetCapabilities(jvmtienv, &desiredCapabilities);
+        /* can be called from any phase */
         jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
         desiredCapabilities.can_set_native_method_prefix = 1;
         jvmtierror = (*jvmtienv)->AddCapabilities(jvmtienv, &desiredCapabilities);
+        check_phase_ret(jvmtierror);
         jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
 }
 
@@ -715,9 +732,11 @@ addOriginalMethodOrderCapability(JPLISAgent * agent) {
     jvmtiError          jvmtierror;
 
     jvmtierror = (*jvmtienv)->GetCapabilities(jvmtienv, &desiredCapabilities);
+    /* can be called from any phase */
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
     desiredCapabilities.can_maintain_original_method_order = 1;
     jvmtierror = (*jvmtienv)->AddCapabilities(jvmtienv, &desiredCapabilities);
+    check_phase_ret(jvmtierror);
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
 }
 
@@ -732,9 +751,11 @@ addRedefineClassesCapability(JPLISAgent * agent) {
 
     if (agent->mRedefineAvailable && !agent->mRedefineAdded) {
         jvmtierror = (*jvmtienv)->GetCapabilities(jvmtienv, &desiredCapabilities);
+        /* can be called from any phase */
         jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
         desiredCapabilities.can_redefine_classes = 1;
         jvmtierror = (*jvmtienv)->AddCapabilities(jvmtienv, &desiredCapabilities);
+        check_phase_ret(jvmtierror);
 
         /*
          * With mixed premain/agentmain agents then it's possible that the
@@ -1026,6 +1047,7 @@ isModifiableClass(JNIEnv * jnienv, JPLISAgent * agent, jclass clazz) {
     jvmtierror = (*jvmtienv)->IsModifiableClass( jvmtienv,
                                                  clazz,
                                                  &is_modifiable);
+    check_phase_ret_false(jvmtierror);
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
 
     return is_modifiable;
@@ -1231,6 +1253,7 @@ redefineClasses(JNIEnv * jnienv, JPLISAgent * agent, jobjectArray classDefinitio
             if (!errorOccurred) {
                 jvmtiError  errorCode = JVMTI_ERROR_NONE;
                 errorCode = (*jvmtienv)->RedefineClasses(jvmtienv, numDefs, classDefs);
+                check_phase_blob_ret(errorCode, deallocate(jvmtienv, (void*)classDefs));
                 errorOccurred = (errorCode != JVMTI_ERROR_NONE);
                 if ( errorOccurred ) {
                     createAndThrowThrowableFromJVMTIErrorCode(jnienv, errorCode);
@@ -1264,6 +1287,7 @@ commonGetClassList( JNIEnv *            jnienv,
                         classLoader,
                         &classCount,
                         &classes);
+    check_phase_ret_blob(jvmtierror, localArray);
     errorOccurred = (jvmtierror != JVMTI_ERROR_NONE);
     jplis_assert(!errorOccurred);
 
@@ -1325,6 +1349,7 @@ getObjectSize(JNIEnv * jnienv, JPLISAgent * agent, jobject objectToSize) {
     jvmtiError  jvmtierror  = JVMTI_ERROR_NONE;
 
     jvmtierror = (*jvmtienv)->GetObjectSize(jvmtienv, objectToSize, &objectSize);
+    check_phase_ret_0(jvmtierror);
     jplis_assert(jvmtierror == JVMTI_ERROR_NONE);
     if ( jvmtierror != JVMTI_ERROR_NONE ) {
         createAndThrowThrowableFromJVMTIErrorCode(jnienv, jvmtierror);
@@ -1374,6 +1399,7 @@ appendToClassLoaderSearch(JNIEnv * jnienv, JPLISAgent * agent, jstring jarFile,
                 } else {
                     jvmtierror = (*jvmtienv)->AddToSystemClassLoaderSearch(jvmtienv, platformChars);
                 }
+                check_phase_ret(jvmtierror);
 
                 if ( jvmtierror != JVMTI_ERROR_NONE ) {
                     createAndThrowThrowableFromJVMTIErrorCode(jnienv, jvmtierror);
@@ -1464,6 +1490,7 @@ setNativeMethodPrefixes(JNIEnv * jnienv, JPLISAgent * agent, jobjectArray prefix
             }
 
             err = (*jvmtienv)->SetNativeMethodPrefixes(jvmtienv, inx, (char**)prefixes);
+            /* can be called from any phase */
             jplis_assert(err == JVMTI_ERROR_NONE);
 
             for (i = 0; i < inx; i++) {
diff --git a/src/share/instrument/JPLISAgent.h b/src/share/instrument/JPLISAgent.h
index 8bc1ace04b..444f4a8145 100644
--- a/src/share/instrument/JPLISAgent.h
+++ b/src/share/instrument/JPLISAgent.h
@@ -266,6 +266,48 @@ setNativeMethodPrefixes(JNIEnv * jnienv, JPLISAgent * agent, jobjectArray prefix
 
 #define jvmti(a) a->mNormalEnvironment.mJVMTIEnv
 
+/*
+ * A set of macros for insulating the JLI method callers from
+ * JVMTI_ERROR_WRONG_PHASE return codes.
+ */
+
+/* for a JLI method where "blob" is executed before simply returning */
+#define check_phase_blob_ret(ret, blob)      \
+    if ((ret) == JVMTI_ERROR_WRONG_PHASE) {  \
+        blob;                                \
+        return;                              \
+    }
+
+/* for a JLI method where simply returning is benign */
+#define check_phase_ret(ret)                 \
+    if ((ret) == JVMTI_ERROR_WRONG_PHASE) {  \
+        return;                              \
+    }
+
+/* for a JLI method where returning zero (0) is benign */
+#define check_phase_ret_0(ret)               \
+    if ((ret) == JVMTI_ERROR_WRONG_PHASE) {  \
+        return 0;                            \
+    }
+
+/* for a JLI method where returning one (1) is benign */
+#define check_phase_ret_1(ret)               \
+    if ((ret) == JVMTI_ERROR_WRONG_PHASE) {  \
+        return 1;                            \
+    }
+
+/* for a case where a specific "blob" must be returned */
+#define check_phase_ret_blob(ret, blob)      \
+    if ((ret) == JVMTI_ERROR_WRONG_PHASE) {  \
+        return (blob);                       \
+    }
+
+/* for a JLI method where returning false is benign */
+#define check_phase_ret_false(ret)           \
+    if ((ret) == JVMTI_ERROR_WRONG_PHASE) {  \
+        return (jboolean) 0;                 \
+    }
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif /* __cplusplus */
diff --git a/src/share/instrument/Reentrancy.c b/src/share/instrument/Reentrancy.c
index e5f9c62a73..3fcc4e7f1d 100644
--- a/src/share/instrument/Reentrancy.c
+++ b/src/share/instrument/Reentrancy.c
@@ -74,6 +74,7 @@ confirmingTLSSet(   jvmtiEnv *      jvmtienv,
                                     jvmtienv,
                                     thread,
                                     newValue);
+    check_phase_ret_blob(error, error);
 
 #if JPLISASSERT_ENABLEASSERTIONS
     assertTLSValue( jvmtienv,
@@ -96,6 +97,7 @@ assertTLSValue( jvmtiEnv *      jvmtienv,
                                 jvmtienv,
                                 thread,
                                 &test);
+    check_phase_ret(error);
     jplis_assert(error == JVMTI_ERROR_NONE);
     jplis_assert(test == expected);
 }
@@ -111,6 +113,7 @@ tryToAcquireReentrancyToken(    jvmtiEnv *  jvmtienv,
                                 jvmtienv,
                                 thread,
                                 &storedValue);
+    check_phase_ret_false(error);
     jplis_assert(error == JVMTI_ERROR_NONE);
     if ( error == JVMTI_ERROR_NONE ) {
         /* if this thread is already inside, just return false and short-circuit */
diff --git a/src/share/instrument/Utilities.c b/src/share/instrument/Utilities.c
index 120b30a862..2bcb25ec30 100644
--- a/src/share/instrument/Utilities.c
+++ b/src/share/instrument/Utilities.c
@@ -46,6 +46,7 @@ allocate(jvmtiEnv * jvmtienv, size_t bytecount) {
     error = (*jvmtienv)->Allocate(jvmtienv,
                                   bytecount,
                                   (unsigned char**) &resultBuffer);
+    /* may be called from any phase */
     jplis_assert(error == JVMTI_ERROR_NONE);
     if ( error != JVMTI_ERROR_NONE ) {
         resultBuffer = NULL;
@@ -66,6 +67,7 @@ deallocate(jvmtiEnv * jvmtienv, void * buffer) {
 
     error = (*jvmtienv)->Deallocate(jvmtienv,
                                     (unsigned char*)buffer);
+    /* may be called from any phase */
     jplis_assert_msg(error == JVMTI_ERROR_NONE, "Can't deallocate memory");
     return;
 }