Setting locale to boost::locale::generator().generate("") causes crashes on macOS 11/arm64

[shawnanastasio]

SuperTux version: 15555065bbfd1e9f364bf1c86c6c2da8d37263fc System information: macOS 11.1, Apple M1/arm64

Expected behavior

Supertux runs

Actual behavior

Supertux crashes on startup

Steps to reproduce actual behavior

Build on macOS 11/arm64. Versions:

clang (Xcode): Apple clang version 12.0.0 (clang-1200.0.32.28) boost (brew): 1.75.0 SDL2 (brew): 2.0.14

The following patch stops the crashes, but according to a comment in the code relies on C++17 features (though on cppreference I'm seeing that the std::locale(string) construct was introduced in C++11, so I'm not sure exactly what part relies on C++17).

diff --git a/src/supertux/main.cpp b/src/supertux/main.cpp
index efd655855..79a11920c 100644
--- a/src/supertux/main.cpp
+++ b/src/supertux/main.cpp
@@ -569,7 +569,7 @@ Main::run(int argc, char** argv)
   // NOTE: when moving to C++ >= 17, keep the try-catch block, but use std::locale:global(std::locale(""));
   try
   {
-    std::locale::global(boost::locale::generator().generate(""));
+    std::locale::global(std::locale(""));
     // Make boost.filesystem use it
     boost::filesystem::path::imbue(std::locale());
   }

Additional debugging information

The following backtrace was recorded in lldb upon launching Supertux. Note the presence of libboost_locale functions at frame #2.

  * thread #1, queue = 'com.Metal.DeviceDispatch', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x0000000194426ad4 libc++abi.dylib`std::type_info::operator==(std::type_info const&) const + 12
    frame #1: 0x0000000194425e1c libc++abi.dylib`__dynamic_cast + 180
    frame #2: 0x0000000101643114 libboost_locale-mt.dylib`std::__1::istreambuf_iterator<char, std::__1::char_traits<char> > boost::locale::impl_icu::num_parse<char>::do_real_get<unsigned short>(std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, std::__1::istreambuf_iterator<char, std::__1::char_traits<char> >, std::__1::ios_base&, unsigned int&, unsigned short&) const + 80
    frame #3: 0x00000001943c3034 libc++.1.dylib`std::__1::basic_istream<char, std::__1::char_traits<char> >& std::__1::__input_arithmetic<unsigned short, char, std::__1::char_traits<char> >(std::__1::basic_istream<char, std::__1::char_traits<char> >&, unsigned short&) + 140
    frame #4: 0x000000010f4a4e10 AGXMetal13_3`___lldb_unnamed_symbol3764$$AGXMetal13_3 + 572
    frame #5: 0x00000001942b8420 libdispatch.dylib`_dispatch_client_callout + 20
    frame #6: 0x00000001942b9c38 libdispatch.dylib`_dispatch_once_callout + 32
    frame #7: 0x000000010f003d24 AGXMetal13_3`___lldb_unnamed_symbol41$$AGXMetal13_3 + 4972
    frame #8: 0x000000010f02ab84 AGXMetal13_3`___lldb_unnamed_symbol497$$AGXMetal13_3 + 76
    frame #9: 0x000000019bce45a8 Metal`-[MTLIOAccelService initWithAcceleratorPort:] + 400
    frame #10: 0x000000019bce43e0 Metal`+[MTLIOAccelService registerService:] + 160
    frame #11: 0x00000001942b8420 libdispatch.dylib`_dispatch_client_callout + 20
    frame #12: 0x00000001942c6a98 libdispatch.dylib`_dispatch_lane_barrier_sync_invoke_and_complete + 60
    frame #13: 0x000000019bd94f14 Metal`MTLIOAccelServiceRegisterService + 108
    frame #14: 0x000000019bce4200 Metal`+[MTLIOAccelDevice registerDevices] + 192
    frame #15: 0x000000019bd0d2fc Metal`invocation function for block in MTLDeviceArrayInitialize() + 1216
    frame #16: 0x00000001942b8420 libdispatch.dylib`_dispatch_client_callout + 20
    frame #17: 0x00000001942b9c38 libdispatch.dylib`_dispatch_once_callout + 32
    frame #18: 0x000000019bce4000 Metal`MTLCopyAllDevices + 244
    frame #19: 0x000000010d34b7f8 AppleMetalOpenGLRenderer`GLDDeviceRec::initWithDisplayMask(unsigned int) + 140
    frame #20: 0x000000010d351014 AppleMetalOpenGLRenderer`gldCreateDevice + 196
    frame #21: 0x00000001d6b641f0 libGFXShared.dylib`gfxInitializeLibrary + 1920
    frame #22: 0x00000001d6d57c18 GLEngine`gliInitializeLibrary + 28
    frame #23: 0x00000001d6b51610 OpenGL`___lldb_unnamed_symbol18$$OpenGL + 140
    frame #24: 0x00000001d6b514cc OpenGL`___lldb_unnamed_symbol17$$OpenGL + 116
    frame #25: 0x00000001d6b51290 OpenGL`___lldb_unnamed_symbol15$$OpenGL + 152
    frame #26: 0x00000001d6b5880c OpenGL`___lldb_unnamed_symbol62$$OpenGL + 2828
    frame #27: 0x00000001d6b57c28 OpenGL`CGLChoosePixelFormat + 100
    frame #28: 0x0000000197099564 AppKit`-[NSOpenGLPixelFormat initWithAttributes:] + 64
    frame #29: 0x00000001017f65e0 libSDL2-2.0.0.dylib`Cocoa_GL_CreateContext(_this=0x0000000104062a00, window=0x0000000103c9d670) at SDL_cocoaopengl.m:289:11 [opt]
    frame #30: 0x00000001017e520c libSDL2-2.0.0.dylib`SDL_GL_CreateContext_REAL(window=0x0000000103c9d670) at SDL_video.c:3624:11 [opt]
    frame #31: 0x000000010176738c libSDL2-2.0.0.dylib`SDL_GL_CreateContext(a=<unavailable>) at SDL_dynapi_procs.h:587:1 [opt]
    frame #32: 0x00000001007c9e40 supertux2`GLVideoSystem::create_gl_context() + 84
    frame #33: 0x00000001007c91f8 supertux2`GLVideoSystem::create_gl_window() + 368
    frame #34: 0x00000001007c8db8 supertux2`GLVideoSystem::GLVideoSystem(bool) + 232
    frame #35: 0x00000001007c9934 supertux2`GLVideoSystem::GLVideoSystem(bool) + 52
    frame #36: 0x000000010080609c supertux2`std::__1::__unique_if<GLVideoSystem>::__unique_single std::__1::make_unique<GLVideoSystem, bool>(bool&&) + 64
    frame #37: 0x0000000100805ccc supertux2`VideoSystem::create(VideoSystem::Enum) + 136
    frame #38: 0x0000000100634548 supertux2`Main::launch_game(CommandLineArguments const&) + 408
    frame #39: 0x00000001006366b8 supertux2`Main::run(int, char**) + 1196
    frame #40: 0x000000010000add4 supertux2`main + 60
    frame #41: 0x0000000194480f34 libdyld.dylib`start + 4

