Use Throwable.getMessage() when logging exceptions.

The VM's internal exception logger was pulling the exception's detail
message out of a field in Throwable.  This gets the wrong answer when
the exception overrides getMessage().

The VM now invokes the appropriate getMessage() method and displays
that instead.  This is of particular interest in CheckJNI failure
messages.

Also, don't assert that switching from "running" to "running" is
a problem.  It's a bit weird, but there's nothing wrong with it,
and there's a fair chance that the updated exception log function
will do it.

Bug 2845581

Change-Id: Id8d77221129ae7b45473e700a79a335164049362

diff --git a/vm/Exception.c b/vm/Exception.c
index 0aeeae5..35151f2 100644 (file)
--- a/vm/Exception.c
+++ b/vm/Exception.c
@@ -152,15 +152,6 @@ bool dvmExceptionStartup(void)
         return false;
     }
 
-    /* and one for the message field, in case we want to show it */
-    gDvm.offJavaLangThrowable_message =
-        dvmFindFieldOffset(gDvm.classJavaLangThrowable,
-            "detailMessage", "Ljava/lang/String;");
-    if (gDvm.offJavaLangThrowable_message < 0) {
-        LOGE("Unable to find Throwable.detailMessage\n");
-        return false;
-    }
-
     /* and one for the cause field, just 'cause */
     gDvm.offJavaLangThrowable_cause =
         dvmFindFieldOffset(gDvm.classJavaLangThrowable,
@@ -1244,6 +1235,50 @@ void dvmLogRawStackTrace(const int* intVals, int stackDepth)
 }
 
 /*
+ * Get the message string.  We'd like to just grab the field out of
+ * Throwable, but the getMessage() function can be overridden by the
+ * sub-class.
+ *
+ * Returns the message string object, or NULL if it wasn't set or
+ * we encountered a failure trying to retrieve it.  The string will
+ * be added to the tracked references table.
+ */
+static StringObject* getExceptionMessage(Object* exception)
+{
+    Thread* self = dvmThreadSelf();
+    Method* getMessageMethod;
+    StringObject* messageStr = NULL;
+
+    assert(exception == self->exception);
+    self->exception = NULL;
+
+    getMessageMethod = dvmFindVirtualMethodHierByDescriptor(exception->clazz,
+            "getMessage", "()Ljava/lang/String;");
+    if (getMessageMethod != NULL) {
+        /* could be in NATIVE mode from CheckJNI, so switch state */
+        ThreadStatus oldStatus = dvmChangeStatus(self, THREAD_RUNNING);
+        JValue result;
+
+        dvmCallMethod(self, getMessageMethod, exception, &result);
+        messageStr = (StringObject*) result.l;
+        dvmAddTrackedAlloc((Object*) messageStr, self);
+
+        dvmChangeStatus(self, oldStatus);
+    } else {
+        LOGW("WARNING: could not find getMessage in %s\n",
+            exception->clazz->descriptor);
+    }
+
+    if (self->exception != NULL) {
+        LOGW("NOTE: exception thrown while retrieving exception message: %s\n",
+            self->exception->clazz->descriptor);
+    }
+
+    self->exception = exception;
+    return messageStr;
+}
+
+/*
  * Print the direct stack trace of the given exception to the log.
  */
 static void logStackTraceOf(Object* exception)
@@ -1255,10 +1290,12 @@ static void logStackTraceOf(Object* exception)
     char* className;
 
     className = dvmDescriptorToDot(exception->clazz->descriptor);
-    messageStr = (StringObject*) dvmGetFieldObject(exception,
-                    gDvm.offJavaLangThrowable_message);
+    messageStr = getExceptionMessage(exception);
     if (messageStr != NULL) {
         char* cp = dvmCreateCstrFromString(messageStr);
+        dvmReleaseTrackedAlloc((Object*) messageStr, dvmThreadSelf());
+        messageStr = NULL;
+
         LOGI("%s: %s\n", className, cp);
         free(cp);
     } else {

diff --git a/vm/Globals.h b/vm/Globals.h
index 3c98872..e7d0194 100644 (file)
--- a/vm/Globals.h
+++ b/vm/Globals.h
@@ -273,7 +273,6 @@ struct DvmGlobals {
 
     /* field offsets - Throwable */
     int         offJavaLangThrowable_stackState;
-    int         offJavaLangThrowable_message;
     int         offJavaLangThrowable_cause;
 
     /* field offsets - java.lang.reflect.* */

diff --git a/vm/Thread.c b/vm/Thread.c
index 50ecdc0..5d0f99d 100644 (file)
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -3105,6 +3105,8 @@ ThreadStatus dvmChangeStatus(Thread* self, ThreadStatus newStatus)
         self->threadId, self->status, newStatus);
 
     oldStatus = self->status;
+    if (oldStatus == newStatus)
+        return oldStatus;
 
     if (newStatus == THREAD_RUNNING) {
         /*
@@ -3157,7 +3159,6 @@ ThreadStatus dvmChangeStatus(Thread* self, ThreadStatus newStatus)
          * the thread is supposed to be suspended.  This is possibly faster
          * on SMP and slightly more correct, but less convenient.
          */
-        assert(oldStatus != THREAD_RUNNING);
         android_atomic_acquire_store(newStatus, &self->status);
         if (self->suspendCount != 0) {
             fullSuspendCheck(self);