Bug 107795 - <limits.h> recursion through <syslimits.h> breaks non-GNU implementations of the C++ stdlib
Summary: <limits.h> recursion through <syslimits.h> breaks non-GNU implementations of ...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: bootstrap (show other bugs)
Version: 12.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-21 15:59 UTC by Louis Dionne
Modified: 2022-11-22 05:29 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-11-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Louis Dionne 2022-11-21 15:59:28 UTC
The <limits.h> header currently includes "syslimits.h", which then recursively includes <limits.h> after defining the _GCC_NEXT_LIMITS_H macro. The <limits.h> header seems to be generated from a bunch of stuff, including `gcc/limitx.h`, which seems to be responsible for that recursive inclusion. Incidentally, this seems to not have been modified in the last 30 years.

This recursive inclusion with a macro is hostile to implementations that want to build on top of GCC's <limits.h> header, since they need to know about that recursive inclusion trick and the _GCC_NEXT_LIMITS_H macro. For example, libc++ currently needs to know about this wrinkle and implement its own <limits.h> header differently depending on whether it's built with GCC or another compiler.

System headers should work such that they can be layered on top of each other without needing this sort of trick. To reproduce the issue we're seeing with a minimal example:

    #!/usr/bin/env bash

    rm -rf __fake_cxx_include && mkdir -p __fake_cxx_include
    cat <<EOF > __fake_cxx_include/limits.h
    #ifndef __MY_LIMITS_H
    #define __MY_LIMITS_H
    #   include_next <limits.h>
    #endif
    EOF

    cat <<EOF | g++-12 -isystem $PWD/__fake_cxx_include -nostdinc++ -v -fsyntax-only -xc++ - -O2 -H
    #include <limits.h>
    #include <wchar.h>
    EOF

When running this script, the output is (on our Docker image):

    . /llvm/__fake_cxx_include/limits.h
    .. /usr/lib/gcc/x86_64-linux-gnu/12/include/limits.h
    ... /usr/lib/gcc/x86_64-linux-gnu/12/include/syslimits.h
    .... /llvm/__fake_cxx_include/limits.h
    . /usr/include/wchar.h
    [...]
    .. /usr/include/x86_64-linux-gnu/bits/wchar.h
    [...]
    .. /usr/include/x86_64-linux-gnu/bits/wchar2.h
    In file included from /usr/include/wchar.h:867,
                    from <stdin>:2:
    /usr/include/x86_64-linux-gnu/bits/wchar2.h:398:3: error: #error "Assumed value of MB_LEN_MAX wrong"
    398 | # error "Assumed value of MB_LEN_MAX wrong"
        |   ^~~~~

Because this <limits.h> header does not know about the GCC-internal macro _GCC_NEXT_LIMITS_H, we fail to recursively re-include <limits.h> and then including <wchar.h> fails in a rather obscure way. Also note that this only fails if we're using -O2 to compile, since -O2 seems to turn on some __fortify stuff which leads to including <wchar2.h>.

Long story short, it would be great if GCC's <limits.h> header could be simplified to avoid recursively including itself through <syslimits.h>.
Comment 1 Andrew Pinski 2022-11-21 16:07:21 UTC
the limits.h here we are talking about does NOT come from libstdc++ but from gcc/gsyslimits.h :

/* syslimits.h stands for the system's own limits.h file.
   If we can use it ok unmodified, then we install this text.
   If fixincludes fixes it, then the fixed version is installed
   instead of this text.  */

#define _GCC_NEXT_LIMITS_H              /* tell gcc's limits.h to recurse */
#include_next <limits.h>
#undef _GCC_NEXT_LIMITS_H
Comment 2 Andrew Pinski 2022-11-21 16:11:20 UTC
>This recursive inclusion with a macro is hostile to implementations that want to build on top of GCC's <limits.h> header

You should not be building on top of GCC's limits.h header at all really.
Rather implementations should have their own.
Comment 3 Jonathan Wakely 2022-11-21 16:57:57 UTC
(In reply to Andrew Pinski from comment #2)
> Rather implementations should have their own.

Or just use GCC's one without change, which is what libstdc++ does. We don't provide any <limits.h> in libstdc++, only <climits>. When you #include <limits.h> with G++ you just get GCC's own <limits.h> as-is.
Comment 4 Louis Dionne 2022-11-21 18:37:40 UTC
(In reply to Andrew Pinski from comment #2)
> You should not be building on top of GCC's limits.h header at all really.
> Rather implementations should have their own.

What do you mean by "implementations"? Do you mean implementations of the C library or compiler implementations, or what?


(In reply to Jonathan Wakely from comment #3)
> (In reply to Andrew Pinski from comment #2)
> > Rather implementations should have their own.
> 
> Or just use GCC's one without change, which is what libstdc++ does. We don't
> provide any <limits.h> in libstdc++, only <climits>. When you #include
> <limits.h> with G++ you just get GCC's own <limits.h> as-is.

Yeah but we may need to add stuff to <limits.h> on some platforms, so we may need to have such a header. Also, I assume you only do that for a subset of headers, because you must have <foo.h> headers in libstdc++ for a few headers that require adding const-correct overloads of e.g. `memchr`?
Comment 5 Andrew Pinski 2022-11-21 18:48:46 UTC
(In reply to Louis Dionne from comment #4)
> (In reply to Andrew Pinski from comment #2)
> > You should not be building on top of GCC's limits.h header at all really.
> > Rather implementations should have their own.
> 
> What do you mean by "implementations"? Do you mean implementations of the C
> library or compiler implementations, or what?

GCC limits.h is the implementation detail of GCC.
Yes I know it gets fuzzy. This is why GCC even has fixincludes to make sure target headers are always correct. See the comment I posted.
If clang/libc++ wants built on top of GCC's implementation of GCC's C implementation, then it will need to similar tricks as GCC does for other targets. GCC does fixincludes trick to support other targets besides Linux even.

> 
> 
> (In reply to Jonathan Wakely from comment #3)
> > (In reply to Andrew Pinski from comment #2)
> > > Rather implementations should have their own.
> > 
> > Or just use GCC's one without change, which is what libstdc++ does. We don't
> > provide any <limits.h> in libstdc++, only <climits>. When you #include
> > <limits.h> with G++ you just get GCC's own <limits.h> as-is.
> 
> Yeah but we may need to add stuff to <limits.h> on some platforms, so we may
> need to have such a header. Also, I assume you only do that for a subset of
> headers, because you must have <foo.h> headers in libstdc++ for a few
> headers that require adding const-correct overloads of e.g. `memchr`?

Yes and some targets such as solaris even includes some of those inside foo.h ... GCC's libstdc++ had issues with that previous and needed some changes.
Comment 6 Jonathan Wakely 2022-11-21 18:49:44 UTC
(In reply to Louis Dionne from comment #4)
> Yeah but we may need to add stuff to <limits.h> on some platforms, so we may
> need to have such a header. Also, I assume you only do that for a subset of
> headers, because you must have <foo.h> headers in libstdc++ for a few
> headers that require adding const-correct overloads of e.g. `memchr`?

No, we get those overloads from libc (or we don't get them at all).

Sensible libc implementations provide the correct overloads for C++ nowadays, e.g. glibc:

#ifdef __CORRECT_ISO_CPP_STRING_H_PROTO
extern "C++"
{
extern char *strchr (char *__s, int __c)
     __THROW __asm ("strchr") __attribute_pure__ __nonnull ((1));
extern const char *strchr (const char *__s, int __c)
     __THROW __asm ("strchr") __attribute_pure__ __nonnull ((1));


Solaris:

#else /* __cplusplus >= 199711L */
extern void *memchr(const void *, int, size_t);
extern char *strchr(const char *, int);

AIX:

#ifdef __cplusplus98__interface__
        extern const char       *strchr(const char *, int);
        extern char             *strchr(      char *, int);


If even AIX libc can do it, anybody can do it.
Comment 7 Andrew Pinski 2022-11-21 18:50:55 UTC
(In reply to Louis Dionne from comment #4)
> Yeah but we may need to add stuff to <limits.h> on some platforms, so we may
> need to have such a header. Also, I assume you only do that for a subset of
> headers, because you must have <foo.h> headers in libstdc++ for a few
> headers that require adding const-correct overloads of e.g. `memchr`?

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30928 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33935 for those issues there.
Comment 8 Louis Dionne 2022-11-21 18:58:54 UTC
(In reply to Andrew Pinski from comment #5)
> (In reply to Louis Dionne from comment #4)
> > (In reply to Andrew Pinski from comment #2)
> > > You should not be building on top of GCC's limits.h header at all really.
> > > Rather implementations should have their own.
> > 
> > What do you mean by "implementations"? Do you mean implementations of the C
> > library or compiler implementations, or what?
> 
> GCC limits.h is the implementation detail of GCC.
> Yes I know it gets fuzzy. This is why GCC even has fixincludes to make sure
> target headers are always correct. See the comment I posted.

When compiling pure C with GCC, what's the order of includes? Is it "C Library includes > GCC builtin includes", or "GCC builtin includes > C Library includes"?

> If clang/libc++ wants built on top of GCC's implementation of GCC's C
> implementation, then it will need to similar tricks as GCC does for other
> targets. GCC does fixincludes trick to support other targets besides Linux
> even.

That's exactly my point -- this bug report is about removing the need to do tricks just to build on top of GCC's <limits.h>. None of the other headers require it AFAICT, so why does this header require it? Is there a reason not to make GCC's <limits.h> friendly to #include_next?
Comment 9 Andrew Pinski 2022-11-21 19:02:00 UTC
(In reply to Louis Dionne from comment #8)
> (In reply to Andrew Pinski from comment #5)
> > (In reply to Louis Dionne from comment #4)
> > > (In reply to Andrew Pinski from comment #2)
> > > > You should not be building on top of GCC's limits.h header at all really.
> > > > Rather implementations should have their own.
> > > 
> > > What do you mean by "implementations"? Do you mean implementations of the C
> > > library or compiler implementations, or what?
> > 
> > GCC limits.h is the implementation detail of GCC.
> > Yes I know it gets fuzzy. This is why GCC even has fixincludes to make sure
> > target headers are always correct. See the comment I posted.
> 
> When compiling pure C with GCC, what's the order of includes? Is it "C
> Library includes > GCC builtin includes", or "GCC builtin includes > C
> Library includes"?

GCC version specific includes > GCC version specific fixincludes > C library includes

That is for C.
C++ is:
libstdc++ library includes > ... (rest same as C).
Comment 10 Andrew Pinski 2022-11-21 19:05:54 UTC
(In reply to Andrew Pinski from comment #9) 
> GCC version specific includes > GCC version specific fixincludes > C library
> includes

And if you think fixincludes it is not needed even with a reasonable new glibc. You would be wrong. floatn.h needed to be fixed up to support C++ P1467R9 .
Comment 11 Louis Dionne 2022-11-21 21:52:51 UTC
(In reply to Andrew Pinski from comment #9)
> 
> GCC version specific includes > GCC version specific fixincludes > C library
> includes
> 
> That is for C.
> C++ is:
> libstdc++ library includes > ... (rest same as C).

Okay, that's great. That's exactly what I want! I want to be able to do:

    libc++ library includes > ... (rest same as C)

What I'm trying to say is precisely that this doesn't work as intended today, because somewhere inside "rest same as C", a header is taking for granted that libstdc++ does NOT implement a <limits.h> header. If it did, then libstdc++ would get the same issue that we are having.

Is there a reason why GCC needs to indirect through <syslimits.h> and recursively include <limits.h>, telling it to recurse using _GCC_NEXT_LIMITS_H?
Comment 12 Andrew Pinski 2022-11-21 22:02:49 UTC
(In reply to Louis Dionne from comment #11)
> (In reply to Andrew Pinski from comment #9)
> > 
> > GCC version specific includes > GCC version specific fixincludes > C library
> > includes
> > 
> > That is for C.
> > C++ is:
> > libstdc++ library includes > ... (rest same as C).
> 
> Okay, that's great. That's exactly what I want! I want to be able to do:
> 
>     libc++ library includes > ... (rest same as C)
> 
> What I'm trying to say is precisely that this doesn't work as intended
> today, because somewhere inside "rest same as C", a header is taking for
> granted that libstdc++ does NOT implement a <limits.h> header. If it did,
> then libstdc++ would get the same issue that we are having.

No because libstdc++ is part of gcc and then gcc's limit.h would be modifi instead.
Comment 13 Louis Dionne 2022-11-21 23:21:40 UTC
Let me rephrase my whole request here.

I understand that what GCC does work for GCC and GCC-adjacent projects. This report is about making the behavior of <limits.h> more friendly to implementations that are not GCC-adjacent and that need to build on top of the GCC machinery. Those implementations expect that they can include_next GCC headers and that no crazy magic is required to make that work, an expectation that is not met at the moment.

Is GCC interested in doing that? If not, you can close this bug as "not to be fixed".

Frankly, I do hope there's such a desire, since on our side we (Clang and libc++) do try to be "friendly" to GCC/libstdc++. For example, Clang is careful to implement attributes and match GCC behavior, and libc++ similarly tries to match libstdc++ behavior and ensures that it works on GCC. This results in a better ecosystem for everyone, but it can't be a one-way street.

If you don't want to even consider fixing this "because it's not a problem within the GCC stack", there's nothing I can do about it, but I think that would be a poor decision. If there's a technical reason why this can't or shouldn't be done, I'm all ears.
Comment 14 Andrew Pinski 2022-11-22 00:05:49 UTC
Let me ask another question.
What does clang/libc++ does on Solaris or aix or any non gcc related target?
Does it provide its own limits.h?
If so stop including the one included one with gcc for when gcc is installed.
Comment 15 Jonathan Wakely 2022-11-22 00:37:01 UTC
Louis, I think the best way to make progress here is to propose a patch. If you can make GCC's header work for all existing users and also work for you, great. I don't see why it would be rejected.

I'll confirm the bug, as I agree it would be nice if the header worked in additional situations. But don't be surprised if nobody does anything about it. You (or other libc++ devs) might need to do that work.
Comment 16 Jonathan Wakely 2022-11-22 00:51:55 UTC
FWIW on Fedora with glibc 2.35 I get:

. /tmp/__fake_cxx_include/limits.h
.. /usr/lib/gcc/x86_64-redhat-linux/12/include/limits.h
... /usr/lib/gcc/x86_64-redhat-linux/12/include/syslimits.h
.... /tmp/__fake_cxx_include/limits.h
. /usr/include/wchar.h
.. /usr/include/bits/libc-header-start.h
... /usr/include/features.h
.... /usr/include/features-time64.h
..... /usr/include/bits/wordsize.h
..... /usr/include/bits/timesize.h
...... /usr/include/bits/wordsize.h
.... /usr/include/sys/cdefs.h
..... /usr/include/bits/wordsize.h
..... /usr/include/bits/long-double.h
.... /usr/include/gnu/stubs.h
..... /usr/include/gnu/stubs-64.h
.. /usr/include/bits/floatn.h
... /usr/include/bits/floatn-common.h
.... /usr/include/bits/long-double.h
.. /usr/lib/gcc/x86_64-redhat-linux/12/include/stddef.h
.. /usr/lib/gcc/x86_64-redhat-linux/12/include/stdarg.h
.. /usr/include/bits/wchar.h
.. /usr/include/bits/types/wint_t.h
.. /usr/include/bits/types/mbstate_t.h
... /usr/include/bits/types/__mbstate_t.h
.. /usr/include/bits/types/__FILE.h
.. /usr/include/bits/types/FILE.h
.. /usr/include/bits/types/locale_t.h
... /usr/include/bits/types/__locale_t.h

No errors.