[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Carlos Santos]

From: Henrique Marks <henrique.marks@datacom.ind.br>

Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 package/protobuf/0001-PowerPC-Support.patch | 54 +++++++++++++++++++++++++++++
 package/protobuf/Config.in                  |  5 ++-
 2 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 package/protobuf/0001-PowerPC-Support.patch

diff --git a/package/protobuf/0001-PowerPC-Support.patch b/package/protobuf/0001-PowerPC-Support.patch
new file mode 100644
index 0000000..aee3717
--- /dev/null
+++ b/package/protobuf/0001-PowerPC-Support.patch
@@ -0,0 +1,54 @@
+From d56c6b19b18dc459c1ea6b720ef015afe72757ea Mon Sep 17 00:00:00 2001
+From: Henrique Marks <henrique.marks@datacom.ind.br>
+Date: Fri, 28 Aug 2015 18:55:49 -0300
+Subject: [PATCH 1/1] Syntax Error Patch
+
+Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
+---
+ src/google/protobuf/stubs/atomicops.h | 10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/src/google/protobuf/stubs/atomicops.h b/src/google/protobuf/stubs/atomicops.h
+index b1336e3..a130b38 100644
+--- a/src/google/protobuf/stubs/atomicops.h
++++ b/src/google/protobuf/stubs/atomicops.h
+@@ -162,7 +162,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ 
+ // Include our platform specific implementation.
+ #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
+-#error "Atomic operations are not supported on your platform"
++"Atomic operations are not supported on your platform"
+ 
+ // ThreadSanitizer, http://clang.llvm.org/docs/ThreadSanitizer.html.
+ #if defined(THREAD_SANITIZER)
+@@ -172,7 +172,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ #if defined(GOOGLE_PROTOBUF_ARCH_IA32) || defined(GOOGLE_PROTOBUF_ARCH_X64)
+ #include <google/protobuf/stubs/atomicops_internals_x86_msvc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Solaris
+@@ -203,15 +203,15 @@ GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #if __has_extension(c_atomic)
+ #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Unknown.
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // On some platforms we need additional declarations to make AtomicWord
+-- 
+1.9.1
+
diff --git a/package/protobuf/Config.in b/package/protobuf/Config.in
index 9ee7e7d..3899ac1 100644
--- a/package/protobuf/Config.in
+++ b/package/protobuf/Config.in
@@ -3,8 +3,7 @@ config BR2_PACKAGE_PROTOBUF
 	depends on BR2_INSTALL_LIBSTDCPP
 	depends on BR2_TOOLCHAIN_HAS_THREADS
 	# See src/google/protobuf/stubs/platform_macros.h for supported archs.
-	# PowerPC doesn't actually work.
-	depends on BR2_arm || BR2_i386 || BR2_mipsel || BR2_x86_64
+	depends on BR2_arm || BR2_i386 || BR2_mipsel || BR2_x86_64 || BR2_powerpc
 	# host-protobuf only builds on certain architectures
 	depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
 	depends on !BR2_STATIC_LIBS
@@ -17,5 +16,5 @@ config BR2_PACKAGE_PROTOBUF
 comment "protobuf needs a toolchain w/ C++, threads, dynamic library"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS \
 		|| BR2_STATIC_LIBS
-	depends on BR2_arm || BR2_i386 || BR2_mipsel || BR2_x86_64
+	depends on BR2_arm || BR2_i386 || BR2_mipsel || BR2_x86_64 || BR2_powerpc
 	depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
-- 
2.5.0

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Thomas Petazzoni]

Carlos,

On Thu, 28 Jan 2016 11:08:35 -0200, Carlos Santos wrote:
> From: Henrique Marks <henrique.marks@datacom.ind.br>
> 
> Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
>  package/protobuf/0001-PowerPC-Support.patch | 54 +++++++++++++++++++++++++++++
>  package/protobuf/Config.in                  |  5 ++-
>  2 files changed, 56 insertions(+), 3 deletions(-)
>  create mode 100644 package/protobuf/0001-PowerPC-Support.patch

This patch doesn't actually work. First there are a number of problems:

 - The patch you apply has technically nothing to do with enabling the
   PowerPC architecture. It seems more related to supporting old
   compilers.

 - The patch you apply is already applied upstream, so in this case, we
   prefer to use the upstream patch directly.

 - You change the architecture dependencies in protobuf/Config.in, but
   forget to propagate this change to the reverse dependencies of
   protobuf, namely the mosh and ola packages. To make this easier,
   I've changed protobuf/Config.in to provide a
   BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS variable, and changed mosh and
   ola to use it. See commit
   https://git.busybox.net/buildroot/commit/?id=abdc56006bf253bec393066f96f69f0a6246b896.

But then, despite those issues, your patch still doesn't build on
PowerPC with the following defconfig for example:

BR2_powerpc=y
BR2_powerpc_8548=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_OLA=y
BR2_PACKAGE_MOSH=y
# BR2_TARGET_ROOTFS_TAR is not set

It fails with:

In file included from ./google/protobuf/stubs/once.h:81:0,
                 from google/protobuf/stubs/common.cc:34:
./google/protobuf/stubs/atomicops.h:209:2: error: #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
In file included from ./google/protobuf/stubs/once.h:81:0,
                 from google/protobuf/stubs/once.cc:38:
./google/protobuf/stubs/atomicops.h:209:2: error: #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
In file included from google/protobuf/stubs/atomicops_internals_x86_msvc.cc:37:0:
./google/protobuf/stubs/atomicops.h:209:2: error: #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
In file included from google/protobuf/stubs/atomicops_internals_x86_gcc.cc:36:0:
./google/protobuf/stubs/atomicops.h:209:2: error: #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR

Looking at atomicops.h, I can read:

#elif defined(__GNUC__)
#if defined(GOOGLE_PROTOBUF_ARCH_IA32) || defined(GOOGLE_PROTOBUF_ARCH_X64)
#include <google/protobuf/stubs/atomicops_internals_x86_gcc.h>
#elif defined(GOOGLE_PROTOBUF_ARCH_ARM) && defined(__linux__)
#include <google/protobuf/stubs/atomicops_internals_arm_gcc.h>
#elif defined(GOOGLE_PROTOBUF_ARCH_AARCH64)
#include <google/protobuf/stubs/atomicops_internals_arm64_gcc.h>
#elif defined(GOOGLE_PROTOBUF_ARCH_ARM_QNX)
#include <google/protobuf/stubs/atomicops_internals_arm_qnx.h>
#elif defined(GOOGLE_PROTOBUF_ARCH_MIPS) || defined(GOOGLE_PROTOBUF_ARCH_MIPS64)
#include <google/protobuf/stubs/atomicops_internals_mips_gcc.h>
#elif defined(__native_client__)
#include <google/protobuf/stubs/atomicops_internals_pnacl.h>
#elif (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) || (__GNUC__ > 4))
#include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
#elif defined(__clang__)
#if __has_extension(c_atomic)
#include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
#else
#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
#endif
#else
#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
#endif

So it means that on i386, on x86-64, on ARM and on MIPS and MIPS64,
there are built-in implementation for the atomic operations.

For all other architectures, it relies on
atomicops_internals_generic_gcc.h, only when gcc >= 4.7. And indeed my
tested toolchain only has gcc 4.5.

In fact atomicops_internals_generic_gcc.h uses the __atomic_*()
built-ins of the compiler, which indeed are only introduced in gcc 4.7.
But on some architectures, they require linking with -latomic. See my
atomic patch series at
http://lists.busybox.net/pipermail/buildroot/2016-February/151672.html,
and especially patch
http://lists.busybox.net/pipermail/buildroot/2016-February/151674.html
which has all the gory details.

And in fact, with those atomics, not only PowerPC can be supported, but
any other architecture (except if protobuf has other
architecture-specific dependencies elsewhere).

So, once my atomic patch series is merged, we could do:

config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
        bool
        default y if BR2_arm
        default y if BR2_i386
        default y if BR2_mipsel
        default y if BR2_x86_64
	default y if BR2_TOOLCHAIN_HAS_ATOMIC
        depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"

*and* ensure protobuf gets linked with -latomic.

What do you think ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Henrique Marks]

Hello

I agree with you, but let me say that the original patch just corrects a "syntax error" in the code !

+ #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
+-#error "Atomic operations are not supported on your platform"
++"Atomic operations are not supported on your platform"

The macro GOOGLE_PROTOBUF_ATOMICOPS_ERROR expands to "another" pre-processor directive (#error "Message"). So if the Macro gets expanded, it will generate a strange message "error: #error ..."

This syntax error correction was submitted upstream, but wasnt applied to protobuf 2.6.1, just to protobuf 3x series. As we cannot drive the push to protobuf 3.x series right now (but we can in the near future, as soon as the first 3.x appears), we submitted a patch to buildroot.

Despite of this, the solution using the atomic patch you sent seems ok. We are using the patch we submmited for six months now, internally, and we try to send upstream everything, as soon as possible, so that we can keep up with the upstream tree. When your solution goes in, we can remove our internal patch (it is submmited in protobuf upstream 3.x anyway).

Thanks for the analysis


----- Mensagem original -----
> De: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> Para: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Enviadas: Quinta-feira, 4 de fevereiro de 2016 21:06:13
> Assunto: Re: [Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

> Carlos,
> 
> On Thu, 28 Jan 2016 11:08:35 -0200, Carlos Santos wrote:
>> From: Henrique Marks <henrique.marks@datacom.ind.br>
>> 
>> Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
>> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
>> ---
>>  package/protobuf/0001-PowerPC-Support.patch | 54 +++++++++++++++++++++++++++++
>>  package/protobuf/Config.in                  |  5 ++-
>>  2 files changed, 56 insertions(+), 3 deletions(-)
>>  create mode 100644 package/protobuf/0001-PowerPC-Support.patch
> 
> This patch doesn't actually work. First there are a number of problems:
> 
> - The patch you apply has technically nothing to do with enabling the
>   PowerPC architecture. It seems more related to supporting old
>   compilers.
> 
> - The patch you apply is already applied upstream, so in this case, we
>   prefer to use the upstream patch directly.
> 
> - You change the architecture dependencies in protobuf/Config.in, but
>   forget to propagate this change to the reverse dependencies of
>   protobuf, namely the mosh and ola packages. To make this easier,
>   I've changed protobuf/Config.in to provide a
>   BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS variable, and changed mosh and
>   ola to use it. See commit
>   https://git.busybox.net/buildroot/commit/?id=abdc56006bf253bec393066f96f69f0a6246b896.
> 
> But then, despite those issues, your patch still doesn't build on
> PowerPC with the following defconfig for example:
> 
> BR2_powerpc=y
> BR2_powerpc_8548=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_INIT_NONE=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> BR2_PACKAGE_OLA=y
> BR2_PACKAGE_MOSH=y
> # BR2_TARGET_ROOTFS_TAR is not set
> 
> It fails with:
> 
> In file included from ./google/protobuf/stubs/once.h:81:0,
>                 from google/protobuf/stubs/common.cc:34:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> In file included from ./google/protobuf/stubs/once.h:81:0,
>                 from google/protobuf/stubs/once.cc:38:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> In file included from
> google/protobuf/stubs/atomicops_internals_x86_msvc.cc:37:0:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> In file included from google/protobuf/stubs/atomicops_internals_x86_gcc.cc:36:0:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> 
> Looking at atomicops.h, I can read:
> 
> #elif defined(__GNUC__)
> #if defined(GOOGLE_PROTOBUF_ARCH_IA32) || defined(GOOGLE_PROTOBUF_ARCH_X64)
> #include <google/protobuf/stubs/atomicops_internals_x86_gcc.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_ARM) && defined(__linux__)
> #include <google/protobuf/stubs/atomicops_internals_arm_gcc.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_AARCH64)
> #include <google/protobuf/stubs/atomicops_internals_arm64_gcc.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_ARM_QNX)
> #include <google/protobuf/stubs/atomicops_internals_arm_qnx.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_MIPS) || defined(GOOGLE_PROTOBUF_ARCH_MIPS64)
> #include <google/protobuf/stubs/atomicops_internals_mips_gcc.h>
> #elif defined(__native_client__)
> #include <google/protobuf/stubs/atomicops_internals_pnacl.h>
> #elif (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) || (__GNUC__ > 4))
> #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
> #elif defined(__clang__)
> #if __has_extension(c_atomic)
> #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
> #else
> #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> #endif
> #else
> #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> #endif
> 
> So it means that on i386, on x86-64, on ARM and on MIPS and MIPS64,
> there are built-in implementation for the atomic operations.
> 
> For all other architectures, it relies on
> atomicops_internals_generic_gcc.h, only when gcc >= 4.7. And indeed my
> tested toolchain only has gcc 4.5.
> 
> In fact atomicops_internals_generic_gcc.h uses the __atomic_*()
> built-ins of the compiler, which indeed are only introduced in gcc 4.7.
> But on some architectures, they require linking with -latomic. See my
> atomic patch series at
> http://lists.busybox.net/pipermail/buildroot/2016-February/151672.html,
> and especially patch
> http://lists.busybox.net/pipermail/buildroot/2016-February/151674.html
> which has all the gory details.
> 
> And in fact, with those atomics, not only PowerPC can be supported, but
> any other architecture (except if protobuf has other
> architecture-specific dependencies elsewhere).
> 
> So, once my atomic patch series is merged, we could do:
> 
> config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
>        bool
>        default y if BR2_arm
>        default y if BR2_i386
>        default y if BR2_mipsel
>        default y if BR2_x86_64
>	default y if BR2_TOOLCHAIN_HAS_ATOMIC
>        depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
> 
> *and* ensure protobuf gets linked with -latomic.
> 
> What do you think ?
> 
> Best regards,
> 
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
Dr. Henrique Marks
henrique.marks at datacom.ind.br
R. Am?rica, 1000 - Eldorado do Sul - RS
CEP: 92990-000 - Brasil
Fone: +55 51 3933 3000 - Ramal 3466

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Thomas Petazzoni]

Hello Henrique,

On Fri, 5 Feb 2016 09:04:53 -0200 (BRST), Henrique Marks wrote:

> I agree with you, but let me say that the original patch just corrects a "syntax error" in the code !
> 
> + #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
> +-#error "Atomic operations are not supported on your platform"
> ++"Atomic operations are not supported on your platform"

Correct. In fact I thought it was only with recent compilers, but it is
probably incorrect regardless of the gcc version, and it doesn't fail
in all cases because we ensure that protobuf is only built on
architecture on which protobuf has built-in support for atomic
operations. And therefore you don't fall into the #else cases where
this bogus error macro is used.

> This syntax error correction was submitted upstream, but wasnt
> applied to protobuf 2.6.1, just to protobuf 3x series. As we cannot
> drive the push to protobuf 3.x series right now (but we can in the
> near future, as soon as the first 3.x appears), we submitted a patch
> to buildroot.

Sure. But in this case, we prefer the patch to be a backport from
upstream, so that it is clearer when bumping that the patch can be
dropped.

And also, when a patch has been accepted by upstream, we have a higher
confidence that the patch is correct.

> Despite of this, the solution using the atomic patch you sent seems
> ok. We are using the patch we submmited for six months now,
> internally, and we try to send upstream everything, as soon as
> possible, so that we can keep up with the upstream tree. When your
> solution goes in, we can remove our internal patch (it is submmited
> in protobuf upstream 3.x anyway).

OK. Could you work on a proper patch series on the atomic series is
merged in master ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Henrique Marks]

Yes, once the atomic series enter master branch, we are going to proceed on with this patch:

- Change protobuf, as you stated.
- Change Dependent Packages, it is four or five last time i checked out.
- Build on powerpc these packages, with gcc > 4.8

I guess this is enough.

Thanks



----- Mensagem original -----
> De: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> Para: "DATACOM" <henrique.marks@datacom.ind.br>
> Cc: "Carlos Santos" <casantos@datacom.ind.br>, buildroot at buildroot.org
> Enviadas: Sexta-feira, 5 de fevereiro de 2016 11:09:09
> Assunto: Re: [Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

> Hello Henrique,
> 
> On Fri, 5 Feb 2016 09:04:53 -0200 (BRST), Henrique Marks wrote:
> 
>> I agree with you, but let me say that the original patch just corrects a "syntax
>> error" in the code !
>> 
>> + #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
>> +-#error "Atomic operations are not supported on your platform"
>> ++"Atomic operations are not supported on your platform"
> 
> Correct. In fact I thought it was only with recent compilers, but it is
> probably incorrect regardless of the gcc version, and it doesn't fail
> in all cases because we ensure that protobuf is only built on
> architecture on which protobuf has built-in support for atomic
> operations. And therefore you don't fall into the #else cases where
> this bogus error macro is used.
> 
>> This syntax error correction was submitted upstream, but wasnt
>> applied to protobuf 2.6.1, just to protobuf 3x series. As we cannot
>> drive the push to protobuf 3.x series right now (but we can in the
>> near future, as soon as the first 3.x appears), we submitted a patch
>> to buildroot.
> 
> Sure. But in this case, we prefer the patch to be a backport from
> upstream, so that it is clearer when bumping that the patch can be
> dropped.
> 
> And also, when a patch has been accepted by upstream, we have a higher
> confidence that the patch is correct.
> 
>> Despite of this, the solution using the atomic patch you sent seems
>> ok. We are using the patch we submmited for six months now,
>> internally, and we try to send upstream everything, as soon as
>> possible, so that we can keep up with the upstream tree. When your
>> solution goes in, we can remove our internal patch (it is submmited
>> in protobuf upstream 3.x anyway).
> 
> OK. Could you work on a proper patch series on the atomic series is
> merged in master ?
> 
> Thanks!
> 
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

-- 
Dr. Henrique Marks
henrique.marks at datacom.ind.br
R. Am?rica, 1000 - Eldorado do Sul - RS
CEP: 92990-000 - Brasil
Fone: +55 51 3933 3000 - Ramal 3466

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Thomas Petazzoni]

Hello,

(Please try to avoid top-posting, this is considered bad practice on
most mailing list)

On Fri, 5 Feb 2016 11:22:58 -0200 (BRST), Henrique Marks wrote:
> Yes, once the atomic series enter master branch, we are going to proceed on with this patch:
> 
> - Change protobuf, as you stated.
> - Change Dependent Packages, it is four or five last time i checked out.
> - Build on powerpc these packages, with gcc > 4.8

Sounds good.

There is only one gotcha/limitation introduced by the atomic series:
the fact that building protobuf with gcc 4.7 will not be allowed,
while in fact it seems to be possible.

On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
types are available built-in, without the need for libatomic. This
means that the __atomic_*() built-ins for those sizes are available in
gcc 4.7.

However, the __atomic_*() built-ins for 8-byte types is implemented via
libatomic, so only available since gcc 4.8.

In Buildroot, to simplify things, we've decided to simply require gcc
4.8 as soon as the architectures has at least one __atomic_*() built-in
variant that requires libatomic.

But in fact, protobuf most likely only uses the 1, 2 and 4-byte
variants, so it *could* technically build with gcc 4.7.

But oh, well, it's probably not a big deal, and we can live with
requiring gcc 4.8 on PowerPC to build protobuf. Is that OK for you?

If we want to do a more fine-grained selection, we would have to
introduce multiple BR2_TOOLCHAIN_HAS_ATOMIC_<x> options, like I've done
for the __sync_*() built-ins. It's possible, but a big annoying
especially since gcc 4.8 has everything needed.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Thomas Petazzoni]

Dear Henrique Marks,

On Fri, 5 Feb 2016 11:22:58 -0200 (BRST), Henrique Marks wrote:
> Yes, once the atomic series enter master branch, we are going to proceed on with this patch:
> 
> - Change protobuf, as you stated.
> - Change Dependent Packages, it is four or five last time i checked out.
> - Build on powerpc these packages, with gcc > 4.8

The atomic series has been merged in master. Can you respin this
protobuf patch, taking into account the comments I made?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Carlos Santos]

----- Original Message -----
> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Sent: Thursday, February 4, 2016 9:06:13 PM
> Subject: Re: [Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

> Carlos,
> 
> On Thu, 28 Jan 2016 11:08:35 -0200, Carlos Santos wrote:
>> From: Henrique Marks <henrique.marks@datacom.ind.br>
>> 
>> Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
>> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
>> ---
>>  package/protobuf/0001-PowerPC-Support.patch | 54 +++++++++++++++++++++++++++++
>>  package/protobuf/Config.in                  |  5 ++-
>>  2 files changed, 56 insertions(+), 3 deletions(-)
>>  create mode 100644 package/protobuf/0001-PowerPC-Support.patch
> 
> This patch doesn't actually work. First there are a number of problems:
> 
> - The patch you apply has technically nothing to do with enabling the
>   PowerPC architecture. It seems more related to supporting old
>   compilers.
> 
> - The patch you apply is already applied upstream, so in this case, we
>   prefer to use the upstream patch directly.
> 
> - You change the architecture dependencies in protobuf/Config.in, but
>   forget to propagate this change to the reverse dependencies of
>   protobuf, namely the mosh and ola packages. To make this easier,
>   I've changed protobuf/Config.in to provide a
>   BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS variable, and changed mosh and
>   ola to use it. See commit
>   https://git.busybox.net/buildroot/commit/?id=abdc56006bf253bec393066f96f69f0a6246b896.
> 
> But then, despite those issues, your patch still doesn't build on
> PowerPC with the following defconfig for example:
> 
> BR2_powerpc=y
> BR2_powerpc_8548=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_INIT_NONE=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> BR2_PACKAGE_OLA=y
> BR2_PACKAGE_MOSH=y
> # BR2_TARGET_ROOTFS_TAR is not set
> 
> It fails with:
> 
> In file included from ./google/protobuf/stubs/once.h:81:0,
>                 from google/protobuf/stubs/common.cc:34:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> In file included from ./google/protobuf/stubs/once.h:81:0,
>                 from google/protobuf/stubs/once.cc:38:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> In file included from
> google/protobuf/stubs/atomicops_internals_x86_msvc.cc:37:0:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> In file included from google/protobuf/stubs/atomicops_internals_x86_gcc.cc:36:0:
> ./google/protobuf/stubs/atomicops.h:209:2: error: #error
> GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> 
> Looking at atomicops.h, I can read:
> 
> #elif defined(__GNUC__)
> #if defined(GOOGLE_PROTOBUF_ARCH_IA32) || defined(GOOGLE_PROTOBUF_ARCH_X64)
> #include <google/protobuf/stubs/atomicops_internals_x86_gcc.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_ARM) && defined(__linux__)
> #include <google/protobuf/stubs/atomicops_internals_arm_gcc.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_AARCH64)
> #include <google/protobuf/stubs/atomicops_internals_arm64_gcc.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_ARM_QNX)
> #include <google/protobuf/stubs/atomicops_internals_arm_qnx.h>
> #elif defined(GOOGLE_PROTOBUF_ARCH_MIPS) || defined(GOOGLE_PROTOBUF_ARCH_MIPS64)
> #include <google/protobuf/stubs/atomicops_internals_mips_gcc.h>
> #elif defined(__native_client__)
> #include <google/protobuf/stubs/atomicops_internals_pnacl.h>
> #elif (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) || (__GNUC__ > 4))
> #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
> #elif defined(__clang__)
> #if __has_extension(c_atomic)
> #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
> #else
> #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> #endif
> #else
> #error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
> #endif
> 
> So it means that on i386, on x86-64, on ARM and on MIPS and MIPS64,
> there are built-in implementation for the atomic operations.
> 
> For all other architectures, it relies on
> atomicops_internals_generic_gcc.h, only when gcc >= 4.7. And indeed my
> tested toolchain only has gcc 4.5.
> 
> In fact atomicops_internals_generic_gcc.h uses the __atomic_*()
> built-ins of the compiler, which indeed are only introduced in gcc 4.7.
> But on some architectures, they require linking with -latomic. See my
> atomic patch series at
> http://lists.busybox.net/pipermail/buildroot/2016-February/151672.html,
> and especially patch
> http://lists.busybox.net/pipermail/buildroot/2016-February/151674.html
> which has all the gory details.
> 
> And in fact, with those atomics, not only PowerPC can be supported, but
> any other architecture (except if protobuf has other
> architecture-specific dependencies elsewhere).
> 
> So, once my atomic patch series is merged, we could do:
> 
> config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
>        bool
>        default y if BR2_arm
>        default y if BR2_i386
>        default y if BR2_mipsel
>        default y if BR2_x86_64

These architecture checks would be superseded by the BR2_TOOLCHAIN_HAS_ATOMIC boolean.

>	default y if BR2_TOOLCHAIN_HAS_ATOMIC
>        depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
> 
> *and* ensure protobuf gets linked with -latomic.

Hum, this requires a patch on configure.ac that that would hardly be accepted upstream.

> What do you think ?

Ok, I will send a new patch.

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

[Carlos Santos]

- Use the recently introduced BR2_TOOLCHAIN_HAS_ATOMIC boolean.

- Import an upstream patch to fix error handling when atomic operations
  are not detected. Without this patch the build fails due to a syntax
  error instead of showing the proper message.

- Add a patch to configure.ac to check if libatomic is needed and force
  linking to it (this patch would unlikely be accepted upstream).

On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
types are available built-in. However, the __atomic_*() built-ins for
8-byte types is implemented via libatomic, so only available since gcc
4.8.

In Buildroot, to simplify things, we've decided to simply require gcc 4.8
as soon as the architectures has at least one __atomic_*() built-in
variant that requires libatomic.

Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
*could* technically build with gcc 4.7. This is probably not a big deal,
and we can live with requiring gcc 4.8 on PowerPC to build protobuf.

Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 ...GLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch | 61 ++++++++++++++++++++++
 ...configure.ac-check-if-libatomic-is-needed.patch | 34 ++++++++++++
 package/protobuf/Config.in                         | 20 +++++--
 3 files changed, 110 insertions(+), 5 deletions(-)
 create mode 100644 package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
 create mode 100644 package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch

diff --git a/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch b/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
new file mode 100644
index 0000000..a46e459
--- /dev/null
+++ b/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
@@ -0,0 +1,61 @@
+From 7ad8690c5d8a875881ba00c3551595a2676538ef Mon Sep 17 00:00:00 2001
+From: George Redivo <george.redivo@datacom.ind.br>
+Date: Mon, 6 Jul 2015 16:56:41 -0300
+Subject: [PATCH] Fix GOOGLE_PROTOBUF_ATOMICOPS_ERROR syntax error
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+It's not possible to define "#error" inside a define.
+It causes 'error: stray ?#? in program' compilation error.
+
+Now the define GOOGLE_PROTOBUF_ATOMICOPS_ERROR is the error message
+and it's used along the code together "#error".
+---
+ src/google/protobuf/stubs/atomicops.h | 10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/src/google/protobuf/stubs/atomicops.h b/src/google/protobuf/stubs/atomicops.h
+index cd20caa..5fa31b0 100644
+--- a/src/google/protobuf/stubs/atomicops.h
++++ b/src/google/protobuf/stubs/atomicops.h
+@@ -173,7 +173,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ 
+ // Include our platform specific implementation.
+ #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
+-#error "Atomic operations are not supported on your platform"
++"Atomic operations are not supported on your platform"
+ 
+ // ThreadSanitizer, http://clang.llvm.org/docs/ThreadSanitizer.html.
+ #if defined(THREAD_SANITIZER)
+@@ -183,7 +183,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ #if defined(GOOGLE_PROTOBUF_ARCH_IA32) || defined(GOOGLE_PROTOBUF_ARCH_X64)
+ #include <google/protobuf/stubs/atomicops_internals_x86_msvc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Solaris
+@@ -218,15 +218,15 @@ GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #if __has_extension(c_atomic)
+ #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Unknown.
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // On some platforms we need additional declarations to make AtomicWord
+-- 
+2.5.0
+
diff --git a/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
new file mode 100644
index 0000000..237bc71
--- /dev/null
+++ b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
@@ -0,0 +1,34 @@
+From 0883fa19d59ece19eec30937c65fd10162ef57b0 Mon Sep 17 00:00:00 2001
+From: Carlos Santos <casantos@datacom.ind.br>
+Date: Wed, 10 Feb 2016 12:54:43 -0200
+Subject: [PATCH] configure.ac: check if libatomic is needed
+
+In Buildroot, to simplify things, we've decided to simply require gcc 4.8
+as soon as the architectures has at least one __atomic_*() built-in
+variant that requires libatomic.
+
+Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
+*could* technically build with gcc 4.7. This is probably not a big deal,
+and we can live with requiring gcc 4.8 on PowerPC to build protobuf.
+
+Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
+---
+ configure.ac | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/configure.ac b/configure.ac
+index c07067c..88d4a0d 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -139,6 +139,8 @@ AM_CONDITIONAL([USE_EXTERNAL_PROTOC], [test "$with_protoc" != "no"])
+ ACX_PTHREAD
+ AC_CXX_STL_HASH
+ 
++AC_SEARCH_LIBS([__atomic_load_4], [atomic])
++
+ case "$target_os" in
+   mingw* | cygwin* | win*)
+     ;;
+-- 
+2.5.0
+
diff --git a/package/protobuf/Config.in b/package/protobuf/Config.in
index 3d4320b..f4f428a 100644
--- a/package/protobuf/Config.in
+++ b/package/protobuf/Config.in
@@ -1,12 +1,22 @@
 # See src/google/protobuf/stubs/platform_macros.h for supported archs.
