From 5e03386555544e39c21236dca0097123edec8769 Mon Sep 17 00:00:00 2001 From: Jonathan Albrecht Date: Fri, 4 Aug 2023 17:44:30 -0700 Subject: [PATCH] Conformance test runner big endian fixes (#13443) Add some missing endian conversions so that the conformance tests can be run on big endian platforms. The message length value created by the conformance test runner is little endian according to the comments in the file but actually was sent in the native endianness of the host. I was able to run the java, python, ruby, php and csharp test executables and they all expect little endian length values so those tests would hang on big endian machines. Only the cpp test executable was using native endian so it has been changed to expect little endian too. Also change the fixed32 and fixed64 functions in binary_json_conformance_test_suite.cc to send the data as little endian which fixes some failures in the python conformance tests on big endian platforms. Closes #13443 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/13443 from linux-on-ibm-z:conformance-runner-little-endian-fix 4ef79489971c9ff4eff524d75afdbe85cf5ce234 PiperOrigin-RevId: 553958649 --- conformance/binary_json_conformance_suite.cc | 25 +++++++++++++------- conformance/conformance_cpp.cc | 9 +++++-- conformance/conformance_test_runner.cc | 5 +++- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 745a40061361..ee2179084101 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -45,6 +45,7 @@ #include "json/json.h" #include "conformance/conformance.pb.h" #include "conformance_test.h" +#include "google/protobuf/endian.h" #include "google/protobuf/test_messages_proto2.pb.h" #include "google/protobuf/test_messages_proto3.pb.h" #include "google/protobuf/wire_format_lite.h" @@ -58,6 +59,7 @@ using google::protobuf::Descriptor; using google::protobuf::FieldDescriptor; using google::protobuf::Message; using google::protobuf::internal::WireFormatLite; +using google::protobuf::internal::little_endian::FromHost; using google::protobuf::util::NewTypeResolverForDescriptorPool; using proto2_messages::TestAllTypesProto2; using protobuf_test_messages::proto3::TestAllTypesProto3; @@ -117,9 +119,18 @@ string longvarint(uint64_t x, int extra) { return string(buf, len); } -// TODO: proper byte-swapping for big-endian machines. -string fixed32(void* data) { return string(static_cast(data), 4); } -string fixed64(void* data) { return string(static_cast(data), 8); } +string fixed32(void* data) { + uint32_t data_le; + std::memcpy(&data_le, data, 4); + data_le = FromHost(data_le); + return string(reinterpret_cast(&data_le), 4); +} +string fixed64(void* data) { + uint64_t data_le; + std::memcpy(&data_le, data, 8); + data_le = FromHost(data_le); + return string(reinterpret_cast(&data_le), 8); +} string delim(const string& buf) { return absl::StrCat(varint(buf.size()), buf); @@ -3100,12 +3111,8 @@ void BinaryAndJsonConformanceSuite::RunJsonTestsForValue() { }, true); RunValidJsonTestWithValidator( - "NullValueInNormalMessage", RECOMMENDED, - R"({"optionalNullValue": null})", - [](const Json::Value& value) { - return value.empty(); - }, - true); + "NullValueInNormalMessage", RECOMMENDED, R"({"optionalNullValue": null})", + [](const Json::Value& value) { return value.empty(); }, true); ExpectSerializeFailureForJson("ValueRejectNanNumberValue", RECOMMENDED, "optional_value: { number_value: nan}"); ExpectSerializeFailureForJson("ValueRejectInfNumberValue", RECOMMENDED, diff --git a/conformance/conformance_cpp.cc b/conformance/conformance_cpp.cc index 2fc9424ec8fe..a7d9c2e014f7 100644 --- a/conformance/conformance_cpp.cc +++ b/conformance/conformance_cpp.cc @@ -46,6 +46,7 @@ #include "absl/status/statusor.h" #include "conformance/conformance.pb.h" #include "conformance/conformance.pb.h" +#include "google/protobuf/endian.h" #include "google/protobuf/test_messages_proto2.pb.h" #include "google/protobuf/test_messages_proto3.pb.h" #include "google/protobuf/test_messages_proto3.pb.h" @@ -228,6 +229,7 @@ absl::StatusOr Harness::ServeConformanceRequest() { // EOF means we're done. return true; } + in_len = internal::little_endian::ToHost(in_len); std::string serialized_input; serialized_input.resize(in_len); @@ -242,9 +244,12 @@ absl::StatusOr Harness::ServeConformanceRequest() { std::string serialized_output; response->SerializeToString(&serialized_output); - uint32_t out_len = static_cast(serialized_output.size()); + uint32_t out_len = internal::little_endian::FromHost( + static_cast(serialized_output.size())); + RETURN_IF_ERROR(WriteFd(STDOUT_FILENO, &out_len, sizeof(out_len))); - RETURN_IF_ERROR(WriteFd(STDOUT_FILENO, serialized_output.data(), out_len)); + RETURN_IF_ERROR(WriteFd(STDOUT_FILENO, serialized_output.data(), + serialized_output.size())); if (verbose_) { ABSL_LOG(INFO) << "conformance-cpp: request=" << request.ShortDebugString() diff --git a/conformance/conformance_test_runner.cc b/conformance/conformance_test_runner.cc index 19497a1e1d05..d22213acaf04 100644 --- a/conformance/conformance_test_runner.cc +++ b/conformance/conformance_test_runner.cc @@ -69,6 +69,7 @@ #include "absl/strings/str_format.h" #include "conformance/conformance.pb.h" #include "conformance_test.h" +#include "google/protobuf/endian.h" using conformance::ConformanceResponse; using google::protobuf::ConformanceTestSuite; @@ -155,7 +156,8 @@ void ForkPipeRunner::RunTest(const std::string &test_name, } current_test_name_ = test_name; - uint32_t len = request.size(); + uint32_t len = + internal::little_endian::FromHost(static_cast(request.size())); CheckedWrite(write_fd_, &len, sizeof(uint32_t)); CheckedWrite(write_fd_, request.c_str(), request.size()); @@ -190,6 +192,7 @@ void ForkPipeRunner::RunTest(const std::string &test_name, return; } + len = internal::little_endian::ToHost(len); response->resize(len); CheckedRead(read_fd_, (void *)response->c_str(), len); }