Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[David CARLIER]

Well, Haproxy uses directly a handmade Makefile and it might be possible to
write a short c code test to detect such features (just thinking aloud).

On 21 February 2018 at 14:15, Dmitry Sivachenko  wrote:

>
> > On 21 Feb 2018, at 16:33, David CARLIER  wrote:
> >
> > Might be irrelevant idea, but is it not possible to detect it via simple
> code test into the Makefile eventually ?
>
>
> Did you mean configure?  :)

Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[Dmitry Sivachenko]

> On 21 Feb 2018, at 16:33, David CARLIER  wrote:
> 
> Might be irrelevant idea, but is it not possible to detect it via simple code 
> test into the Makefile eventually ?


Did you mean configure?  :)

Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[David CARLIER]

Might be irrelevant idea, but is it not possible to detect it via simple
code test into the Makefile eventually ?

Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[Willy Tarreau]

Hi guys,

On Sat, Feb 17, 2018 at 09:15:56PM +0300, Dmitry Sivachenko wrote:
> > On 14 February 2018 at 11:09, Dmitry Sivachenko  wrote:
> >> What about this change?
> >> 
> >> --- work/haproxy-1.8.4/include/common/hathreads.h   2018-02-08 
> >> 13:05:15.0 +
> >> +++ /tmp/hathreads.h2018-02-14 11:06:25.031422000 +
> >> @@ -104,7 +104,7 @@ extern THREAD_LOCAL unsigned long tid_bi
> >> /* 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)
> >> 
> >> 
> 
> 
> > On 14 Feb 2018, at 14:13, David CARLIER  wrote:
> > Whatever works best for you. Regards.
> 
> 
> Well, I wonder if this is worth including into haproxy src?

Well, I'll see with Olivier with whom we've done some of the lockless
changes. We noticed already certain similar issues and figured that we
didn't properly restrict what CPUs should be supported. In practice
given that we do use some compare-and-swap operations that are not
present on i386, the minimally supported CPU should be i486. And I'm
pretty sure that looking closer we'll find other restrictions. So I'd
like that instead we try to cleanly redefine the exact compatbility
matrix so that threads are only enabled where reasonably possible, and
that we don't try to do marvels where it doesn't make sense.

Note that I've had a similar issue when building for a dual-core MIPS,
requiring libatomic for the same symbols, which my toolchain didn't have...
That situation is definitely not the cleanest we can think of!

Cheers,
Willy

Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[Dmitry Sivachenko]

> On 14 February 2018 at 11:09, Dmitry Sivachenko  wrote:
>> What about this change?
>> 
>> --- work/haproxy-1.8.4/include/common/hathreads.h   2018-02-08 
>> 13:05:15.0 +
>> +++ /tmp/hathreads.h2018-02-14 11:06:25.031422000 +
>> @@ -104,7 +104,7 @@ extern THREAD_LOCAL unsigned long tid_bi
>> /* 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)
>> 
>> 


> On 14 Feb 2018, at 14:13, David CARLIER  wrote:
> Whatever works best for you. Regards.


Well, I wonder if this is worth including into haproxy src?

Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[David CARLIER]

Whatever works best for you. Regards.

On 14 February 2018 at 11:09, Dmitry Sivachenko  wrote:

>
> > On 12 Feb 2018, at 17:37, David CARLIER  wrote:
> >
> > I think I m the one behing this relatively recent change ... why not
> adding in the condition the architecture ? e.g. !defined(__clang__) &&
> !defined(__i386__) ... something like this...
> >
> > Hope it is useful.
> >
>
>
> What about this change?
>
> --- work/haproxy-1.8.4/include/common/hathreads.h   2018-02-08
> 13:05:15.0 +
> +++ /tmp/hathreads.h2018-02-14 11:06:25.031422000 +
> @@ -104,7 +104,7 @@ extern THREAD_LOCAL unsigned long tid_bi
> /* 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)
>
>

Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[Dmitry Sivachenko]

> On 12 Feb 2018, at 17:37, David CARLIER  wrote:
> 
> I think I m the one behing this relatively recent change ... why not adding 
> in the condition the architecture ? e.g. !defined(__clang__) && 
> !defined(__i386__) ... something like this...
> 
> Hope it is useful.
> 


What about this change?

--- work/haproxy-1.8.4/include/common/hathreads.h   2018-02-08 
13:05:15.0 +
+++ /tmp/hathreads.h2018-02-14 11:06:25.031422000 +
@@ -104,7 +104,7 @@ extern THREAD_LOCAL unsigned long tid_bi
/* 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)

Fwd: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[David CARLIER]

-- Forwarded message --
From: David CARLIER <devne...@gmail.com>
Date: 12 February 2018 at 14:37
Subject: Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)
To: trtrmi...@gmail.com


I think I m the one behing this relatively recent change ... why not adding
in the condition the architecture ? e.g. !defined(__clang__) &&
!defined(__i386__) ... something like this...

Hope it is useful.

Regards.

Re: haproxy-1.8 build failure on FreeBSD/i386 (clang)

[Dmitry Sivachenko]

> On 11 Feb 2018, at 13:49, Franco Fichtner  wrote:
> 
> Hi,
> 
>> On 11. Feb 2018, at 7:05 AM, Dmitry Sivachenko  wrote:
>> 
>> src/proto_http.c:(.text+0x1209): undefined reference to 
>> `__atomic_fetch_add_8'
> 
> I believe this is a problem with older Clang versions not defining 8-bit
> operations like __atomic_fetch_add_8 on 32-bit.  This particularly affects
> FreeBSD 11.1 on i386 with LLVM 4.0.0.



I get the same error on FreeBSD-current/i386 (clang 5.0.1):

/usr/bin/ld: error: undefined symbol: __atomic_fetch_add_8
>>> referenced by src/proto_http.c
>>>   src/proto_http.o:(http_perform_server_redirect)

/usr/bin/ld: error: undefined symbol: __atomic_fetch_add_8
>>> referenced by src/proto_http.c
>>>   src/proto_http.o:(http_wait_for_request)

<...>

haproxy-1.8 build failure on FreeBSD/i386 (clang)

[Dmitry Sivachenko]

Hello,

haproxy-1.8 does not build on FreeBSD/i386 (clang):

src/proto_http.o: In function `http_perform_server_redirect':
src/proto_http.c:(.text+0x1209): undefined reference to `__atomic_fetch_add_8'
src/proto_http.o: In function `http_wait_for_request':
src/proto_http.c:(.text+0x275a): undefined reference to `__atomic_fetch_add_8'
src/proto_http.c:(.text+0x2e2c): undefined reference to `__atomic_fetch_add_8'
src/proto_http.c:(.text+0x2e48): undefined reference to `__atomic_fetch_add_8'
src/proto_http.c:(.text+0x30bb): undefined reference to `__atomic_fetch_add_8'
src/proto_http.o:src/proto_http.c:(.text+0x3184): more undefined references to 
`__atomic_fetch_add_8' follow
src/time.o: In function `tv_update_date':
src/time.c:(.text+0x631): undefined reference to `__atomic_compare_exchange_8'


In include/common/hathreads.h you have (line 107):
#if defined(__GNUC__) && (__GNUC__ < 4 || __GNUC__ == 4 && __GNUC_MINOR__ < 7) &
& !defined(__clang__)


Why do you exclude clang here?  If I remove !defined(__clang__), it builds fine 
but produces a number of similar warnings:


In file included from src/compression.c:29:
In file included from include/common/cfgparse.h:30:
include/proto/proxy.h:116:2: warning: variable '__new' is uninitialized when
  used within its own initialization [-Wuninitialized]
HA_ATOMIC_UPDATE_MAX(>fe_counters.cps_max,
^~
include/common/hathreads.h:172:55: note: expanded from macro
  'HA_ATOMIC_UPDATE_MAX'
while (__old < __new && !HA_ATOMIC_CAS(val, &__old, __new)); \
 ~~~^~
include/common/hathreads.h:128:26: note: expanded from macro 'HA_ATOMIC_CAS'
typeof((new)) __new = (new);   \
  ~^~~


What is the proper fix for that?  May be remove !defined(__clang__) ?

Thanks!