embench/embench-iot

rand_beebs has undefined behaviour for targets with 32 bit long

amylaar opened this issue · 2 comments

rand_beebs in support/beebsc.c has a signed overflow.
The constant 1103515245L is type long, which is signed.
After in the crc_32 benchmark, after calling calling srand_beebs (0),
the first call to rand_beebs calculates a new seed value of 12345.
Then in the second call, 12345 is multiplied with 1103515245L,
which overflows on targets with 32 bit long, thus invoking undefined behaviour.

That has the practical result that he crc_32 test fails when compiled for a 32 bit target with gcc (observed with gcc10 and gcc11)
and options -O2 -flto . During the second "dom" pass of the compilation started by the linker plugin, the optimizer finds that the
result of the random calculation is always zero (unless there is an overflow, the possibility of which can be disregarded because that would be undefined behaviour).

Thanks @amylaar. Assigned to @PaoloS02 to work out fix for the next release.

If either (or both) of the factors gets its type to unsigned long, the overflow in the multiplication becomes well-defined, and the
product will also have type unsigned long, which also makes any overflow in the sum well-defined.
I.e. you can declare seed as unsigned long. Or change 1103515245L to 1103515245UL . Or both. It becomes well-defined
either way.
I think some compilers warn when mixing signed/unsigned types, to avoid that, you can change both, and change
12345 to 12345U or 12345UL for good measure.