Check at startup that we can resolve all the inline natives.

Also fix a bug where we'd dereference NULL if an inline native's class failed
to resolve.

Also merge this method lookup with Optimize.c's (superior) near-duplicate.

Change-Id: Ic7a95e1f7445b6c9964ddd5e2e1d14d70792a622

diff --git a/vm/Init.c b/vm/Init.c
index 0c7f736..fc6d90b 100644 (file)
--- a/vm/Init.c
+++ b/vm/Init.c
@@ -1296,6 +1296,9 @@ int dvmStartup(int argc, const char* const argv[], bool ignoreUnrecognized,
     if (!dvmDebuggerStartup())
         goto fail;
 
+    if (!dvmInlineNativeCheck())
+        goto fail;
+
     /*
      * Init for either zygote mode or non-zygote mode.  The key difference
      * is that we don't start any additional threads in Zygote mode.
@@ -1349,7 +1352,7 @@ static bool registerSystemNatives(JNIEnv* pEnv)
     self->status = THREAD_NATIVE;
 
     if (jniRegisterSystemMethods(pEnv) < 0) {
-        LOGW("jniRegisterSystemMethods failed\n");
+        LOGE("jniRegisterSystemMethods failed");
         return false;
     }
 
@@ -1553,6 +1556,8 @@ int dvmPrepForDexOpt(const char* bootClassPath, DexOptimizerMode dexOptMode,
         goto fail;
     if (!dvmClassStartup())
         goto fail;
+    if (!dvmInlineNativeCheck())
+        goto fail;
 
     /*
      * We leave gDvm.initializing set to "true" so that, if we're not

diff --git a/vm/InlineNative.c b/vm/InlineNative.c
index a3a9ac8..9db97ba 100644 (file)
--- a/vm/InlineNative.c
+++ b/vm/InlineNative.c
@@ -791,67 +791,123 @@ int dvmGetInlineOpsTableLength(void)
     return NELEM(gDvmInlineOpsTable);
 }
 
-/*
- * Make an inline call for the "debug" interpreter, used when the debugger
- * or profiler is active.
- */
-bool dvmPerformInlineOp4Dbg(u4 arg0, u4 arg1, u4 arg2, u4 arg3,
-    JValue* pResult, int opIndex)
+Method* dvmFindInlinableMethod(const char* classDescriptor,
+    const char* methodName, const char* methodSignature)
 {
-    Thread* self = dvmThreadSelf();
-    bool result;
+    /*
+     * Find the class.
+     */
+    ClassObject* clazz = dvmFindClassNoInit(classDescriptor, NULL);
+    if (clazz == NULL) {
+        LOGE("dvmFindInlinableMethod: can't find class '%s'",
+            classDescriptor);
+        dvmClearException(dvmThreadSelf());
+        return NULL;
+    }
 
-    assert(opIndex >= 0 && opIndex < NELEM(gDvmInlineOpsTable));
+    /*
+     * Method could be virtual or direct.  Try both.  Don't use
+     * the "hier" versions.
+     */
+    Method* method = dvmFindDirectMethodByDescriptor(clazz, methodName,
+        methodSignature);
+    if (method == NULL) {
+        method = dvmFindVirtualMethodByDescriptor(clazz, methodName,
+            methodSignature);
+    }
+    if (method == NULL) {
+        LOGE("dvmFindInlinableMethod: can't find method %s.%s %s",
+            clazz->descriptor, methodName, methodSignature);
+        return NULL;
+    }
 
     /*
-     * Populate the methods table on first use.  It's possible the class
-     * hasn't been resolved yet, so we need to do the full "calling the
-     * method for the first time" routine.  (It's probably okay to skip
-     * the access checks.)
-     *
-     * Currently assuming that we're only inlining stuff loaded by the
-     * bootstrap class loader.  This is a safe assumption for many reasons.
+     * Check that the method is appropriate for inlining.
      */
+    if (!dvmIsFinalClass(clazz) && !dvmIsFinalMethod(method)) {
+        LOGE("dvmFindInlinableMethod: can't inline non-final method %s.%s",
+            clazz->descriptor, method->name);
+        return NULL;
+    }
+    if (dvmIsSynchronizedMethod(method) ||
+            dvmIsDeclaredSynchronizedMethod(method)) {
+        LOGE("dvmFindInlinableMethod: can't inline synchronized method %s.%s",
+            clazz->descriptor, method->name);
+        return NULL;
+    }
+
+    return method;
+}
+
+/*
+ * Populate the methods table on first use.  It's possible the class
+ * hasn't been resolved yet, so we need to do the full "calling the
+ * method for the first time" routine.  (It's probably okay to skip
+ * the access checks.)
+ *
+ * Currently assuming that we're only inlining stuff loaded by the
+ * bootstrap class loader.  This is a safe assumption for many reasons.
+ */
+static Method* resolveInlineNative(int opIndex)
+{
+    assert(opIndex >= 0 && opIndex < NELEM(gDvmInlineOpsTable));
     Method* method = gDvm.inlinedMethods[opIndex];
+    if (method != NULL) {
+        return method;
+    }
+
+    method = dvmFindInlinableMethod(
+        gDvmInlineOpsTable[opIndex].classDescriptor,
+        gDvmInlineOpsTable[opIndex].methodName,
+        gDvmInlineOpsTable[opIndex].methodSignature);
+
     if (method == NULL) {
-        ClassObject* clazz;
+        /* We already reported the error. */
+        return NULL;
+    }
 
-        clazz = dvmFindClassNoInit(
-                gDvmInlineOpsTable[opIndex].classDescriptor, NULL);
-        if (clazz == NULL) {
-            LOGW("Warning: can't find class '%s'\n", clazz->descriptor);
-            goto skip_prof;
-        }
-        method = dvmFindDirectMethodByDescriptor(clazz,
-                    gDvmInlineOpsTable[opIndex].methodName,
-                    gDvmInlineOpsTable[opIndex].methodSignature);
-        if (method == NULL)
-            method = dvmFindVirtualMethodByDescriptor(clazz,
-                        gDvmInlineOpsTable[opIndex].methodName,
-                        gDvmInlineOpsTable[opIndex].methodSignature);
-        if (method == NULL) {
-            LOGW("Warning: can't find method %s.%s %s\n",
-                clazz->descriptor,
-                gDvmInlineOpsTable[opIndex].methodName,
-                gDvmInlineOpsTable[opIndex].methodSignature);
-            goto skip_prof;
-        }
+    gDvm.inlinedMethods[opIndex] = method;
+    IF_LOGV() {
+        char* desc = dexProtoCopyMethodDescriptor(&method->prototype);
+        LOGV("Registered for profile: %s.%s %s\n",
+            method->clazz->descriptor, method->name, desc);
+        free(desc);
+    }
 
-        gDvm.inlinedMethods[opIndex] = method;
-        IF_LOGV() {
-            char* desc = dexProtoCopyMethodDescriptor(&method->prototype);
-            LOGV("Registered for profile: %s.%s %s\n",
-                method->clazz->descriptor, method->name, desc);
-            free(desc);
-        }
+    return method;
+}
+
+/*
+ * Make an inline call for the "debug" interpreter, used when the debugger
+ * or profiler is active.
+ */
+bool dvmPerformInlineOp4Dbg(u4 arg0, u4 arg1, u4 arg2, u4 arg3,
+    JValue* pResult, int opIndex)
+{
+    Method* method = resolveInlineNative(opIndex);
+    if (method == NULL) {
+        return (*gDvmInlineOpsTable[opIndex].func)(arg0, arg1, arg2, arg3,
+            pResult);
     }
 
+    Thread* self = dvmThreadSelf();
     TRACE_METHOD_ENTER(self, method);
-    result = (*gDvmInlineOpsTable[opIndex].func)(arg0, arg1, arg2, arg3,
-                pResult);
+    bool result = (*gDvmInlineOpsTable[opIndex].func)(arg0, arg1, arg2, arg3,
+        pResult);
     TRACE_METHOD_EXIT(self, method);
     return result;
+}
 
-skip_prof:
-    return (*gDvmInlineOpsTable[opIndex].func)(arg0, arg1, arg2, arg3, pResult);
+/*
+ * Check that we can resolve every inline native.
+ */
+bool dvmInlineNativeCheck(void)
+{
+    int op;
+    for (op = 0; op < NELEM(gDvmInlineOpsTable); ++op) {
+        if (resolveInlineNative(op) == NULL) {
+            dvmAbort();
+        }
+    }
+    return true;
 }

diff --git a/vm/InlineNative.h b/vm/InlineNative.h
index a54d9bd..cb31f51 100644 (file)
--- a/vm/InlineNative.h
+++ b/vm/InlineNative.h
@@ -21,8 +21,12 @@
 
 /* startup/shutdown */
 bool dvmInlineNativeStartup(void);
+bool dvmInlineNativeCheck(void);
 void dvmInlineNativeShutdown(void);
 
+Method* dvmFindInlinableMethod(const char* classDescriptor,
+    const char* methodName, const char* methodSignature);
+
 /*
  * Basic 4-argument inline operation handler.
  */

diff --git a/vm/analysis/Optimize.c b/vm/analysis/Optimize.c
index f07addf..5a1deca 100644 (file)
--- a/vm/analysis/Optimize.c
+++ b/vm/analysis/Optimize.c
@@ -61,60 +61,25 @@ InlineSub* dvmCreateInlineSubsTable(void)
     const InlineOperation* ops = dvmGetInlineOpsTable();
     const int count = dvmGetInlineOpsTableLength();
     InlineSub* table;
-    Method* method;
-    ClassObject* clazz;
     int i, tableIndex;
 
     /*
      * Allocate for optimism: one slot per entry, plus an end-of-list marker.
      */
-    table = malloc(sizeof(InlineSub) * (count+1));
+    table = calloc(count + 1, sizeof(InlineSub));
 
     tableIndex = 0;
     for (i = 0; i < count; i++) {
-        clazz = dvmFindClassNoInit(ops[i].classDescriptor, NULL);
-        if (clazz == NULL) {
-            LOGV("DexOpt: can't inline for class '%s': not found\n",
-                ops[i].classDescriptor);
-            dvmClearOptException(dvmThreadSelf());
-        } else {
-            /*
-             * Method could be virtual or direct.  Try both.  Don't use
-             * the "hier" versions.
-             */
-            method = dvmFindDirectMethodByDescriptor(clazz, ops[i].methodName,
-                        ops[i].methodSignature);
-            if (method == NULL)
-                method = dvmFindVirtualMethodByDescriptor(clazz, ops[i].methodName,
-                        ops[i].methodSignature);
-            if (method == NULL) {
-                LOGW("DexOpt: can't inline %s.%s %s: method not found\n",
-                    ops[i].classDescriptor, ops[i].methodName,
-                    ops[i].methodSignature);
-            } else {
-                if (!dvmIsFinalClass(clazz) && !dvmIsFinalMethod(method)) {
-                    LOGW("DexOpt: WARNING: inline op on non-final class/method "
-                         "%s.%s\n",
-                        clazz->descriptor, method->name);
-                    /* fail? */
-                }
-                if (dvmIsSynchronizedMethod(method) ||
-                    dvmIsDeclaredSynchronizedMethod(method))
-                {
-                    LOGW("DexOpt: WARNING: inline op on synchronized method "
-                         "%s.%s\n",
-                        clazz->descriptor, method->name);
-                    /* fail? */
-                }
-
-                table[tableIndex].method = method;
-                table[tableIndex].inlineIdx = i;
-                tableIndex++;
-
-                LOGV("DexOpt: will inline %d: %s.%s %s\n", i,
-                    ops[i].classDescriptor, ops[i].methodName,
-                    ops[i].methodSignature);
-            }
+        Method* method = dvmFindInlinableMethod(ops[i].classDescriptor,
+            ops[i].methodName, ops[i].methodSignature);
+        if (method != NULL) {
+            table[tableIndex].method = method;
+            table[tableIndex].inlineIdx = i;
+            tableIndex++;
+
+            LOGV("DexOpt: will inline %d: %s.%s %s\n", i,
+                ops[i].classDescriptor, ops[i].methodName,
+                ops[i].methodSignature);
         }
     }