Skip to content

USAN: implicit-signed-integer-truncation in FEC HQ phase ECU

Basic info

Bug description

Clang USAN sanitizer test in pipeline found an error:

lib_dec/FEC_HQ_phase_ecu.c:2127:14: runtime error: implicit conversion from type 'int' of value 33124 (32-bit, signed) to type 'int16_t' (aka 'short') changed the value to -32412 (16-bit, signed)
	SUMMARY: UndefinedBehaviorSanitizer: implicit-signed-integer-truncation lib_dec/FEC_HQ_phase_ecu.c:2127:14 in 
	lib_dec/FEC_HQ_phase_ecu.c:2111:20: runtime error: implicit conversion from type 'int' of value 33600 (32-bit, signed) to type 'int16_t' (aka 'short') changed the value to -31936 (16-bit, signed)
	SUMMARY: UndefinedBehaviorSanitizer: implicit-signed-integer-truncation lib_dec/FEC_HQ_phase_ecu.c:2111:20 in

Link to test pipeline: https://forge.3gpp.org/rep/ivas-codec-pc/ivas-codec/-/jobs/325587

Ways to reproduce

⚠ I could not reproduce this. The commands from the logs are:

make clean
make -j CLANG=3
UBSAN_OPTIONS=suppressions=scripts/ubsan.supp,report_error_type=1,log_path=usan_log_catchall ./IVAS_cod -sba +1 -max_band FB -q 32000 48 ltv48_FOA.wav bit
eid-xor -vbr -fer bit scripts/testv/PLperc40mblen50.g192 bit_err
UBSAN_OPTIONS=suppressions=scripts/ubsan.supp,report_error_type=1,log_path=usan_log_catchall ./IVAS_dec -fr 10 FOA 48 bit_err out.wav

For me, the error is not triggered. I looked at the reported code part in function hq_phase_ecu(). The first complaint is about:

    /* seed for own_rand2 */
    seed = *time_offs;
    if ( *num_p > 0 )
    {
        seed += plocs[*num_p - 1]; // <<<-----
    }

plocs and seed are of type ìnt16_t, so potentially a overflow could happen here. As it is for the RNG seed, this could even be ok and intended, but maybe an explicit cast could be added.

The second error is about:

    else
    {
        *time_offs += output_frame; // <<<------
        if ( *time_offs <= 0 )
        {
            /* detect wrap around  of st->time_offs */
            *time_offs = MAX16B; /* continued muting will ensure that the now fixed seeds are not creating tones */
        }

        trans_ana( prevsynth + 2 * output_frame - Lprot, mag_chg, &ph_dith, mag_chg_1st, output_frame, *time_offs, env_stab, 0, alpha, beta, beta_mute, Xavg ); /* 1.0 stable-music,  0.0 speech-like */
    }

Here, also both variables on the marked line are int16_t. But from a first look, I don'T see why therer would be a bigger data type involved.

Also, in the same file at line 2094, an unnecessayr opearion is done, maybe the whole function could be reviewed:

            *time_offs += 0;