Merge "New ActivityManager<->debuggerd protocol for recoverable crashes." am: f62a0533d7 am: 23afbe95eb am: bdfdc46010

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/2485756

Change-Id: Ibd6bf23d65c53df19dcef01753807d6781c7797d
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 88 insertions, 62 deletions

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 1a9d97ec85d9..153faa70296a 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -8645,13 +8645,16 @@ public class ActivityManagerService extends IActivityManager.Stub
             }
         }
 
+        boolean recoverable = eventType.equals("native_recoverable_crash");
+
         EventLogTags.writeAmCrash(Binder.getCallingPid(),
                 UserHandle.getUserId(Binder.getCallingUid()), processName,
                 r == null ? -1 : r.info.flags,
                 crashInfo.exceptionClassName,
                 crashInfo.exceptionMessage,
                 crashInfo.throwFileName,
-                crashInfo.throwLineNumber);
+                crashInfo.throwLineNumber,
+                recoverable ? 1 : 0);
 
         int processClassEnum = processName.equals("system_server") ? ServerProtoEnums.SYSTEM_SERVER
                 : (r != null) ? r.getProcessClassEnum()
@@ -8719,7 +8722,13 @@ public class ActivityManagerService extends IActivityManager.Stub
                 eventType, r, processName, null, null, null, null, null, null, crashInfo,
                 new Float(loadingProgress), incrementalMetrics, null);
 
