api: OIIO_CONTRACT_ASSERT and other hardening improvements (AcademySoftwareFoundation#5006)

lgritz · ssh4net · commit 9897658ffdb0 · 2026-04-02T13:34:24.000+09:00
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:

* 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).

* 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.

* By default, the contract failure response is Enforce, unless it's both
a release build and the hardening level is set to None (in which case
the response will be Ignore). But it's also overrideable optionally on a
per-translation-unit basis by setting OIIO_ASSERTION_RESPONSE_DEFAULT
before any OIIO headers are included (though obviously that only applies
to inline functions or templates, not to any already-compiled code in
the library).

* 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
tiny bit of overhead, since the default is "enforce" for release builds
(previously, using OIIO_DASSERT, it did no checks for release builds).
But the benchmarks seem to idicate that the perf difference is barely
measurable.

I added some benchmarking that proves that the bounds check adds a
minute overhead to an element access for a trivial `span&lt;float&gt;`, maybe
even indescernable. Here are benchmarks comparing raw pointer access,
std::array access, span access with the new checks, span access
carefully bypassing the tests.

Linux workstation, gcc-11, on my work computer:

    pointer operator[]:     647.8 ns (+/- 0.1ns)
    std::array operator[]:  647.8 ns (+/- 0.1ns)
    span operator[] :       657.6 ns (+/- 0.5ns)
    span unsafe indexing:   648.2 ns (+/- 0.2ns)
    span range      :       648.1 ns (+/- 0.1ns)

These are the most stable tests I have, with the least trial-to-trial
variation, and show about a 1.5% speed hit on the bounds-checked span
access itself, which I think will be truly un-measurable in the context
of being interleaved with any other operations that you do with the data
you pull from the span.

Mac Intel, Apple Clang 17, on my (old) personal laptop: (much more
variable timing, probably from MacOS scheduler quirks)

    pointer operator[]:     929.2 ns (+/- 6.7ns)
    std::array operator[]:  913.1 ns (+/- 20.6ns)
    span operator[] :       905.8 ns (+/- 13.3ns)
    span unsafe indexing:   913.9 ns (+/- 16.6ns)
    span range      :       916.4 ns (+/- 20.3ns)

You can see that here there is no obvious penalty, in fact it appears a
little faster, but all within the timing uncertainty of the multiple
trials, so statistically it's hard to discern any penalty.

And a couple more for good measure from our CI, but note that because
these are uncontrolled machines somewhere on the GitHub cloud, the
timings might not be as reliable:

Windows, MSVS 2022:

    pointer operator[]:    3716.3 ns (+/- 6.3ns)
    std::array operator[]: 3715.5 ns (+/- 3.4ns)
    span operator[] :      3715.6 ns (+/- 2.6ns)
    span unsafe indexing:  3712.1 ns (+/- 0.7ns)
    span range      :      3714.2 ns (+/- 2.9ns)

Linux, gcc-14, C++20:

    pointer operator[]:    1130.9 ns (+/- 0.2ns),  884.2 k/s
    std::array operator[]: 1132.0 ns (+/- 0.4ns),  883.4 k/s
    span operator[] :      1133.7 ns (+/- 0.4ns),  882.1 k/s
    span unsafe indexing:  1134.2 ns (+/- 1.6ns),  881.7 k/s
    span range      :      1133.9 ns (+/- 0.7ns),  881.9 k/s

MacOS ARM:

    pointer operator[]:    3456.6 ns (+/- 7.5ns)
    std::array operator[]: 3466.8 ns (+/- 12.2ns)
    span operator[] :      3610.9 ns (+/- 11.0ns)
    span unsafe indexing:  3607.4 ns (+/- 4.9ns)
    span range      :      3612.4 ns (+/- 12.2ns)

Windows with MSVS and Linux with newer g++ don't appear to show any
penalty, and the bracketing of trial times indicates that maybe it's
consistent enough to be meaningful? I can't think of anything I'm doing
wrong here that would throw off the timing or disable the range checking
on these tests.

For MacOS ARM, the span looks like it has about a 4% penalty versus raw
pointers? But OTOH, span bounds-checked vs non-checked vs range-for are
all the same, so maybe the speed vs raw pointer is something else
entirely?

Also please note that a preferred way to avoid these extra bounds checks
entirely is to change an index-oriented loop like

    span s;
    for (size_t i = 0; i &lt; s.size(); ++i)
        foo(s[i]);   // maybe bounds check on each iteration?

to a range based loop:

    span s;
    for (auto&amp; v : s)
        foo(v);

which should be inherently safe and require no in-loop checks at all.

---------

Signed-off-by: Larry Gritz &lt;lg@larrygritz.com&gt;
Signed-off-by: Vlad (Kuzmin) Erium &lt;libalias@gmail.com&gt;
Signed-off-by: Vlad &lt;shaamaan@gmail.com&gt;
diff --git 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
@@ -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
@@ -9,9 +9,161 @@
 #include <cstdio>
 #include <cstdlib>
 
+#include <OpenImageIO/oiioversion.h>
 #include <OpenImageIO/platform.h>
 
 
+
+// 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 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_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.
+#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, 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 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_IGNORE
+#    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
@@ -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
@@ -271,18 +271,30 @@ template<typename T, size_t Rank = 4> 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
@@ -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
@@ -14,6 +14,7 @@
 #include <stdexcept>
 #include <string>
 
+#include <OpenImageIO/dassert.h>
 #include <OpenImageIO/export.h>
 #include <OpenImageIO/oiioversion.h>
 #include <OpenImageIO/platform.h>
@@ -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
@@ -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
diff --git a/src/libutil/strutil.cpp b/src/libutil/strutil.cpp
