Slicer crashes on Windows when volume with NaN voxel value is loaded

[lassoan]

Summary

Slicer crashes when attempting to load a volume that has some NaN-valued voxels.

The crash is reproducible with vtkSlicerVolumesLogicTest1_TestNAN test, which fails on Windows since February 2024 due to a crash in VTK's vtkImageHistogramExecute function in vtkImageHistogram.cxx. This code should return a value between xmin and xmax if x is nan:

x = (x > xmin ? x : xmin);
x = (x < xmax ? x : xmax);

However, x remains nan, therefore xi bin index is set to a negative number, and so outPtr[xi]++; causes access violation. x remains nan due to an MSVC compiler bug: https://stackoverflow.com/questions/77799136/msvc-2022-wrong-optimization-of-maxnan-somenumber

Steps to reproduce

Load testNANInVolume.nrrd => Slicer crashes

Run vtkSlicerVolumesLogicTest1_TestNAN test => fails (for example: https://slicer.cdash.org/tests/27309466)

Environment

• Slicer version: Slicer-5.7.0-204-04-15
• Operating system: Windows, MSVC toolset 19.39.33523

[lassoan]

The crash could be fixed in VTK by adding an explicit if (!std::isnan(x)) check, but maybe it is OK to just wait for a compiler update, as NaN-valued volumes are not very common.

//------------------------------------------------------------------------------
template <class T>
void vtkImageHistogramExecute(vtkImageHistogram* self, vtkImageData* inData,
  vtkImageStencilData* stencil, T* inPtr, int extent[6], vtkIdType* outPtr, int binRange[2],
  double o, double s, int component, int threadId)
{
  vtkImageStencilIterator<T> inIter(inData, stencil, extent, ((threadId == 0) ? self : nullptr));

  // set up components
  int nc = inData->GetNumberOfScalarComponents();
  int c = component;
  if (c < 0)
  {
    nc = 1;
    c = 0;
  }

  // compute shift/scale values for fast bin computation
  double xmin = binRange[0];
  double xmax = binRange[1];
  double xshift = -o;
  double xscale = 1.0 / s;

  // iterate over all spans in the stencil
  while (!inIter.IsAtEnd())
  {
    if (inIter.IsInStencil())
    {
      inPtr = inIter.BeginSpan();
      T* inPtrEnd = inIter.EndSpan();

      // iterate over all voxels in the span
      if (inPtr != inPtrEnd)
      {
        int n = static_cast<int>((inPtrEnd - inPtr) / nc);
        inPtr += c;
        do
        {
          double x = *inPtr;

          if (!std::isnan(x))
          {
            x += xshift;
            x *= xscale;

            // x may be NaN but we don't want xmin and xmax
            // to be ever end up being set to NaN.
            // Therefore x must be used as a second operand
            // of the ternary operator (because result of comparing
            // NaN to any other value is always false).
            x = (x > xmin ? x : xmin);
            x = (x < xmax ? x : xmax);

            int xi = static_cast<int>(x + 0.5);

            outPtr[xi]++;
          }

          inPtr += nc;
        } while (--n);
      }
    }
    inIter.NextSpan();
  }
}

[pieper]

If this crashes with an officially supported compiler then IMO it would be better to fix it rather than waiting for a compiler fix. Research codes sometimes generate NaNs. But it's not a super high priority for me.

[lassoan]

Yes, this crash happens with the latest MSVC compiler. A new version is expected within a few months, which may or may not have a fix for this.