On Fri, 8 Aug 2025 06:30:16 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> https://github.com/openjdk/jdk/commit/0054bbed7fce5b8566655d6910b09b10c952e609 >> (from https://bugs.openjdk.org/browse/JDK-8343756) caused the gtest death >> tests to fail on Windows with the error message "Caught >> std::exception-derived exception escaping the death test statement. >> Exception message: unknown file: error: SEH exception with code 0xc0000005 >> thrown in the test body." The error message is from the catch block in >> https://github.com/google/googletest/blob/v1.14.0/googletest/include/gtest/internal/gtest-death-test-internal.h#L198-L212 >> >> In the failing death tests, the gtests start another process and expect the >> child process to be terminated by JVM error handling code. However, the >> structured exception handling code in the googletest code ends up getting >> executed instead. The death tests expect execution to continue after the >> instruction that triggered the exception by writing to the poissoned page. >> This change proposes build Windows gtests without structured exception >> handling to avoid changing the flow of exceptions in OpenJDK test code. The >> effect of this change is to stop using the [SEH path in the >> HandleSehExceptionsInMethodIfSupported >> method](https://github.com/google/googletest/blob/v1.14.0/googletest/src/gtest.cc#L2603) >> and [directly start the >> test](https://github.com/google/googletest/blob/v1.14.0/googletest/src/gtest.cc#L2612). >> >> All the Windows gtests now pass with this change. > > So, we run gtests on Windows with `--gtest_catch_exceptions=0` which disables > the inbuilt exception handler. > > See issues > - https://bugs.openjdk.org/browse/JDK-8185734 > - https://bugs.openjdk.org/browse/JDK-8267138 (fixed an oversight in 8185734) > > See more details in my [PR description for > 8185734](https://github.com/openjdk/jdk/pull/1757#issue-765285610), that's > why this all sounded so familiar. > > The mechanism in place since 8267138 should be enough for my understanding. > @swesonga, can you find out why this is not sufficient in your case? Is this > option not passed on to your test? > >> That led me to wonder why, on Windows, we build libgtest and rebuild libjvm >> with exceptions enabled, by using -EHsc instead of no -EH option as done for >> the non-gtest libjvm? > > In the libjvm code itself, we don't use C++ exceptions, and we catch all > Windows SEH with `__try`/`__except`. We also don't want standard stack > unwinding with these signals. So no need for `/EHxx`. My assumption is that > the gtest framework itself uses C++ exceptions and therefore needs `/EHsc`. > The documentation is not super clear on some aspects of this > (https://learn.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model), > but it says that for standard C++ exceptions to work `/EHsc` is needed. > >> I was concerned that this might effectively undo JDK-8185734, since the >> suggestion in JBS was to conditionalize some code on GTEST_HAS_SEH being >> true. But it looks like the actual change didn't do that. > > In my PR for 8184734, I wrote: > > " In JBS, @kimbarrett suggested to build gtests with GTEST_HAS_SEH switched > off to prevent gtest from using SEH. Unfortunately that won't work since the > use of death tests means we need SEH. If we switch GTEST_HAS_SEH off, the > death tests don't build. I also do not like this suggestion since this > configuration may have a higher chance of bitrotting upstream." > > So it looks this was the first thing I tried back then, and it failed. > > How is it that we (Oracle) don't see these gtest death test failures in our > > CI? I'm guessing others (like SAP - @tstuefe ?) aren't either, since this > > issue is newly reported while the causing change was made more than 8 > > months ago. > > Should gtests be enabled in the pre-checkin GitHub actions to prevent > regressions? I think they take a few minutes to execute. They are already executed :-) https://github.com/swesonga/jdk/actions/runs/16735029827/job/47374721173 They are launched from the GtestWrapper.java with the `--gtest_catch_exceptions=0` option. That's why it is so strange that this does not work for your case. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26661#issuecomment-3166749667
