concurrencykit / ck

Concurrency primitives, safe memory reclamation mechanisms and non-blocking (including lock-free) data structures designed to aid in the research, design and implementation of high performance concurrent systems developed in C99+.
http://concurrencykit.org/
Other
2.34k stars 312 forks source link

ck_ec test fails on 32-bit musl systems #208

Open nmeum opened 1 year ago

nmeum commented 1 year ago

While working on enabling the concurrencykit test suite for our Alpine Linux package, I noticed that the tests fail on 32-bit architectures. More specifically, the issue is the ck_ec regression test which fails with the following error message:

$ uname -m
i686
$ make -C regressions/ck_ec/validate check
./ck_ec_smoke_test
test_update_counter SP passed.
test_update_counter MP passed.
test_deadline passed.
test_wait passed.
pred wait: 0.002000
pred wait: 0.016000
pred wait: 0.128000
pred wait: 1.000000
test_wait_pred passed.
test_threaded SP passed.
test_threaded MP passed.
./prop_test_slow_wakeup -max_total_time=60
./prop_test_timeutil_add -max_total_time=60
Assertion failed: (uint32_t)actual.tv_sec == (uint32_t)(nanos / (NSEC_MAX + 1)) (prop_test_timeutil_add.c: test_timespec_add: 94)
make: *** [Makefile:28: check] Aborted

Within the prop_test_timeutil_add.c test the problem is the following test case:

https://github.com/concurrencykit/ck/blob/a16642f95c048c65d47107205a2cfc70d099dbd6/regressions/ck_ec/validate/prop_test_timeutil_add.c#L51-L58

This test case is supposed to check that timespec_add clamps its return values to TIME_MAX / NSEC_MAX on overflow. This is achieved by testing the return value of that function against ts_to_nanos which is defined as follows:

https://github.com/concurrencykit/ck/blob/a16642f95c048c65d47107205a2cfc70d099dbd6/regressions/ck_ec/validate/prop_test_timeutil_add.c#L78-L81

The type dword_t is probably supposed to mean “double word” (or something along those lines) and should presumably be twice as large (in terms of byte size) as time_t. The dword_t type is defined here:

https://github.com/concurrencykit/ck/blob/a16642f95c048c65d47107205a2cfc70d099dbd6/regressions/ck_ec/validate/prop_test_timeutil_add.c#L8-L12

The implicit assumption here being that time_t is a 32-bit value. However, TIME_MAX does not make this assumption and is defined over sizeof(time_t) as follows:

https://github.com/concurrencykit/ck/blob/e18e9d0af30c99c7374b623086119148137f5613/src/ck_ec_timeutil.h#L9

This causes the overflow handling of ts_to_nanos to not match the handling of timespec_add if time_t is a 64-bit value on a 32-bit architecture. This is exactly the case on musl libc based systems (like Alpine Linux) since, contrary to glibc, musl-based 32-bit systems are 2038-ready and hence use a 64-bit time_t. Therefore, the test case fails on 32-bit Alpine Linux systems:

(gdb) n
94                      assert(actual.tv_sec == (time_t)(nanos / (NSEC_MAX + 1)));
(gdb) p actual
$1 = {tv_sec = 9223372036854775807, tv_nsec = 999999999}
(gdb) p nanos
$2 = 1000000999
(gdb) n
Assertion failed: actual.tv_sec == (time_t)(nanos / (NSEC_MAX + 1)) (prop_test_timeutil_add.c: test_timespec_add: 94)

I ran into this on concurrencykit 0.7.1, but this should be reproducible with current Git HEAD as well. Since __int128 cannot be used on i686 I am not sure how to best fix this...

P.S.: The same issue applies to prop_test_timeutil_add_ns.c.