Possible false positive report from unittests.exe at libpng.dll!wk_png_write_find_filter

[derekbruening]

From [email protected] on September 17, 2013 10:35:48

http://build.chromium.org/p/chromium.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%282%29/builds/429/steps/memory%20test%3A%20unit_2/logs/stdio [----------] 1 test from ScreenshotTakerTest
[ RUN ] ScreenshotTakerTest.TakeScreenshot
Dr.M
Dr.M Error #1: UNINITIALIZED READ: reading 0x0018ed38-0x0018ed39 1 byte(s) within 0x0018ed38-0x0018ed3c
Dr.M # 0 libpng.dll!wk_png_write_find_filter [third_party\libpng\pngwutil.c:2213]
Dr.M # 1 libpng.dll!wk_png_write_row [third_party\libpng\pngwrite.c:964]
Dr.M # 2 ui.dll!gfx::anonymous namespace'::DoLibpngWrite [ui\gfx\codec\png_codec.cc:648] \~~Dr.M~~ # 3 ui.dll!gfx::anonymous namespace'::EncodeWithCompressionLevel [ui\gfx\codec\png_codec.cc:741]
Dr.M # 4 ui.dll!gfx::PNGCodec::Encode [ui\gfx\codec\png_codec.cc:762]
Dr.M # 5 snapshot.dll!ui::GrabWindowSnapshot [ui\snapshot\snapshot_aura.cc:86]
Dr.M # 6 chrome::GrabWindowSnapshotForUser [chrome\browser\ui\window_snapshot\window_snapshot.cc:22]
Dr.M # 7 anonymous namespace'::GrabWindowSnapshot [chrome\browser\ui\ash\screenshot_taker.cc:230] \~~Dr.M~~ # 8 ScreenshotTaker::HandleTakePartialScreenshot [chrome\browser\ui\ash\screenshot_taker.cc:402] \~~Dr.M~~ # 9 ash::test::ScreenshotTakerTest_TakeScreenshot_Test::TestBody [chrome\browser\ui\ash\screenshot_taker_unittest.cc:123] \~~Dr.M~~#10testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void> [testing\gtest\src\gtest.cc:2051] \~~Dr.M~~ Note: @0:26:30.172 in thread 4088 \~~Dr.M~~ Note: instruction: cmp 0xffffffdc(&#37;ebp) $0x00000080 \~~Dr.M~~ \~~Dr.M~~ Error#2: UNINITIALIZED READ: reading 0x0018ed0c-0x0018ed0d 1 byte(s) within 0x0018ed0c-0x0018ed10 \~~Dr.M~~ # 0 libpng.dll!wk_png_write_find_filter [third_party\libpng\pngwutil.c:2320] \~~Dr.M~~ # 1 libpng.dll!wk_png_write_row [third_party\libpng\pngwrite.c:964] \~~Dr.M~~ # 2 ui.dll!gfx::anonymous namespace'::DoLibpngWrite [ui\gfx\codec\png_codec.cc:648]
Dr.M # 3 ui.dll!gfx::anonymous namespace'::EncodeWithCompressionLevel [ui\gfx\codec\png_codec.cc:741] \~~Dr.M~~ # 4 ui.dll!gfx::PNGCodec::Encode [ui\gfx\codec\png_codec.cc:762] \~~Dr.M~~ # 5 snapshot.dll!ui::GrabWindowSnapshot [ui\snapshot\snapshot_aura.cc:86] \~~Dr.M~~ # 6 chrome::GrabWindowSnapshotForUser [chrome\browser\ui\window_snapshot\window_snapshot.cc:22] \~~Dr.M~~ # 7anonymous namespace'::GrabWindowSnapshot [chrome\browser\ui\ash\screenshot_taker.cc:230]
Dr.M # 8 ScreenshotTaker::HandleTakePartialScreenshot [chrome\browser\ui\ash\screenshot_taker.cc:402]
Dr.M # 9 ash::test::ScreenshotTakerTest_TakeScreenshot_Test::TestBody [chrome\browser\ui\ash\screenshot_taker_unittest.cc:123]
Dr.M #10 testing::internal::HandleExceptionsInMethodIfSupportedtesting::Test,void [testing\gtest\src\gtest.cc:2051]

I can reproduce it on my machine, investigating.

Original issue: http://code.google.com/p/drmemory/issues/detail?id=1335

[derekbruening]

From [email protected] on September 17, 2013 19:38:12

In Release-build chrome:

Error #1: UNINITIALIZED READ: reading register ecx

0 libpng.dll!wk_png_write_find_filter [e:\derek\chromium\src\third_party\libpng\pngwutil.c:2213]

1 libpng.dll!wk_png_write_row [e:\derek\chromium\src\third_party\libpng\pngwrite.c:964]

