[BUG] Divide by zero in iterator on EXR ImageBuf

[rasmusbonnedal]

Describe the bug
When creating an ImageBuf from an EXR and then using a non-const iterator on it I get division by zero. This issue appeared when we upgraded from v2.3.12.0 to v2.4.8.0

To Reproduce

1. Create an ImageBuf from an exr file
2. Create an ImageBuf::Iterator from the imagebuf
3. I get exception integer division by zero

Expected behavior
Not divide by zero

Evidence
Code to reproduce:

OIIO::ImageBuf buf("testsuite\\oiiotool\\ref\\abs.exr");
OIIO::ImageBuf::Iterator<float> it(buf);

Error:

Unhandled exception at 0x00007FF8FF8FC22C (OpenImageIO_d.dll) in mtlx_baker-test.exe: 0xC0000094: Integer division by zero.
imagebuf.cpp:2856                     int xtile  = (x - m_spec.x) / tw;

Platform information:

• OIIO 2.4.8.0
• OS: Windows 11
• C++ compiler: Visual Studio 2019
• Any non-default build flags when you build OIIO:

            "OIIO_BUILD_TESTS": "0",
            "OIIO_DOWNLOAD_MISSING_TESTDATA": "0",
            "BUILD_DOCS": "0",
            "USE_PYTHON": "0"

[rasmusbonnedal]

It seems I was wrong that this happens only tiled exr images. It seems to happen with most exr files. I use one from the OIIO testsuite in the repro sample.

[rasmusbonnedal]

A workaround I found is to make the imagebuf writable before creating the iterator. This does not cause exception:

OIIO::ImageBuf buf("testsuite\\oiiotool\\ref\\abs.exr");
buf.make_writable(true);
OIIO::ImageBuf::Iterator<float> it(buf);

[lgritz]

Sorry about letting this languish for so long. Can you supply a specific exr image that can reproduce the error?

[rasmusbonnedal]

I couldn't reproduce this on master but here is a self-contained repro on v2.4.8.0:

#include <OpenImageIO/benchmark.h>
#include <OpenImageIO/filesystem.h>
#include <OpenImageIO/imagebuf.h>
#include <OpenImageIO/imagebufalgo.h>
#include <OpenImageIO/imagecache.h>
#include <OpenImageIO/imageio.h>
#include <OpenImageIO/unittest.h>

#include <iostream>

using namespace OIIO;

int main(char, char**)
{
    // Make 4x4 1-channel float source image, value 0.5, write it.
    char srcfilename[] = "tmp_f1.exr";
    ImageSpec fsize1(4, 4, 1, TypeFloat);
    ImageBuf src(fsize1);
    ImageBufAlgo::fill(src, 0.5f);
    src.write(srcfilename);
    
    ImageBuf buf(srcfilename);
    std::cout << "OIIO: " << OIIO_VERSION_STRING << std::endl;
    std::cout << "nativespec().tile_width = " << buf.nativespec().tile_width
              << std::endl;
    std::cout << "spec().tile_width = " << buf.spec().tile_width
              << std::endl;

    ImageBuf::Iterator<float> it(buf);

    std::cout << "nativespec().tile_width = " << buf.nativespec().tile_width
              << std::endl;
    std::cout << "spec().tile_width = " << buf.spec().tile_width << std::endl;
    std::cout << "OK" << std::endl;
    Filesystem::remove(srcfilename);
}

This gives me the following output:

C:\WORK\GIT\oiio-exrzerobug\build3 ((HEAD detached at v2.4.8.0))>bin\RelWithDebInfo\divide_zero.exe
OIIO: 2.4.8.0
nativespec().tile_width = 0
spec().tile_width = 4

C:\WORK\GIT\oiio-exrzerobug\build3 ((HEAD detached at v2.4.8.0))>

[rasmusbonnedal]

It repros up to including tag v2.5.0.3-dev:

C:\WORK\GIT\oiio-exrzerobug\build3 ((HEAD detached at v2.5.0.3-dev))>bin\RelWithDebInfo\divide_zero.exe
OIIO: 2.5.0.3
nativespec().tile_width = 0
spec().tile_width = 4

I couldn't build v2.5.3.0-beta1, but v2.5.3.1-beta2 and onwards doesnt repro:

C:\WORK\GIT\oiio-exrzerobug\build4 ((HEAD detached at v2.5.3.1-beta2))>bin\RelWithDebInfo\divide_zero.exe
OIIO: 2.5.3.1beta
nativespec().tile_width = 0
spec().tile_width = 0
nativespec().tile_width = 0
spec().tile_width = 0
OK
OK

[rasmusbonnedal]

It repros on Ubuntu as well:

OIIO: 2.4.8.0
nativespec().tile_width = 0
spec().tile_width = 4
Floating point exception

[rasmusbonnedal]

It seems to me #3986 caused it to not repro. If the ImageBuf uses an ImageCache it still repros on master:

(Change row 22 in the repro to ImageBuf buf(srcfilename, ImageCache::create());)

C:\WORK\GIT\oiio-exrzerobug\build4 (master)>bin\RelWithDebInfo\divide_zero.exe
OIIO: 2.6.0.0beta
nativespec().tile_width = 0
spec().tile_width = 4

[lgritz]

I can reproduce, stay tuned.

[lgritz]

I believe he real fix is #3770

[rasmusbonnedal]

I tested the fix, it works. Nice!

[lgritz]

Apparently nowhere in the testsuite -- or in my experience of anything I'd tried -- was there an example of using a non-const Iterator with a (presumably read-only) ImageBuf referencing a file. I'd always paired ImageBuf("file") with ConstIterator. Using a mutable Iterator with a file-based IB is legal, but it's unusual and probably inefficient, because the mere creation of the Iterator will force the IB it references to immediately read the entire file and store it locally, severing any connection to the ImageCache (which of course is a pass-through read-only device).

Thinking aloud: I wonder if the conversion of the IB to local/writable can be done lazily -- that is, only when you try to write into the IB, not when the Iterator is created. That would make it equally efficient to mistakenly use an Iterator in this case and only have the expense if/when you actually tried to write through the iterator. Maybe it would even eliminate the need for ConstIterator entirely, if the constant checking of "is this iterator referencing a writable IB" every time a write occurs doesn't create a big expense when traversing an image. I'll take a look. It would certainly be a nice simplification to not need two kinds of IB iterators.