Skip to content
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.

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

[BUG] fstream_mingw.h crashed at runtime #4336

Open
johnfea opened this issue Jul 11, 2024 · 0 comments
Open

[BUG] fstream_mingw.h crashed at runtime #4336

johnfea opened this issue Jul 11, 2024 · 0 comments

Comments

@johnfea
Copy link
Contributor

johnfea commented Jul 11, 2024

Describe the bug

I get runtime crash at fstream_mingw.h with mingw gcc.

However, it only happened when dynamically linking OIIO to OSL and static linking libstdc++ and libgcc for both, with mingw.

OpenImageIO version and dependencies

OpenImageIO 2.5.9.0
OpenShadingLanguage master

To Reproduce

Build OIIO dll with mingw with "-static-libstdc++ -static-libgcc"
Build OSL dll with mingw with "-static-libstdc++ -static-libgcc"
Link to OSL dll and call compile().

I had this issue with different mingw compilers including the one shipped with Debian 12. However, I had no problems using fstream_mingw.h in minimal tests.

Evidence

The problem stems from:
std::ios_base::_M_init()
when it calls:
std::locale::operator=(std::locale const&) ()

0 0x00007ffc8004f123 in ntdll!RtlIsZeroMemory () from C:\Windows\SYSTEM32\ntdll.dll #1 0x00007ffc80057ee2 in ntdll!RtlpNtSetValueKey () from C:\Windows\SYSTEM32\ntdll.dll #2 0x00007ffc800581ca in ntdll!RtlpNtSetValueKey () from C:\Windows\SYSTEM32\ntdll.dll #3 0x00007ffc8005de51 in ntdll!RtlpNtSetValueKey () from C:\Windows\SYSTEM32\ntdll.dll #4 0x00007ffc7ff75bf0 in ntdll!RtlGetCurrentServiceSessionId () from C:\Windows\SYSTEM32\ntdll.dll #5 0x00007ffc7ff747b1 in ntdll!RtlFreeHeap () from C:\Windows\SYSTEM32\ntdll.dll #6 0x00007ffc7fc49c9c in msvcrt!free () from C:\Windows\System32\msvcrt.dll #7 0x00007ffc4308bbe0 in std::locale::_Impl::~_Impl() () from libOpenImageIO_Util_d.dll #8 0x00007ffc4308a7b3 in std::locale::_Impl::_M_remove_reference() () from libOpenImageIO_Util_d.dll #9 0x00007ffc4308bfbe in std::locale::operator=(std::locale const&) () from libOpenImageIO_Util_d.dll #10 0x00007ffc430b60ad in std::ios_base::_M_init() () from libOpenImageIO_Util_d.dll #11 0x00007ffc430b7cb1 in std::basic_ios<char, std::char_traits<char> >::init(std::basic_streambuf<char, std::char_traits<char> >*) () from libOpenImageIO_Util_d.dll #12 0x00007ffc42fc2335 in OpenImageIO_v2_5::basic_ofstream<char, std::char_traits<char> >::open_internal ( this=0xc3e8a0, path=L"node_output_surface.oso", mode=std::_S_out) at OpenImageIO/src/include/OpenImageIO/fstream_mingw.h:267 #13 0x00007ffc42fc2408 in OpenImageIO_v2_5::basic_ofstream<char, std::char_traits<char> >::open (this=0xc3e8a0, path=L"node_output_surface.oso", __mode=std::_S_out) at OpenImageIO/src/include/OpenImageIO/fstream_mingw.h:294 #14 0x00007ffc42f7b66f in OpenImageIO_v2_5::Filesystem::open (stream=..., path=..., mode=std::_S_out) at OpenImageIO\src\libutil\filesystem.cpp:608 #15 0x00007ffc2e137ae7 in OSL_v1_14_1::pvt::OSLCompilerImpl::compile(OpenImageIO_v2_5::basic_string_view<char, std::char_traits<char> >, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, OpenImageIO_v2_5::basic_string_view<char, std::char_traits<char> >) () from liboslexec.dll

IF YOU ALREADY HAVE A CODE FIX: There is no need to file a separate issue

I did make a patch that optionally uses boost_nowide for mingw which fixed the issue. However, it seems boost is no longer a dependency for OIIO so this workaround doesn't apply to master without optionally re-introducing boost dependency for OIIO.

Not sure why boost_nowide solved the issue, it probably uses the same GCC specific headers to support UTF-16 filenames in Windows.


diff --git a/src/cmake/compiler.cmake b/src/cmake/compiler.cmake
index ce79003b2..5ce71d14c 100644
--- a/src/cmake/compiler.cmake
+++ b/src/cmake/compiler.cmake
@@ -269,6 +269,18 @@ if (USE_LIBCPLUSPLUS AND CMAKE_COMPILER_IS_CLANG)
 endif ()
 
 
+###########################################################################
+# Option to use boost_nowide for mingw builds instead of OIIO's own
+# fstream_mingw.h. Using it solved a runtime crash when dynamic linking
+# OIIO and static linking libgcc and libstdc++. Both OIIO and any client
+# code using either OIIO::ofstream or OIIO::ifstream need to link to
+# boost_nowide with this option set.
+option (USE_BOOST_NOWIDE "Use boost_nowide instead of fstream_mingw.h to support UTF-8 on Windows for MinGW builds only" OFF)
+if (USE_BOOST_NOWIDE)
+    message (STATUS "Using boost_nowide for mingw builds")
+    add_definitions (-DOIIO_BOOST_NOWIDE)
+endif ()
+
+
 ###########################################################################
 # For gcc >= 5, allow an option to force which version of the C++ ABI to
 # use (mostly this affects the implementation of std::string).
diff --git a/src/cmake/externalpackages.cmake b/src/cmake/externalpackages.cmake
index 3f73cd266..8615c7382 100644
--- a/src/cmake/externalpackages.cmake
+++ b/src/cmake/externalpackages.cmake
@@ -53,6 +53,9 @@ if (MSVC)
 endif ()
 
 set (Boost_COMPONENTS thread)
+if (OIIO_BOOST_NOWIDE)
+    list (APPEND Boost_COMPONENTS nowide)
+endif ()
 if (NOT USE_STD_FILESYSTEM)
     list (APPEND Boost_COMPONENTS filesystem)
 endif ()
diff --git a/src/include/OpenImageIO/filesystem.h b/src/include/OpenImageIO/filesystem.h
index dbaec7c6a..a1b8be0c8 100644
--- a/src/include/OpenImageIO/filesystem.h
+++ b/src/include/OpenImageIO/filesystem.h
@@ -34,7 +34,11 @@
 
 #if defined(_WIN32) && defined(__GLIBCXX__)
 #    define OIIO_FILESYSTEM_USE_STDIO_FILEBUF 1
-#    include <OpenImageIO/fstream_mingw.h>
+#    ifndef OIIO_BOOST_NOWIDE
+#        include <OpenImageIO/fstream_mingw.h>
+#    else
+#        include <boost/nowide/fstream.hpp>
+#    endif
 #endif
 
 
@@ -51,8 +55,14 @@ OIIO_NAMESPACE_BEGIN
 // of ifstream::open or ofstream::open. To properly support UTF-8 encoding on MingW we must
 // use the __gnu_cxx::stdio_filebuf GNU extension that can be used with _wfsopen and returned
 // into a istream which share the same API as ifsteam. The same reasoning holds for ofstream.
-typedef basic_ifstream<char> ifstream;
-typedef basic_ofstream<char> ofstream;
+#    ifndef OIIO_BOOST_NOWIDE
+     typedef basic_ifstream<char> ifstream;
+     typedef basic_ofstream<char> ofstream;
+#    else
+// Alternatively, use Boost nowide instead of directly using __gnu_cxx::stdio_filebuf
+     typedef boost::nowide::ifstream ifstream;
+     typedef boost::nowide::ofstream ofstream;
+#    endif
 #else
 typedef std::ifstream ifstream;
 typedef std::ofstream ofstream;
diff --git a/src/include/OpenImageIO/oiioversion.h.in b/src/include/OpenImageIO/oiioversion.h.in
index 9e2f84d69..58f4f3513 100644
--- a/src/include/OpenImageIO/oiioversion.h.in
+++ b/src/include/OpenImageIO/oiioversion.h.in
@@ -198,5 +198,7 @@ namespace @PROJ_NAME@ = @PROJ_NAMESPACE_V@;
 // Was the project built with TBB support?
 #cmakedefine01 OIIO_TBB
 
