Skip to content

Commit

Permalink
Guard against a null tracker singleton in hooks
Browse files Browse the repository at this point in the history 
Make the hooks resilient against the case where the tracker singleton is
a null pointer. It sort of worked even if they were called on a null
singleton, because most of these methods checked a static `isActive`
variable before accessing any member variables, but this is still
undefined behavior and we should still avoid it.
  • Loading branch information
@godlygeek
godlygeek committed Mar 26, 2022
1 parent b24d557 commit 1ababf2
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 28 deletions.
33 changes: 15 additions & 18 deletions src/bloomberg/pensieve/_pensieve/hooks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) noexc
{
assert(hooks::mmap);
void* ptr = hooks::mmap(addr, length, prot, flags, fd, offset);
tracking_api::Tracker::getTracker()->trackAllocation(ptr, length, hooks::Allocator::MMAP);
tracking_api::Tracker::trackAllocation(ptr, length, hooks::Allocator::MMAP);
return ptr;
}

Expand All @@ -97,15 +97,15 @@ mmap64(void* addr, size_t length, int prot, int flags, int fd, off_t offset) noe
{
assert(hooks::mmap64);
void* ptr = hooks::mmap64(addr, length, prot, flags, fd, offset);
tracking_api::Tracker::getTracker()->trackAllocation(ptr, length, hooks::Allocator::MMAP);
tracking_api::Tracker::trackAllocation(ptr, length, hooks::Allocator::MMAP);
return ptr;
}

int
munmap(void* addr, size_t length) noexcept
{
assert(hooks::munmap);
tracking_api::Tracker::getTracker()->trackDeallocation(addr, length, hooks::Allocator::MUNMAP);
tracking_api::Tracker::trackDeallocation(addr, length, hooks::Allocator::MUNMAP);
return hooks::munmap(addr, length);
}

Expand All @@ -115,7 +115,7 @@ malloc(size_t size) noexcept
assert(hooks::malloc);

void* ptr = hooks::malloc(size);
tracking_api::Tracker::getTracker()->trackAllocation(ptr, size, hooks::Allocator::MALLOC);
tracking_api::Tracker::trackAllocation(ptr, size, hooks::Allocator::MALLOC);
return ptr;
}

Expand All @@ -126,7 +126,7 @@ free(void* ptr) noexcept

// We need to call our API before we call the real free implementation
// to make sure that the pointer is not reused in-between.
tracking_api::Tracker::getTracker()->trackDeallocation(ptr, 0, hooks::Allocator::FREE);
tracking_api::Tracker::trackDeallocation(ptr, 0, hooks::Allocator::FREE);

hooks::free(ptr);
}
Expand All @@ -138,8 +138,8 @@ realloc(void* ptr, size_t size) noexcept

void* ret = hooks::realloc(ptr, size);
if (ret) {
tracking_api::Tracker::getTracker()->trackDeallocation(ptr, 0, hooks::Allocator::FREE);
tracking_api::Tracker::getTracker()->trackAllocation(ret, size, hooks::Allocator::REALLOC);
tracking_api::Tracker::trackDeallocation(ptr, 0, hooks::Allocator::FREE);
tracking_api::Tracker::trackAllocation(ret, size, hooks::Allocator::REALLOC);
}
return ret;
}
Expand All @@ -151,7 +151,7 @@ calloc(size_t num, size_t size) noexcept

void* ret = hooks::calloc(num, size);
if (ret) {
tracking_api::Tracker::getTracker()->trackAllocation(ret, num * size, hooks::Allocator::CALLOC);
tracking_api::Tracker::trackAllocation(ret, num * size, hooks::Allocator::CALLOC);
}
return ret;
}
Expand All @@ -163,10 +163,7 @@ posix_memalign(void** memptr, size_t alignment, size_t size) noexcept

int ret = hooks::posix_memalign(memptr, alignment, size);
if (!ret) {
tracking_api::Tracker::getTracker()->trackAllocation(
*memptr,
size,
hooks::Allocator::POSIX_MEMALIGN);
tracking_api::Tracker::trackAllocation(*memptr, size, hooks::Allocator::POSIX_MEMALIGN);
}
return ret;
}
Expand All @@ -178,7 +175,7 @@ memalign(size_t alignment, size_t size) noexcept

void* ret = hooks::memalign(alignment, size);
if (ret) {
tracking_api::Tracker::getTracker()->trackAllocation(ret, size, hooks::Allocator::MEMALIGN);
tracking_api::Tracker::trackAllocation(ret, size, hooks::Allocator::MEMALIGN);
}
return ret;
}
Expand All @@ -190,7 +187,7 @@ valloc(size_t size) noexcept

void* ret = hooks::valloc(size);
if (ret) {
tracking_api::Tracker::getTracker()->trackAllocation(ret, size, hooks::Allocator::VALLOC);
tracking_api::Tracker::trackAllocation(ret, size, hooks::Allocator::VALLOC);
}
return ret;
}
Expand All @@ -202,7 +199,7 @@ pvalloc(size_t size) noexcept

void* ret = hooks::pvalloc(size);
if (ret) {
tracking_api::Tracker::getTracker()->trackAllocation(ret, size, hooks::Allocator::PVALLOC);
tracking_api::Tracker::trackAllocation(ret, size, hooks::Allocator::PVALLOC);
}
return ret;
}
Expand All @@ -213,7 +210,7 @@ dlopen(const char* filename, int flag) noexcept
assert(hooks::dlopen);