-# PowerPC doesn't actually work.
+#
+# On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
+# types are available built-in. However, the __atomic_*() built-ins for
+# 8-byte types is implemented via libatomic, so only available since gcc
+# 4.8.
+#
+# In Buildroot, to simplify things, we've decided to simply require gcc
+# 4.8 as soon as the architectures has at least one __atomic_*() built-in
+# variant that requires libatomic.
+#
+# Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
+# *could* technically build with gcc 4.7. This is probably not a big deal,
+# and we can live with requiring gcc 4.8 on PowerPC to build protobuf. 
+#
 # host-protobuf only builds on certain architectures
 config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
 	bool
-	default y if BR2_arm
-	default y if BR2_i386
-	default y if BR2_mipsel
-	default y if BR2_x86_64
+	default y if BR2_TOOLCHAIN_HAS_ATOMIC
 	depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
 
 config BR2_PACKAGE_PROTOBUF
-- 
2.5.0

[Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

[Thomas Petazzoni]

Hello,

Thanks, this looks good, with one nit, see below.

On Wed, 10 Feb 2016 13:33:12 -0200, Carlos Santos wrote:

> diff --git a/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
> new file mode 100644
> index 0000000..237bc71
> --- /dev/null
> +++ b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
> @@ -0,0 +1,34 @@
> +From 0883fa19d59ece19eec30937c65fd10162ef57b0 Mon Sep 17 00:00:00 2001
> +From: Carlos Santos <casantos@datacom.ind.br>
> +Date: Wed, 10 Feb 2016 12:54:43 -0200
> +Subject: [PATCH] configure.ac: check if libatomic is needed
> +
> +In Buildroot, to simplify things, we've decided to simply require gcc 4.8
> +as soon as the architectures has at least one __atomic_*() built-in
> +variant that requires libatomic.
> +
> +Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
> +*could* technically build with gcc 4.7. This is probably not a big deal,
> +and we can live with requiring gcc 4.8 on PowerPC to build protobuf.
> +
> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>

The patch description should not mention Buildroot and not mention
Buildroot specific choices. It should be written as if you were going
to submit it upstream, i.e with a proper justification as to why
linking with libatomic may be needed.

And in fact, I'm even going to ask you to submit this patch upstream :-)

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Thomas Petazzoni]

Dear Carlos Santos,

On Wed, 10 Feb 2016 13:25:49 -0200 (BRST), Carlos Santos wrote:

> > config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
> >        bool
> >        default y if BR2_arm
> >        default y if BR2_i386
> >        default y if BR2_mipsel
> >        default y if BR2_x86_64
> 
> These architecture checks would be superseded by the BR2_TOOLCHAIN_HAS_ATOMIC boolean.

Indeed.

> >	default y if BR2_TOOLCHAIN_HAS_ATOMIC
> >        depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
> > 
> > *and* ensure protobuf gets linked with -latomic.
> 
> Hum, this requires a patch on configure.ac that that would hardly be accepted upstream.

Why? If they use __atomic_*() built-ins, then they must link with
libatomic if it exists, since __atomic_*() built-ins are not guaranteed
to be available on all architectures if you don't link with libatomic.

Look for example at the Android NDK documentation, which says that you
should link with libatomic when using <atomic> in C++ :

  http://developer.android.com/ndk/guides/cpp-support.html

(search for "atomic support").

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Carlos Santos]

