int: span and range checking enhancements (AcademySoftwareFoundation#…

…4125)

* New header span_util.h with span-related helpers: 
  - make_span/make_cspan helps for converting vectors & arrays to spans
when passing to templated functions.
  - spancpy, spanset, spanzero are memory-safe, range-checking
substitutions for memcpy, memset when working with spans.
* Add range check DASSERT to span access in debug mode. There were a
couple spots where we were taking the address of an out-of-range value
that needed to be adjusted to use a better idiom.
* ParamValue: extra OIIO_DASSERT range check for get() method.
* Simplify constructors by replacing `&x[0]` with using `data()`, which
is always well-defined.
* Fix typos in span.h comments

The range checking imposes no extra cost for release/optimized builds.
But it might help us for CI analysis and any time we're debugging.

Signed-off-by: Larry Gritz <lg@larrygritz.com>

src/include/OpenImageIO/paramlist.h

@@ -226,6 +226,8 @@ class OIIO_UTIL_API ParamValue {
     // a std::string.
     template<typename T> const T& get(int i = 0) const noexcept
     {
+        OIIO_DASSERT(i >= 0 && i < int(m_nvalues * m_type.basevalues())
+                     && "OIIO::ParamValue::get() range check");
         return (reinterpret_cast<const T*>(data()))[i];
     }
 

src/include/OpenImageIO/span.h

@@ -118,15 +118,15 @@ class span {
     /// Construct from std::vector<T>.
     template<class Allocator>
     constexpr span (std::vector<T, Allocator> &v)
-        : m_data(v.size() ? &v[0] : nullptr), m_size(v.size()) {
+        : m_data(v.data()), m_size(v.size()) {
     }
 
     /// Construct from `const std::vector<T>.` This turns
     /// `const std::vector<T>` into a `span<const T>` (the span isn't const,
     /// but the data it points to will be).
     template<class Allocator>
     span (const std::vector<value_type, Allocator> &v)
-        : m_data(v.size() ? &v[0] : nullptr), m_size(v.size()) { }
+        : m_data(v.data()), m_size(v.size()) { }
 
     /// Construct from mutable element std::array
     template <size_t N>
@@ -138,7 +138,7 @@ class span {
     constexpr span (const std::array<value_type, N>& arr)
         : m_data(arr.data()), m_size(N) {}
 
-    /// Construct an span from an initializer_list.
+    /// Construct a span from an initializer_list.
     constexpr span (std::initializer_list<T> il)
         : span (il.begin(), il.size()) { }
 
@@ -189,8 +189,14 @@ class span {
 
     constexpr pointer data() const noexcept { return m_data; }
 
-    constexpr reference operator[] (size_type idx) const { return m_data[idx]; }
-    constexpr reference operator() (size_type idx) const { return m_data[idx]; }
+    constexpr reference operator[] (size_type idx) const {
+        OIIO_DASSERT(idx < m_size && "OIIO::span::operator[] range check");
+        return m_data[idx];
+    }
+    constexpr reference operator() (size_type idx) const {
+        OIIO_DASSERT(idx < m_size && "OIIO::span::operator() range check");
+        return m_data[idx];
+    }
     reference at (size_type idx) const {
         if (idx >= size())
             throw (std::out_of_range ("OpenImageIO::span::at"));
@@ -249,8 +255,8 @@ OIIO_CONSTEXPR14 bool operator!= (span<T,X> l, span<U,Y> r) {
 
 /// span_strided<T> : a non-owning, mutable reference to a contiguous
 /// array with known length and optionally non-default strides through the
-/// data.  An span_strided<T> is mutable (the values in the array may
-/// be modified), whereas an span_strided<const T> is not mutable.
+/// data.  A span_strided<T> is mutable (the values in the array may
+/// be modified), whereas a span_strided<const T> is not mutable.
 template <typename T, oiio_span_size_type Extent = dynamic_extent>
 class span_strided {
     static_assert (std::is_array<T>::value == false,
@@ -294,20 +300,20 @@ class span_strided {
     /// Construct from std::vector<T>.
     template<class Allocator>
     OIIO_CONSTEXPR14 span_strided (std::vector<T, Allocator> &v)
-        : span_strided(v.size() ? &v[0] : nullptr, v.size(), 1) {}
+        : span_strided(v.data(), v.size(), 1) {}
 
     /// Construct from const std::vector<T>. This turns const std::vector<T>
-    /// into an span_strided<const T> (the span_strided isn't
+    /// into a span_strided<const T> (the span_strided isn't
     /// const, but the data it points to will be).
     template<class Allocator>
     constexpr span_strided (const std::vector<value_type, Allocator> &v)
-        : span_strided(v.size() ? &v[0] : nullptr, v.size(), 1) {}
+        : span_strided(v.data(), v.size(), 1) {}
 
-    /// Construct an span from an initializer_list.
+    /// Construct a span from an initializer_list.
     constexpr span_strided (std::initializer_list<T> il)
         : span_strided (il.begin(), il.size()) { }
 
-    /// Initialize from an span (stride will be 1).
+    /// Initialize from a span (stride will be 1).
     constexpr span_strided (span<T> av)
         : span_strided(av.data(), av.size(), 1) { }
 

src/include/OpenImageIO/span_util.h

@@ -0,0 +1,157 @@
+// Copyright Contributors to the OpenImageIO project.
+// SPDX-License-Identifier: Apache-2.0
+// https://github.com/AcademySoftwareFoundation/OpenImageIO
+
+#pragma once
+
+#include <type_traits>
+
+#include <OpenImageIO/span.h>
+
+
+OIIO_NAMESPACE_BEGIN
+
+// Explanation about make_span/make_cspan:
+//
+// If you want to write a function that takes a span of a known type, you can
+// do so and call it with any of the kinds of containers that you could
+// construct a span from. For example:
+//
+//     void myfunc(span<float> x) { ... }
+//
+//     std::vector<float> v;
+//     myfunc(v);   // OK
+//     float arr[10];
+//     myfunc(arr); // OK
+//
+// But if you want to write a templated function that takes a span of the
+// templated type, you can't do that. For example:
+//
+//     template<typename T>
+//     void myfunc(span<T> x) { ... }
+//
+//     std::vector<float> v;
+//     myfunc(v);   // ERROR
+//     float arr[10];
+//     myfunc(arr); // ERROR
+//
+// The problem is that span<T> is not span<float>, so the compiler can't
+// deduce the template type. You can't even explicitly cast it:
+//
+//     myfunc(span<float>(v));   // ERROR
+//     myfunc(span<float>(arr)); // ERROR
+//
+// The solution is to write `make_span()` and `make_cspan()` function
+// templates that will deduce the template type and return the correct kind of
+// span. So we do so for the particular cases of std::vector<T> and T[N]
+// arrays. This is not a complete solution, but it's enough for our purposes.
+
+// Helpers: make spans out of common containers
+template<typename T, class Allocator>
+inline constexpr span<T>
+make_span(std::vector<T, Allocator>& arg)  // span from vector
+{
+    return { arg };
+}
+
+template<typename T, class Allocator>
+inline constexpr cspan<T>
+make_cspan(const std::vector<T, Allocator>& arg)  // cspan from vector
+{
+    return { arg };
+}
+
+
+template<typename T, size_t N>
+inline constexpr span<T>
+make_span(T (&arg)[N])  // span from C array of known length
+{
+    return { arg };
+}
+
+template<typename T, size_t N>
+inline constexpr cspan<T>
+make_cspan(T (&arg)[N])  // cspan from C array of known length
+{
+    return { arg };
+}
+
+template<typename T>
+inline constexpr cspan<T>
+make_cspan(const T& arg)  // cspan from a single value
+{
+    return { &arg, 1 };
+}
+
+
+
+/// Try to copy `n` items of type `T` from `src[srcoffset...]` to
+/// `dst[dstoffset...]`. Don't read or write outside the respective span
+/// boundaries. Return the number of items actually copied, which should be
+/// `n` if the operation was fully successful, but may be less if the request
+/// could not be satisfied while staying within the span bounds.
+///
+/// This is intended to be used as a memory-safe replacement for memcpy if
+/// you're using spans.
+template<typename T>
+size_t
+spancpy(span<T> dst, size_t dstoffset, cspan<T> src, size_t srcoffset, size_t n)
+{
+    // Where do the requests end (limited by span boundaries)?
+    size_t dstend = std::min(dstoffset + n, std::size(dst));
+    size_t srcend = std::min(srcoffset + n, std::size(src));
+    // How many can/should we copy?
+    size_t ndst = dstend - dstoffset;
+    size_t nsrc = srcend - srcoffset;
+    n           = std::min(ndst, nsrc);
+    memcpy(dst.data() + dstoffset, src.data() + srcoffset, n * sizeof(T));
+    return n;
+}
+
+
+
+/// Try to write `n` copies of `val` into `dst[offset...]`. Don't write
+/// outside the span boundaries. Return the number of items actually written,
+/// which should be `n` if the operation was fully successful, but may be less
+/// if the request could not be satisfied while staying within the span
+/// bounds.
+///
+/// This is intended to be used as a memory-safe replacement for memset if
+/// you're using spans.
+template<typename T>
+size_t
+spanset(span<T> dst, size_t offset, const T& val, size_t n)
+{
+    // Where does the request end (limited by span boundary)?
+    size_t dstend = std::min(offset + n, std::size(dst));
+    // How many can/should we copy?
+    n = dstend - offset;
+    for (size_t i = 0; i < n; ++i)
+        dst[offset + i] = val;
+    return n;
+}
+
+
+
+/// Try to fill `n` elements of `dst[offset...]` with 0-valued bytes. Don't
+/// write outside the span boundaries. Return the number of items actually
+/// written, which should be `n` if the operation was fully successful, but
+/// may be less if the request could not be satisfied while staying within the
+/// span bounds.
+///
+/// This is intended to be used as a memory-safe replacement for
+/// `memset(ptr,0,n)` if you're using spans.
+template<typename T>
+size_t
+spanzero(span<T> dst, size_t offset, size_t n)
+{
+    // Where does the request end (limited by span boundary)?
+    size_t dstend = std::min(offset + n, std::size(dst));
+    // How many can/should we copy?
+    n = dstend - offset;
+    memset(dst.data() + offset, 0, n * sizeof(T));
+    return n;
+}
+
+
+OIIO_NAMESPACE_END

src/libOpenImageIO/deepdata.cpp

@@ -577,8 +577,8 @@ DeepData::set_all_samples(cspan<unsigned int> samples)
             set_samples(p, int(samples[p]));
     } else {
         // Data not yet allocated: copy in one shot
-        m_impl->m_nsamples.assign(&samples[0], &samples[m_npixels]);
-        m_impl->m_capacity.assign(&samples[0], &samples[m_npixels]);
+        m_impl->m_nsamples.assign(samples.data(), samples.data() + m_npixels);
+        m_impl->m_capacity.assign(samples.data(), samples.data() + m_npixels);
     }
 }
 

src/libutil/paramlist_test.cpp

@@ -7,6 +7,8 @@
 
 #include <OpenImageIO/Imath.h>
 #include <OpenImageIO/paramlist.h>
+#include <OpenImageIO/span.h>
+#include <OpenImageIO/span_util.h>
 #include <OpenImageIO/unittest.h>