Fix undefined behaviour reported by ubsan

* calling ddsrt_memdup, ddsrt_strdup with a null pointer (they handle it
  gracefully but forbid it in the interface ...)

* replacement of all pre-C99 flexible arrays (i.e., declaring as
  array[1], then mallocing and using as if array[N]) by C99 flexible
  arrays.

* also add a missing null-pointer test in dds_dispose_ts, and fix the
  test cases that pass a null pointer and a non-writer handle to it to
  instead pass an invalid adress

Signed-off-by: Erik Boasson <eb@ilities.com>

src/core/ddsc/src/dds_instance.c

@@ -238,6 +238,9 @@ dds_return_t dds_dispose_ts (dds_entity_t writer, const void *data, dds_time_t t
   dds_return_t ret;
   dds_writer *wr;
 
+  if (data == NULL)
+    return DDS_RETCODE_BAD_PARAMETER;
+
   if ((ret = dds_writer_lock (writer, &wr)) != DDS_RETCODE_OK)
     return ret;
 

src/core/ddsc/src/dds_qos.c

@@ -22,7 +22,7 @@ static void dds_qos_data_copy_in (ddsi_octetseq_t *data, const void * __restrict
   if (overwrite && data->value)
     ddsrt_free (data->value);
   data->length = (uint32_t) sz;
-  data->value = ddsrt_memdup (value, sz);
+  data->value = value ? ddsrt_memdup (value, sz) : NULL;
 }
 
 static bool dds_qos_data_copy_out (const ddsi_octetseq_t *data, void **value, size_t *sz)

src/core/ddsc/src/dds_whc.c

@@ -60,11 +60,7 @@ struct whc_idxnode {
   seqno_t prune_seq;
   struct ddsi_tkmap_instance *tk;
   uint32_t headidx;
-#if __STDC_VERSION__ >= 199901L
   struct whc_node *hist[];
-#else
-  struct whc_node *hist[1];
-#endif
 };
 
 #if USE_EHH

src/core/ddsc/tests/dispose.c

@@ -560,7 +560,8 @@ CU_Theory((dds_entity_t *writer), ddsc_dispose, non_writers, .init=disposing_ini
 {
     dds_return_t ret;
     DDSRT_WARNING_MSVC_OFF(6387); /* Disable SAL warning on intentional misuse of the API */
-    ret = dds_dispose(*writer, NULL);
+    /* pass a non-null pointer that'll trigger a crash if it is read */
+    ret = dds_dispose(*writer, (void *) 1);
     DDSRT_WARNING_MSVC_ON(6387);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_ILLEGAL_OPERATION);
 }
@@ -714,7 +715,8 @@ CU_Theory((dds_entity_t *writer), ddsc_dispose_ts, non_writers, .init=disposing_
 {
     dds_return_t ret;
     DDSRT_WARNING_MSVC_OFF(6387); /* Disable SAL warning on intentional misuse of the API */
-    ret = dds_dispose_ts(*writer, NULL, g_present);
+    /* pass a non-null pointer that'll trigger a crash if it is read */
+    ret = dds_dispose_ts(*writer, (void *) 1, g_present);
     DDSRT_WARNING_MSVC_ON(6387);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_ILLEGAL_OPERATION);
 }

src/core/ddsi/include/dds/ddsi/ddsi_serdata_default.h

@@ -66,7 +66,7 @@ struct ddsi_serdata_default {
      serdata is 8-byte aligned */
   char pad[8 - ((sizeof (struct ddsi_serdata) + 4) % 8)];
   struct CDRHeader hdr;
-  char data[1];
+  char data[];
 };
 
 struct dds_key_descriptor;

src/core/ddsi/include/dds/ddsi/q_protocol.h

@@ -35,8 +35,8 @@ typedef struct {
 typedef struct nn_sequence_number_set {
   nn_sequence_number_t bitmap_base;
   uint32_t numbits;
-  uint32_t bits[1];
+  uint32_t bits[];
-} nn_sequence_number_set_t; /* Why strict C90? zero-length/flexible array members are far nicer */
+} nn_sequence_number_set_t;
 /* SequenceNumberSet size is base (2 words) + numbits (1 word) +
    bitmap ((numbits+31)/32 words), and this at 4 bytes/word */
 #define NN_SEQUENCE_NUMBER_SET_BITS_SIZE(numbits) ((unsigned) (4 * (((numbits) + 31) / 32)))
@@ -45,7 +45,7 @@ typedef unsigned nn_fragment_number_t;
 typedef struct nn_fragment_number_set {
   nn_fragment_number_t bitmap_base;
   uint32_t numbits;
-  uint32_t bits[1];
+  uint32_t bits[];
 } nn_fragment_number_set_t;
 /* FragmentNumberSet size is base (2 words) + numbits (1 word) +
    bitmap ((numbits+31)/32 words), and this at 4 bytes/word */
@@ -70,12 +70,6 @@ typedef struct nn_udpv4mcgen_address {
   uint8_t idx; /* must be last: then sorting will put them consecutively */
 } nn_udpv4mcgen_address_t;
 
-
-struct cdrstring {
-  uint32_t length;
-  unsigned char contents[1]; /* C90 does not support flex. array members */
-};
-
 #define NN_STATUSINFO_DISPOSE      0x1u
 #define NN_STATUSINFO_UNREGISTER   0x2u
 #define NN_STATUSINFO_STANDARDIZED (NN_STATUSINFO_DISPOSE | NN_STATUSINFO_UNREGISTER)
@@ -317,7 +311,7 @@ typedef struct ParticipantMessageData {
   nn_guid_prefix_t participantGuidPrefix;
   uint32_t kind; /* really 4 octets */
   uint32_t length;
-  char value[1 /* length */];
+  char value[];
 } ParticipantMessageData_t;
 #define PARTICIPANT_MESSAGE_DATA_KIND_UNKNOWN 0x0u
 #define PARTICIPANT_MESSAGE_DATA_KIND_AUTOMATIC_LIVELINESS_UPDATE 0x1u

src/core/ddsi/include/dds/ddsi/q_radmin.h

@@ -47,15 +47,15 @@ struct nn_rmsg_chunk {
      nn_rmsg_setsize after receiving a packet from the kernel and
      before processing it.  */
   uint32_t size;
-  union {
-    /* payload array stretched to whatever it really is */
-    unsigned char payload[1];
 
-    /* to ensure reasonable alignment of payload[] */
+  /* to ensure reasonable alignment of payload[] */
+  union {
     int64_t l;
     double d;
     void *p;
   } u;
+  /* payload array stretched to whatever it really is */
+  unsigned char payload[];
 };
 
 struct nn_rmsg {
@@ -95,7 +95,7 @@ struct nn_rmsg {
 
   struct nn_rmsg_chunk chunk;
 };
-#define NN_RMSG_PAYLOAD(m) ((m)->chunk.u.payload)
+#define NN_RMSG_PAYLOAD(m) ((m)->chunk.payload)
 #define NN_RMSG_PAYLOADOFF(m, o) (NN_RMSG_PAYLOAD (m) + (o))
 
 struct receiver_state {

src/core/ddsi/src/q_plist.c

@@ -712,9 +712,9 @@ static dds_return_t unalias_generic (void * __restrict dst, size_t * __restrict
     {
       case XSTOP:
         return 0;
-      case XO: COMPLEX (XO, ddsi_octetseq_t, x->value = ddsrt_memdup (x->value, x->length)); break;
+      case XO: COMPLEX (XO, ddsi_octetseq_t, if (x->value) { x->value = ddsrt_memdup (x->value, x->length); }); break;
-      case XS: COMPLEX (XS, char *, *x = ddsrt_strdup (*x)); break;
+      case XS: COMPLEX (XS, char *, if (*x) { *x = ddsrt_strdup (*x); }); break;
-      case XZ: COMPLEX (XZ, ddsi_stringseq_t, {
+      case XZ: COMPLEX (XZ, ddsi_stringseq_t, if (x->n) {
         x->strs = ddsrt_memdup (x->strs, x->n * sizeof (*x->strs));
         for (uint32_t i = 0; i < x->n; i++)
           x->strs[i] = ddsrt_strdup (x->strs[i]);

src/core/ddsi/src/q_radmin.c

@@ -320,8 +320,8 @@ static uint32_t max_rmsg_size_w_hdr (uint32_t max_rmsg_size)
      allocate for the worst case, and may waste a few bytes here or
      there. */
   return
-    max_uint32 ((uint32_t) offsetof (struct nn_rmsg, chunk.u.payload),
+    max_uint32 ((uint32_t) offsetof (struct nn_rmsg, chunk.payload),
-                (uint32_t) offsetof (struct nn_rmsg_chunk, u.payload))
+                (uint32_t) offsetof (struct nn_rmsg_chunk, payload))
     + max_rmsg_size;
 }
 
@@ -405,15 +405,15 @@ struct nn_rbuf {
      approach.  Changes would be confined rmsg_new and rmsg_free. */
   unsigned char *freeptr;
 
+  /* to ensure reasonable alignment of raw[] */
   union {
-    /* raw data array, nn_rbuf::size bytes long in reality */
-    unsigned char raw[1];
-
-    /* to ensure reasonable alignment of raw[] */
     int64_t l;
     double d;
     void *p;
   } u;
+
+  /* raw data array, nn_rbuf::size bytes long in reality */
+  unsigned char raw[];
 };
 
 static struct nn_rbuf *nn_rbuf_alloc_new (struct nn_rbufpool *rbufpool)
@@ -421,17 +421,17 @@ static struct nn_rbuf *nn_rbuf_alloc_new (struct nn_rbufpool *rbufpool)
   struct nn_rbuf *rb;
   ASSERT_RBUFPOOL_OWNER (rbufpool);
 
-  if ((rb = ddsrt_malloc (offsetof (struct nn_rbuf, u.raw) + rbufpool->rbuf_size)) == NULL)
+  if ((rb = ddsrt_malloc (sizeof (struct nn_rbuf) + rbufpool->rbuf_size)) == NULL)
     return NULL;
 #if USE_VALGRIND
-  VALGRIND_MAKE_MEM_NOACCESS (rb->u.raw, rbufpool->rbuf_size);
+  VALGRIND_MAKE_MEM_NOACCESS (rb->raw, rbufpool->rbuf_size);
 #endif
 
   rb->rbufpool = rbufpool;
   ddsrt_atomic_st32 (&rb->n_live_rmsg_chunks, 1);
   rb->size = rbufpool->rbuf_size;
   rb->max_rmsg_size = rbufpool->max_rmsg_size;
-  rb->freeptr = rb->u.raw;
+  rb->freeptr = rb->raw;
   DDS_LOG(DDS_LC_RADMIN, "rbuf_alloc_new(%p) = %p\n", (void *) rbufpool, (void *) rb);
   return rb;
 }
@@ -488,17 +488,17 @@ static void *nn_rbuf_alloc (struct nn_rbufpool *rbufpool)
   ASSERT_RBUFPOOL_OWNER (rbufpool);
   rb = rbufpool->current;
   assert (rb != NULL);
-  assert (rb->freeptr >= rb->u.raw);
+  assert (rb->freeptr >= rb->raw);
-  assert (rb->freeptr <= rb->u.raw + rb->size);
+  assert (rb->freeptr <= rb->raw + rb->size);
 
-  if ((uint32_t) (rb->u.raw + rb->size - rb->freeptr) < asize)
+  if ((uint32_t) (rb->raw + rb->size - rb->freeptr) < asize)
   {
     /* not enough space left for new rmsg */
     if ((rb = nn_rbuf_new (rbufpool)) == NULL)
       return NULL;
 
     /* a new one should have plenty of space */
-    assert ((uint32_t) (rb->u.raw + rb->size - rb->freeptr) >= asize);
+    assert ((uint32_t) (rb->raw + rb->size - rb->freeptr) >= asize);
   }
 
   DDS_LOG(DDS_LC_RADMIN, "rmsg_rbuf_alloc(%p, %"PRIu32") = %p\n", (void *) rbufpool, asize, (void *) rb->freeptr);
@@ -587,7 +587,7 @@ static void commit_rmsg_chunk (struct nn_rmsg_chunk *chunk)
 {
   struct nn_rbuf *rbuf = chunk->rbuf;
   DDS_LOG(DDS_LC_RADMIN, "commit_rmsg_chunk(%p)\n", (void *) chunk);
-  rbuf->freeptr = chunk->u.payload + chunk->size;
+  rbuf->freeptr = chunk->payload + chunk->size;
 }
 
 void nn_rmsg_commit (struct nn_rmsg *rmsg)
@@ -688,7 +688,7 @@ void *nn_rmsg_alloc (struct nn_rmsg *rmsg, uint32_t size)
     chunk = newchunk;
   }
 
-  ptr = chunk->u.payload + chunk->size;
+  ptr = chunk->payload + chunk->size;
   chunk->size += size8;
   DDS_LOG(DDS_LC_RADMIN, "rmsg_alloc(%p, %"PRIu32") = %p\n", (void *) rmsg, size, ptr);
 #if USE_VALGRIND

src/core/ddsi/src/q_xmsg.c

@@ -52,7 +52,7 @@ struct nn_xmsgpool {
 struct nn_xmsg_data {
   InfoSRC_t src;
   InfoDST_t dst;
-  char payload[1]; /* of size maxsz */
+  char payload[]; /* of size maxsz */
 };
 
 struct nn_xmsg_chain_elem {