Compiler error with GCC targeting ARM - Atomic32 vs AtomicWord

[Brad Larson]

Hello, I'm getting a compiler error when using GCC to target ARM platforms.  I've patched this locally, but would like to upstream my patch and am not sure on the best generally-acceptable solution.

src/google/protobuf/io/coded_stream.h: In static

member function 'static bool google::protobuf::io::CodedOutputStream::IsDefaultSerializationDeterministic()':

C:\projects\cycling\carlton\external\protobuf\src/google/protobuf/io/coded_stream.h:868:53: error: invalid conversion from 'google::protobuf::internal::AtomicWord* {aka int*}' to 'const volatile

Atomic32* {aka const volatile long int*}' [-fpermissive]

     return google::protobuf::internal::Acquire_Load(&default_serialization_deterministic_);

                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On my platform, int32 is defined as long int and intptr_t is defined as int (They both are 4 bytes).  This is causing the above error.

Locally, I can easily change the type of Atomic32 to intptr_t or just int, or change the type of AtomicWord to int32 or intptr_t.  But this isn't generally acceptable of course.

I'm not sure on what a better solution might be.  I see we already have a check #if GOOGLE_PROTOBUF_ARCH_POWER to typedef Atomic32 to intptr_t in some cases, and extra logic around #if defined(__ILP32__) || defined(GOOGLE_PROTOBUF_OS_NACL).  Could this be simplified with a #if sizeof intptr_t == sizeof int32 check (or similar)?

Very open to any suggestions on how to upstream a fix for this!

Thanks,

Brad

[Brad Larson]

If it is helpful, here are the current typedefs, from atomicops.h:

#if defined(GOOGLE_PROTOBUF_ARCH_POWER)

#if defined(_LP64) || defined(__LP64__)

typedef int32 Atomic32;

typedef intptr_t Atomic64;

#else

typedef intptr_t Atomic32;

typedef int64 Atomic64;

#endif

#else

typedef intptr_t Atomic32;

#ifdef GOOGLE_PROTOBUF_ARCH_64_BIT

// We need to be able to go between Atomic64 and AtomicWord implicitly.  This

// means Atomic64 and AtomicWord should be the same type on 64-bit.

#if defined(__ILP32__) || defined(GOOGLE_PROTOBUF_OS_NACL)

// NaCl's intptr_t is not actually 64-bits on 64-bit!

// http://code.google.com/p/nativeclient/issues/detail?id=1162

// sparcv9's pointer type is 32bits

typedef int64 Atomic64;

#else

typedef intptr_t Atomic64;

#endif

#endif

#endif

// Use AtomicWord for a machine-sized pointer.  It will use the Atomic32 or

// Atomic64 routines below, depending on your architecture.

typedef intptr_t AtomicWord; 

[Feng Xiao]

Hi Brad, can you send your patch as a pull request? It's easier to review the diff there.

--
You received this message because you are subscribed to the Google Groups "Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+unsubscribe@googlegroups.com.
To post to this group, send email to prot...@googlegroups.com.
Visit this group at https://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/d/optout.

[Brad Larson]

On Wednesday, August 9, 2017 at 1:13:54 PM UTC-5, Feng Xiao wrote:

Hi Brad, can you send your patch as a pull request? It's easier to review the diff there.

Hi Feng, thank you for the reply.  I really didn't have a fix in mind and was looking for more of a discussion on what a better fix might be.  However I consulted with a coworker and we came up with this: https://github.com/google/protobuf/pull/3480.  It does solve my problem, but I'm unable to test on the wide variety of hardware platforms supported by GPB.

 

To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+u...@googlegroups.com.

[Feng Xiao]

On Thu, Aug 10, 2017 at 9:53 AM, Brad Larson <bkla...@gmail.com> wrote:

On Wednesday, August 9, 2017 at 1:13:54 PM UTC-5, Feng Xiao wrote:

Hi Brad, can you send your patch as a pull request? It's easier to review the diff there.

Hi Feng, thank you for the reply.  I really didn't have a fix in mind and was looking for more of a discussion on what a better fix might be.  However I consulted with a coworker and we came up with this: https://github.com/google/protobuf/pull/3480.  It does solve my problem, but I'm unable to test on the wide variety of hardware platforms supported by GPB.

Thanks! I think your pull request makes the code cleaner. Made a few minor style comments there.

 

To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+unsubscribe@googlegroups.com.