diff --git a/cpp/src/arrow/c/helpers.h b/cpp/src/arrow/c/helpers.h index 6e4df17f43eb..66ba485bb109 100644 --- a/cpp/src/arrow/c/helpers.h +++ b/cpp/src/arrow/c/helpers.h @@ -57,8 +57,17 @@ inline void ArrowSchemaMove(struct ArrowSchema* src, struct ArrowSchema* dest) { ArrowSchemaMarkReleased(src); } +/// The release callback can be implemented outside the code built with Clang CFI, for +/// example in another language such as Rust. It then has no CFI type, so this call +/// cannot be checked. +#if defined(__clang__) +#define ARROW_C_RELEASE_NO_CFI __attribute__((no_sanitize("cfi-icall"))) +#else +#define ARROW_C_RELEASE_NO_CFI +#endif + /// Release the C schema, if necessary, by calling its release callback -inline void ArrowSchemaRelease(struct ArrowSchema* schema) { +ARROW_C_RELEASE_NO_CFI inline void ArrowSchemaRelease(struct ArrowSchema* schema) { if (!ArrowSchemaIsReleased(schema)) { schema->release(schema); ARROW_C_ASSERT(ArrowSchemaIsReleased(schema), @@ -102,7 +111,7 @@ inline void ArrowDeviceArrayMove(struct ArrowDeviceArray* src, } /// Release the C array, if necessary, by calling its release callback -inline void ArrowArrayRelease(struct ArrowArray* array) { +ARROW_C_RELEASE_NO_CFI inline void ArrowArrayRelease(struct ArrowArray* array) { if (!ArrowArrayIsReleased(array)) { array->release(array); ARROW_C_ASSERT(ArrowArrayIsReleased(array), diff --git a/cpp/src/arrow/flight/transport/grpc/customize_grpc.h b/cpp/src/arrow/flight/transport/grpc/customize_grpc.h index 7836f7c161d6..99e8e4e9927b 100644 --- a/cpp/src/arrow/flight/transport/grpc/customize_grpc.h +++ b/cpp/src/arrow/flight/transport/grpc/customize_grpc.h @@ -86,6 +86,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 @@ -102,16 +109,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..ec5c9462263c 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 @@ -203,72 +204,103 @@ ::grpc::Status FlightDataDeserialize(ByteBuffer* buffer, # pragma GCC diagnostic ignored "-Wstrict-aliasing" #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,