-        mAppErrors.crashApplication(r, crashInfo);
+        // For GWP-ASan recoverable crashes, don't make the app crash (the whole point of
+        // 'recoverable' is that the app doesn't crash). Normally, for nonrecoreable native crashes,
+        // debuggerd will terminate the process, but there's a backup where ActivityManager will
+        // also kill it. Avoid that.
+        if (!recoverable) {
+            mAppErrors.crashApplication(r, crashInfo);
+        }
     }
 
     public void handleApplicationStrictModeViolation(
diff --git a/services/core/java/com/android/server/am/EventLogTags.logtags b/services/core/java/com/android/server/am/EventLogTags.logtags
index 50841ae4488c..81b242155bac 100644
--- a/services/core/java/com/android/server/am/EventLogTags.logtags
+++ b/services/core/java/com/android/server/am/EventLogTags.logtags
@@ -53,7 +53,7 @@ option java_package com.android.server.am
 30037 am_process_start_timeout (User|1|5),(PID|1|5),(UID|1|5),(Process Name|3)
 
 # Unhandled exception
-30039 am_crash (User|1|5),(PID|1|5),(Process Name|3),(Flags|1|5),(Exception|3),(Message|3),(File|3),(Line|1|5)
+30039 am_crash (User|1|5),(PID|1|5),(Process Name|3),(Flags|1|5),(Exception|3),(Message|3),(File|3),(Line|1|5),(Recoverable|1|5)
 # Log.wtf() called
 30040 am_wtf (User|1|5),(PID|1|5),(Process Name|3),(Flags|1|5),(Tag|3),(Message|3)
 
diff --git a/services/core/java/com/android/server/am/NativeCrashListener.java b/services/core/java/com/android/server/am/NativeCrashListener.java
index 94eb07611037..cd119e7e3fbc 100644
--- a/services/core/java/com/android/server/am/NativeCrashListener.java
+++ b/services/core/java/com/android/server/am/NativeCrashListener.java
@@ -64,12 +64,15 @@ final class NativeCrashListener extends Thread {
     class NativeCrashReporter extends Thread {
         ProcessRecord mApp;
         int mSignal;
+        boolean mGwpAsanRecoverableCrash;
         String mCrashReport;
 
-        NativeCrashReporter(ProcessRecord app, int signal, String report) {
+        NativeCrashReporter(ProcessRecord app, int signal, boolean gwpAsanRecoverableCrash,
+                            String report) {
             super("NativeCrashReport");
             mApp = app;
             mSignal = signal;
+            mGwpAsanRecoverableCrash = gwpAsanRecoverableCrash;
             mCrashReport = report;
         }
 
@@ -85,7 +88,9 @@ final class NativeCrashListener extends Thread {
                 ci.stackTrace = mCrashReport;
 
                 if (DEBUG) Slog.v(TAG, "Calling handleApplicationCrash()");
-                mAm.handleApplicationCrashInner("native_crash", mApp, mApp.processName, ci);
+                mAm.handleApplicationCrashInner(
+                        mGwpAsanRecoverableCrash ? "native_recoverable_crash" : "native_crash",
+                        mApp, mApp.processName, ci);
                 if (DEBUG) Slog.v(TAG, "<-- handleApplicationCrash() returned");
             } catch (Exception e) {
                 Slog.e(TAG, "Unable to report native crash", e);
@@ -207,9 +212,14 @@ final class NativeCrashListener extends Thread {
             // permits crash_dump to connect to it. This allows us to trust the
             // received values.
 
-            // first, the pid and signal number
-            int headerBytes = readExactly(fd, buf, 0, 8);
-            if (headerBytes != 8) {
+            // Activity Manager protocol:
+            //  - 32-bit network-byte-order: pid
+            //  - 32-bit network-byte-order: signal number
+            //  - byte: gwpAsanRecoverableCrash
+            //  - bytes: raw text of the dump
+            //  - null terminator
+            int headerBytes = readExactly(fd, buf, 0, 9);
+            if (headerBytes != 9) {
                 // protocol failure; give up
                 Slog.e(TAG, "Unable to read from debuggerd");
                 return;
@@ -217,69 +227,76 @@ final class NativeCrashListener extends Thread {
 
             int pid = unpackInt(buf, 0);
             int signal = unpackInt(buf, 4);
+            boolean gwpAsanRecoverableCrash = buf[8] != 0;
             if (DEBUG) {
-                Slog.v(TAG, "Read pid=" + pid + " signal=" + signal);
+                Slog.v(TAG, "Read pid=" + pid + " signal=" + signal
+                        + " recoverable=" + gwpAsanRecoverableCrash);
+            }
+            if (pid < 0) {
+                Slog.e(TAG, "Bogus pid!");
+                return;
             }
 
             // now the text of the dump
-            if (pid > 0) {
-                final ProcessRecord pr;
-                synchronized (mAm.mPidsSelfLocked) {
-                    pr = mAm.mPidsSelfLocked.get(pid);
+            final ProcessRecord pr;
+            synchronized (mAm.mPidsSelfLocked) {
+                pr = mAm.mPidsSelfLocked.get(pid);
+            }
+            if (pr == null) {
+                Slog.w(TAG, "Couldn't find ProcessRecord for pid " + pid);
+                return;
+            }
+
+            // Don't attempt crash reporting for persistent apps
+            if (pr.isPersistent()) {
+                if (DEBUG) {
+                    Slog.v(TAG, "Skipping report for persistent app " + pr);
                 }
-                if (pr != null) {
-                    // Don't attempt crash reporting for persistent apps
-                    if (pr.isPersistent()) {
-                        if (DEBUG) {
-                            Slog.v(TAG, "Skipping report for persistent app " + pr);
-                        }
-                        return;
-                    }
+                return;
+            }
 
-                    int bytes;
-                    do {
-                        // get some data
-                        bytes = Os.read(fd, buf, 0, buf.length);
-                        if (bytes > 0) {
-                            if (MORE_DEBUG) {
-                                String s = new String(buf, 0, bytes, "UTF-8");
-                                Slog.v(TAG, "READ=" + bytes + "> " + s);
-                            }
-                            // did we just get the EOD null byte?
-                            if (buf[bytes-1] == 0) {
-                                os.write(buf, 0, bytes-1);  // exclude the EOD token
-                                break;
-                            }
-                            // no EOD, so collect it and read more
-                            os.write(buf, 0, bytes);
-                        }
-                    } while (bytes > 0);
-
-                    // Okay, we've got the report.
-                    if (DEBUG) Slog.v(TAG, "processing");
-
-                    // Mark the process record as being a native crash so that the
-                    // cleanup mechanism knows we're still submitting the report
-                    // even though the process will vanish as soon as we let
-                    // debuggerd proceed.
-                    synchronized (mAm) {
-                        synchronized (mAm.mProcLock) {
-                            pr.mErrorState.setCrashing(true);
-                            pr.mErrorState.setForceCrashReport(true);
-                        }
+            int bytes;
+            do {
+                // get some data
+                bytes = Os.read(fd, buf, 0, buf.length);
+                if (bytes > 0) {
+                    if (MORE_DEBUG) {
+                        String s = new String(buf, 0, bytes, "UTF-8");
+                        Slog.v(TAG, "READ=" + bytes + "> " + s);
+                    }
+                    // did we just get the EOD null byte?
+                    if (buf[bytes - 1] == 0) {
+                        os.write(buf, 0, bytes - 1); // exclude the EOD token
+                        break;
+                    }
+                    // no EOD, so collect it and read more
+                    os.write(buf, 0, bytes);
+                }
+            } while (bytes > 0);
+
+            // Okay, we've got the report.
+            if (DEBUG) Slog.v(TAG, "processing");
+
+            // Mark the process record as being a native crash so that the
+            // cleanup mechanism knows we're still submitting the report even
+            // though the process will vanish as soon as we let debuggerd
+            // proceed. This isn't relevant for recoverable crashes, as we don't
+            // show the user an "app crashed" dialogue because the app (by
+            // design) didn't crash.
+            if (!gwpAsanRecoverableCrash) {
+                synchronized (mAm) {
+                    synchronized (mAm.mProcLock) {
+                        pr.mErrorState.setCrashing(true);
+                        pr.mErrorState.setForceCrashReport(true);
                     }
-
-                    // Crash reporting is synchronous but we want to let debuggerd
-                    // go about it business right away, so we spin off the actual
-                    // reporting logic on a thread and let it take it's time.
-                    final String reportString = new String(os.toByteArray(), "UTF-8");
-                    (new NativeCrashReporter(pr, signal, reportString)).start();
-                } else {
-                    Slog.w(TAG, "Couldn't find ProcessRecord for pid " + pid);
                 }
-            } else {
-                Slog.e(TAG, "Bogus pid!");
             }
+
+            // Crash reporting is synchronous but we want to let debuggerd
+            // go about it business right away, so we spin off the actual
+            // reporting logic on a thread and let it take it's time.
+            final String reportString = new String(os.toByteArray(), "UTF-8");
+            (new NativeCrashReporter(pr, signal, gwpAsanRecoverableCrash, reportString)).start();
         } catch (Exception e) {
             Slog.e(TAG, "Exception dealing with report", e);
             // ugh, fail.