astats undefined behaviour on float pcm having peaks much greater than 1.0

[danadam]

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.

[Carl Eugen Hoyos]

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.

[Carl Eugen Hoyos]

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

[Marton Balint]

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

[Elon Musk]

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

[Marton Balint]

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

[danadam]

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.

[Balling]

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

[danadam]

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

[Elon Musk]

Fix your broken compiler.

[danadam]

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

[Marton Balint]

Fixed in c8e1e56509c3f176803cdf40b4019263125fa43f.