diff --git a/cpp/src/arrow/flight/transport/grpc/customize_grpc.h b/cpp/src/arrow/flight/transport/grpc/customize_grpc.h index 4a64e4e07214..3698f2f135c8 100644 --- a/cpp/src/arrow/flight/transport/grpc/customize_grpc.h +++ b/cpp/src/arrow/flight/transport/grpc/customize_grpc.h @@ -112,6 +112,13 @@ ::grpc::Status FlightDataSerialize(const arrow::flight::FlightPayload& msg, // protobuf without copying ARROW_FLIGHT_EXPORT ::grpc::Status FlightDataDeserialize( ::grpc::ByteBuffer* buffer, arrow::flight::internal::FlightData* out); + +// The Arrow structs carried by the `pb::FlightData` messages that WritePayload() +// and ReadPayload() give to gRPC (see serialization_internal.cc). +ARROW_FLIGHT_EXPORT const arrow::flight::FlightPayload& FlightPayloadOfMessage( + const void* message); +ARROW_FLIGHT_EXPORT arrow::flight::internal::FlightData* FlightDataOfMessage( + void* message); } // namespace grpc } // namespace transport } // namespace flight @@ -128,16 +135,16 @@ class SerializationTraits { #endif public: - // In the functions below, we cast back the Message argument to its real - // type (see ReadPayload() and WritePayload() for the initial cast). + // The message carries the Arrow struct to serialize or to deserialize into + // (see ReadPayload() and WritePayload()). static Status Serialize(const MessageType& msg, ByteBuffer* bb, bool* own_buffer) { return arrow::flight::transport::grpc::FlightDataSerialize( - *reinterpret_cast(&msg), bb, own_buffer); + arrow::flight::transport::grpc::FlightPayloadOfMessage(&msg), bb, own_buffer); } static Status Deserialize(ByteBuffer* buffer, MessageType* msg) { return arrow::flight::transport::grpc::FlightDataDeserialize( - buffer, reinterpret_cast(msg)); + buffer, arrow::flight::transport::grpc::FlightDataOfMessage(msg)); } }; diff --git a/cpp/src/arrow/flight/transport/grpc/serialization_internal.cc b/cpp/src/arrow/flight/transport/grpc/serialization_internal.cc index ee13a8e2022f..bbe90709ed50 100644 --- a/cpp/src/arrow/flight/transport/grpc/serialization_internal.cc +++ b/cpp/src/arrow/flight/transport/grpc/serialization_internal.cc @@ -17,6 +17,7 @@ #include "arrow/flight/transport/grpc/serialization_internal.h" +#include #include #include #include @@ -197,78 +198,111 @@ ::grpc::Status FlightDataDeserialize(ByteBuffer* buffer, return ::grpc::Status::OK; } -// The pointer bitcast hack below causes legitimate warnings, silence them. +// offsetof() below is used on types that are not standard-layout, which is +// conditionally-supported. GCC, Clang and MSVC support it for types without +// virtual base classes, as here. #ifndef _WIN32 # pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wstrict-aliasing" +# pragma GCC diagnostic ignored "-Winvalid-offsetof" #endif -// Pointer bitcast explanation: grpc::*Writer::Write() and grpc::*Reader::Read() -// both take a T* argument (here pb::FlightData*). But they don't do anything -// with that argument except pass it to SerializationTraits::Serialize() and -// SerializationTraits::Deserialize(). -// -// Since we control SerializationTraits, we can interpret the -// pointer argument whichever way we want, including cast it back to the original type. -// (see customize_grpc.h). +// grpc::*Writer::Write() and grpc::*Reader::Read() take a T (here +// pb::FlightData) and only pass it to SerializationTraits::Serialize() and +// SerializationTraits::Deserialize(), which we control (see customize_grpc.h). +// So instead of building the protobuf message, we give gRPC an empty, real +// pb::FlightData that carries the Arrow struct, and the traits read the struct +// from it. The message must be a real pb::FlightData: an Arrow struct cast to +// pb::FlightData is not one, which is undefined behavior and which Clang CFI +// (`-fsanitize=cfi-unrelated-cast`) rejects. + +namespace { + +struct FlightPayloadMessage { + pb::FlightData message; + const FlightPayload* payload; +}; + +struct FlightDataMessage { + pb::FlightData message; + flight::internal::FlightData* data; +}; + +} // namespace + +// `message` points to the `message` member of one of the structs above. +const FlightPayload& FlightPayloadOfMessage(const void* message) { + return *reinterpret_cast( + static_cast(message) - offsetof(FlightPayloadMessage, message)) + ->payload; +} + +flight::internal::FlightData* FlightDataOfMessage(void* message) { + return reinterpret_cast(static_cast(message) - + offsetof(FlightDataMessage, message)) + ->data; +} arrow::Result WritePayload( const FlightPayload& payload, ::grpc::ClientReaderWriter* writer) { RETURN_NOT_OK(payload.Validate()); - // Pretend to be pb::FlightData and intercept in SerializationTraits - return writer->Write(*reinterpret_cast(&payload), - ::grpc::WriteOptions()); + FlightPayloadMessage message; + message.payload = &payload; + return writer->Write(message.message, ::grpc::WriteOptions()); } arrow::Result WritePayload( const FlightPayload& payload, ::grpc::ClientReaderWriter* writer) { RETURN_NOT_OK(payload.Validate()); - // Pretend to be pb::FlightData and intercept in SerializationTraits - return writer->Write(*reinterpret_cast(&payload), - ::grpc::WriteOptions()); + FlightPayloadMessage message; + message.payload = &payload; + return writer->Write(message.message, ::grpc::WriteOptions()); } arrow::Result WritePayload( const FlightPayload& payload, ::grpc::ServerReaderWriter* writer) { RETURN_NOT_OK(payload.Validate()); - // Pretend to be pb::FlightData and intercept in SerializationTraits - return writer->Write(*reinterpret_cast(&payload), - ::grpc::WriteOptions()); + FlightPayloadMessage message; + message.payload = &payload; + return writer->Write(message.message, ::grpc::WriteOptions()); } arrow::Result WritePayload(const FlightPayload& payload, ::grpc::ServerWriter* writer) { RETURN_NOT_OK(payload.Validate()); - // Pretend to be pb::FlightData and intercept in SerializationTraits - return writer->Write(*reinterpret_cast(&payload), - ::grpc::WriteOptions()); + FlightPayloadMessage message; + message.payload = &payload; + return writer->Write(message.message, ::grpc::WriteOptions()); } bool ReadPayload(::grpc::ClientReader* reader, flight::internal::FlightData* data) { - // Pretend to be pb::FlightData and intercept in SerializationTraits - return reader->Read(reinterpret_cast(data)); + FlightDataMessage message; + message.data = data; + return reader->Read(&message.message); } bool ReadPayload(::grpc::ClientReaderWriter* reader, flight::internal::FlightData* data) { - // Pretend to be pb::FlightData and intercept in SerializationTraits - return reader->Read(reinterpret_cast(data)); + FlightDataMessage message; + message.data = data; + return reader->Read(&message.message); } bool ReadPayload(::grpc::ServerReaderWriter* reader, flight::internal::FlightData* data) { - // Pretend to be pb::FlightData and intercept in SerializationTraits - return reader->Read(reinterpret_cast(data)); + FlightDataMessage message; + message.data = data; + return reader->Read(&message.message); } bool ReadPayload(::grpc::ServerReaderWriter* reader, flight::internal::FlightData* data) { - // Pretend to be pb::FlightData and intercept in SerializationTraits - return reader->Read(reinterpret_cast(data)); + FlightDataMessage message; + message.data = data; + return reader->Read(&message.message); } bool ReadPayload(::grpc::ClientReaderWriter* reader,