2 ui.dll!gfx::`anonymous namespace'::DoLibpngWrite [e:\derek\chromium\src\ui\gfx\codec\png_codec.cc:648]

# 3 ui.dll!gfx::`anonymous namespace'::EncodeWithCompressionLevel [e:\derek\chromium\src\ui\gfx\codec\png_codec.cc:741]

4 ui.dll!gfx::PNGCodec::Encode [e:\derek\chromium\src\ui\gfx\codec\png_codec.cc:762]

5 snapshot.dll!ui::GrabWindowSnapshot [e:\derek\chromium\src\ui\snapshot\snapshot_aura.cc:86]

6 `anonymous namespace'::GrabWindowSnapshot [e:\derek\chromium\src\chrome\browser\ui\ash\screenshot_taker.cc:230]

7 ScreenshotTaker::HandleTakePartialScreenshot [e:\derek\chromium\src\chrome\browser\ui\ash\screenshot_taker.cc:402]

8 ash::test::ScreenshotTakerTest_TakeScreenshot_Test::TestBody [e:\derek\chromium\src\chrome\browser\ui\ash\screenshot_taker_unittest.cc:123]

9 testing::internal::HandleExceptionsInMethodIfSupportedtesting::TestCase,void [e:\derek\chromium\src\testing\gtest\src\gtest.cc:2051]

#10 testing::Test::Run [e:\derek\chromium\src\testing\gtest\src\gtest.cc:2068]
#11 testing::TestCase::Run [e:\derek\chromium\src\testing\gtest\src\gtest.cc:2351]
Note: @0:46:21.273 in thread 8616
Note: instruction: cmp %ecx $0x00000080

png_write_find_filter(png_structp png_ptr, png_row_infop row_info)
...
best_row = png_ptr->row_buf;
row_buf = best_row;
...
png_bytep rp;
int v;
...
for (i = 0, rp = row_buf + 1; i < row_bytes; i++, rp++)
{
v = *rp;
sum += (v < 128) ? v : 256 - v;
}

called like this:
png_write_find_filter(png_ptr, &(png_ptr->row_info));

03edef5c 6bbc913e libpng!wk_png_write_row+0x1ee [e:\derek\chromium\src\third_party\libpng\pngwrite.c @ 966]
03edef60 431d6368
03edef64 0000012c <-- yes, this param slot is clobbered

03edef74 66429ad3 ui!gfx::`anonymous namespace'::DoLibpngWrite+0x173 [e:\derek\chromium\src\ui\gfx\codec\png_codec.cc @ 648]
03edef78 431d6368
03edef7c 43230ed8

