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

Reply via email to