+// Was the project built with Boost_nowide support?
+#cmakedefine01 OIIO_BOOST_NOWIDE
 
 #endif  /* defined(OPENIMAGEIO_VERSION_H) */
diff --git a/src/libutil/CMakeLists.txt b/src/libutil/CMakeLists.txt
index f873b3eed..288fef84f 100644
--- a/src/libutil/CMakeLists.txt
+++ b/src/libutil/CMakeLists.txt
@@ -20,6 +20,7 @@ target_link_libraries (OpenImageIO_Util
             ${GCC_ATOMIC_LIBRARIES}
             ${OPENIMAGEIO_IMATH_DEPENDENCY_VISIBILITY}
             ${OPENIMAGEIO_IMATH_TARGETS}
+            $<TARGET_NAME_IF_EXISTS:Boost::nowide>
         PRIVATE
             $<TARGET_NAME_IF_EXISTS:Boost::filesystem>
             $<TARGET_NAME_IF_EXISTS:Boost::thread>
diff --git a/src/libutil/filesystem.cpp b/src/libutil/filesystem.cpp
index 0485f6aa9..faba8dd11 100644
--- a/src/libutil/filesystem.cpp
+++ b/src/libutil/filesystem.cpp
@@ -45,7 +45,11 @@ namespace filesystem = boost::filesystem;
 using boost::system::error_code;
 #endif
 
-
+#ifdef OIIO_FILESYSTEM_USE_STDIO_FILEBUF
+#ifdef OIIO_BOOST_NOWIDE
+#include <boost/nowide/fstream.hpp>
+#endif
+#endif
 
 OIIO_NAMESPACE_BEGIN
 
@@ -544,10 +548,15 @@ FILE*
 Filesystem::fopen(string_view path, string_view mode)
 {
 #ifdef _WIN32
+#   if !defined(OIIO_FILESYSTEM_USE_STDIO_FILEBUF) || !defined(OIIO_BOOST_NOWIDE)
     // on Windows fopen does not accept UTF-8 paths, so we convert to wide char
     std::wstring wpath = Strutil::utf8_to_utf16wstring(path);
     std::wstring wmode = Strutil::utf8_to_utf16wstring(mode);
     return ::_wfopen(wpath.c_str(), wmode.c_str());
+#   else
+    // Use boost_nowide with mingw to pass in UTF-8
+    return boost::nowide::fopen(std::string(path).c_str(), std::string(mode).c_str());
+#   endif
 #else
     // on Unix platforms passing in UTF-8 works
     return ::fopen(std::string(path).c_str(), std::string(mode).c_str());
@@ -584,7 +593,7 @@ void
 Filesystem::open(OIIO::ifstream& stream, string_view path,
                  std::ios_base::openmode mode)
 {
-#ifdef _WIN32
+#if defined(_WIN32) && (!defined(OIIO_FILESYSTEM_USE_STDIO_FILEBUF) || !defined(OIIO_BOOST_NOWIDE))
     // Windows std::ifstream accepts non-standard wchar_t*
     // On MingW, we use our own OIIO::ifstream
     std::wstring wpath = Strutil::utf8_to_utf16wstring(path);
@@ -601,7 +610,7 @@ void
 Filesystem::open(OIIO::ofstream& stream, string_view path,
                  std::ios_base::openmode mode)
 {
-#ifdef _WIN32
+#if defined(_WIN32) && (!defined(OIIO_FILESYSTEM_USE_STDIO_FILEBUF) || !defined(OIIO_BOOST_NOWIDE))
     // Windows std::ofstream accepts non-standard wchar_t*
     // On MingW, we use our own OIIO::ofstream
     std::wstring wpath = Strutil::utf8_to_utf16wstring(path);
@@ -616,7 +625,7 @@ Filesystem::open(OIIO::ofstream& stream, string_view path,
 int
 Filesystem::open(string_view path, int flags)
 {
-#ifdef _WIN32
+#if defined(_WIN32) && (!defined(OIIO_FILESYSTEM_USE_STDIO_FILEBUF) || !defined(OIIO_BOOST_NOWIDE))
     // on Windows _open does not accept UTF-8 paths, so we convert to wide
     // char and use _wopen.
     std::wstring wpath = Strutil::utf8_to_utf16wstring(path);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant