Opened 4 years ago
Closed 4 years ago
#9049 closed defect (fixed)
astats undefined behaviour on float pcm having peaks much greater than 1.0
Reported by: | danadam | Owned by: | |
---|---|---|---|
Priority: | important | Component: | avfilter |
Version: | git-master | Keywords: | astats crash |
Cc: | Blocked By: | ||
Blocking: | Reproduced by developer: | no | |
Analyzed by developer: | no |
Description
Summary of the bug:
A float pcm (e.g. f32le) with samples greater than about 262176.004 causes segmentation fault in astats
filter.
(that sample value is about 108 dB higher than 1.0, so admittedly it's not your typical file)
How to reproduce:
Generate full scale sine:
]$ ffmpeg -f lavfi -i aevalsrc="sin(261*2*PI*t):s=44100:d=3" -c pcm_f32le sin.wav
Increase volume by 108dB and run through astats
(still works):
]$ ffmpeg -i sin.wav -af volume=108dB -c pcm_f32le loud1.wav ]$ ffmpeg -i loud1.wav -af astats -f null /dev/null ...
Increase volume by 109dB and run through astats
(crashes):
]$ ffmpeg -i sin.wav -af volume=108dB -c pcm_f32le loud2.wav ]$ ffmpeg -i loud2.wav -af astats -f null /dev/null ffmpeg version N-100545-g15baa0c Copyright (c) 2000-2021 the FFmpeg developers built with gcc 10 (Debian 10.2.1-1) configuration: --prefix=/tmp/ffmpeg_build --extra-cflags=-I/tmp/ffmpeg_build/include --extra-ldflags=-L/tmp/ffmpeg_build/lib --extra-libs='-lpthread -lm' --bindir=/home/danadam/bin --enable-gpl --enable-gnutls --enable-libass --enable-libfreetype --enable-libvorbis --enable-nonfree libavutil 56. 63.100 / 56. 63.100 libavcodec 58.115.102 / 58.115.102 libavformat 58. 65.100 / 58. 65.100 libavdevice 58. 11.103 / 58. 11.103 libavfilter 7. 95.100 / 7. 95.100 libswscale 5. 8.100 / 5. 8.100 libswresample 3. 8.100 / 3. 8.100 libpostproc 55. 8.100 / 55. 8.100 Input #0, wav, from 'loud2.wav': Metadata: encoder : Lavf58.65.100 Duration: 00:00:03.00, bitrate: 1411 kb/s Stream #0:0: Audio: pcm_f32le ([3][0][0][0] / 0x0003), 44100 Hz, mono, flt, 1411 kb/s Stream mapping: Stream #0:0 -> #0:0 (pcm_f32le (native) -> pcm_s16le (native)) Press [q] to stop, [?] for help Segmentation fault
The crash happens in libavfilter/af_astats.c:334
p->histogram[index]++;
because index
has value -2147483648:
index = av_clip(FFABS(nd) * HISTOGRAM_MAX, 0, HISTOGRAM_MAX);
Here nd * HISTOGRAM_MAX
is converted to int and overflows and this is undefined behavior.
Attachments (3)
Change History (14)
by , 4 years ago
Attachment: | gdb.out.txt added |
---|
comment:1 by , 4 years ago
Keywords: | astats added |
---|---|
Priority: | normal → minor |
Summary: | astats crash on float pcm having peaks much greater than 1.0 → astats undefined behaviour on float pcm having peaks much greater than 1.0 |
comment:3 by , 4 years ago
Keywords: | crash added |
---|---|
Priority: | minor → important |
I don't think a sample file is needed, this crash can be reproduced with a single command line:
ffmpeg -f lavfi -i "sine,aformat=sample_fmts=fltp,volume=200dB,astats" -f null x
comment:5 by , 4 years ago
It crashes here with GCC 7.5.0.
0x00000000006b53af <+8399>: mulsd xmm0,xmm8 0x00000000006b53b4 <+8404>: cvttsd2si ebx,xmm0 0x00000000006b53b8 <+8408>: cmp ebx,0x1fff 0x00000000006b53be <+8414>: cmovg ebx,r11d 0x00000000006b53c2 <+8418>: cmp eax,ebx 0x00000000006b53c4 <+8420>: cmovl eax,ebx 0x00000000006b53c7 <+8423>: movsxd rbx,ebx => 0x00000000006b53ca <+8426>: add DWORD PTR [r12+rbx*4+0xf8],0x1
rax 0x0 0 rbx 0xffffffff80000000 -2147483648 r11 0x1fff 8191
by , 4 years ago
Attachment: | gdb-full.out.txt added |
---|
by , 4 years ago
comment:6 by , 4 years ago
please provide the ffmpeg command line running inside gdb including its complete, uncut console output and the backtrace etc.
Added gdb-full.out.txt.
please attach an input file that allows to reproduce.
Added loud2.wav.
I don't think a sample file is needed, this crash can be reproduced with a single command line:
Yep
Unfortunately valgrind and ffmpeg here are fine. Build with clang.
True. If I read things correctly then gcc
optimizes more aggressively than clang
and removes the a < amin
check in av_clip()
function. Presumably because FFABS(nd) * HISTOGRAM_MAX
cannot be negative.
comment:7 by , 4 years ago
Fixed in 89c9c42c5b85b68eddf891e929cfdebd8c163547, I suppose ;)
comment:8 by , 4 years ago
Fixed in 89c9c42c5b85b68eddf891e929cfdebd8c163547, I suppose ;)
Now it crashes 2 lines later in:
p->histogram[av_clip(FFABS(drop) * HISTOGRAM_MAX, 0, HISTOGRAM_MAX)]--;
It is the same situation (gcc (Debian 10.2.1-1)):
0x00005555558d7d1f <filter_channel+5423>: andpd %xmm6,%xmm3 0x00005555558d7d23 <filter_channel+5427>: mulsd %xmm7,%xmm3 0x00005555558d7d27 <filter_channel+5431>: cvttsd2si %xmm3,%eax 0x00005555558d7d2b <filter_channel+5435>: cmp $0x1fff,%eax 0x00005555558d7d30 <filter_channel+5440>: cmovg %ebx,%eax 0x00005555558d7d33 <filter_channel+5443>: add $0x1,%ecx 0x00005555558d7d36 <filter_channel+5446>: cltq => 0x00005555558d7d38 <filter_channel+5448>: subl $0x1,0xf8(%r13,%rax,4)
rax 0xffffffff80000000 -2147483648 rbx 0x1fff 8191 rcx 0x2a 42 r13 0x55555730eb00 93825023404800 eflags 0x10202 [ IF RF ] xmm3 v2_double = {2152157349.40625, 0} xmm6 v2_double = {nan(0xfffffffffffff), 0} xmm7 v2_double = {8191, 0}
In your opinion is that a bug in gcc that should be reported?
Not sure if the question was for me, but AFAIK compilers work with the assumption that the program does not contain UB. Under that assumption this optimization is OK (again, AFAIK :-) ).
Here's that situation distilled to minimum: https://godbolt.org/z/1j5ao8
comment:10 by , 4 years ago
It's not like clang doesn't take advantage of UBs too:
]$ cat ubtest.c #include <stdio.h> int main(int argc, char *argv[]) { (void)argv; double d = (argc > 1 ? 0.5 : 262177); int i = d * 8191; printf("%d\n", i); } ]$ clang -Wall -Wextra -pedantic -O2 ubtest.c && ./a.out 4095
comment:11 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fixed in c8e1e56509c3f176803cdf40b4019263125fa43f.
To make this a valid ticket, please provide the
ffmpeg
command line running inside gdb including its complete, uncut console output and the backtrace etc.