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

Commit 8ccb8be4 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Handle accidental null arguments to Trace APIs. 

We have @NonNull annotations, but not everyone is watching the warning
messages related to them.  Make the implementation a bit more robust
by swapping in the "(null)" literal when someone accidentally leaks
one through.

Bug: 249325721
Test: atest FrameworksCoreTests:TraceTest
Change-Id: Ic0f5a0236fbce88e086f3d68b2e5c6ed6b0aa33c
parent d82a8d2f

core/java/android/os/Trace.java

+0 −9
Original line number Diff line number Diff line
@@ -340,9 +340,6 @@ public final class Trace { 
     * @hide

     */

    public static void instant(long traceTag, String methodName) {

        if (methodName == null) {

            throw new IllegalArgumentException("methodName cannot be null");

        }

        if (isTagEnabled(traceTag)) {

            nativeInstant(traceTag, methodName);

        }
@@ -357,12 +354,6 @@ public final class Trace { 
     * @hide

     */

    public static void instantForTrack(long traceTag, String trackName, String methodName) {

        if (trackName == null) {

            throw new IllegalArgumentException("trackName cannot be null");

        }

        if (methodName == null) {

            throw new IllegalArgumentException("methodName cannot be null");

        }

        if (isTagEnabled(traceTag)) {

            nativeInstantForTrack(traceTag, trackName, methodName);

        }

core/jni/android_os_Trace.cpp

+17 −10
Original line number Diff line number Diff line
@@ -22,6 +22,8 @@ 


#include <array>



static constexpr const char* kNullReplacement = "(null)";



namespace android {



inline static void sanitizeString(char* str) {
@@ -36,6 +38,11 @@ inline static void sanitizeString(char* str) { 


template<typename F>

inline static void withString(JNIEnv* env, jstring jstr, F callback) {

    if (CC_UNLIKELY(jstr == nullptr)) {

        callback(kNullReplacement);

        return;

    }



    // We need to handle the worst case of 1 character -> 4 bytes

    // So make a buffer of size 4097 and let it hold a string with a maximum length

    // of 1024. The extra last byte for the null terminator.
@@ -52,14 +59,14 @@ inline static void withString(JNIEnv* env, jstring jstr, F callback) { 


static void android_os_Trace_nativeTraceCounter(JNIEnv* env, jclass,

        jlong tag, jstring nameStr, jlong value) {

    withString(env, nameStr, [tag, value](char* str) {

    withString(env, nameStr, [tag, value](const char* str) {

        atrace_int64(tag, str, value);

    });

}



static void android_os_Trace_nativeTraceBegin(JNIEnv* env, jclass,

        jlong tag, jstring nameStr) {

    withString(env, nameStr, [tag](char* str) {

    withString(env, nameStr, [tag](const char* str) {

        atrace_begin(tag, str);

    });

}
@@ -70,22 +77,22 @@ static void android_os_Trace_nativeTraceEnd(JNIEnv*, jclass, jlong tag) { 


static void android_os_Trace_nativeAsyncTraceBegin(JNIEnv* env, jclass,

        jlong tag, jstring nameStr, jint cookie) {

    withString(env, nameStr, [tag, cookie](char* str) {

    withString(env, nameStr, [tag, cookie](const char* str) {

        atrace_async_begin(tag, str, cookie);

    });

}



static void android_os_Trace_nativeAsyncTraceEnd(JNIEnv* env, jclass,

        jlong tag, jstring nameStr, jint cookie) {

    withString(env, nameStr, [tag, cookie](char* str) {

    withString(env, nameStr, [tag, cookie](const char* str) {

        atrace_async_end(tag, str, cookie);

    });

}



static void android_os_Trace_nativeAsyncTraceForTrackBegin(JNIEnv* env, jclass,

        jlong tag, jstring trackStr, jstring nameStr, jint cookie) {

    withString(env, trackStr, [env, tag, nameStr, cookie](char* track) {

        withString(env, nameStr, [tag, track, cookie](char* name) {

    withString(env, trackStr, [env, tag, nameStr, cookie](const char* track) {

        withString(env, nameStr, [tag, track, cookie](const char* name) {

            atrace_async_for_track_begin(tag, track, name, cookie);

        });

    });
@@ -93,7 +100,7 @@ static void android_os_Trace_nativeAsyncTraceForTrackBegin(JNIEnv* env, jclass, 


static void android_os_Trace_nativeAsyncTraceForTrackEnd(JNIEnv* env, jclass,

        jlong tag, jstring trackStr, jint cookie) {

    withString(env, trackStr, [tag, cookie](char* track) {

    withString(env, trackStr, [tag, cookie](const char* track) {

        atrace_async_for_track_end(tag, track, cookie);

    });

}
@@ -108,15 +115,15 @@ static void android_os_Trace_nativeSetTracingEnabled(JNIEnv*, jclass, jboolean e 


static void android_os_Trace_nativeInstant(JNIEnv* env, jclass,

        jlong tag, jstring nameStr) {

    withString(env, nameStr, [tag](char* str) {

    withString(env, nameStr, [tag](const char* str) {

        atrace_instant(tag, str);

    });

}



static void android_os_Trace_nativeInstantForTrack(JNIEnv* env, jclass,

        jlong tag, jstring trackStr, jstring nameStr) {

    withString(env, trackStr, [env, tag, nameStr](char* track) {

        withString(env, nameStr, [tag, track](char* name) {

    withString(env, trackStr, [env, tag, nameStr](const char* track) {

        withString(env, nameStr, [tag, track](const char* name) {

            atrace_instant_for_track(tag, track, name);

        });

    });

core/tests/coretests/src/android/os/TraceTest.java

+15 −1
Original line number Diff line number Diff line
@@ -34,6 +34,20 @@ public class TraceTest extends AndroidTestCase 
    private int fMethodCalls = 0;

    private int gMethodCalls = 0;



    public void testNullStrings() {

        Trace.traceCounter(Trace.TRACE_TAG_ACTIVITY_MANAGER, null, 42);

        Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, null);



        Trace.asyncTraceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, null, 42);

        Trace.asyncTraceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER, null, 42);



        Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, null, null, 42);

        Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER, null, 42);



        Trace.instant(Trace.TRACE_TAG_ACTIVITY_MANAGER, null);

        Trace.instantForTrack(Trace.TRACE_TAG_ACTIVITY_MANAGER, null, null);

    }



    @SmallTest

    public void testNativeTracingFromJava()

    {