diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-08-14 05:17:05 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-08-14 05:17:05 +0000 |
commit | 67b6607e061529738384228f1a23159d1ad4215f (patch) | |
tree | 93d699e33848416768e81412a3b42677c161c6d4 | |
parent | 194345ddba19373f6c31a15df5f9fcb432983615 (diff) | |
parent | 946f3948b42e405bc0c21917498f668c41303f5b (diff) | |
download | bionic-android12-mainline-statsd-release.tar.gz |
Snap for 7642458 from 946f3948b42e405bc0c21917498f668c41303f5b to mainline-os-statsd-releaseandroid-mainline-12.0.0_r84android-mainline-12.0.0_r58android12-mainline-statsd-release
Change-Id: Ifb0bac82fadef2275d9bff3f163b6dd6ba3282a5
-rw-r--r-- | libc/Android.bp | 1 | ||||
-rw-r--r-- | libc/bionic/bionic_systrace.cpp | 53 | ||||
-rw-r--r-- | libc/bionic/system_property_set.cpp | 7 | ||||
-rw-r--r-- | libc/malloc_debug/malloc_debug.cpp | 7 | ||||
-rw-r--r-- | libc/malloc_debug/tests/malloc_debug_system_tests.cpp | 67 | ||||
-rw-r--r-- | libc/private/bionic_systrace.h | 8 | ||||
-rw-r--r-- | libc/system_properties/Android.bp | 2 | ||||
-rw-r--r-- | libc/system_properties/include/system_properties/prop_trace.h | 49 | ||||
-rw-r--r-- | libc/system_properties/prop_trace.cpp | 113 | ||||
-rw-r--r-- | libc/system_properties/system_properties.cpp | 10 |
10 files changed, 91 insertions, 226 deletions
diff --git a/libc/Android.bp b/libc/Android.bp index 892262173..2c0656f72 100644 --- a/libc/Android.bp +++ b/libc/Android.bp @@ -1202,6 +1202,7 @@ cc_library_static { }, }, whole_static_libs: [ + "libc_bionic_systrace", "libsystemproperties", ], cppflags: ["-Wold-style-cast"], diff --git a/libc/bionic/bionic_systrace.cpp b/libc/bionic/bionic_systrace.cpp index 0de51c50a..fd9771298 100644 --- a/libc/bionic/bionic_systrace.cpp +++ b/libc/bionic/bionic_systrace.cpp @@ -14,50 +14,52 @@ * limitations under the License. */ -#include "private/bionic_systrace.h" - #include <errno.h> #include <fcntl.h> #include <stdio.h> #include <stdlib.h> #include <string.h> +#include "private/bionic_lock.h" +#include "private/bionic_systrace.h" +#include "private/CachedProperty.h" + #include <async_safe/log.h> #include <cutils/trace.h> // For ATRACE_TAG_BIONIC. -#include "private/CachedProperty.h" -#include "private/bionic_lock.h" - #define WRITE_OFFSET 32 static Lock g_lock; +static CachedProperty g_debug_atrace_tags_enableflags("debug.atrace.tags.enableflags"); +static uint64_t g_tags; +static int g_trace_marker_fd = -1; -bool should_trace(const uint64_t enable_tags) { - static uint64_t tags_val; - static CachedProperty tags_prop(kTraceTagsProp); +static bool should_trace() { g_lock.lock(); - if (tags_prop.DidChange()) { - tags_val = strtoull(tags_prop.Get(), nullptr, 0); + if (g_debug_atrace_tags_enableflags.DidChange()) { + g_tags = strtoull(g_debug_atrace_tags_enableflags.Get(), nullptr, 0); } g_lock.unlock(); - return tags_val & enable_tags; + return ((g_tags & ATRACE_TAG_BIONIC) != 0); } -int get_trace_marker_fd() { - static int opened_trace_marker_fd = -1; +static int get_trace_marker_fd() { g_lock.lock(); - if (opened_trace_marker_fd == -1) { - opened_trace_marker_fd = open("/sys/kernel/tracing/trace_marker", O_CLOEXEC | O_WRONLY); - if (opened_trace_marker_fd == -1) { - opened_trace_marker_fd = open("/sys/kernel/debug/tracing/trace_marker", O_CLOEXEC | O_WRONLY); + if (g_trace_marker_fd == -1) { + g_trace_marker_fd = open("/sys/kernel/tracing/trace_marker", O_CLOEXEC | O_WRONLY); + if (g_trace_marker_fd == -1) { + g_trace_marker_fd = open("/sys/kernel/debug/tracing/trace_marker", O_CLOEXEC | O_WRONLY); } } g_lock.unlock(); - return opened_trace_marker_fd; + return g_trace_marker_fd; } -// event could be 'B' for begin or 'E' for end. -void output_trace(const char* message, const char event) { +void bionic_trace_begin(const char* message) { + if (!should_trace()) { + return; + } + int trace_marker_fd = get_trace_marker_fd(); if (trace_marker_fd == -1) { return; @@ -67,22 +69,13 @@ void output_trace(const char* message, const char event) { // kernel trace_marker. int length = strlen(message); char buf[length + WRITE_OFFSET]; - size_t len = - async_safe_format_buffer(buf, length + WRITE_OFFSET, "%c|%d|%s", event, getpid(), message); + size_t len = async_safe_format_buffer(buf, length + WRITE_OFFSET, "B|%d|%s", getpid(), message); // Tracing may stop just after checking property and before writing the message. // So the write is acceptable to fail. See b/20666100. TEMP_FAILURE_RETRY(write(trace_marker_fd, buf, len)); } -void bionic_trace_begin(const char* message) { - if (!should_trace()) { - return; - } - - output_trace(message); -} - void bionic_trace_end() { if (!should_trace()) { return; diff --git a/libc/bionic/system_property_set.cpp b/libc/bionic/system_property_set.cpp index 6823b6aab..212aafcc1 100644 --- a/libc/bionic/system_property_set.cpp +++ b/libc/bionic/system_property_set.cpp @@ -41,13 +41,12 @@ #include <sys/_system_properties.h> #include <unistd.h> -#include <async_safe/CHECK.h> #include <async_safe/log.h> -#include <system_properties/prop_trace.h> +#include <async_safe/CHECK.h> +#include "private/bionic_defs.h" #include "platform/bionic/macros.h" #include "private/ScopedFd.h" -#include "private/bionic_defs.h" static const char property_service_socket[] = "/dev/socket/" PROP_SERVICE_NAME; static const char* kServiceVersionPropertyName = "ro.property_service.version"; @@ -250,8 +249,6 @@ int __system_property_set(const char* key, const char* value) { if (key == nullptr) return -1; if (value == nullptr) value = ""; - SyspropTrace trace(key, value, nullptr /* prop_info */, PropertyAction::kPropertySet); - if (g_propservice_protocol_version == 0) { detect_protocol_version(); } diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp index 609f030bf..d23ab15c3 100644 --- a/libc/malloc_debug/malloc_debug.cpp +++ b/libc/malloc_debug/malloc_debug.cpp @@ -362,10 +362,9 @@ void debug_finalize() { backtrace_shutdown(); - delete g_debug; - g_debug = nullptr; - - DebugDisableFinalize(); + // In order to prevent any issues of threads freeing previous pointers + // after the main thread calls this code, simply leak the g_debug pointer + // and do not destroy the debug disable pthread key. } void debug_get_malloc_leak_info(uint8_t** info, size_t* overall_size, size_t* info_size, diff --git a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp index 1bfe61e77..68b3a1ec0 100644 --- a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp +++ b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp @@ -30,6 +30,7 @@ #include <fcntl.h> #include <poll.h> #include <signal.h> +#include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <sys/types.h> @@ -449,12 +450,12 @@ TEST(MallocDebugSystemTest, verify_leak_allocation_limit) { } static constexpr int kExpectedExitCode = 30; +static constexpr size_t kMaxThreads = sizeof(uint32_t) * 8; TEST(MallocTests, DISABLED_exit_while_threads_allocating) { - std::atomic_uint32_t thread_mask; - thread_mask = 0; + std::atomic_uint32_t thread_mask = {}; - for (size_t i = 0; i < 32; i++) { + for (size_t i = 0; i < kMaxThreads; i++) { std::thread malloc_thread([&thread_mask, i] { while (true) { void* ptr = malloc(100); @@ -469,7 +470,7 @@ TEST(MallocTests, DISABLED_exit_while_threads_allocating) { } // Wait until each thread has done at least one allocation. - while (thread_mask.load() != 0xffffffff) + while (thread_mask.load() != UINT32_MAX) ; exit(kExpectedExitCode); } @@ -492,6 +493,59 @@ TEST(MallocDebugSystemTest, exit_while_threads_allocating) { } } +TEST(MallocTests, DISABLED_exit_while_threads_freeing_allocs_with_header) { + static constexpr size_t kMaxAllocsPerThread = 1000; + std::atomic_uint32_t thread_mask = {}; + std::atomic_bool run; + + std::vector<std::vector<void*>> allocs(kMaxThreads); + // Pre-allocate a bunch of memory so that we can try to trigger + // the frees after the main thread finishes. + for (size_t i = 0; i < kMaxThreads; i++) { + for (size_t j = 0; j < kMaxAllocsPerThread; j++) { + void* ptr = malloc(8); + ASSERT_TRUE(ptr != nullptr); + allocs[i].push_back(ptr); + } + } + + for (size_t i = 0; i < kMaxThreads; i++) { + std::thread malloc_thread([&thread_mask, &run, &allocs, i] { + thread_mask.fetch_or(1 << i); + while (!run) + ; + for (auto ptr : allocs[i]) { + free(ptr); + } + }); + malloc_thread.detach(); + } + + // Wait until all threads are running. + while (thread_mask.load() != UINT32_MAX) + ; + run = true; + exit(kExpectedExitCode); +} + +TEST(MallocDebugSystemTest, exit_while_threads_freeing_allocs_with_header) { + for (size_t i = 0; i < 50; i++) { + SCOPED_TRACE(::testing::Message() << "Run " << i); + pid_t pid; + // Enable guard to force the use of a header. + ASSERT_NO_FATAL_FAILURE( + Exec("MallocTests.DISABLED_exit_while_threads_freeing_allocs_with_header", "verbose guard", + &pid, kExpectedExitCode)); + + ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"})); + + std::string log_str; + GetLogStr(pid, &log_str, LOG_ID_CRASH); + ASSERT_TRUE(log_str.find("Fatal signal") == std::string::npos) + << "Found crash in log.\nLog message: " << log_str; + } +} + TEST(MallocTests, DISABLED_write_leak_info) { TemporaryFile tf; ASSERT_TRUE(tf.fd != -1); @@ -555,8 +609,11 @@ TEST(MallocTests, DISABLED_malloc_and_backtrace_deadlock) { static constexpr size_t kNumUnwinds = 1000; for (size_t i = 0; i < kNumUnwinds; i++) { std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), tid)); + // Only verify that there is at least one frame in the unwind. + // This is not a test of the unwinder and clang for arm seems to + // produces an increasing number of code that does not have unwind + // information. ASSERT_TRUE(backtrace->Unwind(0)) << "Failed on unwind " << i; - ASSERT_LT(1, backtrace->NumFrames()) << "Failed on unwind " << i; } running = false; thread.join(); diff --git a/libc/private/bionic_systrace.h b/libc/private/bionic_systrace.h index 6b11812bd..dbe173919 100644 --- a/libc/private/bionic_systrace.h +++ b/libc/private/bionic_systrace.h @@ -16,12 +16,8 @@ #pragma once -#include <cutils/trace.h> // For ATRACE_TAG_BIONIC. - #include "platform/bionic/macros.h" -static constexpr char kTraceTagsProp[] = "debug.atrace.tags.enableflags"; - // Tracing class for bionic. To begin a trace at a specified point: // ScopedTrace("Trace message"); // The trace will end when the contructor goes out of scope. @@ -37,9 +33,5 @@ class __LIBC_HIDDEN__ ScopedTrace { BIONIC_DISALLOW_COPY_AND_ASSIGN(ScopedTrace); }; -int get_trace_marker_fd(); -bool should_trace(const uint64_t enable_tags = ATRACE_TAG_BIONIC); -void output_trace(const char* message, const char event = 'B'); - void bionic_trace_begin(const char* message); void bionic_trace_end(); diff --git a/libc/system_properties/Android.bp b/libc/system_properties/Android.bp index 361fc7925..af8bda9f7 100644 --- a/libc/system_properties/Android.bp +++ b/libc/system_properties/Android.bp @@ -18,11 +18,9 @@ cc_library_static { "contexts_serialized.cpp", "prop_area.cpp", "prop_info.cpp", - "prop_trace.cpp", "system_properties.cpp", ], whole_static_libs: [ - "libc_bionic_systrace", "libpropertyinfoparser", ], header_libs: [ diff --git a/libc/system_properties/include/system_properties/prop_trace.h b/libc/system_properties/include/system_properties/prop_trace.h deleted file mode 100644 index 7c65a6d3f..000000000 --- a/libc/system_properties/include/system_properties/prop_trace.h +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright (C) 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. - */ - -#pragma once - -#include "platform/bionic/macros.h" - -#include "prop_info.h" - -// Tracing class for sysprop. To begin a trace at a specified point: -// SyspropTrace trace ("prop_name", "prop_value"); -// The trace will end when the constructor goes out of scope. -// For read-only properties (ro.*), also need to pass prop_info struct. - -enum class PropertyAction { - kPropertyFind = 0, - kPropertySet, - kPropertyGetReadOnly, - kPropertyGetReadWrite, -}; - -class __LIBC_HIDDEN__ SyspropTrace { - public: - explicit SyspropTrace(const char* prop_name, const char* prop_value, const prop_info* pi, - PropertyAction action); - ~SyspropTrace(); - - private: - const char* prop_name_; - const char* prop_value_; - const prop_info* prop_info_; - PropertyAction prop_action_; - bool output_trace_; - - BIONIC_DISALLOW_COPY_AND_ASSIGN(SyspropTrace); -}; diff --git a/libc/system_properties/prop_trace.cpp b/libc/system_properties/prop_trace.cpp deleted file mode 100644 index ac7ff9414..000000000 --- a/libc/system_properties/prop_trace.cpp +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Copyright (C) 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. - */ - -#include "system_properties/prop_trace.h" - -#include <errno.h> -#include <fcntl.h> -#include <stdio.h> -#include <stdlib.h> -#include <string.h> - -#include "private/CachedProperty.h" -#include "private/bionic_lock.h" -#include "private/bionic_systrace.h" - -#include <async_safe/log.h> -#include <cutils/trace.h> // For ATRACE_TAG_SYSPROP. - -#define PROP_TRACE_MSG_LENGTH 1024 - -static bool should_trace_prop(const char* prop_name) { - // Should not trace kTraceTagsProp to avoid infinite recursion. - // Because the following g_trace_enable_flags.Get() will get the property value - // of kTraceTagsProp again, which in turn invokes should_trace_prop() here. - if (prop_name == nullptr || !strcmp(prop_name, kTraceTagsProp)) { - return false; - } - - return should_trace(ATRACE_TAG_SYSPROP); -} - -static void sysprop_trace_end() { - int trace_marker_fd = get_trace_marker_fd(); - if (trace_marker_fd == -1) { - return; - } - - TEMP_FAILURE_RETRY(write(trace_marker_fd, "E|", 2)); -} - -static void get_sysprop_trace_end(const prop_info* pi, const char* prop_value, - bool read_only = false) { - const char* output_value; - char message[PROP_TRACE_MSG_LENGTH]; - - if (read_only) { - if (pi->is_long()) { - output_value = pi->long_value(); - } else { - output_value = pi->value; - } - } else { - output_value = prop_value; - } - - snprintf(message, sizeof(message), "prop_get: %s, value: %s", pi->name, - output_value ? output_value : "null_value"); - output_trace(message, 'E'); // 'E' for end. -} - -SyspropTrace::SyspropTrace(const char* prop_name, const char* prop_value, const prop_info* pi, - PropertyAction action) - : prop_name_(prop_name), - prop_value_(prop_value), - prop_info_(pi), - prop_action_(action), - output_trace_(false) { - if (!should_trace_prop(prop_name)) { - return; - } - - char message[PROP_TRACE_MSG_LENGTH]; - if (prop_action_ == PropertyAction::kPropertyFind) { - snprintf(message, sizeof(message), "prop_find: %s", prop_name_); - } else if (prop_action_ == PropertyAction::kPropertySet) { - snprintf(message, sizeof(message), "prop_set: %s, value: %s", prop_name_, - prop_value_ ? prop_value_ : "null_value"); - } else { - // For property get, the prop_value_ will be resolved then printed in the destructor. - snprintf(message, sizeof(message), "prop_get: %s", prop_name_); - } - - output_trace(message, 'B'); // 'B' for begin. - output_trace_ = true; -} - -SyspropTrace::~SyspropTrace() { - if (!output_trace_) { - return; - } - if (prop_action_ == PropertyAction::kPropertyFind || - prop_action_ == PropertyAction::kPropertySet) { - sysprop_trace_end(); - } else if (prop_action_ == PropertyAction::kPropertyGetReadOnly) { - get_sysprop_trace_end(prop_info_, prop_value_, true /* read_only */); - } else if (prop_action_ == PropertyAction::kPropertyGetReadWrite) { - get_sysprop_trace_end(prop_info_, prop_value_, false /* read_only */); - } - output_trace_ = false; -} diff --git a/libc/system_properties/system_properties.cpp b/libc/system_properties/system_properties.cpp index 344c8383f..1cb15c3df 100644 --- a/libc/system_properties/system_properties.cpp +++ b/libc/system_properties/system_properties.cpp @@ -46,7 +46,6 @@ #include "system_properties/context_node.h" #include "system_properties/prop_area.h" #include "system_properties/prop_info.h" -#include "system_properties/prop_trace.h" #define SERIAL_DIRTY(serial) ((serial)&1) #define SERIAL_VALUE_LEN(serial) ((serial) >> 24) @@ -128,9 +127,6 @@ const prop_info* SystemProperties::Find(const char* name) { return nullptr; } - SyspropTrace trace(name, nullptr /* prop_value */, nullptr /* prop_info */, - PropertyAction::kPropertyFind); - prop_area* pa = contexts_->GetPropAreaForName(name); if (!pa) { async_safe_format_log(ANDROID_LOG_WARN, "libc", "Access denied finding property \"%s\"", name); @@ -205,10 +201,6 @@ void SystemProperties::ReadCallback(const prop_info* pi, // Read only properties don't need to copy the value to a temporary buffer, since it can never // change. We use relaxed memory order on the serial load for the same reason. if (is_read_only(pi->name)) { - // The 2nd argument is not required for read-only property tracing, as the - // value can be obtained via pi->value or pi->long_value(). - SyspropTrace trace(pi->name, nullptr /* prop_value */, pi /* prop_info */, - PropertyAction::kPropertyGetReadOnly); uint32_t serial = load_const_atomic(&pi->serial, memory_order_relaxed); if (pi->is_long()) { callback(cookie, pi->name, pi->long_value(), serial); @@ -219,8 +211,6 @@ void SystemProperties::ReadCallback(const prop_info* pi, } char value_buf[PROP_VALUE_MAX]; - SyspropTrace trace(pi->name, value_buf, pi /* prop_info */, - PropertyAction::kPropertyGetReadWrite); uint32_t serial = ReadMutablePropertyValue(pi, value_buf); callback(cookie, pi->name, value_buf, serial); } |