Issue 524 in webp: A lot of valgrind complaints about uninitialized memory

[nazar… via monorail]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 524 by nazar...@gmail.com: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524

What steps will reproduce the problem?
I do not have a publicly available reproduction, but do have a private one. Either way, attaching valgrind report that should be helpful even without reproduction.

What is the expected output? What do you see instead?
Valgrind complains about uninitialized memory, which, ideally, shouldn't happen.


What version of the product are you using? On what operating system?
Tried Ubuntu 20.10 with libwebp 0.6.1 it ships in standard repos, confirmed independently that it still happens on master branch of libwebp repo.


Please provide any additional information below.

Initially reported here: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1715#note_946583

Attachments:
valgrind-report.txt 156 KB

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[jz… via monorail]

Comment #1 on issue 524 by jz...@google.com: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524#c1

Thanks for the report. We have some valgrind coverage [1], but that's only on decode at the moment. There are msan/asan builds for cwebp & dwebp as well, but it's possible they're not reporting it, we're missing this import path or the data from the application could be the source.

I'll see if I can reproduce this using libwebp tools on their own. If not do you have the gstreamer repro? Anything in particular that needs to be done to use a local build of libwebp?

[1] https://build.webmproject.org/jenkins/job/libwebp__test

[nazar… via monorail]

Comment #2 on issue 524 by nazar...@gmail.com: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524#c2

You can reproduce it if you build gstreamer with above MR or use `restreamio/gstreamer:latest-dev` Ubuntu-based container image that has GStreamer build with that PR already installed (if you want custom libwebp build there you'll have to replace the old one installed from Ubuntu repos with your own build).

So in general this is a standalone reproduction:
1) docker run --rm -it restreamio/gstreamer:latest-dev
2) Below commands should be run inside of container
3) apt-get install --no-install-recommends valgrind ffmpeg
4) ffmpeg -r 30 -f lavfi -i testsrc=decimals=3:size=1280x720 -f lavfi -i anullsrc -t 00:00:15 -vcodec libx264 -g 60 -acodec aac -map 0 -map 1 -f flv test.flv
5) valgrind gst-launch-1.0 filesrc location=test.flv ! queue ! flvdemux name=d ! h264parse ! decodebin ! queue ! videoconvert ! webpenc animated=true ! filesink sync=false async=false location=foo.webp

I'm sure there is a shorter way to reproduce this, but that definitely works, so I hope that helps. Container already has git, gcc and some other tools installed, so you should be able to clone, build and install custom version of libwebp there.

There are simpler GStreamer pipelines in which those do not get triggered, like this one:
valgrind gst-launch-1.0 videotestsrc pattern=ball num-buffers=10 ! video/x-raw, width=400, height=400, framerate=25/1 ! webpenc animated=true ! filesink location=foo.webp

I hope provided information will help to figure out why that is the case exactly.

[jz… via monorail]

Issue 524: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524

This issue is now blocking issue 521.
See https://bugs.chromium.org/p/webp/issues/detail?id=521

[pasca… via monorail]

Comment #4 on issue 524 by pasca...@gmail.com: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524#c4

Hi, thanks for the report.

In order to clear an obvious source of error, could you try filling the input frame with a fixed value (any value. 0 is good.)
*before* calling the libwebp API, please? And then check if the errors are still present?

If there is any uninitialized values in the source, it can get propagated unnoticed by valgrind until it raises an error at the other end of the library in surprising unrelated places.

(of course, if the valgrind disappear after this experiment, it could also just mean a different code path was selected, but...)

[nazar… via monorail]

Comment #5 on issue 524 by nazar...@gmail.com: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524#c5

I'm not a GStreamer developer and my C knowledge is limited. You can look at https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1715/diffs though and then modify the code by doing this:
1) apt-get install nano (or any editor of choice)
2) cd /gst-plugins-bad
3) Do modifications you want
4) ninja -C build install
5) Try to run example mentioned earlier again

[jz… via monorail]

Comment #6 on issue 524 by jz...@google.com: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524#c6

This only reproduces for me inside the container, a local build on debian testing is fine. I didn't try a separate ubuntu instance, but the issue appears to be with the source like suggested in comment #4. In the container only the first 160 pixels are initialized. Adding printf before WebPAnimEncoderAdd() [1] will move the valgrind report to that line [2].

[1]
fprintf(stderr, "frame size %d x %d (%d)\n", enc->webp_picture.width, enc->webp_picture.height, GST_VIDEO_FRAME_COMP_STRIDE (&vframe, 0));
fprintf(stderr, "rgb[480]: %d\n", GST_VIDEO_FRAME_COMP_DATA (&vframe, 0)[480]);
[2]
frame size 1280 x 720 (3840)
==198343== Thread 3 queue1:src:
==198343== Conditional jump or move depends on uninitialised value(s)
==198343== at 0x4BD46F8: printf_positional (vfprintf-internal.c:1996)
==198343== by 0x4BD7195: __vfprintf_internal (vfprintf-internal.c:1683)
==198343== by 0x4BD805F: buffered_vfprintf (vfprintf-internal.c:2295)
==198343== by 0x4C88E02: __fprintf_chk (fprintf_chk.c:33)
==198343== by 0x6D504C0: fprintf (stdio2.h:103)
==198343== by 0x6D504C0: gst_webp_enc_handle_frame (gstwebpenc.c:303)
==198343== by 0x6A194B9: gst_video_encoder_chain (gstvideoencoder.c:1696)
==198343== by 0x48D54CC: gst_pad_chain_data_unchecked (gstpad.c:4441)
==198343== by 0x48D7748: gst_pad_push_data (gstpad.c:4697)
==198343== by 0x48DED81: gst_pad_push (gstpad.c:4816)
==198343== by 0x5527866: gst_base_transform_chain (gstbasetransform.c:2377)
==198343== by 0x48D54CC: gst_pad_chain_data_unchecked (gstpad.c:4441)
==198343== by 0x48D7748: gst_pad_push_data (gstpad.c:4697)

[jz… via monorail]

Updates:
Status: Invalid

Comment #7 on issue 524 by jz...@google.com: A lot of valgrind complaints about uninitialized memory
https://bugs.chromium.org/p/webp/issues/detail?id=524#c7

(No comment was entered for this change.)