From fe700260881c8f59ee2f2dc2308aef3b0cc39734 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 14 Sep 2010 17:42:07 -0700 Subject: [PATCH] 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 --- vm/Init.c | 7 ++- vm/InlineNative.c | 152 +++++++++++++++++++++++++++++++++---------------- vm/InlineNative.h | 4 ++ vm/analysis/Optimize.c | 57 ++++--------------- 4 files changed, 125 insertions(+), 95 deletions(-) diff --git a/vm/Init.c b/vm/Init.c index 0c7f73650..fc6d90bc3 100644 --- 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 a3a9ac87d..9db97baed 100644 --- 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 a54d9bdc1..cb31f5184 100644 --- 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 f07addf01..5a1decab2 100644 --- 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); } } -- 2.11.0