On Thu, 7 Aug 2025 00:24:10 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
> The code in gtest-death-test-internal.h#L198-L212 referenced in the PR > description is conditionalized on GTEST_HAS_EXCEPTIONS, which leads me to > wonder why exceptions are enabled. If not for that, we wouldn't be in that > code. > > 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? > > https://github.com/openjdk/jdk/blame/f95af744b07a9ec87e2507b3d584cbcddc827bbd/make/hotspot/lib/CompileGtest.gmk#L71 > > https://github.com/openjdk/jdk/blame/f95af744b07a9ec87e2507b3d584cbcddc827bbd/make/hotspot/lib/CompileGtest.gmk#L101 > > That decision seems pretty old, like maybe from the initial introduction of > gtest. I haven't tracked down why, or whether the reasons are still valid. I > think it would be better to change that, assuming that's possible. This warning from the C++ compiler (after removing the 2 -EHsc lines) indicates that gtest code is using C++ exception handling: c:\progra~1\mib055~1\2022\enterprise\vc\tools\msvc\14.44.35207\include__msvc_ostream.hpp(781): error C2220: the following warning is treated as an error c:\progra~1\mib055~1\2022\enterprise\vc\tools\msvc\14.44.35207\include__msvc_ostream.hpp(781): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc c:\progra~1\mib055~1\2022\enterprise\vc\tools\msvc\14.44.35207\include__msvc_ostream.hpp(781): note: the template instantiation context (the oldest one first) is c:\repos\googletest\googletest\include\gtest/gtest-message.h(118): note: see reference to function template instantiation 'std::basic_ostream<char,std::char_traits<char>> &std::operator <<<std::char_traits<char>>(std::basic_ostream<char,std::char_traits<char>> &,const char *)' being compiled > 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. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26661#issuecomment-3165102917 PR Comment: https://git.openjdk.org/jdk/pull/26661#issuecomment-3165108189