Menu

#111 STLport 5.2.1: missing prolog / epilog around stl/_threads.h

open-invalid
5
2010-04-30
2010-04-26
No

stl/_threads.h doesn't have the usual prolog and epilog code. This causes trouble in src/allocators.cpp: Headers from /usr/include are included while "std" is still defined as a macro. It's a problem particularly on Solaris whose system headers have "namespace std" around various declarations in C++ mode.

The patch adds a wrapper header for stl/_threads.h that provides prolog and epilog.

Please choose a better header ID if you accept this patch. I didn't know what ID scheme is used and chose 0xf00 for no reason other than that it's unique.

Discussion

  • Jan Echternach

    Jan Echternach - 2010-04-26
     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-04-30
    • assigned_to: nobody --> complement
    • status: open --> open-invalid
     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-04-30

    allocators.cpp has:

    #include "stlport_prefix.h"

    #include <memory>

    #if defined (__GNUC__) && (defined (__CYGWIN__) || defined (__MINGW32__))
    # include <malloc.h>
    #endif

    #if defined (_STLP_PTHREADS) && !defined (_STLP_NO_THREADS)
    # include <pthread_alloc>
    # include <cerrno>
    #endif

    #include <stl/_threads.h>

    So all prolog/epilog/features already included via "stlport_prefix.h" and <memory>.

     
  • Jan Echternach

    Jan Echternach - 2010-04-30

    The error messages on Solaris without this patch:

    "/export/home/jechtern/stlport/test1/STLport-5.2.1/stlport/stl/_cstdio.h", line 31: Error, badinclfile: Could not open include file<../CC/stlp_std/cstdio>.
    "/export/home/jechtern/stlport/test1/STLport-5.2.1/stlport/stl/_cwchar.h", line 36: Error, badinclfile: Could not open include file<../CC/stlp_std/cwchar>.

    It should include <../CC/std/cstdio>, but std is defined as a macro at that place, so it tries to include <../CC/stlp_std/cstdio> instead. Even if the include statement was changed, the contents of the vendor's <cstdio> or <stdio.h> header would still be processed with std defined as stlp_std, and the system header declarations would end up in the wrong namespace.

    std must not be defined as a macro in stl/_cstdio.h:31 and stl/_wchar.h:36. I don't understand what exactly went wrong for std to be still defined in STLport's own headers, but this patch solved this problem for me.

     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-04-30

    Heh, it's really different problem. Way to solution is (one of or in combination):
    - set _STLP_HAS_NO_NEW_C_HEADERS as done in mainstream for gcc
    - fix .SUNWCCh links.

     
  • Jan Echternach

    Jan Echternach - 2010-05-03

    The .SUNWCCh links had already been created when I got those error messages.

    _STLP_HAS_NO_NEW_C_HEADERS avoids part of the issue by using an #include directive which doesn't contain the string "std", but now I get

    #18 "/usr/include/wchar.h"
    using stlp_std :: wint_t ;
    using stlp_std :: clock_t ;
    ...

    in the preprocessor output. Trying to include a system header while "std" is defined as a macro is just wrong.

     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-05-04

    Yes, real problems come from
    - some SUNWCCh links missed in STLport;
    - some 'wrapper' files are missed in STLport;
    - Sun use 'namespace' and other C++ constructions within C (system-wide) headers; this is a problem, if you use compiler not from SUNSoft; gcc looks to avoid some problems of this kind by using fixinc procedure... but provide another problems for us...

    This patch isn't what I want to see here because it not fix problem itself, but use effect from casual files include order.

     
  • Jan Echternach

    Jan Echternach - 2010-05-11

    I'm afraid I don't understand the real problem that you are talking about, and so I won't be able to provide a better patch. Would you like me to open a bug report instead?

     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-05-11

    > Would you like me to open a bug report instead?

    No sense, IMO. Usage of 'new c-style headers' from system (or compiler) lead to contradictions in definitions (like what the first?; what std:: mean indeed?). Approach with pure C headers is much more clear. In the 'master' I try to avoid usage of 'new c-style headers' from system/compiler, but construct it from original system headers (yes, I'm expect it's C). When vendor include C++ constructs in (expected) pure C headers (looks far from fair play --- tie to vendor's C++ compiler), we need a vendor-specific workarounds.

    Play with 'master' first: we are free with solutions here. With this experience we can make decision about STLport-5.2 branch (keep compatibility in solutions/support another platform).

     
  • Jan Echternach

    Jan Echternach - 2010-05-12

    > Usage of 'new c-style headers' from system (or compiler)
    > lead to contradictions in definitions (like what the first?; what std::
    > mean indeed?).

    So far my limited understanding of STLport's macro and namespace magic was that it used stlp_std:: for itself, leaving std:: for the system and the compiler. "std" would only be defined as a macro outside the STLport headers in user code such that the STLport headers would have access to the vendor's std:: namespace.

    > When vendor include C++ constructs in (expected) pure C
    > headers (looks far from fair play --- tie to vendor's C++ compiler)

    Annex D.6 of N3092 is a bit more detailed than the corresponding Annex D.5 of the current standard. It explicitly allows the C headers to provide the declarations and definitions in the std namespace in addition to the global namespace. It would be nice if STLport wouldn't rely on some particular unspecified compiler or C header property here.

    > Play with 'master' first: we are free with solutions here.

    I'm sorry but I won't have time for that. I had hoped that this was a simple problem with a simple solution, but now it looks like we will have to leave it unresolved.

     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-05-17

    > Annex D.6 of N3092 ...
    Thanks for this point. Nevertheless this clause lead to problems with different C++ compilers in system (mangling, libs). This is normal for Windows (heh, do you see 'system' headers there? --- all shipout with compiler), but not normal for *nixes.

     
  • Jan Echternach

    Jan Echternach - 2010-05-17

    > Nevertheless this clause lead to problems with different C++ compilers in system
    > (mangling, libs).

    There's no name mangling. The math functions are declared extern "C".

    The complete <cmath> header of Sun Studio 12 (excluding comments and include guard macros):

    #include <sys/feature_tests.h>
    #include <iso/math_iso.h>

    The <iso/math_iso.h> header as seen by a C++ compiler:

    extern "C" {
    namespace std {
    extern double acos (double);
    ...
    extern double fabs (double);
    ...
    extern "C++" {
    inline double abs (double __X) { return fabs (__X); }
    inline float abs (float __X) { return __fabsf (__X); }
    inline long double abs (long double __X) { return __fabsl (__X); }
    ...
    }
    }
    }

    and finally the <math.h> header as seen by a C++ compiler:

    #include <iso/math_iso.h>
    #include <iso/math_c99.h>
    using std::abs;
    ...
    extern "C" {
    /* various extensions that are neither in C89 nor C99,
    or have been part of a Unix standard before C99 */
    ...
    }

    This is the basic header structure on Solaris, at least since Solaris 8, although the list of declarations and inline functions differs between Solaris versions (for example, Solaris 8 has no std::abs(float) and std::abs(long double) in <iso/math_iso.h>).

     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-05-17

    > There's no name mangling. The math functions are declared extern "C".

    What's about this:
    ...
    extern "C++" {
    inline double abs (double __X) { return fabs (__X); }
    inline float abs (float __X) { return __fabsf (__X); }
    inline long double abs (long double __X) { return __fabsl (__X); }
    ...
    }

    ?

     
  • Jan Echternach

    Jan Echternach - 2010-06-02

    The inline functions are not defined in the C++ runtime library (well, they could be, but they need not be defined as there's also a definition in the header file). So any name mangling differences wouldn't hurt - even if there was a function definition in the C++ runtime library, we wouldn't need to find and use it.

    Actually, a different name mangling could help if another compiler uses different C++ calling conventions. I don't see a case where different mangling of inline function names could cause compile, link or runtime failures in this szenario (i.e. Sun C++ runtime library, and a different C++ compiler trying to use the C functions from the system headers; calling C++ code compiled by the Sun compiler from C++ code compiled by another compiler would be a different matter).

     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-06-02

    Thanks to namespace std declaration in OS headers, we need to write wrapper header either for iso/math_iso.h or for all OS headers that include (directly or indirectly) iso/math_iso.h. I suspect the similar issue for wchars for Solaris.

     
  • Jan Echternach

    Jan Echternach - 2010-06-04

    Right, I hadn't thought about that issue. But iso/*_iso.h headers with namespaces only exist for things that are in a C++ standard, and Solaris doesn't include them directly but only through the usual C headers, at least on the Solaris 10 system I have here:

    $ find /usr/include -type f | xargs grep 'include.*iso/' | sort
    /usr/include/ctype.h:#include <iso/ctype_c99.h>
    /usr/include/ctype.h:#include <iso/ctype_iso.h>
    /usr/include/iso/signal_iso.h:#include <sys/iso/signal_iso.h>
    /usr/include/iso/wchar_iso.h:#include <iso/time_iso.h>
    /usr/include/limits.h:#include <iso/limits_iso.h>
    /usr/include/locale.h:#include <iso/locale_iso.h>
    /usr/include/math.h:#include <iso/math_c99.h>
    /usr/include/math.h:#include <iso/math_iso.h>
    /usr/include/setjmp.h:#include <iso/setjmp_iso.h>
    /usr/include/signal.h:#include <iso/signal_iso.h>
    /usr/include/stdarg.h:#include <iso/stdarg_c99.h>
    /usr/include/stdarg.h:#include <iso/stdarg_iso.h>
    /usr/include/stddef.h:#include <iso/stddef_iso.h>
    /usr/include/stdio.h:#include <iso/stdio_c99.h>
    /usr/include/stdio.h:#include <iso/stdio_iso.h>
    /usr/include/stdlib.h:#include <iso/stdlib_c99.h>
    /usr/include/stdlib.h:#include <iso/stdlib_iso.h>
    /usr/include/string.h:#include <iso/string_iso.h>
    /usr/include/sys/signal.h:#include <sys/iso/signal_iso.h>
    /usr/include/time.h:#include <iso/time_iso.h>
    /usr/include/wchar.h:#include <iso/wchar_c99.h>
    /usr/include/wchar.h:#include <iso/wchar_iso.h>
    /usr/include/wctype.h:#include <iso/wctype_c99.h>
    /usr/include/wctype.h:#include <iso/wctype_iso.h>

    Some of these headers have no "namespace std":
    limits, sys/signal

    Headers with "namespace std":
    ctype, locale, math, setjmp, signal, stdarg, stddef, stdio, stdlib, string, time, wchar, wctype

    All of these already got an STLport wrapper header.

     
  • Petr Ovtchenkov

    Petr Ovtchenkov - 2010-06-04

    Using 'new-style C++ headers' from compiler vendor (it closely coupled with compiler, even if ones shipout with OS---I do with Solaris 7 and 8; headers in 5.x was free from C++, IIRC) is a nightmare: it may include compiler-specific decls, refs to vendor's libs (via extern), it should include C-style defs, but really don't, sometimes declarations in 'new-style C++ headers' conflict with ones from 'old good C headers'; it may include 'old good C headers' with extra #defines (exclude/include some parts [important!] of 'old good C headers').

    If use 'new C++-style headers', STLport's wrappers (headers) become very tricky:

    - STLport's wrapper for 'new C++-style header'
    - OS 'new C++-style header'
    - STLport's wrapper for 'old C-style header'
    - OS 'old C-style header'

    but this should work too:

    - STLport's wrapper for 'old C-style header'
    - OS 'old C-style header'

    and should work in combination.

    After experience with 'new C++-style headers from vendor, if possible' [i.e. STLport-5.2 branch] I try 'avoid new C++-style headers from vendor, when possible' [i.e. master branch] and see that this approach give much better and predictable results with less efforts.

     
  • Jan Echternach

    Jan Echternach - 2010-06-07

    I don't see how the choice of new vs. old C library headers is related to this problem. Sun C++'s <cmath>, for example, contains nothing that <math.h> doesn't also have. See bug https://sourceforge.net/tracker/?func=detail&aid=2993711&group_id=146814&atid=766244 for more details. Using old C headers should properly work on Solaris if STLport doesn't try to overload any functions in the global namespace and "std" is not defined as a macro.

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.