Re: [PATCH] features.h: Fix -Wundef problems
Stefan Tauner <[email protected]> Wed, 3 Dec 2025 03:48:22 +0100
| Newsgroups | gmane.comp.lib.newlib |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 1 Dec 2025 16:49:45 +0100 Corinna Vinschen <[email protected]> wrote: > GLibc adds `defined', but retains the `(MACRO - 0)' expression, even > today, 10 years later. I didn't test it, but shouldn't we do the same? Thanks for bringing this up. Initially I thought that this idiom is just for very old compilers that do not replace undefined macros with 0. However, the real reason seems to be to allow for comparing macros that *are* defined but without value. In such cases (MACRO - 0) decays to -0 which still can be used as operand in a comparison while just MACRO would decay to nil leading to syntax errors. Since defining some of the macros in question without value is actually legal and useful the first version of my patch breaks those. This is for example the case for -D_XOPEN_SOURCE= (NB: the = is important. Without it gcc defines the macro to 1(!)). I also found some embarrassing copy and paste mistakes that I also fixed. Additionally, I have used the idiom for the usages of _FORTIFY_SOURCE and __OPTIMIZE__ that did not have used it previously. __OPTIMIZE__ is set by the compilers if optimization is enabled. gcc and clang set it to 1 in that case regardless of the optimization level (including -Os and -Og). The GCC documentation explicitly states that it is defined with a value of 1. _FORTIFY_SOURCE on the other hand is set by the user. GCC documentation defines the legal values 1 to 3, inclusive. The current code would bail out for "-D_FORTIFY_SOURCE=". So arguably the idiom is overkill for those two but OTOH I don't see any disadvantage either so I left them in for now. V2 is attached. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
0001-features.h-Fix-Wundef-problems.patch
(text/x-patch, 5.4 KB)
From 4667f7d3f13cfc051db048efbf1636ecc25d2532 Mon Sep 17 00:00:00 2001 From: Stefan Tauner <[email protected]> Date: Sat, 29 Nov 2025 22:52:26 +0100 Subject: [PATCH] features.h: Fix -Wundef problems -Wundef warns if an undefined identifier is evaluated in an #if directive. This would be valid as they are replaced with 0. However, it is often an early warning sign and not intentional. To allow for enabling -Wundef even outside system directories (where compilers ignore such problems unless -Wsystem-headers is enabled) this patch adds the required defined() checks. glibc also has been supporting this for 10 years now: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=f248238cf43bd751db29e6f151d6da7645337ff5 I have not exhaustively tested this but you can see the effect with something like the following (+ using -stdc= and/or -D...): echo | gcc -include newlib/libc/include/sys/features.h -Inewlib/libc/include/ -E - -Wundef Signed-off-by: Stefan Tauner <[email protected]> --- newlib/libc/include/sys/features.h | 49 +++++++++++++++++------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/newlib/libc/include/sys/features.h b/newlib/libc/include/sys/features.h index f0f5286ac..195300eb5 100644 --- a/newlib/libc/include/sys/features.h +++ b/newlib/libc/include/sys/features.h @@ -156,15 +156,15 @@ extern "C" { #if !defined(_POSIX_SOURCE) && !defined(_POSIX_C_SOURCE) && \ ((!defined(__STRICT_ANSI__) && !defined(_ANSI_SOURCE)) || \ - (_XOPEN_SOURCE - 0) >= 500) + (defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) >= 500)) #define _POSIX_SOURCE 1 #if !defined(_XOPEN_SOURCE) || (_XOPEN_SOURCE - 0) >= 700 #define _POSIX_C_SOURCE 200809L -#elif (_XOPEN_SOURCE - 0) >= 600 +#elif defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) >= 600 #define _POSIX_C_SOURCE 200112L -#elif (_XOPEN_SOURCE - 0) >= 500 +#elif defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) >= 500 #define _POSIX_C_SOURCE 199506L -#elif (_XOPEN_SOURCE - 0) < 500 +#elif defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) < 500 #define _POSIX_C_SOURCE 2 #endif #endif @@ -286,19 +286,24 @@ extern "C" { #endif #if defined(_ISOC23_SOURCE) || \ - (__STDC_VERSION__ - 0) > 201710L || (__cplusplus - 0) >= 202002L + (defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0) > 201710L) || \ + (defined(__cplusplus) && (__cplusplus - 0) >= 202002L) #define __ISO_C_VISIBLE 2020 #elif defined(_ISOC11_SOURCE) || \ - (__STDC_VERSION__ - 0) >= 201112L || (__cplusplus - 0) >= 201103L + (defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0) >= 201112L) || \ + (defined(__cplusplus) && (__cplusplus - 0) >= 201103L) #define __ISO_C_VISIBLE 2011 -#elif defined(_ISOC99_SOURCE) || (_POSIX_C_SOURCE - 0) >= 200112L || \ - (__STDC_VERSION__ - 0) >= 199901L || defined(__cplusplus) +#elif defined(_ISOC99_SOURCE) || \ + (defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE - 0) >= 200112L) || \ + (defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0) >= 199901L) || \ + defined(__cplusplus) #define __ISO_C_VISIBLE 1999 #else #define __ISO_C_VISIBLE 1990 #endif -#if defined(_LARGEFILE_SOURCE) || (_XOPEN_SOURCE - 0) >= 500 +#if defined(_LARGEFILE_SOURCE) || \ + (defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) >= 500) #define __LARGEFILE_VISIBLE 1 #else #define __LARGEFILE_VISIBLE 0 @@ -310,17 +315,18 @@ extern "C" { #define __MISC_VISIBLE 0 #endif -#if (_POSIX_C_SOURCE - 0) >= 202405L +#if defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE - 0) >= 202405L #define __POSIX_VISIBLE 202405 -#elif (_POSIX_C_SOURCE - 0) >= 200809L +#elif defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE - 0) >= 200809L #define __POSIX_VISIBLE 200809 -#elif (_POSIX_C_SOURCE - 0) >= 200112L +#elif defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE - 0) >= 200112L #define __POSIX_VISIBLE 200112 -#elif (_POSIX_C_SOURCE - 0) >= 199506L +#elif defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE - 0) >= 199506L #define __POSIX_VISIBLE 199506 -#elif (_POSIX_C_SOURCE - 0) >= 199309L +#elif defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE - 0) >= 199309L #define __POSIX_VISIBLE 199309 -#elif (_POSIX_C_SOURCE - 0) >= 2 || defined(_XOPEN_SOURCE) +#elif (defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE - 0) >= 2) || \ + defined(_XOPEN_SOURCE) #define __POSIX_VISIBLE 199209 #elif defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) #define __POSIX_VISIBLE 199009 @@ -334,11 +340,11 @@ extern "C" { #define __SVID_VISIBLE 0 #endif -#if (_XOPEN_SOURCE - 0) >= 700 +#if defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) >= 700 #define __XSI_VISIBLE 700 -#elif (_XOPEN_SOURCE - 0) >= 600 +#elif defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) >= 600 #define __XSI_VISIBLE 600 -#elif (_XOPEN_SOURCE - 0) >= 500 +#elif defined(_XOPEN_SOURCE) && (_XOPEN_SOURCE - 0) >= 500 #define __XSI_VISIBLE 500 #elif defined(_XOPEN_SOURCE) && defined(_XOPEN_SOURCE_EXTENDED) #define __XSI_VISIBLE 4 @@ -348,9 +354,10 @@ extern "C" { #define __XSI_VISIBLE 0 #endif -#if _FORTIFY_SOURCE > 0 && !defined(__cplusplus) && !defined(__lint__) && \ - (__OPTIMIZE__ > 0 || defined(__clang__)) && __GNUC_PREREQ__(4, 1) && \ - !defined(_LIBC) +#if defined(_FORTIFY_SOURCE) && (_FORTIFY_SOURCE - 0) > 0 && \ + !defined(__cplusplus) && !defined(__lint__) && \ + (defined(__OPTIMIZE__) && (__OPTIMIZE__ - 0) > 0 || defined(__clang__)) && \ + __GNUC_PREREQ__(4, 1) && !defined(_LIBC) # if _FORTIFY_SOURCE > 2 && defined(__has_builtin) # if __has_builtin(__builtin_dynamic_object_size) # define __SSP_FORTIFY_LEVEL 3 -- Kind regards, Stefan Tauner