Skip to content

Commit

Permalink
fix(ImageBuf): fix crash when mutable Iterator used with read-IB (#3997)
Browse files Browse the repository at this point in the history
The following sequence would have a floating point exception:

    // Readable IB backed by ImageCache
    ImageBuf buf("file.exr"),  0, 0, ImageCache::create());

    // Make a mutable iterator, even though it's an image file reference.
    ImageBuf::Iterator<float> it(buf);  <----- CRASHES INSIDE HERE!

The problem was in ImageBuf::IteratorBase::init_ib, several steps taken
if it's a mutable IB looking at a "read-only" IC-backed file, including
calling make_writable() on the buffer. But we neglected to set
m_localpixels to true.

Fixes #3770

---------

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz authored Oct 7, 2023
1 parent 41a192b commit d348067
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 11 deletions.
3 changes: 3 additions & 0 deletions src/include/OpenImageIO/imagebuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,8 @@ class OIIO_API ImageBuf {
return m_ib->deep_value_uint(m_x, m_y, m_z, c, s);
}

bool localpixels() const { return m_localpixels; }

// Did we encounter an error while we iterated?
bool has_error() const { return m_readerror; }

Expand Down Expand Up @@ -1369,6 +1371,7 @@ class OIIO_API ImageBuf {
OIIO_DASSERT(m_exists && m_valid); // precondition
OIIO_DASSERT(valid(m_x, m_y, m_z)); // should be true by definition
if (m_localpixels) {
OIIO_DASSERT(m_proxydata != nullptr);
m_proxydata += m_pixel_stride;
if (OIIO_UNLIKELY(m_x >= m_img_xend))
pos_xincr_local_past_end();
Expand Down
22 changes: 11 additions & 11 deletions src/libOpenImageIO/imagebuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3110,17 +3110,17 @@ ImageBuf::IteratorBase::init_ib(WrapMode wrap, bool write)
if (!m_localpixels && write) {
const_cast<ImageBuf*>(m_ib)->make_writable(true);
OIIO_DASSERT(m_ib->storage() != IMAGECACHE);
m_tile = nullptr;
m_proxydata = nullptr;
}
m_img_xbegin = spec.x;
m_img_xend = spec.x + spec.width;
m_img_ybegin = spec.y;
m_img_yend = spec.y + spec.height;
m_img_zbegin = spec.z;
m_img_zend = spec.z + spec.depth;
m_nchannels = spec.nchannels;
// m_tilewidth = spec.tile_width;
m_tile = nullptr;
m_proxydata = nullptr;
m_localpixels = !m_deep;
}
m_img_xbegin = spec.x;
m_img_xend = spec.x + spec.width;
m_img_ybegin = spec.y;
m_img_yend = spec.y + spec.height;
m_img_zbegin = spec.z;
m_img_zend = spec.z + spec.depth;
m_nchannels = spec.nchannels;
m_pixel_stride = m_ib->pixel_stride();
m_x = 1 << 31;
m_y = 1 << 31;
Expand Down
30 changes: 30 additions & 0 deletions src/libOpenImageIO/imagebuf_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,35 @@ test_uncaught_error()



void
test_mutable_iterator_with_imagecache()
{
// 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, 0, 0, ImageCache::create());
// Using the cache, it should look tiled
OIIO_CHECK_EQUAL(buf.spec().tile_width, buf.spec().width);

// Make a mutable iterator, even though it's an image file reference.
// Merely establishing the iterator ought to read the file and make the
// buffer writeable.
ImageBuf::Iterator<float> it(buf);
OIIO_CHECK_EQUAL(buf.spec().tile_width, 0); // should look untiled
OIIO_CHECK_ASSERT(buf.localpixels()); // should look local
for (; !it.done(); ++it)
it[0] = 1.0f;

ImageCache::create()->invalidate(ustring(srcfilename));
Filesystem::remove(srcfilename);
}



int
main(int /*argc*/, char* /*argv*/[])
{
Expand All @@ -533,6 +562,7 @@ main(int /*argc*/, char* /*argv*/[])
"periodic");
iterator_wrap_test<ImageBuf::ConstIterator<float>>(ImageBuf::WrapMirror,
"mirror");
test_mutable_iterator_with_imagecache();

ImageBuf_test_appbuffer();
ImageBuf_test_appbuffer_strided();
Expand Down

0 comments on commit d348067

Please sign in to comment.