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

Commit 807e40ec authored by Mark Salyzyn's avatar Mark Salyzyn
Browse files

liblog: logd: Add android_lookupEventTag_len()

Allows us to mitigate the impact of MAP_PRIVATE and copy on write by
calling android_lookupEventTag_len instead of android_lookupEventTag,
and delaying the copy on write impact to the later.  We return a
string length in a supplied location along with the string pointer
with android_lookupEventTag_len(const EventTagMap* map, size_t* len,
int tag).  The string is not guaranteed to be nul terminated.  Since
android_lookupEventTag() called even once can cause the memory
impact, we will mark it as deprecated, but we currently have no
timeframe for removal since this is a very old interface.

Add an API for __android_log_is_loggable_len() that accepts the non
null terminated content and fixup callers that would gain because the
length is known prior to the call either in the compiler or at
runtime.  Tackle transition to android_lookupEventTag_len() and
fixup callers.

On any application that performs logging (eg: com.android.phone)

/proc/<pid>/smaps before:

xxxxxxxxxx-xxxxxxxxxx rw-p 00000000 fd:00 463 /system/etc/event-log-tags
Size:                 20 kB
Rss:                  20 kB
Pss:                   1 kB
Shared_Clean:          0 kB
Shared_Dirty:         20 kB
Private_Clean:         0 kB
Private_Dirty:         0 kB
Referenced:            0 kB
Anonymous:            20 kB
AnonHugePages:         0 kB
Swap:                  0 kB
SwapPss:               0 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Locked:                0 kB
VmFlags: rd wr mr mw me ac

/proc/<pid>/smaps after:

xxxxxxxxxx-xxxxxxxxxx rw-p 00000000 fd:00 1773 /system/etc/event-log-tags
Size:                 20 kB
Rss:                  20 kB
Pss:                   1 kB
Shared_Clean:         20 kB  (was 0kB)
Shared_Dirty:          0 kB  (was 20kB)
Private_Clean:         0 kB
Private_Dirty:         0 kB
Referenced:           20 kB  (was 0kB)
Anonymous:             0 kB  (was 20kB)
AnonHugePages:         0 kB
Swap:                  0 kB
SwapPss:               0 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Locked:                0 kB
VmFlags: rd wr mr mw me ac

Added liblog-unit-tests --gtest_filter=liblog.event_log_tags to
check for Shared_Clean: to not be 0 and Anonymous: to be 0 for
all processes referencing event-log-tags.  Which can include multiple
references to /system/etc/event-log-tags and future possible refs to
/data/misc/logd/event-log-tags and /dev/event-log-tags.  We want
failure messages to help point to errant code using the deprecated
interface.

This change saves 1/4MB of memory or more on a typical system.

Test: gTest liblog-unit-tests
Bug: 31456426
Change-Id: I9e08e44d9092bd96fe704b5709242e7195281d33
parent 57513bd5
Loading
Loading
Loading
Loading
+5 −2
Original line number Diff line number Diff line
@@ -803,10 +803,12 @@ int android_log_destroy(android_log_context *ctx);
 */
#if LOG_NDEBUG /* Production */
#define android_testLog(prio, tag) \
    (__android_log_is_loggable(prio, tag, ANDROID_LOG_DEBUG) != 0)
    (__android_log_is_loggable_len(prio, tag, (tag && *tag) ? strlen(tag) : 0, \
                                   ANDROID_LOG_DEBUG) != 0)
#else
#define android_testLog(prio, tag) \
    (__android_log_is_loggable(prio, tag, ANDROID_LOG_VERBOSE) != 0)
    (__android_log_is_loggable_len(prio, tag, (tag && *tag) ? strlen(tag) : 0, \
                                   ANDROID_LOG_VERBOSE) != 0)
#endif

/*
@@ -816,6 +818,7 @@ int android_log_destroy(android_log_context *ctx);
 * any other value.
 */
int __android_log_is_loggable(int prio, const char *tag, int default_prio);
int __android_log_is_loggable_len(int prio, const char *tag, size_t len, int default_prio);

int __android_log_security(); /* Device Owner is present */

+8 −1
Original line number Diff line number Diff line
@@ -41,7 +41,14 @@ void android_closeEventTagMap(EventTagMap* map);
/*
 * Look up a tag by index.  Returns the tag string, or NULL if not found.
 */
const char* android_lookupEventTag(const EventTagMap* map, unsigned int tag);
const char* android_lookupEventTag(const EventTagMap* map, unsigned int tag)
    __attribute__((deprecated("use android_lookupEventTag_len() instead to minimize MAP_PRIVATE copy-on-write memory impact")));

/*
 * Look up a tag by index.  Returns the tag string & string length, or NULL if
 * not found.  Returned string is not guaranteed to be nul terminated.
 */
const char* android_lookupEventTag_len(const EventTagMap* map, size_t* len, unsigned int tag);

#ifdef __cplusplus
}
+1 −0
Original line number Diff line number Diff line
@@ -59,6 +59,7 @@ typedef struct AndroidLogEntry_t {
    int32_t pid;
    int32_t tid;
    const char * tag;
    size_t tagLen;
    size_t messageLen;
    const char * message;
} AndroidLogEntry;
+36 −16
Original line number Diff line number Diff line
@@ -35,7 +35,8 @@
 */
typedef struct EventTag {
    uint32_t tagIndex;
    const char* tagStr;
    char*    tagStr;
    size_t   tagLen;
} EventTag;

/*
@@ -139,7 +140,8 @@ LIBLOG_ABI_PUBLIC void android_closeEventTagMap(EventTagMap* map)
 *
 * The entries are sorted by tag number, so we can do a binary search.
 */
LIBLOG_ABI_PUBLIC const char* android_lookupEventTag(const EventTagMap* map,
LIBLOG_ABI_PUBLIC const char* android_lookupEventTag_len(const EventTagMap* map,
                                                         size_t *len,
                                                         unsigned int tag)
{
    int lo = 0;
@@ -157,14 +159,36 @@ LIBLOG_ABI_PUBLIC const char* android_lookupEventTag(const EventTagMap* map,
            hi = mid - 1;
        } else {
            /* found */
            if (len) *len = map->tagArray[mid].tagLen;
            /*
             * b/31456426 to check if gTest can detect copy-on-write issue
             * add the following line to break us:
             *     map->tagArray[mid].tagStr[map->tagArray[mid].tagLen] = '\0';
             * or explicitly use deprecated android_lookupEventTag().
             */
            return map->tagArray[mid].tagStr;
        }
    }

    errno = ENOENT;
    if (len) *len = 0;
    return NULL;
}

LIBLOG_ABI_PUBLIC const char* android_lookupEventTag(const EventTagMap* map,
                                                     unsigned int tag)
{
    size_t len;
    const char* tagStr = android_lookupEventTag_len(map, &len, tag);
    char* cp;

    if (!tagStr) return tagStr;
    cp = (char*)tagStr;
    cp += len;
    if (*cp) *cp = '\0'; /* Trigger copy on write :-( */
    return tagStr;
}

/*
 * Crunch through the file, parsing the contents and creating a tag index.
 */
@@ -337,19 +361,13 @@ static int scanTagLine(char** pData, EventTag* tag, int lineNum)
    /* Determine whether "c" is a valid tag char. */
    while (isalnum(*++cp) || (*cp == '_')) {
    }
    tag->tagLen = cp - tag->tagStr;

    if (*cp == '\n') {
        /* null terminate and return */
        *cp = '\0';
    } else if (isspace(*cp)) {
        /* CRLF or trailin spaces; zap this char, then scan for the '\n' */
        *cp = '\0';

    if (isspace(*cp)) {
        /* just ignore the rest of the line till \n
        TODO: read the tag description that follows the tag name
        */
        while (*++cp != '\n') {
        }
        while (*cp != '\n') ++cp;
    } else {
        fprintf(stderr, "%s: invalid tag chars on line %d\n", OUT_TAG, lineNum);
        errno = EINVAL;
@@ -387,10 +405,12 @@ static int sortTags(EventTagMap* map)
    for (i = 1; i < map->numTags; i++) {
        if (map->tagArray[i].tagIndex == map->tagArray[i - 1].tagIndex) {
            fprintf(stderr,
                "%s: duplicate tag entries (%" PRIu32 ":%s and %" PRIu32 ":%s)\n",
                "%s: duplicate tag entries (%" PRIu32 ":%.*s and %" PRIu32 ":%.*s)\n",
                OUT_TAG,
                map->tagArray[i].tagIndex, map->tagArray[i].tagStr,
                map->tagArray[i - 1].tagIndex, map->tagArray[i - 1].tagStr);
                map->tagArray[i].tagIndex, (int)map->tagArray[i].tagLen,
                map->tagArray[i].tagStr,
                map->tagArray[i - 1].tagIndex, (int)map->tagArray[i - 1].tagLen,
                map->tagArray[i - 1].tagStr);
            errno = EMLINK;
            return -1;
        }
+9 −0
Original line number Diff line number Diff line
@@ -718,3 +718,12 @@ LIBLOG_ABI_PUBLIC int __android_log_is_loggable(int prio,
    int logLevel = def;
    return logLevel >= 0 && prio >= logLevel;
}

LIBLOG_ABI_PUBLIC int __android_log_is_loggable_len(int prio,
                                                    const char *tag __unused,
                                                    size_t len __unused,
                                                    int def)
{
    int logLevel = def;
    return logLevel >= 0 && prio >= logLevel;
}
Loading