void* ret = hooks::dlopen(filename, flag);
if (ret) tracking_api::Tracker::getTracker()->invalidate_module_cache();
if (ret) tracking_api::Tracker::invalidate_module_cache();
return ret;
}

Expand All @@ -224,7 +221,7 @@ dlclose(void* handle) noexcept

int ret = hooks::dlclose(handle);
tracking_api::NativeTrace::flushCache();
if (!ret) tracking_api::Tracker::getTracker()->invalidate_module_cache();
if (!ret) tracking_api::Tracker::invalidate_module_cache();
return ret;
}

Expand All @@ -241,7 +238,7 @@ prctl(int option, ...) noexcept

if (option == PR_SET_NAME) {
char* name = reinterpret_cast<char*>(args[0]);
tracking_api::Tracker::getTracker()->registerThreadName(name);
tracking_api::Tracker::registerThreadName(name);
}

unsigned long ret = hooks::prctl(option, args[0], args[1], args[2], args[3]);
Expand Down
10 changes: 5 additions & 5 deletions src/bloomberg/pensieve/_pensieve/tracking_api.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ Tracker::childFork()
}

void
Tracker::trackAllocation(void* ptr, size_t size, const hooks::Allocator func)
Tracker::trackAllocationImpl(void* ptr, size_t size, const hooks::Allocator func)
{
if (RecursionGuard::isActive || !Tracker::isActive()) {
return;
Expand Down Expand Up @@ -473,7 +473,7 @@ Tracker::trackAllocation(void* ptr, size_t size, const hooks::Allocator func)
}

void
Tracker::trackDeallocation(void* ptr, size_t size, const hooks::Allocator func)
Tracker::trackDeallocationImpl(void* ptr, size_t size, const hooks::Allocator func)
{
if (RecursionGuard::isActive || !Tracker::isActive()) {
return;
Expand All @@ -492,7 +492,7 @@ Tracker::trackDeallocation(void* ptr, size_t size, const hooks::Allocator func)
}

void
Tracker::invalidate_module_cache()
Tracker::invalidate_module_cache_impl()
{
RecursionGuard guard;
d_patcher.overwrite_symbols();
Expand Down Expand Up @@ -544,7 +544,7 @@ dl_iterate_phdr_callback(struct dl_phdr_info* info, [[maybe_unused]] size_t size
}

void
Tracker::updateModuleCache()
Tracker::updateModuleCacheImpl()
{
if (!d_unwind_native_frames) {
return;
Expand All @@ -559,7 +559,7 @@ Tracker::updateModuleCache()
}

void
Tracker::registerThreadName(const char* name)
Tracker::registerThreadNameImpl(const char* name)
{
if (!d_writer->writeRecord(RecordType::THREAD_RECORD, ThreadRecord{thread_id(), name})) {
std::cerr << "pensieve: Failed to write output, deactivating tracking" << std::endl;
Expand Down
52 changes: 47 additions & 5 deletions src/bloomberg/pensieve/_pensieve/tracking_api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,47 @@ class Tracker
static Tracker* getTracker();

// Allocation tracking interface
void trackAllocation(void* ptr, size_t size, hooks::Allocator func);
void trackDeallocation(void* ptr, size_t size, hooks::Allocator func);
void invalidate_module_cache();
void updateModuleCache();
void registerThreadName(const char* name);
__attribute__((always_inline)) inline static void
trackAllocation(void* ptr, size_t size, hooks::Allocator func)
{
Tracker* tracker = getTracker();
if (tracker) {
tracker->trackAllocationImpl(ptr, size, func);
}
}

__attribute__((always_inline)) inline static void
trackDeallocation(void* ptr, size_t size, hooks::Allocator func)
{
Tracker* tracker = getTracker();
if (tracker) {
tracker->trackDeallocationImpl(ptr, size, func);
}
}

__attribute__((always_inline)) inline static void invalidate_module_cache()
{
Tracker* tracker = getTracker();
if (tracker) {
tracker->invalidate_module_cache_impl();
}
}

__attribute__((always_inline)) inline static void updateModuleCache()
{
Tracker* tracker = getTracker();
if (tracker) {
tracker->updateModuleCacheImpl();
}
}

__attribute__((always_inline)) inline static void registerThreadName(const char* name)
{
Tracker* tracker = getTracker();
if (tracker) {
tracker->registerThreadNameImpl(name);
}
}

// RawFrame stack interface
bool pushFrame(const RawFrame& frame);
Expand Down Expand Up @@ -239,6 +275,12 @@ class Tracker
// Methods
frame_id_t registerFrame(const RawFrame& frame);

void trackAllocationImpl(void* ptr, size_t size, hooks::Allocator func);
void trackDeallocationImpl(void* ptr, size_t size, hooks::Allocator func);
void invalidate_module_cache_impl();
void updateModuleCacheImpl();
void registerThreadNameImpl(const char* name);

explicit Tracker(
std::unique_ptr<RecordWriter> record_writer,
bool native_traces,
Expand Down

0 comments on commit 1ababf2

Please sign in to comment.