Libc++ Windows fixes

I believe it does the right thing, presuming you have uselocale that sets the per-thread locale to __l.

Howard

2011/9/26 Howard Hinnant <hhinnant@apple.com>

There isn't a libc++ initialization function.

Perhaps you could add this initialization to __cloc()?

Howard

> If the __foo_l functions are just the wrappers that they seem to be, I can certainly live with removing this layer of indirection and me implementing the missing functions that are now wrongfully bypassed. I think expecting all xlocale functions to be present is fair for future implementations/ports. This would mean some additions to the support/win32/locale.h header only, which would concentrate any real problems there, which is a good thing. All platform-related fixme's and their future fixes (at least the win32 ones) would go in one centralized place, away from the public libc++ headers and core libc++. Other ports can do the same.
>
> In that block of code, there is also a construct
> __locale_raii __current(uselocale(__l), uselocale);
> int __res = sprintf(__s, __format, __args...);
>
> Does this do the right thing; a very wasteful implementation of the correct behavior, or is this just an attempt to not make things worse. This would be important for me implementing missing *_l functions. If the above worked like I hope it does, that becomes very easy, albeit unperformant.

I believe it does the right thing, presuming you have uselocale that sets the per-thread locale to __l.

That is great news! For this to function correctly, I need to call "_configthreadlocale" (http://msdn.microsoft.com/en-us/library/26c0tb7x(v=vs.80).aspx) somewhere before anything else, only once. Is there an existing static initialization function in libc++ I could add this call to?

I'm not a Windows expert, but isn't DllMain design for this kind of initialization ?

It is called once per thread create passing the DLL_THREAD_ATTACH argument.

Thanks!

Ruben
_______________________________________________
cfe-dev mailing list
cfe-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

-- Jean-Daniel

2011/9/26 Jean-Daniel Dupas <devlists@shadowlab.org>

2011/9/26 Howard Hinnant <hhinnant@apple.com>

If the __foo_l functions are just the wrappers that they seem to be, I can certainly live with removing this layer of indirection and me implementing the missing functions that are now wrongfully bypassed. I think expecting all xlocale functions to be present is fair for future implementations/ports. This would mean some additions to the support/win32/locale.h header only, which would concentrate any real problems there, which is a good thing. All platform-related fixme’s and their future fixes (at least the win32 ones) would go in one centralized place, away from the public libc++ headers and core libc++. Other ports can do the same.

In that block of code, there is also a construct
__locale_raii __current(uselocale(__l), uselocale);
int __res = sprintf(__s, __format, __args…);

Does this do the right thing; a very wasteful implementation of the correct behavior, or is this just an attempt to not make things worse. This would be important for me implementing missing *_l functions. If the above worked like I hope it does, that becomes very easy, albeit unperformant.

I believe it does the right thing, presuming you have uselocale that sets the per-thread locale to __l.

That is great news! For this to function correctly, I need to call “_configthreadlocale” (http://msdn.microsoft.com/en-us/library/26c0tb7x%28v=vs.80%29.aspx) somewhere before anything else, only once. Is there an existing static initialization function in libc++ I could add this call to?

I’m not a Windows expert, but isn’t DllMain design for this kind of initialization ?

It is called once per thread create passing the DLL_THREAD_ATTACH argument.

I’ve though about this, but this would break for static libc++.

This issue is worse/different than I first thought. The _configthreadlocale function needs to be called per thread, so this is frankly out of libc++'s control and should be left to the user.

Ruben

I do not believe we can safely set this on a global basis since that
might interfere with user code.

Rather, we would have to, where POSIX code uses uselocale(), retrieve
the current setting of _configthreadlocale, set it to thread-local or
save the current locale, set the correct locale, do our operation,
reset the locale, and then restore either the previous locale or the
global setting.

Sean

Does libc++ really depend on the crappy idea of per-thread locale?

Joerg

libc++ depends on the existence of functions such as:

int
   sprintf_l(char * restrict str, locale_t loc, const char * restrict format, ...);

which are always called with the "C" locale passed to "loc". These functions exist on OS X, and have recently been implemented on FreeBSD. One way of emulating them is using a per-thread locale.

In a nutshell, libc++ needs to be able to get the functionality of sprintf, but using the "C" locale instead of the current global or per-thread locale. And ditto for several other C I/O functions.

Howard

For whatever reason Microsoft is prefixing many standard POSIX functions with an underscore:

int _sprintf_l(
    char *buffer,
    const char *format,
    locale_t locale [,
       argument] ...
);

I hope this helps,
TOM

Zitat von Howard Hinnant <hhinnant@apple.com>:

OK. Basic background of the question is that specifying an explicit
locale is fine in terms of API design and useful to adopt. The concept
of per-thread locale is just insane, so I want to make sure that it
isn't needed for libc++.

Joerg

2011/9/26 Thomas Gamper <icicle@mail.cg.tuwien.ac.at>

For whatever reason Microsoft is prefixing many standard POSIX
functions with an underscore:

http://msdn.microsoft.com/en-us/library/ybk95axf%28v=vs.80%29.aspx

int _sprintf_l(
char *buffer,
const char *format,
locale_t locale [,
argument] …
);

I hope this helps,
TOM

Probably because such identifiers (leading underscore) are reserved for the implementation at global scope. The POSIX Standard “pollutes” the global namespace with identifiers that fall within userland therefore, while MSVC does not :slight_smile:

– Matthieu.

2011/9/26 Thomas Gamper <icicle@mail.cg.tuwien.ac.at>

For whatever reason Microsoft is prefixing many standard POSIX
functions with an underscore:

http://msdn.microsoft.com/en-us/library/ybk95axf%28v=vs.80%29.aspx

int _sprintf_l(
char *buffer,
const char *format,
locale_t locale [,
argument] …
);

I hope this helps,
TOM

Probably because such identifiers (leading underscore) are reserved for the implementation at global scope.

No, identifiers are only reserved if they start with two underscores (__foo) or an underscore and a capital letter (_Foo).

The POSIX Standard “pollutes” the global namespace with identifiers that fall within userland therefore, while MSVC does not :slight_smile:

Only in the same sense that the C standard “pollutes” the global namespace with memcpy and malloc.

Anyway, doesn’t really matter; it’s what they do.

John.

2011/9/26 Thomas Gamper <icicle@mail.cg.tuwien.ac.at>

For whatever reason Microsoft is prefixing many standard POSIX
functions with an underscore:

http://msdn.microsoft.com/en-us/library/ybk95axf%28v=vs.80%29.aspx

int _sprintf_l(
char *buffer,
const char *format,
locale_t locale [,
argument] …
);

I hope this helps,
TOM

Probably because such identifiers (leading underscore) are reserved for the implementation at global scope.

No, identifiers are only reserved if they start with two underscores (__foo) or an underscore and a capital letter (_Foo).

Those names are reserved universally, names begining with an underscore are reserved for the implementation in the global namespace. 17.6.4.3.2\1:

Certain sets of names and function signatures are always reserved to the implementation:
— Each name that contains a double underscore _ _ or begins with an underscore followed by an uppercase
letter (2.12) is reserved to the implementation for any use.
— Each name that begins with an underscore is reserved to the implementation for use as a name in the
global namespace.

The POSIX Standard “pollutes” the global namespace with identifiers that fall within userland therefore, while MSVC does not :slight_smile:

Only in the same sense that the C standard “pollutes” the global namespace with memcpy and malloc.

Except those are documented names already in the standard.

Anyway, doesn’t really matter; it’s what they do.

Right.

2011/9/25 Howard Hinnant <hhinnant@apple.com>

2011/9/23 Howard Hinnant <hhinnant@apple.com>

Can anything be done about this GCC incompatibility: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10980#c7 ? It is preventing me from linking a functional libc++ (as Clang is not capable of producing correct object files to be linked together), and of course, testing the beast :). To fix it, this basically needs the functions in question to not be declared as “always inline”, because GCC produces an error in this case. If Clang does not inline in this case, it should really also produce an error instead of silently ignoring the attribute.

Seems like the easiest thing to do would be to rewrite these without using va_list. Only one or two arguments need to be supported. Just make these always-inline templates.

Thanks for the valuable idea. Attached patch implements these for non-clang compiles (#ifdef clang). I also used the available *_l function variants for those that are available on Windows (MSVC++ runtime 8.0 and later). I also fixed up the win32 support header stuff to work with the full header things required.
I added a mingw bit to the lib/buildit script, that for now quite hackishly links to libsupc++ and allows multiple definitions due to a missing alternative. This will at least allow some testing to happen on the rest of the libc++ code, that isn’t compiled into the library itself.
I envision a full LLVM alternative to libgcc and libsupc++, but that is still a looooong way off.

Comments and especially commits are very welcome!

Ruben

PS: the only thing holding back a test run is an undefined reference in mingw-w64’s winpthreads library.
<windows.patch>

I think it is getting time for a refactoring in .

What appears to be happening is that functions like asprintf_l are getting implemented for each platform, which I think is a good thing. But it appears to be happening in an increasingly convoluted way. For example we have:

#ifdef _LIBCPP_STABLE_APPLE_ABI
__n = asprintf_l(&__bb, 0, “%.0Lf”, __units);
#else
__n = __asprintf_l(&__bb, __cloc(), “%.0Lf”, __units);
#endif

The first branch is only taken by APPLE. The second branch is taken by everyone else and the definition of __asprintf_l is earlier in . This definition sometimes calls vasprintf_l, vasprintf, asprintf, and even (ironically) asprintf_l! And it isn’t easy (at least to me), to see what is happening on each platform.

Suggestion:

#ifdef APPLE

define _LIBCPP_GET_C_LOCALE 0

#else

define _LIBCPP_GET_C_LOCALE __cloc()

#endif

__n = asprintf_l(&__bb, _LIBCPP_GET_C_LOCALE, “%.0Lf”, __units);

And then everyone go off and implement asprintf_l, preferably not in . E.g. (and correct me if I’m wrong), FreeBSD has done this in libc. _WIN32 is doing it in src/support/win32/support.cpp. I’d like to see this block of code:

// OSX has nice foo_l() functions that let you turn off use of the global
// locale. Linux, not so much. The following functions avoid the locale when
// that’s possible and otherwise do the wrong thing. FIXME.
#ifndef _LIBCPP_STABLE_APPLE_ABI

just disappear, or at the very least become greatly reduced.

If this is just replacing all occurences of the __*_l functions in libc++ code with the normal *_l functions, and removing all the always inline wrappers, I can volunteer to do this mechanical work. I guess the only port directly affected by this would be Linux, but that could easily be fixed by introducing a support header and moving the wrapper code from to there.

Ruben

Perhaps the best way to proceed is for someone such as yourself to just untangle the Windows port from this block of code:

// OSX has nice foo_l() functions that let you turn off use of the global
// locale. Linux, not so much. The following functions avoid the locale when
// that's possible and otherwise do the wrong thing. FIXME.
#ifndef _LIBCPP_STABLE_APPLE_ABI

Someone else (perhaps David) could just untangle the FreeBSD port from the same block of code.

Perhaps someone else can untangle the Linux port from this block of code.

Once everybody is untangled from it, we can safely delete it because no one will be using it. But I hesitate to have anyone work this area without the ability to test it for the impacted platform.

Howard

The one thing that someone must do that will impact all platforms is transform this:

#ifdef _LIBCPP_STABLE_APPLE_ABI
    if (sscanf_l(__a, 0, "%p", &__v) != 1)
#else
    if (__sscanf_l(__a, __cloc(), "%p", &__v) != 1)
#endif

to:

#ifdef _LIBCPP_STABLE_APPLE_ABI
    if (sscanf_l(__a, _LIBCPP_GET_C_LOCALE, "%p", &__v) != 1)
#else
    if (__sscanf_l(__a, __cloc(), "%p", &__v) != 1)
#endif

And in a couple of places:

#ifdef _LIBCPP_STABLE_APPLE_ABI
        long long __ll = strtoll_l(__a, &__p2, __base, 0);
#else
        long long __ll = strtoll_l(__a, &__p2, __base, __cloc());
#endif

to:

        long long __ll = strtoll_l(__a, &__p2, __base, _LIBCPP_GET_C_LOCALE);

If you want to take that on, go for it. If you want me to do it, that's fine too. But it will have to wait for a few hours (don't have the cycles at the moment).

Howard

Hi,

I have removed Windows’ dependency on the ugly part that is going to be refactored by mirrorring Mac and FreeBSD through implementing all the necessary *_l functions and dependents.

Other notable changes:

  • support/win32/locale.h: fixed uselocale to enable per-thread locale, because the POSIX definition of this function requires that it only set the current thread’s locale. Consequence is that after a call to this function, per-thread locale will remain enabled. This is intended behavior as far as I can see. As a result, I’m fairly confident the __locale_raii trick works as it should even in multithreaded contexts.
  • include/locale: I still included the variadic template alternatives to the vararg functions. As these wrappers are going to be removed, You could leave this out, but libc++ won’t compile for GCC without them.
  • include/cwchar: I included the support header in order to get the functions required by the standard. this solves the cwchar test issue, but the <wchar.h> test will need to do this manually for _WIN32.

Please comment or apply, thanks.

Ruben

windows.patch (12.3 KB)

Thanks Ruben,

Some minor comments:

2011/9/28 Howard Hinnant <hhinnant@apple.com>

Hi,

I have removed Windows’ dependency on the ugly part that is going to be refactored by mirrorring Mac and FreeBSD through implementing all the necessary *_l functions and dependents.

Other notable changes:

  • support/win32/locale.h: fixed uselocale to enable per-thread locale, because the POSIX definition of this function requires that it only set the current thread’s locale. Consequence is that after a call to this function, per-thread locale will remain enabled. This is intended behavior as far as I can see. As a result, I’m fairly confident the __locale_raii trick works as it should even in multithreaded contexts.
  • include/locale: I still included the variadic template alternatives to the vararg functions. As these wrappers are going to be removed, You could leave this out, but libc++ won’t compile for GCC without them.
  • include/cwchar: I included the support header in order to get the functions required by the standard. this solves the cwchar test issue, but the <wchar.h> test will need to do this manually for _WIN32.

Please comment or apply, thanks.

Ruben

<windows.patch>

Thanks Ruben,

Some minor comments:


In buildit it looks like a line escaped outside of mingw:

ar rcs libc++.a *.o

Yeah, that’s a static library step I missed to remove. Feel free to delete it.


In it looks like you’re making some changes under:

#ifndef _LIBCPP_STABLE_APPLE_ABI

which is contrary to your first sentence above. Did you send the wrong patch?

I see, see below. I removed the now unnecessary workarounds.


In <__config> don’t you want to:

#if APPLE || _WIN32
#define _LIBCPP_STABLE_APPLE_ABI
#endif

instead of:

#if APPLE || FreeBSD || _WIN32
#define _LIBCPP_LOCALE__L_EXTENSIONS 1
#endif

?

I guess you’re right. I defined both in the new patch. libc++ compiles, but I’m kind of baffled by this because there are locale.cpp parts using _DefaultRuneLocale if _LIBCPP_STABLE_APPLE_ABI is defined. I didn’t touch these at all :s so I’m kind of confused why this works at all for me… I’m not seeing what the compiler is seeing :frowning:

These parts of locale.cpp are standard C locale things, so I guess the alternative I need to write is these bits using a hidden (static) C locale object I can always access for the values needed (this would involve modifying __cloc to not create a locale every time it is called, but I was waiting for the refactoring…). Why does FreeBSD not define _LLIBCPP_STABLE_APPLE_ABI?

I’m officially confumbled,

Ruben

windows.patch (9.98 KB)

I'm officially confumbled too, you're not alone. :slight_smile:

You're checked in. Hopefully with a fresh update things will start to unwind and simplify.

I think FreeBSD should define _LLIBCPP_STABLE_APPLE_ABI too. David? And maybe with David's latest patch to FreeBSD, should also #define _LIBCPP_GET_C_LOCALE 0.

Howard