Make QuicFrame a standard-layout struct. C++ provides certain guarantees about the way union fields interact, which are only valid for standard-layout types. Standard-layout types are not allowed to have data members in both base and derived types; this fixes that by moving QuicInlinedFrame::type into individual frames. Also, before C++17, using offsetof() on non-standard-layout types is undefined behavor. Thanks to Stephan Hartmann <stha09@googlemail.com> for pointing this problem out in <https://quiche-review.googlesource.com/c/quiche/+/10600>. PiperOrigin-RevId: 320195673 Change-Id: I179634af3e3a199b7b7320a031b42898a5644824
diff --git a/quic/core/frames/quic_frame.h b/quic/core/frames/quic_frame.h index a8aabfc..b5785b8 100644 --- a/quic/core/frames/quic_frame.h +++ b/quic/core/frames/quic_frame.h
@@ -6,6 +6,7 @@ #define QUICHE_QUIC_CORE_FRAMES_QUIC_FRAME_H_ #include <ostream> +#include <type_traits> #include <vector> #include "net/third_party/quiche/src/quic/core/frames/quic_ack_frame.h" @@ -111,6 +112,8 @@ }; }; +static_assert(std::is_standard_layout<QuicFrame>::value, + "QuicFrame must have a standard layout"); static_assert(sizeof(QuicFrame) <= 24, "Frames larger than 24 bytes should be referenced by pointer."); static_assert(offsetof(QuicStreamFrame, type) == offsetof(QuicFrame, type),
diff --git a/quic/core/frames/quic_handshake_done_frame.h b/quic/core/frames/quic_handshake_done_frame.h index c16c169..1dd3fa0 100644 --- a/quic/core/frames/quic_handshake_done_frame.h +++ b/quic/core/frames/quic_handshake_done_frame.h
@@ -23,6 +23,8 @@ std::ostream& os, const QuicHandshakeDoneFrame& handshake_done_frame); + QuicFrameType type; + // A unique identifier of this control frame. 0 when this frame is received, // and non-zero when sent. QuicControlFrameId control_frame_id = kInvalidControlFrameId;
diff --git a/quic/core/frames/quic_inlined_frame.h b/quic/core/frames/quic_inlined_frame.h index 08c4869..cfa32dc 100644 --- a/quic/core/frames/quic_inlined_frame.h +++ b/quic/core/frames/quic_inlined_frame.h
@@ -5,6 +5,8 @@ #ifndef QUICHE_QUIC_CORE_FRAMES_QUIC_INLINED_FRAME_H_ #define QUICHE_QUIC_CORE_FRAMES_QUIC_INLINED_FRAME_H_ +#include <type_traits> + #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/platform/api/quic_export.h" @@ -16,13 +18,15 @@ // inline and out-of-line frame types. template <typename DerivedT> struct QUIC_EXPORT_PRIVATE QuicInlinedFrame { - QuicInlinedFrame(QuicFrameType type) : type(type) { + QuicInlinedFrame(QuicFrameType type) { + static_cast<DerivedT*>(this)->type = type; + static_assert(std::is_standard_layout<DerivedT>::value, + "Inlined frame must have a standard layout"); static_assert(offsetof(DerivedT, type) == 0, "type must be the first field."); static_assert(sizeof(DerivedT) <= 24, "Frames larger than 24 bytes should not be inlined."); } - QuicFrameType type; }; } // namespace quic
diff --git a/quic/core/frames/quic_max_streams_frame.h b/quic/core/frames/quic_max_streams_frame.h index e1595c0..902dbc2 100644 --- a/quic/core/frames/quic_max_streams_frame.h +++ b/quic/core/frames/quic_max_streams_frame.h
@@ -28,6 +28,8 @@ std::ostream& os, const QuicMaxStreamsFrame& frame); + QuicFrameType type; + // A unique identifier of this control frame. 0 when this frame is received, // and non-zero when sent. QuicControlFrameId control_frame_id = kInvalidControlFrameId;
diff --git a/quic/core/frames/quic_mtu_discovery_frame.h b/quic/core/frames/quic_mtu_discovery_frame.h index 330df21..52e980d 100644 --- a/quic/core/frames/quic_mtu_discovery_frame.h +++ b/quic/core/frames/quic_mtu_discovery_frame.h
@@ -16,6 +16,8 @@ struct QUIC_EXPORT_PRIVATE QuicMtuDiscoveryFrame : public QuicInlinedFrame<QuicMtuDiscoveryFrame> { QuicMtuDiscoveryFrame() : QuicInlinedFrame(MTU_DISCOVERY_FRAME) {} + + QuicFrameType type; }; } // namespace quic
diff --git a/quic/core/frames/quic_padding_frame.h b/quic/core/frames/quic_padding_frame.h index 0918f0f..09ddb89 100644 --- a/quic/core/frames/quic_padding_frame.h +++ b/quic/core/frames/quic_padding_frame.h
@@ -25,6 +25,8 @@ std::ostream& os, const QuicPaddingFrame& padding_frame); + QuicFrameType type; + // -1: full padding to the end of a max-sized packet // otherwise: only pad up to num_padding_bytes bytes int num_padding_bytes = -1;
diff --git a/quic/core/frames/quic_ping_frame.h b/quic/core/frames/quic_ping_frame.h index 5cefdf9..04f5afb 100644 --- a/quic/core/frames/quic_ping_frame.h +++ b/quic/core/frames/quic_ping_frame.h
@@ -23,6 +23,8 @@ std::ostream& os, const QuicPingFrame& ping_frame); + QuicFrameType type; + // A unique identifier of this control frame. 0 when this frame is received, // and non-zero when sent. QuicControlFrameId control_frame_id = kInvalidControlFrameId;
diff --git a/quic/core/frames/quic_stop_waiting_frame.h b/quic/core/frames/quic_stop_waiting_frame.h index 84bfc2a..b05d2e1 100644 --- a/quic/core/frames/quic_stop_waiting_frame.h +++ b/quic/core/frames/quic_stop_waiting_frame.h
@@ -21,6 +21,8 @@ std::ostream& os, const QuicStopWaitingFrame& s); + QuicFrameType type; + // The lowest packet we've sent which is unacked, and we expect an ack for. QuicPacketNumber least_unacked; };
diff --git a/quic/core/frames/quic_stream_frame.h b/quic/core/frames/quic_stream_frame.h index f807ee1..10ad7db 100644 --- a/quic/core/frames/quic_stream_frame.h +++ b/quic/core/frames/quic_stream_frame.h
@@ -35,6 +35,7 @@ bool operator!=(const QuicStreamFrame& rhs) const; + QuicFrameType type; bool fin = false; QuicPacketLength data_length = 0; // TODO(wub): Change to a QuicUtils::GetInvalidStreamId when it is not version
diff --git a/quic/core/frames/quic_streams_blocked_frame.h b/quic/core/frames/quic_streams_blocked_frame.h index 91c7ac9..2df9636 100644 --- a/quic/core/frames/quic_streams_blocked_frame.h +++ b/quic/core/frames/quic_streams_blocked_frame.h
@@ -28,6 +28,8 @@ std::ostream& os, const QuicStreamsBlockedFrame& frame); + QuicFrameType type; + // A unique identifier of this control frame. 0 when this frame is received, // and non-zero when sent. QuicControlFrameId control_frame_id = kInvalidControlFrameId;