[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    =?utf-8?q?=5Bgraphics/krita=5D_/=3A_global=3A_Fix_ECM_exception_handling_semantics_mismatch_in_MSVC?
From:       L. E. Segovia <null () kde ! org>
Date:       2023-01-18 3:55:46
Message-ID: 20230118035546.16E4E1241048 () leptone ! kde ! org
[Download RAW message or body]

Git commit 9b7b48f4da5655035497be17e516b4a826f8b54e by L. E. Segovia.
Committed on 18/01/2023 at 02:42.
Pushed by lsegovia into branch 'master'.

global: Fix ECM exception handling semantics mismatch in MSVC

According to the Extra CMake Modules documentation [1],

> kde_enable_exceptions()
> 
> Enables exceptions for C++ source files compiled for the CMakeLists.txt
> file in the current directory and all subdirectories.

What isn't listed there, however, is that exceptions are enabled through
the usage of -fexceptions (GCC/Clang) and -EHsc (MSVC/Intel) [2]. All
good... except that those mean slightly different things:

> -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.

(source: [3])

> 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.

(source: [4])

These two put together mean that any exception thrown in a callstack
with C code in between works in Unix-y compilers, while they would
crash straight to Qt/QtConcurrent's event loop in MSVC.

The only affected piece of code is the JPEG error catcher,
introduced by Halla in 5c4c327ad429def4eaccc8d7bbd8f70d374a51ae, so
I added a file-level workaround there. A Krita-wide workaround is not
possible at the moment; all consumers of kritapigment would need the
same surgical workaround, because OpenEXR also injects /EHsc through
its exported targets. This was already reported upstream [5].

[1]: https://api.kde.org/ecm/kde-module/KDECompilerSettings.html
[2]: https://invent.kde.org/frameworks/extra-cmake-modules/-/blob/v5.101.0/kde-modules/KDECompilerSettings.cmake#L502-526
 [3]: https://gcc.gnu.org/onlinedocs/gcc-7.3.0/gcc/Code-Gen-Options.html#index-fexceptions
 [4]: https://learn.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=msvc-170#arguments
 [5]: https://github.com/AcademySoftwareFoundation/openexr/issues/1326

CCBUG: 361746
CCMAIL: kimageshop@kde.org

M  +4    -0    CMakeLists.txt
M  +7    -0    plugins/impex/jpeg/CMakeLists.txt

https://invent.kde.org/graphics/krita/commit/9b7b48f4da5655035497be17e516b4a826f8b54e

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 49c905561f..9491c6af82 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -647,6 +647,10 @@ if(MINGW)
 endif(MINGW)
 
 # enable exceptions globally
+# WARNING: in MSVC this will NOT catch exceptions thrown through C code
+# see: 
+# - https://learn.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=msvc-170#arguments
 +# - https://invent.kde.org/frameworks/extra-cmake-modules/-/blob/v5.101.0/kde-modules/KDECompilerSettings.cmake#L502-526
  kde_enable_exceptions()
 
 set(KRITA_DEFAULT_TEST_DATA_DIR ${CMAKE_SOURCE_DIR}/sdk/tests/data/)
diff --git a/plugins/impex/jpeg/CMakeLists.txt b/plugins/impex/jpeg/CMakeLists.txt
index c99eb6674b..1491519e3b 100644
--- a/plugins/impex/jpeg/CMakeLists.txt
+++ b/plugins/impex/jpeg/CMakeLists.txt
@@ -39,5 +39,12 @@ kis_add_library(kritajpegexport MODULE ${kritajpegexport_SOURCES})
 
 target_link_libraries(kritajpegexport kritaui kritaimpex ${JPEG_LIBRARIES} \
${LCMS2_LIBRARIES} LibExiv2::LibExiv2 )  
+if (MSVC AND "${CMAKE_CXX_FLAGS}" MATCHES "-EHsc") 
+    message(WARNING "The JPEG error catcher requires crossing the C ABI on MSVC. \
Exceptions need to be patched in.") +    # It needs to be done at the file level to \
override OpenEXR sneaking /EHsc. +    # See \
https://github.com/AcademySoftwareFoundation/openexr/issues/1326 +    \
set_source_files_properties(kis_jpeg_converter.cc PROPERTIES COMPILE_OPTIONS \
"/EHsc-") +endif()
+
 install(TARGETS kritajpegexport  DESTINATION ${KRITA_PLUGIN_INSTALL_DIR})
 install( PROGRAMS  krita_jpeg.desktop  DESTINATION ${XDG_APPS_INSTALL_DIR})


[prev in list] [next in list] [prev in thread] [next in thread]