Skip to content

B
benchmark
Commit e668e2a1 by Eric Fiselier
Options

Fix #552 - GCC and Clang warn on possibly invalid offsetof usage.

This patch disables the -Winvalid-offsetof warning for GCC and Clang when using it to check the cache lines of the State object. Technically this usage of offsetof is undefined behavior until C++17. However, all major compilers support this application as an extension, as demonstrated by the passing static assert (If a compiler encounters UB during evaluation of a constant expression, that UB must be diagnosed). Unfortunately, Clang and GCC also produce a warning about it. This patch temporarily suppresses the warning using #pragma's in the source file (instead of globally suppressing the warning in the build systems). This way the warning is ignored for both CMake and Bazel builds without having to modify either build system.
parent 68e22894
Showing with 13 additions and 7 deletions
CMakeLists.txt
...@@ -120,13 +120,6 @@ else() ...@@ -120,13 +120,6 @@ else()
cxx_feature_check(THREAD_SAFETY_ATTRIBUTES) cxx_feature_check(THREAD_SAFETY_ATTRIBUTES)
endif() endif()
# GCC doesn't report invalid -Wno-warning flags, so check the positive
# version, and if it exists add the negative.
check_cxx_warning_flag(-Winvalid-offsetof)
if (HAVE_CXX_FLAG_WINVALID_OFFSETOF)
add_cxx_compiler_flag(-Wno-invalid-offsetof)
endif()
# On most UNIX like platforms g++ and clang++ define _GNU_SOURCE as a # On most UNIX like platforms g++ and clang++ define _GNU_SOURCE as a
# predefined macro, which turns on all of the wonderful libc extensions. # predefined macro, which turns on all of the wonderful libc extensions.
# However g++ doesn't do this in Cygwin so we have to define it ourselfs # However g++ doesn't do this in Cygwin so we have to define it ourselfs
......
src/benchmark.cc
...@@ -311,10 +311,23 @@ State::State(size_t max_iters, const std::vector<int>& ranges, int thread_i, ...@@ -311,10 +311,23 @@ State::State(size_t max_iters, const std::vector<int>& ranges, int thread_i,
CHECK(max_iterations != 0) << "At least one iteration must be run"; CHECK(max_iterations != 0) << "At least one iteration must be run";
CHECK_LT(thread_index, threads) << "thread_index must be less than threads"; CHECK_LT(thread_index, threads) << "thread_index must be less than threads";
// 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.
#ifdef __GNUC__
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Winvalid-offsetof"
#endif
// Offset tests to ensure commonly accessed data is on the first cache line. // Offset tests to ensure commonly accessed data is on the first cache line.
const int cache_line_size = 64; const int cache_line_size = 64;
static_assert(offsetof(State, error_occurred_) <= static_assert(offsetof(State, error_occurred_) <=
(cache_line_size - sizeof(error_occurred_)), ""); (cache_line_size - sizeof(error_occurred_)), "");
#ifdef __GNUC__
#pragma GCC diagnostic pop
#endif
} }
void State::PauseTiming() { void State::PauseTiming() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment