OpenEXR force enabling MSVC exceptions breaks unwinding through C code

[amyspark]

Hi,

This bug took me a bit to figure out. Ever since b4d5d86, /EHsc is forced upon all consumers of OpenEXR:

openexr/cmake/LibraryDefine.cmake

Lines 12 to 14 in 436fcd2

	if (MSVC)
	set(_openexr_extra_flags "/EHsc")
	endif()

However, there's a possibly unforeseen side effect. While -fexceptions allows traversing C code (to the best of my understanding),

-fexceptions

Enable exception handling. Generates extra code needed to propagate exceptions. For some targets, this implies GCC generates frame unwind information for all functions, which can produce significant data size overhead, although it does not affect execution. If you do not specify this option, GCC enables it by default for languages like C++ that normally require exception handling, and disables it for languages like C that do not normally require it. However, you may need to enable this option when compiling C code that needs to interoperate properly with exception handlers written in C++. You may also wish to disable this option if you are compiling older C++ programs that don’t use exception handling.

/EHsc definitely does not:

c
When used with /EHs, the compiler assumes that functions declared as extern "C" never throw a C++ exception. It has no effect when used with /EHa (that is, /EHca is equivalent to /EHa). /EHc is ignored if /EHs or /EHa aren't specified.

This subtly broke my attempt to trace an error in Krita's JPEG encoder that went through C code in MSVC, since one of the dependencies of the code publicly inherited OpenEXR's INTERFACE_COMPILE_OPTIONS property.

I believe a proper fix would be to narrow it to /EHs, as that will leave downstream consumers free to disable C code traversal as well, if necessary, otherwise leaving it undisturbed.

[meshula]

I think that's right, but I'm speculating without a way to try it out. Did you try narrowing it to /EHs, and did that resolve your problem? I'm also wondering if there might be an easy way to reproduce the problem, perhaps in our test suite?

[amyspark]

I think that's right, but I'm speculating without a way to try it out. Did you try narrowing it to /EHs, and did that resolve your problem? I'm also wondering if there might be an easy way to reproduce the problem, perhaps in our test suite?

Yes, this commit is the local change to Krita (I plan to push it once I've finished the branch, so commit hash is subject to change).

To reproduce it, you need to make it cross at least one DLL boundary. For instance, in this case we have three DLL boundaries:

1. Krita's main event loop OR QtConcurrent's manager (if called from a packaged task) (C++)
2. the JPEG plugin, which defines the throwable function to be called from libjpeg (C++)
3. libjpeg's functions (C)

If libjpeg calls the throwable function, which is done in response to an unrecoverable error, it is possible to make MSVC catch it. However, once told to Continue, the exception will unwind past the stack frames belonging to 3, past 2's catch clause (due to /EHsc) and finally hitting 1. Given we do not expect exceptions hit the event loop, this usually leads to process termination or deadlock.

[meshula]

Ok, thanks!

cc/ @kdt3rd