#424 undefined behavior when decoding LPC subframes

[Markus]

When decoding LPC subframes using libFLAC (FLAC__lpc_restore_signal) a shift operation with a potentially negative right operand (lp_quantization) is done. The C standard states that in this case the behavior is undefined. There might not be any files having negative values or platforms doing what is expected, still this isn't fully correct.

[Erik]

How did you find this? Was it using some tool? If so, which one and what was the full message? Or is there an example file that triggers this?

[Markus]

I'm implementing a decoder on a soft core processor using libFLAC as reference.
There is no specific problem and I couldn't find a file with a negative value.

FLAC format

Quantized linear predictor coefficient shift needed in bits (NOTE: this number is signed two's-complement).

ISO/IEC 9899:1999 6.5.7.3

If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined.

The point is that libFLAC uses a statement that has undefined behavior and it's result depends on compiler and platform. Some might shift in opposite direction others will just interpret as unsigned.

[lvqcl]

During encoding, the value of lp_quantization is calculated by FLAC__lpc_quantize_coefficients() function in lpc.c.
This function makes sure that it is always non-negative. Also, there's a comment in it:

/* negative shift is very rare but due to design flaw, negative shift is
 * a NOP in the decoder, so it must be handled specially by scaling down
 * coeffs
 */

Added: but it seems that ffmpeg verifies its value on decoding:

qlevel = get_sbits(&s->gb, 5);
if (qlevel < 0) {
    av_log(s->avctx, AV_LOG_ERROR, "qlevel %d not supported, maybe buggy stream\n",
           qlevel);
    return AVERROR_INVALIDDATA;
}

Added2: IMHO it should be mentioned in FLAC docs that its value MUST be non-negative.