From 8956216492296b3ec9d3238328d94517daeffca4 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Sun, 30 Oct 2022 07:40:37 -0500 Subject: [PATCH] Win32: Using /EHr, annotate inner_bootstrap again as noexcept to try to force the termination that we otheriwse can't seem to get. --- setup.py | 2 +- src/greenlet/greenlet.cpp | 35 +++++++++++++++-------- src/greenlet/greenlet_compiler_compat.hpp | 6 ++++ src/greenlet/greenlet_greenlet.hpp | 2 +- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/setup.py b/setup.py index 4c2e728..a4248af 100755 --- a/setup.py +++ b/setup.py @@ -55,7 +55,7 @@ # with it. Leaving off the "c" should just result in slower, safer code. # Other options: # "r" == Always generate standard confirming checks for noexcept blocks, terminating - # if violated. + # if violated. IMPORTANT: We rely on this. # See https://docs.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=msvc-160 handler = "/EHsr" cpp_compile_args.append(handler) diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp index 9d30fa2..7777e9f 100644 --- a/src/greenlet/greenlet.cpp +++ b/src/greenlet/greenlet.cpp @@ -1270,7 +1270,7 @@ UserGreenlet::g_initialstub(void* mark) explicitly to us. Either way, the ``err`` variable is created twice at the same memory location, but possibly having different ``origin`` values. Note that it's not - constructed for the second time until the switch actually happens.` + constructed for the second time until the switch actually happens. */ if (err.status == 1) { // This never returns! @@ -1297,7 +1297,7 @@ UserGreenlet::g_initialstub(void* mark) void -UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) +UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) G_NOEXCEPT_WIN32 { // The arguments here would be another great place for move. // As it is, we take them as a reference so that when we clear @@ -1370,19 +1370,28 @@ UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) catch(...) { // Unhandled C++ exception! - // If we don't catch this here, most platforms will just - // abort() the process. But on 64-bit Windows with older - // versions of the C runtime, this can actually corrupt - // memory and just return. We see this when compiling with - // the Windows 7.0 SDK targeting Windows Server 2008, but - // not when using the Appveyor Visual Studio 2019 image. - // So this currently only affects Python 2.7 on Windows - // 64. That is, the tests pass and the runtime aborts + // If we declare ourselves as noexcept, if we don't catch + // this here, most platforms will just abort() the + // process. But on 64-bit Windows with older versions of + // the C runtime, this can actually corrupt memory and + // just return. We see this when compiling with the + // Windows 7.0 SDK targeting Windows Server 2008, but not + // when using the Appveyor Visual Studio 2019 image. So + // this currently only affects Python 2.7 on Windows 64. + // That is, the tests pass and the runtime aborts // everywhere else. // // However, if we catch it and try to continue with a // Python error, then all Windows 64 bit platforms corrupt - // memory. So all we can do is manually abort. + // memory. So all we can do is manually abort, hopefully + // with a good error message. (Note that the above was + // tested WITHOUT the `/EHr` switch being used at compile + // time, so MSVC may have "optimized" out important + // checking. Using that switch, we may be in a better + // place in terms of memory corruption.) But sometimes it + // can't be caught here at all, which is confusing but not + // terribly surprising; so again, the G_NOEXCEPT_WIN32 + // plus "/EHr". // // Hopefully the basic C stdlib is still functional enough // for us to at least print an error. @@ -1391,7 +1400,9 @@ UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) // platforms, specifically at least Linux/gcc/libstdc++. They use // an exception to unwind the stack when a background // thread exits. (See comments about G_NOEXCEPT.) So this - // may not actually represent anything untoward. + // may not actually represent anything untoward. On those + // platforms we allow throws of this to propagate, or + // attempt to anyway. # if defined(WIN32) || defined(_WIN32) Py_FatalError( "greenlet: Unhandled C++ exception from a greenlet run function. " diff --git a/src/greenlet/greenlet_compiler_compat.hpp b/src/greenlet/greenlet_compiler_compat.hpp index 779ccf5..ecaeb32 100644 --- a/src/greenlet/greenlet_compiler_compat.hpp +++ b/src/greenlet/greenlet_compiler_compat.hpp @@ -122,5 +122,11 @@ typedef unsigned int uint32_t; # define UNUSED(x) UNUSED_ ## x #endif +#if defined(_MSC_VER) +# define G_NOEXCEPT_WIN32 G_NOEXCEPT +#else +# define G_NOEXCEPT_WIN32 +#endif + #endif diff --git a/src/greenlet/greenlet_greenlet.hpp b/src/greenlet/greenlet_greenlet.hpp index f9a1b83..292fce4 100644 --- a/src/greenlet/greenlet_greenlet.hpp +++ b/src/greenlet/greenlet_greenlet.hpp @@ -589,7 +589,7 @@ namespace greenlet protected: virtual switchstack_result_t g_initialstub(void* mark); private: - void inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run); + void inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run) G_NOEXCEPT_WIN32; }; class MainGreenlet : public Greenlet