Permalink
Browse files

Fix native method logging to show local references rather than direct…

… pointers.

This is necessary (but not sufficient) for debugging third-party JNI bugs.
It's the second half of the logging story, but still doesn't address the
question of "how does the developer turn on the logging?".

This removes the variant JNI bridges at the cost of adding a couple of
booleans to struct Method. Performance is about the same, except synchronized
native methods are quite a bit faster after the change.

Before:
                             benchmark  ns linear runtime
                      _emptyJniMethod0 333 ==========
                      _emptyJniMethod6 367 ===========
                     _emptyJniMethod6L 921 ==============================
                _emptyJniStaticMethod0 259 ========
                _emptyJniStaticMethod6 287 =========
               _emptyJniStaticMethod6L 873 ============================
    _emptyJniStaticSynchronizedMethod0 404 =============
          _emptyJniSynchronizedMethod0 452 ==============

After:
                             benchmark  ns linear runtime
                      _emptyJniMethod0 344 ==========
                      _emptyJniMethod6 348 ==========
                     _emptyJniMethod6L 969 ==============================
                _emptyJniStaticMethod0 265 ========
                _emptyJniStaticMethod6 293 =========
               _emptyJniStaticMethod6L 968 =============================
    _emptyJniStaticSynchronizedMethod0 265 ========
          _emptyJniSynchronizedMethod0 323 ==========

A better optimization for the case where there are reference arguments
would be to keep a list of argument indexes in the struct Method, so we
could iterate directly over those arguments that need converting to
local references. That would also let us do something about the overhead
of repeatedly looking up which local reference table and cookie to use.

But now is not the time.

Change-Id: Ie32daca1b31be057a44f1ed4b5d28d1634380e1d
  • Loading branch information...
1 parent 9a4c89f commit a6e94ff55517438569d207e3ed552c8c127bcac9 @enh enh committed Jun 30, 2011
Showing with 204 additions and 373 deletions.
  1. +16 −46 vm/CheckJni.cpp
  2. +167 −186 vm/Jni.cpp
  3. +2 −19 vm/JniInternal.h
  4. +0 −111 vm/Native.cpp
  5. +0 −8 vm/Native.h
  6. +19 −3 vm/oo/Object.h
View
@@ -139,52 +139,15 @@ static inline bool callNeedsCheck(const u4* args, JValue* pResult,
/*
* Check a call into native code.
*/
-void dvmCheckCallJNIMethod_general(const u4* args, JValue* pResult,
+void dvmCheckCallJNIMethod(const u4* args, JValue* pResult,
const Method* method, Thread* self)
{
- dvmCallJNIMethod_general(args, pResult, method, self);
+ dvmCallJNIMethod(args, pResult, method, self);
if (callNeedsCheck(args, pResult, method, self)) {
checkCallResultCommon(args, pResult, method, self);
}
}
-/*
- * Check a synchronized call into native code.
- */
-void dvmCheckCallJNIMethod_synchronized(const u4* args, JValue* pResult,
- const Method* method, Thread* self)
-{
- dvmCallJNIMethod_synchronized(args, pResult, method, self);
- if (callNeedsCheck(args, pResult, method, self)) {
- checkCallResultCommon(args, pResult, method, self);
- }
-}
-
-/*
- * Check a virtual call with no reference arguments (other than "this").
- */
-void dvmCheckCallJNIMethod_virtualNoRef(const u4* args, JValue* pResult,
- const Method* method, Thread* self)
-{
- dvmCallJNIMethod_virtualNoRef(args, pResult, method, self);
- if (callNeedsCheck(args, pResult, method, self)) {
- checkCallResultCommon(args, pResult, method, self);
- }
-}
-
-/*
- * Check a static call with no reference arguments (other than "clazz").
- */
-void dvmCheckCallJNIMethod_staticNoRef(const u4* args, JValue* pResult,
- const Method* method, Thread* self)
-{
- dvmCallJNIMethod_staticNoRef(args, pResult, method, self);
- if (callNeedsCheck(args, pResult, method, self)) {
- checkCallResultCommon(args, pResult, method, self);
- }
-}
-
-
/*
* ===========================================================================
* JNI function helpers
@@ -508,19 +471,26 @@ class ScopedCheck {
void check(bool entry, const char* fmt0, ...) {
va_list ap;
- // If both "-Xcheck:jni" and "-Xjnitrace:" are enabled, we print trace messages
- // when a native method that matches the Xjnitrace argument calls a JNI function
- // such as NewByteArray.
bool shouldTrace = false;
const Method* method = NULL;
- if (gDvm.jniTrace && mHasMethod) {
+ if ((gDvm.jniTrace || gDvmJni.logThirdPartyJni) && mHasMethod) {
// We need to guard some of the invocation interface's calls: a bad caller might
// use DetachCurrentThread or GetEnv on a thread that's not yet attached.
if ((mFlags & kFlag_Invocation) == 0 || dvmThreadSelf() != NULL) {
method = dvmGetCurrentJNIMethod();
- if (strstr(method->clazz->descriptor, gDvm.jniTrace) != NULL) {
- shouldTrace = true;
- }
+ }
+ }
+ if (method != NULL) {
+ // If both "-Xcheck:jni" and "-Xjnitrace:" are enabled, we print trace messages
+ // when a native method that matches the Xjnitrace argument calls a JNI function
+ // such as NewByteArray.
+ if (gDvm.jniTrace && strstr(method->clazz->descriptor, gDvm.jniTrace) != NULL) {
+ shouldTrace = true;
+ }
+ // If -Xjniopts:logThirdPartyJni is on, we want to log any JNI function calls
+ // made by a third-party native method.
+ if (gDvmJni.logThirdPartyJni) {
+ shouldTrace |= method->shouldTrace;
}
}
Oops, something went wrong.

0 comments on commit a6e94ff

Please sign in to comment.