0:008> U 0x6bbcb7ad
libpng!wk_png_write_find_filter+0x6d [e:\derek\chromium\src\third_party\libpng\pngwutil.c @ 2210]:
6bbcb7ad 8d4900 lea ecx,[ecx]
6bbcb7b0 0fb64c3a01 movzx ecx,byte ptr [edx+edi+0x1]
6bbcb7b5 894dd4 mov [ebp-0x2c],ecx
6bbcb7b8 81f980000000 cmp ecx,0x80
0:000> dt libpng!png_structp 03edef60
0x431d6368
+0x000 jmpbuf : [16] 65925064
+0x040 error_fn : 0x664289d0 ui!gfx::anonymous namespace'::LogLibPNGDecodeError+0 +0x044 warning_fn : 0x664289f0 ui!gfx::anonymous namespace'::LogLibPNGEncodeWarning+0
+0x048 error_ptr : (null)
+0x04c write_data_fn : 0x66428b70 ui!gfx::`anonymous namespace'::EncoderWriteCallback+0
+0x050 read_data_fn : (null)
+0x054 io_ptr : 0x03edf178
+0x058 read_user_transform_fn : (null)
+0x05c user_transform_ptr : (null)
+0x060 user_transform_depth : 0 ''
+0x061 user_transform_channels : 0 ''
+0x064 mode : 0x401
+0x068 flags : 2
+0x06c transformations : 0
+0x070 zstream : z_stream_s
+0x0ac zbuf : 0x431901b8 ""
+0x0b0 zbuf_size : 0x2000
+0x0b4 zlib_level : -1
+0x0b8 zlib_method : 8
+0x0bc zlib_window_bits : 15
+0x0c0 zlib_mem_level : 8
+0x0c4 zlib_strategy : 1
+0x0c8 width : 0x64
+0x0cc height : 0x64
+0x0d0 num_rows : 0x64
+0x0d4 usr_width : 0x64
+0x0d8 rowbytes : 0x12c
+0x0dc iwidth : 0
+0x0e0 row_number : 0
+0x0e4 prev_row : 0x432312c8 ""
+0x0e8 row_buf : 0x43231028 "" Ptr32 to UChar

...

0:000> dd 0x43231028-20
43231008 00000000 00000000 00000000 00000130
43231018 52440004 00000003 00000000 00000000
43231028 00000000 00000000 00000000 00000000
43231038 00000000 00000000 00000000 00000000
0:000> dt drmemorylib!chunk_header_t 0x43231010
+0x000 user_data : (null)
+0x004 alloc_size : 0x130
+0x008 flags : 4 == MALLOC_ALLOCATOR_MALLOC
+0x00a magic : 0x5244
+0x00c u :

0:000> dt mc
+0x008 edi : 0
+0x01c edx : 0x43231028
+0x020 ecx : 0
slow_path 0x6bbcb7b0: movzx 0x01(%edx,%edi,1) -> %ecx
memref: read @0x6bbcb7b0 0x43231029 0x1 bytes (pre-dword 0xfc 0xff)

0x4323102x was last written here:
slow_path 0x6bbca95e: mov $0x00 -> (%eax) | 0x3
memref: write @0x6bbca95e 0x43231028 0x1 bytes (pre-dword 0xff 0xff)

libpng!wk_png_write_start_row+0x45 [e:\derek\chromium\src\third_party\libpng\pngwutil.c @ 1786]:
6bbca955 8986e8000000 mov [esi+0xe8],eax
6bbca95b 83c408 add esp,0x8
6bbca95e c60000 mov byte ptr [eax],0x0
6bbca961 f6862101000010 test byte ptr [esi+0x121],0x10

png_write_start_row(png_structp png_ptr)
...
/* Set up row buffer */
png_ptr->row_buf = (png_bytep)png_malloc(png_ptr,
(png_uint_32)buf_size);
png_ptr->row_buf[0] = PNG_FILTER_VALUE_NONE;

replace_malloc 301
malloc 0x43231028-0x43231155

0 libpng.dll!wk_png_malloc+0x19 [e:\derek\chromium\src\third_party\libpng\pngmem.c:511+0x7](0x6bbc204a <libpng.dll+0x204a) modid:0

1 libpng.dll!wk_png_write_start_row+0x44 [e:\derek\chromium\src\third_party\libpng\pngwutil.c:1786+0x6](0x6bbca955 <libpng.dll+0xa955) modid:0

2 libpng.dll!wk_png_write_row+0x3d [e:\derek\chromium\src\third_party\libpng\pngwrite.c:841+0x5](0x6bbc8f8e <libpng.dll+0x8f8e) modid:0

3 ui.dll!gfx::`anonymous namespace'::DoLibpngWrite+0x172 [e:\derek\chromium\src\ui\gfx\codec\png_codec.cc:648+0x7] (0x66429ad3 <ui.dll+0x49ad3>) modid:0

# 4 ui.dll!gfx::`anonymous namespace'::EncodeWithCompressionLevel+0x1fb [e:\derek\chromium\src\ui\gfx\codec\png_codec.cc:741+0x30](0x66429d1c <ui.dll+0x49d1c) modid:0

png_malloc does not zero (there is a png_calloc for zeroing).

the whole thing is uninitialized except that first byte:
0:000> dyb @@(((char *)drmemorylib!umbra_map->shadow_table[0x4323])) + (0x43231028/4)) L10
76543210 76543210 76543210 76543210
-------- -------- -------- --------
43a3462a 11111100 11111111 11111111 11111111 fc ff ff ff
43a3462e 11111111 11111111 11111111 11111111 ff ff ff ff
43a34632 11111111 11111111 11111111 11111111 ff ff ff ff
43a34636 11111111 11111111 11111111 11111111 ff ff ff ff

caller DoLibpngWrite():
// Needs conversion using a separate buffer.
row_buffer = new unsigned char[width * output_color_components];
for (int y = 0; y < height; y ++) {
converter(&input[y * row_byte_width], width, row_buffer, NULL);
png_write_row(png_ptr, row_buffer);
}
delete[] row_buffer;

Looks like a real uninit to me: it mallocs 301 bytes for png_ptr->row_buf,
sets the very first byte to 0, and proceeds to read from the second byte
onward.

This Chrome suppression matches:
{
bug_30704f
Memcheck:Uninitialized
...
fun:wk_png_write_find_filter
fun:wk_png_write_row
}

The bug ( http://crbug.com/30704 ) is mostly about zlib with that #f tacked on.

Also xref http://crbug.com/174174 "Several uninitialized reads in png routines after r180271 "
and http://crbug.com/159725 "Uninit access in ResourceBundleImageTest"

Owner: [email protected]

[derekbruening]

From [email protected] on September 17, 2013 20:12:08

I'm not convinced this isn't unimportant: we'd need to talk to someone
knowledgeable about what libpng is doing here. Thus I'd prefer not to add
a DrMem-wide default suppression. Let's add a Chromium-specific
suppression to match the Valgrind one => r223777 .

Status: WontFix
Labels: -Bug-FalsePositive ThirdParty-Bug