From 87db41a49c687040a1df7f4fa6c7b5c786feff10 Mon Sep 17 00:00:00 2001 From: devtejasx Date: Fri, 31 Jul 2026 04:42:10 +0530 Subject: [PATCH] Replace the offsetof cache-line check with a runtime check (#1877) State is not a standard-layout type, so offsetof(State, skipped_) is only conditionally-supported. Compilers diagnose it, and the static_assert was kept quiet by a stack of vendor pragmas for GCC, clang, ICC, NVCC and NVHPC. That stack does not cover every compiler: #1877 is clang-18/19 on Windows reporting it anyway. Measure the offset on the object being constructed instead. That is well defined, needs no pragmas, and expresses the same invariant. The check uses a new BM_CHECK_ALWAYS, which is BM_CHECK without the NDEBUG opt-out, so it still holds in a release build. A State is constructed once per benchmark instance and thread, never inside the iteration loop. Verified: with the bound lowered to 8 bytes, a release build fails at run time with src/benchmark.cc:231: State: Check `... <= 8 - ...' failed. the commonly accessed members of State must fit in the first cache line and src/benchmark.cc compiles clean under -Winvalid-offsetof -Wall -Wextra -Werror with both gcc 15 and clang 20. --- src/benchmark.cc | 48 ++++++++++++------------------------------------ src/check.h | 13 +++++++++---- 2 files changed, 21 insertions(+), 40 deletions(-) diff --git a/src/benchmark.cc b/src/benchmark.cc index c7baa103d9..1f7d947508 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -222,42 +223,17 @@ State::State(std::string name, IterationCount max_iters, } } - // Note: The use of offsetof below is technically undefined until C++17 - // because State is not a standard layout type. However, all compilers - // currently provide well-defined behavior as an extension (which is - // demonstrated since constexpr evaluation must diagnose all undefined - // behavior). However, GCC and Clang also warn about this use of offsetof, - // which must be suppressed. -#if defined(__INTEL_COMPILER) -#pragma warning push -#pragma warning(disable : 1875) -#elif defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Winvalid-offsetof" -#endif -#if defined(__NVCC__) -#pragma nv_diagnostic push -#pragma nv_diag_suppress 1427 -#endif -#if defined(__NVCOMPILER) -#pragma diagnostic push -#pragma diag_suppress offset_in_non_POD_nonstandard -#endif - // Offset tests to ensure commonly accessed data is on the first cache line. - const int cache_line_size = 64; - static_assert( - offsetof(State, skipped_) <= (cache_line_size - sizeof(skipped_)), ""); -#if defined(__INTEL_COMPILER) -#pragma warning pop -#elif defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif -#if defined(__NVCC__) -#pragma nv_diagnostic pop -#endif -#if defined(__NVCOMPILER) -#pragma diagnostic pop -#endif + // Ensure commonly accessed data is on the first cache line. State is not a + // standard-layout type, so the offset is measured on the live object. + // + // The check is enabled under NDEBUG as well: a State is constructed once per + // benchmark instance and thread, never inside the iteration loop. + BM_CHECK_ALWAYS(reinterpret_cast(&skipped_) - + reinterpret_cast(this) <= + /*cache_line_size=*/64 - + static_cast(sizeof(skipped_))) + << "the commonly accessed members of State must fit in the first cache " + "line"; } void State::PauseTiming() { diff --git a/src/check.h b/src/check.h index aa8c78c92f..b6fc22ff0b 100644 --- a/src/check.h +++ b/src/check.h @@ -79,15 +79,20 @@ class CheckHandler { } // end namespace internal } // end namespace benchmark -// The BM_CHECK macro returns a std::ostream object that can have extra -// information written to it. -#ifndef NDEBUG -#define BM_CHECK(b) \ +// Like BM_CHECK, but enabled in every build configuration. Reserved for +// invariants that are cheap enough to verify unconditionally and that must not +// be violated silently in a release build. +#define BM_CHECK_ALWAYS(b) \ (b ? ::benchmark::internal::GetNullLogInstance() \ : ::benchmark::internal::CheckHandler( \ std::string_view(#b), std::string_view(__FILE__), \ std::string_view(__func__), __LINE__) \ .GetLog()) + +// The BM_CHECK macro returns a std::ostream object that can have extra +// information written to it. +#ifndef NDEBUG +#define BM_CHECK(b) BM_CHECK_ALWAYS(b) #else #define BM_CHECK(b) ::benchmark::internal::GetNullLogInstance() #endif