api: OIIO_CONTRACT_ASSERT and other hardening improvements

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

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)

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

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;
     }

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

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]; }

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; }
 

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