Copilot commented on code in PR #55480:
URL: https://github.com/apache/arrow/pull/55480#discussion_r4527528277
##########
cpp/src/arrow/flight/transport/grpc/serialization_internal.cc:
##########
@@ -203,72 +204,103 @@ ::grpc::Status FlightDataDeserialize(ByteBuffer* buffer,
# pragma GCC diagnostic ignored "-Wstrict-aliasing"
#endif
-// Pointer bitcast explanation: grpc::*Writer<T>::Write() and
grpc::*Reader<T>::Read()
-// both take a T* argument (here pb::FlightData*). But they don't do anything
-// with that argument except pass it to SerializationTraits<T>::Serialize() and
-// SerializationTraits<T>::Deserialize().
-//
-// Since we control SerializationTraits<pb::FlightData>, we can interpret the
-// pointer argument whichever way we want, including cast it back to the
original type.
-// (see customize_grpc.h).
+// grpc::*Writer<T>::Write() and grpc::*Reader<T>::Read() take a T (here
+// pb::FlightData) and only pass it to SerializationTraits<T>::Serialize() and
+// SerializationTraits<T>::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<const FlightPayloadMessage*>(
+ static_cast<const char*>(message) -
offsetof(FlightPayloadMessage, message))
Review Comment:
Both wrappers contain a polymorphic `pb::FlightData`, so GCC warns that
these `offsetof` expressions use non-standard-layout types. This breaks default
Debug Flight builds because `SetupCxxFlags.cmake:289–296,349` enables
`-Werror`; the existing pragma suppresses only strict-aliasing warnings. Avoid
non-standard-layout `offsetof` in both helpers (also line 239), or explicitly
suppress its diagnostic if relying on the compiler extension is intentional.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
