From 8ca35b577ed306dd8c12f0d7046c9d0cd2d742e9 Mon Sep 17 00:00:00 2001 From: yairgott Date: Fri, 31 Jan 2025 02:12:45 +0000 Subject: [PATCH] Fixing grpc crash The issue stemmed from gRPC's dependency on 16-byte aligned memory addresses. The fix ensures all memory allocations enforce 16-byte alignment. --- .bazelrc | 4 +- MODULE.bazel | 2 +- vmsdk/.bazelrc | 42 ---------- vmsdk/src/memory_allocation_overrides.cc | 99 +++++++++++++++--------- vmsdk/src/memory_allocation_overrides.h | 36 +++------ 5 files changed, 77 insertions(+), 106 deletions(-) delete mode 100644 vmsdk/.bazelrc diff --git a/.bazelrc b/.bazelrc index 6e67f6f..dbb90c8 100644 --- a/.bazelrc +++ b/.bazelrc @@ -29,7 +29,9 @@ build --copt=-mfma build --copt=-ffp-contract=off build --copt=-flax-vector-conversions -build -c opt +build:debug --copt=-O0 --copt=-fno-omit-frame-pointer --copt=-DNDEBUG + +#build -c opt test --copt=-O1 test --compilation_mode=fastbuild diff --git a/MODULE.bazel b/MODULE.bazel index 4b7c4d4..5994e5a 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -6,7 +6,7 @@ bazel_dep(name = "rules_cc", version = "0.0.16") bazel_dep(name = "abseil-cpp", version = "20240722.0.bcr.1", repo_name = "com_google_absl") bazel_dep(name = "protobuf", version = "29.2", repo_name = "com_google_protobuf") bazel_dep(name = "boringssl", version = "0.20241024.0") -bazel_dep(name = "grpc", version = "1.63.1.bcr.1", repo_name = "com_github_grpc_grpc") +bazel_dep(name = "grpc", version = "1.69.0", repo_name = "com_github_grpc_grpc") bazel_dep(name = "re2", version = "2024-07-02", repo_name = "com_googlesource_code_re2") # rename to com_google_re2 bazel_dep(name = "rules_proto", version = "7.1.0") bazel_dep(name = "googletest", version = "1.15.2", dev_dependency = True, repo_name = "com_google_googletest") diff --git a/vmsdk/.bazelrc b/vmsdk/.bazelrc deleted file mode 100644 index ac1c742..0000000 --- a/vmsdk/.bazelrc +++ /dev/null @@ -1,42 +0,0 @@ -build --cxxopt=-std=c++20 --cxxopt=-fno-stack-protector --copt=-Wno-narrowing --copt=-fno-exceptions --cxxopt=-Wno-unused-private-field --cxxopt=-Wno-defaulted-function-deleted - -build --host_cxxopt=-std=c++20 --host_cxxopt=-fno-stack-protector --host_copt=-Wno-narrowing --host_copt=-fno-exceptions --host_cxxopt=-Wno-unused-private-field --host_cxxopt=-Wno-defaulted-function-deleted - -# Remove security hardening as it clashes with nolibc -build --copt -U_FORTIFY_SOURCE -build --host_copt -U_FORTIFY_SOURCE - -build --copt=-fPIC - -# Developer/CI config with more compiler warnings: --config=dev -build:dev --copt=-Wall -build:dev --copt=-Werror - -# ASan: --config=asan -build:asan --//gwpsan:sanitizer=asan -build:asan --copt=-fsanitize=address -build:asan --copt=-fsanitize-address-use-after-scope -build:asan --copt=-fsanitize-address-use-after-return=runtime -build:asan --copt=-DADDRESS_SANITIZER -build:asan --linkopt=-fsanitize=address -build:asan --cc_output_directory_tag=asan - -# MSan: --config=msan (Note: need MSan'ified stdlibs!) -build:msan --//gwpsan:sanitizer=msan -build:msan --copt=-fsanitize=memory -build:msan --copt=-fsanitize-memory-track-origins -build:msan --copt=-DMEMORY_SANITIZER -build:msan --linkopt=-fsanitize=memory -build:msan --cc_output_directory_tag=msan - -# ASan-enabled fuzzer: --config=asan-libfuzzer -build:asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine=@rules_fuzzing//fuzzing/engines:libfuzzer -build:asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine_instrumentation=libfuzzer -build:asan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine_sanitizer=asan -build:asan-libfuzzer --cc_output_directory_tag=asan-libfuzzer - -# MSan-enabled fuzzer: --config=msan-libfuzzer -build:msan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine=@rules_fuzzing//fuzzing/engines:libfuzzer -build:msan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine_instrumentation=libfuzzer -build:msan-libfuzzer --@rules_fuzzing//fuzzing:cc_engine_sanitizer=msan -build:msan-libfuzzer --cc_output_directory_tag=msan-libfuzzer \ No newline at end of file diff --git a/vmsdk/src/memory_allocation_overrides.cc b/vmsdk/src/memory_allocation_overrides.cc index a393b76..53185c1 100644 --- a/vmsdk/src/memory_allocation_overrides.cc +++ b/vmsdk/src/memory_allocation_overrides.cc @@ -82,17 +82,18 @@ class SystemAllocTracker { // infinite recursion when tracking pointers. template struct RawSystemAllocator { + // NOLINTNEXTLINE typedef T value_type; - RawSystemAllocator() noexcept {} + RawSystemAllocator() = default; template constexpr RawSystemAllocator(const RawSystemAllocator&) noexcept {} - + // NOLINTNEXTLINE T* allocate(std::size_t n) { ReportAllocMemorySize(n * sizeof(T)); return static_cast(__real_malloc(n * sizeof(T))); } - + // NOLINTNEXTLINE void deallocate(T* p, std::size_t) { ReportFreeMemorySize(sizeof(T)); __real_free(p); @@ -158,6 +159,18 @@ extern "C" { // Our allocator doesn't support tracking system memory size, so we just // return 0. __attribute__((weak)) size_t empty_usable_size(void* ptr) noexcept { return 0; } + +// For Valkey allocation - we need to ensure alignment by taking advantage of +// jemalloc alignment properties, as there is no aligned malloc module +// function. +// +// "... Chunks are always aligned to multiples of the chunk size..." +// +// See https://linux.die.net/man/3/jemalloc +size_t AlignSize(size_t size, int alignment = 16) { + return (size + alignment - 1) & ~(alignment - 1); +} + void* __wrap_malloc(size_t size) noexcept { if (!vmsdk::IsUsingValkeyAlloc()) { auto ptr = @@ -165,7 +178,9 @@ void* __wrap_malloc(size_t size) noexcept { vmsdk::SystemAllocTracker::GetInstance().TrackPointer(ptr); return ptr; } - return vmsdk::PerformAndTrackMalloc(size, RedisModule_Alloc, + // Forcing 16-byte alignment in Valkey, which may otherwise return 8-byte + // aligned memory. + return vmsdk::PerformAndTrackMalloc(AlignSize(size), RedisModule_Alloc, RedisModule_MallocUsableSize); } void __wrap_free(void* ptr) noexcept { @@ -185,6 +200,7 @@ void __wrap_free(void* ptr) noexcept { RedisModule_MallocUsableSize); } } +// NOLINTNEXTLINE void* __wrap_calloc(size_t __nmemb, size_t size) noexcept { if (!vmsdk::IsUsingValkeyAlloc()) { auto ptr = vmsdk::PerformAndTrackCalloc(__nmemb, size, __real_calloc, @@ -195,13 +211,17 @@ void* __wrap_calloc(size_t __nmemb, size_t size) noexcept { return vmsdk::PerformAndTrackCalloc(__nmemb, size, RedisModule_Calloc, RedisModule_MallocUsableSize); } + void* __wrap_realloc(void* ptr, size_t size) noexcept { bool was_tracked = false; if (ptr != nullptr) { was_tracked = vmsdk::SystemAllocTracker::GetInstance().UntrackPointer(ptr); } if (vmsdk::IsUsingValkeyAlloc() && !was_tracked) { - return vmsdk::PerformAndTrackRealloc(ptr, size, RedisModule_Realloc, + // Forcing 16-byte alignment in Valkey, which may otherwise return 8-byte + // aligned memory. + return vmsdk::PerformAndTrackRealloc(ptr, AlignSize(size), + RedisModule_Realloc, RedisModule_MallocUsableSize); } else { auto new_ptr = vmsdk::PerformAndTrackRealloc(ptr, size, __real_realloc, @@ -210,6 +230,7 @@ void* __wrap_realloc(void* ptr, size_t size) noexcept { return new_ptr; } } +// NOLINTNEXTLINE void* __wrap_aligned_alloc(size_t __alignment, size_t __size) noexcept { if (!vmsdk::IsUsingValkeyAlloc()) { auto ptr = vmsdk::PerformAndTrackAlignedAlloc( @@ -218,23 +239,18 @@ void* __wrap_aligned_alloc(size_t __alignment, size_t __size) noexcept { return ptr; } - // For Valkey allocation - we need to ensure alignment by taking advantage of - // jemalloc alignment properties, as there is no aligned malloc module - // function. - // - // "... Chunks are always aligned to multiples of the chunk size..." - // - // See https://linux.die.net/man/3/jemalloc - size_t new_size = (__size + __alignment - 1) & ~(__alignment - 1); - return vmsdk::PerformAndTrackMalloc(new_size, RedisModule_Alloc, + return vmsdk::PerformAndTrackMalloc(AlignSize(__size, __alignment), + RedisModule_Alloc, RedisModule_MallocUsableSize); } + int __wrap_malloc_usable_size(void* ptr) noexcept { if (vmsdk::SystemAllocTracker::GetInstance().IsTracked(ptr)) { return empty_usable_size(ptr); } return RedisModule_MallocUsableSize(ptr); } +// NOLINTNEXTLINE int __wrap_posix_memalign(void** r, size_t __alignment, size_t __size) { *r = __wrap_aligned_alloc(__alignment, __size); return 0; @@ -245,28 +261,35 @@ void* __wrap_valloc(size_t size) noexcept { } // extern "C" -size_t get_new_alloc_size(size_t __sz) { - if (__sz == 0) return 1; - return __sz; +size_t GetNewAllocSize(size_t size) { + if (size == 0) { + return 1; + } + return size; } -void* operator new(size_t __sz) noexcept(false) { - return __wrap_malloc(get_new_alloc_size(__sz)); +void* operator new(size_t size) noexcept(false) { + return __wrap_malloc(GetNewAllocSize(size)); } void operator delete(void* p) noexcept { __wrap_free(p); } -void operator delete(void* p, size_t __sz) noexcept { __wrap_free(p); } -void* operator new[](size_t __sz) noexcept(false) { +void operator delete(void* p, size_t size) noexcept { __wrap_free(p); } +void* operator new[](size_t size) noexcept(false) { // A non-null pointer is expected to be returned even if size = 0. - if (__sz == 0) __sz++; - return __wrap_malloc(__sz); + if (size == 0) { + size++; + } + return __wrap_malloc(size); } void operator delete[](void* p) noexcept { __wrap_free(p); } -void operator delete[](void* p, size_t __sz) noexcept { __wrap_free(p); } -void* operator new(size_t __sz, const std::nothrow_t& nt) noexcept { - return __wrap_malloc(get_new_alloc_size(__sz)); +// NOLINTNEXTLINE +void operator delete[](void* p, size_t size) noexcept { __wrap_free(p); } +// NOLINTNEXTLINE +void* operator new(size_t size, const std::nothrow_t& nt) noexcept { + return __wrap_malloc(GetNewAllocSize(size)); } -void* operator new[](size_t __sz, const std::nothrow_t& nt) noexcept { - return __wrap_malloc(get_new_alloc_size(__sz)); +// NOLINTNEXTLINE +void* operator new[](size_t size, const std::nothrow_t& nt) noexcept { + return __wrap_malloc(GetNewAllocSize(size)); } void operator delete(void* p, const std::nothrow_t& nt) noexcept { __wrap_free(p); @@ -274,14 +297,14 @@ void operator delete(void* p, const std::nothrow_t& nt) noexcept { void operator delete[](void* p, const std::nothrow_t& nt) noexcept { __wrap_free(p); } -void* operator new(size_t __sz, std::align_val_t alignment) noexcept(false) { +void* operator new(size_t size, std::align_val_t alignment) noexcept(false) { return __wrap_aligned_alloc(static_cast(alignment), - get_new_alloc_size(__sz)); + GetNewAllocSize(size)); } -void* operator new(size_t __sz, std::align_val_t alignment, +void* operator new(size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept { return __wrap_aligned_alloc(static_cast(alignment), - get_new_alloc_size(__sz)); + GetNewAllocSize(size)); } void operator delete(void* p, std::align_val_t alignment) noexcept { __wrap_free(p); @@ -290,18 +313,18 @@ void operator delete(void* p, std::align_val_t alignment, const std::nothrow_t&) noexcept { __wrap_free(p); } -void operator delete(void* p, size_t __sz, +void operator delete(void* p, size_t size, std::align_val_t alignment) noexcept { __wrap_free(p); } -void* operator new[](size_t __sz, std::align_val_t alignment) noexcept(false) { +void* operator new[](size_t size, std::align_val_t alignment) noexcept(false) { return __wrap_aligned_alloc(static_cast(alignment), - get_new_alloc_size(__sz)); + GetNewAllocSize(size)); } -void* operator new[](size_t __sz, std::align_val_t alignment, +void* operator new[](size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept { return __wrap_aligned_alloc(static_cast(alignment), - get_new_alloc_size(__sz)); + GetNewAllocSize(size)); } void operator delete[](void* p, std::align_val_t alignment) noexcept { __wrap_free(p); @@ -310,7 +333,7 @@ void operator delete[](void* p, std::align_val_t alignment, const std::nothrow_t&) noexcept { __wrap_free(p); } -void operator delete[](void* p, size_t __sz, +void operator delete[](void* p, size_t size, std::align_val_t alignment) noexcept { __wrap_free(p); } diff --git a/vmsdk/src/memory_allocation_overrides.h b/vmsdk/src/memory_allocation_overrides.h index f3a94df..c9c0966 100644 --- a/vmsdk/src/memory_allocation_overrides.h +++ b/vmsdk/src/memory_allocation_overrides.h @@ -89,42 +89,30 @@ void* __wrap_valloc(size_t size) noexcept; // NOLINTNEXTLINE #define valloc(...) __wrap_valloc(__VA_ARGS__) -// NOLINTNEXTLINE -void* operator new(size_t __sz) noexcept(false); +void* operator new(size_t size) noexcept(false); void operator delete(void* p) noexcept; -// NOLINTNEXTLINE -void operator delete(void* p, size_t __sz) noexcept; -// NOLINTNEXTLINE -void* operator new[](size_t __sz) noexcept(false); +void operator delete(void* p, size_t size) noexcept; +void* operator new[](size_t size) noexcept(false); void operator delete[](void* p) noexcept; -// NOLINTNEXTLINE -void operator delete[](void* p, size_t __sz) noexcept; -// NOLINTNEXTLINE -void* operator new(size_t __sz, const std::nothrow_t& nt) noexcept; -// NOLINTNEXTLINE -void* operator new[](size_t __sz, const std::nothrow_t& nt) noexcept; +void operator delete[](void* p, size_t size) noexcept; +void* operator new(size_t size, const std::nothrow_t& nt) noexcept; +void* operator new[](size_t size, const std::nothrow_t& nt) noexcept; void operator delete(void* p, const std::nothrow_t& nt) noexcept; void operator delete[](void* p, const std::nothrow_t& nt) noexcept; -// NOLINTNEXTLINE -void* operator new(size_t __sz, std::align_val_t alignment) noexcept(false); -// NOLINTNEXTLINE -void* operator new(size_t __sz, std::align_val_t alignment, +void* operator new(size_t size, std::align_val_t alignment) noexcept(false); +void* operator new(size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept; void operator delete(void* p, std::align_val_t alignment) noexcept; void operator delete(void* p, std::align_val_t alignment, const std::nothrow_t&) noexcept; -// NOLINTNEXTLINE -void operator delete(void* p, size_t __sz, std::align_val_t alignment) noexcept; -// NOLINTNEXTLINE -void* operator new[](size_t __sz, std::align_val_t alignment) noexcept(false); -// NOLINTNEXTLINE -void* operator new[](size_t __sz, std::align_val_t alignment, +void operator delete(void* p, size_t size, std::align_val_t alignment) noexcept; +void* operator new[](size_t size, std::align_val_t alignment) noexcept(false); +void* operator new[](size_t size, std::align_val_t alignment, const std::nothrow_t&) noexcept; void operator delete[](void* p, std::align_val_t alignment) noexcept; void operator delete[](void* p, std::align_val_t alignment, const std::nothrow_t&) noexcept; -// NOLINTNEXTLINE -void operator delete[](void* p, size_t __sz, +void operator delete[](void* p, size_t size, std::align_val_t alignment) noexcept; inline void SetRealAllocators(void* (*malloc_fn)(size_t),