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

poco-1.6.0: Unicode Converter Test seg faults on Cygwin #723

Closed
drstacey opened this issue Feb 23, 2015 · 5 comments
Closed

poco-1.6.0: Unicode Converter Test seg faults on Cygwin #723

drstacey opened this issue Feb 23, 2015 · 5 comments

Comments

@drstacey
Copy link

@drstacey drstacey commented Feb 23, 2015

The Unicode Converter Test is instantiated twice, once with a Poco::UTF16String and once with a Poco::UTF32String. On Cygwin, the Poco::UTF16String version runs fine, but the Poco::UTF32String version seg faults.

I have been able to trace this a little. runTests() calls Poco::UnicodeConverter::convert(), which in turn appends characters to the wide string using std::basic_string<>::operator+=(). The string has insufficient capacity, so calls std::basic_string<>::reserve() to allocate some memory, and it is this call to reserve() that crashes.

On Cygwin, __SIZEOF_WCHAR_T__ is 2, so Poco::UTF16String is typedef'd to be a std::wstring, and Poco::UTF32String is a std::basic_string<> instantiated with UTF32Char and UTF32CharTraits. In this configuration, the Unicode Converter Test passes when instantiated with a Poco::UTF16String. However, if the definition of Poco::UTF16String is changed to be a std::basic_string<> instantiated with UTF16Char and UTF16CharTraits, then the Unicode Converter Test seg faults in both instantiations.

Did you see such crashes during testing? The reason I ask is that commit ac8c9ad wraps the Unicode Converter Test with #ifndef POCO_NO_WSTRING. This is confusing, as neither the test nor the routines being tested use wchar_t or std::wstring explicitly. Indeed, the whole point of Poco::UTF16String and Poco::UTF32String is that these routines will work even when std::wstring isn't defined. Please could you elaborate why this commit was made, and what the #ifdef is protecting.

@aleks-f
Copy link
Member

@aleks-f aleks-f commented Feb 24, 2015

AFAIK, you are right; POCO_NO_WSTRING is obsolete and not needed anymore. It was needed before the traits for wide characters were introduced.

As for the other problems you encountered, it is best if you can do the changes to fix them, run tests on Cygwin as well check regression on Windows and Linux and send pull request. Otherwise, it may take a while until we can address this.

@drstacey
Copy link
Author

@drstacey drstacey commented Mar 23, 2015

I have finally managed to reduce this down to a simple programme that exhibits the crash. The interesting thing is that the seg fault only happens when I link as shared - when linking statically everything works fine. So either I have made some silly mistake in creating the shared library, or there is something going on with the Cygwin linker.

It looks as though this isn't a Poco problem - but please be so good as to hold this ticket open a little longer until I know for sure.

@drstacey
Copy link
Author

@drstacey drstacey commented Apr 16, 2015

Finally, I think I understand what's going on. It's all down to the implementation of std::basic_string<> in libstdc++. In this, std::basic_string<> contains a static array of bytes that represent an empty string [1]. If your string happens to be empty, the internals of std::basic_string<> point at this byte array rather than dynamically creating storage. At various points in the std::basic_string<> code, it tests to see if the address of the internal storage matches this byte array and acts accordingly.

There is supposed to be exactly one of these byte arrays for each instantiation of std::basic_string<>. However, in this case there were two - one in the PocoFoundation shared DLL and one in the test harness executable. Hence testing the pointer failed (the internal storage was pointing at the 'wrong' static byte array), and the std::basic_string<> code tried to 'delete' and area of memory that was never 'new'ed. Bang!

Reading the gcc documentation [2], it appears that on Linux the compiler resolves this automatically by following the 'Borland' model, but on Cygwin it does not. Indeed, any platform that is not Linux ELF, Solaris 2 or native Windows will fall foul of this problem - I'm not sure how many targets you support would exhibit this problem.

The solution is to compile with '-frepo', which works, although it has quite an impact on the compilation time. Please could you add '-frepo' to the compiler flags when building shared libraries under Cygwin. As I said above, you may have to add this to any platform using libstdc++ that is not one of Linux ELF, Solaris 2 or native Windows.

Hope this is helpful, and thank you for your patience with this ticket.

[1] - basic_string.h line 178, member '_S_empty_rep_storage'.
[2] - https://gcc.gnu.org/onlinedocs/gcc-4.9.2/gcc/Template-Instantiation.html

@aleks-f
Copy link
Member

@aleks-f aleks-f commented Apr 16, 2015

Thanks, could you send pull request to development branch, please?

@drstacey
Copy link
Author

@drstacey drstacey commented Apr 23, 2015

The following patch is for build/config/CYGWIN. It corrects the problem where by Poco::UTF16String and Poco::UTF32String cause a seg fault when passed across a DLL boundary.

--- CYGWIN.old  2014-12-22 08:04:42.000000000 +0000
+++ CYGWIN  2015-04-22 23:26:08.219112100 +0100
@@ -39,10 +39,10 @@
 #
 # Compiler and Linker Flags
 #
-CFLAGS          = 
+CFLAGS          = -O2 -pipe -Wimplicit-function-declaration
 CFLAGS32        =
 CFLAGS64        =
-CXXFLAGS        = -DPOCO_NO_FPENVIRONMENT -DPOCO_NO_WSTRING -DPOCO_NO_SHAREDMEMORY
+CXXFLAGS        = -O2 -pipe
 CXXFLAGS32      =
 CXXFLAGS64      =
 LINKFLAGS       =
@@ -52,8 +52,8 @@
 STATICOPT_CXX   =
 STATICOPT_LINK  = -static
 SHAREDOPT_CC    = 
-SHAREDOPT_CXX   = 
-SHAREDOPT_LINK  = 
+SHAREDOPT_CXX   = -frepo
+SHAREDOPT_LINK  = -frepo
 DEBUGOPT_CC     = -g -D_DEBUG
 DEBUGOPT_CXX    = -g -D_DEBUG
 DEBUGOPT_LINK   = -g

It turned out that simply adding '-frepo' caused a rather strange linker error, so I have added additional compiler flags making this very similar to how Poco is built for the Cygwin repository.

Hope this is helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants