Bug 63566 - atomic builtins detection not quite correct
Summary: atomic builtins detection not quite correct
Status: NEW
Alias: None
Product: APR
Classification: Unclassified
Component: APR (show other bugs)
Version: 1.7.0
Hardware: Other NetBSD
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-16 13:56 UTC by Valery Ushakov
Modified: 2020-01-10 19:18 UTC (History)
2 users (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Valery Ushakov 2019-07-16 13:56:16 UTC
Building apr and httpd on NetBSD/macppc (32-bit powerpc) fails:

/bin/sh /export/netbsd/cvs/pkgsrc-quarterly/www/apache24/work.powerpc/httpd-2.4.39/build/libtool --silent --mode=link gcc  -pthread  -pipe -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include/apr-1 -I/usr/pkg/include -I/usr/include -L/usr/pkg/lib -Wl,-R/usr/pkg/lib -L/usr/lib -Wl,-R/usr/lib -o htpasswd  htpasswd.lo passwd_common.lo /usr/pkg/lib/libaprutil-1.la -lexpat /usr/pkg/lib/libapr-1.la -lrt -lcrypt -lpthread -lcrypt
/export/netbsd/cvs/pkgsrc-quarterly/www/apache24/work.powerpc/.buildlink/lib/libapr-1.so: undefined reference to `__sync_fetch_and_add_8'
/export/netbsd/cvs/pkgsrc-quarterly/www/apache24/work.powerpc/.buildlink/lib/libapr-1.so: undefined reference to `__sync_val_compare_and_swap_8'
/export/netbsd/cvs/pkgsrc-quarterly/www/apache24/work.powerpc/.buildlink/lib/libapr-1.so: undefined reference to `__sync_lock_test_and_set_8'
/export/netbsd/cvs/pkgsrc-quarterly/www/apache24/work.powerpc/.buildlink/lib/libapr-1.so: undefined reference to `__sync_fetch_and_sub_8'
/export/netbsd/cvs/pkgsrc-quarterly/www/apache24/work.powerpc/.buildlink/lib/libapr-1.so: undefined reference to `__sync_sub_and_fetch_8'
*** Error code 1

apr tries to detect if the compiler HAVE_ATOMIC_BUILTINS by doing a
configure test with an unsigned long test variable and it succeeds b/c
ppc has 4-byte atomics.  Then apr uses that condition to cover all of
its atomic functions, including 8-byte, which are not implemented on
ppc.  So gcc emits calls to external functions for them and those are
unresolved.

apr should probably do the configure test with a known 8 byte type
instead, to make the test match the actual usage of the condition.

I was able to work around this by building with
ap_cv_atomic_builtins=no in the environment to force its hand.

Specifically for ppc it also needed an additional change to
include/arch/unix/apr_arch_atomic.h.  That header tests for ppc by
checking __PPC__ and __ppc__ which are not defined, but doesn't check
__powerpc__ which is.
Comment 1 Ivan A. Melnikov 2019-09-11 14:13:03 UTC
I'm getting the same error on linux on my mipsel (32-bit LE) machines. On 32-bit MIPS gcc also does not have 64-bit built-in atomics. I'm currently disabling nonportable atomics for our mips32 builds.

> apr should probably do the configure test with a known 8 byte type
> instead, to make the test match the actual usage of the condition.

Since there some provisioning for having special implementation for 32-bit atomics and using generic code for 64-bit atomics, I think it's better to have two separate checks -- for 32-bit and 64-bit builtins. I'd provide a patch but I'm not sure what is the best way of creating uint64_t value in the check code without including stdint.h.
Comment 2 Luke Morrison 2020-01-10 19:18:44 UTC
Also happens on FreeBSD 12.1 (32-bit powerpc) for the same reasons.

Ideally, the test in configure which detects for compiler intrinsic should be able to separately discriminate between atomic32 and atomic64, and then have the option of defining one or both of HAVE_ATOMIC_BUILTINS as well as NEED_ATOMICS_GENERIC64.

To wit:
Each of the following combinations are possibly valid, depending on CPU architecture and compiler:

1) No compiler built-ins available at all, and no CPU-specific code has been provided by APR itself to efficiently implement atomics; USE_ATOMICS_GENERIC will be automatically set true.

2) No compiler built-ins available, but CPU-specific code is provided by APR to implement at least atomic32: USE_ATOMICS_GENERIC will be false, and NEED_ATOMICS_GENERIC64 may or may not be true depending on the CPU's capabilities.

3) Compiler built-ins are available for both atomic32 and atomic64: USE_ATOMICS_GENERIC and NEED_ATOMICC_GENERIC64 are both false.

4) Compiler built-ins are available for atomic32, but not for atomic64: USE_ATOMICS_GENERIC will be false, and NEED_ATOMICS_GENERIC64 ought to be true.

Currently, cases 1, 2, and 3 can be dealt with successfully with apr 1.7.0.
However, case 4 will not be detected, and it is implicitly replaced with case 3, resulting in a broken build.