From owner-freebsd-current@freebsd.org Thu Jul 26 17:56:06 2018 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 948511055EF8 for ; Thu, 26 Jul 2018 17:56:06 +0000 (UTC) (envelope-from marklmi@yahoo.com) Received: from sonic307-9.consmr.mail.gq1.yahoo.com (sonic307-9.consmr.mail.gq1.yahoo.com [98.137.64.33]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 04C71723C2 for ; Thu, 26 Jul 2018 17:56:05 +0000 (UTC) (envelope-from marklmi@yahoo.com) X-YMail-OSG: EYrXbAwVM1lqtAcvRuNLwDjxagDAYFJ_Bokbud6J_4v8OSmQbvcB8g85pXEjF6v OjP6WQHr.7mwdq2_DE0fPu5AaypN3_NaN_6cqzUdvCXfydx6lRHS.xjZLUrUNvdbHi15l3gZfxLm b.J6KKEXIss_TCJHsf4JKnZWQovOsO24dx2m5B.q_8DUmwoWzCBslDUQqN2NplRxrhiMJVPERAtj kBRVLeXqZEHKCXE8w5cMO_z6tXJ9bD0SiuofZM.22HtzZrwB.y8Q4JKLwThz9fq4sRjEhJD7IwLw WkCVRobQagZOZN9yowVzIcDm5VGLLSOcB6QTYzM_Dqanm7qxN.6iKxUZU1Tfxb2PCXHnlaDPqkHL 6Lx3bbJhKpFYd36Ux1SgqplUdYlc3IBjDfn0wFgXS3CCwU2uvORZ68VaaLIknqDK6Vz5tfejlOYj FD0Qng0cBPOdXmPkCZOWzCe88_Pc2NepfWG61OX2VxmvcmLa3amwffDCFAMxxflL1NE7e60PHhqa cN_OGrB50cvTqmCPGjK8HEgvWwEn4lzqBpfpEjmbvDaA_WuoZ0._oYGEMLLagHoju3nb8mfi.LjD rOxyCWgONQ84QBF8UyyN9xlGK.VTybYIVqQmYr7VELagD2NRdzXhb2AyNqc7rNBkwA9nzg6e6G1F p.CzVDPbpNx1GxQX0H2eIj04VV8I0fPt0Z3MdqhuC9UZiUVfIxN1sVcB46ODaeZFJFPiwql.rL55 Nb3hphoMRuH4jDrEOQzjhDl_yIvPaV_O1E27IvbbM3paHKytLn__hH0QoEBuVTzy2ZHnTLY6hyR2 kUZaxLVn7cqpDcraOy519eu73vTG1r7RYggZ5AniAr8VOwpqRKZSy5sjmtK2aMSy9wMCOIXUATBx IT1Z_ANSMnd6Tm90sz51F8oqatgSh3oWjQYUzVPiGrkz7M1SpmHJAkXU7fcmdSurZyEn.uqTKqEt Q_Plg7szgFHumY1D71pxSrV1wbEBZVCZNlnjrhb_7OX36FpuvlcD3yqPt9g3w6IrfC1DDwdn69J_ _6oj9FLOaEylHC0udnQ-- Received: from sonic.gate.mail.ne1.yahoo.com by sonic307.consmr.mail.gq1.yahoo.com with HTTP; Thu, 26 Jul 2018 17:56:04 +0000 Received: from ip70-189-131-151.lv.lv.cox.net (EHLO [192.168.0.105]) ([70.189.131.151]) by smtp413.mail.gq1.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID f610466ce0fc186c1b733e448d8a127f; Thu, 26 Jul 2018 17:55:59 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: Re: head -r336568 and -r336570 appears to have made ci.freebsg.org's FreeBSD-head-amd64-gcc fail either than it had been (error: operand type 'struct *' is incompatible with argument 1 of '__atomic_fetch_add') From: Mark Millard In-Reply-To: <1ea2a8d0-b27a-503a-0a8b-48d7fbcd8fcb@FreeBSD.org> Date: Thu, 26 Jul 2018 10:55:58 -0700 Cc: Konstantin Belousov , FreeBSD Current Content-Transfer-Encoding: quoted-printable Message-Id: <0103123A-2D77-4D64-8FF6-97CD521CA7A8@yahoo.com> References: <95fdbf29-6c11-77a6-27a3-2d0dc30f1668@FreeBSD.org> <788B1EE7-EFC9-4AD4-9FD1-9876D0121189@yahoo.com> <9D40F38E-F1DC-4A3F-8792-09AD30D8802B@yahoo.com> <1ea2a8d0-b27a-503a-0a8b-48d7fbcd8fcb@FreeBSD.org> To: John Baldwin X-Mailer: Apple Mail (2.3445.9.1) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Jul 2018 17:56:06 -0000 On 2018-Jul-26, at 10:21 AM, John Baldwin wrote: > On 7/25/18 6:52 PM, Mark Millard wrote: >>=20 >>=20 >> On 2018-Jul-25, at 2:10 PM, Mark Millard = wrote: >>=20 >>=20 >>=20 >>> On 2018-Jul-25, at 10:09 AM, Mark Millard = wrote: >>>=20 >>>> On 2018-Jul-25, at 8:39 AM, John Baldwin = wrote: >>>>=20 >>>>> On 7/24/18 11:39 PM, Mark Millard wrote: >>>>>> On 2018-Jul-24, at 10:32 PM, Mark Millard = wrote: >>>>>>=20 >>>>>>> = https://ci.freebsd.org/job/FreeBSD-head-amd64-gcc/6597/consoleText >>>>>>> (head -r336573 after the prior 6596's -r336565 ): >>>>>>>=20 >>>>>>> --- all_subdir_lib/ofed --- >>>>>>> In file included from = /workspace/src/contrib/ofed/librdmacm/cma.h:43:0, >>>>>>> from /workspace/src/contrib/ofed/librdmacm/acm.c:42: >>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h: In function = 'fastlock_init': >>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h:60:2: error: invalid = initializer >>>>>>> atomic_store(&lock->cnt, 0); >>>>>>> ^ >>>>>>> In file included from = /workspace/src/contrib/ofed/librdmacm/acm.c:42:0: >>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h: In function = 'fastlock_acquire': >>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h:68:2: error: operand = type 'struct *' is incompatible with argument 1 of = '__atomic_fetch_add' >>>>>>> if (atomic_fetch_add(&lock->cnt, 1) > 0) >>>>>>> ^~ >>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h: In function = 'fastlock_release': >>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h:73:2: error: operand = type 'struct *' is incompatible with argument 1 of = '__atomic_fetch_sub' >>>>>>> if (atomic_fetch_sub(&lock->cnt, 1) > 1) >>>>>>> ^~ >>>>>>> . . . >>>>>>> --- all_subdir_lib/ofed --- >>>>>>> *** [acm.o] Error code 1 >>>>>>>=20 >>>>>>>=20 >>>>>>> = https://ci.freebsd.org/job/FreeBSD-head-amd64-gcc/6621/consoleText ( for >>>>>>> -r336700 ) still shows this type of error. >>>>>>=20 >>>>>>=20 >>>>>> [I should have a subject with "head -r336568 through -r336570 . . = .".] >>>>>>=20 >>>>>> =46rom what I can tell looking around having something like: >>>>>>=20 >>>>>> if (atomic_fetch_add(&lock->cnt, 1) > 0) >>>>>>=20 >>>>>> involve a __atomic_fetch_add indicates that: >>>>>>=20 >>>>>> = /usr/local/lib/gcc/x86_64-unknown-freebsd12.0/6.4.0/include/stdatomic.h >>>>>>=20 >>>>>> was in use instead of FreeBSD's stdatomic.h file. >>>>>>=20 >>>>>> If this is right, then the issue may be tied to head -r335782 >>>>>> implicitly changing the order of the include file directory >>>>>> searching for builds via the devel/*-gcc . >>>>>>=20 >>>>>> (I reverted -r335782 in my environment some time ago and have >>>>>> not run into this problem in my context so far.) >>>>>=20 >>>>> C11 atomics should work fine with compiler-provided headers since = they >>>>> are a part of the language (and the system stdatomic.h simply = attempts >>>>> to mimic the compiler-provided header in case it is missing). >>>>>=20 >>>>> Simple standalone tests of _Atomic(int) with GCC don't trigger = those >>>>> failures when using its stdatomic.h, so there is probably = something else >>>>> going on with kernel includes being used while building the = library, >>>>> etc. The last time we had this issue with stdarg.h it was because = a >>>>> header shared between the kernel and userland always used = '' >>>>> which is correct for the kernel but not for userland. >>>>=20 >>>> I did misread the headers. FreeBSD has the likes of: >>>>=20 >>>> #if defined(__CLANG_ATOMICS) >>>> . . . >>>> #define atomic_fetch_add_explicit(object, operand, order) = \ >>>> __c11_atomic_fetch_add(object, operand, order) >>>> . . . >>>> #elif defined(__GNUC_ATOMICS) >>>> . . . >>>> #define atomic_fetch_add_explicit(object, operand, order) = \ >>>> __atomic_fetch_add(&(object)->__val, operand, order) >>>> . . . >>>> #endif >>>> . . . >>>> #define atomic_fetch_add(object, operand) = \ >>>> atomic_fetch_add_explicit(object, operand, memory_order_seq_cst) >>>>=20 >>>> so __atomic_fetch_add would occur. >>>>=20 >>>> But so far I do not see the problem with -r335782 reverted. I last = built >>>> -r336693 last night via devel/amd64-gcc (via xtoolchain). >>>>=20 >>>> =46rom what I can tell FreeBSD defines: >>>>=20 >>>> #if !defined(__CLANG_ATOMICS) >>>> #define _Atomic(T) struct { volatile T = __val; } >>>> #endif >>>>=20 >>>> and that struct is being used in &(object)->__val is what the >>>> error reports are about. So that would be, for example, >>>>=20 >>>> &(&lock->cnt)->__val >>>>=20 >>>> This would appear to suggest that __val itself had a type meeting: >>>>=20 >>>> operand type struct >>>>=20 >>>> for T in _Atomic(T) . >>>>=20 >>>> (This is independent of just what the issue traces back to: just >>>> the net result on ci.freebsd.org . No claim that you are right >>>> or wrong here. I'll not be looking any more until this afternoon >>>> or night.) >>>=20 >>> Going in a somewhat different direction . . . >>>=20 >>> Looking around I found https://bugs.llvm.org/show_bug.cgi?id=3D26462 >>> which is titled: >>>=20 >>> 26462 =E2=80=93 GCC/clang C11 _Atomic incompatibility >>>=20 >>> It appears that the normal source of platform ABI definitions are >>> not explicit/detailed in the area and allow for incompatibilities >>> in this area. clang and gcc made differing choices absent being >>> constrained to match. >>>=20 >>> An example (a powerpc64 context was indicated): >>>=20 >>> struct A16 { char val[16]; };=20 >>> _Atomic struct A16 a16;=20 >>> // GCC: >>> _Static_assert(_Alignof(a16) =3D=3D 16, "");=20 >>> // Clang: >>> _Static_assert(_Alignof(a16) =3D=3D 1, "");=20 >>>=20 >>>=20 >>> Non-power-of-2 is a general problem >>> (not a powerpc64 context from what I can >>> tell): >>>=20 >>> struct A3 { char val[3]; }; >>> _Atomic struct A3 a3; >>> // GCC: >>> _Static_assert(sizeof(a3) =3D=3D 3, ""); >>> _Static_assert(_Alignof(a3) =3D=3D 1, ""); >>> // Clang: >>> _Static_assert(sizeof(a3) =3D=3D 4, ""); >>> _Static_assert(_Alignof(a3) =3D=3D 4, ""); >>>=20 >>>=20 >>>=20 >>> Comment 6 (by John McCall) is relevant: >>>=20 >>> QUOTE >>> Anyway, while I prefer the Clang rule, the GCC rule is defensible, = as are any number of other rules. The important point, however, is that = having this discussion is not the right approach to solving this = problem. The layout of _Atomic(T) is ABI. ABI rules are not generally = determined by compiler implementors making things up as they go along, = or at least they shouldn't be. The Darwin ABI for _Atomic is the rule = implemented in Clang, which we actually did think about carefully when = we adopted it. Other platforms need to make their own call, and it = probably shouldn't just be "whatever's implemented in GCC", especially = on other platforms where GCC is not the system compiler. >>> END QUOTE >>>=20 >>>=20 >>> (I do nto claim to have proivided all the material that should >>> be read in https://bugs.llvm.org/show_bug.cgi?id=3D26462 .) >>>=20 >>> It may be that FreeBSD needs to be the source of the ABI definitions >>> involved if clang and gcc freeBSD builds are to be interoperable in >>> this area. But this could mean avoiding builtins? >>>=20 >>> If any of this is inlined and so not behind a more stable interface, >>> it looks like clang and gcc can not be mixed for the same instances >>> of various _Atomic possibilities. >>>=20 >>>=20 >>=20 >>=20 >> Going back to the code being compiled and=20 >> confirming your note: >> ( /usr/src/contrib/ofed/librdmacm/cma.h ) >>=20 >> typedef struct { >> sem_t sem; >> _Atomic(int) cnt; >> } fastlock_t; >> . . . >> static inline void fastlock_acquire(fastlock_t *lock) >> { >> if (atomic_fetch_add(&lock->cnt, 1) > 0) >> sem_wait(&lock->sem); >> } >>=20 >> So lock->cnt is an _Atomic(int) , i.e., >>=20 >> struct { volatile int __val; } >>=20 >> so overall: >>=20 >> typedef struct { >> sem_t sem; >> struct { volatile int __val; } cnt; >> } fastlock_t; >>=20 >>=20 >> for which: atomic_fetch_add(&lock->cnt, 1) has >> for A filled-in in the C11 language official: >>=20 >> atomic_fetch_add (volatile A* obj, M arg) >>=20 >> (generic function) being: >>=20 >> atomic_fetch_add (volatile struct { volatile int __val; }* obj, M = arg) >>=20 >> and a direct type-check of that notation for obj would find: >>=20 >> operand type 'struct *' >>=20 >> and that would propagate to GCC's translation to __atomic_fetch_add >> via gcc's stdatomic.h : >>=20 >> #define atomic_fetch_add(PTR, VAL) __atomic_fetch_add ((PTR), (VAL), = \ >> __ATOMIC_SEQ_CST) >>=20 >> So, yes, it looks like the: >>=20 >> #if !defined(__CLANG_ATOMICS) >> #define _Atomic(T) struct { volatile T = __val; } >> #endif >=20 > So where are you seeing this btw? In head the conditional is = different: >=20 > #if !defined(__cplusplus) && !__has_extension(c_atomic) && \ > !__has_extension(cxx_atomic) > /* > * No native support for _Atomic(). Place object in structure to = prevent > * most forms of direct non-atomic access. > */ > #define _Atomic(T) struct { T volatile __val; } > #endif >=20 > And external GCC does support those extensions so should end up not = defining > an _Atomic macro, and in fact when I use -E with external GCC it = leaves > _Atomic(int) alone: >=20 > % x86_64-unknown-freebsd11.2-gcc -E bar.c > ... > # 1 "bar.c" > # 1 "/usr/include/sys/cdefs.h" 1 3 4 > # 2 "bar.c" 2 > # 1 = "/usr/local/lib/gcc/x86_64-unknown-freebsd11.2/6.4.0/include/stdatomic.h" = 1 3 4 > # 29 = "/usr/local/lib/gcc/x86_64-unknown-freebsd11.2/6.4.0/include/stdatomic.h" = 3 4 >=20 > # 29 = "/usr/local/lib/gcc/x86_64-unknown-freebsd11.2/6.4.0/include/stdatomic.h" = 3 4 > typedef enum > { > memory_order_relaxed =3D 0, > memory_order_consume =3D 1, > memory_order_acquire =3D 2, > memory_order_release =3D 3, > memory_order_acq_rel =3D 4, > memory_order_seq_cst =3D 5 > } memory_order; >=20 >=20 > typedef _Atomic _Bool atomic_bool; > typedef _Atomic char atomic_char; > ... >=20 > So cdefs.h isn't overriding _Atomic and should be using the = CLANG_ATOMICS case > for external GCC. Some of my research was when I did not have FreeBSD available and was = over the web and I did not cross check all of it later. I apparently looked at some older source at some point. I now see what you report for the #if test. Sorry for the confusion. However, there is in /usr/src/sys/sys/cdefs.h : /* * Testing against Clang-specific extensions. */ #ifndef __has_attribute #define __has_attribute(x) 0 #endif #ifndef __has_extension #define __has_extension __has_feature #endif #ifndef __has_feature #define __has_feature(x) 0 #endif so if those are clang specific then: #if !defined(__cplusplus) && !__has_extension(c_atomic) && \ !__has_extension(cxx_atomic) will test for C code: !0 && !0 && !0 and then the code will select to: #define _Atomic(T) struct { T volatile __val; } https://clang.llvm.org/docs/LanguageExtensions.html (for clang 7) lists: QUOTE __has_feature and __has_extension These function-like macros take a single identifier argument that is the = name of a feature. __has_feature evaluates to 1 if the feature is both = supported by Clang and standardized in the current language standard or = 0 if not (but see below), while __has_extension evaluates to 1 if the = feature is supported by Clang in the current language (either as a = language extension or a standard language feature) or 0 if not. They can = be used like this: #ifndef __has_feature // Optional of course. =20 #define __has_feature(x) 0 // Compatibility with non-clang compilers. #endif #ifndef __has_extension =20 #define __has_extension __has_feature // Compatibility with pre-3.0 = compilers. #endif END QUOTE so I think that: #define _Atomic(T) struct { T volatile __val; } is being done. =3D=3D=3D Mark Millard marklmi at yahoo.com ( dsl-only.net went away in early 2018-Mar)