----- Original Message -----
> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Sent: Wednesday, February 10, 2016 1:57:00 PM
> Subject: Re: [Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

> Dear Carlos Santos,
> 
> On Wed, 10 Feb 2016 13:25:49 -0200 (BRST), Carlos Santos wrote:
> 
>> > config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
>> >        bool
>> >        default y if BR2_arm
>> >        default y if BR2_i386
>> >        default y if BR2_mipsel
>> >        default y if BR2_x86_64
>> 
>> These architecture checks would be superseded by the BR2_TOOLCHAIN_HAS_ATOMIC
>> boolean.
> 
> Indeed.
> 
>> >	default y if BR2_TOOLCHAIN_HAS_ATOMIC
>> >        depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
>> > 
>> > *and* ensure protobuf gets linked with -latomic.
>> 
>> Hum, this requires a patch on configure.ac that that would hardly be accepted
>> upstream.
> 
> Why? If they use __atomic_*() built-ins, then they must link with
> libatomic if it exists, since __atomic_*() built-ins are not guaranteed
> to be available on all architectures if you don't link with libatomic.
> 
> Look for example at the Android NDK documentation, which says that you
> should link with libatomic when using <atomic> in C++ :
> 
>  http://developer.android.com/ndk/guides/cpp-support.html
> 
> (search for "atomic support").

The problem is the same you found on mpd (commit 84533029c70a4dffb2cd2e4f05e3903fd1b8fcd9) in branch master. With the patch in configure.ac I get this:

$ owerpc-e500v2-linux-gnuspe-ldd --root target/ target/usr/bin/protoc
        libprotobuf.so.9 => /usr/lib/libprotobuf.so.9 (0xdeadbeef)
        libatomic.so.1 => /lib/libatomic.so.1 (0xdeadbeef)
        libpthread.so.0 => /lib/libpthread.so.0 (0xdeadbeef)
        libc.so.6 => /lib/libc.so.6 (0xdeadbeef)
        ld.so.1 => /lib/ld.so.1 (0xdeadbeef)
        libz.so.1 => /usr/lib/libz.so.1 (0xdeadbeef)
        libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0xdeadbeef)
        libm.so.6 => /lib/libm.so.6 (0xdeadbeef)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0xdeadbeef)
        libprotoc.so.9 => /usr/lib/libprotoc.so.9 (0xdeadbeef)

Without the patch protoc is not linked to libatomic.so.1.

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Thomas Petazzoni]

Dear Carlos Santos,

On Wed, 10 Feb 2016 14:32:06 -0200 (BRST), Carlos Santos wrote:

> The problem is the same you found on mpd (commit 84533029c70a4dffb2cd2e4f05e3903fd1b8fcd9) in branch master. With the patch in configure.ac I get this:
> 
> $ owerpc-e500v2-linux-gnuspe-ldd --root target/ target/usr/bin/protoc
>         libprotobuf.so.9 => /usr/lib/libprotobuf.so.9 (0xdeadbeef)
>         libatomic.so.1 => /lib/libatomic.so.1 (0xdeadbeef)
>         libpthread.so.0 => /lib/libpthread.so.0 (0xdeadbeef)
>         libc.so.6 => /lib/libc.so.6 (0xdeadbeef)
>         ld.so.1 => /lib/ld.so.1 (0xdeadbeef)
>         libz.so.1 => /usr/lib/libz.so.1 (0xdeadbeef)
>         libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0xdeadbeef)
>         libm.so.6 => /lib/libm.so.6 (0xdeadbeef)
>         libgcc_s.so.1 => /lib/libgcc_s.so.1 (0xdeadbeef)
>         libprotoc.so.9 => /usr/lib/libprotoc.so.9 (0xdeadbeef)

Yes, looks good.

> Without the patch protoc is not linked to libatomic.so.1.

Indeed.

And so, what's the problem ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Sent: Wednesday, February 10, 2016 2:44:25 PM
> Subject: Re: [Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

> Dear Carlos Santos,
> 
> On Wed, 10 Feb 2016 14:32:06 -0200 (BRST), Carlos Santos wrote:
> 
>> The problem is the same you found on mpd (commit
>> 84533029c70a4dffb2cd2e4f05e3903fd1b8fcd9) in branch master. With the patch in
>> configure.ac I get this:
>> 
>> $ owerpc-e500v2-linux-gnuspe-ldd --root target/ target/usr/bin/protoc
>>         libprotobuf.so.9 => /usr/lib/libprotobuf.so.9 (0xdeadbeef)
>>         libatomic.so.1 => /lib/libatomic.so.1 (0xdeadbeef)
>>         libpthread.so.0 => /lib/libpthread.so.0 (0xdeadbeef)
>>         libc.so.6 => /lib/libc.so.6 (0xdeadbeef)
>>         ld.so.1 => /lib/ld.so.1 (0xdeadbeef)
>>         libz.so.1 => /usr/lib/libz.so.1 (0xdeadbeef)
>>         libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0xdeadbeef)
>>         libm.so.6 => /lib/libm.so.6 (0xdeadbeef)
>>         libgcc_s.so.1 => /lib/libgcc_s.so.1 (0xdeadbeef)
>>         libprotoc.so.9 => /usr/lib/libprotoc.so.9 (0xdeadbeef)
> 
> Yes, looks good.
> 
>> Without the patch protoc is not linked to libatomic.so.1.
> 
> Indeed.
> 
> And so, what's the problem ?

No problem, IMO. It's a solution. :-)

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Sent: Wednesday, February 10, 2016 2:44:25 PM
> Subject: Re: [Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

> Dear Carlos Santos,
> 
> On Wed, 10 Feb 2016 14:32:06 -0200 (BRST), Carlos Santos wrote:
> 
>> The problem is the same you found on mpd (commit
>> 84533029c70a4dffb2cd2e4f05e3903fd1b8fcd9) in branch master. With the patch in
>> configure.ac I get this:
>> 
>> $ owerpc-e500v2-linux-gnuspe-ldd --root target/ target/usr/bin/protoc
>>         libprotobuf.so.9 => /usr/lib/libprotobuf.so.9 (0xdeadbeef)
>>         libatomic.so.1 => /lib/libatomic.so.1 (0xdeadbeef)
>>         libpthread.so.0 => /lib/libpthread.so.0 (0xdeadbeef)
>>         libc.so.6 => /lib/libc.so.6 (0xdeadbeef)
>>         ld.so.1 => /lib/ld.so.1 (0xdeadbeef)
>>         libz.so.1 => /usr/lib/libz.so.1 (0xdeadbeef)
>>         libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0xdeadbeef)
>>         libm.so.6 => /lib/libm.so.6 (0xdeadbeef)
>>         libgcc_s.so.1 => /lib/libgcc_s.so.1 (0xdeadbeef)
>>         libprotoc.so.9 => /usr/lib/libprotoc.so.9 (0xdeadbeef)
> 
> Yes, looks good.
> 
>> Without the patch protoc is not linked to libatomic.so.1.
> 
> Indeed.
> 
> And so, what's the problem ?

Henrique just warned me (offline) about the misunderstanding. The "problem" here is that the patch on configure.ac would not be accepted upstream, since their detection of the atomic operations already works. This is not big deal, IMO, so if you are satisfied with the patch then go ahead and apply it.

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

[Carlos Santos]

[Thanks, Zimbra, for messing rearranging the messages in the inbox, so I answer them in the wrong order].

> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org, "henrique marks" <henrique.marks@datacom.ind.br>
> Sent: Wednesday, February 10, 2016 1:50:37 PM
> Subject: Re: [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

> Hello,
> 
> Thanks, this looks good, with one nit, see below.
> 
> On Wed, 10 Feb 2016 13:33:12 -0200, Carlos Santos wrote:
> 
>> diff --git
>> a/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
>> b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
>> new file mode 100644
>> index 0000000..237bc71
>> --- /dev/null
>> +++ b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
>> @@ -0,0 +1,34 @@
>> +From 0883fa19d59ece19eec30937c65fd10162ef57b0 Mon Sep 17 00:00:00 2001
>> +From: Carlos Santos <casantos@datacom.ind.br>
>> +Date: Wed, 10 Feb 2016 12:54:43 -0200
>> +Subject: [PATCH] configure.ac: check if libatomic is needed
>> +
>> +In Buildroot, to simplify things, we've decided to simply require gcc 4.8
>> +as soon as the architectures has at least one __atomic_*() built-in
>> +variant that requires libatomic.
>> +
>> +Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
>> +*could* technically build with gcc 4.7. This is probably not a big deal,
>> +and we can live with requiring gcc 4.8 on PowerPC to build protobuf.
>> +
>> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> 
> The patch description should not mention Buildroot and not mention
> Buildroot specific choices. It should be written as if you were going
> to submit it upstream, i.e with a proper justification as to why
> linking with libatomic may be needed.

This patch only exists to appease Buildroot but, anyway, I can rewrite the comment.

> And in fact, I'm even going to ask you to submit this patch upstream :-)

They don't need this. Their detection of the atomic built-ins already works without additional help.

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

[Arnout Vandecappelle]

On 10-02-16 16:33, Carlos Santos wrote:
[snip]
> +#
> +# On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
> +# types are available built-in. However, the __atomic_*() built-ins for
> +# 8-byte types is implemented via libatomic, so only available since gcc
> +# 4.8.
> +#
> +# In Buildroot, to simplify things, we've decided to simply require gcc
> +# 4.8 as soon as the architectures has at least one __atomic_*() built-in
> +# variant that requires libatomic.
> +#
> +# Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
> +# *could* technically build with gcc 4.7. This is probably not a big deal,
> +# and we can live with requiring gcc 4.8 on PowerPC to build protobuf. 
> +#
>  # host-protobuf only builds on certain architectures
>  config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
>  	bool
> -	default y if BR2_arm
> -	default y if BR2_i386
> -	default y if BR2_mipsel
> -	default y if BR2_x86_64

 Sorry for my confusion, but why do you remove these? It will still work with
gcc 4.7 on arm, i386, mipsel and x86_64 because it will use its custom atomic
implementation, no?


 Regards,
 Arnout

> +	default y if BR2_TOOLCHAIN_HAS_ATOMIC
>  	depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
[snip]

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

[Arnout Vandecappelle]

On 10-02-16 19:42, Carlos Santos wrote:
> [Thanks, Zimbra, for messing rearranging the messages in the inbox, so I answer them in the wrong order].
> 
>> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
>> To: "Carlos Santos" <casantos@datacom.ind.br>
>> Cc: buildroot at buildroot.org, "henrique marks" <henrique.marks@datacom.ind.br>
>> Sent: Wednesday, February 10, 2016 1:50:37 PM
>> Subject: Re: [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins
> 
>> Hello,
>>
>> Thanks, this looks good, with one nit, see below.
>>
>> On Wed, 10 Feb 2016 13:33:12 -0200, Carlos Santos wrote:
>>
>>> diff --git
>>> a/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
>>> b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
>>> new file mode 100644
>>> index 0000000..237bc71
>>> --- /dev/null
>>> +++ b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
>>> @@ -0,0 +1,34 @@
>>> +From 0883fa19d59ece19eec30937c65fd10162ef57b0 Mon Sep 17 00:00:00 2001
>>> +From: Carlos Santos <casantos@datacom.ind.br>
>>> +Date: Wed, 10 Feb 2016 12:54:43 -0200
>>> +Subject: [PATCH] configure.ac: check if libatomic is needed
>>> +
>>> +In Buildroot, to simplify things, we've decided to simply require gcc 4.8
>>> +as soon as the architectures has at least one __atomic_*() built-in
>>> +variant that requires libatomic.
>>> +
>>> +Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
>>> +*could* technically build with gcc 4.7. This is probably not a big deal,
>>> +and we can live with requiring gcc 4.8 on PowerPC to build protobuf.
>>> +
>>> +Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
>>
>> The patch description should not mention Buildroot and not mention
>> Buildroot specific choices. It should be written as if you were going
>> to submit it upstream, i.e with a proper justification as to why
>> linking with libatomic may be needed.
> 
> This patch only exists to appease Buildroot but, anyway, I can rewrite the comment.
> 
>> And in fact, I'm even going to ask you to submit this patch upstream :-)
> 
> They don't need this. Their detection of the atomic built-ins already works without additional help.

 Clearly it doesn't work, or this wouldn't be needed...

 Except if they specifically want to exclude any architecture for which they
don't have their custom atomics implementation.

 By the way, you check for atomic_4, which happens to be a special case on
microblaze: it doesn't need -latomic for atomic_4, but it does for the others.
So if they do use any other atomic operation, it would be better to check for
atomic_2 or atomic_1 (or better yet, all of them).

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Thomas Petazzoni]

Carlos,

On Wed, 10 Feb 2016 16:30:19 -0200 (BRST), Carlos Santos wrote:

> Henrique just warned me (offline) about the misunderstanding. The
> "problem" here is that the patch on configure.ac would not be
> accepted upstream, since their detection of the atomic operations
> already works.

What? It certainly cannot work if they don't link against libatomic.
Try on an architecture like SPARC, and you will see that if you don't
link with libatomic, the build will fail.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

[Carlos Santos]

> From: "Arnout Vandecappelle" <arnout@mind.be>
> To: "Carlos Santos" <casantos@datacom.ind.br>, buildroot at buildroot.org
> Cc: "thomas petazzoni" <thomas.petazzoni@free-electrons.com>
> Sent: Wednesday, February 10, 2016 6:00:16 PM
> Subject: Re: [Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

> On 10-02-16 16:33, Carlos Santos wrote:
> [snip]
>> +#
>> +# On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
>> +# types are available built-in. However, the __atomic_*() built-ins for
>> +# 8-byte types is implemented via libatomic, so only available since gcc
>> +# 4.8.
>> +#
>> +# In Buildroot, to simplify things, we've decided to simply require gcc
>> +# 4.8 as soon as the architectures has at least one __atomic_*() built-in
>> +# variant that requires libatomic.
>> +#
>> +# Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
>> +# *could* technically build with gcc 4.7. This is probably not a big deal,
>> +# and we can live with requiring gcc 4.8 on PowerPC to build protobuf.
>> +#
>>  # host-protobuf only builds on certain architectures
>>  config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
>>  	bool
>> -	default y if BR2_arm
>> -	default y if BR2_i386
>> -	default y if BR2_mipsel
>> -	default y if BR2_x86_64
> 
> Sorry for my confusion, but why do you remove these? It will still work with
> gcc 4.7 on arm, i386, mipsel and x86_64 because it will use its custom atomic
> implementation, no?

Ok, I will restore the prevoius defaults, allowing protobuf to build with gcc < 4.8.

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org
> Sent: Wednesday, February 10, 2016 6:13:16 PM
> Subject: Re: [Buildroot] [PATCH 1/1] protobuf: apply patch to compile for PowerPC

> Carlos,
> 
> On Wed, 10 Feb 2016 16:30:19 -0200 (BRST), Carlos Santos wrote:
> 
>> Henrique just warned me (offline) about the misunderstanding. The
>> "problem" here is that the patch on configure.ac would not be
>> accepted upstream, since their detection of the atomic operations
>> already works.
> 
> What? It certainly cannot work if they don't link against libatomic.
> Try on an architecture like SPARC, and you will see that if you don't
> link with libatomic, the build will fail.

I tried. It worked on SPARC (with the libatomic patch) but failed on SPARC64 due to a missing declaration of "Atomic64".

I guess their SPARC code aims Solaris, not Linux/buildroot.

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH 1/1] protobuf: fix detection of __atomic_*() built-ins

[Carlos Santos]

- Use the recently introduced BR2_TOOLCHAIN_HAS_ATOMIC boolean.

- Import an upstream patch to fix error handling when atomic operations
  are not detected. Without this patch the build fails due to a syntax
  error instead of showing the proper message.

- Add a patch to configure.ac to check if libatomic is needed and force
  linking to it (this patch would unlikely be accepted upstream).

- Disable build for SPARC64.

On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
types are available built-in. However, the __atomic_*() built-ins for
8-byte types is implemented via libatomic, so only available since gcc
4.8.

In Buildroot, to simplify things, we've decided to simply require gcc 4.8
as soon as the architectures has at least one __atomic_*() built-in
variant that requires libatomic.

Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
*could* technically build with gcc 4.7. This is probably not a big deal,
and we can live with requiring gcc 4.8 on PowerPC to build protobuf.

The SPARC64 build fails due to a missing definition of Atomic64. This
has been fixed on the master branch but the build still breaks due to
undefined references to internal NoBarrier_Atomic*() functions.
Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 ...GLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch | 61 ++++++++++++++++++++++
 ...configure.ac-check-if-libatomic-is-needed.patch | 34 ++++++++++++
 package/protobuf/Config.in                         | 21 +++++++-
 3 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
 create mode 100644 package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch

diff --git a/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch b/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
new file mode 100644
index 0000000..c271ea4
--- /dev/null
+++ b/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
@@ -0,0 +1,61 @@
+From 50982f711de6ad58f6e0bef01a75d2b9cf35f5dc Mon Sep 17 00:00:00 2001
+From: George Redivo <george.redivo@datacom.ind.br>
+Date: Mon, 6 Jul 2015 16:56:41 -0300
+Subject: [PATCH 1/2] Fix GOOGLE_PROTOBUF_ATOMICOPS_ERROR syntax error
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+It's not possible to define "#error" inside a define.
+It causes 'error: stray ?#? in program' compilation error.
+
+Now the define GOOGLE_PROTOBUF_ATOMICOPS_ERROR is the error message
+and it's used along the code together "#error".
+---
+ src/google/protobuf/stubs/atomicops.h | 10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/src/google/protobuf/stubs/atomicops.h b/src/google/protobuf/stubs/atomicops.h
+index b1336e3..a130b38 100644
+--- a/src/google/protobuf/stubs/atomicops.h
++++ b/src/google/protobuf/stubs/atomicops.h
+@@ -162,7 +162,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ 
+ // Include our platform specific implementation.
+ #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
+-#error "Atomic operations are not supported on your platform"
++"Atomic operations are not supported on your platform"
+ 
+ // ThreadSanitizer, http://clang.llvm.org/docs/ThreadSanitizer.html.
+ #if defined(THREAD_SANITIZER)
+@@ -172,7 +172,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ #if defined(GOOGLE_PROTOBUF_ARCH_IA32) || defined(GOOGLE_PROTOBUF_ARCH_X64)
+ #include <google/protobuf/stubs/atomicops_internals_x86_msvc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Solaris
+@@ -203,15 +203,15 @@ GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #if __has_extension(c_atomic)
+ #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Unknown.
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // On some platforms we need additional declarations to make AtomicWord
+-- 
+2.5.0
+
diff --git a/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
new file mode 100644
index 0000000..a70a23e
--- /dev/null
+++ b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
@@ -0,0 +1,34 @@
+From f020fe05a20dfcd16cd7df833dcf3cdeef770538 Mon Sep 17 00:00:00 2001
+From: Carlos Santos <casantos@datacom.ind.br>
+Date: Thu, 11 Feb 2016 10:58:35 -0200
+Subject: [PATCH 2/2] configure.ac: check if libatomic is needed
+
+Compilation of protobuf for PowerPC and SPARC may fail due to missing
+references to __atomic_fetch_add_4 and __atomic_compare_exchange_4.
+
+The __atomic_*() intrinsics for all sizes are provided by libatomic when
+gcc is >= 4.8. This can be achieved by adding this to configure.ac:
+
+    AC_SEARCH_LIBS([__atomic_fetch_add_4], [atomic])
+
+Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
+---
+ configure.ac | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/configure.ac b/configure.ac
+index c07067c..88d4a0d 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -139,6 +139,8 @@ AM_CONDITIONAL([USE_EXTERNAL_PROTOC], [test "$with_protoc" != "no"])
+ ACX_PTHREAD
+ AC_CXX_STL_HASH
+ 
++AC_SEARCH_LIBS([__atomic_load_4], [atomic])
++
+ case "$target_os" in
+   mingw* | cygwin* | win*)
+     ;;
+-- 
+2.5.0
+
diff --git a/package/protobuf/Config.in b/package/protobuf/Config.in
index 3d4320b..3215a07 100644
--- a/package/protobuf/Config.in
+++ b/package/protobuf/Config.in
@@ -1,5 +1,22 @@
 # See src/google/protobuf/stubs/platform_macros.h for supported archs.
