-
Notifications
You must be signed in to change notification settings - Fork 626
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
VS 2010 broken: exporting std::string subclass crashes #86
Comments
That's what I did as well. At the moment, I have no Windows machines to build on, so I can't reliably do the work and prepare a fix. I am wondering if you might submit a pull request? |
Sadly, this is out of my league. But if you could provide a patch as a starting point I'd be willing to test and try to make it work. |
As I recall, it was really straight forward. Basically make the following modification, and everywhere a string is assigned or referenced (for example from what()), make it operate on _message instead of the this pointer. The compiler will bitch until you are done, which makes it easy :) class BaseExc: public std::exception Really, the difficulty would not be at the OpenEXR end, but at the client software end, where there is a lot of code that tries to mess with the exception on the assumption that it is actually a string. As an alternative to modifying client code, I had a go at making BaseExc fully interoperable with string without being a string, which turns out to be a freak load of work and doesn't solve the problem in the end, because the code that wants Exc to be a string has got complicated data structures that require polymorphic compatibility with string, so it's best just to do the simple thing and fix the client code wherever the issue comes up. Since this is an EXR 2.0 we're working on here, maybe we can move ahead with this fix... |
I would not worry about the client code of The Foundry, SideFX or On 01/22/2014 06:34 AM, Nick Porcino wrote:
check out www.pointcloud9.com Sebastian Elsner - Pipeline Technical Director - RISE t: +49 30 20180300 florian@risefx.com RISE FX GmbH |
I have added all the changes you suggested, it compiles, but the warning: |
The next step is to adorn BaseExc. class IEX_EXPORT BaseExc ... and you should be good! |
Still: warning C4251: 'Iex_2_1::BaseExc::_message' : class 'std::basic_string<_Elem,_Traits,_Ax>' needs to have dll-interface to be used by clients of class 'Iex_2_1::BaseExc' I read a bit about the problem, (http://www.microsoft-questions.com/microsoft/VC-Language/30952961/a-solution-to-warning-c4251--class-needs-to-have-dllinterface.aspx), but the fix is still in the dark for me. |
In the case of std::string, this warning is harmless, because IIRC MS removed the static member variables from the string implementation in VS2008. That said, are you compiling your project with the multithreaded dll option? That's necessary. Also, once you have made sure you are running/linking with the DLL version of the MSVC libraries, is the _DLL macro defined for you at runtime? I don't recall if the compiler sets it automatically, if it doesn't, you'll have to set it in the preprocess section of the vcproj. If you are running MT DLL, and _DLL is set, and you get a warning, you should nonetheless be able to run successfully without crashing when an exception is thrown. |
Here are two more reasons to finish this modification. The issue described in the alembic post is the same, as is this issue from the exr mailing list. The problem stems from the members of the base exception being exported, not the class definition. Date: Wed, 12 Feb 2014 18:50:21 +1100 Hi Hope this is the forum to ask. I'm building pyilmbase 1.0.0 under llinux Centos 6.2, with ilmbase 1.0.3, gcc4.1.2 and boost 1.44.0 (also tried all combinations of the newer version of pyilmbase 2.0.0/1 against ilmbase 2.0 with newer compiler gcc446, and several flavours boost. In all cases I can build successfully and import iex just fine, but importing imath does SegFault, it does crashed on line 241 of PyIex.h 240│ const TypeTranslator<IEX_NAMESPACE::BaseExc>::ClassDesc *baseDesc = baseExcTranslator().template findClassDesc(baseExcTranslator().firstClassDesc()); baseDesc ends up with a nullPointer (const PyIex::TypeTranslator<Iex_2_0::BaseExc>::ClassDesc *) 0x0 As importing iex works fine, I changed the code in imathmodule.cpp to kind of copy how the Exception gets registered. < PyIex::registerExcImath::NullVecExc,Iex::MathExc("NullVecExc","imath"); < PyIex::registerExcImath::IntVecNormalizeExc,Iex::MathExc("IntVecNormalizeExc","imath");
This way I can import imath, and in python land I can see that the imath module contains the registered exceptions and I can raise/try/catch them. but this code change seems fishy since nothing else seems to have reported and the code in the latest version remains the same. I saw someone else did the same change (back in 2012) https://groups.google.com/forum/#!searchin/alembic-discussion/Cannot$20build$20windows/alembic-discussion/OMOvhg0Nv7o/VnJRrByVv10J but I'm pretty sure I am missing something more obvious also it caught my attention that I get this warning on gdb PyIex::TypeTranslator<Iex_2_0::BaseExc>::findClassDesc<Iex_2_0::MathExc> (this=0x6e6510, cd=0x6eaa00) at /scratch/federicon/git/rezExternalPackages/pyilmbase/2.0.0/build/2/pyilmbase-prefix/src/pyilmbase/PyIex/Py could it be related to the realtime type info? I made sure that ilmbase and pyilmbase were compiled wihout the -fno-rtti so it should be on by default Hope someone can help me find the issue Thanks in advance |
I have played around with the changes you proposed, but I do not feel confident about my c++ skills. For example there seem to be static functions left over, which I do not kown how to handle correctly.... |
I grepped the exception headers, which static functions do you mean? |
sorry for the late reply, busy times. I got the changes to work, but now openexr fails to compile because it does assumes in some places that exceptions are strings. For example: REPLACE_EXC (e, "Cannot read image file "
"\"" << fileName << "\". " << e); This does not work. I tried to overload with: std::string& operator<<(const BaseExc& rhs);
std::string& operator<<(const char *s);
std::string& operator<<(const std::string &s);
std::string& operator<<(std::stringstream &s); but I cannot get it to work. Is this the correct approach? |
sorry for the noise, hit the wrong button. |
Looking into the OpenEXR issue backlog. This appears to have been addressed in v2.3.0 |
The issue was already discussed on the mailing list, this is just for the sake of completion:
nick.porcino@gmail.com
Right, on windows one can't derive from standard string and then export the
class. When the exception is thrown, Iex will crash deep in heap code in
the Stdlib.
halfdan@sidefx.com
The problem is that BaseExc derives from std::string, which doesn't have a dllexport in MSVC2010 and onwards, and so in our case, actually broke the build.
We patched our version so that BaseExc uses std::string as a member variable, rather than deriving from it. This was easy enough to do, since this change only affects testBaseExc.cpp.
The text was updated successfully, but these errors were encountered: