Esp-idf: Warning from newlib/include/sys/cdefs.h

[paulreimer]

As discussed in this issue (https://github.com/dvidelabs/flatcc/issues/77#issuecomment-363529004), when compiling c files:

In file included from ./lib/flatcc/include/flatcc/flatcc_flatbuffers.h:24:0,
                 from ./lib/flatcc/include/flatcc/flatcc_builder.h:68,
                 from ./lib/flatcc/src/runtime/builder.c:21:
./lib/flatcc/include/flatcc/portable/pstdalign.h:48:0: warning: "_Alignas" redefined
 #define _Alignas(t) __attribute__((__aligned__(t)))
 ^
In file included from /esp-idf/components/newlib/include/stdlib.h:19:0,
                 from ./lib/flatcc/src/runtime/builder.c:16:
/esp-idf/components/newlib/include/sys/cdefs.h:281:0: note: this is the location of the previous definition
 #define _Alignas(x)  __aligned(x)
 ^
In file included from ./lib/flatcc/include/flatcc/flatcc_flatbuffers.h:24:0,
                 from ./lib/flatcc/include/flatcc/flatcc_builder.h:68,
                 from ./lib/flatcc/src/runtime/builder.c:21:
./lib/flatcc/include/flatcc/portable/pstdalign.h:49:0: warning: "_Alignof" redefined
 #define _Alignof(t) __alignof__(t)
 ^
In file included from /esp-idf/components/newlib/include/stdlib.h:19:0,
                 from ./lib/flatcc/src/runtime/builder.c:16:
/esp-idf/components/newlib/include/sys/cdefs.h:288:0: note: this is the location of the previous definition
 #define _Alignof(x)  __alignof(x)
 ^
CC build/request_manager/lib/flatcc/src/runtime/json_parser.o
In file included from ./lib/flatcc/include/flatcc/flatcc_flatbuffers.h:24:0,
                 from ./lib/flatcc/include/flatcc/flatcc_builder.h:68,
                 from ./lib/flatcc/include/flatcc/flatcc_json_parser.h:20,
                 from ./lib/flatcc/src/runtime/json_parser.c:2:
./lib/flatcc/include/flatcc/portable/pstdalign.h:48:0: warning: "_Alignas" redefined
 #define _Alignas(t) __attribute__((__aligned__(t)))

And the esp-idf file at which has the other definition:
https://github.com/espressif/esp-idf/blob/master/components/newlib/include/sys/cdefs.h#L273-L289

The library author suggests this is an esp-idf issue:

it is an issue with cdefs because it doesn't check if __alignas_is_defined is defined
https://github.com/dvidelabs/flatcc/issues/77#issuecomment-363530016

And I guess that file needs to be modified to #define __alignas_is_defined, and also check if it is already defined to avoid redefining?

[projectgus]

The underscore prefix is reserved (by the C standard) for compiler & system libraries. Newlib is the system C library for ESP-IDF.

We can't stop application code from defining names with underscore prefixes. However the point of having a reserved namespace is that the system can define these names without having to do checks for use of these names outside the compiler & system libs.

I had a look at the issue and I understand that the flatcc author is working around some stdlibs which don't set some of these macros. From a quick look, I think the best solution would be that pstdalign.h should directly include system stdalign.h, which will define whatever the system libc defines on that platform. After this in pstdalign.h it's then safe to use #ifndef checks to define any macros that may be missing in that system stdlib implementation, without the possibility of stepping on the system libc's toes if someone includes stdalign later on in the same compilation unit (which looks like what is happening here).

Note this isn't specific to ESP-IDF, upstream newlib has this behaviour so any newlib-based system will experience this:
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/cdefs.h;h=fc564a5c602be4d328e525dd5f19e4ee58bb4406;hb=HEAD#l280

(Maybe glibc doesn't do this, I can only guess why or why not...)

[projectgus]

Feel free to reopen if you think I'm misinterpreting the C standard or the problem here.

However in general if there's a conflict between the compiler & system libraries and a non-system library over an underscore-prefixed name, for better or worse it's the non-system library which has to adapt to this.

[paulreimer]

OK thanks for your attention! I will mention your suggestions back to the flatcc maintainer.

FWIW I am able to workaround the problem in the interim by adding to CFLAGS for the specific objects: CFLAGS += -D__alignas_is_defined=1. But I am interested in a cleaner solution.

[projectgus]

OK, I'm glad to hear you have a workaround for now!

[mikkelfj]

The thing is the pstdalign.h does include stdalign.h but it cannot do so when it cannot be sure that it is present - in that case it falls back to the library provided defines designed to handle this case.

In my view pstdalign.h, part of the portable library, does take the form of a limited system library, otherwise it couldn't do what it does - provide missing library functionality.

If newlib is sufficiently popular it could be identified by pstdalign, but it is already difficult enough to support broken glibc in different versions and C++ changing behaviour every third year or so. As a fallback users can opt out defining a suitable macro which in this case is __alignas_is_defined.

[projectgus]

Hi @mikkelfj ,

Thanks for weighing in here with your perspective on the matter.

does include stdalign.h but it cannot do so when it cannot be sure that it is present

I had another look at pstdalign.h and I see the predicament now.

(BTW one thing I noticed is that gcc 5 or newer seems to always have a C11 compliant stdalign.h bundled with it, but maybe there's an exception to that lurking out there...)

Defining __alignas_is_defined in our libc will not solve this problem, at least without breaking otherwise correct code. The C11 standard says that __alignas_is_defined should be defined in stdalign.h. And for gcc it is: https://github.com/gcc-mirror/gcc/blob/da8dff89fa9398f04b107e388cb706517ced9505/gcc/ginclude/stdalign.h#L26 (same header as our gcc 5.2.0). Maybe we could also include stdalign.h from cdefs.h, but this may have other unintended consequences.

@PerMalmberg , if the workaround you have shows any problems then instead append -std=c11 to the CFLAGS for building the flatcc component only. This should work, as it looks like flatcc's portability library headers are only needed by the implementation not the public interface.

[mikkelfj]

Another workaround might be for the end-user to include stdalign.h before flatcc headers when it is sure that header is present. This still only works if __alignas_is_defined is indeed defined in that header.

[PerMalmberg]

@projectgus Thanks for thinking about me, but I think you meant to tag @paulreimer

[projectgus]

@mikkelfj Yes, __alignas_is_defined is defined there by gcc. See the comment above yours (with a link to the header).

Are the "portability" headers required for using the flatcc library (ie included from the public interface headers), or only for compiling the library?

@PerMalmberg oops, sorry!

[mikkelfj]

@projectgus I'm not sure that it is in general, but for some versions yes. The problem pstdalign.h originally solved was that clang defined _Alignas as a builtin with std=c11, but glibc stdalign.h did not define alignas or alignof. I don't recall the entire history, but I could imagine gcc defining neither _Alignas nor alignas until recent versions, which are being tested for in pstdalign.h. So if you have on older version of gcc/glibc, it would not solve the problem. But, if newlib always imports a recent glibc stdalign.h header, that might work.

[mikkelfj]

@projectgus I see that newlib is fairly widely used. Do you have any suggestions for how to detect presence of newlib to avoid the exception blocking inclusion of stdalign.h?

https://github.com/dvidelabs/flatcc/blob/775675487889a5bc5a7970493f1025ce5996393e/include/flatcc/portable/pstdalign.h#L19-L23

The detection might require inclusion of a header, but in that case it should be standard header such as

[projectgus]

Yes, newlib is pretty widely used.

A few points which may help you figure something out:

• Neither glibc nor newlib include stdalign.h. This is a toolchain header, it comes from the gcc sources (as per above link) - it's not a file in the glibc source tree or the newlib source tree. AFAIK whatever libc gcc is built against, stdalign.h will come from gcc itself.
• As far as I can tell, gcc 4.7 and newer include some kind of stdalign.h (mentioned in release notes). In recent gcc (from 5, I think) it is definitely C11-compliant, meaning it defines the macros listed in the standard - alignas, alignof, __alignas_is_defined and __alignof_is_defined. I haven't looked at gcc 4.7 source to see what it's like there.
• gcc 5 and newer include a macro, __has_include, which can be used to determine if a given header exists. This is also supported in Clang. You can test for existence of the feature itself with #ifdef __has_include.
• On newlib, including unistd.h or string.h (among others) will pull in sys/features.h which will define __NEWLIB__ (newlib major version) and __NEWLIB_MINOR__ (newlib minor version). I'm not sure this helps you though, because existence of stdalign.h will depend on the gcc version not the newlib version...

So, to answer your question, I would actually suggest something like:

#if !defined(__cplusplus) && ((__GNUC__ > 4) || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7)))
#include <stdalign.h>
#endif

#if !__alignas_is_defined
// deficient stdalign.h, do some workaround
#endif

#if !__alignof_is_defined
// deficient stdalign.h, do some other workaround
#endif

Above should work on any libc for gcc 4.7+. Older gcc versions you already set PORTABLE_C11_STDALIGN_MISSING. For Clang, maybe try checking for __has_include...?

[mikkelfj]

Thanks for the pointers, that is informative. I believe I already do what you suggest though, except stdaligh.h inclusion also requires stdc=c11. Perhaps it suffice to remove the C11 requirement for the GCC case after checking version.

[projectgus]

Perhaps it suffice to remove the C11 requirement for the GCC case after checking version.

I think that will probably be enough.

[mikkelfj]

Thanks, FYI I added the tests early in the file similar to C++14 handling and also added #ifndef around _Alignas and friends, and added the option to define HAVE_STDALIGN_H
https://github.com/dvidelabs/flatcc/blob/issue-77/include/flatcc/portable/pstdalign.h

[projectgus]

Looks pretty comprehensive!

Note that due to preprocessor awkwardness you can't write this:
#elif defined(__has_include) && __has_include(<stdalign.h>)

The preprocessor tries to expand the whole line at once and then evaluate it, and NONEXISTENTMACRO(X) is an expansion error. So it will error if !defined(__has_include)

[mikkelfj]

NONEXISTENTMACRO

Never encountered that problem, but will look into it, thanks.

[mikkelfj]

Never encountered that problem, but will look into it, thanks.

Well Travis just failed on that, and the suggested change made it work again!

[mikkelfj]

Changes integrated into flatcc master branch and confirmed to resolve the issue.

[paulreimer]

You guys are awesome