[ovs-dev] [PATCH] byte-order: Fix undefined behavior of BYTES_TO_BE32.

[Ben Pfaff]

A left shift that would produce a result that is not representable
by the type of the expression's result has "undefined behavior"
according to the C language standard. Avoid this by casting values
that could set the upper bit to unsigned types.

Also document and convert a macro to a function.

While we're at it, delete the unused macro BE16S_TO_BE32.

Found via gcc's undefined behavior sanitizer.

Reported-by: Lance Richardson <lrichard at redhat.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 lib/byte-order.h | 21 +++++++++++++--------
 lib/flow.c       |  2 +-
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/lib/byte-order.h b/lib/byte-order.h
index e864658f9b59..b2a9082bbada 100644
--- a/lib/byte-order.h
+++ b/lib/byte-order.h
@@ -98,17 +98,22 @@ uint32_byteswap(uint32_t crc) {
          ((((ovs_be64) (VALUE)) & UINT64_C(0xff00000000000000)) >> 56))
 #endif
 
+/* Returns the ovs_be32 that you would get from:
+ *
+ *    union { uint8_t b[4]; ovs_be32 be32; } x = { .b = { b0, b1, b2, b3 } };
+ *    return x.be32;
+ *
+ * but without the undefined behavior. */
+static inline ovs_be32
+bytes_to_be32(uint8_t b0, uint8_t b1, uint8_t b2, uint8_t b3)
+{
 #if WORDS_BIGENDIAN
-#define BYTES_TO_BE32(B1, B2, B3, B4) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) << 24 | (B2) << 16 | (B3) << 8 | (B4))
-#define BE16S_TO_BE32(B1, B2) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) << 16 | (B2))
+    uint32_t x = ((uint32_t) b0 << 24) | (b1 << 16) | (b2 << 8) | b3;
 #else
-#define BYTES_TO_BE32(B1, B2, B3, B4) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) | (B2) << 8 | (B3) << 16 | (B4) << 24)
-#define BE16S_TO_BE32(B1, B2) \
-    (OVS_FORCE ovs_be32)((uint32_t)(B1) | (B2) << 16)
+    uint32_t x = ((uint32_t) b3 << 24) | (b2 << 16) | (b1 << 8) | b0;
 #endif
+    return (OVS_FORCE ovs_be32) x;
+}
 
 /* These functions zero-extend big-endian values to longer ones,
  * or truncate long big-endian value to shorter ones. */
diff --git a/lib/flow.c b/lib/flow.c
index 1f51b66e7b44..d73e796a2f45 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -823,7 +823,7 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
 
     packet->l4_ofs = (char *)data - frame;
     miniflow_push_be32(mf, nw_frag,
-                       BYTES_TO_BE32(nw_frag, nw_tos, nw_ttl, nw_proto));
+                       bytes_to_be32(nw_frag, nw_tos, nw_ttl, nw_proto));
 
     if (OVS_LIKELY(!(nw_frag & FLOW_NW_FRAG_LATER))) {
         if (OVS_LIKELY(nw_proto == IPPROTO_TCP)) {
-- 
2.10.2