From 75f28245a8c67b578e590b89221d63a1e6d63db8 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Wed, 26 Apr 2017 17:19:26 +0300 Subject: [PATCH] core: fix persistence bug in RAW I/O code - persistence.cpp code expects special sizeof value for passed structures - this assumption is lead to memory corruption problems - fixed/workarounded test to prevent memory corruption on Linux 32-bit systems --- modules/core/src/persistence.cpp | 5 +- modules/core/test/test_io.cpp | 84 ++++++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 17 deletions(-) diff --git a/modules/core/src/persistence.cpp b/modules/core/src/persistence.cpp index 2e834f67fd..d7aa1445e7 100644 --- a/modules/core/src/persistence.cpp +++ b/modules/core/src/persistence.cpp @@ -4907,7 +4907,7 @@ cvReadRawDataSlice( const CvFileStorage* fs, CvSeqReader* reader, { char* data0 = (char*)_data; int fmt_pairs[CV_FS_MAX_FMT_PAIRS*2], k = 0, fmt_pair_count; - int i = 0, offset = 0, count = 0; + int i = 0, count = 0; CV_CHECK_FILE_STORAGE( fs ); @@ -4918,9 +4918,11 @@ cvReadRawDataSlice( const CvFileStorage* fs, CvSeqReader* reader, CV_Error( CV_StsBadSize, "The readed sequence is a scalar, thus len must be 1" ); fmt_pair_count = icvDecodeFormat( dt, fmt_pairs, CV_FS_MAX_FMT_PAIRS ); + size_t step = ::icvCalcStructSize(dt, 0); for(;;) { + int offset = 0; for( k = 0; k < fmt_pair_count; k++ ) { int elem_type = fmt_pairs[k*2+1]; @@ -5038,6 +5040,7 @@ cvReadRawDataSlice( const CvFileStorage* fs, CvSeqReader* reader, offset = (int)(data - data0); } + data0 += step; } end_loop: diff --git a/modules/core/test/test_io.cpp b/modules/core/test/test_io.cpp index 33af4c8b76..1e3ea4935d 100644 --- a/modules/core/test/test_io.cpp +++ b/modules/core/test/test_io.cpp @@ -614,13 +614,41 @@ struct data_t typedef float f; typedef double d; - u u1 ;u u2 ; i i1 ; - i i2 ;i i3 ; - d d1 ; - d d2 ; - i i4 ; + /*0x00*/ u u1 ;u u2 ; i i1 ; + /*0x08*/ i i2 ;i i3 ; + /*0x10*/ d d1 ; + /*0x18*/ d d2 ; + /*0x20*/ i i4 ;i required_alignment_field_for_linux32; + /* + * OpenCV persistence.cpp stuff expects: sizeof(data_t) = alignSize(36, sizeof(largest type = double)) = 40 + * Some compilers on some archs returns sizeof(data_t) = 36 due struct packaging UB + */ - static inline const char * signature() { return "2u3i2di"; } + static inline const char * signature() { + if (sizeof(data_t) != 40) + { + printf("sizeof(data_t)=%d, u1=%p u2=%p i1=%p i2=%p i3=%p d1=%p d2=%p i4=%p\n", (int)sizeof(data_t), + &(((data_t*)0)->u1), + &(((data_t*)0)->u2), + &(((data_t*)0)->i1), + &(((data_t*)0)->i2), + &(((data_t*)0)->i3), + &(((data_t*)0)->d1), + &(((data_t*)0)->d2), + &(((data_t*)0)->i4) + ); + } + CV_Assert(sizeof(data_t) == 40); + CV_Assert((size_t)&(((data_t*)0)->u1) == 0x0); + CV_Assert((size_t)&(((data_t*)0)->u2) == 0x1); + CV_Assert((size_t)&(((data_t*)0)->i1) == 0x4); + CV_Assert((size_t)&(((data_t*)0)->i2) == 0x8); + CV_Assert((size_t)&(((data_t*)0)->i3) == 0xc); + CV_Assert((size_t)&(((data_t*)0)->d1) == 0x10); + CV_Assert((size_t)&(((data_t*)0)->d2) == 0x18); + CV_Assert((size_t)&(((data_t*)0)->i4) == 0x20); + return "2u3i2di"; + } }; TEST(Core_InputOutput, filestorage_base64_basic) @@ -718,16 +746,24 @@ TEST(Core_InputOutput, filestorage_base64_basic) fs.release(); } - for (int i = 0; i < 1000; i++) { - // TODO: Solve this bug in `cvReadRawData` - //EXPECT_EQ(rawdata[i].u1, 1); - //EXPECT_EQ(rawdata[i].u2, 2); - //EXPECT_EQ(rawdata[i].i1, 1); - //EXPECT_EQ(rawdata[i].i2, 2); - //EXPECT_EQ(rawdata[i].i3, 3); - //EXPECT_EQ(rawdata[i].d1, 0.1); - //EXPECT_EQ(rawdata[i].d2, 0.2); - //EXPECT_EQ(rawdata[i].i4, i); + int errors = 0; + for (int i = 0; i < 1000; i++) + { + EXPECT_EQ((int)rawdata[i].u1, 1); + EXPECT_EQ((int)rawdata[i].u2, 2); + EXPECT_EQ((int)rawdata[i].i1, 1); + EXPECT_EQ((int)rawdata[i].i2, 2); + EXPECT_EQ((int)rawdata[i].i3, 3); + EXPECT_EQ(rawdata[i].d1, 0.1); + EXPECT_EQ(rawdata[i].d2, 0.2); + EXPECT_EQ((int)rawdata[i].i4, i); + if (::testing::Test::HasNonfatalFailure()) + { + printf("i = %d\n", i); + errors++; + } + if (errors >= 3) + break; } EXPECT_TRUE(no_type_id); @@ -741,9 +777,25 @@ TEST(Core_InputOutput, filestorage_base64_basic) EXPECT_EQ(_2d_in.cols , _2d_out.cols); EXPECT_EQ(_2d_in.dims , _2d_out.dims); EXPECT_EQ(_2d_in.depth(), _2d_out.depth()); + + errors = 0; for(int i = 0; i < _2d_out.rows; ++i) + { for (int j = 0; j < _2d_out.cols; ++j) + { EXPECT_EQ(_2d_in.at(i, j), _2d_out.at(i, j)); + if (::testing::Test::HasNonfatalFailure()) + { + printf("i = %d, j = %d\n", i, j); + errors++; + } + if (errors >= 3) + { + i = _2d_out.rows; + break; + } + } + } EXPECT_EQ(_nd_in.rows , _nd_out.rows); EXPECT_EQ(_nd_in.cols , _nd_out.cols);