Hi,

there are a number of problems with the atomics in the latest apr release. All the results are on Debian sid(unstable).

On powerpc we got a segfault:

Thread 1 (Thread 0xf79226a0 (LWP 458077)):
#0  0x009dc6d0 in mutex_hash (mem=0xffd5f420) at ./atomic/unix/mutex64.c:79
        mutex = <optimized out>
        mutex = <optimized out>
#1 apr_atomic_set64 (mem=mem@entry=0xffd5f420, val=2) at ./atomic/unix/mutex64.c:104
        mutex = <optimized out>
#2 0x00a66ce4 in test_set64 (tc=tc@entry=0xffd5f454, data=data@entry=0x0) at ./test/testatomic.c:236
        y64 = 18434909205434407696
#3 0x00a4ae38 in abts_run_test (ts=ts@entry=0x1f80260, f=f@entry=0xa66ca0 <test_set64>, value=value@entry=0x0) at ./test/abts.c:186
        tc = {failed = 0, suite = 0x1f80230}
        ss = 0x1f80230
#4  0x00a693dc in testatomic (suite=0x1f80260) at ./test/testatomic.c:940
No locals.
#5 0x00a4a3b8 in main (argc=<optimized out>, argv=<optimized out>) at ./test/abts.c:444
        i = <optimized out>
        rv = <optimized out>
        list_provided = <optimized out>
        suite = <optimized out>
(gdb) p hash_mutex
$1 = (apr_thread_mutex_t **) 0x0
(gdb) q

Fixed in http://svn.apache.org/viewvc?view=revision&revision=1907442 . Build log at https://buildd.debian.org/status/fetch.php?pkg=apr&arch=powerpc&ver=1.7.2-1&stamp=1675382351&raw=0

On armel, we got a link error:

/usr/bin/ld: ../.libs/libapr-1.so: undefined reference to `__atomic_fetch_sub_8'
/usr/bin/ld: ../.libs/libapr-1.so: undefined reference to `__atomic_load_8'
/usr/bin/ld: ../.libs/libapr-1.so: undefined reference to `__atomic_exchange_8'
/usr/bin/ld: ../.libs/libapr-1.so: undefined reference to `__atomic_store_8'
/usr/bin/ld: ../.libs/libapr-1.so: undefined reference to `__atomic_fetch_add_8' /usr/bin/ld: ../.libs/libapr-1.so: undefined reference to `__atomic_compare_exchange_8'

Build log at https://buildd.debian.org/status/logs.php?pkg=apr&arch=armel&suite=sid
Fixed in http://svn.apache.org/viewvc?view=revision&revision=1907441


However, even with both fixes, there still seem to be problems. On powerpc, there was the same test failure as in PR 66457:
testatomic: Line 908: Unexpected value

Log at https://buildd.debian.org/status/fetch.php?pkg=apr&arch=powerpc&ver=1.7.2-2&stamp=1675512084&raw=0

If I am not mistaken, on 32 bit architectures you always need to ensure manually that both halves of a 64 bit value are written atomically. So code like this

apr_atomic_set64()
...
   *mem = val;

seems wrong.

Also, this construct

#if defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__)
#define WEAK_MEMORY_ORDERING 1
#else
#define WEAK_MEMORY_ORDERING 0
#endif

seems to be very fragile. According to the table in https://en.wikipedia.org/wiki/Memory_ordering , basically all architectures except x86, amd64, and s390 have weak ordering. So if we need a list of archs, we must define the list of architectures that have strong ordering and treat everything else as weak.

BTW, the state of atomics in 1.7.0 has not been perfect, either. I had applied this workaround to make it work:

https://salsa.debian.org/apache-team/apr/-/blob/debian/1.7.0-8/debian/patches/generic-64bit-atomics.patch

But unfortunately I didn't have time to create a proper patch for upstream.

Cheers,
Stefan

Reply via email to