Address issues reported by Undefined Behavior Sanitizer running IlmImfTest

IlmBase/IlmThread/IlmThread.cpp

@@ -72,6 +72,12 @@ Thread::~Thread ()
         _thread.join ();
 }
 
+void
+Thread::join()
+{
+    if ( _thread.joinable () )
+        _thread.join ();
+}
 
 void
 Thread::start ()
@@ -108,6 +114,12 @@ Thread::start ()
 {
     throw IEX_NAMESPACE::NoImplExc ("Threads not supported on this platform.");
 }
+
+void
+Thread::join ()
+{
+    throw IEX_NAMESPACE::NoImplExc ("Threads not supported on this platform.");
+}
 #   endif
 #endif
 

IlmBase/IlmThread/IlmThread.h

@@ -129,6 +129,11 @@ class Thread
     ILMTHREAD_EXPORT void         start ();
     ILMTHREAD_EXPORT virtual void run () = 0;
 
+    //
+    // wait for thread to exit - must be called before deleting thread
+    //
+    void join();
+
   private:
 
 #ifdef ILMBASE_FORCE_CXX03

IlmBase/IlmThread/IlmThreadPool.cpp

@@ -431,8 +431,10 @@ DefaultThreadPoolProvider::finish ()
     // Join all the threads
     //
     for (size_t i = 0; i != curT; ++i)
+    {
+        _data.threads[i]->join();
         delete _data.threads[i];
-
+    }
     Lock lock1 (_data.taskMutex);
 #ifdef ILMBASE_FORCE_CXX03
     Lock lock2 (_data.stopMutex);

IlmBase/IlmThread/IlmThreadPosix.cpp

@@ -85,6 +85,13 @@ Thread::~Thread ()
     assert (error == 0);
 }
 
+void
+Thread::join ()
+{
+    int error = ::pthread_join (_thread, 0);
+    assert (error == 0);
+}
+
 
 void
 Thread::start ()

IlmBase/IlmThread/IlmThreadWin32.cpp

@@ -85,6 +85,14 @@ Thread::~Thread ()
     assert (ok);
 }
 
+void
+Thread::join ()
+{
+    DWORD status = ::WaitForSingleObject (_thread, INFINITE);
+    assert (status ==  WAIT_OBJECT_0);
+    bool ok = ::CloseHandle (_thread) != FALSE;
+    assert (ok);
+}
 
 void
 Thread::start ()

OpenEXR/IlmImf/ImfCompositeDeepScanLine.cpp

@@ -323,7 +323,6 @@ class LineCompositeTask : public Task
 
 };
 
