Fix undefined behavior: shift exponent is too large

Previously, the bestOrder value was being decremented inside
the loop. This caused the following:

In the case where the loop enters with bestOrder == 2, the
value is decremented to 1, and the following operations occurs:

m_tnsPredGains[channelIndex] >> (8 * bestOrder - 16)

bestOrder being 1 - this evaluates to (8 - 16), causing an integer
underflow, and a right shift by 4294967288. m_tnsPredGains is a
32-bit unsigned int, right-shifting by a value > 32 is undefined.

I think the intent of the loop conditional is to avoid running the
calculation with bestOrder == 1 - as far as I can tell, this is
the lowest possible value.

By re-arranging the comparison and using a prefix decrement,
we can exhibit the same behavior but avoid the case where
bestOrder == 1 within the loop body.
This commit is contained in:
John Regan 2022-07-07 09:38:07 -04:00
parent 4ef4bc3c96
commit c2665b890c
2 changed files with 2 additions and 4 deletions

View File

@ -731,9 +731,8 @@ unsigned ExhaleEncoder::getOptParCorCoeffs (const SfbGroupData& grpData, const u
predGainCurr = (temp >> 24) & UCHAR_MAX;
predGainPrev = (temp >> 16) & UCHAR_MAX;
while ((bestOrder > 1) && (predGainPrev >= predGainCurr)) // lowest-order gain max.
while ((predGainPrev >= predGainCurr) && --bestOrder > 1) // lowest-order gain max.
{
bestOrder--;
predGainCurr = predGainPrev;
predGainPrev = (temp >> (8 * bestOrder - 16)) & UCHAR_MAX;
}

View File

@ -66,9 +66,8 @@ unsigned SpecAnalyzer::getLinPredCoeffs (short parCorCoeffs[MAX_PREDICTION_ORDER
predGainCurr = (m_tnsPredGains[channelIndex] >> 24) & UCHAR_MAX;
predGainPrev = (m_tnsPredGains[channelIndex] >> 16) & UCHAR_MAX;
while ((bestOrder > 1) && (predGainPrev >= predGainCurr)) // find lowest-order gain maximum
while ((predGainPrev >= predGainCurr) && --bestOrder > 1) // find lowest-order gain maximum
{
bestOrder--;
predGainCurr = predGainPrev;
predGainPrev = (m_tnsPredGains[channelIndex] >> (8 * bestOrder - 16)) & UCHAR_MAX;
}