[shawnanastasio]

Update: My patch to use std::locale("") as the global locale ends up breaking the game in other ways, though it does solve the crashes. It causes the following error messages to be printed:

[WARNING] /Volumes/shawnanastasio_home/opt/supertux/src/supertux/sector.cpp:664 [/Volumes/shawnanastasio_home/opt/supertux/src/util/reader_error.hpp:64] <stream>:1: expected real in expression:
    8,736
[WARNING] /Volumes/shawnanastasio_home/opt/supertux/src/supertux/sector.cpp:664 [/Volumes/shawnanastasio_home/opt/supertux/src/util/reader_error.hpp:64] <stream>:1: expected real in expression:
    8,736
[WARNING] /Volumes/shawnanastasio_home/opt/supertux/src/supertux/sector.cpp:664 [/Volumes/shawnanastasio_home/opt/supertux/src/util/reader_error.hpp:64] <stream>:1: expected real in expression:
    8,768
[WARNING] /Volumes/shawnanastasio_home/opt/supertux/src/supertux/sector.cpp:664 [/Volumes/shawnanastasio_home/opt/supertux/src/util/reader_error.hpp:64] <stream>:1: expected real in expression:
    8,800
[WARNING] /Volumes/shawnanastasio_home/opt/supertux/src/supertux/sector.cpp:664 [/Volumes/shawnanastasio_home/opt/supertux/src/util/reader_error.hpp:64] <stream>:1: expected real in expression:
    8,832
[WARNING] /Volumes/shawnanastasio_home/opt/supertux/src/supertux/sector.cpp:664 [/Volumes/shawnanastasio_home/opt/supertux/src/util/reader_error.hpp:64] <stream>:1: expected real in expression:
    9,440
[WARNING] /Volumes/shawnanastasio_home/opt/supertux/src/supertux/sector.cpp:664 [/Volumes/shawnanastasio_home/opt/supertux/src/util/reader_error.hpp:64] <stream>:1: expected real in expression:
    9,536

