Bug 205453 - 11.0-CURRENT libcxxrt/guard.cc uses C11's _Static_assert in conditionally-compiled C++ code and when it is used buildworld fails for syntax errors in g++ compilers
Summary: 11.0-CURRENT libcxxrt/guard.cc uses C11's _Static_assert in conditionally-com...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: powerpc Any
: --- Affects Only Me
Assignee: freebsd-toolchain (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-20 07:37 UTC by Mark Millard
Modified: 2016-03-27 22:32 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Millard 2015-12-20 07:37:12 UTC
[There might be some debate if g++ compilers should directly support C11's _Static_assert. I've pick the component to submit under under the answer "no".]

A 6 line program can show the behavior of FreeBSD's sys/cdefs.h's _Static_assert handling vs. g++ compilers for C++ code that ties to use _Static_assert, even in contexts where libcxxrt/gaurd.cc is not involved:

# more main.cc
#include "/usr/include/sys/cdefs.h"
_Static_assert(1,"Test");
int main(void)
{
    return 0;
}

For example:

# g++49 main.cc
main.cc:2:15: error: expected constructor, destructor, or type conversion before '(' token
 _Static_assert(1,"Test");
               ^
This is also true with the include commented out as well.

g++49, g++5, and powerpc64-portbld-freebsd11.0-g++ all reject the above source the same way that libcxxrt/guard.cc compiles are rejected during powerpc64-portbld-freebsd11.0-g++ based buildworld lib32 -m32 compiles. 

gcc49, gcc5, and powerpc64-portbld-freebsd11.0-gcc all accept the above instead (when in main.c instead of main.cc so it is handle as C code), with or without the include. _Static_assert is specific to C11 and is not part of C++. It takes explicit definitions to make the syntax acceptable as C++.

Note: clang++ (3.7) accepts the use of the C11 _Static_assert, with or without the include, going well outside the C++ language definition.

With that as the simplified-context the FreeBSD details involved in the buildworld failures are:

The sys/cdefs.h include is not (re-)defining the _Static_assert notation for C++ compiles by g++ compilers to translate to C++ notation or to any syntax acceptable to the g++ compilers. The following sys/cdefs.h code seems to assume that if _Static_assert is supported for gcc it is also supported for g++, which is not currently the case because of the distinct languages involved.

#if !__has_extension(c_static_assert)
#if (defined(__cplusplus) && __cplusplus >= 201103L) || \
    __has_extension(cxx_static_assert)
#define _Static_assert(x, y)    static_assert(x, y)
#elif __GNUC_PREREQ__(4,6)
/* Nothing, gcc 4.6 and higher has _Static_assert built-in */
#elif defined(__COUNTER__)
#define _Static_assert(x, y)    __Static_assert(x, __COUNTER__)
#define __Static_assert(x, y)   ___Static_assert(x, y)
#define ___Static_assert(x, y)  typedef char __assert_ ## y[(x) ? 1 : -1] \
                                __unused
#else
#define _Static_assert(x, y)    struct __hack
#endif

Why a C++ source is using a C11-only declaration syntax instead of a C++ syntax I do not know. But as long as FreeBSD does such it would appear that the above code was supposed to provide a translation to C++ syntax --or for g++ to a g++ compatible syntax. Otherwise libcxxrt/guard.cc just needs to be fixed to be C++ compliant (spanning fairly modern clang++ and g++).

The libcxxrt/guard.cc non-arm/non-LP64/non-little-endian context comes from the #if/#elif/. . . structure:

#ifdef __arm__
. . .
#elif defined(_LP64)
. . .
#else
typedef uint32_t guard_lock_t;
#       if defined(__LITTLE_ENDIAN__)
. . .
#       else
typedef struct {
        uint32_t init_half;
        uint32_t lock_half;
} guard_t;
_Static_assert(sizeof(guard_t) == sizeof(uint64_t), "");
static const uint32_t LOCKED = 1;
static const uint32_t INITIALISED = static_cast<guard_lock_t>(1) << 24;
#       endif
#define LOCK_PART(guard) (&(guard)->lock_half)
#define INIT_PART(guard) (&(guard)->init_half)
#endif

An example of the buildworld failure that reached the _Static_assert is:

--- guard.po ---
/usr/local/bin/powerpc64-portbld-freebsd11.0-g++ -m32 -mcpu=powerpc . . .   -c /usr/src/lib/libcxxrt/../../contrib/libcxxrt/guard.cc -o guard.po
. . .
--- guard.po ---
/usr/src/lib/libcxxrt/../../contrib/libcxxrt/guard.cc:104:15: error: expected constructor, destructor, or type conversion before '(' token
 _Static_assert(sizeof(guard_t) == sizeof(uint64_t), "");
Comment 1 Mark Millard 2015-12-20 08:31:43 UTC
I should also have noted that clang++ can report the __Static_assert syntax as not-C++ syntax but C11 instead:

# clang++ -Wall -pedantic main.cc
main.cc:2:1: warning: _Static_assert is a C11-specific feature [-Wc11-extensions]
_Static_assert(1,"Test");
^
1 warning generated.
Comment 2 Dimitry Andric freebsd_committer freebsd_triage 2015-12-20 16:29:47 UTC
Hm, this _Static_assert has an interesting history.  The original review from Baptiste, https://reviews.freebsd.org/D1390, used static_assert(), but this required -std=c++11 to compile, otherwise you would get:

contrib/libcxxrt/guard.cc:104:1: error: C++ requires a type specifier for all declarations
static_assert(sizeof(guard_t) == sizeof(uint64_t), "");
^~~~~~~~~~~~~

This is the version upstream eventually also used, since they apparently assume C++11 there.  David suggested changing it to _Static_assert(): "This should work if you change it to _Static_assert, which I think we support for all C/C++ versions."

Now that I look at the code again, I am not entirely sure why the static assertion is only for the big endian #ifdef block.  It would seem more useful to put it a few lines lower, for the !_LP64 case.

That said, even when moving the _Static_assert() like that, it compiles fine for me, both with base gcc, and several versions of ports gcc (I tried gcc 4.8, 4.9 and 5.2).

On the other hand, your sample program indeed does not compile with the ports versions of gcc.  I'm not sure where those versions are getting their version of _Static_assert() from, though...
Comment 3 Mark Millard 2015-12-20 16:56:40 UTC
As for where _Static_assert and static_assert are gotten from:

_Static_assert in C11 and static_assert in C++11 exist even for free-standing implementations. As I understand it is even stronger: no explicit headers should be required unless the notation from one language is being used in the other (ignoring simulating for older versions of languages). The handling should be automatic/built-in even for source with no #includes involved.
Comment 4 Mark Millard 2015-12-20 18:03:12 UTC
FYI: My src.conf in use for the failing powerpc64-gcc "X" toolchain based buildworld and buildkernel (with gcc49 acting as the host toolchain) was:

# more ~/src.configs/src.conf.powerpc64-xtoolchain.powerpc64-host 
KERNCONF=GENERIC64vtsc-NODEBUG
TARGET=powerpc
.if ${.MAKE.LEVEL} == 0
TARGET_ARCH=powerpc64
.export TARGET_ARCH
.endif
WITHOUT_CROSS_COMPILER=
WITHOUT_CLANG_EXTRAS=
WITH_FAST_DEPEND=
WITH_LIBCPLUSPLUS=
WITH_LIB32=
WITH_BOOT=
WITH_CLANG=
WITH_CLANG_IS_CC=
WITH_CLANG_FULL=
WITH_LLDB=
WITHOUT_GCC=
WITHOUT_GNUCXX=
NO_WERROR=
MALLOC_PRODUCTION=
WITH_DEBUG=
WITH_DEBUG_FILES=
CROSS_TOOLCHAIN=powerpc64-gcc
.if ${.MAKE.LEVEL} == 0
CC=/usr/local/bin/gcc49
CXX=/usr/local/bin/g++49
CPP=/usr/local/bin/cpp49
.export CC
.export CXX
.export CPP
.endif

So WITH_LIBCPLUSPLUS. make.conf was empty. gcc49/g++49 had been built without 32 bit (lib32) support. gcc49 built powerpc64-gcc's update; the older powerpc64-gcc built gcc49. gcc 4.2.1 is/was not present.

The kernel configuration turns on both vt and sc and turns off ps3.
Comment 5 Dimitry Andric freebsd_committer freebsd_triage 2016-03-27 22:32:51 UTC
Fixed in r297299.