Skip to content

Commit

Permalink
Conformance test runner big endian fixes (protocolbuffers#13443)
Browse files Browse the repository at this point in the history
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 protocolbuffers#13443

COPYBARA_INTEGRATE_REVIEW=protocolbuffers#13443 from linux-on-ibm-z:conformance-runner-little-endian-fix 4ef7948
PiperOrigin-RevId: 553958649
  • Loading branch information
jonathan-albrecht-ibm authored and copybara-github committed Aug 5, 2023
1 parent d13d67b commit 5e03386
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 12 deletions.
25 changes: 16 additions & 9 deletions conformance/binary_json_conformance_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;
Expand Down Expand Up @@ -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<char*>(data), 4); }
string fixed64(void* data) { return string(static_cast<char*>(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<char*>(&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<char*>(&data_le), 8);
}

string delim(const string& buf) {
return absl::StrCat(varint(buf.size()), buf);
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions conformance/conformance_cpp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -228,6 +229,7 @@ absl::StatusOr<bool> 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);
Expand All @@ -242,9 +244,12 @@ absl::StatusOr<bool> Harness::ServeConformanceRequest() {
std::string serialized_output;
response->SerializeToString(&serialized_output);

uint32_t out_len = static_cast<uint32_t>(serialized_output.size());
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(), out_len));
RETURN_IF_ERROR(WriteFd(STDOUT_FILENO, serialized_output.data(),
serialized_output.size()));

if (verbose_) {
ABSL_LOG(INFO) << "conformance-cpp: request=" << request.ShortDebugString()
Expand Down
5 changes: 4 additions & 1 deletion conformance/conformance_test_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<uint32_t>(request.size()));
CheckedWrite(write_fd_, &len, sizeof(uint32_t));
CheckedWrite(write_fd_, request.c_str(), request.size());

Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 5e03386

Please sign in to comment.