Use of undefined values in SNS decoding due to wrong scope of array variable
git SHA: 518d9f24
Running
make clean
make -j CLANG=1
./IVAS_cod -stereo 48000 48 ltv48_STEREO.wav bit
./IVAS_dec stereo 48 bit out.wavresults in:
==351477==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x1ce7915 in L_sub /local/knj/ivas-basop/lib_com/basop32.c:1750:10
#1 0x2780c67 in sns_interpolate_scalefactors_fx /local/knj/ivas-basop/lib_com/ivas_sns_com_fx.c:245:58
#2 0xe0b17f in stereo_mdct_core_dec /local/knj/ivas-basop/lib_dec/ivas_stereo_mdct_core_dec.c:375:17
#3 0x1908325 in ivas_core_dec /local/knj/ivas-basop/lib_dec/ivas_core_dec.c:448:17
#4 0x1944eb7 in ivas_cpe_dec_fx /local/knj/ivas-basop/lib_dec/ivas_cpe_dec_fx.c:436:9
#5 0x89bcfa in ivas_jbm_dec_tc /local/knj/ivas-basop/lib_dec/ivas_jbm_dec.c:146:9
#6 0x4eab24 in IVAS_DEC_GetTcSamples /local/knj/ivas-basop/lib_dec/lib_dec_fx.c:1071:9
#7 0x4e4685 in IVAS_DEC_GetSamples /local/knj/ivas-basop/lib_dec/lib_dec_fx.c:904:17
#8 0x4c2e0a in decodeG192 /local/knj/ivas-basop/apps/decoder.c:1675:28
#9 0x4a6ea7 in main /local/knj/ivas-basop/apps/decoder.c:576:17
#10 0x7f661a566249 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#11 0x7f661a566304 in __libc_start_main csu/../csu/libc-start.c:360:3
#12 0x421540 in _start (/local/knj/ivas-basop/IVAS_dec+0x421540)The reason is a bug in ivas_mdct_core_dec.c, see the snippet below starting from line 735 with some annotations from me. To decode the SNS parameters for the two stereo channels, a loop over the channels (i.e. looping 2 times) is done. Inside, a temporary variable SNS_Q_fx is declared to store the decoded (fixed-point) SNS parameters. In each loop the SNS parameters for only one channel are decoded, but assigned to both channels. In the second loop iteration, the first channels SNS parameters are thus assigned again, but from a new local variable as the previous SNS_Q_fx has gone out of scope in the meantime. Usually, I guess that the memory area from the last loop iteration has not been assigned to something else, so the old values might still be around, but in general the new buffer array can point to anywhere.
Here the current state of the file with added comments from me:
for ( ch = 0; ch < CPE_CHANNELS; ch++ )
{
st = sts[ch];
if ( st->mct_chan_mode != MCT_CHAN_MODE_IGNORE )
{
#ifdef IVAS_FLOAT_FIXED
Word32 SNS_Q_fx[CPE_CHANNELS][NB_DIV][M]; // <-- Newly in scope for each channel
Word16 q_snsq = 0;
sns_avq_dec_fx( param_lpc[ch], SNS_Q_fx[ch], &q_snsq, st->L_frame, st->numlpc );
for ( int i = 0; i < CPE_CHANNELS; i++ ) // <- loop over channels here done for every channel
{
for ( int j = 0; j < NB_DIV; j++ )
{
for ( k = 0; k < M; k++ )
{
sns[i][j][k] = fix_to_float( SNS_Q_fx[i][j][k], q_snsq ); // <- potentially uninitialized data is assigned here
}
}
}My suggestion would be to change this to e.g.:
for ( ch = 0; ch < CPE_CHANNELS; ch++ )
{
st = sts[ch];
if ( st->mct_chan_mode != MCT_CHAN_MODE_IGNORE )
{
#ifdef IVAS_FLOAT_FIXED
Word32 SNS_Q_fx[NB_DIV][M]; // <-- only declare for one channel
Word16 q_snsq = 0;
sns_avq_dec_fx( param_lpc[ch], SNS_Q_fx, &q_snsq, st->L_frame, st->numlpc );
for ( int j = 0; j < NB_DIV; j++ )
{
for ( k = 0; k < M; k++ )
{
sns[ch][j][k] = fix_to_float( SNS_Q_fx[j][k], q_snsq );
}
}