From af756d78ac042a2eed2417c5250d4b675d43bf93 Mon Sep 17 00:00:00 2001 From: Adrian Kummerlaender Date: Wed, 17 Feb 2016 15:02:48 +0100 Subject: Implement static allocator for initialization The previous interposition logic based on plain usage of `dlsym` analogously to various online examples led to a deadlock during _neovim_ startup. This deadlock was caused by _neovim_'s custom memory allocation library _jemalloc_ because it calls `mmap` during its initialization phase. The problem with calling `mmap` during initialization is that this already leads to executing `libChangeLog`'s `mmap` version whoes static `actual_mmap` function pointer is not initialized at this point in time. This is detected and leads to a call to `dlsym` to remedy this situation. Sadly `dlsym` in turn requires memory allocation using `calloc` which leads us back to initializing _jemalloc_ and as such to a deadlock. I first saw this as a bug in _jemalloc_ which seemed to be confirmed by a short search in my search engine of choice. This prompted me to create an appropriate [bug report](https://github.com/jemalloc/jemalloc/issues/329) which was dismissed as a problem in the way `mmap` was interposed and not as a bug in the library. Thus it seems to be accepted practice that it is not the responsibility of a custom memory allocator to cater to the initialization needs of other libraries relying on function interposition. This is of course a valid position as the whole issue is a kind of _chicken and egg_ problem where both sides can be argued. To cut to the chase I was left with the only option of working around this deadlock by adapting `libChangeLog` to call `dlsym` without relying on the wrapped application's memory allocator of choice. The most straight forward way to do this is to provide another custom memory allocator alongside the _payload_ function interpositions of `mmap` and friends. `init/alloc.cc` implements such a selectively transparent memory allocator that offers a small static buffer for usage in the context of executing `dlsym`.The choice between forwarding memory allocation requests to the wrapped application's allocator and using the static buffer is governed by `init::dlsymContext`. This tiny helper class maintains an `dlsym_level` counter by posing as a scope guard. The end result of this extension to `libChangeLog` is that it now also works with applications using _jemalloc_ such as _neovim_ and should overall be much more robust during its initialization phase. --- CMakeLists.txt | 1 + src/actual.h | 10 +++++++--- src/change_log.cc | 45 +++++++++++++++++++++++++++++++++++++++++++-- src/init/alloc.cc | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/init/alloc.h | 22 ++++++++++++++++++++++ 5 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 src/init/alloc.cc create mode 100644 src/init/alloc.h diff --git a/CMakeLists.txt b/CMakeLists.txt index be9e91e..4dfd8df 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,6 +14,7 @@ add_library( ChangeLog SHARED src/change_log.cc + src/init/alloc.cc src/utility/logger.cc src/tracking/path_matcher.cc src/tracking/change_tracker.cc diff --git a/src/actual.h b/src/actual.h index cb08d42..a860590 100644 --- a/src/actual.h +++ b/src/actual.h @@ -1,10 +1,10 @@ +#ifndef CHANGE_SRC_ACTUAL_FUNCTION_H_ +#define CHANGE_SRC_ACTUAL_FUNCTION_H_ + #ifndef _GNU_SOURCE #define _GNU_SOURCE #endif -#ifndef CHANGE_SRC_ACTUAL_FUNCTION_H_ -#define CHANGE_SRC_ACTUAL_FUNCTION_H_ - #include #include #include @@ -12,6 +12,8 @@ #include #include +#include "init/alloc.h" + namespace actual { template @@ -19,6 +21,8 @@ using ptr = Result(*)(Arguments...); template FunctionPtr get_ptr(const std::string& symbol_name) { + init::dlsymContext guard; + const void* symbol_address{ dlsym(RTLD_NEXT, symbol_name.c_str()) }; FunctionPtr actual_function{}; diff --git a/src/change_log.cc b/src/change_log.cc index e0dc179..5f3b4f8 100644 --- a/src/change_log.cc +++ b/src/change_log.cc @@ -1,5 +1,6 @@ #include "actual.h" +#include "init/alloc.h" #include "utility/io.h" #include "utility/logger.h" #include "tracking/path_matcher.h" @@ -14,8 +15,8 @@ static std::unique_ptr logger; static std::unique_ptr matcher; static std::unique_ptr tracker; -void init() __attribute__ ((constructor)); -void init() { +void initialize() __attribute__ ((constructor)); +void initialize() { if ( getenv("CHANGE_LOG_TARGET") != NULL ) { fd_guard = std::make_unique( getenv("CHANGE_LOG_TARGET") @@ -81,6 +82,46 @@ inline void track_remove(const std::string& path) { } } +void free(void* ptr) { + static actual::ptr actual_free{}; + + if ( !actual_free ) { + actual_free = actual::get_ptr("free"); + } + + if ( !init::from_static_buffer(ptr) ) { + actual_free(ptr); + } +} + +void* malloc(size_t size) { + static actual::ptr actual_malloc{}; + + if ( init::dlsymContext::is_active() ) { + return init::static_malloc(size); + } else { + if ( !actual_malloc ) { + actual_malloc = actual::get_ptr("malloc"); + } + + return actual_malloc(size); + } +} + +void* calloc(size_t block, size_t size) { + static actual::ptr actual_calloc{}; + + if ( init::dlsymContext::is_active() ) { + return init::static_calloc(block, size); + } else { + if ( !actual_calloc ) { + actual_calloc = actual::get_ptr("calloc"); + } + + return actual_calloc(block, size); + } +} + ssize_t write(int fd, const void* buffer, size_t count) { static actual::ptr actual_write{}; diff --git a/src/init/alloc.cc b/src/init/alloc.cc new file mode 100644 index 0000000..d18917b --- /dev/null +++ b/src/init/alloc.cc @@ -0,0 +1,50 @@ +#include "alloc.h" + +#include +#include + +namespace init { + +std::uint8_t dlsym_level = 0; +size_t buffer_offset = 0; +char buffer[4096]; + +void* static_malloc(size_t size) { + if ( buffer_offset + size >= sizeof(buffer) ) { + std::fprintf(stderr, "static allocator out of memory: %lu\n", buffer_offset); + exit(1); + } + + buffer_offset += size; + + return buffer + buffer_offset; +} + +void* static_calloc(size_t block, size_t size) { + void* const mem = static_malloc(block * size); + + for ( size_t i = 0; i < (block * size); ++i ) { + *(static_cast(mem) + i) = '\0'; + } + + return mem; +} + +bool from_static_buffer(void* ptr) { + return static_cast(ptr) >= buffer + && static_cast(ptr) <= buffer + sizeof(buffer); +} + +dlsymContext::dlsymContext() { + ++dlsym_level; +} + +dlsymContext::~dlsymContext() { + --dlsym_level; +} + +bool dlsymContext::is_active() { + return dlsym_level > 0; +} + +} diff --git a/src/init/alloc.h b/src/init/alloc.h new file mode 100644 index 0000000..cd1228d --- /dev/null +++ b/src/init/alloc.h @@ -0,0 +1,22 @@ +#ifndef CHANGE_SRC_INIT_ALLOC_H_ +#define CHANGE_SRC_INIT_ALLOC_H_ + +#include + +namespace init { + +void* static_malloc(size_t size); +void* static_calloc(size_t block, size_t size); + +bool from_static_buffer(void* ptr); + +struct dlsymContext { + dlsymContext(); + ~dlsymContext(); + + static bool is_active(); +}; + +} + +#endif // CHANGE_SRC_INIT_ALLOC_H_ -- cgit v1.2.3