From 246980d2169c9afac2ee5bec947941c8065a6abb Mon Sep 17 00:00:00 2001 From: divitsinghall Date: Thu, 25 Dec 2025 14:01:05 +0530 Subject: [PATCH] [Build] Fix ABI ODR violation for C++20 consumers linking against C++17 Folly --- CMake/FollyConfigChecks.cmake | 16 ++++++++ CMake/folly-config.h.cmake | 11 ++++++ folly/CppAttributes.h | 74 ++++++++++++++++++++++++++++++++++- folly/Portability.h | 34 ++++++++++++++++ 4 files changed, 133 insertions(+), 2 deletions(-) diff --git a/CMake/FollyConfigChecks.cmake b/CMake/FollyConfigChecks.cmake index 99326416807..7a017aca43b 100644 --- a/CMake/FollyConfigChecks.cmake +++ b/CMake/FollyConfigChecks.cmake @@ -164,6 +164,22 @@ check_cxx_source_compiles(" FOLLY_HAVE_EXTRANDOM_SFMT19937 ) +# Check if [[no_unique_address]] attribute is available at library compile time. +# This is critical for ABI stability: the attribute changes struct layout, +# and if headers are compiled with a different C++ standard than the library, +# ODR violations and memory corruption can occur. +# See: https://github.com/facebook/folly/issues/2477 +check_cxx_source_compiles(" + struct Empty {}; + struct Test { + [[no_unique_address]] Empty e; + int x; + }; + static_assert(sizeof(Test) == sizeof(int), \"\"); + int main() { return 0; }" + FOLLY_LIBRARY_HAS_NO_UNIQUE_ADDRESS +) + check_cxx_source_runs(" #include #include diff --git a/CMake/folly-config.h.cmake b/CMake/folly-config.h.cmake index 04a0911eaa2..e3c421b3855 100644 --- a/CMake/folly-config.h.cmake +++ b/CMake/folly-config.h.cmake @@ -85,3 +85,14 @@ #cmakedefine01 FOLLY_HAVE_LIBRT #cmakedefine01 FOLLY_HAVE_VSOCK + +// Record the C++ standard version used to compile the Folly library. +// This enables ABI compatibility checks when headers are included by +// applications compiled with a different C++ standard version. +// See: https://github.com/facebook/folly/issues/2477 +#define FOLLY_LIBRARY_CXX_STANDARD @CMAKE_CXX_STANDARD@ + +// Record whether [[no_unique_address]] was active when building the library. +// This attribute changes struct layout and can cause ODR violations if +// headers are compiled with a different C++ standard than the library. +#cmakedefine01 FOLLY_LIBRARY_HAS_NO_UNIQUE_ADDRESS diff --git a/folly/CppAttributes.h b/folly/CppAttributes.h index 52c958cb3ad..302c15a1ea5 100644 --- a/folly/CppAttributes.h +++ b/folly/CppAttributes.h @@ -16,11 +16,22 @@ /** * GCC compatible wrappers around clang attributes. + * + * ABI STABILITY NOTE: + * Some attributes (notably [[no_unique_address]]) affect struct layout. + * When the Folly library is compiled with one C++ standard and headers are + * included from an application compiled with a different standard, ODR + * violations can occur, leading to memory corruption and crashes. + * + * This header includes logic to detect such mismatches and force the headers + * to use the same layout as the compiled library. + * See: https://github.com/facebook/folly/issues/2477 */ #pragma once #include +#include #ifndef __has_attribute #define FOLLY_HAS_ATTRIBUTE(x) 0 @@ -115,15 +126,74 @@ * * sizeof(NonEmpty1); // may be == sizeof(int) * sizeof(NonEmpty2); // must be > sizeof(int) + * + * ABI STABILITY: + * This attribute changes struct layout. When linking against a pre-compiled + * Folly library, we MUST use the same layout as the library was compiled with. + * If the library was compiled without [[no_unique_address]] support (e.g., + * C++17), we must NOT apply the attribute even if the current compilation + * would support it (e.g., C++20). Doing otherwise causes ODR violations, + * memory corruption, and crashes. + * See: https://github.com/facebook/folly/issues/2477 */ + +// First, detect what the current compilation unit supports #if FOLLY_HAS_CPP_ATTRIBUTE(no_unique_address) -#define FOLLY_ATTR_NO_UNIQUE_ADDRESS no_unique_address +#define FOLLY_CURRENT_HAS_NO_UNIQUE_ADDRESS 1 +#define FOLLY_CURRENT_NO_UNIQUE_ADDRESS_ATTR no_unique_address #elif FOLLY_HAS_CPP_ATTRIBUTE(msvc::no_unique_address) -#define FOLLY_ATTR_NO_UNIQUE_ADDRESS msvc::no_unique_address +#define FOLLY_CURRENT_HAS_NO_UNIQUE_ADDRESS 1 +#define FOLLY_CURRENT_NO_UNIQUE_ADDRESS_ATTR msvc::no_unique_address +#else +#define FOLLY_CURRENT_HAS_NO_UNIQUE_ADDRESS 0 +#endif + +// Now determine what to actually use, respecting library ABI +#if defined(FOLLY_LIBRARY_HAS_NO_UNIQUE_ADDRESS) +// We have library configuration available - use it for ABI compatibility + +#if FOLLY_LIBRARY_HAS_NO_UNIQUE_ADDRESS && FOLLY_CURRENT_HAS_NO_UNIQUE_ADDRESS +// Both library and current compilation support it - use the attribute +#define FOLLY_ATTR_NO_UNIQUE_ADDRESS FOLLY_CURRENT_NO_UNIQUE_ADDRESS_ATTR + +#elif FOLLY_LIBRARY_HAS_NO_UNIQUE_ADDRESS && \ + !FOLLY_CURRENT_HAS_NO_UNIQUE_ADDRESS +// Library was compiled with it, but current compilation doesn't support it. +// This is an error - the application will have wrong struct layouts. +#error \ + "Folly library was compiled with [[no_unique_address]] support, but " \ + "current compilation does not support it. This will cause ABI " \ + "incompatibility. Please compile with C++20 or later." + +#elif !FOLLY_LIBRARY_HAS_NO_UNIQUE_ADDRESS && \ + FOLLY_CURRENT_HAS_NO_UNIQUE_ADDRESS +// Library was compiled WITHOUT [[no_unique_address]], but current compilation +// supports it. We MUST NOT use the attribute to maintain ABI compatibility. +// Issue a warning to inform users of this situation. +#if defined(__GNUC__) || defined(__clang__) +#pragma message( \ + "Warning: Folly library was compiled without [[no_unique_address]] " \ + "(likely C++17), but current compilation supports it. Disabling the " \ + "attribute to maintain ABI compatibility. Consider recompiling Folly " \ + "with C++20 for optimal performance. See issue #2477.") +#endif +#define FOLLY_ATTR_NO_UNIQUE_ADDRESS /* disabled for ABI compatibility */ + #else +// Neither library nor current compilation support it #define FOLLY_ATTR_NO_UNIQUE_ADDRESS #endif +#else +// No library configuration available (header-only use or building Folly itself) +// Use whatever the current compilation supports +#if FOLLY_CURRENT_HAS_NO_UNIQUE_ADDRESS +#define FOLLY_ATTR_NO_UNIQUE_ADDRESS FOLLY_CURRENT_NO_UNIQUE_ADDRESS_ATTR +#else +#define FOLLY_ATTR_NO_UNIQUE_ADDRESS +#endif +#endif + #if FOLLY_HAS_CPP_ATTRIBUTE(clang::no_destroy) #define FOLLY_ATTR_CLANG_NO_DESTROY clang::no_destroy #else diff --git a/folly/Portability.h b/folly/Portability.h index 12ce8551f22..b613a5d5f4b 100644 --- a/folly/Portability.h +++ b/folly/Portability.h @@ -34,6 +34,40 @@ static_assert(FOLLY_CPLUSPLUS >= 201703L, "__cplusplus >= 201703L"); +/** + * ABI Compatibility Check + * + * When linking against a pre-compiled Folly library, the C++ standard version + * used by the application should match the library to avoid ODR violations. + * Key layout-affecting features differ between C++17 and C++20: + * - [[no_unique_address]] attribute availability and behavior + * - Coroutine support structures + * - std::span vs folly::span selection + * + * See: https://github.com/facebook/folly/issues/2477 + */ +#if defined(FOLLY_LIBRARY_CXX_STANDARD) && !defined(FOLLY_SKIP_ABI_CHECK) +// Determine if there's a C++ standard version mismatch that could affect ABI +#if (FOLLY_LIBRARY_CXX_STANDARD < 20 && FOLLY_CPLUSPLUS >= 202002L) || \ + (FOLLY_LIBRARY_CXX_STANDARD >= 20 && FOLLY_CPLUSPLUS < 202002L) +// There's a C++17/C++20 boundary crossing, which affects [[no_unique_address]] +#if defined(__GNUC__) || defined(__clang__) +#pragma message( \ + "Warning: Folly library was compiled with C++" \ + FOLLY_PP_STRINGIZE(FOLLY_LIBRARY_CXX_STANDARD) \ + ", but application is using a different C++ standard. " \ + "This may cause ABI issues. See issue #2477. " \ + "Define FOLLY_SKIP_ABI_CHECK to suppress this warning.") +#endif +#endif +#endif + +// Helper macro for stringizing (used in the warning above) +#ifndef FOLLY_PP_STRINGIZE +#define FOLLY_PP_STRINGIZE_IMPL(x) #x +#define FOLLY_PP_STRINGIZE(x) FOLLY_PP_STRINGIZE_IMPL(x) +#endif + #if defined(__GNUC__) && !defined(__clang__) #if defined(FOLLY_CONFIG_TEMPORARY_DOWNGRADE_GCC) static_assert(__GNUC__ >= 9, "__GNUC__ >= 9");