Fix minor bug in dispatch table initialization order.
Other minor changes:
* document assignment that relies on _Atomic assignments to use
atomic_store.
* consistently use atomic_store when assigning to atomics.
* remove incorrect comment.
Test: m
Test: flash & boot sailfish
Change-Id: I4789c08f7ac28a2de8d6925d03af354514bfd9d7
diff --git a/libc/bionic/malloc_common.cpp b/libc/bionic/malloc_common.cpp
index 8d8d420..9a80767 100644
--- a/libc/bionic/malloc_common.cpp
+++ b/libc/bionic/malloc_common.cpp
@@ -366,12 +366,6 @@
}
static bool InitMallocFunctions(void* impl_handler, MallocDispatch* table, const char* prefix) {
- // We initialize free first to prevent the following situation:
- // Heapprofd's MallocMalloc is installed, and an allocation is observed
- // and logged to the heap dump. The corresponding free happens before
- // heapprofd's MallocFree is installed, and is not logged in the heap
- // dump. This leads to the allocation wrongly being active in the heap
- // dump indefinitely.
if (!InitMallocFunction<MallocFree>(impl_handler, &table->free, prefix, "free")) {
return false;
}
@@ -572,6 +566,16 @@
}
atomic_store(&g_heapprofd_init_func, init_func);
+ // We assign free first explicitly to prevent the case where we observe a
+ // alloc, but miss the corresponding free because of initialization order.
+ //
+ // This is safer than relying on the declaration order inside
+ // MallocDispatch at the cost of an extra atomic pointer write on
+ // initialization.
+ atomic_store(&globals->malloc_dispatch.free, dispatch_table.free);
+ // The struct gets assigned elementwise and each of the elements is an
+ // _Atomic. Assigning to an _Atomic is an atomic_store operation.
+ // The assignment is done in declaration order.
globals->malloc_dispatch = dispatch_table;
info_log("%s: malloc %s enabled", getprogname(), prefix);
@@ -679,7 +683,7 @@
extern "C" void InstallInitHeapprofdHook(int) {
if (!atomic_exchange(&g_heapprofd_init_in_progress, true)) {
__libc_globals.mutate([](libc_globals* globals) {
- globals->malloc_dispatch.malloc = InitHeapprofdHook;
+ atomic_store(&globals->malloc_dispatch.malloc, InitHeapprofdHook);
});
}
}