From ed5854840fd4f657c9f612bc16c643bf2c69b342 Mon Sep 17 00:00:00 2001 From: xulei25 Date: Sun, 1 Nov 2026 19:24:36 +0800 Subject: [PATCH] Honor cacheline alignment of pooled objects in C++14 builds ResourcePool, ObjectPool, their LocalPool and SocketPool are declared with BAIDU_CACHELINE_ALIGNMENT and created with plain new. C++17 uses aligned operator new for them, but C++14 builds (the CMake and Make default with protobuf < 22) do not, and glibc returns only 16-byte aligned storage. With Clang 14 this corrupts sockets created during a client connect and crashes in Socket::ResetFileDescriptor. For C++14 builds, CMake and config_brpc.sh now link an over-aligned allocation with -faligned-new, so both the compiler and the C++ runtime are checked, and fail configuration when that does not work. The flag is added to the build, to brpc.pc for pkg-config consumers and, as before, to config.mk. C++17 builds are unchanged. Add a test that fails when aligned new is not in effect. --- CMakeLists.txt | 29 +++++++++++++++++++++++++++++ cmake/brpc.pc.in | 2 +- config_brpc.sh | 19 +++++++++++++++++++ test/resource_pool_unittest.cpp | 30 ++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0966595d13..d1bdc6341c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -198,6 +198,35 @@ if(Protobuf_VERSION VERSION_GREATER 4.21) absl::variant ) endif() + +# Cacheline-aligned types such as ResourcePool are created with plain +# `new'. C++17 uses the aligned operator new for them; C++14 does so only with +# -faligned-new, and otherwise glibc returns 16-byte aligned storage. Their +# headers are installed, so consumers need the flag too (see brpc.pc). +set(BRPC_PC_EXTRA_CFLAGS "") +if(BRPC_CXX_STANDARD LESS 17) + include(CheckCXXSourceCompiles) + set(_brpc_saved_required_flags "${CMAKE_REQUIRED_FLAGS}") + set(CMAKE_REQUIRED_FLAGS + "${CMAKE_REQUIRED_FLAGS} -std=c++${BRPC_CXX_STANDARD} -faligned-new") + # Link an over-aligned allocation so the C++ runtime is checked as well. + check_cxx_source_compiles(" + struct alignas(64) BrpcAlignedNewProbe { char c; }; + int main() { + BrpcAlignedNewProbe* p = new BrpcAlignedNewProbe; + delete p; + return 0; + }" BRPC_HAS_ALIGNED_NEW) + set(CMAKE_REQUIRED_FLAGS "${_brpc_saved_required_flags}") + if(NOT BRPC_HAS_ALIGNED_NEW) + message(FATAL_ERROR + "C++${BRPC_CXX_STANDARD} builds need -faligned-new and an aligned " + "operator new in the C++ runtime (GCC 7+, Clang 4+ or libc++). " + "Use a newer toolchain or build with C++17.") + endif() + list(APPEND BRPC_COMMON_COMPILE_OPTIONS $<$:-faligned-new>) + set(BRPC_PC_EXTRA_CFLAGS " -faligned-new") +endif() find_package(Threads REQUIRED) list(REMOVE_DUPLICATES BRPC_COMMON_INCLUDE_DIRS) diff --git a/cmake/brpc.pc.in b/cmake/brpc.pc.in index 723dab4f13..2dd6effeb9 100644 --- a/cmake/brpc.pc.in +++ b/cmake/brpc.pc.in @@ -21,6 +21,6 @@ libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@ Name: brpc Description: Apache bRPC is an Industrial-grade RPC framework using C++ Language, which is often used in high performance system such as Search, Storage, Machine learning, Advertisement, Recommendation etc. "bRPC" means "better RPC". Version: @BRPC_VERSION@ -Cflags: -I${includedir} +Cflags: -I${includedir}@BRPC_PC_EXTRA_CFLAGS@ Libs: -L${libdir}/ -lbrpc Libs.private: @BRPC_PRIVATE_LIBS@ diff --git a/config_brpc.sh b/config_brpc.sh index b77f81bed8..578c4d81e8 100755 --- a/config_brpc.sh +++ b/config_brpc.sh @@ -396,6 +396,25 @@ else print_success "Found protobuf version $PROTOBUF_VERSION" fi +# Cacheline-aligned types such as ResourcePool are created with plain +# `new'. C++17 uses the aligned operator new for them; C++14 does so only with +# -faligned-new, and otherwise glibc returns 16-byte aligned storage. Link an +# over-aligned allocation so the C++ runtime is checked as well. config.mk +# carries the flag to code built against this tree. +if [ "$CXXFLAGS" = "-std=c++14" ]; then + ALIGNED_NEW_PROBE=$(mktemp "${TMPDIR:-/tmp}/brpc_aligned_new.XXXXXX") + if printf '%s\n' 'struct alignas(64) P { char c; };' \ + 'int main() { P* p = new P; delete p; return 0; }' | \ + $CXX $CXXFLAGS -faligned-new -x c++ - -o "$ALIGNED_NEW_PROBE" >/dev/null 2>&1; then + CXXFLAGS="${CXXFLAGS} -faligned-new" + rm -f "$ALIGNED_NEW_PROBE" + else + rm -f "$ALIGNED_NEW_PROBE" + >&2 $ECHO "C++14 builds need -faligned-new and an aligned operator new in the C++ runtime (GCC 7+, Clang 4+ or libc++)" + exit 1 + fi +fi + CPPFLAGS= if [ $WITH_ASAN != 0 ]; then diff --git a/test/resource_pool_unittest.cpp b/test/resource_pool_unittest.cpp index d3f6fbf792..1770f22b7d 100644 --- a/test/resource_pool_unittest.cpp +++ b/test/resource_pool_unittest.cpp @@ -456,4 +456,34 @@ TEST_F(ResourcePoolTest, verify_get) { clear_resources(); } + +struct BAIDU_CACHELINE_ALIGNMENT CachelineObject { + int value; +}; + +TEST_F(ResourcePoolTest, plain_new_honors_cacheline_alignment) { + // Pools, their per-thread LocalPool and brpc's SocketPool are + // cacheline-aligned and created with plain `new'. C++14 builds need + // -faligned-new for that; glibc malloc then returns 16-byte aligned + // storage. tcmalloc, linked into this test, may hide the problem, so + // also require the compiler to use aligned new. +#if !defined(__cpp_aligned_new) + FAIL() << "Build with C++17 or -faligned-new"; +#endif + std::vector objects; + for (int i = 0; i < 256; ++i) { + objects.push_back(new CachelineObject); + } + size_t misaligned = 0; + for (size_t i = 0; i < objects.size(); ++i) { + // The compiler may assume `new' honored alignof and fold a direct + // check to false; reading through a volatile keeps the real address. + volatile uintptr_t addr = reinterpret_cast(objects[i]); + if (addr % alignof(CachelineObject)) { + ++misaligned; + } + delete objects[i]; + } + ASSERT_EQ(0u, misaligned); +} } // namespace