Bug #3407: libosmocore utils_test isqrt_test() may trigger sanitizer - libosmocore - Open Source Mobile Communications

Actions

Copy link

Bug #3407

closed

libosmocore utils_test isqrt_test() may trigger sanitizer

Added by neels over 5 years ago. Updated over 5 years ago.

Status:

Resolved

Priority:

Normal

Assignee:

neels

Category:

Target version:

Start date:

07/19/2018

Due date:

% Done:

90%

Spec Reference:

Description

Testing integer square-root
../../../src/libosmocore/tests/utils/utils_test.c:445:18: runtime error: signed integer overflow: 60369 * 60369 cannot be represented in type 'int'

I think it's a really bad idea to use random data in a regression test.
It thwarts the reproducibility and causes sporadic failure, like this one.
Rather pick constants from all corners of the desired value range.

Added by:

commit 15a5f8de00c9c11a985ab16279ffae02b1e4667f
Author: Harald Welte <laforge@gnumonks.org>
Date:   Wed Jun 6 16:58:17 2018 +0200

    Add osmo_isqrt32() to compute 32bit integer square root

    Change-Id: I2b96db6e037e72e92317fec874877e473a1cf909

Actions

Copy link

Updated by neels over 5 years ago

It seems to me this test is guaranteed to fail, yet doesn't ever seem to print "ERROR"

                uint32_t r = rand();   <--- a value up to UINT32_MAX
                uint32_t sq = x*x;     <--- multiplied by itself will overflow for anything > UINT16_MAX
                uint32_t y = osmo_isqrt32(sq); <--- so the original x is then by definition unrecoverable
                if (y != x)
                        printf("ERROR: x=%u, sq=%u, osmo_isqrt(%u) = %u\n", x, sq, sq, y); <-- I never see this, expect to see it a lot.

Is it some algorithmic magic? I doubt that we want to have any magic like that?

Actions

Copy link