Add Windows conformance runner and testee support (#28130) Adds Windows support for the conformance runner and C++ conformance testee. For the conformance runner, this splits `ForkPipeRunner` into POSIX and Win32 implementations. The testee is switched to portable `FILE*` I/O, with binary mode on Windows. Also makes two related changes: * `waitpid(..., WEXITED)` is changed to `waitpid(..., 0)` because `WEXITED` is a `waitid()` flag. `waitpid(..., 0)` already waits for the child to terminate. * Adds `protobuf_configure_target` to the CMake conformance targets so they get the necessary target flags, e.g. `/utf-8`, which is important on Windows. Otherwise wires the change into CI so the Windows target functionally mirrors the existing macOS target. Resolves this existing Windows TODO in `fork_pipe_runner.cc`: ``` // TODO: make this work on Windows, instead of using these // UNIX-specific APIs. // // There is a platform-agnostic API in // src/google/protobuf/compiler/subprocess.h // // However that API only supports sending a single message to the subprocess. // We really want to be able to send messages and receive responses one at a // time: // // 1. Spawning a new process for each test would take way too long for thousands // of tests and subprocesses like java that can take 100ms or more to start // up. // // 2. Sending all the tests in one big message and receiving all results in one // big message would take away our visibility about which test(s) caused a // crash or other fatal error. It would also give us only a single failure // instead of all of them. ``` Closes #28130 #test-continuous FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/28130 from dcodeIO:windows 4872b82d608dff91672bad47229bb60542a8971a PiperOrigin-RevId: 968704895
diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index 02a8dd2..e62f41b 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml
@@ -450,6 +450,8 @@ strategy: fail-fast: false # Don't cancel all jobs if one fails. matrix: + # TODO: Add UTF-8 targets to + # Windows configurations below once rules_fuzz is fixed. include: - name: MacOS Bazel os: macos-14 @@ -462,15 +464,15 @@ - name: Windows Bazel os: windows-2022 cache_key: windows-2022-msvc-cl - bazel: test //src/... --config=msvc-cl --test_tag_filters=-conformance --build_tag_filters=-conformance + bazel: test //src/... conformance:conformance_framework_tests --config=msvc-cl - name: Windows Bazel C++20 os: windows-2022 cache_key: windows-2022-msvc-cl - bazel: test //src/... --config=msvc-cl --cxxopt="-std:c++20" --test_tag_filters=-conformance --build_tag_filters=-conformance + bazel: test //src/... conformance:conformance_framework_tests --config=msvc-cl --cxxopt="-std:c++20" - name: Windows Bazel clang-cl os: windows-2022 cache_key: windows-2022-clang-cl - bazel: test //src/... --test_tag_filters=-conformance --build_tag_filters=-conformance + bazel: test //src/... conformance:conformance_framework_tests name: ${{ matrix.continuous-only && inputs.continuous-prefix || '' }} ${{ matrix.name }} runs-on: ${{ matrix.os }} steps: @@ -504,7 +506,7 @@ - name: Windows CMake os: windows-2022 flags: >- - -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON -Dprotobuf_BUILD_SHARED_LIBS=OFF -Dprotobuf_BUILD_EXAMPLES=ON vsversion: '2022' @@ -512,7 +514,7 @@ - name: Windows CMake 32-bit os: windows-2022 flags: >- - -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON vsversion: '2022' windows-arch: 'win32' cache-prefix: windows-2022-win32-cmake @@ -520,16 +522,16 @@ - name: Windows CMake Shared os: windows-2022 flags: >- - -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON -Dprotobuf_BUILD_SHARED_LIBS=ON vsversion: '2022' cache-prefix: windows-2022-cmake - name: Windows CMake Install os: windows-2022 install-flags: >- - -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON flags: >- - -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON -Dprotobuf_REMOVE_INSTALLED_HEADERS=ON -Dprotobuf_BUILD_PROTOBUF_BINARIES=OFF vsversion: '2022'
diff --git a/conformance/BUILD b/conformance/BUILD index f94a990..e462ab2 100644 --- a/conformance/BUILD +++ b/conformance/BUILD
@@ -187,7 +187,11 @@ cc_library( name = "fork_pipe_runner", - srcs = ["fork_pipe_runner.cc"], + srcs = [ + "fork_pipe_runner.cc", + "fork_pipe_runner_posix.cc", + "fork_pipe_runner_win32.cc", + ], hdrs = ["fork_pipe_runner.h"], deps = [ ":conformance_cc_proto",
diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index e297675..6c9d75b 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc
@@ -2040,10 +2040,17 @@ {delim(""), delim("")}, {delim("Hello world!"), delim("Hello world!")}, {delim("\'\"\?\\\a\b\f\n\r\t\v"), - delim("\'\"\?\\\a\b\f\n\r\t\v")}, // escape + delim("\'\"\?\\\a\b\f\n\r\t\v")}, // escape +#ifndef _WIN32 + // MSVC doesn't support unicode literals or \u escapes without the + // /utf8 flag. All 3 of the following should produce the same result + // as the \x escape that follows. {delim("谷歌"), delim("谷歌")}, // Google in Chinese {delim("\u8C37\u6B4C"), delim("谷歌")}, // unicode escape {delim("\u8c37\u6b4c"), delim("谷歌")}, // lowercase unicode +#endif // !_WIN32 + {delim("\xe8\xb0\xb7\xe6\xad\x8c"), + delim("\xe8\xb0\xb7\xe6\xad\x8c")}, {delim("\xF0\x9F\x98\x81"), delim("\xF0\x9F\x98\x81")}, // emoji: 😁 }); @@ -2663,7 +2670,9 @@ ExpectParseFailureForJson("Int32FieldStringValuePartiallyNumericComma", REQUIRED, R"({"optionalInt32": "12,34"})"); ExpectParseFailureForJson("Int32FieldStringValuePartiallyNumericUnicode", - REQUIRED, R"({"optionalInt32": "12谷歌34"})"); + REQUIRED, + "{\"optionalInt32\": \"12\xe8\xb0\xb7\xe6\xad\x8c" + "34\"}"); ExpectParseFailureForJson("Int32FieldStringValueNonNumeric", REQUIRED, R"({"optionalInt32": "abc"})"); @@ -2824,7 +2833,9 @@ ExpectParseFailureForJson("FloatFieldStringValuePartiallyNumericComma", REQUIRED, R"({"optionalFloat": "12,34"})"); ExpectParseFailureForJson("FloatFieldStringValuePartiallyNumericUnicode", - REQUIRED, R"({"optionalFloat": "12谷歌34"})"); + REQUIRED, + "{\"optionalFloat\": \"12\xe8\xb0\xb7\xe6\xad\x8c" + "34\"}"); // Parser reject boolean values for float fields. ExpectParseFailureForJson("FloatFieldTrueValue", REQUIRED, @@ -2957,19 +2968,27 @@ RunValidJsonTest("StringField", REQUIRED, R"({"optionalString": "Hello world!"})", R"(optional_string: "Hello world!")"); + RunValidJsonTest("StringFieldEscape", REQUIRED, + R"({"optionalString": "\"\\\/\b\f\n\r\t"})", + R"(optional_string: "\"\\/\b\f\n\r\t")"); +#ifndef _WIN32 + // MSVC doesn't support unicode literals or \u escapes without the /utf8 flag. + // All 3 of the following should produce the same result as the \x escape + // that follows. RunValidJsonTest("StringFieldUnicode", REQUIRED, // Google in Chinese. R"({"optionalString": "谷歌"})", R"(optional_string: "谷歌")"); - RunValidJsonTest("StringFieldEscape", REQUIRED, - R"({"optionalString": "\"\\\/\b\f\n\r\t"})", - R"(optional_string: "\"\\/\b\f\n\r\t")"); RunValidJsonTest("StringFieldUnicodeEscape", REQUIRED, R"({"optionalString": "\u8C37\u6B4C"})", R"(optional_string: "谷歌")"); RunValidJsonTest("StringFieldUnicodeEscapeWithLowercaseHexLetters", REQUIRED, R"({"optionalString": "\u8c37\u6b4c"})", R"(optional_string: "谷歌")"); +#endif // !_WIN32 + RunValidJsonTest("StringFieldUnicodeBackslashXEscape", REQUIRED, + "{\"optionalString\": \"\xe8\xb0\xb7\xe6\xad\x8c\"}", + "optional_string: \"\xe8\xb0\xb7\xe6\xad\x8c\""); RunValidJsonTest( "StringFieldSurrogatePair", REQUIRED, // The character is an emoji: grinning face with smiling eyes. 😁
diff --git a/conformance/binary_wireformat_test.cc b/conformance/binary_wireformat_test.cc index 2e7d867..8400986 100644 --- a/conformance/binary_wireformat_test.cc +++ b/conformance/binary_wireformat_test.cc
@@ -95,38 +95,14 @@ INSTANTIATE_TEST_SUITE_P( TagTest, TagTest, testing::ValuesIn(std::vector<TagTestCase>{ - {.name = "Varint", - .fieldnum = 1, - .wire_type = WireType::kVarint, - .expected = "\010"}, - {.name = "Fixed32", - .fieldnum = 2, - .wire_type = WireType::kFixed32, - .expected = "\025"}, - {.name = "Fixed64", - .fieldnum = 3, - .wire_type = WireType::kFixed64, - .expected = "\031"}, - {.name = "LengthPrefixed", - .fieldnum = 4, - .wire_type = WireType::kLengthPrefixed, - .expected = "\042"}, - {.name = "StartGroup", - .fieldnum = 5, - .wire_type = WireType::kStartGroup, - .expected = "\053"}, - {.name = "EndGroup", - .fieldnum = 6, - .wire_type = WireType::kEndGroup, - .expected = "\064"}, - {.name = "TwoByteTag", - .fieldnum = 256, - .wire_type = WireType::kStartGroup, - .expected = "\203\020"}, - {.name = "ThreeByteTag", - .fieldnum = 0xFFFF, - .wire_type = WireType::kStartGroup, - .expected = "\373\377\037"}}), + {"Varint", 1, WireType::kVarint, "\010"}, + {"Fixed32", 2, WireType::kFixed32, "\025"}, + {"Fixed64", 3, WireType::kFixed64, "\031"}, + {"LengthPrefixed", 4, WireType::kLengthPrefixed, "\042"}, + {"StartGroup", 5, WireType::kStartGroup, "\053"}, + {"EndGroup", 6, WireType::kEndGroup, "\064"}, + {"TwoByteTag", 256, WireType::kStartGroup, "\203\020"}, + {"ThreeByteTag", 0xFFFF, WireType::kStartGroup, "\373\377\037"}}), [](const testing::TestParamInfo<TagTest::ParamType>& info) { return info.param.name; });
diff --git a/conformance/conformance_cpp.cc b/conformance/conformance_cpp.cc index 50becc0..08a68f7 100644 --- a/conformance/conformance_cpp.cc +++ b/conformance/conformance_cpp.cc
@@ -8,7 +8,6 @@ #include <errno.h> #include <stdarg.h> #include <stdlib.h> -#include <unistd.h> #include <cstddef> #include <cstdint> @@ -16,6 +15,11 @@ #include <memory> #include <string> +#ifdef _WIN32 +#include <fcntl.h> +#include <io.h> +#endif + #include "google/protobuf/any.pb.h" #include "google/protobuf/api.pb.h" #include "google/protobuf/duration.pb.h" @@ -68,15 +72,14 @@ using TestAllTypesProto3Editions = ::protobuf_test_messages::editions::proto3::TestAllTypesProto3; -absl::Status ReadFd(int fd, char* buf, size_t len) { +absl::Status ReadFile(FILE* file, char* buf, size_t len) { while (len > 0) { - ssize_t bytes_read = read(fd, buf, len); + size_t bytes_read = fread(buf, 1, len, file); if (bytes_read == 0) { - return absl::DataLossError("unexpected EOF"); - } - - if (bytes_read < 0) { + if (feof(file)) { + return absl::DataLossError("unexpected EOF"); + } return absl::ErrnoToStatus(errno, "error reading from test runner"); } @@ -86,9 +89,9 @@ return absl::OkStatus(); } -absl::Status WriteFd(int fd, const void* buf, size_t len) { - if (static_cast<size_t>(write(fd, buf, len)) != len) { - return absl::ErrnoToStatus(errno, "error reading to test runner"); +absl::Status WriteFile(FILE* file, const void* buf, size_t len) { + if (fwrite(buf, 1, len, file) != len) { + return absl::ErrnoToStatus(errno, "error writing to test runner"); } return absl::OkStatus(); } @@ -224,8 +227,7 @@ absl::StatusOr<bool> Harness::ServeConformanceRequest() { uint32_t in_len; - if (!ReadFd(STDIN_FILENO, reinterpret_cast<char*>(&in_len), sizeof(in_len)) - .ok()) { + if (!ReadFile(stdin, reinterpret_cast<char*>(&in_len), sizeof(in_len)).ok()) { // EOF means we're done. return true; } @@ -233,7 +235,7 @@ std::string serialized_input; serialized_input.resize(in_len); - RETURN_IF_ERROR(ReadFd(STDIN_FILENO, &serialized_input[0], in_len)); + RETURN_IF_ERROR(ReadFile(stdin, &serialized_input[0], in_len)); ConformanceRequest request; ABSL_CHECK(request.ParseFromString(serialized_input)); @@ -248,9 +250,12 @@ uint32_t out_len = internal::little_endian::FromHost( static_cast<uint32_t>(serialized_output.size())); - RETURN_IF_ERROR(WriteFd(STDOUT_FILENO, &out_len, sizeof(out_len))); - RETURN_IF_ERROR(WriteFd(STDOUT_FILENO, serialized_output.data(), - serialized_output.size())); + RETURN_IF_ERROR(WriteFile(stdout, &out_len, sizeof(out_len))); + RETURN_IF_ERROR( + WriteFile(stdout, serialized_output.data(), serialized_output.size())); + if (fflush(stdout) != 0) { + return absl::ErrnoToStatus(errno, "error flushing to test runner"); + } if (verbose_) { ABSL_LOG(INFO) << "conformance-cpp: request=" @@ -264,6 +269,10 @@ } // namespace google int main() { +#ifdef _WIN32 + _setmode(_fileno(stdin), _O_BINARY); + _setmode(_fileno(stdout), _O_BINARY); +#endif google::protobuf::Harness harness; int total_runs = 0; while (true) {
diff --git a/conformance/conformance_test_runner.cc b/conformance/conformance_test_runner.cc index a28862b..9d4a801 100644 --- a/conformance/conformance_test_runner.cc +++ b/conformance/conformance_test_runner.cc
@@ -30,11 +30,13 @@ // 3. testee sends 4-byte length M (little endian) // 4. testee sends M bytes representing a ConformanceResponse proto -#include <signal.h> #include <stdio.h> +#ifndef _WIN32 +#include <signal.h> #include <sys/types.h> #include <sys/wait.h> #include <unistd.h> +#endif #include <algorithm> #include <cctype>
diff --git a/conformance/fork_pipe_runner.cc b/conformance/fork_pipe_runner.cc index 3f447a8..ecd2dbe 100644 --- a/conformance/fork_pipe_runner.cc +++ b/conformance/fork_pipe_runner.cc
@@ -32,43 +32,20 @@ #include "fork_pipe_runner.h" -#include <errno.h> -#include <signal.h> -#include <stdio.h> -#include <sys/types.h> -#include <sys/wait.h> -#include <unistd.h> - -#include <chrono> // NOLINT(build/c++11) #include <cstdint> -#include <cstdio> -#include <cstdlib> -#include <cstring> -#include <future> // NOLINT(build/c++11) -#include <memory> #include <string> -#include <vector> #include "absl/log/absl_log.h" -#include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "conformance/conformance.pb.h" #include "google/protobuf/endian.h" -#define STRINGIFY(x) #x -#define TOSTRING(x) STRINGIFY(x) -#define CHECK_SYSCALL(call) \ - if (call < 0) { \ - perror(#call " " __FILE__ ":" TOSTRING(__LINE__)); \ - exit(1); \ - } - namespace google { namespace protobuf { std::string ForkPipeRunner::RunTest(absl::string_view test_name, absl::string_view request) { - if (child_pid_ < 0) { + if (!IsTestProgramRunning()) { SpawnTestProgram(); } current_test_name_ = std::string(test_name); @@ -76,37 +53,20 @@ uint32_t len = internal::little_endian::FromHost(static_cast<uint32_t>(request.size())); - CheckedWrite(write_fd_, &len, sizeof(uint32_t)); - CheckedWrite(write_fd_, request.data(), request.size()); + CheckedWrite(&len, sizeof(uint32_t)); + CheckedWrite(request.data(), request.size()); std::string response; - if (!TryRead(read_fd_, &len, sizeof(uint32_t))) { - // We failed to read from the child, assume a crash and try to reap. - ABSL_LOG(INFO) << "Trying to reap child, pid=" << child_pid_; - - int status = 0; - waitpid(child_pid_, &status, WEXITED); - - std::string error_msg; + bool timed_out = false; + if (!TryRead(&len, sizeof(uint32_t), &timed_out)) { conformance::ConformanceResponse response_obj; - if (WIFEXITED(status)) { - if (WEXITSTATUS(status) == 0) { - absl::StrAppendFormat(&error_msg, - "child timed out, killed by signal %d", - WTERMSIG(status)); - response_obj.set_timeout_error(error_msg); - } else { - absl::StrAppendFormat(&error_msg, "child exited, status=%d", - WEXITSTATUS(status)); - response_obj.set_runtime_error(error_msg); - } - } else if (WIFSIGNALED(status)) { - absl::StrAppendFormat(&error_msg, "child killed by signal %d", - WTERMSIG(status)); - } + std::string error_msg = GetTestProgramFailure(timed_out); ABSL_LOG(INFO) << error_msg; - child_pid_ = -1; - + if (timed_out) { + response_obj.set_timeout_error(error_msg); + } else { + response_obj.set_runtime_error(error_msg); + } // TODO: Remove this suppression. (void)response_obj.SerializeToString(&response); return response; @@ -114,138 +74,9 @@ len = internal::little_endian::ToHost(len); response.resize(len); - CheckedRead(read_fd_, (void *)response.c_str(), len); + CheckedRead((void*)response.c_str(), len); return response; } -// TODO: make this work on Windows, instead of using these -// UNIX-specific APIs. -// -// There is a platform-agnostic API in -// src/google/protobuf/compiler/subprocess.h -// -// However that API only supports sending a single message to the subprocess. -// We really want to be able to send messages and receive responses one at a -// time: -// -// 1. Spawning a new process for each test would take way too long for thousands -// of tests and subprocesses like java that can take 100ms or more to start -// up. -// -// 2. Sending all the tests in one big message and receiving all results in one -// big message would take away our visibility about which test(s) caused a -// crash or other fatal error. It would also give us only a single failure -// instead of all of them. -void ForkPipeRunner::SpawnTestProgram() { - int toproc_pipe_fd[2]; - int fromproc_pipe_fd[2]; - if (pipe(toproc_pipe_fd) < 0 || pipe(fromproc_pipe_fd) < 0) { - perror("pipe"); - exit(1); - } - - pid_t pid = fork(); - if (pid < 0) { - perror("fork"); - exit(1); - } - - if (pid) { - // Parent. - CHECK_SYSCALL(close(toproc_pipe_fd[0])); - CHECK_SYSCALL(close(fromproc_pipe_fd[1])); - write_fd_ = toproc_pipe_fd[1]; - read_fd_ = fromproc_pipe_fd[0]; - child_pid_ = pid; - } else { - // Child. - CHECK_SYSCALL(close(STDIN_FILENO)); - CHECK_SYSCALL(close(STDOUT_FILENO)); - CHECK_SYSCALL(dup2(toproc_pipe_fd[0], STDIN_FILENO)); - CHECK_SYSCALL(dup2(fromproc_pipe_fd[1], STDOUT_FILENO)); - - CHECK_SYSCALL(close(toproc_pipe_fd[0])); - CHECK_SYSCALL(close(fromproc_pipe_fd[1])); - CHECK_SYSCALL(close(toproc_pipe_fd[1])); - CHECK_SYSCALL(close(fromproc_pipe_fd[0])); - - std::unique_ptr<char[]> executable(new char[executable_.size() + 1]); - memcpy(executable.get(), executable_.c_str(), executable_.size()); - executable[executable_.size()] = '\0'; - - std::vector<const char *> argv; - argv.push_back(executable.get()); - ABSL_LOG(INFO) << argv[0]; - for (size_t i = 0; i < executable_args_.size(); ++i) { - argv.push_back(executable_args_[i].c_str()); - ABSL_LOG(INFO) << executable_args_[i]; - } - argv.push_back(nullptr); - // Never returns. - CHECK_SYSCALL(execv(executable.get(), const_cast<char **>(argv.data()))); - } -} - -void ForkPipeRunner::CheckedWrite(int fd, const void *buf, size_t len) { - if (static_cast<size_t>(write(fd, buf, len)) != len) { - ABSL_LOG(FATAL) << current_test_name_ - << ": error writing to test program: " << strerror(errno); - } -} - -bool ForkPipeRunner::TryRead(int fd, void *buf, size_t len) { - size_t ofs = 0; - while (len > 0) { - std::future<ssize_t> future = std::async( - std::launch::async, - [](int fd, void *buf, size_t ofs, size_t len) { - return read(fd, (char *)buf + ofs, len); - }, - fd, buf, ofs, len); - std::future_status status = future.wait_for(std::chrono::seconds(30)); - if (status == std::future_status::timeout) { - ABSL_LOG(ERROR) << current_test_name_ << ": timeout from test program"; - kill(child_pid_, SIGQUIT); - // TODO: Only log in flag-guarded mode, since reading output - // from SIGQUIT is slow and verbose. - std::vector<char> err; - err.resize(5000); - ssize_t err_bytes_read; - size_t err_ofs = 0; - do { - err_bytes_read = read(fd, (void *)&err[err_ofs], err.size() - err_ofs); - err_ofs += static_cast<size_t>(err_bytes_read); - } while (err_bytes_read > 0 && err_ofs < err.size()); - ABSL_LOG(ERROR) << "child_pid_=" << child_pid_ << " SIGQUIT: \n" - << &err[0]; - return false; - } - - ssize_t bytes_read = future.get(); - if (bytes_read == 0) { - ABSL_LOG(ERROR) << current_test_name_ - << ": unexpected EOF from test program"; - return false; - } else if (bytes_read < 0) { - ABSL_LOG(ERROR) << current_test_name_ - << ": error reading from test program: " - << strerror(errno); - return false; - } - - len -= static_cast<size_t>(bytes_read); - ofs += static_cast<size_t>(bytes_read); - } - - return true; -} - -void ForkPipeRunner::CheckedRead(int fd, void *buf, size_t len) { - if (!TryRead(fd, buf, len)) { - ABSL_LOG(FATAL) << current_test_name_ - << ": error reading from test program: " << strerror(errno); - } -} - } // namespace protobuf } // namespace google
diff --git a/conformance/fork_pipe_runner.h b/conformance/fork_pipe_runner.h index dea9f1a..f06e871 100644 --- a/conformance/fork_pipe_runner.h +++ b/conformance/fork_pipe_runner.h
@@ -9,7 +9,6 @@ #define GOOGLE_PROTOBUF_CONFORMANCE_FORK_PIPE_RUNNER_H__ #include <cstddef> -#include <cstdint> #include <string> #include <vector> @@ -26,32 +25,36 @@ class ForkPipeRunner : public ConformanceTestRunner { public: ForkPipeRunner(absl::string_view executable, - absl::Span<const std::string> executable_args) - : child_pid_(-1), - executable_(executable), - executable_args_(executable_args.begin(), executable_args.end()) {} + absl::Span<const std::string> executable_args); - explicit ForkPipeRunner(const std::string& executable) - : child_pid_(-1), executable_(executable) {} + explicit ForkPipeRunner(absl::string_view executable); - ~ForkPipeRunner() override = default; + ~ForkPipeRunner() override; + + ForkPipeRunner(const ForkPipeRunner&) = delete; + ForkPipeRunner& operator=(const ForkPipeRunner&) = delete; std::string RunTest(absl::string_view test_name, absl::string_view request) override; private: + // Platform-specific process and pipe handles live in the implementation + // files. + struct State; + void SpawnTestProgram(); - void CheckedWrite(int fd, const void* buf, size_t len); - bool TryRead(int fd, void* buf, size_t len); - void CheckedRead(int fd, void* buf, size_t len); + bool IsTestProgramRunning() const; + void CloseTestProgram(); + std::string GetTestProgramFailure(bool timed_out); + void CheckedWrite(const void* buf, size_t len); + bool TryRead(void* buf, size_t len, bool* timed_out); + void CheckedRead(void* buf, size_t len); - int write_fd_; - int read_fd_; - pid_t child_pid_; std::string executable_; const std::vector<std::string> executable_args_; std::string current_test_name_; + State* state_; }; } // namespace protobuf
diff --git a/conformance/fork_pipe_runner_posix.cc b/conformance/fork_pipe_runner_posix.cc new file mode 100644 index 0000000..84a6183 --- /dev/null +++ b/conformance/fork_pipe_runner_posix.cc
@@ -0,0 +1,228 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#include "fork_pipe_runner.h" + +#ifndef _WIN32 + +#include <errno.h> +#include <signal.h> +#include <stdio.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <unistd.h> + +#include <chrono> // NOLINT(build/c++11) +#include <cstddef> +#include <cstdio> +#include <cstdlib> +#include <cstring> +#include <future> // NOLINT(build/c++11) +#include <memory> +#include <string> +#include <vector> + +#include "absl/log/absl_log.h" +#include "absl/strings/str_format.h" +#include "absl/strings/string_view.h" +#include "absl/types/span.h" + +#define STRINGIFY(x) #x +#define TOSTRING(x) STRINGIFY(x) +#define CHECK_SYSCALL(call) \ + if (call < 0) { \ + perror(#call " " __FILE__ ":" TOSTRING(__LINE__)); \ + exit(1); \ + } + +namespace google { +namespace protobuf { + +struct ForkPipeRunner::State { + int write_fd = -1; + int read_fd = -1; + pid_t child_pid = -1; +}; + +ForkPipeRunner::ForkPipeRunner(absl::string_view executable, + absl::Span<const std::string> executable_args) + : executable_(executable), + executable_args_(executable_args.begin(), executable_args.end()), + state_(new State) {} + +ForkPipeRunner::ForkPipeRunner(absl::string_view executable) + : executable_(executable), state_(new State) {} + +ForkPipeRunner::~ForkPipeRunner() { + CloseTestProgram(); + delete state_; +} + +bool ForkPipeRunner::IsTestProgramRunning() const { + return state_->child_pid >= 0; +} + +void ForkPipeRunner::SpawnTestProgram() { + int toproc_pipe_fd[2]; + int fromproc_pipe_fd[2]; + if (pipe(toproc_pipe_fd) < 0 || pipe(fromproc_pipe_fd) < 0) { + perror("pipe"); + exit(1); + } + + pid_t pid = fork(); + if (pid < 0) { + perror("fork"); + exit(1); + } + + if (pid) { + // Parent. + CHECK_SYSCALL(close(toproc_pipe_fd[0])); + CHECK_SYSCALL(close(fromproc_pipe_fd[1])); + state_->write_fd = toproc_pipe_fd[1]; + state_->read_fd = fromproc_pipe_fd[0]; + state_->child_pid = pid; + } else { + // Child. + CHECK_SYSCALL(close(STDIN_FILENO)); + CHECK_SYSCALL(close(STDOUT_FILENO)); + CHECK_SYSCALL(dup2(toproc_pipe_fd[0], STDIN_FILENO)); + CHECK_SYSCALL(dup2(fromproc_pipe_fd[1], STDOUT_FILENO)); + + CHECK_SYSCALL(close(toproc_pipe_fd[0])); + CHECK_SYSCALL(close(fromproc_pipe_fd[1])); + CHECK_SYSCALL(close(toproc_pipe_fd[1])); + CHECK_SYSCALL(close(fromproc_pipe_fd[0])); + + std::unique_ptr<char[]> executable(new char[executable_.size() + 1]); + memcpy(executable.get(), executable_.c_str(), executable_.size()); + executable[executable_.size()] = '\0'; + + std::vector<const char*> argv; + argv.reserve(executable_args_.size() + 2); + argv.push_back(executable.get()); + ABSL_LOG(INFO) << argv[0]; + for (size_t i = 0; i < executable_args_.size(); ++i) { + argv.push_back(executable_args_[i].c_str()); + ABSL_LOG(INFO) << executable_args_[i]; + } + argv.push_back(nullptr); + // Never returns. + CHECK_SYSCALL(execv(executable.get(), const_cast<char**>(argv.data()))); + } +} + +void ForkPipeRunner::CheckedWrite(const void* buf, size_t len) { + if (static_cast<size_t>(write(state_->write_fd, buf, len)) != len) { + ABSL_LOG(FATAL) << current_test_name_ + << ": error writing to test program: " << strerror(errno); + } +} + +bool ForkPipeRunner::TryRead(void* buf, size_t len, bool* timed_out) { + *timed_out = false; + size_t ofs = 0; + while (len > 0) { + std::future<ssize_t> future = std::async( + std::launch::async, + [](int read_fd, void* buf, size_t ofs, size_t len) { + return read(read_fd, (char*)buf + ofs, len); + }, + state_->read_fd, buf, ofs, len); + std::future_status status = future.wait_for(std::chrono::seconds(30)); + if (status == std::future_status::timeout) { + ABSL_LOG(ERROR) << current_test_name_ << ": timeout from test program"; + *timed_out = true; + kill(state_->child_pid, SIGQUIT); + // TODO: Only log in flag-guarded mode, since reading output + // from SIGQUIT is slow and verbose. + std::vector<char> err; + err.resize(5000); + ssize_t err_bytes_read; + size_t err_ofs = 0; + do { + err_bytes_read = + read(state_->read_fd, (void*)&err[err_ofs], err.size() - err_ofs); + if (err_bytes_read > 0) { + err_ofs += static_cast<size_t>(err_bytes_read); + } + } while (err_bytes_read > 0 && err_ofs < err.size()); + ABSL_LOG(ERROR) << "child_pid_=" << state_->child_pid << " SIGQUIT: \n" + << &err[0]; + return false; + } + + ssize_t bytes_read = future.get(); + if (bytes_read == 0) { + ABSL_LOG(ERROR) << current_test_name_ + << ": unexpected EOF from test program"; + return false; + } else if (bytes_read < 0) { + ABSL_LOG(ERROR) << current_test_name_ + << ": error reading from test program: " + << strerror(errno); + return false; + } + + len -= static_cast<size_t>(bytes_read); + ofs += static_cast<size_t>(bytes_read); + } + + return true; +} + +void ForkPipeRunner::CheckedRead(void* buf, size_t len) { + bool timed_out = false; + if (!TryRead(buf, len, &timed_out)) { + ABSL_LOG(FATAL) << current_test_name_ + << ": error reading from test program: " << strerror(errno); + } +} + +std::string ForkPipeRunner::GetTestProgramFailure(bool timed_out) { + // We failed to read from the child, assume a crash and try to reap. + ABSL_LOG(INFO) << "Trying to reap child, pid=" << state_->child_pid; + + int status = 0; + waitpid(state_->child_pid, &status, 0); + + std::string error_msg; + if (timed_out) { + if (WIFSIGNALED(status)) { + absl::StrAppendFormat(&error_msg, "child timed out, killed by signal %d", + WTERMSIG(status)); + } else { + error_msg = "child timed out"; + } + } else if (WIFEXITED(status)) { + absl::StrAppendFormat(&error_msg, "child exited, status=%d", + WEXITSTATUS(status)); + } else if (WIFSIGNALED(status)) { + absl::StrAppendFormat(&error_msg, "child killed by signal %d", + WTERMSIG(status)); + } + CloseTestProgram(); + return error_msg; +} + +void ForkPipeRunner::CloseTestProgram() { + if (state_->write_fd >= 0) { + close(state_->write_fd); + state_->write_fd = -1; + } + if (state_->read_fd >= 0) { + close(state_->read_fd); + state_->read_fd = -1; + } + state_->child_pid = -1; +} + +} // namespace protobuf +} // namespace google + +#endif // !_WIN32
diff --git a/conformance/fork_pipe_runner_win32.cc b/conformance/fork_pipe_runner_win32.cc new file mode 100644 index 0000000..4f30e15 --- /dev/null +++ b/conformance/fork_pipe_runner_win32.cc
@@ -0,0 +1,268 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2026 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#ifdef _WIN32 + +#include "fork_pipe_runner.h" + +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include <windows.h> + +#include <chrono> // NOLINT(build/c++11) +#include <cstddef> +#include <future> // NOLINT(build/c++11) +#include <limits> +#include <sstream> +#include <string> + +#include "absl/log/absl_log.h" +#include "absl/strings/str_format.h" +#include "absl/strings/string_view.h" + +namespace google { +namespace protobuf { + +namespace { + +std::string WindowsErrorMessage(DWORD error) { + std::ostringstream oss; + oss << "Windows error " << error; + return oss.str(); +} + +std::string LastSystemError() { return WindowsErrorMessage(GetLastError()); } + +std::string QuoteCommandLineArg(absl::string_view arg) { + if (arg.empty()) { + return "\"\""; + } + + bool needs_quotes = false; + for (char c : arg) { + if (c == ' ' || c == '\t' || c == '"' || c == '\\') { + needs_quotes = true; + break; + } + } + if (!needs_quotes) { + return std::string(arg); + } + + std::string quoted = "\""; + size_t backslashes = 0; + for (char c : arg) { + if (c == '\\') { + ++backslashes; + } else if (c == '"') { + quoted.append(backslashes * 2 + 1, '\\'); + quoted.push_back(c); + backslashes = 0; + } else { + quoted.append(backslashes, '\\'); + backslashes = 0; + quoted.push_back(c); + } + } + quoted.append(backslashes * 2, '\\'); + quoted.push_back('"'); + return quoted; +} + +struct ReadChunk { + std::ptrdiff_t bytes_read; + DWORD error_code; +}; + +} // namespace + +struct ForkPipeRunner::State { + HANDLE child_process = NULL; + HANDLE write_handle = NULL; + HANDLE read_handle = NULL; +}; + +ForkPipeRunner::ForkPipeRunner(absl::string_view executable, + absl::Span<const std::string> executable_args) + : executable_(executable), + executable_args_(executable_args.begin(), executable_args.end()), + state_(new State) {} + +ForkPipeRunner::ForkPipeRunner(absl::string_view executable) + : executable_(executable), state_(new State) {} + +ForkPipeRunner::~ForkPipeRunner() { + CloseTestProgram(); + delete state_; +} + +bool ForkPipeRunner::IsTestProgramRunning() const { + return state_->child_process != NULL; +} + +void ForkPipeRunner::SpawnTestProgram() { + SECURITY_ATTRIBUTES security_attributes; + security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); + security_attributes.bInheritHandle = TRUE; + security_attributes.lpSecurityDescriptor = NULL; + + HANDLE child_stdin_read = NULL; + HANDLE child_stdin_write = NULL; + HANDLE child_stdout_read = NULL; + HANDLE child_stdout_write = NULL; + + if (!CreatePipe(&child_stdin_read, &child_stdin_write, &security_attributes, + 0) || + !SetHandleInformation(child_stdin_write, HANDLE_FLAG_INHERIT, 0) || + !CreatePipe(&child_stdout_read, &child_stdout_write, &security_attributes, + 0) || + !SetHandleInformation(child_stdout_read, HANDLE_FLAG_INHERIT, 0)) { + ABSL_LOG(FATAL) << "pipe setup failed: " << LastSystemError(); + } + + std::string command_line = QuoteCommandLineArg(executable_); + ABSL_LOG(INFO) << executable_; + for (const std::string& arg : executable_args_) { + command_line.push_back(' '); + command_line.append(QuoteCommandLineArg(arg)); + ABSL_LOG(INFO) << arg; + } + + STARTUPINFOA startup_info; + ZeroMemory(&startup_info, sizeof(startup_info)); + startup_info.cb = sizeof(startup_info); + startup_info.dwFlags = STARTF_USESTDHANDLES; + startup_info.hStdInput = child_stdin_read; + startup_info.hStdOutput = child_stdout_write; + startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); + + PROCESS_INFORMATION process_info; + ZeroMemory(&process_info, sizeof(process_info)); + if (!CreateProcessA(NULL, &command_line[0], NULL, NULL, TRUE, 0, NULL, NULL, + &startup_info, &process_info)) { + ABSL_LOG(FATAL) << "CreateProcess failed: " << LastSystemError(); + } + + CloseHandle(process_info.hThread); + CloseHandle(child_stdin_read); + CloseHandle(child_stdout_write); + state_->write_handle = child_stdin_write; + state_->read_handle = child_stdout_read; + state_->child_process = process_info.hProcess; +} + +void ForkPipeRunner::CheckedWrite(const void* buf, size_t len) { + DWORD bytes_written = 0; + if (len > static_cast<size_t>(std::numeric_limits<DWORD>::max()) || + !WriteFile(state_->write_handle, buf, static_cast<DWORD>(len), + &bytes_written, NULL) || + static_cast<size_t>(bytes_written) != len) { + ABSL_LOG(FATAL) << current_test_name_ + << ": error writing to test program: " << LastSystemError(); + } +} + +bool ForkPipeRunner::TryRead(void* buf, size_t len, bool* timed_out) { + *timed_out = false; + size_t offset = 0; + while (len > 0) { + std::future<ReadChunk> future = std::async( + std::launch::async, + [](HANDLE read_handle, void* buf, size_t offset, size_t len) { + DWORD bytes_read = 0; + if (len > static_cast<size_t>(std::numeric_limits<DWORD>::max())) { + len = static_cast<size_t>(std::numeric_limits<DWORD>::max()); + } + if (!ReadFile(read_handle, static_cast<char*>(buf) + offset, + static_cast<DWORD>(len), &bytes_read, NULL)) { + return ReadChunk{-1, GetLastError()}; + } + return ReadChunk{static_cast<std::ptrdiff_t>(bytes_read), 0}; + }, + state_->read_handle, buf, offset, len); + std::future_status status = future.wait_for(std::chrono::seconds(30)); + if (status == std::future_status::timeout) { + ABSL_LOG(ERROR) << current_test_name_ << ": timeout from test program"; + *timed_out = true; + TerminateProcess(state_->child_process, 1); + WaitForSingleObject(state_->child_process, 5000); + return false; + } + + ReadChunk chunk = future.get(); + if (chunk.bytes_read == 0) { + ABSL_LOG(ERROR) << current_test_name_ + << ": unexpected EOF from test program"; + return false; + } else if (chunk.bytes_read < 0) { + ABSL_LOG(ERROR) << current_test_name_ + << ": error reading from test program: " + << WindowsErrorMessage(chunk.error_code); + return false; + } + + len -= static_cast<size_t>(chunk.bytes_read); + offset += static_cast<size_t>(chunk.bytes_read); + } + + return true; +} + +void ForkPipeRunner::CheckedRead(void* buf, size_t len) { + bool timed_out = false; + if (!TryRead(buf, len, &timed_out)) { + ABSL_LOG(FATAL) << current_test_name_ + << ": error reading from test program: " + << LastSystemError(); + } +} + +std::string ForkPipeRunner::GetTestProgramFailure(bool timed_out) { + std::string error_msg; + if (timed_out) { + CloseTestProgram(); + return "child timed out"; + } + + DWORD status = 0; + if (state_->child_process != NULL) { + WaitForSingleObject(state_->child_process, 5000); + } + if (state_->child_process != NULL && + GetExitCodeProcess(state_->child_process, &status)) { + if (status == STILL_ACTIVE) { + error_msg = "child failed while still active"; + } else { + absl::StrAppendFormat(&error_msg, "child exited, status=%d", status); + } + } else { + absl::StrAppendFormat(&error_msg, "child failed: %s", LastSystemError()); + } + CloseTestProgram(); + return error_msg; +} + +void ForkPipeRunner::CloseTestProgram() { + if (state_->write_handle != NULL) { + CloseHandle(state_->write_handle); + state_->write_handle = NULL; + } + if (state_->read_handle != NULL) { + CloseHandle(state_->read_handle); + state_->read_handle = NULL; + } + if (state_->child_process != NULL) { + CloseHandle(state_->child_process); + state_->child_process = NULL; + } +} + +} // namespace protobuf +} // namespace google + +#endif // _WIN32
diff --git a/conformance/test_manager.cc b/conformance/test_manager.cc index ffa0f0f..f46bae1 100644 --- a/conformance/test_manager.cc +++ b/conformance/test_manager.cc
@@ -4,6 +4,7 @@ #include <cctype> #include <cstddef> #include <fstream> +#include <ios> #include <string> #include <vector> @@ -124,7 +125,7 @@ } absl::Status TestManager::SaveFailureList(absl::string_view filename) const { - std::ofstream outfile(std::string{filename}); + std::ofstream outfile(std::string{filename}, std::ios::binary); // Calculate alignment. size_t alignment = 0;
diff --git a/conformance/testee_test.cc b/conformance/testee_test.cc index cf02463..29e1fd0 100644 --- a/conformance/testee_test.cc +++ b/conformance/testee_test.cc
@@ -119,7 +119,7 @@ TestResult result = testee.CreateTest("foo", TestStrictness::kRequired) .ParseBinary(TestAllTypesProto2::descriptor(), Wire("wire")) - .SerializeText({.print_unknown_fields = true}); + .SerializeText({/*print_unknown_fields=*/true}); EXPECT_EQ(result.name(), "Required.Proto2.ProtobufInput.foo.TextFormatOutput"); @@ -169,7 +169,7 @@ TestResult result = testee.CreateTest("foo", TestStrictness::kRequired) .ParseJson(TestAllTypesProto2::descriptor(), "json", - {.ignore_unknown_fields = true}) + {/*ignore_unknown_fields=*/true}) .SerializeBinary(); EXPECT_EQ(result.name(), "Required.Proto2.JsonInput.foo.ProtobufOutput");
diff --git a/src/file_lists.cmake b/src/file_lists.cmake index 2081b86..774b8a9 100644 --- a/src/file_lists.cmake +++ b/src/file_lists.cmake
@@ -1176,6 +1176,8 @@ ${protobuf_SOURCE_DIR}/conformance/conformance_test_runner.cc ${protobuf_SOURCE_DIR}/conformance/failure_list_trie_node.cc ${protobuf_SOURCE_DIR}/conformance/fork_pipe_runner.cc + ${protobuf_SOURCE_DIR}/conformance/fork_pipe_runner_posix.cc + ${protobuf_SOURCE_DIR}/conformance/fork_pipe_runner_win32.cc ${protobuf_SOURCE_DIR}/conformance/text_format_conformance_suite.cc )