[v3,2/7] malloc/obstack.h: Assume GCC supports __extension__

Message ID be3612db57e27055384fbd7ef32223cf358d721f.1762784073.git.alx@kernel.org (mailing list archive)
State New
Headers
Series Assume certain compiler features are available |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Alejandro Colomar Nov. 10, 2025, 2:17 p.m. UTC
  GCC supports __extension__ since at least GCC 2.8.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 malloc/obstack.h | 4 ----
 1 file changed, 4 deletions(-)
  

Comments

Paul Eggert Nov. 10, 2025, 4:39 p.m. UTC | #1
On 2025-11-10 06:17, Alejandro Colomar wrote:
> -# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
> -#  define __extension__
> -# endif

If we're dropping support for GCC 2.7 and earlier in public-facing 
include files, this should be accompanied by an item in NEWS.

Also, there should be a section in the manual that says what user 
compilers are supported. For GCC we can simply say something like "2.8 
or later"; for other compilers we needn't list specific compilers or 
versions, merely what C features are assumed. I suggest doing this by 
uncommenting the last paragraph in maint.texi (and thus bringing back an 
old compatibility appendix) and bringing it up to date. For the patches 
in this series, the following features should be documented in that part 
of the manual:

* #warning
* statement expressions (assuming we insist on them)
* long long and associated macros like LLONG_MAX
* a stdarg.h that defines va_list

Doing this documentation would perhaps be the most painful and important 
part of the whole exercise.
  
Joseph Myers Nov. 10, 2025, 6:32 p.m. UTC | #2
On Mon, 10 Nov 2025, Alejandro Colomar wrote:

> GCC supports __extension__ since at least GCC 2.8.

This is a file shared with gnulib, but __extension__ is generally only 
relevant with -pedantic; there's no problem with supporting its absence 
with non-GCC compilers, so no real advantage in removing this code.

In fact, GCC 2.7 (the oldest version supporting ELF for any target 
supported now by glibc) *does* support __extension__, so the more 
interesting question about such a condition is whether there are specific 
sub-cases of __extension__ that are used in installed headers and are the 
reason for the 2.8 version here.
  
Joseph Myers Nov. 10, 2025, 6:47 p.m. UTC | #3
On Mon, 10 Nov 2025, Paul Eggert wrote:

> Also, there should be a section in the manual that says what user compilers
> are supported. For GCC we can simply say something like "2.8 or later"; for
> other compilers we needn't list specific compilers or versions, merely what C
> features are assumed. I suggest doing this by uncommenting the last paragraph
> in maint.texi (and thus bringing back an old compatibility appendix) and
> bringing it up to date. For the patches in this series, the following features
> should be documented in that part of the manual:
> 
> * #warning
> * statement expressions (assuming we insist on them)
> * long long and associated macros like LLONG_MAX
> * a stdarg.h that defines va_list
> 
> Doing this documentation would perhaps be the most painful and important part
> of the whole exercise.

Note there are separate considerations here:

* For users: certain language features (on top of C90 / C++98) are 
required to use the headers, or certain features in the headers.  In some 
cases, the language feature is only needed for a very specific, limited 
part of the headers (for example, <complex.h> requires __builtin_complex 
for the CMPLX macros, but everything else in the installed headers should 
work without that feature).

This does *not* need to mention features only relevant for optimizations 
or extra diagnostics, because the relevant parts of the headers can be 
conditional without affecting the API provided to users.  It's only 
language / compiler features that are needed to provide a particular 
public API that are relevant here.  On that basis, I don't see any need to 
require #warning, and I doubt there's a need for statement expressions 
(but maybe a detailed analysis could show some feature to be hard to 
provided in the headers without them).

The interactions between compiler and library headers (e.g. 
__need___va_list) are a tricky sub-part of this.  I think the aim in such 
cases is that the fallback, if the header doesn't support the __need_* 
protocol from the GCC headers, is that the glibc headers will still work 
but might expose more declarations from the compiler headers than that 
particular header is meant to (e.g. __need___va_list is meant to avoid 
exposing names such as va_list, just declaring __gnuc_va_list instead).

* For glibc development: there is a *rationale* for each feature for why 
some things are problematic to support without that feature, typically for 
ABI reasons.  Documenting that makes sense as well, but for glibc 
developers rather than for end users as the target audience.
  
Alejandro Colomar Nov. 10, 2025, 11:05 p.m. UTC | #4
Hi Paul,

On Mon, Nov 10, 2025 at 08:39:03AM -0800, Paul Eggert wrote:
> On 2025-11-10 06:17, Alejandro Colomar wrote:
> > -# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
> > -#  define __extension__
> > -# endif
> 
> If we're dropping support for GCC 2.7 and earlier in public-facing include
> files, this should be accompanied by an item in NEWS.
> 
> Also, there should be a section in the manual that says what user compilers
> are supported. For GCC we can simply say something like "2.8 or later"; for
> other compilers we needn't list specific compilers or versions, merely what
> C features are assumed. I suggest doing this by uncommenting the last
> paragraph in maint.texi (and thus bringing back an old compatibility
> appendix) and bringing it up to date. For the patches in this series, the
> following features should be documented in that part of the manual:
> 
> * #warning

I'll discard this, as if affects gnulib.

> * statement expressions (assuming we insist on them)

I'll drop this for now.

> * long long and associated macros like LLONG_MAX

Yup, I'll document this.  I'll start with long long, which I guess has
less controversy.

> * a stdarg.h that defines va_list

The relevant change is:

	 #if defined __USE_XOPEN2K || defined __USE_XOPEN2K8
	-# ifdef __GNUC__
	-#  ifndef _VA_LIST_DEFINED
	-typedef __gnuc_va_list va_list;
	-#   define _VA_LIST_DEFINED
	-#  endif
	-# else
	+# ifndef __GNUC__
	 #  include <stdarg.h>
	 # endif
	 #endif

I'm only changing the assumption if __GNUC__ is defined.  This change
assumes that GCC provides va_list in its own header.  However, I'm not
changing any assumptions for non-__GNUC__ compilers.

> Doing this documentation would perhaps be the most painful and important
> part of the whole exercise.


Have a lovely night!
Alex
  
Alejandro Colomar Nov. 10, 2025, 11:31 p.m. UTC | #5
Hi Joseph,

On Mon, Nov 10, 2025 at 06:32:38PM +0000, Joseph Myers wrote:
> On Mon, 10 Nov 2025, Alejandro Colomar wrote:
> 
> > GCC supports __extension__ since at least GCC 2.8.
> 
> This is a file shared with gnulib, but __extension__ is generally only 
> relevant with -pedantic; there's no problem with supporting its absence 
> with non-GCC compilers, so no real advantage in removing this code.
> 
> In fact, GCC 2.7 (the oldest version supporting ELF for any target 
> supported now by glibc) *does* support __extension__, so the more 
> interesting question about such a condition is whether there are specific 
> sub-cases of __extension__ that are used in installed headers and are the 
> reason for the 2.8 version here.

git-blame(1) in gnulib points to this:

	commit 6421fd46004655f39afca254ad02b72ab97f5ecb
	Author: Paul Eggert <eggert@cs.ucla.edu>
	Date:   Thu Mar 27 13:38:41 2014 -0700

	    obstack: Remove ancient NeXTSTEP gcc support conditional
	    
	    This change will ease merging with glibc.  The "#if ... __NEXT__"
	    causes a warning with -Wundef which glibc now enables by default.
	    Problem reported by Will Newton in
	    <http://lists.gnu.org/archive/html/bug-gnulib/2014-03/msg00032.html>.
	    glibc <sys/cdefs.h> now uses __extension__ for GCC 2.8 or later,
	    so go with that.
	    * lib/obstack.h (__extension__):

	[...]

	diff --git a/lib/obstack.h b/lib/obstack.h
	index f847a53706..f92492fc68 100644
	--- a/lib/obstack.h
	+++ b/lib/obstack.h
	@@ -258,10 +258,7 @@ extern int obstack_exit_failure;
	 #define obstack_memory_used(h) _obstack_memory_used (h)
	 
	 #if defined __GNUC__
	-/* NextStep 2.0 cc is really gcc 1.93 but it defines __GNUC__ = 2 and
	-   does not implement __extension__.  But that compiler doesn't define
	-   __GNUC_MINOR__.  */
	-# if __GNUC__ < 2 || (__NeXT__ && !__GNUC_MINOR__)
	+# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
	 #  define __extension__
	 # endif

That commit message probably refers to this in glibc:

	$ cat ./misc/sys/cdefs.h | grep -C2 __extension__
	/* It is possible to compile containing GCC extensions even if GCC is
	   run in pedantic mode if the uses are carefully marked using the
	   `__extension__' keyword.  But this is not generally available before
	   version 2.8.  */
	#if !(__GNUC_PREREQ (2,8) || defined __clang__)
	# define __extension__		/* Ignore */
	#endif

However, it seems to ignore this:

	commit bc244b778ac3e17089454050ac86662ad51f7fd0
	Author: Jim Meyering <jim@meyering.net>
	Date:   Fri Nov 4 16:50:15 1994 +0000

	    merge with 3.9u1

	[...]
	diff --git a/lib/obstack.h b/lib/obstack.h
	index 01767195da..840a63baaa 100644
	--- a/lib/obstack.h
	+++ b/lib/obstack.h
	[...]
	@@ -269,7 +269,10 @@ int obstack_chunk_size (struct obstack *obstack);
	 #define obstack_blank_fast(h,n) ((h)->next_free += (n))
	 ^L
	 #if defined (__GNUC__) && defined (__STDC__)
	-#if __GNUC__ < 2
	+/* NextStep 2.0 cc is really gcc 1.93 but it defines __GNUC__ = 2 and
	+   does not implement __extension__.  But that compiler doesn't define
	+   __GNUC_MINOR__.  */
	+#if __GNUC__ < 2 || !__GNUC_MINOR__
	 #define __extension__
	 #endif
	 
Which claims that GCC 2 is fine with __extension__.

The commit that added the 2.8 check in <sys/cdefs.h> is
8325d82cf391 (1998-10-05; "Update."), whose ChangeLog only says:

    1998-10-05  Ulrich Drepper  <drepper@cygnus.com>
    
            * misc/sys/cdefs.h (__extension__): Define as empty if no gcc or
            gcc before 2.8 is used.

so, I don't really know why Ulrich did that.  There's some possibility
that he was wrong.  Or maybe it was really not widely available on some
GCC forks that claimed different __GNUC__ and __GNUC_MINOR__ values.
Who knows.


Have a lovely night!
Alex
  
Joseph Myers Nov. 11, 2025, 4:55 p.m. UTC | #6
On Tue, 11 Nov 2025, Alejandro Colomar wrote:

> > * a stdarg.h that defines va_list
> 
> The relevant change is:
> 
> 	 #if defined __USE_XOPEN2K || defined __USE_XOPEN2K8
> 	-# ifdef __GNUC__
> 	-#  ifndef _VA_LIST_DEFINED
> 	-typedef __gnuc_va_list va_list;
> 	-#   define _VA_LIST_DEFINED
> 	-#  endif
> 	-# else
> 	+# ifndef __GNUC__
> 	 #  include <stdarg.h>
> 	 # endif
> 	 #endif
> 
> I'm only changing the assumption if __GNUC__ is defined.  This change
> assumes that GCC provides va_list in its own header.  However, I'm not
> changing any assumptions for non-__GNUC__ compilers.

I think this change (to <stdio.h>) is actually incorrect for modern GCC 
(indeed, I don't see how the patched glibc would have passed the conform/ 
tests).

Early on, the header does:

#define __need___va_list
#include <stdarg.h>

The point of that is, if the __need_* protocol is working correctly, to 
define __gnuc_va_list but *not* va_list (because ISO C, for example, does 
not allow <stdio.h> to define va_list, despite having interfaces such as 
vprintf that involve va_list in their types).  So with GCC, or compatible 
compilers, __gnuc_va_list will be defined, but not va_list - unless the 
user had previously included <stdarg.h> themself without __need___va_list 
defined.

But then there is the POSIX requirement (CX-tagged in the 2008 and 2024 
editions, XSI-tagged in the 2004 edition) to define va_list in <stdio.h>.  
That's what the code you're changing implements.  And your change would 
cause <stdio.h> to fail to implement that requirement with current GCC.
  

Patch

diff --git a/malloc/obstack.h b/malloc/obstack.h
index 7e3dee21c8..1c74d85c9b 100644
--- a/malloc/obstack.h
+++ b/malloc/obstack.h
@@ -258,10 +258,6 @@  extern int obstack_exit_failure;
 #define obstack_memory_used(h) _obstack_memory_used (h)
 
 #if defined __GNUC__
-# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
-#  define __extension__
-# endif
-
 /* For GNU C, if not -traditional,
    we can define these macros to compute all args only once
    without using a global variable.