-# PowerPC doesn't actually work.
+#
+# On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
+# types are available built-in. However, the __atomic_*() built-ins for
+# 8-byte types is implemented via libatomic, so only available since gcc
+# 4.8.
+#
+# In Buildroot, to simplify things, we've decided to simply require gcc
+# 4.8 as soon as the architectures has at least one __atomic_*() built-in
+# variant that requires libatomic.
+#
+# Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
+# *could* technically build with gcc 4.7. This is probably not a big deal,
+# and we can live with requiring gcc 4.8 on PowerPC to build protobuf. 
+#
+# The SPARC64 build fails due to a missing definition of Atomic64. This
+# has been fixed on the master branch but the build still breaks due to
+# undefined references to internal NoBarrier_Atomic*() functions.
+#
 # host-protobuf only builds on certain architectures
 config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
 	bool
@@ -7,6 +24,8 @@ config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
 	default y if BR2_i386
 	default y if BR2_mipsel
 	default y if BR2_x86_64
+	default y if BR2_TOOLCHAIN_HAS_ATOMIC
+	depends on !BR2_sparc64 # missing definition of Atomic64
 	depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
 
 config BR2_PACKAGE_PROTOBUF
-- 
2.5.0

[Buildroot] [PATCH v2 0/1] protobuf: fix detection of __atomic_*() built-ins

[Carlos Santos]

- Clarify that we *will* attempt to submit upstream the patch in
  configure.ac.

- Small improvements on the wording of the commit message.

Carlos Santos (1):
  protobuf: fix detection of __atomic_*() built-ins

 ...GLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch | 61 ++++++++++++++++++++++
 ...configure.ac-check-if-libatomic-is-needed.patch | 34 ++++++++++++
 package/protobuf/Config.in                         | 21 +++++++-
 3 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
 create mode 100644 package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch

-- 
2.5.0

[Buildroot] [PATCH v2 1/1] protobuf: fix detection of __atomic_*() built-ins

[Carlos Santos]

- Use the recently introduced BR2_TOOLCHAIN_HAS_ATOMIC boolean.

- Import an upstream patch to fix error handling when atomic operations
  are not detected. Without this patch the build fails due to a syntax
  error instead of showing the proper message.

- Add a patch to configure.ac to check if libatomic is needed and force
  linking to it (we will attempt to submit this upstream).

- Disable build for SPARC64 because it fails due to a missing definition
  of Atomic64.

On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
types are available built-in. The corresponding built-ins for 8-byte
types, however, are implemented via libatomic, so requiring gcc >= 4.8.

In Buildroot, to simplify things, it was decided to require gcc 4.8 as
soon as the architectures has at least one __atomic_*() built-in variant
that requires libatomic.

Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
*could* technically build with gcc 4.7. This is probably not a big deal,
and we can live with requiring gcc 4.8 on PowerPC to build protobuf. The
same restriction applies to SPARC.

The build for SPARC64 breaks even using the master branch of protobuf
due to undefined references to some NoBarrier_Atomic*() functions.

Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 ...GLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch | 61 ++++++++++++++++++++++
 ...configure.ac-check-if-libatomic-is-needed.patch | 34 ++++++++++++
 package/protobuf/Config.in                         | 21 +++++++-
 3 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
 create mode 100644 package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch

diff --git a/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch b/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
new file mode 100644
index 0000000..c271ea4
--- /dev/null
+++ b/package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
@@ -0,0 +1,61 @@
+From 50982f711de6ad58f6e0bef01a75d2b9cf35f5dc Mon Sep 17 00:00:00 2001
+From: George Redivo <george.redivo@datacom.ind.br>
+Date: Mon, 6 Jul 2015 16:56:41 -0300
+Subject: [PATCH 1/2] Fix GOOGLE_PROTOBUF_ATOMICOPS_ERROR syntax error
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+It's not possible to define "#error" inside a define.
+It causes 'error: stray ?#? in program' compilation error.
+
+Now the define GOOGLE_PROTOBUF_ATOMICOPS_ERROR is the error message
+and it's used along the code together "#error".
+---
+ src/google/protobuf/stubs/atomicops.h | 10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/src/google/protobuf/stubs/atomicops.h b/src/google/protobuf/stubs/atomicops.h
+index b1336e3..a130b38 100644
+--- a/src/google/protobuf/stubs/atomicops.h
++++ b/src/google/protobuf/stubs/atomicops.h
+@@ -162,7 +162,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ 
+ // Include our platform specific implementation.
+ #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
+-#error "Atomic operations are not supported on your platform"
++"Atomic operations are not supported on your platform"
+ 
+ // ThreadSanitizer, http://clang.llvm.org/docs/ThreadSanitizer.html.
+ #if defined(THREAD_SANITIZER)
+@@ -172,7 +172,7 @@ Atomic64 Release_Load(volatile const Atomic64* ptr);
+ #if defined(GOOGLE_PROTOBUF_ARCH_IA32) || defined(GOOGLE_PROTOBUF_ARCH_X64)
+ #include <google/protobuf/stubs/atomicops_internals_x86_msvc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Solaris
+@@ -203,15 +203,15 @@ GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #if __has_extension(c_atomic)
+ #include <google/protobuf/stubs/atomicops_internals_generic_gcc.h>
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // Unknown.
+ #else
+-GOOGLE_PROTOBUF_ATOMICOPS_ERROR
++#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR
+ #endif
+ 
+ // On some platforms we need additional declarations to make AtomicWord
+-- 
+2.5.0
+
diff --git a/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
new file mode 100644
index 0000000..a70a23e
--- /dev/null
+++ b/package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch
@@ -0,0 +1,34 @@
+From f020fe05a20dfcd16cd7df833dcf3cdeef770538 Mon Sep 17 00:00:00 2001
+From: Carlos Santos <casantos@datacom.ind.br>
+Date: Thu, 11 Feb 2016 10:58:35 -0200
+Subject: [PATCH 2/2] configure.ac: check if libatomic is needed
+
+Compilation of protobuf for PowerPC and SPARC may fail due to missing
+references to __atomic_fetch_add_4 and __atomic_compare_exchange_4.
+
+The __atomic_*() intrinsics for all sizes are provided by libatomic when
+gcc is >= 4.8. This can be achieved by adding this to configure.ac:
+
+    AC_SEARCH_LIBS([__atomic_fetch_add_4], [atomic])
+
+Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
+---
+ configure.ac | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/configure.ac b/configure.ac
+index c07067c..88d4a0d 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -139,6 +139,8 @@ AM_CONDITIONAL([USE_EXTERNAL_PROTOC], [test "$with_protoc" != "no"])
+ ACX_PTHREAD
+ AC_CXX_STL_HASH
+ 
++AC_SEARCH_LIBS([__atomic_load_4], [atomic])
++
+ case "$target_os" in
+   mingw* | cygwin* | win*)
+     ;;
+-- 
+2.5.0
+
diff --git a/package/protobuf/Config.in b/package/protobuf/Config.in
index 3d4320b..3215a07 100644
--- a/package/protobuf/Config.in
+++ b/package/protobuf/Config.in
@@ -1,5 +1,22 @@
 # See src/google/protobuf/stubs/platform_macros.h for supported archs.
