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<float>`.

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 <[email protected]>

Author: lgritz

src/include/OpenImageIO/dassert.h

@@ -9,9 +9,158 @@
 #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 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

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