(In reply to Jean-Yves Avenard [:jya] from comment #7)
(In reply to Andreas Pehrson [:pehrsons] from comment #4)
Created attachment 9056517 [details]
Bug 1542685 - Avoid integer overflows by multiplying doubles. r?padenot
SaferMultDiv(time, audioScale, videoScale) could easily result in overflow
because all three args are roughly equal, and SaferMultDiv would always do the
multiplication first. The worst-case is then multiplying an int64_t to another
int64_t that have very similar values. Since we represent time here in
microseconds, this would overflow after only 50 minutes.
Not it cannot. And it doesnt' do the multiplication first, that's precisely the problem it's designed to avoid
SaferMultDiv can only overflow if the end result itself doesn't fit on 64 bits.
Using double only means you lose accuracy as the mantissa is only 52 bits.
See , aka
return CheckedInt64(remainder) * mul / div + CheckedInt64(major) * mul;
CheckedInt64(remainder) * mul happens before the division with
div is larger than either
mul, swapping doesn't help, and
remainder ends up equal to
aValue. The multiplication above then overflows and the division that later happens cannot make the CheckedInt64 valid again, even if it would have brought the result into valid int64-range.
An obvious example proving you wrong is
SaferMultDiv(2^63-2, 2^63-2, 2^63-1). Expanded this becomes
return CheckedInt64(2^63-2) * (2^63-2) / (2^63-1) + CheckedInt64(0) * (2^63-2);