Use undefined behavior sanitizer (ubsan) as part of CI (AcademySoftwa…

…reFoundation#3565)

In addition to address and leak sanitizers, also turn on the undefined
behavior sanitizer in our CI run. We can do it in the same job as
asan/lsan.

Some things that come along for the ride, including fixes for things
that ubsan uncovered:

* src/build-scripts/ubsan-suppressions.txt can hold suppression list
  (though we don't have any now, it just sets up the space for it for
  future needs).

* Split the "targa" test so that the parts that directly need the OIIO
  python module (which are all related to thumbnails) are now in
  "targa-thumbnails" test. That has to be disabled for the sanitizers,
  since we can't arrange for python to b linked to the sanitizer
  libraries.

* Found vint4::store has a potentially unaligned integer store.

* Found Strutil::concat is not necessarily safe if either of the
  strings are empty.

.github/workflows/ci.yml

@@ -127,10 +127,10 @@ jobs:
             cxx_compiler: clang++
             cxx_std: 17
             python_ver: 3.9
-            setenvs: export SANITIZE=address
-                            OIIO_CMAKE_FLAGS="-DSANITIZE=address -DUSE_PYTHON=0"
+            setenvs: export SANITIZE=address,undefined
+                            OIIO_CMAKE_FLAGS="-DSANITIZE=address,undefined -DUSE_PYTHON=0"
                             CMAKE_BUILD_TYPE=Debug
-                            CTEST_TEST_TIMEOUT=300
+                            CTEST_TEST_TIMEOUT=600
                             CTEST_EXCLUSIONS="broken|cmake-consumer|png-damaged"
           - desc: oldest/hobbled gcc6.3/C++14 py2.7 boost-1.66 exr-2.3 no-sse no-ocio
             # Oldest versions of the dependencies that we can muster, and various

src/build-scripts/ci-startup.bash

@@ -21,6 +21,7 @@ export LD_LIBRARY_PATH=$OpenImageIO_ROOT/lib64:$LD_LIBRARY_PATH
 export OIIO_LIBRARY_PATH=$OpenImageIO_ROOT/lib
 export LSAN_OPTIONS=suppressions=$PWD/src/build-scripts/nosanitize.txt
 export ASAN_OPTIONS=print_suppressions=0
+export UBSAN_OPTIONS=suppressions=$PWD/src/build-scripts/ubsan-suppressions.txt
 
 export PYTHON_VERSION=${PYTHON_VERSION:="2.7"}
 export PYTHONPATH=$OpenImageIO_ROOT/lib/python${PYTHON_VERSION}/site-packages:$PYTHONPATH

src/build-scripts/ci-test.bash

@@ -7,7 +7,7 @@ set -ex
 : ${CTEST_EXCLUSIONS:="broken"}
 : ${CTEST_TEST_TIMEOUT:=180}
 
-$OpenImageIO_ROOT/bin/oiiotool --help
+$OpenImageIO_ROOT/bin/oiiotool --help || /bin/true
 
 echo "Parallel test ${CTEST_PARALLEL_LEVEL}"
 echo "Default timeout ${CTEST_TEST_TIMEOUT}"

src/cmake/testing.cmake

@@ -311,6 +311,11 @@ macro (oiio_add_all_tests)
     oiio_add_tests (targa
                     ENABLEVAR ENABLE_TARGA
                     IMAGEDIR oiio-images)
+    if (USE_PYTHON AND NOT SANITIZE)
+        oiio_add_tests (targa-thumbnail
+                    ENABLEVAR ENABLE_TARGA
+                    IMAGEDIR oiio-images)
+    endif()
     oiio_add_tests (tiff-suite tiff-depths tiff-misc
                     IMAGEDIR oiio-images/libtiffpic)
     oiio_add_tests (webp

src/doc/Doxyfile

@@ -2157,6 +2157,7 @@ INCLUDE_FILE_PATTERNS  = *.h
 PREDEFINED             = DOXYGEN_SHOULD_SKIP_THIS \
                          OIIO_DOXYGEN:=1 \
                          OIIO_NO_SANITIZE_ADDRESS:= \
+                         OIIO_NO_SANITIZE_UNDEFINED:= \
                          OIIO_API:= \
                          OIIO_UTIL_API:= \
                          OIIOC_API:= \

src/include/OpenImageIO/platform.h

@@ -489,14 +489,25 @@
 // false positives that you can't easily get rid of.
 // This should work for any clang >= 3.3 and gcc >= 4.8, which are
 // guaranteed by our minimum requirements.
-#if defined(__clang__) || (defined(__GNUC__) && !defined(__INTEL_COMPILER)) \
+#if defined(__clang__) || (OIIO_GNUC_VERSION > 90000 && !defined(__INTEL_COMPILER)) \
                        || __has_attribute(no_sanitize_address)
 #    define OIIO_NO_SANITIZE_ADDRESS __attribute__((no_sanitize_address))
 #else
 #    define OIIO_NO_SANITIZE_ADDRESS
 #endif
 
 
+// OIIO_NO_SANITIZE_UNDEFINED can be used to mark a function that you don't
+// want undefined behavior sanitizer to catch. Only use this if you know there
+// are false positives that you can't easily get rid of.
+#if defined(__clang__) || (OIIO_GNUC_VERSION > 90000 && !defined(__INTEL_COMPILER)) \
+                       || __has_attribute(no_sanitize)
+#    define OIIO_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize("undefined")))
+#else
+#    define OIIO_NO_SANITIZE_UNDEFINED
+#endif
+
+
 // OIIO_RETURNS_NONNULL following a function declaration of a function
 // indicates that the pointer returned by the function is guaranteed to
 // never be nullptr.

src/include/OpenImageIO/simd.h

@@ -4712,7 +4712,7 @@ OIIO_FORCEINLINE void vint4::store (unsigned char *values) const {
     vint4 merged2 = shuffle_sse<2,2,2,2>(merged);  // CD00 ...
     vint4 shifted2 = merged2 << 16;                // 00CD ...
     vint4 result = merged | shifted2;              // ABCD ...
-    *(int*)values = result[0]; //extract<0>(result);
+    memcpy(values, &result, 4);  // memcpy because it may be unaligned
     // At this point, values[] should hold A,B,C,D
 #else
     SIMD_DO (values[i] = m_val[i]);

src/libutil/strutil.cpp

@@ -46,6 +46,11 @@
 #    define STBI__ASAN OIIO_NO_SANITIZE_ADDRESS
 #endif
 #define stbsp__uintptr std::uintptr_t
+
+#ifdef OpenImageIO_SANITIZE
+#    define STB_SPRINTF_NOUNALIGNED
+#endif
+
 #include "stb_sprintf.h"
 
 
@@ -776,16 +781,15 @@ Strutil::split(string_view str, std::vector<string_view>& result,
 std::string
 Strutil::concat(string_view s, string_view t)
 {
-    size_t sl  = s.size();
-    size_t tl  = t.size();
+    size_t sl = s.size();
+    size_t tl = t.size();
+    if (sl == 0)
+        return t;
+    if (tl == 0)
+        return s;
     size_t len = sl + tl;
-    std::unique_ptr<char[]> heap_buf;
-    char local_buf[256];
-    char* buf = local_buf;
-    if (len > sizeof(local_buf)) {
-        heap_buf.reset(new char[len]);
-        buf = heap_buf.get();
-    }
+    char* buf;
+    OIIO_ALLOCATE_STACK_OR_HEAP(buf, char, len);
     memcpy(buf, s.data(), sl);
     memcpy(buf + sl, t.data(), tl);
     return std::string(buf, len);

testsuite/targa-thumbnail/ref/out.txt

@@ -0,0 +1,20 @@
+Comparing "CBW8.TGA.tif" and "ref/CBW8.TGA.tif"
+PASS
+Comparing "CCM8.TGA.tif" and "ref/CCM8.TGA.tif"
+PASS
+Comparing "CTC16.TGA.tif" and "ref/CTC16.TGA.tif"
+PASS
+Comparing "CTC24.TGA.tif" and "ref/CTC24.TGA.tif"
+PASS
+Comparing "CTC32.TGA.tif" and "ref/CTC32.TGA.tif"
+PASS
+Comparing "UBW8.TGA.tif" and "ref/UBW8.TGA.tif"
+PASS
+Comparing "UCM8.TGA.tif" and "ref/UCM8.TGA.tif"
+PASS
+Comparing "UTC16.TGA.tif" and "ref/UTC16.TGA.tif"
+PASS
+Comparing "UTC24.TGA.tif" and "ref/UTC24.TGA.tif"
+PASS
+Comparing "UTC32.TGA.tif" and "ref/UTC32.TGA.tif"
+PASS

testsuite/targa-thumbnail/run.py

@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import OpenImageIO as oiio
+import os
+
+redirect = " >> out.txt 2>> out.err.txt"
+
+imagedir = OIIO_TESTSUITE_IMAGEDIR + "/targa"
+
+files = [ "CBW8.TGA", "CCM8.TGA", "CTC16.TGA", "CTC24.TGA", "CTC32.TGA",
+          "UBW8.TGA", "UCM8.TGA", "UTC16.TGA", "UTC24.TGA", "UTC32.TGA",
+          "round_grill.tga" ]
+
+
+# Test ability to extract thumbnails
+outputs = [ ]
+for f in files:
+    fin = imagedir + "/" + f
+    buf = oiio.ImageBuf(fin)
+    if buf.has_thumbnail :
+        thumbname = f + ".tif"
+        command += run_app(pythonbin + " src/extractthumb.py " + fin + " " + thumbname)
+        outputs += [ thumbname ]
+
+outputs += [ 'out.txt' ]

testsuite/targa/ref/out.txt

@@ -195,23 +195,3 @@ src/1x1.tga          :    1 x    1, 3 channel, uint2 targa
     targa:version: 1
 Comparing "src/1x1.tga" and "1x1.tga"
 PASS
-Comparing "CBW8.TGA.tif" and "ref/CBW8.TGA.tif"
-PASS
-Comparing "CCM8.TGA.tif" and "ref/CCM8.TGA.tif"
-PASS
-Comparing "CTC16.TGA.tif" and "ref/CTC16.TGA.tif"
-PASS
-Comparing "CTC24.TGA.tif" and "ref/CTC24.TGA.tif"
-PASS
-Comparing "CTC32.TGA.tif" and "ref/CTC32.TGA.tif"
-PASS
-Comparing "UBW8.TGA.tif" and "ref/UBW8.TGA.tif"
-PASS
-Comparing "UCM8.TGA.tif" and "ref/UCM8.TGA.tif"
-PASS
-Comparing "UTC16.TGA.tif" and "ref/UTC16.TGA.tif"
-PASS
-Comparing "UTC24.TGA.tif" and "ref/UTC24.TGA.tif"
-PASS
-Comparing "UTC32.TGA.tif" and "ref/UTC32.TGA.tif"
-PASS

testsuite/targa/run.py

@@ -1,8 +1,5 @@
 #!/usr/bin/env python
 
-import OpenImageIO as oiio
-import os
-
 redirect = " >> out.txt 2>> out.err.txt"
 
 imagedir = OIIO_TESTSUITE_IMAGEDIR + "/targa"
@@ -14,16 +11,6 @@
     command += rw_command (imagedir, f)
 
 
-# Test ability to extract thumbnails
-outputs = [ ]
-for f in files:
-    fin = imagedir + "/" + f
-    buf = oiio.ImageBuf(fin)
-    if buf.has_thumbnail :
-        thumbname = f + ".tif"
-        command += run_app(pythonbin + " src/extractthumb.py " + fin + " " + thumbname)
-        outputs += [ thumbname ]
-
 # Test corrupted files
 command += iconvert("-v src/crash1.tga crash1.exr", failureok = True)
 command += oiiotool("--oiioattrib try_all_readers 0 src/crash2.tga -o crash2.exr", failureok = True)