clock: Fix unchecked overflows in linear regression code

The initial calculation for the precision shift was wrong and would allow for
overflows during the calculations which were not detected and lead to wrong
results.

Also add a test for a case where overflows where previously not detected and
caused a completely wrong result.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7406>
This commit is contained in:
Sebastian Dröge 2024-08-23 15:56:41 +03:00 committed by GStreamer Marge Bot
parent 4106ad4ae6
commit 41f3fab3d4
2 changed files with 52 additions and 5 deletions

View file

@ -4638,10 +4638,20 @@ gst_calculate_linear_regression (const GstClockTime * xy,
*/ */
/* Guess how many bits we might need for the usual distribution of input, /* Guess how many bits we might need for the usual distribution of input,
* with a fallback loop that drops precision if things go pear-shaped */ * with a fallback loop that drops precision if things go pear-shaped.
max_bits = gst_log2 (MAX (xmax - xmin, ymax - ymin)) * 7 / 8 + gst_log2 (n); *
if (max_bits > 64) * Each calculation of tmp during the iteration is multiplying two numbers and
pshift = max_bits - 64; * then adding them together, or adding them together and then multiplying
* them. The second case is worse and means we need at least twice
* (multiplication) as many bits as the biggest number needs, plus another bit
* (addition). At most 63 bits (signed 64 bit integer) are available.
*
* That means that each number must require at most (63 - 1) / 2 bits = 31
* bits of storage.
*/
max_bits = gst_log2 (MAX (1, MAX (xmax - xmin, ymax - ymin)));
if (max_bits > 31)
pshift = max_bits - 31;
i = 0; i = 0;
do { do {

View file

@ -1894,6 +1894,41 @@ static const GstClockTime times4[] = {
60, 100 60, 100
}; };
static const GstClockTime times5[] = {
3097492530135419, 3097492507369715,
3097492729101770, 3097492707369715,
3097493128807704, 3097493107369715,
3097493328386576, 3097493307369715,
3097493728105969, 3097493707369715,
3097494207585880, 3097494187369715,
3097495048016903, 3097495027369715,
3097496287625601, 3097496267369715,
3097497447848684, 3097497427369715,
3097498888090268, 3097498867369715,
3097499128283739, 3097499107369715,
3097501168351161, 3097501147369715,
3097502448473187, 3097502427369715,
3097502729226046, 3097502707369715,
3097503009343256, 3097502987369715,
3097503089910694, 3097503067369715,
3097503131675320, 3097503107369715,
3097503179587096, 3097503147369715,
3097504741040566, 3097504707369715,
3097504941855342, 3097504907369715,
3097506820823108, 3097506787369715,
3097508259718567, 3097508227369715,
3097508421611551, 3097508387369715,
3097509422069495, 3097509387369715,
3097511460842652, 3097511427369715,
3097511740612276, 3097511707369715,
3097513169836240, 3097513147369715,
3097514020101841, 3097513987369715,
3097515540180269, 3097515507369715,
3097516618966664, 3097516587369715,
3097518098778648, 3097518067369715,
3097518938513564, 3097518907369715
};
struct test_entry struct test_entry
{ {
gint n; gint n;
@ -1912,7 +1947,9 @@ struct test_entry
2321009753483354451}, { 2321009753483354451}, {
32, times3, 291922315691409, 162234934150296, 1370930728180888261, 32, times3, 291922315691409, 162234934150296, 1370930728180888261,
4392719527011673456}, { 4392719527011673456}, {
6, times4, 60, 100, 2, 1} 6, times4, 60, 100, 2, 1},
{G_N_ELEMENTS (times5) / 2, times5, 3097518938513564, 3097518903890035,
7927902279407500000, 7932215807091104881},
}; };
GST_START_TEST (test_regression) GST_START_TEST (test_regression)