Commenting out the call to std::locale::global all together solves both the crashes and the error messages above:

--- a/src/supertux/main.cpp
+++ b/src/supertux/main.cpp
@@ -569,7 +569,7 @@ Main::run(int argc, char** argv)
   // NOTE: when moving to C++ >= 17, keep the try-catch block, but use std::locale:global(std::locale(""));
   try
   {
-    std::locale::global(boost::locale::generator().generate(""));
+    //std::locale::global(std::locale(""));
     // Make boost.filesystem use it
     boost::filesystem::path::imbue(std::locale());
   }

This is obviously not an upstreamable change, but perhaps the call to std::locale::global can be ifdef'd out for macOS at least? The default locale on macOS should be UTF-8 compatible, so these calls shouldn't be necessary.

[tobbi]

@Grumbel If you have time, please take a look. I'm not knowledgeable enough about this.

[Grumbel]

The expected real in expression issue is caused by locale getting into the serialization and replacing the dot with a comma and can be fixed with this patch:

--- a/src/supertux/game_object_factory.cpp
+++ b/src/supertux/game_object_factory.cpp
@@ -284,6 +284,7 @@ std::unique_ptr<GameObject>
 GameObjectFactory::create(const std::string& name, const Vector& pos, const Direction& dir, const std::string& data) const
 {
   std::stringstream lisptext;
+  lisptext.imbue(std::locale::classic());
   lisptext << "(" << name << "\n"
            << " (x " << pos.x << ")"
            << " (y " << pos.y << ")" << data;

No idea what's going wrong with boost::locale::generator().generate("").

[tobbi]

@shawnanastasio Could you please incorporate the suggested changes by grumbel and update the PR? Thanks!

[shawnanastasio]

@shawnanastasio Could you please incorporate the suggested changes by grumbel and update the PR? Thanks!

Do you mean I should create a PR that incorporates Grumbel's changes along with my own? (This ticket is just a bug report.) Unless I'm mistaken, @Grumbel's patch doesn't affect the crashing with boost_locale, so we still need to decide how to handle that. My current thinking is that it might be best to just conditionally guard out the std::locale::global call on macOS, but if the project is fine with switching out the boost_locale call entirely for std::locale("") and potentially introducing a C++17 dependency, then I can submit that as well.

[Grumbel]

Does anybody know what boost::locale::generator().generate("") actually does? The documentation says it's setting the "system default locale", but which is that actually? It's not the same as std::locale(""), when I test it it looks more like std::locale::classic(), but if that's the case that whole block of code would seem unnecessary.

For reference:

• std::locale::classic() - standard C locale, default
• std::locale() - the globally set locale (i.e. what was set with std::locale::global(loc))
• std::locale("") - the user preferred locale (i.e. what was set with LC_ALL or LANG environment variables)

This all goes back to #706

Also worth mentioning that none of our serialization code currently handles locales, so my above patch isn't enough, the Writer and maybe other stuff would need to get fixed as well, as all of them expect std::locale::classic().

[Grumbel]

After a bit of further digging, it seems that boost::locale::generator().generate("") does give you something similar to std::locale(""), but with all the numeric formatting changed stripped out. So it won't break input/output, while std::locale("") would.

Back to the original issue, fixing the issue by just moving the #endif a bit further down to make all the locale initialization Win32-only is certainly an option, as I don't think we are using it for anything other than boost::filesystem::path::imbue(), which itself is only necessary for Win32 to begin with.

However I don't think the crash is a fault of the code to begin with. Seems more like some incompatible library build slipped in or something along the lines, boost::locale::generator().generate("") shouldn't crash just by itself.

[shawnanastasio]

After a bit of further digging, it seems that boost::locale::generator().generate("") does give you something similar to std::locale(""), but with all the numeric formatting changed stripped out. So it won't break input/output, while std::locale("") would.

...

However I don't think the crash is a fault of the code to begin with. Seems more like some incompatible library build slipped in or something along the lines, boost::locale::generator().generate("") shouldn't crash just by itself.

Interesting. It definitely sounds like some upstream incompatibility with boost-locale and macOS' system libraries rather than a supertux bug then.

Back to the original issue, fixing the issue by just moving the #endif a bit further down to make all the locale initialization Win32-only is certainly an option, as I don't think we are using it for anything other than boost::filesystem::path::imbue(), which itself is only necessary for Win32 to begin with.

I think this is a reasonable solution, and if everyone's okay with it then I can submit a PR.

[shawnanastasio]

Created https://github.com/SuperTux/supertux/pull/1663.

[tobbi]

I am going to close this, since #1663 got merged.