Portable alignas/alignof breaks in GCC 8.3 when using C++11

[neilamoore]

After an update to GCC 8.3.1, we start getting a nasty error from C++ files related to the memory header:

In file included from /opt/rh/devtoolset-8/root/usr/include/c++/8/bits/shared_ptr.h:52,
                 from /opt/rh/devtoolset-8/root/usr/include/c++/8/memory:81,
                 from <censored>.cpp:32:
/opt/rh/devtoolset-8/root/usr/include/c++/8/bits/shared_ptr_base.h: In static member function 'static const std::type_info& std::_Sp_make_shared_tag::Sti()':
/opt/rh/devtoolset-8/root/usr/include/c++/8/bits/shared_ptr_base.h:511:49: error: 'tag' was not declared in this scope
       return reinterpret_cast<const type_info&>(tag)

I found a similar bug report in the GCC Bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89464

I've narrowed it down to some change between GCC 8.2 and 8.3 that make all the various compatibility checks in include/flatcc/portable/pstdalign.h fail and redefine alignas and alignof.

A fairly simple patch fixes it for my environment, but I'm pretty sure it's not actually correct:

diff --git a/include/flatcc/portable/pstdalign.h b/include/flatcc/portable/pstdalign.h
index 8d1e436..8ee91f5 100644
--- a/include/flatcc/portable/pstdalign.h
+++ b/include/flatcc/portable/pstdalign.h
@@ -45,7 +45,7 @@
 #endif
 #endif

-#if !defined PORTABLE_HAS_INCLUDE_STDALIGN
+#if !defined(PORTABLE_HAS_INCLUDE_STDALIGN)
 #if defined(__has_include)
 #if __has_include(<stdalign.h>)
 #define PORTABLE_HAS_INCLUDE_STDALIGN 1
@@ -117,6 +117,8 @@ extern "C" {

 #endif /* __STDC__ */

+#if __cplusplus < 201103
+
 #ifndef alignas
 #define alignas _Alignas
 #endif
@@ -128,6 +130,8 @@ extern "C" {
 #define __alignas_is_defined 1
 #define __alignof_is_defined 1

+#endif
+
 #ifdef __cplusplus
 }
 #endif

I'm available to try any patches to fix this properly, I'll be carrying this one in a fork for now.

[mikkelfj]

Thanks for digging into this. I take it you have read the comments at the top of pstdalign.h. It is a complete mess. The only thing to do is to test for GCC version and adapt. Testing for C++ version is not sufficient because it can't be trusted.

While it is possible to add a GCC version check, I am not sure how GCC behaves without -std=... defined, or with -std=c++11 or -std=c++14 etc. for the different versions.

Even so, a GCC check might not be enough if GCC is being used with newlib etc., so the std library must also be checked. But note that newlib makes a point out of not being detectable.

As it is I can't merge it, as you also suggest, but feel free to come up with more suggestions.

[mikkelfj]

Looking deeper into the problem:

1. If it breaks in GCC 8.3 but not in GCC 8.2 it ought to be enough to check GCC version because the action is to do nothing but include . Any use of newlib or other alternative library would have to come to the same conclusion.

2. There is something fishy going on:

The following section in pstdalign, from line 59: https://github.com/dvidelabs/flatcc/blob/2c1402f5cba9b08671d9bad3e47d2548e3781f43/include/flatcc/portable/pstdalign.h#L58-L73

This ought to define __alignas_is_defined and thereby not enter line 117 where your patch is applied. If I read this correctly, the only way this could not be defined is if cplusplus is not defined, or the c++ version is older than `#if cplusplus > 201103`.

So, it might be that GCC 8.3 changes behaviour when __cplusplus == 201103, i.e. when -std=c++11 (I think). If so the patch is to detect GCC version as an alternative to the the ___cplusplus version check in line 63.

If we then look at you patch, you a test for __cplusplus < 201103, so this seems to confirm my suspicion.

GCC actually attempts to clean up the mess previously introduced, only to make things worse.

[mikkelfj]

As to your fork - until we get this fixed, it may be simpler for you to just define -D__alignas_is_defined=1 as a build time flag.

[mikkelfj]

Try this branch https://github.com/dvidelabs/flatcc/tree/issue130

I made a specific check for C++11 and GCC >= 8.3 early in the file.

Note: I don't know if this starts from 8.3.1 or 8.3.0 if you can pinpoint the exact version and suggest updates to version check, it would be helpful.

https://github.com/dvidelabs/flatcc/blob/8742637778c183dc5f3408e0a7fc3c3476594711/include/flatcc/portable/pstdalign.h#L40-L49

[neilamoore]

This patch works for me on 8.3.1 (and is much better than mine :laughing:), I can't check 8.3 easily as my distro waited until 8.3.1 to package 8.3.

I also couldn't narrow down via changelog/commitlog of when this would have come in, I can't even find where those symbols are defined for C++ in GCC source at all.

I can't find exactly why the behavior changed and I can't easily test 8.3, so I'd consider this a fix unless/until someone else has an issue.

[mikkelfj]

My guess is that 8.3 stopped defining __alignas_is_defined and this breaks the logic because it is supposed to only happen in C++14. Not sure what the C++ std says. The fix in pstdalign.h is non-trivial because it must consider other compilers as well.

I'll merge this as it is better than the alternative regardless.

[mikkelfj]

Deleted branch issue130 after merging to master.