On Fri, 8 Aug 2025 06:30:16 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
> 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. Thank you for this detailed background and explanation. Turns out we have 2 separate gtest runs happening, one that matches the GitHub actions test you pointed out and another that doesn't have the `--gtest_catch_exceptions=0` flag. I was focused on the latter scenario but based on your explanation, the correct resolution is for us to not run gtests without that flag. I will close this pull request and update our test infrastructure to remove that scenario. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26661#issuecomment-3180036471