-
 void
 composite_line(int y,
                int start,
@@ -386,16 +385,18 @@ composite_line(int y,
            {
 
                float value = output_pixel[ _Data->_bufferMap[channel_number] ]; // value to write
-
+               intptr_t base = reinterpret_cast<intptr_t>(it.slice().base);
 
                 // cast to half float if necessary
                if(it.slice().type==OPENEXR_IMF_INTERNAL_NAMESPACE::FLOAT)
                {
-                   * (float *)(it.slice().base + y*it.slice().yStride + x*it.slice().xStride) = value;
+                   float* ptr = reinterpret_cast<float*>(base + y*it.slice().yStride + x*it.slice().xStride);
+                   *ptr  = value;
                }
                else if(it.slice().type==HALF)
                {
-                   * (half *)(it.slice().base + y*it.slice().yStride + x*it.slice().xStride) = half(value);
+                   half* ptr =  reinterpret_cast<half*>(base + y*it.slice().yStride + x*it.slice().xStride);
+                   *ptr = half(value);
                }
 
                channel_number++;

OpenEXR/IlmImf/ImfDeepScanLineInputFile.cpp

@@ -1425,6 +1425,20 @@ DeepScanLineInputFile::readPixels (int scanLine)
 }
 
 
+namespace
+{
+struct I64Bytes
+{
+    uint8_t b[8];
+};
+
+
+union bytesOrInt64
+{
+    I64Bytes b;
+    Int64 i;
+};
+}
 void
 DeepScanLineInputFile::rawPixelData (int firstScanLine,
                                      char *pixelData,
@@ -1507,12 +1521,16 @@ DeepScanLineInputFile::rawPixelData (int firstScanLine,
 
     // copy the values we have read into the output block
     *(int *) pixelData = yInFile;
-    *(Int64 *) (pixelData+4) =sampleCountTableSize;
-    *(Int64 *) (pixelData+12) = packedDataSize;
-
+    bytesOrInt64 tmp;
+    tmp.i=sampleCountTableSize;
+    memcpy(pixelData+4,&tmp.b,8);
+    tmp.i = packedDataSize;
+    memcpy(pixelData+12,&tmp.b,8);
+
     // didn't read the unpackedsize - do that now
-    Xdr::read<StreamIO> (*_data->_streamData->is, *(Int64 *) (pixelData+20));
-
+    Xdr::read<StreamIO> (*_data->_streamData->is,tmp.i);
+    memcpy(pixelData+20,&tmp.b,8);
+
     // read the actual data
     _data->_streamData->is->read(pixelData+28, sampleCountTableSize+packedDataSize);
 

OpenEXR/IlmImf/ImfDeepScanLineOutputFile.cpp

@@ -1411,6 +1411,23 @@ DeepScanLineOutputFile::copyPixels (DeepScanLineInputPart &in)
     copyPixels(*in.file);
 }
 
+
+// helper structure to read Int64 from non 8 byte aligned addresses
+namespace
+{
+struct I64Bytes
+{
+    uint8_t b[8];
+};
+
+
+union bytesOrInt64
+{
+    I64Bytes b;
+    Int64 i;
+};
+}
+
 void
 DeepScanLineOutputFile::copyPixels (DeepScanLineInputFile &in)
 {
@@ -1487,9 +1504,16 @@ DeepScanLineOutputFile::copyPixels (DeepScanLineInputFile &in)
 
         // extract header from block to pass to writePixelData
 
-        Int64 packedSampleCountSize = *(Int64 *) (&data[4]);
-        Int64 packedDataSize = *(Int64 *) (&data[12]);
-        Int64 unpackedDataSize = *(Int64 *) (&data[20]);
+        bytesOrInt64 tmp;
+        memcpy(&tmp.b,&data[4],8);
+        Int64 packedSampleCountSize = tmp.i;
+
+        memcpy(&tmp.b,&data[12],8);
+        Int64 packedDataSize = tmp.i;
+
+        memcpy(&tmp.b,&data[20],8);
+        Int64 unpackedDataSize = tmp.i;
+
         const char * sampleCountTable = &data[0]+28;
         const char * pixelData = sampleCountTable + packedSampleCountSize;
 

OpenEXR/IlmImf/ImfFrameBuffer.cpp

@@ -89,7 +89,7 @@ Slice::Make (
     bool                        xTileCoords,
     bool                        yTileCoords)
 {
-    char* base = reinterpret_cast<char*> (const_cast<void *> (ptr));
+    intptr_t base = reinterpret_cast<intptr_t> (const_cast<void *> (ptr));
     if (xStride == 0)
     {
         switch (type)
@@ -117,7 +117,7 @@ Slice::Make (
 
     return Slice (
         type,
-        base - offx - offy,
+        reinterpret_cast<char*>(base - offx - offy),
         xStride,
         yStride,
         xSampling,

OpenEXR/IlmImf/ImfInputFile.cpp

@@ -308,6 +308,10 @@ bufferedReadPixels (InputFile::Data* ifd, int scanLine1, int scanLine2)
 	    while (modp (yStart, toSlice.ySampling) != 0)
 		++yStart;
 
+
+            intptr_t fromBase = reinterpret_cast<intptr_t>(fromSlice.base);
+            intptr_t toBase = reinterpret_cast<intptr_t>(toSlice.base);
+
             for (int y = yStart;
 		 y <= maxYThisRow;
 		 y += toSlice.ySampling)
@@ -316,14 +320,13 @@ bufferedReadPixels (InputFile::Data* ifd, int scanLine1, int scanLine2)
                 // Set the pointers to the start of the y scanline in
                 // this row of tiles
 		//
-
-                fromPtr = fromSlice.base +
+                fromPtr = reinterpret_cast<char*> (fromBase +
                           (y - tileRange.min.y) * fromSlice.yStride +
-                          xStart * fromSlice.xStride;
+                          xStart * fromSlice.xStride);
 
-                toPtr = toSlice.base +
+                toPtr = reinterpret_cast<char*> (toBase +
                         divp (y, toSlice.ySampling) * toSlice.yStride +
-                        divp (xStart, toSlice.xSampling) * toSlice.xStride;
+                        divp (xStart, toSlice.xSampling) * toSlice.xStride);
 
 		//
                 // Copy all pixels for the scanline in this row of tiles

OpenEXR/IlmImf/ImfMisc.cpp

@@ -1390,9 +1390,10 @@ namespace
 //
 
 struct FBytes { uint8_t b[4]; };
-union bytesOrFloat {
+union bytesUintOrFloat {
   FBytes b;
   float f;
+  unsigned int u;
 } ;
 }
 
@@ -1408,7 +1409,9 @@ convertInPlace (char *& writePtr,
 
         for (size_t j = 0; j < numPixels; ++j)
         {
-            Xdr::write <CharPtrIO> (writePtr, *(const unsigned int *) readPtr);
+            union bytesUintOrFloat tmp;
+            tmp.b = * reinterpret_cast<const FBytes *>( readPtr );
+            Xdr::write <CharPtrIO> (writePtr, tmp.u);
             readPtr += sizeof(unsigned int);
         }
         break;
@@ -1426,7 +1429,7 @@ convertInPlace (char *& writePtr,
 
         for (size_t j = 0; j < numPixels; ++j)
         {
-            union bytesOrFloat tmp;
+            union bytesUintOrFloat tmp;
             tmp.b = * reinterpret_cast<const FBytes *>( readPtr );
             Xdr::write <CharPtrIO> (writePtr, tmp.f);
             readPtr += sizeof(float);

OpenEXR/IlmImf/ImfOutputFile.cpp

@@ -558,13 +558,18 @@ LineBufferTask::execute ()
                     // If necessary, convert the pixel data to Xdr format.
 		    // Then store the pixel data in _ofd->lineBuffer.
                     //
-
-                    const char *linePtr = slice.base +
-                                          divp (y, slice.ySampling) *
+                    // slice.base may be 'negative' but
+                    // pointer arithmetic is not allowed to overflow, so
+                    // perform computation with the non-pointer 'intptr_t' instead
+                    //
+                    intptr_t base =  reinterpret_cast<intptr_t>(slice.base);
+                    intptr_t linePtr =  base + divp (y, slice.ySampling) *
                                           slice.yStride;
 
-                    const char *readPtr = linePtr + dMinX * slice.xStride;
-                    const char *endPtr  = linePtr + dMaxX * slice.xStride;
+                    const char *readPtr = reinterpret_cast<const char*>(linePtr +
+                                          dMinX * slice.xStride);
+                    const char *endPtr  = reinterpret_cast<const char*>(linePtr +
+                                          dMaxX * slice.xStride);
 
                     copyFromFrameBuffer (writePtr, readPtr, endPtr,
                                          slice.xStride, _ofd->format,

OpenEXR/IlmImf/ImfRgbaFile.cpp

@@ -369,6 +369,7 @@ RgbaOutputFile::ToYca::writePixels (int numScanLines)
 			    "\"" << _outputFile.fileName() << "\".");
     }
 
+    intptr_t base = reinterpret_cast<intptr_t>(_fbBase);
     if (_writeY && !_writeC)
     {
 	//
@@ -385,8 +386,9 @@ RgbaOutputFile::ToYca::writePixels (int numScanLines)
 
 	    for (int j = 0; j < _width; ++j)
 	    {
-		_tmpBuf[j] = _fbBase[_fbYStride * _currentScanLine +
-				     _fbXStride * (j + _xMin)];
+		_tmpBuf[j] = *reinterpret_cast<Rgba*>(base + sizeof(Rgba)*
+		(_fbYStride * _currentScanLine +
+				     _fbXStride * (j + _xMin)));
 	    }
 
 	    //
@@ -418,10 +420,13 @@ RgbaOutputFile::ToYca::writePixels (int numScanLines)
 	    // frame buffer into _tmpBuf.
 	    //
 
+            intptr_t base = reinterpret_cast<intptr_t>(_fbBase);
+
 	    for (int j = 0; j < _width; ++j)
 	    {
-		_tmpBuf[j + N2] = _fbBase[_fbYStride * _currentScanLine +
-					  _fbXStride * (j + _xMin)];
+                const Rgba* ptr = reinterpret_cast<const Rgba*>(base+sizeof(Rgba)*
+		(_fbYStride * _currentScanLine + _fbXStride * (j + _xMin)) );
+		_tmpBuf[j + N2] = *ptr;
 	    }
 
 	    //
@@ -1081,9 +1086,13 @@ RgbaInputFile::FromYca::readPixels (int scanLine)
 
     fixSaturation (_yw, _width, _buf2, _tmpBuf);
 
-    for (int i = 0; i < _width; ++i)
-	_fbBase[_fbYStride * scanLine + _fbXStride * (i + _xMin)] = _tmpBuf[i];
 
+    intptr_t base = reinterpret_cast<intptr_t>(_fbBase);
+    for (int i = 0; i < _width; ++i)
+    {
+        Rgba* ptr = reinterpret_cast<Rgba*>(base + sizeof(Rgba)*(_fbYStride * scanLine + _fbXStride * (i + _xMin)));
+        *ptr = _tmpBuf[i];
+    }
     _currentScanLine = scanLine;
 }
 
@@ -1335,10 +1344,11 @@ RgbaInputFile::readPixels (int scanLine1, int scanLine2)
             //
             const Slice* s = _inputFile->frameBuffer().findSlice(_channelNamePrefix + "Y");
             Box2i dataWindow = _inputFile->header().dataWindow();
+            intptr_t base = reinterpret_cast<intptr_t>(s->base);
 
             for( int scanLine = scanLine1  ; scanLine <= scanLine2 ; scanLine++ )
             {
-                char* rowBase = s->base + scanLine*s->yStride;
+                intptr_t rowBase = base + scanLine*s->yStride;
                 for(int x = dataWindow.min.x ; x <= dataWindow.max.x ; ++x )
                 {
                     Rgba* pixel = reinterpret_cast<Rgba*>(rowBase+x*s->xStride);