From 3dd3ec351476171efb5ce540557abf9ed456f2a6 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 2 Jun 2020 15:39:21 -0700 Subject: [PATCH] logd: Drop the LogStatistics dependency on LogBufferElement Other log buffers may not use LogBufferElement, so we should decouple the two classes. This uses an intermediate LogStatisticsElement structs instead of passing a large number of parameters to each function. This additionally moves IsBinary() and the GetTag() functions out into LogUtils.h since they can be used generically by other users. Test: logging unit tests Change-Id: I71f53257342c067bcccd5aa00bae47f714cd7c66 --- logd/ChattyLogBuffer.cpp | 9 +-- logd/LogBufferElement.cpp | 25 +++++--- logd/LogBufferElement.h | 6 +- logd/LogStatistics.cpp | 74 ++++++++++----------- logd/LogStatistics.h | 131 ++++++++++++++++++++------------------ logd/LogUtils.h | 14 ++++ logd/SimpleLogBuffer.cpp | 11 ++-- 7 files changed, 151 insertions(+), 119 deletions(-) diff --git a/logd/ChattyLogBuffer.cpp b/logd/ChattyLogBuffer.cpp index c2e89fcf2..62c8629c4 100644 --- a/logd/ChattyLogBuffer.cpp +++ b/logd/ChattyLogBuffer.cpp @@ -74,7 +74,8 @@ static enum match_type Identical(const LogBufferElement& elem, const LogBufferEl } // audit message (except sequence number) identical? - if (last.IsBinary() && lenl > static_cast(sizeof(android_log_event_string_t)) && + if (IsBinary(last.log_id()) && + lenl > static_cast(sizeof(android_log_event_string_t)) && lenr > static_cast(sizeof(android_log_event_string_t))) { if (fastcmp(msgl, msgr, sizeof(android_log_event_string_t) - sizeof(int32_t))) { return DIFFERENT; @@ -205,9 +206,9 @@ LogBufferElementCollection::iterator ChattyLogBuffer::Erase(LogBufferElementColl #endif if (coalesce) { - stats()->Erase(element); + stats()->Erase(element.ToLogStatisticsElement()); } else { - stats()->Subtract(element); + stats()->Subtract(element.ToLogStatisticsElement()); } it = SimpleLogBuffer::Erase(it); @@ -533,7 +534,7 @@ bool ChattyLogBuffer::Prune(log_id_t id, unsigned long pruneRows, uid_t caller_u if (leading) { it = Erase(it); } else { - stats()->Drop(element); + stats()->Drop(element.ToLogStatisticsElement()); element.SetDropped(1); if (last.coalesce(&element, 1)) { it = Erase(it, true); diff --git a/logd/LogBufferElement.cpp b/logd/LogBufferElement.cpp index 172a75786..ef9f1cf36 100644 --- a/logd/LogBufferElement.cpp +++ b/logd/LogBufferElement.cpp @@ -86,7 +86,7 @@ LogBufferElement::~LogBufferElement() { uint32_t LogBufferElement::GetTag() const { // Binary buffers have no tag. - if (!IsBinary()) { + if (!IsBinary(log_id())) { return 0; } @@ -95,12 +95,21 @@ uint32_t LogBufferElement::GetTag() const { return tag_; } - // For non-dropped messages, we get the tag from the message header itself. - if (msg_len_ < sizeof(android_event_header_t)) { - return 0; - } + return MsgToTag(msg(), msg_len()); +} - return reinterpret_cast(msg_)->tag; +LogStatisticsElement LogBufferElement::ToLogStatisticsElement() const { + return LogStatisticsElement{ + .uid = uid(), + .pid = pid(), + .tid = tid(), + .tag = GetTag(), + .realtime = realtime(), + .msg = msg(), + .msg_len = msg_len(), + .dropped_count = dropped_count(), + .log_id = log_id(), + }; } uint16_t LogBufferElement::SetDropped(uint16_t value) { @@ -218,7 +227,7 @@ size_t LogBufferElement::PopulateDroppedMessage(char*& buffer, LogStatistics* st type, dropped_count(), (dropped_count() > 1) ? "s" : ""); size_t hdrLen; - if (IsBinary()) { + if (IsBinary(log_id())) { hdrLen = sizeof(android_log_event_string_t); } else { hdrLen = 1 + sizeof(tag); @@ -232,7 +241,7 @@ size_t LogBufferElement::PopulateDroppedMessage(char*& buffer, LogStatistics* st } size_t retval = hdrLen + len; - if (IsBinary()) { + if (IsBinary(log_id())) { android_log_event_string_t* event = reinterpret_cast(buffer); diff --git a/logd/LogBufferElement.h b/logd/LogBufferElement.h index 5b13e32c7..fd5d88fd9 100644 --- a/logd/LogBufferElement.h +++ b/logd/LogBufferElement.h @@ -24,7 +24,7 @@ #include "LogWriter.h" -class LogStatistics; +#include "LogStatistics.h" #define EXPIRE_HOUR_THRESHOLD 24 // Only expire chatty UID logs to preserve // non-chatty UIDs less than this age in hours @@ -40,13 +40,13 @@ class __attribute__((packed)) LogBufferElement { LogBufferElement(LogBufferElement&& elem); ~LogBufferElement(); - bool IsBinary() const { return (log_id_ == LOG_ID_EVENTS) || (log_id_ == LOG_ID_SECURITY); } - uint32_t GetTag() const; uint16_t SetDropped(uint16_t value); bool FlushTo(LogWriter* writer, LogStatistics* parent, bool lastSame); + LogStatisticsElement ToLogStatisticsElement() const; + log_id_t log_id() const { return static_cast(log_id_); } uid_t uid() const { return uid_; } pid_t pid() const { return pid_; } diff --git a/logd/LogStatistics.cpp b/logd/LogStatistics.cpp index a2acab7aa..04fd59d95 100644 --- a/logd/LogStatistics.cpp +++ b/logd/LogStatistics.cpp @@ -29,6 +29,8 @@ #include +#include "LogBufferElement.h" + static const uint64_t hourSec = 60 * 60; static const uint64_t monthSec = 31 * 24 * hourSec; @@ -87,10 +89,10 @@ void LogStatistics::AddTotal(log_id_t log_id, uint16_t size) { ++mElementsTotal[log_id]; } -void LogStatistics::Add(const LogBufferElement& element) { +void LogStatistics::Add(const LogStatisticsElement& element) { auto lock = std::lock_guard{lock_}; - log_id_t log_id = element.log_id(); - uint16_t size = element.msg_len(); + log_id_t log_id = element.log_id; + uint16_t size = element.msg_len; mSizes[log_id] += size; ++mElements[log_id]; @@ -99,7 +101,7 @@ void LogStatistics::Add(const LogBufferElement& element) { // evaluated and trimmed, thus recording size and number of // elements, but we must recognize the manufactured dropped // entry as not contributing to the lifetime totals. - if (element.dropped_count()) { + if (element.dropped_count) { ++mDroppedElements[log_id]; } else { mSizesTotal[log_id] += size; @@ -107,7 +109,7 @@ void LogStatistics::Add(const LogBufferElement& element) { ++mElementsTotal[log_id]; } - log_time stamp(element.realtime()); + log_time stamp(element.realtime); if (mNewest[log_id] < stamp) { // A major time update invalidates the statistics :-( log_time diff = stamp - mNewest[log_id]; @@ -132,19 +134,19 @@ void LogStatistics::Add(const LogBufferElement& element) { return; } - uidTable[log_id].Add(element.uid(), element); - if (element.uid() == AID_SYSTEM) { - pidSystemTable[log_id].Add(element.pid(), element); + uidTable[log_id].Add(element.uid, element); + if (element.uid == AID_SYSTEM) { + pidSystemTable[log_id].Add(element.pid, element); } if (!enable) { return; } - pidTable.Add(element.pid(), element); - tidTable.Add(element.tid(), element); + pidTable.Add(element.pid, element); + tidTable.Add(element.tid, element); - uint32_t tag = element.GetTag(); + uint32_t tag = element.tag; if (tag) { if (log_id == LOG_ID_SECURITY) { securityTagTable.Add(tag, element); @@ -153,42 +155,42 @@ void LogStatistics::Add(const LogBufferElement& element) { } } - if (!element.dropped_count()) { + if (!element.dropped_count) { tagNameTable.Add(TagNameKey(element), element); } } -void LogStatistics::Subtract(const LogBufferElement& element) { +void LogStatistics::Subtract(const LogStatisticsElement& element) { auto lock = std::lock_guard{lock_}; - log_id_t log_id = element.log_id(); - uint16_t size = element.msg_len(); + log_id_t log_id = element.log_id; + uint16_t size = element.msg_len; mSizes[log_id] -= size; --mElements[log_id]; - if (element.dropped_count()) { + if (element.dropped_count) { --mDroppedElements[log_id]; } - if (mOldest[log_id] < element.realtime()) { - mOldest[log_id] = element.realtime(); + if (mOldest[log_id] < element.realtime) { + mOldest[log_id] = element.realtime; } if (log_id == LOG_ID_KERNEL) { return; } - uidTable[log_id].Subtract(element.uid(), element); - if (element.uid() == AID_SYSTEM) { - pidSystemTable[log_id].Subtract(element.pid(), element); + uidTable[log_id].Subtract(element.uid, element); + if (element.uid == AID_SYSTEM) { + pidSystemTable[log_id].Subtract(element.pid, element); } if (!enable) { return; } - pidTable.Subtract(element.pid(), element); - tidTable.Subtract(element.tid(), element); + pidTable.Subtract(element.pid, element); + tidTable.Subtract(element.tid, element); - uint32_t tag = element.GetTag(); + uint32_t tag = element.tag; if (tag) { if (log_id == LOG_ID_SECURITY) { securityTagTable.Subtract(tag, element); @@ -197,37 +199,37 @@ void LogStatistics::Subtract(const LogBufferElement& element) { } } - if (!element.dropped_count()) { + if (!element.dropped_count) { tagNameTable.Subtract(TagNameKey(element), element); } } // Atomically set an entry to drop // entry->setDropped(1) must follow this call, caller should do this explicitly. -void LogStatistics::Drop(const LogBufferElement& element) { +void LogStatistics::Drop(const LogStatisticsElement& element) { auto lock = std::lock_guard{lock_}; - log_id_t log_id = element.log_id(); - uint16_t size = element.msg_len(); + log_id_t log_id = element.log_id; + uint16_t size = element.msg_len; mSizes[log_id] -= size; ++mDroppedElements[log_id]; - if (mNewestDropped[log_id] < element.realtime()) { - mNewestDropped[log_id] = element.realtime(); + if (mNewestDropped[log_id] < element.realtime) { + mNewestDropped[log_id] = element.realtime; } - uidTable[log_id].Drop(element.uid(), element); - if (element.uid() == AID_SYSTEM) { - pidSystemTable[log_id].Drop(element.pid(), element); + uidTable[log_id].Drop(element.uid, element); + if (element.uid == AID_SYSTEM) { + pidSystemTable[log_id].Drop(element.pid, element); } if (!enable) { return; } - pidTable.Drop(element.pid(), element); - tidTable.Drop(element.tid(), element); + pidTable.Drop(element.pid, element); + tidTable.Drop(element.tid, element); - uint32_t tag = element.GetTag(); + uint32_t tag = element.tag; if (tag) { if (log_id == LOG_ID_SECURITY) { securityTagTable.Drop(tag, element); diff --git a/logd/LogStatistics.h b/logd/LogStatistics.h index 6a46adbd9..33a4d63f7 100644 --- a/logd/LogStatistics.h +++ b/logd/LogStatistics.h @@ -38,7 +38,6 @@ #include #include -#include "LogBufferElement.h" #include "LogUtils.h" #define log_id_for_each(i) \ @@ -46,6 +45,18 @@ class LogStatistics; +struct LogStatisticsElement { + uid_t uid; + pid_t pid; + pid_t tid; + uint32_t tag; + log_time realtime; + const char* msg; + uint16_t msg_len; + uint16_t dropped_count; + log_id_t log_id; +}; + template class LogHashtable { std::unordered_map map; @@ -113,7 +124,7 @@ class LogHashtable { } } - iterator Add(const TKey& key, const LogBufferElement& element) { + iterator Add(const TKey& key, const LogStatisticsElement& element) { iterator it = map.find(key); if (it == map.end()) { it = map.insert(std::make_pair(key, TEntry(element))).first; @@ -133,14 +144,14 @@ class LogHashtable { return it; } - void Subtract(const TKey& key, const LogBufferElement& element) { + void Subtract(const TKey& key, const LogStatisticsElement& element) { iterator it = map.find(key); if (it != map.end() && it->second.Subtract(element)) { map.erase(it); } } - void Drop(const TKey& key, const LogBufferElement& element) { + void Drop(const TKey& key, const LogStatisticsElement& element) { iterator it = map.find(key); if (it != map.end()) { it->second.Drop(element); @@ -156,13 +167,13 @@ class LogHashtable { class EntryBase { public: EntryBase() : size_(0) {} - explicit EntryBase(const LogBufferElement& element) : size_(element.msg_len()) {} + explicit EntryBase(const LogStatisticsElement& element) : size_(element.msg_len) {} size_t getSizes() const { return size_; } - void Add(const LogBufferElement& element) { size_ += element.msg_len(); } - bool Subtract(const LogBufferElement& element) { - size_ -= element.msg_len(); + void Add(const LogStatisticsElement& element) { size_ += element.msg_len; } + bool Subtract(const LogStatisticsElement& element) { + size_ -= element.msg_len; return !size_; } @@ -193,20 +204,20 @@ class EntryBase { class EntryBaseDropped : public EntryBase { public: EntryBaseDropped() : dropped_(0) {} - explicit EntryBaseDropped(const LogBufferElement& element) - : EntryBase(element), dropped_(element.dropped_count()) {} + explicit EntryBaseDropped(const LogStatisticsElement& element) + : EntryBase(element), dropped_(element.dropped_count) {} size_t dropped_count() const { return dropped_; } - void Add(const LogBufferElement& element) { - dropped_ += element.dropped_count(); + void Add(const LogStatisticsElement& element) { + dropped_ += element.dropped_count; EntryBase::Add(element); } - bool Subtract(const LogBufferElement& element) { - dropped_ -= element.dropped_count(); + bool Subtract(const LogStatisticsElement& element) { + dropped_ -= element.dropped_count; return EntryBase::Subtract(element) && !dropped_; } - void Drop(const LogBufferElement& element) { + void Drop(const LogStatisticsElement& element) { dropped_ += 1; EntryBase::Subtract(element); } @@ -217,15 +228,15 @@ class EntryBaseDropped : public EntryBase { class UidEntry : public EntryBaseDropped { public: - explicit UidEntry(const LogBufferElement& element) - : EntryBaseDropped(element), uid_(element.uid()), pid_(element.pid()) {} + explicit UidEntry(const LogStatisticsElement& element) + : EntryBaseDropped(element), uid_(element.uid), pid_(element.pid) {} uid_t key() const { return uid_; } uid_t uid() const { return key(); } pid_t pid() const { return pid_; } - void Add(const LogBufferElement& element) { - if (pid_ != element.pid()) { + void Add(const LogStatisticsElement& element) { + if (pid_ != element.pid) { pid_ = -1; } EntryBaseDropped::Add(element); @@ -250,10 +261,10 @@ class PidEntry : public EntryBaseDropped { pid_(pid), uid_(android::pidToUid(pid)), name_(android::pidToName(pid)) {} - explicit PidEntry(const LogBufferElement& element) + explicit PidEntry(const LogStatisticsElement& element) : EntryBaseDropped(element), - pid_(element.pid()), - uid_(element.uid()), + pid_(element.pid), + uid_(element.uid), name_(android::pidToName(pid_)) {} PidEntry(const PidEntry& element) : EntryBaseDropped(element), @@ -277,14 +288,14 @@ class PidEntry : public EntryBaseDropped { } } - void Add(const LogBufferElement& element) { - uid_t incoming_uid = element.uid(); + void Add(const LogStatisticsElement& element) { + uid_t incoming_uid = element.uid; if (uid() != incoming_uid) { uid_ = incoming_uid; free(name_); - name_ = android::pidToName(element.pid()); + name_ = android::pidToName(element.pid); } else { - Add(element.pid()); + Add(element.pid); } EntryBaseDropped::Add(element); } @@ -306,11 +317,11 @@ class TidEntry : public EntryBaseDropped { pid_(pid), uid_(android::pidToUid(tid)), name_(android::tidToName(tid)) {} - explicit TidEntry(const LogBufferElement& element) + explicit TidEntry(const LogStatisticsElement& element) : EntryBaseDropped(element), - tid_(element.tid()), - pid_(element.pid()), - uid_(element.uid()), + tid_(element.tid), + pid_(element.pid), + uid_(element.uid), name_(android::tidToName(tid_)) {} TidEntry(const TidEntry& element) : EntryBaseDropped(element), @@ -336,16 +347,16 @@ class TidEntry : public EntryBaseDropped { } } - void Add(const LogBufferElement& element) { - uid_t incoming_uid = element.uid(); - pid_t incoming_pid = element.pid(); + void Add(const LogStatisticsElement& element) { + uid_t incoming_uid = element.uid; + pid_t incoming_pid = element.pid; if (uid() != incoming_uid || pid() != incoming_pid) { uid_ = incoming_uid; pid_ = incoming_pid; free(name_); - name_ = android::tidToName(element.tid()); + name_ = android::tidToName(element.tid); } else { - Add(element.tid()); + Add(element.tid); } EntryBaseDropped::Add(element); } @@ -362,22 +373,19 @@ class TidEntry : public EntryBaseDropped { class TagEntry : public EntryBaseDropped { public: - explicit TagEntry(const LogBufferElement& element) - : EntryBaseDropped(element), - tag_(element.GetTag()), - pid_(element.pid()), - uid_(element.uid()) {} + explicit TagEntry(const LogStatisticsElement& element) + : EntryBaseDropped(element), tag_(element.tag), pid_(element.pid), uid_(element.uid) {} uint32_t key() const { return tag_; } pid_t pid() const { return pid_; } uid_t uid() const { return uid_; } const char* name() const { return android::tagToName(tag_); } - void Add(const LogBufferElement& element) { - if (uid_ != element.uid()) { + void Add(const LogStatisticsElement& element) { + if (uid_ != element.uid) { uid_ = -1; } - if (pid_ != element.pid()) { + if (pid_ != element.pid) { pid_ = -1; } EntryBaseDropped::Add(element); @@ -396,9 +404,10 @@ struct TagNameKey { std::string* alloc; std::string_view name; // Saves space if const char* - explicit TagNameKey(const LogBufferElement& element) : alloc(nullptr), name("", strlen("")) { - if (element.IsBinary()) { - uint32_t tag = element.GetTag(); + explicit TagNameKey(const LogStatisticsElement& element) + : alloc(nullptr), name("", strlen("")) { + if (IsBinary(element.log_id)) { + uint32_t tag = element.tag; if (tag) { const char* cp = android::tagToName(tag); if (cp) { @@ -412,13 +421,13 @@ struct TagNameKey { name = std::string_view(alloc->c_str(), alloc->size()); return; } - const char* msg = element.msg(); + const char* msg = element.msg; if (!msg) { name = std::string_view("chatty", strlen("chatty")); return; } ++msg; - uint16_t len = element.msg_len(); + uint16_t len = element.msg_len; len = (len <= 1) ? 0 : strnlen(msg, len - 1); if (!len) { name = std::string_view("", strlen("")); @@ -480,11 +489,11 @@ struct std::hash class TagNameEntry : public EntryBase { public: - explicit TagNameEntry(const LogBufferElement& element) + explicit TagNameEntry(const LogStatisticsElement& element) : EntryBase(element), - tid_(element.tid()), - pid_(element.pid()), - uid_(element.uid()), + tid_(element.tid), + pid_(element.pid), + uid_(element.uid), name_(element) {} const TagNameKey& key() const { return name_; } @@ -494,14 +503,14 @@ class TagNameEntry : public EntryBase { const char* name() const { return name_.data(); } size_t getNameAllocLength() const { return name_.getAllocLength(); } - void Add(const LogBufferElement& element) { - if (uid_ != element.uid()) { + void Add(const LogStatisticsElement& element) { + if (uid_ != element.uid) { uid_ = -1; } - if (pid_ != element.pid()) { + if (pid_ != element.pid) { pid_ = -1; } - if (tid_ != element.tid()) { + if (tid_ != element.tid) { tid_ = -1; } EntryBase::Add(element); @@ -590,14 +599,14 @@ class LogStatistics { LogStatistics(bool enable_statistics); void AddTotal(log_id_t log_id, uint16_t size) EXCLUDES(lock_); - void Add(const LogBufferElement& entry) EXCLUDES(lock_); - void Subtract(const LogBufferElement& entry) EXCLUDES(lock_); + void Add(const LogStatisticsElement& entry) EXCLUDES(lock_); + void Subtract(const LogStatisticsElement& entry) EXCLUDES(lock_); // entry->setDropped(1) must follow this call - void Drop(const LogBufferElement& entry) EXCLUDES(lock_); + void Drop(const LogStatisticsElement& entry) EXCLUDES(lock_); // Correct for coalescing two entries referencing dropped content - void Erase(const LogBufferElement& element) EXCLUDES(lock_) { + void Erase(const LogStatisticsElement& element) EXCLUDES(lock_) { auto lock = std::lock_guard{lock_}; - log_id_t log_id = element.log_id(); + log_id_t log_id = element.log_id; --mElements[log_id]; --mDroppedElements[log_id]; } diff --git a/logd/LogUtils.h b/logd/LogUtils.h index c472167fd..96aa1d349 100644 --- a/logd/LogUtils.h +++ b/logd/LogUtils.h @@ -60,6 +60,20 @@ static inline const char* strnstr(const char* s, ssize_t len, } } +// Returns true if the log buffer is meant for binary logs. +static inline bool IsBinary(log_id_t log_id) { + return log_id == LOG_ID_EVENTS || log_id == LOG_ID_STATS || log_id == LOG_ID_SECURITY; +} + +// Returns the numeric log tag for binary log messages. +static inline uint32_t MsgToTag(const char* msg, uint16_t msg_len) { + if (msg_len < sizeof(android_event_header_t)) { + return 0; + } + + return reinterpret_cast(msg)->tag; +} + static inline bool worstUidEnabledForLogid(log_id_t id) { return (id == LOG_ID_MAIN) || (id == LOG_ID_SYSTEM) || (id == LOG_ID_RADIO) || (id == LOG_ID_EVENTS); diff --git a/logd/SimpleLogBuffer.cpp b/logd/SimpleLogBuffer.cpp index 5ab8e0983..b4b354636 100644 --- a/logd/SimpleLogBuffer.cpp +++ b/logd/SimpleLogBuffer.cpp @@ -61,11 +61,8 @@ bool SimpleLogBuffer::ShouldLog(log_id_t log_id, const char* msg, uint16_t len) int prio = ANDROID_LOG_INFO; const char* tag = nullptr; size_t tag_len = 0; - if (log_id == LOG_ID_EVENTS || log_id == LOG_ID_STATS) { - if (len < sizeof(android_event_header_t)) { - return false; - } - int32_t numeric_tag = reinterpret_cast(msg)->tag; + if (IsBinary(log_id)) { + int32_t numeric_tag = MsgToTag(msg, len); tag = tags_->tagToName(numeric_tag); if (tag) { tag_len = strlen(tag); @@ -105,7 +102,7 @@ void SimpleLogBuffer::LogInternal(LogBufferElement&& elem) { log_id_t log_id = elem.log_id(); logs_.emplace_back(std::move(elem)); - stats_->Add(logs_.back()); + stats_->Add(logs_.back().ToLogStatisticsElement()); MaybePrune(log_id); reader_list_->NotifyNewLog(1 << log_id); } @@ -317,7 +314,7 @@ bool SimpleLogBuffer::Prune(log_id_t id, unsigned long prune_rows, uid_t caller_ return true; } - stats_->Subtract(element); + stats_->Subtract(element.ToLogStatisticsElement()); it = Erase(it); if (--prune_rows == 0) { return false;