Merge "Reland "Fix GWP hooks not being restored, leading to crashes.""
diff --git a/libc/Android.bp b/libc/Android.bp
index aa5d435..fbf7ec2 100644
--- a/libc/Android.bp
+++ b/libc/Android.bp
@@ -1202,6 +1202,7 @@
},
},
whole_static_libs: [
+ "libc_bionic_systrace",
"libsystemproperties",
],
cppflags: ["-Wold-style-cast"],
@@ -1858,7 +1859,9 @@
cc_library_headers {
name: "libc_llndk_headers",
- visibility: ["//visibility:private"],
+ visibility: [
+ "//external/musl",
+ ],
llndk: {
llndk_headers: true,
},
@@ -1978,8 +1981,12 @@
static: {
system_shared_libs: [],
},
- shared: {
- system_shared_libs: ["libc"],
+ target: {
+ bionic: {
+ shared: {
+ system_shared_libs: ["libc"],
+ },
+ },
},
//TODO (dimitry): This is to work around b/24465209. Remove after root cause is fixed
@@ -2070,7 +2077,7 @@
cc_defaults {
name: "crt_defaults",
defaults: ["crt_and_memtag_defaults"],
- default_shared_libs: [],
+ system_shared_libs: [],
}
cc_defaults {
diff --git a/libc/NOTICE b/libc/NOTICE
index 26fef05..8260112 100644
--- a/libc/NOTICE
+++ b/libc/NOTICE
@@ -855,22 +855,6 @@
-------------------------------------------------------------------
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.
-
--------------------------------------------------------------------
-
-Copyright (C) 2020 The Android Open Source Project
All rights reserved.
Redistribution and use in source and binary forms, with or without
@@ -899,6 +883,22 @@
-------------------------------------------------------------------
Copyright (C) 2021 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.
+
+-------------------------------------------------------------------
+
+Copyright (C) 2021 The Android Open Source Project
All rights reserved.
Redistribution and use in source and binary forms, with or without
diff --git a/libc/bionic/bionic_systrace.cpp b/libc/bionic/bionic_systrace.cpp
index 0de51c5..fd97712 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 @@
// 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/strerror.cpp b/libc/bionic/strerror.cpp
index 5733567..0deb200 100644
--- a/libc/bionic/strerror.cpp
+++ b/libc/bionic/strerror.cpp
@@ -196,8 +196,7 @@
length = async_safe_format_buffer(buf, buf_len, "Unknown error %d", error_number);
}
if (length >= buf_len) {
- errno_restorer.override(ERANGE);
- return -1;
+ return ERANGE;
}
return 0;
diff --git a/libc/bionic/system_property_set.cpp b/libc/bionic/system_property_set.cpp
index 6823b6a..212aafc 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 @@
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/tests/malloc_debug_system_tests.cpp b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
index b20cef7..59729d0 100644
--- a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
@@ -29,6 +29,7 @@
#include <errno.h>
#include <fcntl.h>
#include <poll.h>
+#include <setjmp.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
@@ -44,6 +45,7 @@
#include <log/log_read.h>
#include <atomic>
+#include <mutex>
#include <string>
#include <thread>
#include <vector>
@@ -57,8 +59,6 @@
// is enabled. These tests don't run be default, and are executed
// by wrappers that will enable various malloc debug features.
-static constexpr time_t kTimeoutSeconds = 10;
-
extern "C" bool GetInitialArgs(const char*** args, size_t* num_args) {
static const char* initial_args[] = {"--slow_threshold_ms=30000",
"--deadline_threshold_ms=1200000"};
@@ -67,171 +67,381 @@
return true;
}
-static void Exec(const char* test_name, const char* debug_options, pid_t* pid, int exit_code = 0,
- time_t timeout_seconds = kTimeoutSeconds) {
- int fds[2];
- ASSERT_NE(-1, pipe(fds));
- ASSERT_NE(-1, fcntl(fds[0], F_SETFL, O_NONBLOCK));
- if ((*pid = fork()) == 0) {
- ASSERT_EQ(0, setenv("LIBC_DEBUG_MALLOC_OPTIONS", debug_options, 1));
- close(fds[0]);
- close(STDIN_FILENO);
- close(STDOUT_FILENO);
- close(STDERR_FILENO);
- ASSERT_NE(0, dup2(fds[1], STDOUT_FILENO));
- ASSERT_NE(0, dup2(fds[1], STDERR_FILENO));
-
- std::vector<const char*> args;
- // Get a copy of this argument so it doesn't disappear on us.
- std::string exec(testing::internal::GetArgvs()[0]);
- args.push_back(exec.c_str());
- args.push_back("--gtest_also_run_disabled_tests");
- std::string filter_arg = std::string("--gtest_filter=") + test_name;
- args.push_back(filter_arg.c_str());
- // Need this because some code depends on exit codes from the test run
- // but the isolation runner does not support that.
- args.push_back("--no_isolate");
- args.push_back(nullptr);
- execv(args[0], reinterpret_cast<char* const*>(const_cast<char**>(args.data())));
- exit(20);
- }
- ASSERT_NE(-1, *pid);
- close(fds[1]);
-
- std::string output;
- std::vector<char> buffer(4096);
- time_t start_time = time(nullptr);
- bool done = false;
- while (true) {
- struct pollfd read_fd = {.fd = fds[0], .events = POLLIN};
- if (TEMP_FAILURE_RETRY(poll(&read_fd, 1, 1)) > 0) {
- ssize_t bytes = TEMP_FAILURE_RETRY(read(fds[0], buffer.data(), sizeof(buffer) - 1));
- if (bytes == -1 && errno == EAGAIN) {
- continue;
+class LogReader {
+ public:
+ LogReader(pid_t pid, log_id log) {
+ std::call_once(log_start_time_flag_, []() {
+ // Use this to figure out the point at which to start grabbing the log.
+ // This avoids accidentally grabbing data from a previous process with
+ // the same pid.
+ log_start_time_ = {};
+ logger_list* list = android_logger_list_open(LOG_ID_MAIN, ANDROID_LOG_NONBLOCK, 1000, 0);
+ if (list == nullptr) {
+ return;
}
- ASSERT_NE(-1, bytes);
- if (bytes == 0) {
+ log_msg log_msg;
+ int ret = android_logger_list_read(list, &log_msg);
+ android_logger_list_close(list);
+ if (ret <= 0) {
+ return;
+ }
+ log_start_time_.tv_sec = log_msg.entry.sec;
+ log_start_time_.tv_nsec = log_msg.entry.nsec;
+ });
+
+ std::call_once(jmp_data_key_flag_, []() {
+ pthread_key_create(&jmp_data_key_, [](void* ptr) { free(ptr); });
+ signal(SIGUSR1, [](int) {
+ jmp_buf* jb = reinterpret_cast<jmp_buf*>(pthread_getspecific(jmp_data_key_));
+ if (jb != nullptr) {
+ // The thread reading the log is in a blocking read call that
+ // cannot be interrupted. In order to get out of this read loop,
+ // it's necessary to call longjmp when a SIGUSR1 signal is sent
+ // to the thread.
+ longjmp(*jb, 1);
+ }
+ });
+ });
+
+ reader_.reset(new std::thread([this, pid, log] {
+ tid_.store(gettid(), std::memory_order_release);
+ logger_list* list;
+ while (true) {
+ // Do not use non-blocking mode so that the two threads
+ // are essentially asleep and not consuming any cpu.
+ list = android_logger_list_open(log, 0, 1000, pid);
+ if (list != nullptr) {
+ break;
+ }
+ // Wait for a short time for the log to become available.
+ usleep(1000);
+ }
+
+ jmp_buf* jb = reinterpret_cast<jmp_buf*>(malloc(sizeof(jmp_buf)));
+ if (jb == nullptr) {
+ printf("Failed to allocate memory for jmp_buf\n");
+ return;
+ }
+ pthread_setspecific(jmp_data_key_, jb);
+ if (setjmp(*jb) != 0) {
+ // SIGUSR1 signal hit, we need to terminate the thread.
+ android_logger_list_free(list);
+ return;
+ }
+
+ while (true) {
+ log_msg msg;
+ int actual = android_logger_list_read(list, &msg);
+ if (actual < 0) {
+ if (actual == -EINTR) {
+ // Interrupted retry.
+ continue;
+ }
+ // Unknown error.
+ break;
+ } else if (actual == 0) {
+ // Nothing left to read.
+ break;
+ }
+ // Do not allow SIGUSR1 while processing the log message.
+ // This avoids a deadlock if the thread is being terminated
+ // at this moment.
+ sigset64_t mask_set;
+ sigprocmask64(SIG_SETMASK, nullptr, &mask_set);
+ sigaddset64(&mask_set, SIGUSR1);
+ sigprocmask64(SIG_SETMASK, &mask_set, nullptr);
+
+ {
+ // Lock while appending to the data.
+ std::lock_guard<std::mutex> guard(data_lock_);
+ char* msg_str = msg.msg();
+ // Make sure the message is not empty and recent.
+ if (msg_str != nullptr && (msg.entry.sec > log_start_time_.tv_sec ||
+ (msg.entry.sec == log_start_time_.tv_sec &&
+ msg.entry.nsec > log_start_time_.tv_nsec))) {
+ // Skip the tag part of the message.
+ char* tag = msg_str + 1;
+ msg_str = tag + strlen(tag) + 1;
+ log_data_ += msg_str;
+ if (log_data_.back() != '\n') {
+ log_data_ += '\n';
+ }
+ }
+ }
+
+ // Re-enable SIGUSR1
+ sigprocmask64(SIG_SETMASK, nullptr, &mask_set);
+ sigdelset64(&mask_set, SIGUSR1);
+ sigprocmask64(SIG_SETMASK, &mask_set, nullptr);
+ }
+ android_logger_list_free(list);
+ }));
+ }
+
+ virtual ~LogReader() {
+ tgkill(getpid(), tid_.load(std::memory_order_acquire), SIGUSR1);
+ reader_->join();
+ }
+
+ std::string GetLog() {
+ std::lock_guard<std::mutex> guard(data_lock_);
+ return log_data_;
+ }
+
+ private:
+ std::unique_ptr<std::thread> reader_;
+ std::string log_data_;
+ std::mutex data_lock_;
+ std::atomic<pid_t> tid_;
+
+ static std::once_flag jmp_data_key_flag_;
+ static pthread_key_t jmp_data_key_;
+
+ static std::once_flag log_start_time_flag_;
+ static log_time log_start_time_;
+};
+
+std::once_flag LogReader::jmp_data_key_flag_;
+pthread_key_t LogReader::jmp_data_key_;
+
+std::once_flag LogReader::log_start_time_flag_;
+log_time LogReader::log_start_time_;
+
+class MallocDebugSystemTest : public ::testing::Test {
+ protected:
+ void SetUp() override {
+ expected_log_strings_.clear();
+ unexpected_log_strings_.clear();
+
+ // All tests expect this message to be present.
+ expected_log_strings_.push_back("malloc debug enabled");
+ }
+
+ void Exec(const char* test_name, const char* debug_options, int expected_exit_code = 0) {
+ for (size_t i = 0; i < kMaxRetries; i++) {
+ ASSERT_NO_FATAL_FAILURE(InternalExec(test_name, debug_options, expected_exit_code));
+
+ // Due to log messages sometimes getting lost, if a log message
+ // is not present, allow retrying the test.
+ std::string error_msg;
+ if (!CheckExpectedLogStrings(&error_msg)) {
+ ASSERT_NE(i, kMaxRetries - 1) << error_msg;
+ usleep(1000);
+ }
+
+ // This doesn't need to be retried since if the log message is
+ // present, that is an immediate fail.
+ ASSERT_NO_FATAL_FAILURE(VerifyUnexpectedLogStrings());
+ }
+ }
+
+ void InternalExec(const char* test_name, const char* debug_options, int expected_exit_code) {
+ int fds[2];
+ ASSERT_NE(-1, pipe(fds));
+ ASSERT_NE(-1, fcntl(fds[0], F_SETFL, O_NONBLOCK));
+ if ((pid_ = fork()) == 0) {
+ ASSERT_EQ(0, setenv("LIBC_DEBUG_MALLOC_OPTIONS", debug_options, 1));
+ close(fds[0]);
+ close(STDIN_FILENO);
+ close(STDOUT_FILENO);
+ close(STDERR_FILENO);
+ ASSERT_NE(0, dup2(fds[1], STDOUT_FILENO));
+ ASSERT_NE(0, dup2(fds[1], STDERR_FILENO));
+
+ std::vector<const char*> args;
+ // Get a copy of this argument so it doesn't disappear on us.
+ std::string exec(testing::internal::GetArgvs()[0]);
+ args.push_back(exec.c_str());
+ args.push_back("--gtest_also_run_disabled_tests");
+ std::string filter_arg = std::string("--gtest_filter=") + test_name;
+ args.push_back(filter_arg.c_str());
+ // Need this because some code depends on exit codes from the test run
+ // but the isolation runner does not support that.
+ args.push_back("--no_isolate");
+ args.push_back(nullptr);
+ execv(args[0], reinterpret_cast<char* const*>(const_cast<char**>(args.data())));
+ exit(20);
+ }
+ ASSERT_NE(-1, pid_);
+ close(fds[1]);
+
+ // Create threads to read the log output from the forked process as
+ // soon as possible in case there is something flooding the log.
+ log_main_.reset(new LogReader(pid_, LOG_ID_MAIN));
+ log_crash_.reset(new LogReader(pid_, LOG_ID_CRASH));
+
+ output_.clear();
+ std::vector<char> buffer(4096);
+ time_t start_time = time(nullptr);
+ bool read_done = false;
+ while (true) {
+ struct pollfd read_fd = {.fd = fds[0], .events = POLLIN};
+ if (TEMP_FAILURE_RETRY(poll(&read_fd, 1, 1)) > 0) {
+ ssize_t bytes = TEMP_FAILURE_RETRY(read(fds[0], buffer.data(), sizeof(buffer) - 1));
+ if (bytes == -1 && errno == EAGAIN) {
+ continue;
+ }
+ ASSERT_NE(-1, bytes);
+ if (bytes == 0) {
+ read_done = true;
+ break;
+ }
+ output_.append(buffer.data(), bytes);
+ }
+
+ if ((time(nullptr) - start_time) > kReadOutputTimeoutSeconds) {
+ kill(pid_, SIGINT);
+ break;
+ }
+ }
+
+ bool done = false;
+ int status;
+ start_time = time(nullptr);
+ while (true) {
+ int wait_pid = waitpid(pid_, &status, WNOHANG);
+ if (pid_ == wait_pid) {
done = true;
break;
}
- output.append(buffer.data(), bytes);
+ if ((time(nullptr) - start_time) > kWaitpidTimeoutSeconds) {
+ break;
+ }
+ }
+ if (!done) {
+ kill(pid_, SIGKILL);
+ start_time = time(nullptr);
+ while (true) {
+ int kill_status;
+ int wait_pid = waitpid(pid_, &kill_status, WNOHANG);
+ if (wait_pid == pid_ || (time(nullptr) - start_time) > kWaitpidTimeoutSeconds) {
+ break;
+ }
+ }
}
- if ((time(nullptr) - start_time) > timeout_seconds) {
- kill(*pid, SIGINT);
- break;
- }
- }
- EXPECT_TRUE(done) << "Timed out while reading data, output:\n" << output;
+ // Check timeout conditions first.
+ ASSERT_TRUE(read_done) << "Timed out while reading data, output:\n" << output_;
+ ASSERT_TRUE(done) << "Timed out waiting for waitpid, output:\n" << output_;
- done = false;
- int status;
- start_time = time(nullptr);
- while (true) {
- int wait_pid = waitpid(*pid, &status, WNOHANG);
- if (*pid == wait_pid) {
- done = true;
- break;
- }
- if ((time(nullptr) - start_time) > timeout_seconds) {
- break;
- }
+ ASSERT_FALSE(WIFSIGNALED(status)) << "Failed with signal " << WTERMSIG(status) << "\nOutput:\n"
+ << output_;
+ ASSERT_EQ(expected_exit_code, WEXITSTATUS(status)) << "Output:\n" << output_;
}
- if (!done) {
- kill(*pid, SIGKILL);
- start_time = time(nullptr);
+
+ bool CheckExpectedLogStrings(std::string* error_msg) {
+ time_t start = time(nullptr);
+ std::string missing_match;
+ std::string log_str;
while (true) {
- int kill_status;
- int wait_pid = waitpid(*pid, &kill_status, WNOHANG);
- if (wait_pid == *pid || (time(nullptr) - start_time) > timeout_seconds) {
+ log_str = log_main_->GetLog();
+ missing_match.clear();
+ // Look for the expected strings.
+ for (auto str : expected_log_strings_) {
+ if (log_str.find(str) == std::string::npos) {
+ missing_match = str;
+ break;
+ }
+ }
+ if (missing_match.empty()) {
+ return true;
+ }
+ if ((time(nullptr) - start) > kLogTimeoutSeconds) {
break;
}
}
+
+ *error_msg = android::base::StringPrintf("Didn't find string '%s' in log output:\n%s",
+ missing_match.c_str(), log_str.c_str());
+ return false;
+ }
+
+ void VerifyUnexpectedLogStrings() {
+ std::string log_str = log_main_->GetLog();
+ for (auto str : unexpected_log_strings_) {
+ ASSERT_TRUE(log_str.find(str) == std::string::npos)
+ << "Unexpectedly found string '" << str << "' in log output:\n"
+ << log_str;
+ }
}
- ASSERT_TRUE(done) << "Timed out waiting for waitpid, output:\n" << output;
- ASSERT_FALSE(WIFSIGNALED(status))
- << "Failed with signal " << WTERMSIG(status) << "\nOutput:\n" << output;
- ASSERT_EQ(exit_code, WEXITSTATUS(status)) << "Output:\n" << output;
-}
-
-static void GetLogStr(pid_t pid, std::string* log_str, log_id log = LOG_ID_MAIN) {
- log_str->clear();
-
- logger_list* list;
- list = android_logger_list_open(log, ANDROID_LOG_NONBLOCK, 1000, pid);
- ASSERT_TRUE(list != nullptr);
-
- while (true) {
- log_msg msg;
- ssize_t actual = android_logger_list_read(list, &msg);
- if (actual < 0) {
- if (actual == -EINTR) {
- // Interrupted retry.
- continue;
- } else if (actual == -EAGAIN) {
- // Non-blocking EOF, finished.
- break;
- } else {
- break;
+ void VerifyLeak(const char* test_prefix) {
+ struct FunctionInfo {
+ const char* name;
+ size_t size;
+ };
+ static FunctionInfo functions[] = {
+ {
+ "aligned_alloc",
+ 1152,
+ },
+ {
+ "calloc",
+ 1123,
+ },
+ {
+ "malloc",
+ 1123,
+ },
+ {
+ "memalign",
+ 1123,
+ },
+ {
+ "posix_memalign",
+ 1123,
+ },
+ {
+ "reallocarray",
+ 1123,
+ },
+ {
+ "realloc",
+ 1123,
+ },
+#if !defined(__LP64__)
+ {
+ "pvalloc",
+ 4096,
+ },
+ {
+ "valloc",
+ 1123,
}
- } else if (actual == 0) {
- break;
- }
- ASSERT_EQ(msg.entry.pid, pid);
+#endif
+ };
- char* msg_str = msg.msg();
- if (msg_str != nullptr) {
- char* tag = msg_str + 1;
- msg_str = tag + strlen(tag) + 1;
- *log_str += msg_str;
- if (log_str->back() != '\n') {
- *log_str += '\n';
- }
+ size_t match_len = expected_log_strings_.size() + 1;
+ expected_log_strings_.resize(match_len);
+ for (size_t i = 0; i < sizeof(functions) / sizeof(FunctionInfo); i++) {
+ SCOPED_TRACE(testing::Message()
+ << functions[i].name << " expected size " << functions[i].size);
+
+ expected_log_strings_[match_len - 1] =
+ android::base::StringPrintf("leaked block of size %zu at", functions[i].size);
+
+ std::string test = std::string("MallocTests.DISABLED_") + test_prefix + functions[i].name;
+ ASSERT_NO_FATAL_FAILURE(Exec(test.c_str(), "verbose backtrace leak_track"));
}
}
- android_logger_list_close(list);
-}
+ std::unique_ptr<LogReader> log_main_;
+ std::unique_ptr<LogReader> log_crash_;
+ pid_t pid_;
+ std::string output_;
+ std::vector<std::string> expected_log_strings_;
+ std::vector<std::string> unexpected_log_strings_;
-static void FindStrings(pid_t pid, std::vector<const char*> match_strings,
- std::vector<const char*> no_match_strings = std::vector<const char*>{},
- time_t timeout_seconds = kTimeoutSeconds) {
- std::string log_str;
- time_t start = time(nullptr);
- std::string missing_match;
- while (true) {
- GetLogStr(pid, &log_str);
- missing_match.clear();
- // Look for the expected strings.
- for (auto str : match_strings) {
- if (log_str.find(str) == std::string::npos) {
- missing_match = str;
- break;
- }
- }
-
- // Verify the unexpected strings are not present.
- for (auto str : no_match_strings) {
- ASSERT_TRUE(log_str.find(str) == std::string::npos) << "Unexpectedly found '" << str << "' in log output:\n" << log_str;
- }
- if (missing_match.empty()) {
- return;
- }
- if ((time(nullptr) - start) > timeout_seconds) {
- break;
- }
- }
- ASSERT_EQ("", missing_match) << "Didn't find expected log output:\n" << log_str;
-}
+ static constexpr size_t kReadOutputTimeoutSeconds = 180;
+ static constexpr size_t kWaitpidTimeoutSeconds = 10;
+ static constexpr size_t kLogTimeoutSeconds = 5;
+ static constexpr size_t kMaxRetries = 3;
+};
TEST(MallocTests, DISABLED_smoke) {}
-TEST(MallocDebugSystemTest, smoke) {
- pid_t pid;
- ASSERT_NO_FATAL_FAILURE(Exec("MallocTests.DISABLED_smoke", "verbose backtrace", &pid));
-
- ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"}));
+TEST_F(MallocDebugSystemTest, smoke) {
+ Exec("MallocTests.DISABLED_smoke", "verbose backtrace");
}
static void SetAllocationLimit() {
@@ -383,69 +593,11 @@
}
#endif
-static void VerifyLeak(const char* test_prefix) {
- struct FunctionInfo {
- const char* name;
- size_t size;
- };
- static FunctionInfo functions[] = {
- {
- "aligned_alloc",
- 1152,
- },
- {
- "calloc",
- 1123,
- },
- {
- "malloc",
- 1123,
- },
- {
- "memalign",
- 1123,
- },
- {
- "posix_memalign",
- 1123,
- },
- {
- "reallocarray",
- 1123,
- },
- {
- "realloc",
- 1123,
- },
-#if !defined(__LP64__)
- {
- "pvalloc",
- 4096,
- },
- {
- "valloc",
- 1123,
- }
-#endif
- };
-
- for (size_t i = 0; i < sizeof(functions) / sizeof(FunctionInfo); i++) {
- pid_t pid;
- SCOPED_TRACE(testing::Message() << functions[i].name << " expected size " << functions[i].size);
- std::string test = std::string("MallocTests.DISABLED_") + test_prefix + functions[i].name;
- ASSERT_NO_FATAL_FAILURE(Exec(test.c_str(), "verbose backtrace leak_track", &pid));
-
- std::string expected_leak = android::base::StringPrintf("leaked block of size %zu at", functions[i].size);
- EXPECT_NO_FATAL_FAILURE(FindStrings(
- pid, std::vector<const char*>{"malloc debug enabled", expected_leak.c_str()}));
- }
-}
-
-TEST(MallocDebugSystemTest, verify_leak) {
+TEST_F(MallocDebugSystemTest, verify_leak) {
VerifyLeak("leak_memory_");
}
-TEST(MallocDebugSystemTest, verify_leak_allocation_limit) {
+TEST_F(MallocDebugSystemTest, verify_leak_allocation_limit) {
VerifyLeak("leak_memory_limit_");
}
@@ -477,19 +629,15 @@
// Verify that exiting while other threads are doing malloc operations,
// that there are no crashes.
-TEST(MallocDebugSystemTest, exit_while_threads_allocating) {
+TEST_F(MallocDebugSystemTest, exit_while_threads_allocating) {
for (size_t i = 0; i < 100; i++) {
SCOPED_TRACE(::testing::Message() << "Run " << i);
- pid_t pid;
ASSERT_NO_FATAL_FAILURE(Exec("MallocTests.DISABLED_exit_while_threads_allocating",
- "verbose backtrace", &pid, kExpectedExitCode));
+ "verbose backtrace", 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);
+ std::string log_str = log_crash_->GetLog();
ASSERT_TRUE(log_str.find("Fatal signal") == std::string::npos)
- << "Found crash in log.\nLog message: " << log_str;
+ << "Found crash in log.\nLog message: " << log_str << " pid: " << pid_;
}
}
@@ -528,21 +676,17 @@
exit(kExpectedExitCode);
}
-TEST(MallocDebugSystemTest, exit_while_threads_freeing_allocs_with_header) {
+TEST_F(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));
+ 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);
+ std::string log_str = log_crash_->GetLog();
ASSERT_TRUE(log_str.find("Fatal signal") == std::string::npos)
- << "Found crash in log.\nLog message: " << log_str;
+ << "Found crash in log.\nLog message: " << log_str << " pid: " << pid_;
}
}
@@ -571,21 +715,18 @@
free(ptr);
}
-TEST(MallocDebugSystemTest, write_leak_info_no_header) {
- pid_t pid;
- ASSERT_NO_FATAL_FAILURE(Exec("MallocTests.DISABLED_write_leak_info", "verbose backtrace", &pid, 0));
-
- ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"},
-
- std::vector<const char*>{" HAS INVALID TAG ", "USED AFTER FREE ", "UNKNOWN POINTER "}));
+TEST_F(MallocDebugSystemTest, write_leak_info_no_header) {
+ unexpected_log_strings_.push_back(" HAS INVALID TAG ");
+ unexpected_log_strings_.push_back("USED AFTER FREE ");
+ unexpected_log_strings_.push_back("UNKNOWN POINTER ");
+ Exec("MallocTests.DISABLED_write_leak_info", "verbose backtrace");
}
-TEST(MallocDebugSystemTest, write_leak_info_header) {
- pid_t pid;
- ASSERT_NO_FATAL_FAILURE(Exec("MallocTests.DISABLED_write_leak_info", "verbose backtrace guard", &pid, 0));
-
- ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"},
- std::vector<const char*>{" HAS INVALID TAG ", "USED AFTER FREE ", "UNKNOWN POINTER "}));
+TEST_F(MallocDebugSystemTest, write_leak_info_header) {
+ unexpected_log_strings_.push_back(" HAS INVALID TAG ");
+ unexpected_log_strings_.push_back("USED AFTER FREE ");
+ unexpected_log_strings_.push_back("UNKNOWN POINTER ");
+ Exec("MallocTests.DISABLED_write_leak_info", "verbose backtrace guard");
}
TEST(MallocTests, DISABLED_malloc_and_backtrace_deadlock) {
@@ -619,13 +760,9 @@
thread.join();
}
-TEST(MallocDebugSystemTest, malloc_and_backtrace_deadlock) {
- pid_t pid;
- ASSERT_NO_FATAL_FAILURE(Exec("MallocTests.DISABLED_malloc_and_backtrace_deadlock",
- "verbose verify_pointers", &pid, 0, 180));
-
+TEST_F(MallocDebugSystemTest, malloc_and_backtrace_deadlock) {
// Make sure that malloc debug is enabled and that no timeouts occur during
// unwinds.
- ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"},
- std::vector<const char*>{"Timed out waiting for "}));
+ unexpected_log_strings_.push_back("Timed out waiting for ");
+ Exec("MallocTests.DISABLED_malloc_and_backtrace_deadlock", "verbose verify_pointers", 0);
}
diff --git a/libc/private/bionic_systrace.h b/libc/private/bionic_systrace.h
index 6b11812..dbe1739 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 @@
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/rust/Android.bp b/libc/rust/Android.bp
new file mode 100644
index 0000000..44cf848
--- /dev/null
+++ b/libc/rust/Android.bp
@@ -0,0 +1,30 @@
+rust_bindgen {
+ name: "libsystem_properties_bindgen",
+ wrapper_src: "system_properties_bindgen.hpp",
+ crate_name: "system_properties_bindgen",
+ source_stem: "bindings",
+
+ bindgen_flags: [
+ "--size_t-is-usize",
+ "--allowlist-function=__system_property_find",
+ "--allowlist-function=__system_property_read_callback",
+ "--allowlist-function=__system_property_set",
+ "--allowlist-function=__system_property_wait",
+ ],
+}
+
+rust_library {
+ name: "libsystem_properties-rust",
+ crate_name: "system_properties",
+ srcs: [
+ "system_properties.rs",
+ ],
+ rustlibs: [
+ "libanyhow",
+ "libsystem_properties_bindgen",
+ "libthiserror",
+ ],
+ shared_libs: [
+ "libbase",
+ ],
+}
diff --git a/libc/rust/system_properties.rs b/libc/rust/system_properties.rs
new file mode 100644
index 0000000..189e8ee
--- /dev/null
+++ b/libc/rust/system_properties.rs
@@ -0,0 +1,227 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+//! This crate provides the PropertyWatcher type, which watches for changes
+//! in Android system properties.
+
+use anyhow::{anyhow, Context, Result as AnyhowResult};
+use system_properties_bindgen::prop_info as PropInfo;
+use std::os::raw::c_char;
+use std::ptr::null;
+use std::{
+ ffi::{c_void, CStr, CString},
+ str::Utf8Error,
+};
+use thiserror::Error;
+
+/// Errors this crate can generate
+#[derive(Error, Debug)]
+pub enum PropertyWatcherError {
+ /// We can't watch for a property whose name contains a NUL character.
+ #[error("Cannot convert name to C string")]
+ BadNameError(#[from] std::ffi::NulError),
+ /// We can only watch for properties that exist when the watcher is created.
+ #[error("System property is absent")]
+ SystemPropertyAbsent,
+ /// __system_property_wait timed out despite being given no timeout.
+ #[error("Wait failed")]
+ WaitFailed,
+ /// read callback was not called
+ #[error("__system_property_read_callback did not call callback")]
+ ReadCallbackNotCalled,
+ /// read callback gave us a NULL pointer
+ #[error("__system_property_read_callback gave us a NULL pointer instead of a string")]
+ MissingCString,
+ /// read callback gave us a bad C string
+ #[error("__system_property_read_callback gave us a non-UTF8 C string")]
+ BadCString(#[from] Utf8Error),
+ /// read callback returned an error
+ #[error("Callback failed")]
+ CallbackError(#[from] anyhow::Error),
+ /// Failure in setting the system property
+ #[error("__system_property_set failed.")]
+ SetPropertyFailed,
+}
+
+/// Result type specific for this crate.
+pub type Result<T> = std::result::Result<T, PropertyWatcherError>;
+
+/// PropertyWatcher takes the name of an Android system property such
+/// as `keystore.boot_level`; it can report the current value of this
+/// property, or wait for it to change.
+pub struct PropertyWatcher {
+ prop_name: CString,
+ prop_info: *const PropInfo,
+ serial: system_properties_bindgen::__uint32_t,
+}
+
+impl PropertyWatcher {
+ /// Create a PropertyWatcher for the named system property.
+ pub fn new(name: &str) -> Result<Self> {
+ Ok(Self { prop_name: CString::new(name)?, prop_info: null(), serial: 0 })
+ }
+
+ // Lazy-initializing accessor for self.prop_info.
+ fn get_prop_info(&mut self) -> Option<*const PropInfo> {
+ if self.prop_info.is_null() {
+ // Unsafe required for FFI call. Input and output are both const.
+ // The returned pointer is valid for the lifetime of the program.
+ self.prop_info = unsafe {
+ system_properties_bindgen::__system_property_find(self.prop_name.as_ptr())
+ };
+ }
+ if self.prop_info.is_null() {
+ None
+ } else {
+ Some(self.prop_info)
+ }
+ }
+
+ fn read_raw(prop_info: *const PropInfo, mut f: impl FnOnce(Option<&CStr>, Option<&CStr>)) {
+ // Unsafe function converts values passed to us by
+ // __system_property_read_callback to Rust form
+ // and pass them to inner callback.
+ unsafe extern "C" fn callback(
+ res_p: *mut c_void,
+ name: *const c_char,
+ value: *const c_char,
+ _: system_properties_bindgen::__uint32_t,
+ ) {
+ let name = if name.is_null() { None } else { Some(CStr::from_ptr(name)) };
+ let value = if value.is_null() { None } else { Some(CStr::from_ptr(value)) };
+ let f = &mut *res_p.cast::<&mut dyn FnMut(Option<&CStr>, Option<&CStr>)>();
+ f(name, value);
+ }
+
+ let mut f: &mut dyn FnOnce(Option<&CStr>, Option<&CStr>) = &mut f;
+
+ // Unsafe block for FFI call. We convert the FnOnce
+ // to a void pointer, and unwrap it in our callback.
+ unsafe {
+ system_properties_bindgen::__system_property_read_callback(
+ prop_info,
+ Some(callback),
+ &mut f as *mut _ as *mut c_void,
+ )
+ }
+ }
+
+ /// Call the passed function, passing it the name and current value
+ /// of this system property. See documentation for
+ /// `__system_property_read_callback` for details.
+ /// Returns an error if the property is empty or doesn't exist.
+ pub fn read<T, F>(&mut self, f: F) -> Result<T>
+ where
+ F: FnOnce(&str, &str) -> anyhow::Result<T>,
+ {
+ let prop_info = self.get_prop_info().ok_or(PropertyWatcherError::SystemPropertyAbsent)?;
+ let mut result = Err(PropertyWatcherError::ReadCallbackNotCalled);
+ Self::read_raw(prop_info, |name, value| {
+ // use a wrapping closure as an erzatz try block.
+ result = (|| {
+ let name = name.ok_or(PropertyWatcherError::MissingCString)?.to_str()?;
+ let value = value.ok_or(PropertyWatcherError::MissingCString)?.to_str()?;
+ f(name, value).map_err(PropertyWatcherError::CallbackError)
+ })()
+ });
+ result
+ }
+
+ // Waits for the property that self is watching to be created. Returns immediately if the
+ // property already exists.
+ fn wait_for_property_creation(&mut self) -> Result<()> {
+ let mut global_serial = 0;
+ loop {
+ match self.get_prop_info() {
+ Some(_) => return Ok(()),
+ None => {
+ // Unsafe call for FFI. The function modifies only global_serial, and has
+ // no side-effects.
+ if !unsafe {
+ // Wait for a global serial number change, then try again. On success,
+ // the function will update global_serial with the last version seen.
+ system_properties_bindgen::__system_property_wait(
+ null(),
+ global_serial,
+ &mut global_serial,
+ null(),
+ )
+ } {
+ return Err(PropertyWatcherError::WaitFailed);
+ }
+ }
+ }
+ }
+ }
+
+ /// Wait for the system property to change. This
+ /// records the serial number of the last change, so
+ /// race conditions are avoided.
+ pub fn wait(&mut self) -> Result<()> {
+ // If the property is null, then wait for it to be created. Subsequent waits will
+ // skip this step and wait for our specific property to change.
+ if self.prop_info.is_null() {
+ return self.wait_for_property_creation();
+ }
+
+ let mut new_serial = self.serial;
+ // Unsafe block to call __system_property_wait.
+ // All arguments are private to PropertyWatcher so we
+ // can be confident they are valid.
+ if !unsafe {
+ system_properties_bindgen::__system_property_wait(
+ self.prop_info,
+ self.serial,
+ &mut new_serial,
+ null(),
+ )
+ } {
+ return Err(PropertyWatcherError::WaitFailed);
+ }
+ self.serial = new_serial;
+ Ok(())
+ }
+}
+
+/// Reads a system property.
+pub fn read(name: &str) -> AnyhowResult<String> {
+ PropertyWatcher::new(name)
+ .context("Failed to create a PropertyWatcher.")?
+ .read(|_name, value| Ok(value.to_owned()))
+ .with_context(|| format!("Failed to read the system property {}.", name))
+}
+
+/// Writes a system property.
+pub fn write(name: &str, value: &str) -> AnyhowResult<()> {
+ if
+ // Unsafe required for FFI call. Input and output are both const and valid strings.
+ unsafe {
+ // If successful, __system_property_set returns 0, otherwise, returns -1.
+ system_properties_bindgen::__system_property_set(
+ CString::new(name)
+ .context("Failed to construct CString from name.")?
+ .as_ptr(),
+ CString::new(value)
+ .context("Failed to construct CString from value.")?
+ .as_ptr(),
+ )
+ } == 0
+ {
+ Ok(())
+ } else {
+ Err(anyhow!(PropertyWatcherError::SetPropertyFailed))
+ }
+}
diff --git a/libc/rust/system_properties_bindgen.hpp b/libc/rust/system_properties_bindgen.hpp
new file mode 100644
index 0000000..307cd6c
--- /dev/null
+++ b/libc/rust/system_properties_bindgen.hpp
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2021 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 "sys/system_properties.h"
diff --git a/libc/system_properties/Android.bp b/libc/system_properties/Android.bp
index 361fc79..af8bda9 100644
--- a/libc/system_properties/Android.bp
+++ b/libc/system_properties/Android.bp
@@ -18,11 +18,9 @@
"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 7c65a6d..0000000
--- 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 ac7ff94..0000000
--- 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 344c838..1cb15c3 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 @@
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 @@
// 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 @@
}
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);
}
diff --git a/libm/Android.bp b/libm/Android.bp
index 6c3abd1..52229b6 100644
--- a/libm/Android.bp
+++ b/libm/Android.bp
@@ -509,7 +509,11 @@
tidy_checks: ["-cert-dcl16-c"],
include_dirs: ["bionic/libc"],
- system_shared_libs: ["libc"],
+ target: {
+ bionic: {
+ system_shared_libs: ["libc"],
+ },
+ },
sanitize: {
address: false,
diff --git a/tests/Android.bp b/tests/Android.bp
index 476b8f5..0f9af41 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -406,6 +406,7 @@
"string_nofortify_test.cpp",
"string_test.cpp",
"string_posix_strerror_r_test.cpp",
+ "string_posix_strerror_r_wrapper.cpp",
"strings_nofortify_test.cpp",
"strings_test.cpp",
"struct_layout_test.cpp",
diff --git a/tests/string_posix_strerror_r_test.cpp b/tests/string_posix_strerror_r_test.cpp
index 596684b..c4757ae 100644
--- a/tests/string_posix_strerror_r_test.cpp
+++ b/tests/string_posix_strerror_r_test.cpp
@@ -14,51 +14,40 @@
* limitations under the License.
*/
-#undef _GNU_SOURCE
-#include <features.h> // Get __BIONIC__ or __GLIBC__ so we can tell what we're using.
-
-#if defined(__GLIBC__)
-
-// At the time of writing, libcxx -- which is dragged in by gtest -- assumes
-// declarations from glibc of things that aren't available without _GNU_SOURCE.
-// This means we can't even build this test (which is a problem because that
-// means it doesn't get included in CTS).
-// For glibc 2.15, the symbols in question are:
-// at_quick_exit, quick_exit, vasprintf, strtoll_l, strtoull_l, and strtold_l.
-
-# if __GLIBC_PREREQ(2, 19)
-# error check whether we can build this now...
-# endif
-
-#else
-
-#include <string.h>
-
#include <errno.h>
#include <gtest/gtest.h>
+// Defined in string_posix_strerror_r_wrapper.cpp as a wrapper around the posix
+// strerror_r to work around an incompatibility between libc++ (required by
+// gtest) and !_GNU_SOURCE.
+int posix_strerror_r(int errnum, char* buf, size_t buflen);
+
TEST(string, posix_strerror_r) {
char buf[256];
// Valid.
- ASSERT_EQ(0, strerror_r(0, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(0, buf, sizeof(buf)));
ASSERT_STREQ("Success", buf);
- ASSERT_EQ(0, strerror_r(1, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(1, buf, sizeof(buf)));
ASSERT_STREQ("Operation not permitted", buf);
+#if defined(__BIONIC__)
// Invalid.
- ASSERT_EQ(0, strerror_r(-1, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(-1, buf, sizeof(buf)));
ASSERT_STREQ("Unknown error -1", buf);
- ASSERT_EQ(0, strerror_r(1234, buf, sizeof(buf)));
+ ASSERT_EQ(0, posix_strerror_r(1234, buf, sizeof(buf)));
ASSERT_STREQ("Unknown error 1234", buf);
+#else
+ // glibc returns EINVAL for unknown errors
+ ASSERT_EQ(EINVAL, posix_strerror_r(-1, buf, sizeof(buf)));
+ ASSERT_EQ(EINVAL, posix_strerror_r(1234, buf, sizeof(buf)));
+#endif
// Buffer too small.
errno = 0;
memset(buf, 0, sizeof(buf));
- ASSERT_EQ(-1, strerror_r(4567, buf, 2));
- ASSERT_STREQ("U", buf);
- // The POSIX strerror_r sets errno to ERANGE (the GNU one doesn't).
- ASSERT_EQ(ERANGE, errno);
+ ASSERT_EQ(ERANGE, posix_strerror_r(EPERM, buf, 2));
+ ASSERT_STREQ("O", buf);
+ // POSIX strerror_r returns an error without updating errno.
+ ASSERT_EQ(0, errno);
}
-
-#endif
diff --git a/tests/string_posix_strerror_r_wrapper.cpp b/tests/string_posix_strerror_r_wrapper.cpp
new file mode 100644
index 0000000..78d5d90
--- /dev/null
+++ b/tests/string_posix_strerror_r_wrapper.cpp
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+#undef _GNU_SOURCE
+#include <string.h>
+
+// At the time of writing, libcxx -- which is dragged in by gtest -- assumes
+// declarations from glibc of things that aren't available without _GNU_SOURCE.
+// This means we can't even build a test that directly calls the posix
+// strerror_r. Add a wrapper in a separate file that doesn't use any gtest.
+// For glibc 2.15, the symbols in question are:
+// at_quick_exit, quick_exit, vasprintf, strtoll_l, strtoull_l, and strtold_l.
+
+int posix_strerror_r(int errnum, char* buf, size_t buflen) {
+ return strerror_r(errnum, buf, buflen);
+}