-# PowerPC doesn't actually work.
+#
+# On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
+# types are available built-in. However, the __atomic_*() built-ins for
+# 8-byte types is implemented via libatomic, so only available since gcc
+# 4.8.
+#
+# In Buildroot, to simplify things, we've decided to simply require gcc
+# 4.8 as soon as the architectures has at least one __atomic_*() built-in
+# variant that requires libatomic.
+#
+# Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
+# *could* technically build with gcc 4.7. This is probably not a big deal,
+# and we can live with requiring gcc 4.8 on PowerPC to build protobuf. 
+#
+# The SPARC64 build fails due to a missing definition of Atomic64. This
+# has been fixed on the master branch but the build still breaks due to
+# undefined references to internal NoBarrier_Atomic*() functions.
+#
 # host-protobuf only builds on certain architectures
 config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
 	bool
@@ -7,6 +24,8 @@ config BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
 	default y if BR2_i386
 	default y if BR2_mipsel
 	default y if BR2_x86_64
+	default y if BR2_TOOLCHAIN_HAS_ATOMIC
+	depends on !BR2_sparc64 # missing definition of Atomic64
 	depends on BR2_HOSTARCH = "x86_64" || BR2_HOSTARCH = "x86"
 
 config BR2_PACKAGE_PROTOBUF
-- 
2.5.0

[Buildroot] [PATCH v2 0/1] protobuf: fix detection of __atomic_*() built-ins

[Carlos Santos]

> From: "Carlos Santos" <casantos@datacom.ind.br>
> To: buildroot at buildroot.org
> Cc: "thomas petazzoni" <thomas.petazzoni@free-electrons.com>
> Sent: Wednesday, February 17, 2016 3:43:00 PM
> Subject: [Buildroot] [PATCH v2 0/1] protobuf: fix detection of __atomic_*()	built-ins

> - Clarify that we *will* attempt to submit upstream the patch in
>  configure.ac.

In progress:

  - https://goo.gl/Kzju4X
  - https://goo.gl/v6bypJ

> - Small improvements on the wording of the commit message.
> 
> Carlos Santos (1):
>  protobuf: fix detection of __atomic_*() built-ins
> 
> ...GLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch | 61 ++++++++++++++++++++++
> ...configure.ac-check-if-libatomic-is-needed.patch | 34 ++++++++++++
> package/protobuf/Config.in                         | 21 +++++++-
> 3 files changed, 115 insertions(+), 1 deletion(-)
> create mode 100644
> package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
> create mode 100644
> package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch

Carlos Santos (Casantos)
DATACOM, P&D

[Buildroot] [PATCH v2 0/1] protobuf: fix detection of __atomic_*() built-ins

[Thomas Petazzoni]

Dear Carlos Santos,

On Wed, 17 Feb 2016 16:33:00 -0200 (BRST), Carlos Santos wrote:
> > From: "Carlos Santos" <casantos@datacom.ind.br>
> > To: buildroot at buildroot.org
> > Cc: "thomas petazzoni" <thomas.petazzoni@free-electrons.com>
> > Sent: Wednesday, February 17, 2016 3:43:00 PM
> > Subject: [Buildroot] [PATCH v2 0/1] protobuf: fix detection of __atomic_*()	built-ins
> 
> > - Clarify that we *will* attempt to submit upstream the patch in
> >  configure.ac.
> 
> In progress:
> 
>   - https://goo.gl/Kzju4X
>   - https://goo.gl/v6bypJ

Excellent, thanks for working on this!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2 1/1] protobuf: fix detection of __atomic_*() built-ins

[Arnout Vandecappelle]

On 02/17/16 18:43, Carlos Santos wrote:
> - Use the recently introduced BR2_TOOLCHAIN_HAS_ATOMIC boolean.
> 
> - Import an upstream patch to fix error handling when atomic operations
>   are not detected. Without this patch the build fails due to a syntax
>   error instead of showing the proper message.
> 
> - Add a patch to configure.ac to check if libatomic is needed and force
>   linking to it (we will attempt to submit this upstream).
> 
> - Disable build for SPARC64 because it fails due to a missing definition
>   of Atomic64.
> 
> On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
> types are available built-in. The corresponding built-ins for 8-byte
> types, however, are implemented via libatomic, so requiring gcc >= 4.8.
> 
> In Buildroot, to simplify things, it was decided to require gcc 4.8 as
> soon as the architectures has at least one __atomic_*() built-in variant
> that requires libatomic.
> 
> Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
> *could* technically build with gcc 4.7. This is probably not a big deal,
> and we can live with requiring gcc 4.8 on PowerPC to build protobuf. The
> same restriction applies to SPARC.
> 
> The build for SPARC64 breaks even using the master branch of protobuf
> due to undefined references to some NoBarrier_Atomic*() functions.
> 
> Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 The comment in Config.in is perhaps a bit heavy, but it definitely doesn't hurt.

[snip]

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 1/1] protobuf: fix detection of __atomic_*() built-ins

[Thomas Petazzoni]

Hello,

On Wed, 17 Feb 2016 15:43:01 -0200, Carlos Santos wrote:
> - Use the recently introduced BR2_TOOLCHAIN_HAS_ATOMIC boolean.
> 
> - Import an upstream patch to fix error handling when atomic operations
>   are not detected. Without this patch the build fails due to a syntax
>   error instead of showing the proper message.
> 
> - Add a patch to configure.ac to check if libatomic is needed and force
>   linking to it (we will attempt to submit this upstream).
> 
> - Disable build for SPARC64 because it fails due to a missing definition
>   of Atomic64.
> 
> On PowerPC, the __atomic_*() built-ins for 1-byte, 2-byte and 4-byte
> types are available built-in. The corresponding built-ins for 8-byte
> types, however, are implemented via libatomic, so requiring gcc >= 4.8.
> 
> In Buildroot, to simplify things, it was decided to require gcc 4.8 as
> soon as the architectures has at least one __atomic_*() built-in variant
> that requires libatomic.
> 
> Since protobuf most likely only uses the 1, 2 and 4-byte variants, it
> *could* technically build with gcc 4.7. This is probably not a big deal,
> and we can live with requiring gcc 4.8 on PowerPC to build protobuf. The
> same restriction applies to SPARC.
> 
> The build for SPARC64 breaks even using the master branch of protobuf
> due to undefined references to some NoBarrier_Atomic*() functions.
> 
> Signed-off-by: Henrique Marks <henrique.marks@datacom.ind.br>
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
>  ...GLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch | 61 ++++++++++++++++++++++
>  ...configure.ac-check-if-libatomic-is-needed.patch | 34 ++++++++++++
>  package/protobuf/Config.in                         | 21 +++++++-
>  3 files changed, 115 insertions(+), 1 deletion(-)
>  create mode 100644 package/protobuf/0001-Fix-GOOGLE_PROTOBUF_ATOMICOPS_ERROR-syntax-error.patch
>  create mode 100644 package/protobuf/0002-configure.ac-check-if-libatomic-is-needed.patch

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com