Skip to content

Commit 0b644cc

Browse files
committed
fix(ImageBuf): fix crash when mutable Iterator used with read-IB (AcademySoftwareFoundation#3997)
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 AcademySoftwareFoundation#3770 --------- Signed-off-by: Larry Gritz <[email protected]>
1 parent 5a1292c commit 0b644cc

File tree

3 files changed

+44
-11
lines changed

3 files changed

+44
-11
lines changed

src/include/OpenImageIO/imagebuf.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,8 @@ class OIIO_API ImageBuf {
13241324
return m_ib->deep_value_uint(m_x, m_y, m_z, c, s);
13251325
}
13261326

1327+
bool localpixels() const { return m_localpixels; }
1328+
13271329
// Did we encounter an error while we iterated?
13281330
bool has_error() const { return m_readerror; }
13291331

@@ -1369,6 +1371,7 @@ class OIIO_API ImageBuf {
13691371
OIIO_DASSERT(m_exists && m_valid); // precondition
13701372
OIIO_DASSERT(valid(m_x, m_y, m_z)); // should be true by definition
13711373
if (m_localpixels) {
1374+
OIIO_DASSERT(m_proxydata != nullptr);
13721375
m_proxydata += m_pixel_stride;
13731376
if (OIIO_UNLIKELY(m_x >= m_img_xend))
13741377
pos_xincr_local_past_end();

src/libOpenImageIO/imagebuf.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3110,17 +3110,17 @@ ImageBuf::IteratorBase::init_ib(WrapMode wrap, bool write)
31103110
if (!m_localpixels && write) {
31113111
const_cast<ImageBuf*>(m_ib)->make_writable(true);
31123112
OIIO_DASSERT(m_ib->storage() != IMAGECACHE);
3113-
m_tile = nullptr;
3114-
m_proxydata = nullptr;
3115-
}
3116-
m_img_xbegin = spec.x;
3117-
m_img_xend = spec.x + spec.width;
3118-
m_img_ybegin = spec.y;
3119-
m_img_yend = spec.y + spec.height;
3120-
m_img_zbegin = spec.z;
3121-
m_img_zend = spec.z + spec.depth;
3122-
m_nchannels = spec.nchannels;
3123-
// m_tilewidth = spec.tile_width;
3113+
m_tile = nullptr;
3114+
m_proxydata = nullptr;
3115+
m_localpixels = !m_deep;
3116+
}
3117+
m_img_xbegin = spec.x;
3118+
m_img_xend = spec.x + spec.width;
3119+
m_img_ybegin = spec.y;
3120+
m_img_yend = spec.y + spec.height;
3121+
m_img_zbegin = spec.z;
3122+
m_img_zend = spec.z + spec.depth;
3123+
m_nchannels = spec.nchannels;
31243124
m_pixel_stride = m_ib->pixel_stride();
31253125
m_x = 1 << 31;
31263126
m_y = 1 << 31;

src/libOpenImageIO/imagebuf_test.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,35 @@ test_uncaught_error()
511511

512512

513513

514+
void
515+
test_mutable_iterator_with_imagecache()
516+
{
517+
// Make 4x4 1-channel float source image, value 0.5, write it.
518+
char srcfilename[] = "tmp_f1.exr";
519+
ImageSpec fsize1(4, 4, 1, TypeFloat);
520+
ImageBuf src(fsize1);
521+
ImageBufAlgo::fill(src, 0.5f);
522+
src.write(srcfilename);
523+
524+
ImageBuf buf(srcfilename, 0, 0, ImageCache::create());
525+
// Using the cache, it should look tiled
526+
OIIO_CHECK_EQUAL(buf.spec().tile_width, buf.spec().width);
527+
528+
// Make a mutable iterator, even though it's an image file reference.
529+
// Merely establishing the iterator ought to read the file and make the
530+
// buffer writeable.
531+
ImageBuf::Iterator<float> it(buf);
532+
OIIO_CHECK_EQUAL(buf.spec().tile_width, 0); // should look untiled
533+
OIIO_CHECK_ASSERT(buf.localpixels()); // should look local
534+
for (; !it.done(); ++it)
535+
it[0] = 1.0f;
536+
537+
ImageCache::create()->invalidate(ustring(srcfilename));
538+
Filesystem::remove(srcfilename);
539+
}
540+
541+
542+
514543
int
515544
main(int /*argc*/, char* /*argv*/[])
516545
{
@@ -533,6 +562,7 @@ main(int /*argc*/, char* /*argv*/[])
533562
"periodic");
534563
iterator_wrap_test<ImageBuf::ConstIterator<float>>(ImageBuf::WrapMirror,
535564
"mirror");
565+
test_mutable_iterator_with_imagecache();
536566

537567
ImageBuf_test_appbuffer();
538568
ImageBuf_test_appbuffer_strided();

0 commit comments

Comments
 (0)