| Message ID | 20220808150641.19475-1-nicolas.cavallari@green-communications.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] package/libgit2: depends on native 64bit atomics | expand |
Hello Nicolas, On Mon, 8 Aug 2022 17:06:40 +0200 Nicolas Cavallari <nicolas.cavallari@green-communications.fr> wrote: > libgit2 does not know about libatomic_ops. > > This wasn't a problem before because it is a shared library, so the > missing symbols were simply added as undefined, and no project currently > depend on libgit2 to expose the problem. > > The next version of libgit2 can also build a binary, which will expose > the problem. > > Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr> Which build failure do you have exactly? Indeed when building for SPARC, what I see are calls to __atomic_compare_exchange_4, __atomic_fetch_add_4, __atomic_fetch_sub_4. Is this what you are referring to? If yes, then these are not sync builtins (https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html), but atomic builtins (https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html). The Buildroot options BR2_TOOLCHAIN_HAS_SYNC_xyz are for sync builtins. The Buildroot option BR2_TOOLCHAIN_HAS_ATOMIC is for atomic builtins. So, your package needs to: depends on BR2_TOOLCHAIN_HAS_ATOMIC and then in its .mk file, so something like this: ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y) ... make sure your package links against libatomic.so by passing -latomic in LDFLAGS endif Note that libatomic.so is distinct from libatomic_ops.so. libatomic.so is provided together with the gcc runtime, and implements the atomic builtins on architecture where they are not implemented directly by the compiler (a runtime library is needed). Best regards, Thomas
On 08/08/2022 22:41, Thomas Petazzoni wrote: > Hello Nicolas, > > On Mon, 8 Aug 2022 17:06:40 +0200 > Nicolas Cavallari <nicolas.cavallari@green-communications.fr> wrote: > >> libgit2 does not know about libatomic_ops. >> >> This wasn't a problem before because it is a shared library, so the >> missing symbols were simply added as undefined, and no project currently >> depend on libgit2 to expose the problem. >> >> The next version of libgit2 can also build a binary, which will expose >> the problem. >> >> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr> > > Which build failure do you have exactly? Indeed when building for > SPARC, what I see are calls to __atomic_compare_exchange_4, > __atomic_fetch_add_4, __atomic_fetch_sub_4. > > Is this what you are referring to? > > If yes, then these are not sync builtins > (https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html), but > atomic builtins > (https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html). > > The Buildroot options BR2_TOOLCHAIN_HAS_SYNC_xyz are for sync builtins. > > The Buildroot option BR2_TOOLCHAIN_HAS_ATOMIC is for atomic builtins. > > So, your package needs to: > > depends on BR2_TOOLCHAIN_HAS_ATOMIC > > and then in its .mk file, so something like this: > > ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y) > ... make sure your package links against libatomic.so by passing -latomic in LDFLAGS > endif > > Note that libatomic.so is distinct from libatomic_ops.so. libatomic.so > is provided together with the gcc runtime, and implements the atomic > builtins on architecture where they are not implemented directly by the > compiler (a runtime library is needed). It's a bit more complicated than that and i probably mixed things up. libgit2 can use sync builtins or atomic builtins depending on the gcc version. But if we assume that the new gcc minimum version is 4.9, then I can probably just ignore the sync builtins.
Hello Nicolas, On Tue, 9 Aug 2022 17:57:16 +0200 Nicolas Cavallari <nicolas.cavallari@green-communications.fr> wrote: > It's a bit more complicated than that and i probably mixed things up. > > libgit2 can use sync builtins or atomic builtins depending on the gcc > version. > > But if we assume that the new gcc minimum version is 4.9, then I can > probably just ignore the sync builtins. Looking at src/thread.h in libgit2, we can see: # if (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 1)) # error Atomic primitives do not exist on this version of gcc; configure libgit2 with -DUSE_THREADS=OFF # elif (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7)) # define GIT_BUILTIN_ATOMIC # else # define GIT_BUILTIN_SYNC # endif So basically, with gcc >= 4.7, libgit2 uses atomic built-ins, with gcc < 4.7, it uses sync built-ins. However neither are guaranteed to be present on all architectures. So if we want to be fully correct, we would probably need something like this: depends on (BR2_TOOLCHAIN_HAS_ATOMIC && BR2_TOOLCHAIN_GCC_AT_LEAST_4_7) || \ (BR2_TOOLCHAIN_HAS_SYNC_4 && BR2_TOOLCHAIN_HAS_SYNC_8 && !BR2_TOOLCHAIN_GCC_AT_LEAST_4_7) Best regards, Thomas
diff --git a/package/libgit2/Config.in b/package/libgit2/Config.in index a6a9728ff0..9c97676c4a 100644 --- a/package/libgit2/Config.in +++ b/package/libgit2/Config.in @@ -1,6 +1,8 @@ config BR2_PACKAGE_LIBGIT2 bool "libgit2" depends on !BR2_STATIC_LIBS # libhttpparser + depends on BR2_TOOLCHAIN_HAS_SYNC_8 + depends on BR2_TOOLCHAIN_HAS_SYNC_4 select BR2_PACKAGE_LIBHTTPPARSER select BR2_PACKAGE_ZLIB help
libgit2 does not know about libatomic_ops. This wasn't a problem before because it is a shared library, so the missing symbols were simply added as undefined, and no project currently depend on libgit2 to expose the problem. The next version of libgit2 can also build a binary, which will expose the problem. Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr> --- package/libgit2/Config.in | 2 ++ 1 file changed, 2 insertions(+)