From ac9c4304b476bb6f03e079bcc7b281eeaf398ed2 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 13 Jan 2026 12:53:11 -0800 Subject: [PATCH 1/3] api: improve hardening macros and the start of contract assertions Review: we have long had two assertion macros: OIIO_ASSERT which aborts upon failure in Debug builds and prints but continues in Release builds, and OIIO_DASSERT which aborts in Debug builds and is completely inactive for Relase builds. Inspired by C++26 contracts, and increasingly available "hardening modes" in major compilers (especially with the LLVM/clang project's libc++), I'm introducing some new verification helpers. New macro `OIIO_CONTRACT_ASSERT` more closely mimics C++26 contract_assert in many ways, and perhaps will simply wrap C++ contract_assert when C++26 is on our menu. Important ways that OIIO_CONTRACT_ASSERT differs from OIIO_ASSERT and OIIO_DASSERT in a few ways, described below. * Keeping in line with C++ contracts, there are 4 possible responses to a failed contract assertion: Ignore, Observe (print only), Enforce (print and abort) and Quick-Enforce (just abort). * By default, the contract failure response is Ignore for release builds and Enforce for debug builds. But it's overrideable (independent of Release/Debug, and optionally on a per-translation-unit basis) by setting OIIO_ASSERTION_RESPONSE_DEFAULT before any OIIO headers are included. * Also define hardening levels: None, Fast, Extensive, and Debug, mimicking the levels of libc++. The idea is that maybe there will be some CONTRACT_ASSERT checks you only want to do for certain hardening levels. * Macros for explicit hardening levels: OIIO_HARDENING_ASSERT_FAST(), EXTENSIVE(), and DEBUG(), which call CONTRACT_ASSERT only when the hardening level is what's required or stricter. I also changed the bounds checking in operator[] of string_view, span, and image_span to use the contract assertions. Note that this adds a little bit of overhead, since the default is "enforce" for release builds. I added some benchmarking that proves that the bounds check adds only about 20% overhead to an element access for a trivial `span`. For more complex things, or code that does more than just repeatedly access elements with bounds checks, I expect this overhead to be negligible. Since libc++ and upcoming C++ standards do the same for most container types, I expect the compilers to get better and better at eliding these checks when they can determine that it's an in-bounds access. Also please note that one way to avoid these extra bounds checks entirely is to change an index-oriented loop like span s; for (size_t i = 0; i < s.size(); ++i) foo(s[i]); // maybe bounds check on each iteration? to a range based loop: span s; for (auto& v : s) foo(v); which is inherently safe and requires no in-loop checks at all. Signed-off-by: Larry Gritz --- src/include/OpenImageIO/dassert.h | 149 ++++++++++++++++++++++++++ src/include/OpenImageIO/hash.h | 7 +- src/include/OpenImageIO/image_span.h | 26 +++-- src/include/OpenImageIO/span.h | 14 +-- src/include/OpenImageIO/string_view.h | 22 ++-- src/libutil/errorhandler.cpp | 11 ++ src/libutil/span_test.cpp | 61 ++++++++++- src/libutil/strutil.cpp | 2 +- 8 files changed, 268 insertions(+), 24 deletions(-) diff --git a/src/include/OpenImageIO/dassert.h b/src/include/OpenImageIO/dassert.h index db7cf65976..5d26bcdebc 100644 --- a/src/include/OpenImageIO/dassert.h +++ b/src/include/OpenImageIO/dassert.h @@ -9,9 +9,158 @@ #include #include +#include #include + +// General resources about security and hardening for C++: +// +// https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html +// https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html +// https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html +// https://libcxx.llvm.org/Hardening.html +// https://cheatsheetseries.owasp.org/cheatsheets/C-Based_Toolchain_Hardening_Cheat_Sheet.html +// https://stackoverflow.com/questions/13544512/what-is-the-most-hardened-set-of-options-for-gcc-compiling-c-c +// https://medium.com/@simontoth/daily-bit-e-of-c-hardened-mode-of-standard-library-implementations-18be2422c372 +// https://en.cppreference.com/w/cpp/contract +// https://en.cppreference.com/w/cpp/language/contracts + + + +// Define hardening levels for OIIO: which checks should we do? +// NONE - YOLO mode, no extra checks (not recommended) +// FAST - Minimal checks that have low performance impact +// EXTENSIVE - More thorough checks, may impact performance +// DEBUG - Maximum checks, for debugging purposes +#define OIIO_HARDENING_NONE 0 +#define OIIO_HARDENING_FAST 1 +#define OIIO_HARDENING_EXTENSIVE 2 +#define OIIO_HARDENING_DEBUG 3 + +// OIIO_HARDENING_DEFAULT defines the default hardening level we actually use. +// By default, we use NONE for release builds and DEBUG for debug builds. But +// any translation unit (including clients of OIIO) may override this by +// defining OIIO_HARDENING_DEFAULT before including any OIIO headers. But note +// that this only affects calls to inline functions or templates defined in +// the headers. Non-inline functions compiled into the OIIO library, including +// OIIO internal code itself, will have been compiled with whatever hardening +// level was selected when the library was built. +#ifndef OIIO_HARDENING_DEFAULT +# ifdef NDEBUG +# define OIIO_HARDENING_DEFAULT OIIO_HARDENING_NONE +# else +# define OIIO_HARDENING_DEFAULT OIIO_HARDENING_DEBUG +# endif +#endif + + +/// Choices for what to do when a contract assertion fails. +/// This mimics the C++26 standard's std::contract behavior. +#define OIIO_ASSERTION_RESPONSE_IGNORE 0 +#define OIIO_ASSERTION_RESPONSE_OBSERVE 1 +#define OIIO_ASSERTION_RESPONSE_ENFORCE 2 +#define OIIO_ASSERTION_RESPONSE_QUICK_ENFORCE 3 + +// OIIO_ASSERTION_RESPONSE_DEFAULT defines the default response to failed +// contract assertions. By default, in NONE hardening mode and in release +// builds, we do nothing. In all other cases, we abort. But any translation +// unit (including clients of OIIO) may override this by defining +// OIIO_ASSERTION_RESPONSE_DEFAULT before including any OIIO headers. But note +// that this only affects calls to inline functions or templates defined in +// the headers. Non-inline functions compiled into the OIIO library, including +// OIIO internal code itself, will have been compiled with whatever response +// was selected when the library was built. +#ifndef OIIO_ASSERTION_RESPONSE_DEFAULT +# if OIIO_HARDENING_DEFAULT == OIIO_HARDENING_NONE && defined(NDEBUG) +# define OIIO_ASSERTION_RESPONSE_DEFAULT OIIO_ASSERTION_RESPONSE_ENFORCE +# else +# define OIIO_ASSERTION_RESPONSE_DEFAULT OIIO_ASSERTION_RESPONSE_ENFORCE +# endif +#endif + + + +// `OIIO_CONTRACT_ASSERT(condition)` checks if the condition is met, and if +// not, calls the contract violation handler with the appropriate response. +// `OIIO_CONTRACT_ASSERT_MSG(condition, msg)` is the same, but allows a +// different message to be passed to the handler. +#if OIIO_ASSERTION_RESPONSE_DEFAULT == OIIO_ASSERTION_RESPONSE_IGNORE +# define OIIO_CONTRACT_ASSERT_MSG(condition, message) (void)0 +#elif OIIO_ASSERTION_RESPONSE_DEFAULT == OIIO_ASSERTION_RESPONSE_QUICK_ENFORCE +# define OIIO_CONTRACT_ASSERT_MSG(condition, message) \ + (OIIO_LIKELY(condition) ? ((void)0) : (std::abort(), (void)0)) +#elif OIIO_ASSERTION_RESPONSE_DEFAULT == OIIO_ASSERTION_RESPONSE_OBSERVE +# define OIIO_CONTRACT_ASSERT_MSG(condition, message) \ + (OIIO_LIKELY(condition) ? ((void)0) \ + : (OIIO::contract_violation_handler( \ + __FILE__ ":" OIIO_STRINGIZE(__LINE__), \ + OIIO_PRETTY_FUNCTION, message), \ + (void)0)) +#elif OIIO_ASSERTION_RESPONSE_DEFAULT == OIIO_ASSERTION_RESPONSE_ENFORCE +# define OIIO_CONTRACT_ASSERT_MSG(condition, message) \ + (OIIO_LIKELY(condition) ? ((void)0) \ + : (OIIO::contract_violation_handler( \ + __FILE__ ":" OIIO_STRINGIZE(__LINE__), \ + OIIO_PRETTY_FUNCTION, message), \ + std::abort(), (void)0)) +#else +# error "Unknown OIIO_ASSERTION_RESPONSE_DEFAULT" +#endif + +#define OIIO_CONTRACT_ASSERT(condition) \ + OIIO_CONTRACT_ASSERT_MSG(condition, #condition) + +// Macros to use to select whether or not to do a contract check, based on the +// hardening level: +// - OIIO_HARDENING_ASSERT_FAST : only checks contract for >= FAST hardening. +// - OIIO_HARDENING_ASSERT_EXTENSIVE : only checks contract for >= EXTENSIVE. +// - OIIO_HARDENING_ASSERT_DEBUG : only checks contract for DEBUG hardening. +#if OIIO_HARDENING_DEFAULT >= OIIO_HARDENING_FAST +# define OIIO_HARDENING_ASSERT_FAST_MSG(condition, message) \ + OIIO_CONTRACT_ASSERT_MSG(condition, message) +#else +# define OIIO_HARDENING_ASSERT_FAST_MSG(...) (void)0 +#endif + +#if OIIO_HARDENING_DEFAULT >= OIIO_HARDENING_EXTENSIVE +# define OIIO_HARDENING_ASSERT_EXTENSIVE_MSG(condition, message) \ + OIIO_CONTRACT_ASSERT_MSG(condition, message) +#else +# define OIIO_HARDENING_ASSERT_EXTENSIVE_MSG(...) (void)0 +#endif + +#if OIIO_HARDENING_DEFAULT >= OIIO_HARDENING_DEBUG +# define OIIO_HARDENING_ASSERT_DEBUG_MSG(condition, message) \ + OIIO_CONTRACT_ASSERT_MSG(condition, message) +#else +# define OIIO_HARDENING_ASSERT_DEBUG_MSG(...) (void)0 +#endif + +#define OIIO_HARDENING_ASSERT_NONE(condition) \ + OIIO_HARDENING_ASSERT_NONE_MSG(condition, #condition) +#define OIIO_HARDENING_ASSERT_FAST(condition) \ + OIIO_HARDENING_ASSERT_FAST_MSG(condition, #condition) +#define OIIO_HARDENING_ASSERT_EXTENSIVE(condition) \ + OIIO_HARDENING_ASSERT_EXTENSIVE_MSG(condition, #condition) +#define OIIO_HARDENING_ASSERT_DEBUG(condition) \ + OIIO_HARDENING_ASSERT_DEBUG_MSG(condition, #condition) + + +OIIO_NAMESPACE_3_1_BEGIN +// Internal contract assertion handler +OIIO_UTIL_API void +contract_violation_handler(const char* location, const char* function, + const char* msg = ""); +OIIO_NAMESPACE_3_1_END + +OIIO_NAMESPACE_BEGIN +#ifndef OIIO_DOXYGEN +using v3_1::contract_violation_handler; +#endif +OIIO_NAMESPACE_END + + /// OIIO_ABORT_IF_DEBUG is a call to abort() for debug builds, but does /// nothing for release builds. #ifndef NDEBUG diff --git a/src/include/OpenImageIO/hash.h b/src/include/OpenImageIO/hash.h index 8424869a97..9005040b86 100644 --- a/src/include/OpenImageIO/hash.h +++ b/src/include/OpenImageIO/hash.h @@ -247,8 +247,11 @@ strhash (string_view s) size_t len = s.length(); if (! len) return 0; unsigned int h = 0; - for (size_t i = 0; i < len; ++i) { - h += (unsigned char)(s[i]); + for (auto c : s) { + // Note: by using range for here, instead of looping over indices and + // calling operator[] to get each char, we avoid the bounds checking + // that operator[] does. + h += (unsigned char)(c); h += h << 10; h ^= h >> 6; } diff --git a/src/include/OpenImageIO/image_span.h b/src/include/OpenImageIO/image_span.h index 2684c15504..0ee7d2dcaf 100644 --- a/src/include/OpenImageIO/image_span.h +++ b/src/include/OpenImageIO/image_span.h @@ -271,18 +271,30 @@ template class image_span { /// Return a pointer to the value at channel c, pixel (x,y,z). inline T* getptr(int c, int x, int y, int z = 0) const { - // Bounds check in debug mode - OIIO_DASSERT(unsigned(c) < unsigned(nchannels()) - && unsigned(x) < unsigned(width()) - && unsigned(y) < unsigned(height()) - && unsigned(z) < unsigned(depth())); if constexpr (Rank == 2) { OIIO_DASSERT(y == 0 && z == 0); + OIIO_CONTRACT_ASSERT(unsigned(c) < unsigned(nchannels()) + && unsigned(x) < unsigned(width())); + return (T*)((char*)data() + c * chanstride()); } else if constexpr (Rank == 3) { OIIO_DASSERT(z == 0); + OIIO_CONTRACT_ASSERT(unsigned(c) < unsigned(nchannels()) + && unsigned(x) < unsigned(width()) + && unsigned(y) < unsigned(height())); + return (T*)((char*)data() + c * chanstride() + x * xstride() + + y * ystride()); + } else { + // Rank == 4 + OIIO_CONTRACT_ASSERT(unsigned(c) < unsigned(nchannels()) + && unsigned(x) < unsigned(width()) + && unsigned(y) < unsigned(height()) + && unsigned(z) < unsigned(depth())); + return (T*)((char*)data() + c * chanstride() + x * xstride() + + y * ystride() + z * zstride()); } - return (T*)((char*)data() + c * chanstride() + x * xstride() - + y * ystride() + z * zstride()); +#ifdef __INTEL_COMPILER + return nullptr; // should never get here, but icc is confused +#endif } /// Return a pointer to the value at channel 0, pixel (x,y,z). diff --git a/src/include/OpenImageIO/span.h b/src/include/OpenImageIO/span.h index f1c49dafdc..b184809a75 100644 --- a/src/include/OpenImageIO/span.h +++ b/src/include/OpenImageIO/span.h @@ -207,28 +207,28 @@ class span { /// optimized builds, there is no bounds check. Note: this is different /// from C++ std::span, which never bounds checks `operator[]`. constexpr reference operator[] (size_type idx) const { - OIIO_DASSERT(idx < m_size && "OIIO::span::operator[] range check"); + OIIO_CONTRACT_ASSERT(idx < m_size); return m_data[idx]; } constexpr reference operator() (size_type idx) const { - OIIO_DASSERT(idx < m_size && "OIIO::span::operator() range check"); + OIIO_CONTRACT_ASSERT(idx < m_size); return m_data[idx]; } /// Bounds-checked access, throws an assertion if out of range. reference at (size_type idx) const { if (idx >= size()) - throw (std::out_of_range ("OpenImageIO::span::at")); + throw (std::out_of_range ("OIIO::span::at")); return m_data[idx]; } /// The first element of the span. constexpr reference front() const noexcept { - OIIO_DASSERT(m_size >= 1); + OIIO_CONTRACT_ASSERT(m_size >= 1); return m_data[0]; } /// The last element of the span. constexpr reference back() const noexcept { - OIIO_DASSERT(m_size >= 1); + OIIO_CONTRACT_ASSERT(m_size >= 1); return m_data[size() - 1]; } @@ -374,14 +374,16 @@ class span_strided { constexpr stride_type stride() const noexcept { return m_stride; } constexpr reference operator[] (size_type idx) const { + OIIO_CONTRACT_ASSERT(idx < m_size); return m_data[m_stride*idx]; } constexpr reference operator() (size_type idx) const { + OIIO_CONTRACT_ASSERT(idx < m_size); return m_data[m_stride*idx]; } reference at (size_type idx) const { if (idx >= size()) - throw (std::out_of_range ("OpenImageIO::span_strided::at")); + throw (std::out_of_range ("OIIO::span_strided::at")); return m_data[m_stride*idx]; } constexpr reference front() const noexcept { return m_data[0]; } diff --git a/src/include/OpenImageIO/string_view.h b/src/include/OpenImageIO/string_view.h index b959bb9d5d..d07854ceb6 100644 --- a/src/include/OpenImageIO/string_view.h +++ b/src/include/OpenImageIO/string_view.h @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -205,11 +206,12 @@ class basic_string_view { /// Is the view empty, containing no characters? constexpr bool empty() const noexcept { return m_len == 0; } - /// Element access of an individual character. For debug build, does - /// bounds check with assertion. For optimized builds, there is no bounds - /// check. Note: this is different from C++ std::span, which never bounds - /// checks `operator[]`. - constexpr const_reference operator[](size_type pos) const { return m_chars[pos]; } + /// Element access of an individual character. For debug or hardened + /// builds, does bounds check with assertion. + constexpr const_reference operator[](size_type pos) const { + OIIO_CONTRACT_ASSERT(pos < m_len); + return m_chars[pos]; + } /// Element access with bounds checking and exception if out of bounds. constexpr const_reference at(size_t pos) const { @@ -218,9 +220,15 @@ class basic_string_view { return m_chars[pos]; } /// The first character of the view. - constexpr const_reference front() const { return m_chars[0]; } + constexpr const_reference front() const { + OIIO_CONTRACT_ASSERT(m_len >= 1); + return m_chars[0]; + } /// The last character of the view. - constexpr const_reference back() const { return m_chars[m_len - 1]; } + constexpr const_reference back() const { + OIIO_CONTRACT_ASSERT(m_len >= 1); + return m_chars[m_len - 1]; + } /// Return the underlying data pointer to the first character. constexpr const_pointer data() const noexcept { return m_chars; } diff --git a/src/libutil/errorhandler.cpp b/src/libutil/errorhandler.cpp index d0d1a2c385..417ed2aca8 100644 --- a/src/libutil/errorhandler.cpp +++ b/src/libutil/errorhandler.cpp @@ -58,4 +58,15 @@ ErrorHandler::operator()(int errcode, const std::string& msg) fflush(stderr); } + + +void +contract_violation_handler(const char* location, const char* function, + const char* msg) +{ + Strutil::print(stderr, "{} ({}): Contract assertion failed: {}\n", location, + function, msg ? msg : ""); + fflush(stderr); +} + OIIO_NAMESPACE_3_1_END diff --git a/src/libutil/span_test.cpp b/src/libutil/span_test.cpp index 041f59644b..4f208eea46 100644 --- a/src/libutil/span_test.cpp +++ b/src/libutil/span_test.cpp @@ -6,6 +6,8 @@ #include #include +#include +#include #include #include #include @@ -14,6 +16,27 @@ using namespace OIIO; +static int iterations = 100000; +static int ntrials = 5; + + +static void +getargs(int argc, char* argv[]) +{ + ArgParse ap; + ap.intro( + "span_test -- unit test and spans for OpenImageIO/span.h\n" OIIO_INTRO_STRING) + .usage("span_test [options]"); + + ap.arg("--iters %d", &iterations) + .help(Strutil::fmt::format("Number of iterations (default: {})", + iterations)); + ap.arg("--trials %d", &ntrials).help("Number of trials"); + + ap.parse_args(argc, (const char**)argv); +} + + void test_span() @@ -457,9 +480,44 @@ test_spanzero() +void +benchmark_span() +{ + Benchmarker bench; + bench.iterations(iterations).trials(ntrials); + bench.work(1000); + std::array f; + std::fill(f.begin(), f.end(), 1.0f); + int sum = 0; + bench("span operator[]", [&]() { + int t = 0; + for (size_t i = 0; i < f.size(); ++i) + DoNotOptimize(t += f[i]); + sum += t; + }); + bench("span range", [&]() { + int t = 0; + for (auto x : f) + DoNotOptimize(t += x); + sum += t; + }); +} + + + int -main(int /*argc*/, char* /*argv*/[]) +main(int argc, char* argv[]) { +#if !defined(NDEBUG) || defined(OIIO_CI) || defined(OIIO_CODE_COVERAGE) + // For the sake of test time, reduce the default iterations for DEBUG, + // CI, and code coverage builds. Explicit use of --iters or --trials + // will override this, since it comes before the getargs() call. + iterations /= 10; + ntrials = 1; +#endif + + getargs(argc, argv); + test_span(); test_span_mutable(); test_span_initlist(); @@ -475,6 +533,7 @@ main(int /*argc*/, char* /*argv*/[]) test_spancpy(); test_spanset(); test_spanzero(); + benchmark_span(); return unit_test_failures; } diff --git a/src/libutil/strutil.cpp b/src/libutil/strutil.cpp index 17060728d7..dc053df65e 100644 --- a/src/libutil/strutil.cpp +++ b/src/libutil/strutil.cpp @@ -152,7 +152,7 @@ c_str(string_view str) // in C++17 string_view. So maybe we'll find ourselves relying on it a // lot less, and therefore the performance hit of doing it foolproof // won't be as onerous. - if (str[str.size()] == 0) // 0-terminated + if (str.data()[str.size()] == 0) // 0-terminated return str.data(); // Rare case: may not be 0-terminated. Bite the bullet and construct a From a591e1a83fe26b54ee9edfd3cad1c32efb5d4bfa Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 24 Jan 2026 19:44:53 -0800 Subject: [PATCH 2/3] revision: more thorough benchmarking, change NDEBUG hardening default to IGNORE Signed-off-by: Larry Gritz --- src/build-scripts/ci-benchmark.bash | 7 ++-- src/cmake/compiler.cmake | 6 ++-- src/include/OpenImageIO/dassert.h | 35 +++++++++--------- src/libutil/span_test.cpp | 56 ++++++++++++++++++++--------- 4 files changed, 65 insertions(+), 39 deletions(-) diff --git a/src/build-scripts/ci-benchmark.bash b/src/build-scripts/ci-benchmark.bash index 7d5a5f0a33..d3b3551b1b 100755 --- a/src/build-scripts/ci-benchmark.bash +++ b/src/build-scripts/ci-benchmark.bash @@ -13,13 +13,14 @@ ls build ls $BUILD_BIN_DIR mkdir -p build/benchmarks -for t in image_span_test ; do +for t in image_span_test span_test ; do echo echo echo "$t" echo "========================================================" - ${BUILD_BIN_DIR}/$t > build/benchmarks/$t.out - cat build/benchmarks/$t.out + OpenImageIO_CI=0 ${BUILD_BIN_DIR}/$t | tee build/benchmarks/$t.out + # Note: set OpenImageIO_CI=0 to avoid CI-specific automatic reduction of + # the number of trials and iterations. echo "========================================================" echo "========================================================" echo diff --git a/src/cmake/compiler.cmake b/src/cmake/compiler.cmake index 8dffb97d36..6936ac51b9 100644 --- a/src/cmake/compiler.cmake +++ b/src/cmake/compiler.cmake @@ -499,12 +499,12 @@ endif () # https://cheatsheetseries.owasp.org/cheatsheets/C-Based_Toolchain_Hardening_Cheat_Sheet.html if (${CMAKE_BUILD_TYPE} STREQUAL "Debug") - set (${PROJ_NAME}_HARDENING_DEFAULT 2) + set (${PROJ_NAME}_HARDENING_DEFAULT 2) # Extensive else () - set (${PROJ_NAME}_HARDENING_DEFAULT 1) + set (${PROJ_NAME}_HARDENING_DEFAULT 1) # Fast endif () set_cache (${PROJ_NAME}_HARDENING ${${PROJ_NAME}_HARDENING_DEFAULT} - "Turn on security hardening features 0, 1, 2, 3") + "Turn on security hardening features 0=none, 1=fast, 2=extensive, 3=debug") # Implementation: add_compile_definitions (${PROJ_NAME}_HARDENING_DEFAULT=${${PROJ_NAME}_HARDENING}) if (${PROJ_NAME}_HARDENING GREATER_EQUAL 1) diff --git a/src/include/OpenImageIO/dassert.h b/src/include/OpenImageIO/dassert.h index 5d26bcdebc..db3e8992b9 100644 --- a/src/include/OpenImageIO/dassert.h +++ b/src/include/OpenImageIO/dassert.h @@ -39,41 +39,44 @@ #define OIIO_HARDENING_DEBUG 3 // OIIO_HARDENING_DEFAULT defines the default hardening level we actually use. -// By default, we use NONE for release builds and DEBUG for debug builds. But -// any translation unit (including clients of OIIO) may override this by -// defining OIIO_HARDENING_DEFAULT before including any OIIO headers. But note -// that this only affects calls to inline functions or templates defined in -// the headers. Non-inline functions compiled into the OIIO library, including -// OIIO internal code itself, will have been compiled with whatever hardening -// level was selected when the library was built. +// By default, we use FAST for release builds and DEBUG for debug builds. But +// it can be overridden: +// - For OIIO internals, at OIIO build time with the `OIIO_HARDENING` CMake +// variable. +// - For other projects using OIIO's headers, any translation unit may +// override this by defining OIIO_HARDENING_DEFAULT before including any +// OIIO headers. But note that this only affects calls to inline functions +// or templates defined in the headers. Non-inline functions compiled into +// the OIIO library itself will have been compiled with whatever hardening +// level was selected when the library was built. #ifndef OIIO_HARDENING_DEFAULT # ifdef NDEBUG -# define OIIO_HARDENING_DEFAULT OIIO_HARDENING_NONE +# define OIIO_HARDENING_DEFAULT OIIO_HARDENING_FAST # else # define OIIO_HARDENING_DEFAULT OIIO_HARDENING_DEBUG # endif #endif -/// Choices for what to do when a contract assertion fails. -/// This mimics the C++26 standard's std::contract behavior. +// Choices for what to do when a contract assertion fails. +// This mimics the C++26 standard's std::contract behavior. #define OIIO_ASSERTION_RESPONSE_IGNORE 0 #define OIIO_ASSERTION_RESPONSE_OBSERVE 1 #define OIIO_ASSERTION_RESPONSE_ENFORCE 2 #define OIIO_ASSERTION_RESPONSE_QUICK_ENFORCE 3 // OIIO_ASSERTION_RESPONSE_DEFAULT defines the default response to failed -// contract assertions. By default, in NONE hardening mode and in release -// builds, we do nothing. In all other cases, we abort. But any translation +// contract assertions. By default, we enforce them, UNLESS we are a release +// mode build that has set the hardening mode to NONE. But any translation // unit (including clients of OIIO) may override this by defining // OIIO_ASSERTION_RESPONSE_DEFAULT before including any OIIO headers. But note // that this only affects calls to inline functions or templates defined in -// the headers. Non-inline functions compiled into the OIIO library, including -// OIIO internal code itself, will have been compiled with whatever response -// was selected when the library was built. +// the headers. Non-inline functions compiled into the OIIO library itself +// will have been compiled with whatever response was selected when the +// library was built. #ifndef OIIO_ASSERTION_RESPONSE_DEFAULT # if OIIO_HARDENING_DEFAULT == OIIO_HARDENING_NONE && defined(NDEBUG) -# define OIIO_ASSERTION_RESPONSE_DEFAULT OIIO_ASSERTION_RESPONSE_ENFORCE +# define OIIO_ASSERTION_RESPONSE_DEFAULT OIIO_ASSERTION_RESPONSE_IGNORE # else # define OIIO_ASSERTION_RESPONSE_DEFAULT OIIO_ASSERTION_RESPONSE_ENFORCE # endif diff --git a/src/libutil/span_test.cpp b/src/libutil/span_test.cpp index 4f208eea46..2953d7140a 100644 --- a/src/libutil/span_test.cpp +++ b/src/libutil/span_test.cpp @@ -485,21 +485,38 @@ benchmark_span() { Benchmarker bench; bench.iterations(iterations).trials(ntrials); - bench.work(1000); - std::array f; - std::fill(f.begin(), f.end(), 1.0f); - int sum = 0; + const size_t N = 1000; + // bench.work(N); + std::array fstdarr; + std::fill(fstdarr.begin(), fstdarr.end(), 1.0f); + bench("pointer operator[]", [&]() { + float* fptr(fstdarr.data()); + float t = 0.0f; + for (size_t i = 0; i < N; ++i) + DoNotOptimize(t += fptr[i]); + }); + bench("std::array operator[]", [&]() { + float t = 0.0f; + for (size_t i = 0; i < N; ++i) + DoNotOptimize(t += fstdarr[i]); + }); bench("span operator[]", [&]() { - int t = 0; - for (size_t i = 0; i < f.size(); ++i) - DoNotOptimize(t += f[i]); - sum += t; + span fspan(fstdarr); + float t = 0.0f; + for (size_t i = 0; i < N; ++i) + DoNotOptimize(t += fspan[i]); + }); + bench("span unsafe indexing", [&]() { + span fspan(fstdarr); + float t = 0.0f; + for (size_t i = 0; i < N; ++i) + DoNotOptimize(t += fspan.data()[i]); }); bench("span range", [&]() { - int t = 0; - for (auto x : f) + span fspan(fstdarr); + float t = 0.0f; + for (auto x : fspan) DoNotOptimize(t += x); - sum += t; }); } @@ -508,13 +525,18 @@ benchmark_span() int main(int argc, char* argv[]) { -#if !defined(NDEBUG) || defined(OIIO_CI) || defined(OIIO_CODE_COVERAGE) - // For the sake of test time, reduce the default iterations for DEBUG, - // CI, and code coverage builds. Explicit use of --iters or --trials - // will override this, since it comes before the getargs() call. - iterations /= 10; - ntrials = 1; + // For the sake of test time, reduce the default number of benchmarking + // trials and iterations for DEBUG, CI, and code coverage builds. Explicit + // use of --iters or --trials will override this, since it comes before + // the getargs() call. + if (Strutil::eval_as_bool(Sysutil::getenv("OpenImageIO_CI")) +#if !defined(NDEBUG) || defined(OIIO_CODE_COVERAGE) + || true #endif + ) { + iterations /= 10; + ntrials = 1; + } getargs(argc, argv); From 7b8450f047ef2a974ddfc358e528ce55f4b2b41d Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 31 Jan 2026 10:26:18 -0800 Subject: [PATCH 3/3] Try to mask array length from compiler to inhibit elision of the bounds check Signed-off-by: Larry Gritz --- src/libutil/span_test.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libutil/span_test.cpp b/src/libutil/span_test.cpp index 2953d7140a..2e030214e4 100644 --- a/src/libutil/span_test.cpp +++ b/src/libutil/span_test.cpp @@ -19,6 +19,10 @@ using namespace OIIO; static int iterations = 100000; static int ntrials = 5; +// Intentionally not static so the compiler can't optimize away its value +int Nlen_unknown = 0; + + static void getargs(int argc, char* argv[]) @@ -33,6 +37,9 @@ getargs(int argc, char* argv[]) iterations)); ap.arg("--trials %d", &ntrials).help("Number of trials"); + // Fake option to hide from compiler how big it will be + ap.arg("--unknown %d", &Nlen_unknown).hidden(); + ap.parse_args(argc, (const char**)argv); } @@ -489,27 +496,28 @@ benchmark_span() // bench.work(N); std::array fstdarr; std::fill(fstdarr.begin(), fstdarr.end(), 1.0f); + size_t Nlen = Nlen_unknown ? size_t(Nlen_unknown) : N; bench("pointer operator[]", [&]() { float* fptr(fstdarr.data()); float t = 0.0f; - for (size_t i = 0; i < N; ++i) + for (size_t i = 0; i < Nlen; ++i) DoNotOptimize(t += fptr[i]); }); bench("std::array operator[]", [&]() { float t = 0.0f; - for (size_t i = 0; i < N; ++i) + for (size_t i = 0; i < Nlen; ++i) DoNotOptimize(t += fstdarr[i]); }); bench("span operator[]", [&]() { span fspan(fstdarr); float t = 0.0f; - for (size_t i = 0; i < N; ++i) + for (size_t i = 0; i < Nlen; ++i) DoNotOptimize(t += fspan[i]); }); bench("span unsafe indexing", [&]() { span fspan(fstdarr); float t = 0.0f; - for (size_t i = 0; i < N; ++i) + for (size_t i = 0; i < Nlen; ++i) DoNotOptimize(t += fspan.data()[i]); }); bench("span range", [&]() {