Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0918c93a authored by Kurt Nelson's avatar Kurt Nelson
Browse files

Plumb real stack traces through StrictMode 

Currently StrictMode uses a string representation of the entire stack
trace throughout. Switching to passing Throwables will allow callback
consumers to traverse an array.

It does not regress the performance test added in ag/3083879.

Test: adb shell am instrument -w -e class android.os.StrictModeTest \
    com.android.perftests.core/android.support.test.runner.AndroidJUnitRunner

timeThreadViolation_mean=332071
timeThreadViolation_median=328184
timeThreadViolation_min=311253
timeThreadViolation_standardDeviation=16106

timeCrossBinderThreadViolationNoStrictMode_mean=1843599
timeCrossBinderThreadViolationNoStrictMode_median=1824457
timeCrossBinderThreadViolationNoStrictMode_min=1810186
timeCrossBinderThreadViolationNoStrictMode_standardDeviation=43539

timeCrossBinderThreadViolation_mean=2300256
timeCrossBinderThreadViolation_median=2148796
timeCrossBinderThreadViolation_min=1792660
timeCrossBinderThreadViolation_standardDeviation=472271

timeVmViolationNoStrictMode_mean=27794864
timeVmViolationNoStrictMode_median=26617027
timeVmViolationNoStrictMode_min=23994153
timeVmViolationNoStrictMode_standardDeviation=3384362

timeVmViolation_mean=32878535
timeVmViolation_median=34775241
timeVmViolation_min=28373537
timeVmViolation_standardDeviation=3462046

timeThreadViolationNoStrictMode_mean=373863
timeThreadViolationNoStrictMode_median=388998
timeThreadViolationNoStrictMode_min=333664
timeThreadViolationNoStrictMode_standardDeviation=33219

Bug: 62458734
Change-Id: I6b3924be91f19654c502e0ec2f44cc07d6e86e3f
Test: cts-tradefed run cts-dev --module CtsOsTestCases --test
android.os.cts.StrictModeTest
parent 1767ab6d

api/test-current.txt

+4 −3
Original line number Diff line number Diff line
@@ -32044,17 +32044,18 @@ package android.os { 
  public static final class StrictMode.ViolationInfo implements android.os.Parcelable {

    ctor public StrictMode.ViolationInfo();

    ctor public StrictMode.ViolationInfo(java.lang.Throwable, int);

    ctor public StrictMode.ViolationInfo(java.lang.String, java.lang.Throwable, int);

    ctor public deprecated StrictMode.ViolationInfo(java.lang.String, java.lang.Throwable, int);

    ctor public StrictMode.ViolationInfo(android.os.Parcel);

    ctor public StrictMode.ViolationInfo(android.os.Parcel, boolean);

    method public int describeContents();

    method public void dump(android.util.Printer, java.lang.String);

    method public java.lang.String getMessagePrefix();

    method public java.lang.String getStackTrace();

    method public java.lang.String getViolationDetails();

    method public void writeToParcel(android.os.Parcel, int);

    field public static final android.os.Parcelable.Creator<android.os.StrictMode.ViolationInfo> CREATOR;

    field public java.lang.String broadcastIntentAction;

    field public final android.app.ApplicationErrorReport.CrashInfo crashInfo;

    field public int durationMillis;

    field public final java.lang.String message;

    field public int numAnimationsRunning;

    field public long numInstances;

    field public final int policy;

core/java/android/os/StrictMode.java

+137 −64
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import android.annotation.Nullable; 
import android.annotation.TestApi;

import android.app.ActivityManager;

import android.app.ActivityThread;

import android.app.ApplicationErrorReport;

import android.app.IActivityManager;

import android.content.BroadcastReceiver;

import android.content.Context;
@@ -48,8 +47,10 @@ import java.io.PrintWriter; 
import java.io.StringWriter;

import java.net.InetAddress;

import java.net.UnknownHostException;

import java.util.ArrayDeque;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Deque;

import java.util.HashMap;

import java.util.concurrent.atomic.AtomicInteger;
@@ -352,8 +353,8 @@ public final class StrictMode { 
                } else {

                    msg = "StrictMode policy violation:";

                }

                if (info.crashInfo != null) {

                    Log.d(TAG, msg + " " + info.crashInfo.stackTrace);

                if (info.hasStackTrace()) {

                    Log.d(TAG, msg + " " + info.getStackTrace());

                } else {

                    Log.d(TAG, msg + " missing stack trace!");

                }
@@ -1247,28 +1248,6 @@ public final class StrictMode { 
        }

    }



    /** Like parsePolicyFromMessage(), but returns the violation. */

    private static int parseViolationFromMessage(String message) {

        if (message == null) {

            return 0;

        }

        int violationIndex = message.indexOf("violation=");

        if (violationIndex == -1) {

            return 0;

        }

        int numberStartIndex = violationIndex + "violation=".length();

        int numberEndIndex = message.indexOf(' ', numberStartIndex);

        if (numberEndIndex == -1) {

            numberEndIndex = message.length();

        }

        String violationString = message.substring(numberStartIndex, numberEndIndex);

        try {

            return Integer.parseInt(violationString);

        } catch (NumberFormatException e) {

            return 0;

        }

    }



    private static final ThreadLocal<ArrayList<ViolationInfo>> violationsBeingTimed =

            new ThreadLocal<ArrayList<ViolationInfo>>() {

                @Override
@@ -1516,7 +1495,7 @@ public final class StrictMode { 
        // to people who push/pop temporary policy in regions of code,

        // hence the policy being passed around.

        void handleViolation(final ViolationInfo info) {

            if (info == null || info.crashInfo == null || info.crashInfo.stackTrace == null) {

            if (info == null || !info.hasStackTrace()) {

                Log.wtf(TAG, "unexpected null stacktrace");

                return;

            }
@@ -1530,7 +1509,7 @@ public final class StrictMode { 
                    gatheredViolations.set(violations);

                }

                for (ViolationInfo previous : violations) {

                    if (info.crashInfo.stackTrace.equals(previous.crashInfo.stackTrace)) {

                    if (info.getStackTrace().equals(previous.getStackTrace())) {

                        // Duplicate. Don't log.

                        return;

                    }
@@ -1576,8 +1555,7 @@ public final class StrictMode { 
            }



            if (violationMaskSubset != 0) {

                int violationBit = parseViolationFromMessage(info.crashInfo.exceptionMessage);

                violationMaskSubset |= violationBit;

                violationMaskSubset |= info.getViolationBit();

                final int savedPolicyMask = getThreadPolicyMask();



                final boolean justDropBox = (info.policy & THREAD_PENALTY_MASK) == PENALTY_DROPBOX;
@@ -1622,8 +1600,7 @@ public final class StrictMode { 
    }



    private static void executeDeathPenalty(ViolationInfo info) {

        int violationBit = parseViolationFromMessage(info.crashInfo.exceptionMessage);

        throw new StrictModeViolation(info.policy, violationBit, null);

        throw new StrictModeViolation(info.policy, info.getViolationBit(), null);

    }



    /**
@@ -2027,21 +2004,14 @@ public final class StrictMode { 
     * read back all the encoded violations.

     */

    /* package */ static void readAndHandleBinderCallViolations(Parcel p) {

        // Our own stack trace to append

        StringWriter sw = new StringWriter();

        sw.append("# via Binder call with stack:\n");

        PrintWriter pw = new FastPrintWriter(sw, false, 256);

        new LogStackTrace().printStackTrace(pw);

        pw.flush();

        String ourStack = sw.toString();



        LogStackTrace localCallSite = new LogStackTrace();

        final int policyMask = getThreadPolicyMask();

        final boolean currentlyGathering = (policyMask & PENALTY_GATHER) != 0;



        final int size = p.readInt();

        for (int i = 0; i < size; i++) {

            final ViolationInfo info = new ViolationInfo(p, !currentlyGathering);

            info.crashInfo.appendStackTrace(ourStack);

            info.addLocalStack(localCallSite);

            BlockGuard.Policy policy = BlockGuard.getThreadPolicy();

            if (policy instanceof AndroidBlockGuardPolicy) {

                ((AndroidBlockGuardPolicy) policy).handleViolationWithTimingAttempt(info);
@@ -2366,10 +2336,18 @@ public final class StrictMode { 
     */

    @TestApi

    public static final class ViolationInfo implements Parcelable {

        public final String message;

        /** Some VM violations provide additional information outside the throwable. */

        @Nullable private final String mMessagePrefix;



        /** Stack and other stuff info. */

        @Nullable public final ApplicationErrorReport.CrashInfo crashInfo;

        /** Stack and violation details. */

        @Nullable private final Throwable mThrowable;



        private final Deque<Throwable> mBinderStack = new ArrayDeque<>();



        /** Memoized stack trace of full violation. */

        @Nullable private String mStackTrace;

        /** Memoized violation bit. */

        private int mViolationBit;



        /** The strict mode policy mask at the time of violation. */

        public final int policy;
@@ -2404,19 +2382,25 @@ public final class StrictMode { 


        /** Create an uninitialized instance of ViolationInfo */

        public ViolationInfo() {

            message = null;

            crashInfo = null;

            mMessagePrefix = null;

            mThrowable = null;

            policy = 0;

        }



        /** Create an instance of ViolationInfo. */

        public ViolationInfo(Throwable tr, int policy) {

            this(null, tr, policy);

        }



        /** Create an instance of ViolationInfo initialized from an exception. */

        public ViolationInfo(String message, Throwable tr, int policy) {

            this.message = message;

            crashInfo = new ApplicationErrorReport.CrashInfo(tr);

        /**

         * Create an instance of ViolationInfo initialized from an exception with a message prefix.

         *

         * @deprecated prefixes belong in the Throwable.

         */

        @Deprecated

        public ViolationInfo(String messagePrefix, Throwable tr, int policy) {

            this.mMessagePrefix = messagePrefix;

            this.mThrowable = tr;

            violationUptimeMillis = SystemClock.uptimeMillis();

            this.policy = policy;

            this.numAnimationsRunning = ValueAnimator.getCurrentAnimationsCount();
@@ -2446,11 +2430,101 @@ public final class StrictMode { 
            }

        }



        /** Equivalent output to {@link ApplicationErrorReport.CrashInfo#stackTrace}. */

        public String getStackTrace() {

            if (mThrowable != null && mStackTrace == null) {

                StringWriter sw = new StringWriter();

                PrintWriter pw = new FastPrintWriter(sw, false, 256);

                mThrowable.printStackTrace(pw);

                for (Throwable t : mBinderStack) {

                    pw.append("# via Binder call with stack:\n");

                    t.printStackTrace(pw);

                }

                pw.flush();

                pw.close();

                mStackTrace = sw.toString();

            }

            return mStackTrace;

        }



        /**

         * Optional message describing this violation.

         *

         * @hide

         */

        @TestApi

        public String getViolationDetails() {

            if (mThrowable != null) {

                return mThrowable.getMessage();

            } else {

                return "";

            }

        }



        /**

         * A handful of VM violations provide extra information that should be presented before

         * {@link #getViolationDetails()}.

         *

         * @hide

         */

        @TestApi

        public String getMessagePrefix() {

            return mMessagePrefix != null ? mMessagePrefix : "";

        }



        /** If this violation has a useful stack trace.

         *

         * @hide

         */

        public boolean hasStackTrace() {

            return mThrowable != null;

        }



        /**

         * Add a {@link Throwable} from the current process that caused the underlying violation.

         *

         * @hide

         */

        void addLocalStack(Throwable t) {

            mBinderStack.addFirst(t);

        }



        /**

         * Retrieve the type of StrictMode violation.

         *

         * @hide

         */

        int getViolationBit() {

            if (mThrowable == null || mThrowable.getMessage() == null) {

                return 0;

            }

            if (mViolationBit != 0) {

                return mViolationBit;

            }

            String message = mThrowable.getMessage();

            int violationIndex = message.indexOf("violation=");

            if (violationIndex == -1) {

                return 0;

            }

            int numberStartIndex = violationIndex + "violation=".length();

            int numberEndIndex = message.indexOf(' ', numberStartIndex);

            if (numberEndIndex == -1) {

                numberEndIndex = message.length();

            }

            String violationString = message.substring(numberStartIndex, numberEndIndex);

            try {

                mViolationBit = Integer.parseInt(violationString);

                return mViolationBit;

            } catch (NumberFormatException e) {

                return 0;

            }

        }



        @Override

        public int hashCode() {

            int result = 17;

            if (crashInfo != null) {

                result = 37 * result + crashInfo.stackTrace.hashCode();

            if (mThrowable != null) {

                result = 37 * result + mThrowable.hashCode();

            }

            if (numAnimationsRunning != 0) {

                result *= 37;
@@ -2478,11 +2552,11 @@ public final class StrictMode { 
         *     should be removed.

         */

        public ViolationInfo(Parcel in, boolean unsetGatheringBit) {

            message = in.readString();

            if (in.readInt() != 0) {

                crashInfo = new ApplicationErrorReport.CrashInfo(in);

            } else {

                crashInfo = null;

            mMessagePrefix = in.readString();

            mThrowable = (Throwable) in.readSerializable();

            int binderStackSize = in.readInt();

            for (int i = 0; i < binderStackSize; i++) {

                mBinderStack.add((Throwable) in.readSerializable());

            }

            int rawPolicy = in.readInt();

            if (unsetGatheringBit) {
@@ -2502,12 +2576,11 @@ public final class StrictMode { 
        /** Save a ViolationInfo instance to a parcel. */

        @Override

        public void writeToParcel(Parcel dest, int flags) {

            dest.writeString(message);

            if (crashInfo != null) {

                dest.writeInt(1);

                crashInfo.writeToParcel(dest, flags);

            } else {

                dest.writeInt(0);

            dest.writeString(mMessagePrefix);

            dest.writeSerializable(mThrowable);

            dest.writeInt(mBinderStack.size());

            for (Throwable t : mBinderStack) {

                dest.writeSerializable(t);

            }

            int start = dest.dataPosition();

            dest.writeInt(policy);
@@ -2542,8 +2615,8 @@ public final class StrictMode { 


        /** Dump a ViolationInfo instance to a Printer. */

        public void dump(Printer pw, String prefix) {

            if (crashInfo != null) {

                crashInfo.dump(pw, prefix);

            if (mThrowable != null) {

                pw.println(prefix + "stackTrace: " + getStackTrace());

            }

            pw.println(prefix + "policy: " + policy);

            if (durationMillis != -1) {

services/core/java/com/android/server/am/ActivityManagerService.java

+4 −4
Original line number Diff line number Diff line
@@ -14239,12 +14239,12 @@ public class ActivityManagerService extends IActivityManager.Stub 
                }

            }

            sb.append("\n");

            if (info.crashInfo != null && info.crashInfo.stackTrace != null) {

                sb.append(info.crashInfo.stackTrace);

            if (info.hasStackTrace()) {

                sb.append(info.getStackTrace());

                sb.append("\n");

            }

            if (info.message != null) {

                sb.append(info.message);

            if (info.getViolationDetails() != null) {

                sb.append(info.getViolationDetails());

                sb.append("\n");

            }