From 5e2a8849c75693f2d4a78e3474ba7d711aeaef46 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 18 Jun 2019 12:35:20 -0700 Subject: [PATCH 1/3] LockAgent: Add ability to generate a native crash Use the crasher to create a tombstone with the violation as the abort message. Test: m Test: manual Change-Id: I8c5041a4fbffedb11b6b9c564ab1e214551896bf --- tools/lock_agent/Android.bp | 4 ++ tools/lock_agent/agent.cpp | 59 ++++++++++++++++++- .../com/android/lock_checker/LockHook.java | 11 ++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/tools/lock_agent/Android.bp b/tools/lock_agent/Android.bp index 2125ad682bb65..408946b288369 100644 --- a/tools/lock_agent/Android.bp +++ b/tools/lock_agent/Android.bp @@ -15,6 +15,8 @@ cc_library { include_dirs: [ // NDK headers aren't available in platform NDK builds. "libnativehelper/include_jni", + // Use ScopedUtfChars. + "libnativehelper/header_only_include", ], header_libs: [ "libopenjdkjvmti_headers", @@ -33,6 +35,8 @@ cc_binary_host { include_dirs: [ // NDK headers aren't available in platform NDK builds. "libnativehelper/include_jni", + // Use ScopedUtfChars. + "libnativehelper/header_only_include", ], header_libs: [ "libopenjdkjvmti_headers", diff --git a/tools/lock_agent/agent.cpp b/tools/lock_agent/agent.cpp index 59bfa2bf849b9..5b1d52e809aa1 100644 --- a/tools/lock_agent/agent.cpp +++ b/tools/lock_agent/agent.cpp @@ -19,6 +19,8 @@ #include #include +#include + #include #include @@ -26,10 +28,14 @@ #include #include #include +#include #include #include #include +#include + +#include // We need dladdr. #if !defined(__APPLE__) && !defined(_WIN32) @@ -61,6 +67,7 @@ namespace { JavaVM* gJavaVM = nullptr; +bool gForkCrash = false; // Converts a class name to a type descriptor // (ex. "java.lang.String" to "Ljava/lang/String;") @@ -372,7 +379,7 @@ void prepareHook(jvmtiEnv* env) { } } -jint attach(JavaVM* vm, char* options ATTRIBUTE_UNUSED, void* reserved ATTRIBUTE_UNUSED) { +jint attach(JavaVM* vm, char* options, void* reserved ATTRIBUTE_UNUSED) { gJavaVM = vm; jvmtiEnv* env; @@ -383,9 +390,59 @@ jint attach(JavaVM* vm, char* options ATTRIBUTE_UNUSED, void* reserved ATTRIBUTE prepareHook(env); + std::vector config = android::base::Split(options, ","); + for (const std::string& c : config) { + if (c == "native_crash") { + gForkCrash = true; + } + } + return JVMTI_ERROR_NONE; } +extern "C" JNIEXPORT +jboolean JNICALL Java_com_android_lock_1checker_LockHook_getNativeHandlingConfig(JNIEnv*, jclass) { + return gForkCrash ? JNI_TRUE : JNI_FALSE; +} + +extern "C" JNIEXPORT void JNICALL Java_com_android_lock_1checker_LockHook_nWtf(JNIEnv* env, jclass, + jstring msg) { + if (!gForkCrash || msg == nullptr) { + return; + } + + // Create a native crash with the given message. Decouple from the current crash to create a + // tombstone but continue on. + // + // TODO: Once there are not so many reports, consider making this fatal for the calling process. + ScopedUtfChars utf(env, msg); + if (utf.c_str() == nullptr) { + return; + } + const char* args[] = { + "/system/bin/lockagent_crasher", + utf.c_str(), + nullptr + }; + pid_t pid = fork(); + if (pid < 0) { + return; + } + if (pid == 0) { + // Double fork so we return quickly. Leave init to deal with the zombie. + pid_t pid2 = fork(); + if (pid2 == 0) { + execv(args[0], const_cast(args)); + _exit(1); + __builtin_unreachable(); + } + _exit(0); + __builtin_unreachable(); + } + int status; + waitpid(pid, &status, 0); // Ignore any results. +} + extern "C" JNIEXPORT jint JNICALL Agent_OnAttach(JavaVM* vm, char* options, void* reserved) { return attach(vm, options, reserved); } diff --git a/tools/lock_agent/java/com/android/lock_checker/LockHook.java b/tools/lock_agent/java/com/android/lock_checker/LockHook.java index efab1d2172252..86c97d340c5be 100644 --- a/tools/lock_agent/java/com/android/lock_checker/LockHook.java +++ b/tools/lock_agent/java/com/android/lock_checker/LockHook.java @@ -72,14 +72,20 @@ public class LockHook { private static final LockChecker[] sCheckers; + private static boolean sNativeHandling = false; + static { sHandlerThread = new HandlerThread("LockHook:wtf", Process.THREAD_PRIORITY_BACKGROUND); sHandlerThread.start(); sHandler = new WtfHandler(sHandlerThread.getLooper()); sCheckers = new LockChecker[] { new OnThreadLockChecker() }; + + sNativeHandling = getNativeHandlingConfig(); } + private static native boolean getNativeHandlingConfig(); + static boolean shouldDumpStacktrace(StacktraceHasher hasher, Map dumpedSet, T val, AnnotatedStackTraceElement[] st, int from, int to) { final String stacktraceHash = hasher.stacktraceHash(st, from, to); @@ -175,8 +181,13 @@ public class LockHook { private static void handleViolation(Violation v) { String msg = v.toString(); Log.wtf(TAG, msg); + if (sNativeHandling) { + nWtf(msg); // Also send to native. + } } + private static native void nWtf(String msg); + /** * Generates a hash for a given stacktrace of a {@link Throwable}. */ From 8695b40720dcda3ded314c7ec962578883e83f78 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 18 Jun 2019 12:36:46 -0700 Subject: [PATCH 2/3] LockAgent: Add option to synthesize Java crash logging Add the ability to dump a "crash" to logcat. Test: m Test: manual Change-Id: I0692a91df995883e526a718fe95f0d3568ac9328 --- .../com/android/internal/os/RuntimeInit.java | 25 +++++++++-------- tools/lock_agent/agent.cpp | 8 ++++++ .../com/android/lock_checker/LockHook.java | 12 +++++++++ .../lock_checker/OnThreadLockChecker.java | 27 +++++++++++++++++-- 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/core/java/com/android/internal/os/RuntimeInit.java b/core/java/com/android/internal/os/RuntimeInit.java index eac150dbd320d..1de2e7272f4d1 100644 --- a/core/java/com/android/internal/os/RuntimeInit.java +++ b/core/java/com/android/internal/os/RuntimeInit.java @@ -64,6 +64,19 @@ public class RuntimeInit { return Log.printlns(Log.LOG_ID_CRASH, Log.ERROR, tag, msg, tr); } + public static void logUncaught(String threadName, String processName, int pid, Throwable e) { + StringBuilder message = new StringBuilder(); + // The "FATAL EXCEPTION" string is still used on Android even though + // apps can set a custom UncaughtExceptionHandler that renders uncaught + // exceptions non-fatal. + message.append("FATAL EXCEPTION: ").append(threadName).append("\n"); + if (processName != null) { + message.append("Process: ").append(processName).append(", "); + } + message.append("PID: ").append(pid); + Clog_e(TAG, message.toString(), e); + } + /** * Logs a message when a thread encounters an uncaught exception. By * default, {@link KillApplicationHandler} will terminate this process later, @@ -85,17 +98,7 @@ public class RuntimeInit { if (mApplicationObject == null && (Process.SYSTEM_UID == Process.myUid())) { Clog_e(TAG, "*** FATAL EXCEPTION IN SYSTEM PROCESS: " + t.getName(), e); } else { - StringBuilder message = new StringBuilder(); - // The "FATAL EXCEPTION" string is still used on Android even though - // apps can set a custom UncaughtExceptionHandler that renders uncaught - // exceptions non-fatal. - message.append("FATAL EXCEPTION: ").append(t.getName()).append("\n"); - final String processName = ActivityThread.currentProcessName(); - if (processName != null) { - message.append("Process: ").append(processName).append(", "); - } - message.append("PID: ").append(Process.myPid()); - Clog_e(TAG, message.toString(), e); + logUncaught(t.getName(), ActivityThread.currentProcessName(), Process.myPid(), e); } } } diff --git a/tools/lock_agent/agent.cpp b/tools/lock_agent/agent.cpp index 5b1d52e809aa1..c639427e645e5 100644 --- a/tools/lock_agent/agent.cpp +++ b/tools/lock_agent/agent.cpp @@ -68,6 +68,7 @@ namespace { JavaVM* gJavaVM = nullptr; bool gForkCrash = false; +bool gJavaCrash = false; // Converts a class name to a type descriptor // (ex. "java.lang.String" to "Ljava/lang/String;") @@ -394,6 +395,8 @@ jint attach(JavaVM* vm, char* options, void* reserved ATTRIBUTE_UNUSED) { for (const std::string& c : config) { if (c == "native_crash") { gForkCrash = true; + } else if (c == "java_crash") { + gJavaCrash = true; } } @@ -405,6 +408,11 @@ jboolean JNICALL Java_com_android_lock_1checker_LockHook_getNativeHandlingConfig return gForkCrash ? JNI_TRUE : JNI_FALSE; } +extern "C" JNIEXPORT jboolean JNICALL +Java_com_android_lock_1checker_LockHook_getSimulateCrashConfig(JNIEnv*, jclass) { + return gJavaCrash ? JNI_TRUE : JNI_FALSE; +} + extern "C" JNIEXPORT void JNICALL Java_com_android_lock_1checker_LockHook_nWtf(JNIEnv* env, jclass, jstring msg) { if (!gForkCrash || msg == nullptr) { diff --git a/tools/lock_agent/java/com/android/lock_checker/LockHook.java b/tools/lock_agent/java/com/android/lock_checker/LockHook.java index 86c97d340c5be..35c75cbbae7c3 100644 --- a/tools/lock_agent/java/com/android/lock_checker/LockHook.java +++ b/tools/lock_agent/java/com/android/lock_checker/LockHook.java @@ -16,6 +16,7 @@ package com.android.lock_checker; +import android.app.ActivityThread; import android.os.Handler; import android.os.HandlerThread; import android.os.Looper; @@ -24,6 +25,7 @@ import android.os.Process; import android.util.Log; import android.util.LogWriter; +import com.android.internal.os.RuntimeInit; import com.android.internal.os.SomeArgs; import com.android.internal.util.StatLogger; @@ -73,6 +75,7 @@ public class LockHook { private static final LockChecker[] sCheckers; private static boolean sNativeHandling = false; + private static boolean sSimulateCrash = false; static { sHandlerThread = new HandlerThread("LockHook:wtf", Process.THREAD_PRIORITY_BACKGROUND); @@ -82,9 +85,11 @@ public class LockHook { sCheckers = new LockChecker[] { new OnThreadLockChecker() }; sNativeHandling = getNativeHandlingConfig(); + sSimulateCrash = getSimulateCrashConfig(); } private static native boolean getNativeHandlingConfig(); + private static native boolean getSimulateCrashConfig(); static boolean shouldDumpStacktrace(StacktraceHasher hasher, Map dumpedSet, T val, AnnotatedStackTraceElement[] st, int from, int to) { @@ -184,6 +189,12 @@ public class LockHook { if (sNativeHandling) { nWtf(msg); // Also send to native. } + if (sSimulateCrash) { + RuntimeInit.logUncaught("LockAgent", + ActivityThread.isSystem() ? "system_server" + : ActivityThread.currentProcessName(), + Process.myPid(), v.getException()); + } } private static native void nWtf(String msg); @@ -308,5 +319,6 @@ public class LockHook { } interface Violation { + Throwable getException(); } } diff --git a/tools/lock_agent/java/com/android/lock_checker/OnThreadLockChecker.java b/tools/lock_agent/java/com/android/lock_checker/OnThreadLockChecker.java index e4e721156b2b0..e74ccf9d50698 100644 --- a/tools/lock_agent/java/com/android/lock_checker/OnThreadLockChecker.java +++ b/tools/lock_agent/java/com/android/lock_checker/OnThreadLockChecker.java @@ -228,6 +228,8 @@ class OnThreadLockChecker implements LockHook.LockChecker { AnnotatedStackTraceElement[] mStack; OrderData mOppositeData; + private static final int STACK_OFFSET = 4; + Violation(Thread self, Object alreadyHeld, Object lock, AnnotatedStackTraceElement[] stack, OrderData oppositeData) { this.mSelfTid = (int) self.getId(); @@ -284,6 +286,26 @@ class OnThreadLockChecker implements LockHook.LockChecker { lock.getClass().getName()); } + // Synthesize an exception. + public Throwable getException() { + RuntimeException inner = new RuntimeException("Previously locked"); + inner.setStackTrace(synthesizeStackTrace(mOppositeData.mStack)); + + RuntimeException outer = new RuntimeException(toString(), inner); + outer.setStackTrace(synthesizeStackTrace(mStack)); + + return outer; + } + + private StackTraceElement[] synthesizeStackTrace(AnnotatedStackTraceElement[] stack) { + + StackTraceElement[] out = new StackTraceElement[stack.length - STACK_OFFSET]; + for (int i = 0; i < out.length; i++) { + out[i] = stack[i + STACK_OFFSET].getStackTraceElement(); + } + return out; + } + public String toString() { StringBuilder sb = new StringBuilder(); sb.append("Lock inversion detected!\n"); @@ -294,7 +316,7 @@ class OnThreadLockChecker implements LockHook.LockChecker { sb.append(" on thread ").append(mOppositeData.mTid).append(" (") .append(mOppositeData.mThreadName).append(")"); sb.append(" at:\n"); - sb.append(getAnnotatedStackString(mOppositeData.mStack, 4, + sb.append(getAnnotatedStackString(mOppositeData.mStack, STACK_OFFSET, describeLocking(mAlreadyHeld, "will lock"), getTo(mOppositeData.mStack, mLock) + 1, " | ")); sb.append(" Locking "); @@ -303,7 +325,8 @@ class OnThreadLockChecker implements LockHook.LockChecker { sb.append(describeLock(mLock)); sb.append(" on thread ").append(mSelfTid).append(" (").append(mSelfName).append(")"); sb.append(" at:\n"); - sb.append(getAnnotatedStackString(mStack, 4, describeLocking(mLock, "will lock"), + sb.append(getAnnotatedStackString(mStack, STACK_OFFSET, + describeLocking(mLock, "will lock"), getTo(mStack, mAlreadyHeld) + 1, " | ")); return sb.toString(); From f52b970c1c27356e940ce164817f9a08998f89eb Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Thu, 20 Jun 2019 12:28:49 -0700 Subject: [PATCH 3/3] LockAgent: Add agent parameters to start_lock_agent script Add parsing of "--agent-options" parameter to configure optional agent features. Sample: adb shell setprop wrap.system_server "start_with_lockagent --agent-options native_crash,java_crash" Test: m Test: manual Change-Id: Ie70d0155096838782cb76f8111a42eba0c20e75d --- tools/lock_agent/start_with_lockagent.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/lock_agent/start_with_lockagent.sh b/tools/lock_agent/start_with_lockagent.sh index 953922230a11d..70ed5c5e1dd70 100755 --- a/tools/lock_agent/start_with_lockagent.sh +++ b/tools/lock_agent/start_with_lockagent.sh @@ -1,5 +1,13 @@ #!/system/bin/sh + +AGENT_OPTIONS= +if [[ "$1" == --agent-options ]] ; then + shift + AGENT_OPTIONS="=$1" + shift +fi + APP=$1 shift -$APP -Xplugin:libopenjdkjvmti.so -agentpath:liblockagent.so $@ +$APP -Xplugin:libopenjdkjvmti.so "-agentpath:liblockagent.so$AGENT_OPTIONS" $@