Hi Dmitry,
On Sat, Jun 20, 2020 at 03:30:55PM +0300, Dmitry Sivachenko wrote:
> Hello!
>
> I am trying to compile haproxy-2.2-dev10 on FreeBSD-12/i386 (i386 is
> important here) with clang version 9.0.1.
>
> I get the following linker error:
>
> LD haproxy
> ld: error: undefined symbol: __atomic_fetch_add_8
> >>> referenced by backend.c
> >>> src/backend.o:(assign_server)
> >>> referenced by backend.c
> >>> src/backend.o:(assign_server)
> >>> referenced by backend.c
> >>> src/backend.o:(assign_server_and_queue)
> >>> referenced by backend.c
> >>> src/backend.o:(assign_server_and_queue)
> >>> referenced by backend.c
> >>> src/backend.o:(assign_server_and_queue)
> >>> referenced by backend.c
> >>> src/backend.o:(assign_server_and_queue)
> >>> referenced by backend.c
> >>> src/backend.o:(connect_server)
> >>> referenced by backend.c
> >>> src/backend.o:(connect_server)
> >>> referenced by backend.c
> >>> src/backend.o:(connect_server)
> >>> referenced by backend.c
> >>> src/backend.o:(srv_redispatch_connect)
> >>> referenced 233 more times
>
> ld: error: undefined symbol: __atomic_store_8
>
> For some time we apply the following patch to build on FreeBSD/i386:
>
> --- include/common/hathreads.h.orig 2018-02-17 18:17:22.21940 +
> +++ include/common/hathreads.h 2018-02-17 18:18:44.598422000 +
> @@ -104,7 +104,7 @@ extern THREAD_LOCAL unsigned long tid_bit; /* The bit
> /* TODO: thread: For now, we rely on GCC builtins but it could be a good
> idea to
> * have a header file regrouping all functions dealing with threads. */
>
> -#if defined(__GNUC__) && (__GNUC__ < 4 || __GNUC__ == 4 && __GNUC_MINOR__ <
> 7) && !defined(__clang__)
> +#if (defined(__GNUC__) && (__GNUC__ < 4 || __GNUC__ == 4 && __GNUC_MINOR__ <
> 7) && !defined(__clang__)) || (defined(__clang__) && defined(__i386__))
> /* gcc < 4.7 */
>
> #define HA_ATOMIC_ADD(val, i)__sync_add_and_fetch(val, i)
>
> (it is from older -dev but still applies to include/haproxy/atomic.h and
> fixes the build).
>
> If this patch is correct for i386, may be we include it to haproxy sources?
The problem seems to be that clang assumes that the atomic access may be
done unaligned. Fixing most of it is pretty straight forward, it mostly
involves adding __attribute__((aligned(64))) on all the relevant
structure members. However, no amount of cajoling helps for trace.c, as
long as we use ev_ptr, it always assumes it may be unaligned.
Even more funny, while
HA_ATOMIC_STORE(>report_events, 0); will work if report_events is
flagged as 64bits-aligned, HA_ATOMIC_STORE(ev_ptr == >report_events
? >report_events : >report_events, 0) will make clang unhappy.
So I don't know how to handle that properly. Willy, I think Dmitry's
solution of using the old builtins for clang/i386 might be the less
intrusive way of fixing it.
> PS: with that patch applied I get the following warning which can have sense:
>
> src/stick_table.c:3462:12: warning: result of comparison 'unsigned long' >
> 4294967295 is always false [-Wtautological-type-limit-compare]
> val > 0x)
> ~~~ ^ ~~
>
This one sounds mostly harmless, I guess whe should use a long long
here, and strtoull().
Olivier
