Opened 3 years ago

Closed 3 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)

gdb.out.txt (11.9 KB ) - added by danadam 3 years ago.
gdb-full.out.txt (14.3 KB ) - added by danadam 3 years ago.
loud2.wav (516.9 KB ) - added by danadam 3 years ago.

Download all attachments as: .zip

Change History (14)

by danadam, 3 years ago

Attachment: gdb.out.txt added

comment:1 by Carl Eugen Hoyos, 3 years ago

Keywords: astats added
Priority: normalminor
Summary: astats crash on float pcm having peaks much greater than 1.0astats undefined behaviour on float pcm having peaks much greater than 1.0

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.

comment:2 by Carl Eugen Hoyos, 3 years ago

Or even better, please attach an input file that allows to reproduce.

comment:3 by Marton Balint, 3 years ago

Keywords: crash added
Priority: minorimportant

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:4 by Elon Musk, 3 years ago

Unfortunately valgrind and ffmpeg here are fine. Build with clang.

comment:5 by Marton Balint, 3 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 danadam, 3 years ago

Attachment: gdb-full.out.txt added

by danadam, 3 years ago

Attachment: loud2.wav added

comment:6 by danadam, 3 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 Balling, 3 years ago

Fixed in 89c9c42c5b85b68eddf891e929cfdebd8c163547, I suppose ;) In your opinion is that a bug in gcc that should be reported?

Last edited 3 years ago by Balling (previous) (diff)

comment:8 by danadam, 3 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:9 by Elon Musk, 3 years ago

Fix your broken compiler.

comment:10 by danadam, 3 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 Marton Balint, 3 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.