boost::execution_monitor impl under windows
From searching, this was last discussed in Sep 2003, but didn't come to an adequate conclusion - and things have changed since then anyway. Including prg_exec_monitor.hpp on VS2005 produces the following warning: execution_monitor.ipp(216) : warning C4535: calling _set_se_translator() requires /EHa It is my opinion that the use of _set_se_translator here is a mistake for the following reasons: 1. It doesn't achieve what it's trying to achieve if my test involves multiple threads. The translator is thread specific and there is no way for my thread function to apply the translator (especially not when I may not have access to the thread function). 2. It requires the use of /EHa which has an effect on the code being run. Code under test should be run using the same options as release, and /EHa is not appropriate for (most) release code. 3. If I've really triggered a system fault then I can no longer trust the state of my app, so I cannot rely on it to correctly report its problem. I may have just destroyed the stack, attempting to unwind is just going to cause more problems - system errors should be handled externally. I accept that others may prefer the simplicity of trying to handle system failures internally, so I propose changing to execution_monitor.ipp to only define BOOST_MS_STRUCTURED_EXCEPTION_HANDLING if BOOST_NO_MS_STRUCTURED_EXCEPTION_HANDLING is not defined: # ifndef BOOST_NO_MS_STRUCTURED_EXCEPTION_HANDLING # define BOOST_MS_STRUCTURED_EXCEPTION_HANDLING # endif I will then carry out my testing after running something like: "Program Files\Debugging Tools for Windows\cdb" -iaec "-c \".lines;!analyze -v;kP 4;q\"" to catch system errors. Thoughts? Jim PS. What I'd really like to see is a cross platform out of process JIT reporter for just one application. . ________________________________________________________________________ This e-mail, and any attachment, is confidential. If you have received it in error, do not use or disclose the information in any way, notify me immediately, and please delete it from your system. ________________________________________________________________________
James Talbut <James.Talbut <at> omg3d.com> writes:
From searching, this was last discussed in Sep 2003, but didn't come to an adequate conclusion - and things have changed since then anyway.
Including prg_exec_monitor.hpp on VS2005 produces the following warning: execution_monitor.ipp(216) : warning C4535: calling _set_se_translator() requires /EHa
In fact latest version of Boost.Test (see svn) doesn't use _set_se_translator for SE handling. In only used to implement "throw;" trick in case user requested not to catch SE.
It is my opinion that the use of _set_se_translator here is a mistake for the following reasons:
1. It doesn't achieve what it's trying to achieve if my test involves multiple threads. The translator is thread specific and there is no way for my thread function to apply the translator (especially not when I may not have access to the thread function).
a) Boost.Test is not thread safe anyway at the moment b) Thread-specific translator is what we would need in this case anyway, right?
2. It requires the use of /EHa which has an effect on the code being run. Code under test should be run using the same options as release, and /EHa is not appropriate for (most) release code.
a) I recommend testing debug builds b) You can always add /EHa to your release build options. It may affect performace of your application, but this is notthe point, right?
3. If I've really triggered a system fault then I can no longer trust the state of my app, so I cannot rely on it to correctly report its problem. I may have just destroyed the stack, attempting to unwind is just going to cause more problems - system errors should be handled externally.
Message above is only a warning. If you don't specify /EHa SE are not going to be caught anyway. The same result can be achieved by using -- catch_system_errors=no in case if you did specify above option during compilation. Both cases shoudl serve your purpose. Gennadiy
I am bothered by two aspects of the handling of SEH: 1. The whole idea of handling SEH from within the proc affected is doomed to fail in some circumstances (though only in some, so I'm not saying it's not worth doing anything). 2. Boost libraries should not produce any warnings on any supported compiler (or, at least, should be configurable so as to not produce any warnings).
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Gennadiy Rozental
In fact latest version of Boost.Test (see svn) doesn't use _set_se_translator for SE handling. In only used to implement "throw;" trick in case user requested not to catch SE. This trick isn't required if the compiler is a recent one and /EHa is not used (which it shouldn't ever be), so can't you get rid of it completely?
a) Boost.Test is not thread safe anyway at the moment What aspects of it aren't? I don't think the test harness should expect to have test macros used in other threads, but it should expect that other threads exist and do things unrelated to it.
b) Thread-specific translator is what we would need in this case anyway, right? Well, it's not possible to put any translators in place (you don't have control of the threads) so that won't help. You could try doing something with SetUnhandledExceptionFilter, but that still won't always work (threads could be created in DllMain before you get the chance to install the filter).
a) I recommend testing debug builds I recommend testing all builds.
b) You can always add /EHa to your release build options. It may affect performace of your application, but this is notthe point, right? On the contrary, it's absolutely the point. I want to test the code that I will actually be releasing - and this is performance sensitive code.
Message above is only a warning. If you don't specify /EHa SE are not going to be caught anyway. The same result can be achieved by using -- catch_system_errors=no in case if you did specify above option during compilation. Both cases shoudl serve your purpose. But I want to get rid of the warning. I wish it was possible to detect the use of /EHa from the code being compiled (so you could only use it if it was going to do some good), but it isn't. Actually if I remember correctly from when I last asked the compiler devs about it there is some horrendously hacky and version specific way to do it, but I don't think you want to go there.
Jim ________________________________________________________________________ This e-mail, and any attachment, is confidential. If you have received it in error, do not use or disclose the information in any way, notify me immediately, and please delete it from your system. ________________________________________________________________________
James Talbut <James.Talbut <at> omg3d.com> writes:
I am bothered by two aspects of the handling of SEH: 1. The whole idea of handling SEH from within the proc affected is doomed to fail in some circumstances (though only in some, so I'm not saying it's not worth doing anything). 2. Boost libraries should not produce any warnings on any supported compiler (or, at least, should be configurable so as to not produce any warnings).
specify /EHa and use --catch_system_errors=no and you got both
a) Boost.Test is not thread safe anyway at the moment What aspects of it aren't?
In many aspects. Starting with proper SEH handlign to the guarding internal framework structures.
I don't think the test harness should expect to have test macros used in other threads,
why not?
but it should expect that other threads exist and do things unrelated to it.
b) Thread-specific translator is what we would need in this case anyway, right? Well, it's not possible to put any translators in place (you don't have control of the threads) so that won't help. You could try doing something with SetUnhandledExceptionFilter, but that still won't always work (threads could be created in DllMain before you get the chance to install the filter).
I will have to revisit the status quo when/if I'll work on multithreading support.
b) You can always add /EHa to your release build options. It may affect performace of your application, but this is notthe point, right? On the contrary, it's absolutely the point. I want to test the code that I will actually be releasing - and this is performance sensitive code.
For many reasons I believe it may not be a good idea. But if you insist you can go ahead and build without /EHa.
Message above is only a warning. If you don't specify /EHa SE are not going to be caught anyway. The same result can be achieved by using -- catch_system_errors=no in case if you did specify above option during compilation. Both cases shoudl serve your purpose. But I want to get rid of the warning.
you can always suppress it in your code. Gennadiy
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Gennadiy Rozental
Message above is only a warning. If you don't specify /EHa SE are not going to be caught anyway. The same result can be achieved by using -- catch_system_errors=no in case if you did specify above option during compilation. Both cases shoudl serve your purpose. But I want to get rid of the warning.
you can always suppress it in your code.
Gennadiy
But I shouldn't need to, and it would be easier to put a #ifdef around the usage in the library so that no-one else needs to. The recommendation to use /EHa is not one that the MSVC devs would agree with: http://blogs.msdn.com/dcook/archive/2007/03/28/exceptional-wisdom.aspx so I think it's a mistake to encourage its use from a boost library. I think that the changes you've made to use __try instead of _set_se_translator are great and are (with the exception of trying to handle AVs) in keeping with all of these recommendations. It's just a shame that you've still got _set_se_translator and a recommendation to use /EHa in there. Jim ________________________________________________________________________ This e-mail, and any attachment, is confidential. If you have received it in error, do not use or disclose the information in any way, notify me immediately, and please delete it from your system. ________________________________________________________________________
James Talbut <James.Talbut <at> omg3d.com> writes:
you can always suppress it in your code. But I shouldn't need to, and it would be easier to put a #ifdef around the usage in the library so that no-one else needs to.
Any extra define complicate the library. It needs to be supported, considered during components updates and properly explained in documentation.
The recommendation to use /EHa is not one that the MSVC devs would agree with: http://blogs.msdn.com/dcook/archive/2007/03/28/exceptional-wisdom.aspx so I think it's a mistake to encourage its use from a boost library.
Ok. Thanks for the link. But I agree with comment: it's a bit oversimplified.
It's just a shame that you've still got _set_se_translator and a recommendation to use /EHa in there.
I guess I can remove _set_se_translator for VC8.0 and newer. This should prevent warning. Gennadiy
participants (2)
-
Gennadiy Rozental -
James Talbut