Merge changes Id5f03945,I69c74471

* changes:
  logd: add fuzzer for SerializedLogBuffer and other improvements
  logd: join() the SerializedLogBuffer deleter thread

logd/LogTags.cpp

@@ -276,7 +276,9 @@ void LogTags::ReadFileEventLogTags(const char* filename, bool warn) {
             cp++;
         }
     } else if (warn) {
+#ifdef __ANDROID__
         LOG(ERROR) << "Cannot read " << filename;
+#endif
     }
 }
 

logd/SerializedLogBuffer.cpp

@@ -16,8 +16,9 @@
 
 #include "SerializedLogBuffer.h"
 
+#include <sys/prctl.h>
+
 #include <limits>
-#include <thread>
 
 #include <android-base/logging.h>
 #include <android-base/scopeguard.h>
@@ -31,7 +32,11 @@ SerializedLogBuffer::SerializedLogBuffer(LogReaderList* reader_list, LogTags* ta
     Init();
 }
 
-SerializedLogBuffer::~SerializedLogBuffer() {}
+SerializedLogBuffer::~SerializedLogBuffer() {
+    if (deleter_thread_.joinable()) {
+        deleter_thread_.join();
+    }
+}
 
 void SerializedLogBuffer::Init() {
     log_id_for_each(i) {
@@ -119,15 +124,44 @@ void SerializedLogBuffer::MaybePrune(log_id_t log_id) {
     }
 }
 
+void SerializedLogBuffer::StartDeleterThread() {
+    if (deleter_thread_running_) {
+        return;
+    }
+    if (deleter_thread_.joinable()) {
+        deleter_thread_.join();
+    }
+    auto new_thread = std::thread([this] { DeleterThread(); });
+    deleter_thread_.swap(new_thread);
+    deleter_thread_running_ = true;
+}
+
 // Decompresses the chunks, call LogStatistics::Subtract() on each entry, then delete the chunks and
 // the list.  Note that the SerializedLogChunk objects have been removed from logs_ and their
 // references have been deleted from any SerializedFlushToState objects, so this can be safely done
-// without holding lock_.  It is done in a separate thread to avoid delaying the writer thread.  The
+// without holding lock_.  It is done in a separate thread to avoid delaying the writer thread.
-// lambda for the thread takes ownership of the 'chunks' list and thus when the thread ends and the
+void SerializedLogBuffer::DeleterThread() {
-// lambda is deleted, the objects are deleted.
+    prctl(PR_SET_NAME, "logd.deleter");
-void SerializedLogBuffer::DeleteLogChunks(std::list<SerializedLogChunk>&& chunks, log_id_t log_id) {
+    while (true) {
-    auto delete_thread = std::thread{[chunks = std::move(chunks), log_id, this]() mutable {
+        std::list<SerializedLogChunk> local_chunks_to_delete;
-        for (auto& chunk : chunks) {
+        log_id_t log_id;
+        {
+            auto lock = std::lock_guard{lock_};
+            log_id_for_each(i) {
+                if (!chunks_to_delete_[i].empty()) {
+                    local_chunks_to_delete = std::move(chunks_to_delete_[i]);
+                    chunks_to_delete_[i].clear();
+                    log_id = i;
+                    break;
+                }
+            }
+            if (local_chunks_to_delete.empty()) {
+                deleter_thread_running_ = false;
+                return;
+            }
+        }
+
+        for (auto& chunk : local_chunks_to_delete) {
             chunk.IncReaderRefCount();
             int read_offset = 0;
             while (read_offset < chunk.write_offset()) {
@@ -137,8 +171,7 @@ void SerializedLogBuffer::DeleteLogChunks(std::list<SerializedLogChunk>&& chunks
             }
             chunk.DecReaderRefCount(false);
         }
-    }};
+    }
-    delete_thread.detach();
 }
 
 void SerializedLogBuffer::NotifyReadersOfPrune(
@@ -164,13 +197,9 @@ bool SerializedLogBuffer::Prune(log_id_t log_id, size_t bytes_to_free, uid_t uid
         }
     }
 
+    StartDeleterThread();
+
     auto& log_buffer = logs_[log_id];
-
-    std::list<SerializedLogChunk> chunks_to_prune;
-    auto prune_chunks = android::base::make_scope_guard([&chunks_to_prune, log_id, this] {
-        DeleteLogChunks(std::move(chunks_to_prune), log_id);
-    });
-
     auto it = log_buffer.begin();
     while (it != log_buffer.end()) {
         if (oldest != nullptr && it->highest_sequence_number() >= oldest->start()) {
@@ -193,7 +222,8 @@ bool SerializedLogBuffer::Prune(log_id_t log_id, size_t bytes_to_free, uid_t uid
             }
         } else {
             size_t buffer_size = it_to_prune->PruneSize();
-            chunks_to_prune.splice(chunks_to_prune.end(), log_buffer, it_to_prune);
+            chunks_to_delete_[log_id].splice(chunks_to_delete_[log_id].end(), log_buffer,
+                                             it_to_prune);
             if (buffer_size >= bytes_to_free) {
                 return true;
             }

logd/SerializedLogBuffer.h

@@ -21,6 +21,7 @@
 #include <list>
 #include <mutex>
 #include <queue>
+#include <thread>
 #include <vector>
 
 #include <android-base/thread_annotations.h>
@@ -60,7 +61,9 @@ class SerializedLogBuffer final : public LogBuffer {
             REQUIRES_SHARED(lock_);
     void NotifyReadersOfPrune(log_id_t log_id, const std::list<SerializedLogChunk>::iterator& chunk)
             REQUIRES(reader_list_->reader_threads_lock());
-    void DeleteLogChunks(std::list<SerializedLogChunk>&& chunks, log_id_t log_id);
+
+    void StartDeleterThread() REQUIRES(lock_);
+    void DeleterThread();
 
     LogReaderList* reader_list_;
     LogTags* tags_;
@@ -70,5 +73,9 @@ class SerializedLogBuffer final : public LogBuffer {
     std::list<SerializedLogChunk> logs_[LOG_ID_MAX] GUARDED_BY(lock_);
     RwLock lock_;
 
+    std::list<SerializedLogChunk> chunks_to_delete_[LOG_ID_MAX] GUARDED_BY(lock_);
+    std::thread deleter_thread_ GUARDED_BY(lock_);
+    bool deleter_thread_running_ GUARDED_BY(lock_) = false;
+
     std::atomic<uint64_t> sequence_ = 1;
 };

logd/fuzz/Android.bp

@@ -13,11 +13,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-cc_fuzz {
+
-    name: "log_buffer_log_fuzzer",
+cc_defaults {
-    srcs: [
+    name: "log_fuzzer_defaults",
-        "log_buffer_log_fuzzer.cpp",
-    ],
     static_libs: [
         "libbase",
         "libcutils",
@@ -25,9 +23,25 @@ cc_fuzz {
         "liblog",
         "liblogd",
         "libcutils",
-        "libsysutils",
         "libz",
         "libzstd",
     ],
-    cflags: ["-Werror"],
+    cflags: ["-Wextra"],
+    host_supported: true,
+}
+
+cc_fuzz {
+    name: "log_buffer_log_fuzzer",
+    defaults: ["log_fuzzer_defaults"],
+    srcs: [
+        "log_buffer_log_fuzzer.cpp",
+    ],
+}
+
+cc_fuzz {
+    name: "serialized_log_buffer_fuzzer",
+    defaults: ["log_fuzzer_defaults"],
+    srcs: [
+        "serialized_log_buffer_fuzzer.cpp",
+    ],
 }

logd/fuzz/log_buffer_log_fuzzer.cpp

@@ -15,10 +15,13 @@
  */
 #include <string>
 
+#include <android-base/logging.h>
+
 #include "../ChattyLogBuffer.h"
 #include "../LogReaderList.h"
 #include "../LogReaderThread.h"
 #include "../LogStatistics.h"
+#include "../SerializedLogBuffer.h"
 
 // We don't want to waste a lot of entropy on messages
 #define MAX_MSG_LENGTH 5
@@ -27,7 +30,20 @@
 #define MIN_TAG_ID 1000
 #define TAG_MOD 10
 
-namespace android {
+#ifndef __ANDROID__
+unsigned long __android_logger_get_buffer_size(log_id_t) {
+    return 1024 * 1024;
+}
+
+bool __android_logger_valid_buffer_size(unsigned long) {
+    return true;
+}
+#endif
+
+char* android::uidToName(uid_t) {
+    return strdup("fake");
+}
+
 struct LogInput {
   public:
     log_id_t log_id;
@@ -79,9 +95,13 @@ int write_log_messages(const uint8_t** pdata, size_t* data_left, LogBuffer* log_
     return 1;
 }
 
-char* uidToName(uid_t) {
+class NoopWriter : public LogWriter {
-    return strdup("fake");
+  public:
-}
+    NoopWriter() : LogWriter(0, true) {}
+    bool Write(const logger_entry&, const char*) override { return true; }
+
+    std::string name() const override { return "noop_writer"; }
+};
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     // We want a random tag length and a random remaining message length
@@ -89,11 +109,18 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
         return 0;
     }
 
+    android::base::SetMinimumLogSeverity(android::base::ERROR);
+
     LogReaderList reader_list;
     LogTags tags;
     PruneList prune_list;
     LogStatistics stats(true);
-    LogBuffer* log_buffer = new ChattyLogBuffer(&reader_list, &tags, &prune_list, &stats);
+    std::unique_ptr<LogBuffer> log_buffer;
+#ifdef FUZZ_SERIALIZED
+    log_buffer.reset(new SerializedLogBuffer(&reader_list, &tags, &stats));
+#else
+    log_buffer.reset(new ChattyLogBuffer(&reader_list, &tags, &prune_list, &stats));
+#endif
     size_t data_left = size;
     const uint8_t** pdata = &data;
 
@@ -102,12 +129,30 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     log_id_for_each(i) { log_buffer->SetSize(i, 10000); }
 
     while (data_left >= sizeof(LogInput) + 2 * sizeof(uint8_t)) {
-        if (!write_log_messages(pdata, &data_left, log_buffer, &stats)) {
+        if (!write_log_messages(pdata, &data_left, log_buffer.get(), &stats)) {
             return 0;
         }
     }
 
+    // Read out all of the logs.
+    {
+        auto lock = std::unique_lock{reader_list.reader_threads_lock()};
+        std::unique_ptr<LogWriter> test_writer(new NoopWriter());
+        std::unique_ptr<LogReaderThread> log_reader(
+                new LogReaderThread(log_buffer.get(), &reader_list, std::move(test_writer), true, 0,
+                                    kLogMaskAll, 0, {}, 1, {}));
+        reader_list.reader_threads().emplace_back(std::move(log_reader));
+    }
+
+    // Wait until the reader has finished.
+    while (true) {
+        usleep(50);
+        auto lock = std::unique_lock{reader_list.reader_threads_lock()};
+        if (reader_list.reader_threads().size() == 0) {
+            break;
+        }
+    }
+
     log_id_for_each(i) { log_buffer->Clear(i, 0); }
     return 0;
 }
-}  // namespace android

logd/fuzz/serialized_log_buffer_fuzzer.cpp

@@ -0,0 +1,19 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define FUZZ_SERIALIZED
+
+#include "log_buffer_log_fuzzer.cpp"