Bug 398153 - Apparent false positive of uninitialised values in libjpeg-turbo
Summary: Apparent false positive of uninitialised values in libjpeg-turbo
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: 3.13.0
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-02 11:57 UTC by Nick C.
Modified: 2020-02-03 03:46 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
square8x8.jpg - example input (659 bytes, image/jpeg)
2020-02-03 03:41 UTC, nh2
Details
Another image reproducing the problem (2.09 KB, image/jpeg)
2020-02-03 03:46 UTC, nh2
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nick C. 2018-09-02 11:57:43 UTC
Hi,
I have some code that is decoding a JPEG using libjpeg-turbo.
The code then prints out the values of a few of the decoded pixels, but Valgrind reports that the pixels are uninitialised:

==11097== Syscall param write(buf) points to uninitialised byte(s)
==11097==    at 0x5ED0187: write (write.c:27)
==11097==    by 0x5E4B1BC: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1203)
==11097==    by 0x5E4CF50: new_do_write (fileops.c:457)
==11097==    by 0x5E4CF50: _IO_do_write@@GLIBC_2.2.5 (fileops.c:433)
==11097==    by 0x5E4D402: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:798)
==11097==    by 0x5E4834A: putc (putc.c:31)
==11097==    by 0x5593239: std::ostream::put(char) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==11097==    by 0x5593462: std::basic_ostream<char, std::char_traits<char> >& std::endl<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==11097==    by 0x863272: conPrint(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (ConPrint.cpp:27)
==11097==    by 0x3D3383: JPEGDecoder::test(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (jpegdecoder.cpp:435)
==11097==    by 0x61803D: TestSuite::test(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, bool) (TestSuite.cpp:287)
==11097==    by 0x53D975: NonInteractive::run(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (NonInteractive.cpp:211)
==11097==    by 0x6B10A0: main (indigo_console.cpp:48)
==11097==  Address 0x6436fc5 is 5 bytes inside a block of size 1,024 alloc'd
==11097==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11097==    by 0x5E3E18B: _IO_file_doallocate (filedoalloc.c:101)
==11097==    by 0x5E4E378: _IO_doallocbuf (genops.c:365)
==11097==    by 0x5E4D497: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:759)
==11097==    by 0x5E4B9EC: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1266)
==11097==    by 0x5E3F976: fwrite (iofwrite.c:39)
==11097==    by 0x5593773: std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==11097==    by 0x86325D: conPrint(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (ConPrint.cpp:27)
==11097==    by 0x61800C: TestSuite::test(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, bool) (TestSuite.cpp:275)
==11097==    by 0x53D975: NonInteractive::run(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (NonInteractive.cpp:211)
==11097==    by 0x6B10A0: main (indigo_console.cpp:48)
==11097==  Uninitialised value was created by a heap allocation
==11097==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11097==    by 0x16DCDA8: alloc_sarray (in /home/nick/indigo/output/test_builds/indigo_console)
==11097==    by 0x170022B: jinit_d_main_controller (in /home/nick/indigo/output/test_builds/indigo_console)
==11097==    by 0x16D2729: jinit_master_decompress (in /home/nick/indigo/output/test_builds/indigo_console)
==11097==    by 0x16CCA7C: jpeg_start_decompress (in /home/nick/indigo/output/test_builds/indigo_console)
==11097==    by 0x3D1E13: JPEGDecoder::decode(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (jpegdecoder.cpp:148)
==11097==    by 0x3D311C: JPEGDecoder::test(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (jpegdecoder.cpp:418)
==11097==    by 0x61803D: TestSuite::test(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, bool) (TestSuite.cpp:287)
==11097==    by 0x53D975: NonInteractive::run(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (NonInteractive.cpp:211)
==11097==    by 0x6B10A0: main (indigo_console.cpp:48)


The test driver code looks like:


Reference<Map2D> im = JPEGDecoder::decode(indigo_base_dir, TestUtils::getIndigoTestReposDir() + "/testscenes/ColorChecker_sRGB_from_Ref.jpg");
			testAssert(im->getMapWidth() == 1080);
			testAssert(im->getMapHeight() == 768);
			testAssert(im->getBytesPerPixel() == 3);
			testAssert(dynamic_cast<const ImageMapUInt8*>(im.getPointer()) != NULL);
			
			// Try saving it.
			// x=1 fails
			// y=1 is fine
			// y=2 fails
			size_t sum = 0;
			ImageMapUInt8* m = dynamic_cast<ImageMapUInt8*>(im.getPointer());
			for(int x=0; x<1; ++x)
				for(int y=0; y<2; ++y)
					sum += m->getPixel(x, y)[0];
			printVar(sum);


I suspect this is a false positive, since the actual pixel values themselves seem fine (e.g. they look like they were initialised.).

This happens on all JPEG files I have tested with so far.
If needed, a specific one that triggers the bug can be downloaded here: https://www.dropbox.com/s/ik3a32allc1mwqr/square8x8.jpg?dl=0

Steps to repro would be something like:
* Install libjpeg-turbo (I installed using the source https://github.com/libjpeg-turbo/libjpeg-turbo/archive/master.zip)
* Use some code to call the libjpeg-turbo API to load a JPEG file.  (I can provide some of this code if needed)
* Do something with the resulting pixel values.

Please see the bug report here: https://github.com/libjpeg-turbo/libjpeg-turbo/issues/277 for more details

I've had a look, but haven't managed to find any precise likely suspects for false positives.
However as noted in the linked bug report on Github, setting the JSIMD_FORCENONE env var to 1 does suppress the error.
Therefore the error seems to be related to SSE usage in libjpeg-turbo.

I think the likely code causing the issue is in the jconst_idct_islow_sse2 function, which is defined in libjpeg-turbo-master\simd\jidctint-sse2-64.asm.

Thanks!
Comment 1 Nick C. 2018-09-04 08:36:29 UTC
Actually scratch that.

jsimd_idct_islow_sse2() does not result in any valgrind errors.

jsimd_idct_islow_avx2() results in Valgrind errors.
Comment 2 nh2 2020-02-03 00:59:17 UTC
Related: https://bugs.kde.org/show_bug.cgi?id=394227
Comment 3 nh2 2020-02-03 03:41:33 UTC
Created attachment 125638 [details]
square8x8.jpg - example input

Copying the example input from the linked dropbox into an issue tracker for longevity.
Comment 4 nh2 2020-02-03 03:46:46 UTC
Created attachment 125639 [details]
Another image reproducing the problem

I've uploaded another (all-black) image reproducing the problem for me.