aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libc/bionic/malloc_heapprofd.cpp21
-rw-r--r--libc/malloc_debug/malloc_debug.cpp49
-rw-r--r--libc/malloc_debug/tests/malloc_debug_system_tests.cpp70
-rw-r--r--libc/stdio/local.h1
-rw-r--r--libc/stdio/refill.c2
-rw-r--r--libc/stdio/stdio.cpp17
-rw-r--r--libc/stdlib/atexit.c3
-rw-r--r--libdl/Android.bp2
-rw-r--r--linker/Android.bp1
-rw-r--r--tests/stdio_test.cpp22
10 files changed, 161 insertions, 27 deletions
diff --git a/libc/bionic/malloc_heapprofd.cpp b/libc/bionic/malloc_heapprofd.cpp
index 2aeb9bfee..5d3735d27 100644
--- a/libc/bionic/malloc_heapprofd.cpp
+++ b/libc/bionic/malloc_heapprofd.cpp
@@ -143,22 +143,23 @@ static void MaybeInstallInitHeapprofdHook(int) {
}
}
+constexpr char kHeapprofdProgramPropertyPrefix[] = "heapprofd.enable.";
+constexpr size_t kHeapprofdProgramPropertyPrefixSize = sizeof(kHeapprofdProgramPropertyPrefix) - 1;
+constexpr size_t kMaxCmdlineSize = 512;
+
static bool GetHeapprofdProgramProperty(char* data, size_t size) {
- constexpr char prefix[] = "heapprofd.enable.";
- // - 1 to skip nullbyte, which we will write later.
- constexpr size_t prefix_size = sizeof(prefix) - 1;
- if (size < prefix_size) {
+ if (size < kHeapprofdProgramPropertyPrefixSize) {
error_log("%s: Overflow constructing heapprofd property", getprogname());
return false;
}
- memcpy(data, prefix, prefix_size);
+ memcpy(data, kHeapprofdProgramPropertyPrefix, kHeapprofdProgramPropertyPrefixSize);
int fd = open("/proc/self/cmdline", O_RDONLY | O_CLOEXEC);
if (fd == -1) {
error_log("%s: Failed to open /proc/self/cmdline", getprogname());
return false;
}
- char cmdline[128];
+ char cmdline[kMaxCmdlineSize];
ssize_t rd = read(fd, cmdline, sizeof(cmdline) - 1);
close(fd);
if (rd == -1) {
@@ -167,7 +168,7 @@ static bool GetHeapprofdProgramProperty(char* data, size_t size) {
}
cmdline[rd] = '\0';
char* first_arg = static_cast<char*>(memchr(cmdline, '\0', rd));
- if (first_arg == nullptr || first_arg == cmdline + size - 1) {
+ if (first_arg == nullptr) {
error_log("%s: Overflow reading cmdline", getprogname());
return false;
}
@@ -192,12 +193,12 @@ static bool GetHeapprofdProgramProperty(char* data, size_t size) {
}
size_t name_size = static_cast<size_t>(first_arg - start);
- if (name_size >= size - prefix_size) {
+ if (name_size >= size - kHeapprofdProgramPropertyPrefixSize) {
error_log("%s: overflow constructing heapprofd property.", getprogname());
return false;
}
// + 1 to also copy the trailing null byte.
- memcpy(data + prefix_size, start, name_size + 1);
+ memcpy(data + kHeapprofdProgramPropertyPrefixSize, start, name_size + 1);
return true;
}
@@ -213,7 +214,7 @@ bool HeapprofdShouldLoad() {
return true;
}
- char program_property[128];
+ char program_property[kHeapprofdProgramPropertyPrefixSize + kMaxCmdlineSize];
if (!GetHeapprofdProgramProperty(program_property,
sizeof(program_property))) {
return false;
diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp
index 91e1d2610..53fcead01 100644
--- a/libc/malloc_debug/malloc_debug.cpp
+++ b/libc/malloc_debug/malloc_debug.cpp
@@ -29,6 +29,7 @@
#include <errno.h>
#include <inttypes.h>
#include <malloc.h>
+#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -103,6 +104,32 @@ void* debug_valloc(size_t size);
__END_DECLS
// ------------------------------------------------------------------------
+class ScopedConcurrentLock {
+ public:
+ ScopedConcurrentLock() {
+ pthread_rwlock_rdlock(&lock_);
+ }
+ ~ScopedConcurrentLock() {
+ pthread_rwlock_unlock(&lock_);
+ }
+
+ static void Init() {
+ pthread_rwlockattr_t attr;
+ // Set the attribute so that when a write lock is pending, read locks are no
+ // longer granted.
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
+ pthread_rwlock_init(&lock_, &attr);
+ }
+
+ static void BlockAllOperations() {
+ pthread_rwlock_wrlock(&lock_);
+ }
+
+ private:
+ static pthread_rwlock_t lock_;
+};
+pthread_rwlock_t ScopedConcurrentLock::lock_;
+
static void InitAtfork() {
static pthread_once_t atfork_init = PTHREAD_ONCE_INIT;
pthread_once(&atfork_init, []() {
@@ -257,6 +284,8 @@ bool debug_initialize(const MallocDispatch* malloc_dispatch, bool* zygote_child,
info_log("%s: malloc debug enabled", getprogname());
}
+ ScopedConcurrentLock::Init();
+
return true;
}
@@ -265,6 +294,10 @@ void debug_finalize() {
return;
}
+ // Make sure that there are no other threads doing debug allocations
+ // before we kill everything.
+ ScopedConcurrentLock::BlockAllOperations();
+
// Turn off capturing allocations calls.
DebugDisableSet(true);
@@ -292,6 +325,8 @@ void debug_finalize() {
void debug_get_malloc_leak_info(uint8_t** info, size_t* overall_size, size_t* info_size,
size_t* total_memory, size_t* backtrace_size) {
+ ScopedConcurrentLock lock;
+
ScopedDisableDebugCalls disable;
// Verify the arguments.
@@ -325,6 +360,7 @@ size_t debug_malloc_usable_size(void* pointer) {
if (DebugCallsDisabled() || pointer == nullptr) {
return g_dispatch->malloc_usable_size(pointer);
}
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
if (!VerifyPointer(pointer, "malloc_usable_size")) {
@@ -388,6 +424,7 @@ void* debug_malloc(size_t size) {
if (DebugCallsDisabled()) {
return g_dispatch->malloc(size);
}
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
void* pointer = InternalMalloc(size);
@@ -463,6 +500,7 @@ void debug_free(void* pointer) {
if (DebugCallsDisabled() || pointer == nullptr) {
return g_dispatch->free(pointer);
}
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
if (g_debug->config().options() & RECORD_ALLOCS) {
@@ -480,6 +518,7 @@ void* debug_memalign(size_t alignment, size_t bytes) {
if (DebugCallsDisabled()) {
return g_dispatch->memalign(alignment, bytes);
}
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
if (bytes == 0) {
@@ -558,6 +597,7 @@ void* debug_realloc(void* pointer, size_t bytes) {
if (DebugCallsDisabled()) {
return g_dispatch->realloc(pointer, bytes);
}
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
if (pointer == nullptr) {
@@ -676,6 +716,7 @@ void* debug_calloc(size_t nmemb, size_t bytes) {
if (DebugCallsDisabled()) {
return g_dispatch->calloc(nmemb, bytes);
}
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
size_t size;
@@ -737,6 +778,8 @@ int debug_malloc_info(int options, FILE* fp) {
if (DebugCallsDisabled() || !g_debug->TrackPointers()) {
return g_dispatch->malloc_info(options, fp);
}
+ ScopedConcurrentLock lock;
+ ScopedDisableDebugCalls disable;
MallocXmlElem root(fp, "malloc", "version=\"debug-malloc-1\"");
std::vector<ListInfoType> list;
@@ -786,6 +829,7 @@ int debug_posix_memalign(void** memptr, size_t alignment, size_t size) {
int debug_iterate(uintptr_t base, size_t size, void (*callback)(uintptr_t, size_t, void*),
void* arg) {
+ ScopedConcurrentLock lock;
if (g_debug->TrackPointers()) {
// Since malloc is disabled, don't bother acquiring any locks.
for (auto it = PointerData::begin(); it != PointerData::end(); ++it) {
@@ -800,6 +844,7 @@ int debug_iterate(uintptr_t base, size_t size, void (*callback)(uintptr_t, size_
}
void debug_malloc_disable() {
+ ScopedConcurrentLock lock;
g_dispatch->malloc_disable();
if (g_debug->pointer) {
g_debug->pointer->PrepareFork();
@@ -807,6 +852,7 @@ void debug_malloc_disable() {
}
void debug_malloc_enable() {
+ ScopedConcurrentLock lock;
if (g_debug->pointer) {
g_debug->pointer->PostForkParent();
}
@@ -817,6 +863,7 @@ ssize_t debug_malloc_backtrace(void* pointer, uintptr_t* frames, size_t max_fram
if (DebugCallsDisabled() || pointer == nullptr) {
return 0;
}
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
if (!(g_debug->config().options() & BACKTRACE)) {
@@ -870,6 +917,7 @@ static void write_dump(FILE* fp) {
}
bool debug_write_malloc_leak_info(FILE* fp) {
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
std::lock_guard<std::mutex> guard(g_dump_lock);
@@ -883,6 +931,7 @@ bool debug_write_malloc_leak_info(FILE* fp) {
}
void debug_dump_heap(const char* file_name) {
+ ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
std::lock_guard<std::mutex> guard(g_dump_lock);
diff --git a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
index 71e8ebf73..f85c45b87 100644
--- a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
@@ -42,13 +42,15 @@
#include <log/log.h>
#include <string>
+#include <thread>
#include <vector>
#include "private/bionic_malloc.h"
-static constexpr time_t kTimeoutSeconds = 5;
+static constexpr time_t kTimeoutSeconds = 10;
-static void Exec(const char* test_name, const char* debug_options, pid_t* pid) {
+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));
@@ -94,7 +96,8 @@ static void Exec(const char* test_name, const char* debug_options, pid_t* pid) {
output.append(buffer.data(), bytes);
}
- if ((time(nullptr) - start_time) > kTimeoutSeconds) {
+ if ((time(nullptr) - start_time) > timeout_seconds) {
+ kill(*pid, SIGINT);
break;
}
}
@@ -109,7 +112,7 @@ static void Exec(const char* test_name, const char* debug_options, pid_t* pid) {
done = true;
break;
}
- if ((time(nullptr) - start_time) > kTimeoutSeconds) {
+ if ((time(nullptr) - start_time) > timeout_seconds) {
break;
}
}
@@ -119,21 +122,23 @@ static void Exec(const char* test_name, const char* debug_options, pid_t* pid) {
while (true) {
int kill_status;
int wait_pid = waitpid(*pid, &kill_status, WNOHANG);
- if (wait_pid == *pid || (time(nullptr) - start_time) > kTimeoutSeconds) {
+ if (wait_pid == *pid || (time(nullptr) - start_time) > timeout_seconds) {
break;
}
}
}
ASSERT_TRUE(done) << "Timed out waiting for waitpid, output:\n" << output;
- ASSERT_EQ(0, WEXITSTATUS(status)) << "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) {
+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_ID_MAIN, ANDROID_LOG_RDONLY | ANDROID_LOG_NONBLOCK, 1000, pid);
+ list = android_logger_list_open(log, ANDROID_LOG_RDONLY | ANDROID_LOG_NONBLOCK, 1000, pid);
ASSERT_TRUE(list != nullptr);
while (true) {
@@ -168,7 +173,8 @@ static void GetLogStr(pid_t pid, std::string* log_str) {
android_logger_list_close(list);
}
-static void FindStrings(pid_t pid, std::vector<const char*> match_strings) {
+static void FindStrings(pid_t pid, std::vector<const char*> match_strings,
+ time_t timeout_seconds = kTimeoutSeconds) {
std::string log_str;
time_t start = time(nullptr);
bool found_all;
@@ -184,7 +190,7 @@ static void FindStrings(pid_t pid, std::vector<const char*> match_strings) {
if (found_all) {
return;
}
- if ((time(nullptr) - start) > kTimeoutSeconds) {
+ if ((time(nullptr) - start) > timeout_seconds) {
break;
}
}
@@ -414,3 +420,47 @@ TEST(MallocDebugSystemTest, verify_leak) {
TEST(MallocDebugSystemTest, verify_leak_allocation_limit) {
VerifyLeak("leak_memory_limit_");
}
+
+static constexpr int kExpectedExitCode = 30;
+
+TEST(MallocTests, DISABLED_exit_while_threads_allocating) {
+ std::atomic_uint32_t thread_mask;
+ thread_mask = 0;
+
+ for (size_t i = 0; i < 32; i++) {
+ std::thread malloc_thread([&thread_mask, i] {
+ while (true) {
+ void* ptr = malloc(100);
+ if (ptr == nullptr) {
+ exit(1000);
+ }
+ free(ptr);
+ thread_mask.fetch_or(1 << i);
+ }
+ });
+ malloc_thread.detach();
+ }
+
+ // Wait until each thread has done at least one allocation.
+ while (thread_mask.load() != 0xffffffff)
+ ;
+ exit(kExpectedExitCode);
+}
+
+// Verify that exiting while other threads are doing malloc operations,
+// that there are no crashes.
+TEST(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));
+
+ 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;
+ }
+}
diff --git a/libc/stdio/local.h b/libc/stdio/local.h
index d306a2130..1ecf1220f 100644
--- a/libc/stdio/local.h
+++ b/libc/stdio/local.h
@@ -204,6 +204,7 @@ __LIBC32_LEGACY_PUBLIC__ int __sclose(void*);
__LIBC32_LEGACY_PUBLIC__ int _fwalk(int (*)(FILE*));
off64_t __sseek64(void*, off64_t, int);
+int __sflush_locked(FILE*);
int __swhatbuf(FILE*, size_t*, int*);
wint_t __fgetwc_unlock(FILE*);
wint_t __ungetwc(wint_t, FILE*);
diff --git a/libc/stdio/refill.c b/libc/stdio/refill.c
index cfa2bfd45..1df419180 100644
--- a/libc/stdio/refill.c
+++ b/libc/stdio/refill.c
@@ -40,7 +40,7 @@ static int
lflush(FILE *fp)
{
if ((fp->_flags & (__SLBF|__SWR)) == (__SLBF|__SWR))
- return (__sflush(fp)); /* ignored... */
+ return (__sflush_locked(fp)); /* ignored... */
return (0);
}
diff --git a/libc/stdio/stdio.cpp b/libc/stdio/stdio.cpp
index 4cec757e5..91c7689f8 100644
--- a/libc/stdio/stdio.cpp
+++ b/libc/stdio/stdio.cpp
@@ -106,7 +106,7 @@ FILE* stdin = &__sF[0];
FILE* stdout = &__sF[1];
FILE* stderr = &__sF[2];
-static pthread_mutex_t __stdio_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+static pthread_mutex_t __stdio_mutex = PTHREAD_MUTEX_INITIALIZER;
static uint64_t __get_file_tag(FILE* fp) {
// Don't use a tag for the standard streams.
@@ -211,21 +211,23 @@ found:
}
int _fwalk(int (*callback)(FILE*)) {
- pthread_mutex_lock(&__stdio_mutex);
int result = 0;
for (glue* g = &__sglue; g != nullptr; g = g->next) {
FILE* fp = g->iobs;
for (int n = g->niobs; --n >= 0; ++fp) {
- ScopedFileLock sfl(fp);
if (fp->_flags != 0 && (fp->_flags & __SIGN) == 0) {
result |= (*callback)(fp);
}
}
}
- pthread_mutex_unlock(&__stdio_mutex);
return result;
}
+extern "C" __LIBC_HIDDEN__ void __libc_stdio_cleanup(void) {
+ // Equivalent to fflush(nullptr), but without all the locking since we're shutting down anyway.
+ _fwalk(__sflush);
+}
+
static FILE* __fopen(int fd, int flags) {
#if !defined(__LP64__)
if (fd > SHRT_MAX) {
@@ -520,6 +522,11 @@ int __sflush(FILE* fp) {
return 0;
}
+int __sflush_locked(FILE* fp) {
+ ScopedFileLock sfl(fp);
+ return __sflush(fp);
+}
+
int __sread(void* cookie, char* buf, int n) {
FILE* fp = reinterpret_cast<FILE*>(cookie);
return TEMP_FAILURE_RETRY(read(fp->_file, buf, n));
@@ -1061,7 +1068,7 @@ int wscanf(const wchar_t* fmt, ...) {
}
static int fflush_all() {
- return _fwalk(__sflush);
+ return _fwalk(__sflush_locked);
}
int fflush(FILE* fp) {
diff --git a/libc/stdlib/atexit.c b/libc/stdlib/atexit.c
index 692e0c0fc..0efb11854 100644
--- a/libc/stdlib/atexit.c
+++ b/libc/stdlib/atexit.c
@@ -188,7 +188,8 @@ restart:
/* If called via exit(), flush output of all open files. */
if (dso == NULL) {
- fflush(NULL);
+ extern void __libc_stdio_cleanup(void);
+ __libc_stdio_cleanup();
}
/* BEGIN android-changed: call __unregister_atfork if dso is not null */
diff --git a/libdl/Android.bp b/libdl/Android.bp
index b1ee5ab22..fbbe7ba29 100644
--- a/libdl/Android.bp
+++ b/libdl/Android.bp
@@ -47,6 +47,7 @@ cc_library {
ldflags: [
"-Wl,--exclude-libs=libgcc.a",
+ "-Wl,--exclude-libs=libgcc_stripped.a",
"-Wl,--exclude-libs=libclang_rt.builtins-arm-android.a",
"-Wl,--exclude-libs=libclang_rt.builtins-aarch64-android.a",
"-Wl,--exclude-libs=libclang_rt.builtins-x86-android.a",
@@ -127,6 +128,7 @@ cc_library {
ldflags: [
"-Wl,--exclude-libs=libgcc.a",
+ "-Wl,--exclude-libs=libgcc_stripped.a",
"-Wl,--exclude-libs=libclang_rt.builtins-arm-android.a",
"-Wl,--exclude-libs=libclang_rt.builtins-aarch64-android.a",
"-Wl,--exclude-libs=libclang_rt.builtins-x86-android.a",
diff --git a/linker/Android.bp b/linker/Android.bp
index 73328dad2..728dec575 100644
--- a/linker/Android.bp
+++ b/linker/Android.bp
@@ -319,6 +319,7 @@ cc_library {
ldflags: [
"-Wl,--exclude-libs=libgcc.a",
+ "-Wl,--exclude-libs=libgcc_stripped.a",
"-Wl,--exclude-libs=libclang_rt.builtins-arm-android.a",
"-Wl,--exclude-libs=libclang_rt.builtins-aarch64-android.a",
"-Wl,--exclude-libs=libclang_rt.builtins-x86-android.a",
diff --git a/tests/stdio_test.cpp b/tests/stdio_test.cpp
index 65a942c37..c237d6d32 100644
--- a/tests/stdio_test.cpp
+++ b/tests/stdio_test.cpp
@@ -29,6 +29,7 @@
#include <locale.h>
#include <string>
+#include <thread>
#include <vector>
#include <android-base/file.h>
@@ -2577,3 +2578,24 @@ TEST(STDIO_TEST, dev_std_files) {
ASSERT_LT(0, length);
ASSERT_EQ("/proc/self/fd/2", std::string(path, length));
}
+
+TEST(STDIO_TEST, fread_with_locked_file) {
+ // Reading an unbuffered/line-buffered file from one thread shouldn't block on
+ // files locked on other threads, even if it flushes some line-buffered files.
+ FILE* fp1 = fopen("/dev/zero", "r");
+ ASSERT_TRUE(fp1 != nullptr);
+ flockfile(fp1);
+
+ std::thread([] {
+ for (int mode : { _IONBF, _IOLBF }) {
+ FILE* fp2 = fopen("/dev/zero", "r");
+ ASSERT_TRUE(fp2 != nullptr);
+ setvbuf(fp2, nullptr, mode, 0);
+ ASSERT_EQ('\0', fgetc(fp2));
+ fclose(fp2);
+ }
+ }).join();
+
+ funlockfile(fp1);
+ fclose(fp1);
+}
generated by cgit v1.2.3 (git 2.25.1) at 2024-05-21 02:59:02 +0000