Hi!
On 8/8/2018 3:56 PM, Stephan Bergmann wrote:
On 07/08/18 18:14, Stephan Bergmann wrote:
As we speak, I happen to run into your CppunitTest_cppuhelper_qa_misc
failure with my MSVC build (with latest Visual Studio 2017, see above)
too, will investigate. (The test currently only runs (late) during
`make check`, see <https://gerrit.libreoffice.org/#/c/58696/> "No need
for CppunitTest_cppuhelper_qa_misc to be a subsequentcheck".)
Mike is working on a fix at <https://gerrit.libreoffice.org/#/c/58730/>
"Don't break __CxxDetectRethrow contract"
I seem to be unable to come with a solution, so I decided to summarize
my findings here.
So, with commit 6ddecf61ecada646fbd6f8809270d47289727e8a, Caolán has
uncovered a hidden bug that must have been there for some time. Function
cpp_call() in bridges/source/cpp_uno/msvc_win32_*/uno2cpp.cxx uses SEH
(__try-__except statement) to guard the called method;
CPPU_CURRENT_NAMESPACE::mscx_filterCppException() is used there as the
__except expression; and the latter uses CRT's *internal*
__CxxDetectRethrow() function to check if the handled C++ exception is
being re-thrown. __CxxDetectRethrow() checks the opaque exception
descriptor passed by pointer (which has internal EHExceptionRecord*
type), and basically detects if two conditions are true: the exception
is C++ exception (using PER_IS_MSVC_EH macro), and that it has no
exception information (using PER_PTHROW macro); see comment in
ExFilterRethrow in MSVCRT's crt/src/vcruntime/frame.cpp for the same check.
Unfortunately, the function __CxxDetectRethrow() modifies global state:
it increments an internal variable (__ProcessingThrow++), which is used
throughout CRT's exception handling code to detect that unwinding is in
progress when its value is non-zero (one comment says "we are
unwinding... possibly called from a dtor()"). In that case, e.g.,
std::current_exception() returns empty exception_ptr (related code is in
CRT's __ExceptionPtr::_CurrentException() method). As documented in the
beginning of the crt/src/vcruntime/mgdframe.cpp, where
__CxxDetectRethrow() is defined, the function is used internally when
C++ exceptions are handled in COM+. It is designed to be used in
specific conditions, and after some other companion functions have been
called (specifically, __CxxExceptionFilter and
__CxxRegisterExceptionObject); finally, __CxxUnregisterExceptionObject
must be called. All the mentioned functions also modify
__ProcessingThrow; they also modify other internal variables (global
state), allocate and release memory; and __CxxUnregisterExceptionObject
also destructs the exception object.
All of the described complexity made me unable to come with a proper way
to call those functions: they expect some state of SEH; they modify the
state; not calling __CxxExceptionFilter leads to still unpaired
decrements of __ProcessingThrow; trying to finally call
__CxxUnregisterExceptionObject destroys the object that we tested in our
mscx_filterCppException, which leads to use-after-delete. So the problem
now remains: it's not enough to just revert the Caolán's commit
mentioned above, since we obviously break CRT state now and then, and
the revert would just hide this, not solve.
The related code (using __CxxDetectRethrow to check for the exception
being rethrown) was introduced with commit
37d3cdd8d280f509ffa37e47c4706213f4dcda7c from 2003; but I don't know if
the function already had current behaviour back then.
Just for reference, I paste here the reference code given in
crt/src/vcruntime/mgdframe.cpp as the example of how the functions work
together:
////////////////////////////////////////////////////////////////////////////////
// Model of C++ eh in COM+
//
// void func()
// {
// try {
// TryBody();
// } catch (cpp_object o)
// {
// CatchOBody();
// } catch (...)
// {
// CatchAllBody();
// }
// }
//
// Turns into this:
//
//
// void func()
// {
// int rethrow;
// // One per try block
// int isCxxException;
// // One per catch(...)
// __try {
// TryBody();
// }
// __except(__CxxExceptionFilter(exception,
// typeinfo(cpp_object),
// flags,
// &o))
// // This is how it's done already
// {
// // Begin catch(object) prefix
// char *storage = _alloca(__CxxQueryExceptionSize());
// rethrow = false;
// __CxxRegisterExceptionObject(exception,
// storage);
// __try {
// __try {
// // End catch(object) prefix
// CatchOBody();
// // Begin catch(object) suffix
// } __except(rethrow = __CxxDetectRethrow(exception),
// EXCEPTION_CONTINUE_SEARCH)
// {}
// }
// __finally
// {
// __CxxUnregisterExceptionObject(storage,
// rethrow);
// }
// // End catch(object) suffix
// }
// __except(1)
// {
// // Begin catch(...) prefix
// char *storage = _alloca(__CxxQueryExceptionSize());
// rethrow = false;
// isCxxException = __CxxRegisterExceptionObject(exception,
// storage);
// __try
// {
// __try
// {
// // End catch(...) prefix
// CatchAllBody();
// // Begin catch(...) suffix
// } __except(rethrow = __CxxDetectRethrow(exception),
// EXCEPTION_CONTINUE_SEARCH)
// {}
// } __finally
// {
// if (isCxxException)
// __CxxUnregisterExceptionObject(storage, rethrow);
// }
// // End catch(...) suffix
// }
// }
//
////////////////////////////////////////////////////////////////////////////////
--
Best regards,
Mike Kaganski
Context
- Re: Windows Debug builds failure: 'DbgGUIInitSolarMutexCheck': identifier not found (continued)
Privacy Policy |
Impressum (Legal Info) |
Copyright information: Unless otherwise specified, all text and images
on this website are licensed under the
Creative Commons Attribution-Share Alike 3.0 License.
This does not include the source code of LibreOffice, which is
licensed under the Mozilla Public License (
MPLv2).
"LibreOffice" and "The Document Foundation" are
registered trademarks of their corresponding registered owners or are
in actual use as trademarks in one or more countries. Their respective
logos and icons are also subject to international copyright laws. Use
thereof is explained in our
trademark policy.