Skip to content

_Locinfo: Use _wsetlocale to query and restore locales #5781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

_Locinfo: Use _wsetlocale to query and restore locales #5781

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb5ad5b
_LocInfo: Use _wsetlocale to query and restore locales
lb90 Oct 14, 2025
7cb20fb
Keep ABI for `_Locinfo`
frederick-vs-ja Oct 16, 2025
77f93aa
Merge pull request #1 from frederick-vs-ja/fix-issue-5780-keeping-abi
lb90 Oct 16, 2025
46e08a9
Add test for GH #5780
lb90 Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stl/inc/xlocinfo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ private:
_Yarn<char> _Months; // month names
_Yarn<wchar_t> _W_Days; // wide weekday names
_Yarn<wchar_t> _W_Months; // wide month names
_Yarn<char> _Oldlocname; // old locale name to revert to on destruction
_Yarn<wchar_t> _Oldlocname; // old locale name to revert to on destruction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this breaks ABI: If the linker ends up choosing old _Locinfo::_Locinfo_ctor and new _Locinfo::_Locinfo_dtor or vice versa, one piece of code assumes that _Oldlocname holds a char* and the other assumes it holds a wchar_t*.

The changed class must receive a new name.

But we can't just outright remove the old class and change function signatures to use the new name, because it seems the old name appears in some exported symbols.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this is an ABI break. The _CRTIMP2_PURE_IMPORT in extern "C++" class _CRTIMP2_PURE_IMPORT _Locinfo indicates that the entire class is separately compiled, and its member functions live in locale.cpp and locale0.cpp which are being modified here. (This is an old, bad practice that we've strongly moved away from. When separately compiled code is necessary, we prefer to use "flat C exports" that accept plain C arguments. This gives us the most flexibility in the headers.)

This case is very interesting because aside from the definition in <xlocinfo>, _Oldlocname is never mentioned by the headers. It's only manipulated by locale.cpp and locale0.cpp. There's an important fact: There Is Only One STL (dll or static lib), and we never have to worry about mismatch between the STL's separately compiled components, only between outdated user code and newer STL dll/libs.

So while we can't mess with _Locinfo's name (at most, we could add a new name, but then we'd need to maintain a separate implementation, and I'm not sure if it could be header-only, or if it could be injected into the import lib without causing headaches especially for /clr and /clr:pure), and we can't change _Oldlocname's type without also changing _Locinfo's name, I think we could change _Oldlocname's content. Specifically, if _Locinfo::_Locinfo_ctor() stored UTF-8, then _Locinfo::_Locinfo_dtor() could decode and use it. Old user binaries running against the new STL DLL (or old user objects/libs linked against the new STL static LIB) would get matching new _Locinfo::_Locinfo_ctor() / _Locinfo::_Locinfo_dtor() talking UTF-8 to each other, but at no point would the user code actually care about the content of _Oldlocname, only its type of _Yarn<char>.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lb90 I opened lb90#1 for this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks a lot!

_Yarn<char> _Newlocname; // new locale name for this object
};
_STD_END
Expand Down
4 changes: 1 addition & 3 deletions stl/src/locale.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,7 @@ void __CLRCALL_PURE_OR_CDECL locale::_Locimp::_Locimp_Addfac(

void __CLRCALL_PURE_OR_CDECL _Locinfo::_Locinfo_ctor(
_Locinfo* pLocinfo, int cat, const char* locname) { // capture a named locale
const char* oldlocname = setlocale(LC_ALL, nullptr);

pLocinfo->_Oldlocname = oldlocname == nullptr ? "" : oldlocname;
pLocinfo->_Oldlocname = _wsetlocale(LC_ALL, nullptr);
_Locinfo_Addcats(pLocinfo, cat, locname);
}

Expand Down
7 changes: 2 additions & 5 deletions stl/src/locale0.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,8 @@ void __CLRCALL_PURE_OR_CDECL locale::_Locimp::_Locimp_dtor(_Locimp* _This) { //

void __CLRCALL_PURE_OR_CDECL _Locinfo::_Locinfo_ctor(
_Locinfo* pLocinfo, const char* locname) { // switch to a named locale
const char* oldlocname = setlocale(LC_ALL, nullptr);
pLocinfo->_Oldlocname = _wsetlocale(LC_ALL, nullptr);

pLocinfo->_Oldlocname = oldlocname == nullptr ? "" : oldlocname;
if (locname != nullptr) {
locname = setlocale(LC_ALL, locname);
}
Expand All @@ -233,9 +232,7 @@ void __CLRCALL_PURE_OR_CDECL _Locinfo::_Locinfo_ctor(
}

void __CLRCALL_PURE_OR_CDECL _Locinfo::_Locinfo_dtor(_Locinfo* pLocinfo) { // destroy a _Locinfo object, revert locale
if (!pLocinfo->_Oldlocname._Empty()) {
setlocale(LC_ALL, pLocinfo->_Oldlocname._C_str());
}
_wsetlocale(LC_ALL, pLocinfo->_Oldlocname._C_str());
}
_STD_END

Expand Down