| Message ID | 20170409105110.11850-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | aed84ee4d1b0c9e315a84b1ee0918fa49ee9cc09 |
| Headers | show |
On Sun, 9 Apr 2017 at 14:57 James Almer <jamrial@gmail.com> wrote: > Fixes valgrind warnings about "Use of uninitialised value of size 8" > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > This probably just silences a bunch of false possitives. > > libavcodec/svq1dec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > LGTM
On 4/9/2017 11:51 AM, Kieran Kunhya wrote: > On Sun, 9 Apr 2017 at 14:57 James Almer <jamrial@gmail.com> wrote: > >> Fixes valgrind warnings about "Use of uninitialised value of size 8" >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> This probably just silences a bunch of false possitives. >> >> libavcodec/svq1dec.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> > > LGTM Pushed, thanks.
On Sun, Apr 09, 2017 at 07:51:10AM -0300, James Almer wrote: > Fixes valgrind warnings about "Use of uninitialised value of size 8" how can this be reproduced ? is this a regression ? > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > This probably just silences a bunch of false possitives. if thats the case, then the fix is wrong valgrind bugs should be fixed in valgrind or the entries should be added to the valgrind suppression file [...]
On 4/9/2017 10:46 PM, Michael Niedermayer wrote: > On Sun, Apr 09, 2017 at 07:51:10AM -0300, James Almer wrote: >> Fixes valgrind warnings about "Use of uninitialised value of size 8" > > how can this be reproduced ? fate-svq1, fate-svq1-headerswap and fate-vsynth{1,2,3,_lena}-svq1 when configured with --toolchain=valgrind-memcheck A similar failure can be seen in all ffv1 vsynth tests as well, but i couldn't find what caused them or where. > is this a regression ? Fate didn't always complain about this, so it's either something introduced by a change in our tree, or a valgrind bug introduced in a relatively recent version. The reports in http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef are kinda broken and report a nonsense commit as the "last known good ref", so i can't say when it started failing. > > >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- > >> This probably just silences a bunch of false possitives. > > if thats the case, then the fix is wrong > valgrind bugs should be fixed in valgrind or the entries should be > added to the valgrind suppression file Assuming it is after all a false positive, zero initializing stack is harmless and gets rid of the noise. But i agree it's not ideal. > > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Sun, Apr 09, 2017 at 11:10:16PM -0300, James Almer wrote: [...] > > is this a regression ? > > Fate didn't always complain about this, so it's either something > introduced by a change in our tree, or a valgrind bug introduced in a > relatively recent version. > > The reports in > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef > are kinda broken and report a nonsense commit as the "last known good > ref", so i can't say when it started failing. That was because I couldn't upgrade for a long time and bumped both GCC and Valgrind (and actually the whole system) at once. Sorry, I couldn't do it gradually.
On Mon, Apr 10, 2017 at 08:02:34AM +0200, Clément Bœsch wrote: > On Sun, Apr 09, 2017 at 11:10:16PM -0300, James Almer wrote: > [...] > > > is this a regression ? > > > > Fate didn't always complain about this, so it's either something > > introduced by a change in our tree, or a valgrind bug introduced in a > > relatively recent version. > > > > > The reports in > > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef > > are kinda broken and report a nonsense commit as the "last known good > > ref", so i can't say when it started failing. > > That was because I couldn't upgrade for a long time and bumped both GCC > and Valgrind (and actually the whole system) at once. Sorry, I couldn't do > it gradually. does your fate client have optimizations enabled ? i dont see them being disabled, and iam not sure valgrind + full compiler optimizations are a good idea if they are enabled, please try to disable them to check if that causes the false positives my local valgrind shows nothing for svq1 [...]
On Mon, Apr 10, 2017 at 01:37:54PM +0200, Michael Niedermayer wrote: > On Mon, Apr 10, 2017 at 08:02:34AM +0200, Clément Bœsch wrote: > > On Sun, Apr 09, 2017 at 11:10:16PM -0300, James Almer wrote: > > [...] > > > > is this a regression ? > > > > > > Fate didn't always complain about this, so it's either something > > > introduced by a change in our tree, or a valgrind bug introduced in a > > > relatively recent version. > > > > > > > > The reports in > > > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef > > > are kinda broken and report a nonsense commit as the "last known good > > > ref", so i can't say when it started failing. > > > > That was because I couldn't upgrade for a long time and bumped both GCC > > and Valgrind (and actually the whole system) at once. Sorry, I couldn't do > > it gradually. > > does your fate client have optimizations enabled ? > i dont see them being disabled, and iam not sure valgrind + full > compiler optimizations are a good idea > > if they are enabled, please try to disable them to check if that > causes the false positives > Yeah actually, only --extra-cflags=-fno-tree-vectorize is needed to make fate-vsynth3-svq1 pass. I thought it was supposed to be disabled? > my local valgrind shows nothing for svq1 With aed84ee4d reverted? --toolchain=valgrind-memcheck --disable-stripping?
On Mon, Apr 10, 2017 at 02:04:30PM +0200, Clément Bœsch wrote: > On Mon, Apr 10, 2017 at 01:37:54PM +0200, Michael Niedermayer wrote: > > On Mon, Apr 10, 2017 at 08:02:34AM +0200, Clément Bœsch wrote: > > > On Sun, Apr 09, 2017 at 11:10:16PM -0300, James Almer wrote: > > > [...] > > > > > is this a regression ? > > > > > > > > Fate didn't always complain about this, so it's either something > > > > introduced by a change in our tree, or a valgrind bug introduced in a > > > > relatively recent version. > > > > > > > > > > > The reports in > > > > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef > > > > are kinda broken and report a nonsense commit as the "last known good > > > > ref", so i can't say when it started failing. > > > > > > That was because I couldn't upgrade for a long time and bumped both GCC > > > and Valgrind (and actually the whole system) at once. Sorry, I couldn't do > > > it gradually. > > > > does your fate client have optimizations enabled ? > > i dont see them being disabled, and iam not sure valgrind + full > > compiler optimizations are a good idea > > > > if they are enabled, please try to disable them to check if that > > causes the false positives > > > > Yeah actually, only --extra-cflags=-fno-tree-vectorize is needed to make > fate-vsynth3-svq1 pass. I thought it was supposed to be disabled? > > > my local valgrind shows nothing for svq1 > > With aed84ee4d reverted? --toolchain=valgrind-memcheck > --disable-stripping? reverted, yes no toolchain, no stripping just tried it under valgrind but thats with a relativly old gcc [...]
On Mon, Apr 10, 2017 at 02:19:54PM +0200, Michael Niedermayer wrote: > On Mon, Apr 10, 2017 at 02:04:30PM +0200, Clément Bœsch wrote: > > On Mon, Apr 10, 2017 at 01:37:54PM +0200, Michael Niedermayer wrote: > > > On Mon, Apr 10, 2017 at 08:02:34AM +0200, Clément Bœsch wrote: > > > > On Sun, Apr 09, 2017 at 11:10:16PM -0300, James Almer wrote: > > > > [...] > > > > > > is this a regression ? > > > > > > > > > > Fate didn't always complain about this, so it's either something > > > > > introduced by a change in our tree, or a valgrind bug introduced in a > > > > > relatively recent version. > > > > > > > > > > > > > > The reports in > > > > > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef > > > > > are kinda broken and report a nonsense commit as the "last known good > > > > > ref", so i can't say when it started failing. > > > > > > > > That was because I couldn't upgrade for a long time and bumped both GCC > > > > and Valgrind (and actually the whole system) at once. Sorry, I couldn't do > > > > it gradually. > > > > > > does your fate client have optimizations enabled ? > > > i dont see them being disabled, and iam not sure valgrind + full > > > compiler optimizations are a good idea > > > > > > if they are enabled, please try to disable them to check if that > > > causes the false positives > > > > > > > Yeah actually, only --extra-cflags=-fno-tree-vectorize is needed to make > > fate-vsynth3-svq1 pass. I thought it was supposed to be disabled? > > > > > my local valgrind shows nothing for svq1 > > > > > With aed84ee4d reverted? --toolchain=valgrind-memcheck > > --disable-stripping? > > reverted, yes > no toolchain, no stripping just tried it under valgrind > > but thats with a relativly old gcc > Actually i was wrong: the GCC option that makes valgrind go crazy is unswitch-loops, not tree-vectorize. Do you have it in your GCC?
On Mon, Apr 10, 2017 at 02:42:42PM +0200, Clément Bœsch wrote: > On Mon, Apr 10, 2017 at 02:19:54PM +0200, Michael Niedermayer wrote: > > On Mon, Apr 10, 2017 at 02:04:30PM +0200, Clément Bœsch wrote: > > > On Mon, Apr 10, 2017 at 01:37:54PM +0200, Michael Niedermayer wrote: > > > > On Mon, Apr 10, 2017 at 08:02:34AM +0200, Clément Bœsch wrote: > > > > > On Sun, Apr 09, 2017 at 11:10:16PM -0300, James Almer wrote: > > > > > [...] > > > > > > > is this a regression ? > > > > > > > > > > > > Fate didn't always complain about this, so it's either something > > > > > > introduced by a change in our tree, or a valgrind bug introduced in a > > > > > > relatively recent version. > > > > > > > > > > > > > > > > > The reports in > > > > > > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef > > > > > > are kinda broken and report a nonsense commit as the "last known good > > > > > > ref", so i can't say when it started failing. > > > > > > > > > > That was because I couldn't upgrade for a long time and bumped both GCC > > > > > and Valgrind (and actually the whole system) at once. Sorry, I couldn't do > > > > > it gradually. > > > > > > > > does your fate client have optimizations enabled ? > > > > i dont see them being disabled, and iam not sure valgrind + full > > > > compiler optimizations are a good idea > > > > > > > > if they are enabled, please try to disable them to check if that > > > > causes the false positives > > > > > > > > > > Yeah actually, only --extra-cflags=-fno-tree-vectorize is needed to make > > > fate-vsynth3-svq1 pass. I thought it was supposed to be disabled? > > > > > > > my local valgrind shows nothing for svq1 > > > > > > > > With aed84ee4d reverted? --toolchain=valgrind-memcheck > > > --disable-stripping? > > > > reverted, yes > > no toolchain, no stripping just tried it under valgrind > > > > but thats with a relativly old gcc > > > > Actually i was wrong: the GCC option that makes valgrind go crazy is > unswitch-loops, not tree-vectorize. Do you have it in your GCC? its in man gcc on the box i tried on. maybe gcc has improved between version 4 an 6 but it doesnt really matter if it fixes all the false positives, please add it and then lets revert the workaround (s?) thx [...]
On Mon, Apr 10, 2017 at 02:48:51PM +0200, Michael Niedermayer wrote: > On Mon, Apr 10, 2017 at 02:42:42PM +0200, Clément Bœsch wrote: > > On Mon, Apr 10, 2017 at 02:19:54PM +0200, Michael Niedermayer wrote: > > > On Mon, Apr 10, 2017 at 02:04:30PM +0200, Clément Bœsch wrote: > > > > On Mon, Apr 10, 2017 at 01:37:54PM +0200, Michael Niedermayer wrote: > > > > > On Mon, Apr 10, 2017 at 08:02:34AM +0200, Clément Bœsch wrote: > > > > > > On Sun, Apr 09, 2017 at 11:10:16PM -0300, James Almer wrote: > > > > > > [...] > > > > > > > > is this a regression ? > > > > > > > > > > > > > > Fate didn't always complain about this, so it's either something > > > > > > > introduced by a change in our tree, or a valgrind bug introduced in a > > > > > > > relatively recent version. > > > > > > > > > > > > > > > > > > > > The reports in > > > > > > > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrindundef > > > > > > > are kinda broken and report a nonsense commit as the "last known good > > > > > > > ref", so i can't say when it started failing. > > > > > > > > > > > > That was because I couldn't upgrade for a long time and bumped both GCC > > > > > > and Valgrind (and actually the whole system) at once. Sorry, I couldn't do > > > > > > it gradually. > > > > > > > > > > does your fate client have optimizations enabled ? > > > > > i dont see them being disabled, and iam not sure valgrind + full > > > > > compiler optimizations are a good idea > > > > > > > > > > if they are enabled, please try to disable them to check if that > > > > > causes the false positives > > > > > > > > > > > > > Yeah actually, only --extra-cflags=-fno-tree-vectorize is needed to make > > > > fate-vsynth3-svq1 pass. I thought it was supposed to be disabled? > > > > > > > > > my local valgrind shows nothing for svq1 > > > > > > > > > > > With aed84ee4d reverted? --toolchain=valgrind-memcheck > > > > --disable-stripping? > > > > > > reverted, yes > > > no toolchain, no stripping just tried it under valgrind > > > > > > but thats with a relativly old gcc > > > > > > > Actually i was wrong: the GCC option that makes valgrind go crazy is > > unswitch-loops, not tree-vectorize. Do you have it in your GCC? > > its in man gcc on the box i tried on. > maybe gcc has improved between version 4 an 6 > but it doesnt really matter > > if it fixes all the false positives, please add it and then lets > revert the workaround (s?) > I reverted and I'm in the process of reporting to valgrind.
On Mon, Apr 10, 2017 at 04:10:14PM +0200, Clément Bœsch wrote:
> I reverted and I'm in the process of reporting to valgrind.
Here you go:
https://bugs.kde.org/show_bug.cgi?id=378622
On Mon, Apr 10, 2017 at 04:14:44PM +0200, Clément Bœsch wrote: > On Mon, Apr 10, 2017 at 04:10:14PM +0200, Clément Bœsch wrote: > > I reverted and I'm in the process of reporting to valgrind. > > Here you go: > https://bugs.kde.org/show_bug.cgi?id=378622 Thanks! [...]
diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c index d3e60c3a4a..e5e43fc8e2 100644 --- a/libavcodec/svq1dec.c +++ b/libavcodec/svq1dec.c @@ -160,7 +160,7 @@ static int svq1_decode_block_intra(GetBitContext *bitbuf, uint8_t *pixels, uint8_t *list[63]; uint32_t *dst; const uint32_t *codebook; - int entries[6]; + int entries[6] = { 0 }; int i, j, m, n; int stages; unsigned mean; @@ -227,7 +227,7 @@ static int svq1_decode_block_non_intra(GetBitContext *bitbuf, uint8_t *pixels, uint8_t *list[63]; uint32_t *dst; const uint32_t *codebook; - int entries[6]; + int entries[6] = { 0 }; int i, j, m, n; int stages; unsigned mean;
Fixes valgrind warnings about "Use of uninitialised value of size 8" Signed-off-by: James Almer <jamrial@gmail.com> --- This probably just silences a bunch of false possitives. libavcodec/svq1dec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)