Bug 51749 - Including <algorithm> pollutes global namespace
Summary: Including <algorithm> pollutes global namespace
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 57582 59945 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-01-04 09:51 UTC by nospam.kotarou.dono
Modified: 2024-07-26 12:03 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-01-26 00:00:00


Attachments
Preprocessed file (26.98 KB, application/octet-stream)
2012-01-04 09:51 UTC, nospam.kotarou.dono
Details

Note You need to log in before you can comment on or make changes to this bug.
Description nospam.kotarou.dono 2012-01-04 09:51:46 UTC
Created attachment 26235 [details]
Preprocessed file

Including the header file <algorithm> on Linux seems to pollute the global namespace. I discovered this while attempting to put <sys/select.h> in a namespace. A quick example is given below:

test.cpp:

#include <algorithm>
fd_set set; // Should not compile
int main() { return 0; }

Compiles with: g++ test.cpp -o test

gcc -v:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/home/kotarou3/usr/bin/../libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../mingw/gcc-src/configure --prefix=/home/kotarou3/work/gcc-build/../gcc --with-gmp=/home/kotarou3/work/gcc-build/../mingw/gmp --with-mpfr=/home/kotarou3/work/gcc-build/../mingw/mpfr --with-mpc=/home/kotarou3/work/gcc-build/../mingw/mpc --enable-languages=c,c++ --disable-win32-registry --enable-checking=release --enable-shared
Thread model: posix
gcc version 4.7.0 20111231 (experimental) (GCC)
Comment 1 Richard Biener 2012-01-04 10:16:56 UTC
bits/stl_algo.h ends up including cstdlib.
Comment 2 Paolo Carlini 2012-01-04 10:25:45 UTC
Thus I understand you are new to GCC, because the problem was already there in, eg, gcc3, and very likely the original HP/SGI STL! It's because of the use of rand(), or a similar system function, which requires including <cstdlib>. Note, all of this is in general allowed by the standard: including a <c*> header as an implementation detail, and <cstdlib> specifically including <stdlib.h> as an implementation detail. The latter unfortunately on these particular systems also provides 'set'.

In principle we could handle this specific manifestation of the annoyance by eg, adding to the compiler a __builtin_rand, but it's too late for 4.7, or by exporting from the .so some __rand function. The latter idea doesn't sound too bad to me, wasn't done in the original HP/SGI code because it didn't come with any .src files.

Anyway, beware that many more of these issues are lurking around, we don't control the underlying libc and the stuff its headers provide in the global namespace, sooner or later you may find unexpected names in it.
Comment 3 nospam.kotarou.dono 2012-01-07 04:14:06 UTC
Yes I'm new to GCC :)

If it won't be able to make it in to 4.7, will it be able to make it in to 4.7.1 or 4.8?
Comment 4 Jakub Jelinek 2012-01-07 08:10:31 UTC
4.8.
Comment 5 nospam.kotarou.dono 2012-01-08 12:14:56 UTC
Also found another similar problem while compiling on Windows with mingw-w64. Including <chrono> also pollutes the global namespace.

text.cpp:

#include <chrono>
timeval t; // Shouldn't compile
int main() { return 0; }

Compiles with: g++ -std=gnu++11 test.cpp -o test

GCC version is exactly the same as the one in my first comment.

PS. Should I report this specific problem to mingw-w64 instead?
Comment 6 Jonathan Wakely 2012-01-08 12:41:16 UTC
(In reply to comment #5)
> Also found another similar problem while compiling on Windows with mingw-w64.
> Including <chrono> also pollutes the global namespace.

In general you'll just have to live with some of these bugs, we're not going to reimplement the entire system's libc so libstdc++ has to be built on top of OS features and not all of them can easily be hidden or redeclared with names reserved names.

> PS. Should I report this specific problem to mingw-w64 instead?

No, it's not mingw-specific
Comment 7 nospam.kotarou.dono 2012-01-17 08:19:32 UTC
I also noticed that quite a few of the c* files dump their entire C counterpart in the global namespace. Is it supposed to do that according to the standard? If not, it sounds like something that can fixed just by wrapping the #include in a namespace std block.

I noticed:
cstdio dumps entire stdio.h
cstdlib dumps entire stdlib.h
cmath dumps entire math.h
cctype dumps entire ctype.h
Comment 8 Paolo Carlini 2012-01-17 10:52:06 UTC
Yes, clarified to be legal by LWG 456. As I tried already to explain here and elsewhere, unless the C++ library has some sort of control over the underlying C library this kind of issue can reappear here and there at any moment, even if for 4.8 we can handle the specific <algorithm> case better as far as its implementation details are concerned.
Comment 9 Jonathan Wakely 2012-01-17 11:37:34 UTC
(In reply to comment #7)
> If not, it sounds like something that can fixed just by wrapping the #include
> in a namespace std block.

That could never work.

1) namespace are ignored for extern "C" functions

2) if you also surrounded them in extern "C++" that would cause your program to see a declaration of extern "C++" std::puts(const char*), which would not be found in libc.so because that contains a definition for extern "C" puts.

3) as Paolo said, it's legal anyway
Comment 10 nospam.kotarou.dono 2012-01-17 12:08:36 UTC
Yea, I just read about http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6257 so I'm okay with that.
Comment 11 Jonathan Wakely 2012-01-17 12:36:50 UTC
please stop changing the component to 'libobjc'
Comment 12 nospam.kotarou.dono 2012-01-17 12:50:01 UTC
I didn't change it o.o
Maybe something's wrong with my browser
Comment 13 Jonathan Wakely 2012-01-17 12:54:18 UTC
I assume you were just hitting "refresh" on this page. If you (accidentally) changed the component once and refresh, most browsers will keep your chosen selection in the form, so if you then submit another comment you'll re-submit your choice of 'libobjc'.  It didn't happen on your last comment anyway, so you seem to have stopped :)
Comment 14 nospam.kotarou.dono 2012-01-29 04:40:07 UTC
Just reporting another one~ (Yes, I can live with it not being fixed, but bugzilla is for reporting bugs, right?)

Including <string> eventually includes pthreads.h, dumping the whole file in the global namespace. Example:

$ cat | g++ -xc++ - -o - > /dev/null
#include <string>
pid_t pid = 0; // Should not compile
int main() { return (short)pid; }
$ echo $?
0
Comment 15 Paolo Carlini 2012-01-29 10:38:59 UTC
This one is evidently due to the reference-counted nature of our string class, and is also very, very old. It will go away automagically when we'll move to no-reference-counting for C++11 (like the current ext/vstring.h). Really, I only want to add that these issue are well known.
Comment 16 Paolo Carlini 2013-06-11 08:59:50 UTC
*** Bug 57582 has been marked as a duplicate of this bug. ***
Comment 17 Nick Maclaren 2013-06-11 09:39:30 UTC
I will just add to comment 8 that dumping large chunks of the POSIX
namespace in isn't legal, unless WG21 have completely lost their
marbles :-)

But, as people have said, this isn't fixable by hacking - not least
because it would have to be done in translation phase 4, not 8, so
namespaces would not be a permissable tool.  We shall have to wait
for a proper fix.
Comment 18 Jonathan Wakely 2013-06-11 09:44:25 UTC
Comment 8 is only referring to <cstdio> et al putting standard C names in the global namespace as well as in namespace std, which is legal. We all agree it's not legal to also put names that aren't from ISO C (e.g. POSIX names) in the global namespace.
Comment 19 Jakub Jelinek 2013-06-11 09:51:11 UTC
(In reply to Jonathan Wakely from comment #18)
> Comment 8 is only referring to <cstdio> et al putting standard C names in
> the global namespace as well as in namespace std, which is legal. We all
> agree it's not legal to also put names that aren't from ISO C (e.g. POSIX
> names) in the global namespace.

Has anyone checked recently what exactly we still need -D_GNU_SOURCE for in the public headers (as opposed to just the libstdc++ code), as compared to say _ISOC99_SOURCE for glibc, at least for C++11?
Comment 20 Jonathan Wakely 2013-06-11 09:54:21 UTC
(In reply to Jakub Jelinek from comment #19)
> Has anyone checked recently what exactly we still need -D_GNU_SOURCE for in
> the public headers (as opposed to just the libstdc++ code), as compared to
> say _ISOC99_SOURCE for glibc, at least for C++11?

I haven't checked, it would be useful to do so.  There is some stuff used by the --enable-clocale=gnu code such as uselocale (which is now part of POSIX but not C99) but it might be possible to only require that from .cc files not in headers.
Comment 21 Jakub Jelinek 2013-06-11 10:05:17 UTC
(In reply to Jonathan Wakely from comment #20)
> (In reply to Jakub Jelinek from comment #19)
> > Has anyone checked recently what exactly we still need -D_GNU_SOURCE for in
> > the public headers (as opposed to just the libstdc++ code), as compared to
> > say _ISOC99_SOURCE for glibc, at least for C++11?
> 
> I haven't checked, it would be useful to do so.  There is some stuff used by
> the --enable-clocale=gnu code such as uselocale (which is now part of POSIX
> but not C99) but it might be possible to only require that from .cc files
> not in headers.

In __gnu_cxx namespace:
  extern "C" __typeof(uselocale) __uselocale;
and in std namespace:
  typedef __locale_t            __c_locale;

suggests that it really is looking for the uselocale function just to get the prototype and declares the function itself, and we need an alternative to __locale_t.  We can't include xlocale.h, because that would make locale_t symbol visible, and can't typedef __locale_t, because that could clash with <locale.h>
definition -D_GNU_SOURCE etc.
Thus it would need to be something like:
// Hackish forward declaration in global namespace.
struct __locale_struct;
and then just do:
  extern "C" __locale_struct *__uselocale (__locale_struct *);
in __gnu_cxx namespace and
  typedef __locale_struct *      __c_locale;
in std namespace.
Comment 22 Paolo Carlini 2013-06-11 10:10:48 UTC
If I remember correctly, the gthr.h included by ext/atomicity.h is also a big issue. If we switch to a non-reference-counted basic_string we can make progress but another usage is in <locale>, global locale & co.
Comment 23 Jakub Jelinek 2013-06-11 10:21:16 UTC
So, perhaps this is solveable only through cooperation with glibc folks.
I guess a start for that would be to do an audit of the libstdc++ headers, what symbols does it need from the glibc headers, and categorize them (symbols that
are defined in the C headers unconditionally, not guarded by any feature test macros, symbols guarded by feature test macros that C++ wants to have visible in the *.h headers (candidates for _ISOCXX98_SOURCE, _ISOCXX11_SOURCE, _ISOCXX14_SOURCE feature test macros?), symbols we don't want to be visible in the C headers but libstdc++ headers use privately for the implementation (export as __ prefixed alternatives?).  If glibc is changed, the question would be what should be the new G++ feature test macro defaults, for -std=gnu++{98,11,1y} I guess it should include the POSIX etc. stuff by default too, for -std=c++{98,11,1y} perhaps it should be strict and require users to use feature test macros explicitly.
Comment 24 Jonathan Wakely 2013-06-11 10:24:26 UTC
(In reply to Jakub Jelinek from comment #23)
> changed, the question would be what should be the new G++ feature test macro
> defaults, for -std=gnu++{98,11,1y} I guess it should include the POSIX etc.
> stuff by default too, for -std=c++{98,11,1y} perhaps it should be strict and
> require users to use feature test macros explicitly.

That sounds good to me.
Comment 25 Nick Maclaren 2013-06-11 11:08:11 UTC
Strictly, don't you mean feature selection macros?

It isn't worth being too perfectionist on this, as the standards (both
de jure and de facto) are an ungodly mess, and it is getting steadily
harder to write portable code in C/C++/POSIX/etc.  However, I strongly
agree that taking the hard line (especially with -std=<formal standard>)
is the best way to go.

On the other hand, it is quite likely that a stricter line may require
new options or feature selection macros - because, for example, there
are codes that need a specific version of the language but also the
extensions of a specific version of POSIX.  Ugh.
Comment 26 Jakub Jelinek 2013-06-11 11:21:19 UTC
No, I mean what I wrote, see
http://pubs.opengroup.org/onlinepubs/007904975/functions/xsh_chap02_02.html
info libc 'Feature Test Macros'
etc.
Comment 27 Jonathan Wakely 2014-01-26 00:06:21 UTC
*** Bug 59945 has been marked as a duplicate of this bug. ***
Comment 28 Jonathan Wakely 2014-01-26 10:19:28 UTC
PR 11196 is another example of the same general issue.

In addition to the uselocale stuff mentioned in comment 21 ...

<ext/stdio_sync_filebuf.h> uses ftello64, fseeko64, etc. If glibc provided __ftello64 etc instead we could use them, but another option that works now is to define only _LARGEFILE64_SOURCE, rather than everything implied by _GNU_SOURCE.

-std=c++11 and -std=gnu++11 need to define _ISOC99_SOURCE, and libstdc++ should have a replacement for _GLIBCXX_USE_C99 so that it isn't true for -std=c++98 (but would be for -std=gnu++98, since that would define _GNU_SOURCE which includes the C99 library anyway).

gthr-posix.h needs some _XOPEN_SOURCE value.

<util/testsuite_hooks.h> uses pid_t which needs a POSIX/XOPEN/BSD/GNU feature macro (which would be OK except that at least one tests uses -std=c++11 when it should probably use -std=gnu++11)

So *most* of what we need is provided by _ISOC99_SOURCE and _LARGEFILE64_SOURCE and POSIX. It's probably acceptable (and certainly more correct) not to define _GLIBCXX_USE_C99 with -std=c++98 and figure out the few C99 functions that we absolutely must have even in strict c++98 mode and find workarounds for them.

I'll look at this after GCC 4.9 is released.
Comment 29 jsm-csl@polyomino.org.uk 2014-01-26 17:24:09 UTC
As was discussed a couple of years ago, the glibc maintainers are willing 
to work with the libstdc++ maintainers on providing whatever features 
libstdc++ headers need in a namespace-clean way (not exposing symbols not 
reserved by the relevant version of standard C++, when standard libstdc++ 
headers are used in a strict mode such as -std=c++98 or -std=c++11 and 
libstdc++ is built for recent-enough glibc).  But the libstdc++ people 
will need to work out exactly what interfaces are needed so we can work 
out how to expose them cleanly.

https://sourceware.org/ml/libc-alpha/2012-03/msg00311.html

(In the case of <func>64, providing __<func>64 is a good idea for other 
reasons where not already provided: it would allow fixing 
<https://sourceware.org/bugzilla/show_bug.cgi?id=14106>, 
_FILE_OFFSET_BITS=64 not being namespace-clean.  So I consider such a 
change appropriate for, at least, any function in a POSIX or C standard 
with a <func>64 version used for _FILE_OFFSET_BITS=64.)

I advise taking discussion of the details to libc-alpha, once you have 
more idea of exactly what features you would like glibc headers to expose 
in a C++-namespace-clean way.
Comment 30 Jonathan Wakely 2014-01-26 22:50:18 UTC
Thanks, Joseph, I'll determine what changes we would need. I hope we can make some progress just by being a bit smarter in libstdc++, and then only need glibc changes for what's left after that.
Comment 31 Jonathan Wakely 2014-03-10 19:01:14 UTC
*** Bug 60491 has been marked as a duplicate of this bug. ***
Comment 32 Yaakov Selkowitz 2016-04-07 05:19:09 UTC
In an effort to enable C99-in-C++ functionality on newlib-based targets (including Cygwin and RTEMS), we just overhauled our feature test macros to be functionally compatible with glibc's.  Ignoring the lack of missing long double real and complex math functions (which are anyways guarded separately), we were able to thereby enable the rest of _GLIBCXX_USE_C99* functionality without the -D_GNU_SOURCE hack.

Our approach was (in glibc terms):

* enable __USE_ISOC99 in features.h if defined(__cplusplus), for C99-in-TR1;

* enable __USE_ISOC11 in features.h if __cplusplus >= 201103L, for *quick_exit in C++11.

* define the _POSIX_* capability macros in <unistd.h> regardless of __USE_POSIX*, for gthr-posix.h.

Note that the public -D_ISOC*_SOURCE macros were NOT defined in g++.

However, we don't need ftello64 (Cygwin's off_t is naturally 64-bit) nor do we have the locale_t group of functions, which according to the Linux man-pages are SUSv4.  These functions may be best served by their own special guards.
Comment 33 Jonathan Wakely 2016-04-07 11:01:52 UTC
Thanks for the info. The locale_t functionality is the main thing that will need work, but it's good to know you were able to get everything else working well.