protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

OpenBSD 32-bit build of 3.20.0 fails; arenastring alignment static_assert fails

Open sthen opened this issue 2 years ago • 5 comments

What version of protobuf and what language are you using? Version: 3.20.0 Language: C++

What operating system (Linux, Windows, ...) and version? OpenBSD/i386 (i.e. 32-bit) 7.1-current

What runtime / compiler are you using (e.g., python version or gcc version) Clang 13.0.0

What did you do? Steps to reproduce the behavior: Try to compile

What did you expect to see Successful build

What did you see instead?

/pobj/protobuf-3.20.0/protobuf-3.20.0/src/google/protobuf/arenastring.cc:67:1: error: static_assert failed due to requirement '(kStringAlign > kNewAlign ? kStringAlign : kNewAlign) >= 8' ""
static_assert((kStringAlign > kNewAlign ? kStringAlign : kNewAlign) >= 8, "");
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Anything else we should know about your project / environment kNewAlign is set in the #ifdef'd lines above; in this case it is 4 bytes and comes from __STDCPP_DEFAULT_NEW_ALIGNMENT__. alignof(::max_align_t) and alignof(std::max_align_t) are 8 bytes.

sthen avatar Apr 15 '22 14:04 sthen

This was fixed in OpenBSD's LLVM by adjusting the target info to take our malloc alignment into account, which effectively changes __STDCPP_DEFAULT_NEW_ALIGNMENT__ to be 16 on i386:

https://github.com/openbsd/src/commit/dc1895c3dde03d61ceaa07854da438264dccc5d6

Maybe FreeBSD will want a similar fix @sunpoet ?

botovq avatar Apr 22 '22 08:04 botovq

Seeing a whole bunch of build failures related to this with buildroot's(linux cross compilation toolkit) autobuilders on a few different architectures.

jameshilliard avatar Jun 08 '22 07:06 jameshilliard

See also this: https://bugzilla.mozilla.org/show_bug.cgi?id=1785285#c12

Per that discussion, this just looks like a buggy expectation in protobuf's source code. Not knowing the protobuf source, I do not yet have a suggested fix. It would be appreciated if someone who does could weigh in.

jhgit avatar Aug 27 '22 16:08 jhgit

I believe this is already fixed at head. Can you confirm?

sbenzaquen avatar Aug 29 '22 15:08 sbenzaquen

I believe this is already fixed at head. Can you confirm?

I believe that head has a change to address one of the alignment problems - specifically the one mentioned by the OP regarding kStringAlign.

As another similar alignment problem example, however, the build for FreeBSD/i386 (32-bit x86 which has alignof(uint64_t) = 4) still fails with the recent 21.5 release:

In file included from ./google/protobuf/arena.h:52:
 ./google/protobuf/arena_impl.h:643:10: error: requested alignment is less than minimum alignment of 8 for type 'google::protobuf::internal::ThreadSafeArena::CacheAlignedLifecycleIdGenerator'
  struct alignas(kCacheAlignment) CacheAlignedLifecycleIdGenerator {
         ^
1 error generated.

This is not exactly the same error as the one described by the OP, but it is closely related. It is another case where requiring alignment of 8 bytes causes build errors on certain platforms.

jhgit avatar Sep 07 '22 20:09 jhgit

CacheAlignedLifecycleIdGenerator no longer exists at head. Please verify if this problem persist on some other code.

sbenzaquen avatar Apr 19 '23 16:04 sbenzaquen

#if defined(__FreeBSD__) && defined(__i386__)
#define CACHE_ALIGNED __attribute__((aligned(kCacheAlignment)))
#else
#define CACHE_ALIGNED alignas(kCacheAlignment)
#endif

  struct CacheAlignedLifecycleIdGenerator {
  	std::atomic<LifecycleIdAtomic> id;
  } CACHE_ALIGNED;
  static CacheAlignedLifecycleIdGenerator lifecycle_id_generator_;

ThorsDev avatar Sep 14 '23 23:09 ThorsDev