googletest icon indicating copy to clipboard operation
googletest copied to clipboard

[Bug]: C++20: gtest attempts to invoke deleted stream insertion operators :: Still a problem for MSVC UNICODE builds

Open GerHobbelt opened this issue 2 years ago • 1 comments
trafficstars

Describe the issue

Same error as #3266, different cause (probably, if I read the entrails of #3266 right).

Steps to reproduce the problem

Build gtest code in MSVC2022 as a full UNICODE ("Use Unicode Character Set") build & /std:c++20 compiler option set. (We use our own in-house MSVC projects for that, enforcing unified builds of all libraries employed around here)

Results in this:

googletest\googletest\src\gtest.cc(5026,5): error C2280: 'std::basic_ostream<char,std::char_traits<char>> &std::operator <<<std::char_traits<char>>(std::basic_ostream<char,std::char_traits<char>> &,const wchar_t *)': attempting to reference a deleted function
1>C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\ostream(965,31): message : see declaration of 'std::operator <<'
googletest\googletest\src\gtest.cc(5026,5): error C2088: '<<': illegal for class

What version of GoogleTest are you using?

Bleeding edge master from this repo, patched in a few other places. (Did a code review/comparison, and this is issue exists in vanilla mainline as-is.)

What operating system and version are you using?

Win10/64, latest updates as of time of writing this issue.

What compiler and version are you using?

MSVC2022 Version 17.4.3

What build system are you using?

in-house managed MSVC2022 solution + project files.

Additional context

No response

GerHobbelt avatar Dec 29 '22 16:12 GerHobbelt

Fast fix: https://github.com/GerHobbelt/googletest/commit/f017ee751852e1082435c20a71c9084d289ee1d6

diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc
index e8ee1af8..143f59a5 100644
--- a/googletest/src/gtest.cc
+++ b/googletest/src/gtest.cc
@@ -5011,8 +5011,21 @@ void StreamingListener::SocketWriter::MakeConnection() {
   const int error_num =
       getaddrinfo(host_name_.c_str(), port_num_.c_str(), &hints, &servinfo);
   if (error_num != 0) {
-    GTEST_LOG_(WARNING) << "stream_result_to: getaddrinfo() failed: "
-                        << gai_strerror(error_num);
+	  GTEST_LOG_(WARNING) << "stream_result_to: getaddrinfo() failed: "
+		  /*
+		   * VC configurations may define UNICODE, to indicate to the C RTL that
+		   * WCHAR functions are preferred.
+		   * This affects functions like gai_strerror(), which is implemented as
+		   * an alias macro for gai_strerrorA() (which returns a const char *) or
+		   * gai_strerrorW() (which returns a const WCHAR *).  This source file
+		   * assumes POSIX declarations, so prefer the non-UNICODE definitions.
+		   */
+#if defined(UNICODE) && defined(gai_strerror)
+		  << gai_strerrorA(error_num)
+#else
+		  << gai_strerror(error_num)
+#endif
+		  ;
   }
 
   // Loop through all the results and connect to the first we can.

Comment has been ripped, pardon, äh, quoted 😉 straight from openssl\crypto\bio\bio_addr.c where they deal with this particular issue in a different (and a, for us, in gtest, quite undesirable, way: #undef UNICODE. 🤢 )

A system-wide search for gia_strerror here listed this as the only relevant spot where this issue (MSVC using gia_strerrorW) occurs. Of course, YMMV.

GerHobbelt avatar Dec 29 '22 16:12 GerHobbelt

You seem to be running a modified build of GoogleTest, as neither the line numbers nor the macro conditions match up. I assume you are on commit 71140c3ca7a87bb1b5b9c9f1500fea8858cce344. In particular, this condition

// Determines whether test results can be streamed to a socket.
#if GTEST_OS_LINUX || GTEST_OS_GNU_KFREEBSD || GTEST_OS_DRAGONFLY || GTEST_OS_FREEBSD || GTEST_OS_NETBSD || GTEST_OS_OPENBSD || GTEST_OS_GNU_HURD
#define GTEST_CAN_STREAM_RESULTS_ 1
#endif

suggests that the code you are trying to compile should be disabled on Windows, as the code says

#if GTEST_CAN_STREAM_RESULTS_
...
  if (error_num != 0) {
    GTEST_LOG_(WARNING) << "stream_result_to: getaddrinfo() failed: " << gai_strerror(error_num);
  }

To be honest, I cannot tell how you are compiling this code (as merely adding || GTEST_OS_WINDOWS to the condition produces many errors), but given that this is not a supported configuration and that the code is normally disabled on Windows, it does not seem like a bug, and so I would suggest maintaining a local patch for this along with the other patches.

higher-performance avatar Jan 09 '23 21:01 higher-performance