codegen: Handle zero_terminated attribute in demashaller

Make sure the output array is zero terminated to simplify
code using the output structures.

Changed in generated code:

    diff -ru gen/generated_client_demarshallers.c common/generated_client_demarshallers.c
    --- gen/generated_client_demarshallers.c	2021-02-21 15:13:42.004307087 +0000
    +++ common/generated_client_demarshallers.c	2021-02-21 15:13:58.916513426 +0000
    @@ -565,15 +565,24 @@
         return NULL;
     }

    -static uint8_t * parse_array_uint8(uint8_t *message_start, SPICE_GNUC_UNUSED uint8_t *message_end, uint8_t *struct_data, PointerInfo *this_ptr_info)
    +static uint8_t * parse_array_uint8_terminated(uint8_t *message_start, SPICE_GNUC_UNUSED uint8_t *message_end, uint8_t *struct_data, PointerInfo *this_ptr_info)
     {
         uint8_t *in = message_start + this_ptr_info->offset;
         uint8_t *end;

         end = struct_data;
         memcpy(end, in, this_ptr_info->nelements);
    +#if defined(__GNUC__)
    +#pragma GCC diagnostic push
    +#pragma GCC diagnostic ignored "-Wstringop-overflow"
    +#endif
    +    ((char *) (end))[this_ptr_info->nelements] = 0;
    +#if defined(__GNUC__)
    +#pragma GCC diagnostic pop
    +#endif
         in += this_ptr_info->nelements;
         end += this_ptr_info->nelements;
    +    end += 1;
         return end;
     }

    @@ -622,7 +631,8 @@
                 dst_info_host_data__array__nelements = host_size__value;

                 dst_info_host_data__array__nw_size = dst_info_host_data__array__nelements;
    -            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements;
    +            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements + sizeof(uint8_t);
    +            dst_info_host_data__array__mem_size = SPICE_ALIGN(dst_info_host_data__array__mem_size, 4);
                 if (SPICE_UNLIKELY(dst_info_host_data__array__nw_size > (uintptr_t) (message_end - message_start - host_data__value))) {
                     goto error;
                 }
    @@ -650,7 +660,8 @@
                 dst_info_cert_subject_data__array__nelements = cert_subject_size__value;

                 dst_info_cert_subject_data__array__nw_size = dst_info_cert_subject_data__array__nelements;
    -            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements;
    +            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements + sizeof(uint8_t);
    +            dst_info_cert_subject_data__array__mem_size = SPICE_ALIGN(dst_info_cert_subject_data__array__mem_size, 4);
                 if (SPICE_UNLIKELY(dst_info_cert_subject_data__array__nw_size > (uintptr_t) (message_end - message_start - cert_subject_data__value))) {
                     goto error;
                 }
    @@ -685,14 +696,14 @@
             out->dst_info.sport = consume_uint16(&in);
             out->dst_info.host_size = consume_uint32(&in);
             ptr_info[n_ptr].offset = consume_uint32(&in);
    -        ptr_info[n_ptr].parse = parse_array_uint8;
    +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
             ptr_info[n_ptr].dest = (void **)&out->dst_info.host_data;
             host_data__array__nelements = out->dst_info.host_size;
             ptr_info[n_ptr].nelements = host_data__array__nelements;
             n_ptr++;
             out->dst_info.cert_subject_size = consume_uint32(&in);
             ptr_info[n_ptr].offset = consume_uint32(&in);
    -        ptr_info[n_ptr].parse = parse_array_uint8;
    +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
             ptr_info[n_ptr].dest = (void **)&out->dst_info.cert_subject_data;
             cert_subject_data__array__nelements = out->dst_info.cert_subject_size;
             ptr_info[n_ptr].nelements = cert_subject_data__array__nelements;
    @@ -1050,7 +1061,8 @@
             host_data__array__nelements = host_size__value;

             host_data__array__nw_size = host_data__array__nelements;
    -        host_data__array__mem_size = sizeof(uint8_t) * host_data__array__nelements;
    +        host_data__array__mem_size = sizeof(uint8_t) * host_data__array__nelements + sizeof(uint8_t);
    +        host_data__array__mem_size = SPICE_ALIGN(host_data__array__mem_size, 4);
             if (SPICE_UNLIKELY(host_data__array__nw_size > (uintptr_t) (message_end - message_start - host_data__value))) {
                 goto error;
             }
    @@ -1078,7 +1090,8 @@
             cert_subject_data__array__nelements = cert_subject_size__value;

             cert_subject_data__array__nw_size = cert_subject_data__array__nelements;
    -        cert_subject_data__array__mem_size = sizeof(uint8_t) * cert_subject_data__array__nelements;
    +        cert_subject_data__array__mem_size = sizeof(uint8_t) * cert_subject_data__array__nelements + sizeof(uint8_t);
    +        cert_subject_data__array__mem_size = SPICE_ALIGN(cert_subject_data__array__mem_size, 4);
             if (SPICE_UNLIKELY(cert_subject_data__array__nw_size > (uintptr_t) (message_end - message_start - cert_subject_data__value))) {
                 goto error;
             }
    @@ -1107,13 +1120,13 @@
         out->sport = consume_uint16(&in);
         out->host_size = consume_uint32(&in);
         ptr_info[n_ptr].offset = consume_uint32(&in);
    -    ptr_info[n_ptr].parse = parse_array_uint8;
    +    ptr_info[n_ptr].parse = parse_array_uint8_terminated;
         ptr_info[n_ptr].dest = (void **)&out->host_data;
         ptr_info[n_ptr].nelements = host_data__array__nelements;
         n_ptr++;
         out->cert_subject_size = consume_uint32(&in);
         ptr_info[n_ptr].offset = consume_uint32(&in);
    -    ptr_info[n_ptr].parse = parse_array_uint8;
    +    ptr_info[n_ptr].parse = parse_array_uint8_terminated;
         ptr_info[n_ptr].dest = (void **)&out->cert_subject_data;
         ptr_info[n_ptr].nelements = cert_subject_data__array__nelements;
         n_ptr++;
    @@ -1338,7 +1351,8 @@
                 dst_info_host_data__array__nelements = host_size__value;

                 dst_info_host_data__array__nw_size = dst_info_host_data__array__nelements;
    -            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements;
    +            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements + sizeof(uint8_t);
    +            dst_info_host_data__array__mem_size = SPICE_ALIGN(dst_info_host_data__array__mem_size, 4);
                 if (SPICE_UNLIKELY(dst_info_host_data__array__nw_size > (uintptr_t) (message_end - message_start - host_data__value))) {
                     goto error;
                 }
    @@ -1366,7 +1380,8 @@
                 dst_info_cert_subject_data__array__nelements = cert_subject_size__value;

                 dst_info_cert_subject_data__array__nw_size = dst_info_cert_subject_data__array__nelements;
    -            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements;
    +            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements + sizeof(uint8_t);
    +            dst_info_cert_subject_data__array__mem_size = SPICE_ALIGN(dst_info_cert_subject_data__array__mem_size, 4);
                 if (SPICE_UNLIKELY(dst_info_cert_subject_data__array__nw_size > (uintptr_t) (message_end - message_start - cert_subject_data__value))) {
                     goto error;
                 }
    @@ -1401,14 +1416,14 @@
             out->dst_info.sport = consume_uint16(&in);
             out->dst_info.host_size = consume_uint32(&in);
             ptr_info[n_ptr].offset = consume_uint32(&in);
    -        ptr_info[n_ptr].parse = parse_array_uint8;
    +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
             ptr_info[n_ptr].dest = (void **)&out->dst_info.host_data;
             host_data__array__nelements = out->dst_info.host_size;
             ptr_info[n_ptr].nelements = host_data__array__nelements;
             n_ptr++;
             out->dst_info.cert_subject_size = consume_uint32(&in);
             ptr_info[n_ptr].offset = consume_uint32(&in);
    -        ptr_info[n_ptr].parse = parse_array_uint8;
    +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
             ptr_info[n_ptr].dest = (void **)&out->dst_info.cert_subject_data;
             cert_subject_data__array__nelements = out->dst_info.cert_subject_size;
             ptr_info[n_ptr].nelements = cert_subject_data__array__nelements;
    @@ -7582,7 +7597,8 @@
             name__array__nelements = name_size__value;

             name__array__nw_size = name__array__nelements;
    -        name__array__mem_size = sizeof(uint8_t) * name__array__nelements;
    +        name__array__mem_size = sizeof(uint8_t) * name__array__nelements + sizeof(uint8_t);
    +        name__array__mem_size = SPICE_ALIGN(name__array__mem_size, 4);
             if (SPICE_UNLIKELY(name__array__nw_size > (uintptr_t) (message_end - message_start - name__value))) {
                 goto error;
             }
    @@ -7609,7 +7625,7 @@

         out->name_size = consume_uint32(&in);
         ptr_info[n_ptr].offset = consume_uint32(&in);
    -    ptr_info[n_ptr].parse = parse_array_uint8;
    +    ptr_info[n_ptr].parse = parse_array_uint8_terminated;
         ptr_info[n_ptr].dest = (void **)&out->name;
         ptr_info[n_ptr].nelements = name__array__nelements;
         n_ptr++;

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>

7 files changed, 91 insertions, 4 deletions

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index efc4c99..2a111be 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -384,6 +384,10 @@ def write_validate_array_item(writer, container, item, scope, parent_scope, star
         # TODO: Overflow check the multiplication
         if array.has_attr("ptr_array"):
             writer.assign(mem_size, "sizeof(void *) + SPICE_ALIGN(%s * %s, 4)" % (element_type.sizeof(), nelements))
+        elif array.has_attr('zero_terminated'):
+            # don't use +1 here to avoid possible integer overflow or suboptimizations
+            writer.assign(mem_size, "%s * %s + %s" % (element_type.sizeof(), nelements, element_type.sizeof()))
+            writer.assign(mem_size, "SPICE_ALIGN(%s, 4)" % mem_size);
         else:
             writer.assign(mem_size, "%s * %s" % (element_type.sizeof(), nelements))
         want_mem_size = False
@@ -724,7 +728,10 @@ def write_switch_parser(writer, container, switch, dest, scope):
 
 def write_parse_ptr_function(writer, target_type):
     if target_type.is_array():
-        parse_function = "parse_array_%s" % target_type.element_type.primitive_type()
+        if target_type.has_attr("zero_terminated"):
+            parse_function = "parse_array_%s_terminated" % target_type.element_type.primitive_type()
+        else:
+            parse_function = "parse_array_%s" % target_type.element_type.primitive_type()
     else:
         parse_function = "parse_struct_%s" % target_type.c_type()
     if writer.is_generated("parser", parse_function):
@@ -787,10 +794,28 @@ def write_array_parser(writer, member, nelements, array, dest, scope):
         if array.has_attr("ptr_array"):
             raise Exception("Attribute ptr_array not supported for arrays of int8/uint8")
         writer.statement("memcpy(%s, in, %s)" % (array_start, nelements))
+        if array.has_attr("zero_terminated"):
+            indentation = writer.indentation
+            writer.indentation = 0;
+            writer.writeln("#if defined(__GNUC__)")
+            writer.writeln("#pragma GCC diagnostic push")
+            writer.writeln("#pragma GCC diagnostic ignored \"-Wstringop-overflow\"")
+            writer.writeln("#endif")
+            writer.indentation = indentation;
+            writer.assign("((char *) (%s))[%s]" % (array_start, nelements), 0)
+            writer.indentation = 0;
+            writer.writeln("#if defined(__GNUC__)")
+            writer.writeln("#pragma GCC diagnostic pop")
+            writer.writeln("#endif")
+            writer.indentation = indentation;
         writer.increment("in", nelements)
         if at_end:
             writer.increment("end", nelements)
+            if array.has_attr("zero_terminated"):
+                writer.increment("end", 1)
     else:
+        if array.has_attr("zero_terminated"):
+            raise Exception("Attribute zero_terminated specified for wrong array")
         with writer.index() as index:
             if member:
                 array_pos = "%s[%s]" % (array_start, index)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index e3eb8fd..d68dc67 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -553,6 +553,8 @@ class ArrayType(Type):
             return writer.writeln('%s *%s[0];' % (self.c_type(), name))
         if member.has_end_attr():
             return writer.writeln('%s %s[0];' % (self.c_type(), name))
+        if self.is_constant_length() and self.has_attr("zero_terminated"):
+            return writer.writeln('%s %s[%s];' % (self.c_type(), name, self.size + 1))
         if self.is_constant_length():
             return writer.writeln('%s %s[%s];' % (self.c_type(), name, self.size))
         if self.is_identifier_length():
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 05d9ba2..be40bc4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -136,7 +136,8 @@ generated_test_marshallers.c: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEP
 generated_test_marshallers.h: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
 	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-marshallers --server --include test-marshallers.h -H $< $@ >/dev/null
 generated_test_demarshallers.c: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
-	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-demarshallers --client --include test-marshallers.h $< $@ >/dev/null
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-demarshallers --client --include test-marshallers.h \
+	--generated-declaration-file generated_test_messages.h $< $@ >/dev/null
 generated_test_enums.h: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
 	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py -e $< $@ >/dev/null
 
diff --git a/tests/meson.build b/tests/meson.build
index bd53565..c5df468 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,13 @@ if spice_common_generate_client_code or spice_common_generate_server_code
   targets = [
       ['test_marshallers', test_proto, 'generated_test_marshallers.c', ['--generate-marshallers', '--server', '--include', 'test-marshallers.h', '@INPUT@', '@OUTPUT@']],
       ['test_marshallers_h', test_proto, 'generated_test_marshallers.h', ['--generate-marshallers', '--server', '--include', 'test-marshallers.h', '-H', '@INPUT@', '@OUTPUT@']],
-      ['test_demarshallers', test_proto, 'generated_test_demarshallers.c', ['--generate-demarshallers', '--client', '--include', 'test-marshallers.h', '@INPUT@', '@OUTPUT@']],
+      ['test_demarshallers', test_proto, 'generated_test_demarshallers.c', [
+        '--generate-demarshallers',
+        '--client',
+        '--include', 'test-marshallers.h',
+        '--generated-declaration-file', 'generated_test_messages.h',
+        '@INPUT@', '@OUTPUT@'
+      ]],
       ['test_enums_h', test_proto, 'generated_test_enums.h', ['-e', '@INPUT@', '@OUTPUT@']],
   ]
 
diff --git a/tests/test-marshallers.c b/tests/test-marshallers.c
index afa681d..d8c34d6 100644
--- a/tests/test-marshallers.c
+++ b/tests/test-marshallers.c
@@ -1,5 +1,5 @@
 /*
-   Copyright (C) 2015-2018 Red Hat, Inc.
+   Copyright (C) 2015-2021 Red Hat, Inc.
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -90,6 +90,44 @@ uint8_t * spice_parse_msg(uint8_t *message_start, uint8_t *message_end,
                           uint32_t channel, uint16_t message_type, int minor,
                           size_t *size_out, message_destructor_t *free_message);
 
+static void test_zerolen1(void)
+{
+    static uint8_t data[] = {
+        'd', 'a', 't', 'a', // txt1
+        123, // sep1
+        4, 0, 0, 0, // len
+        26, 0, 0, 0, // ptr to txt2
+        'n','e','k','o', // txt3
+        12, 13, 14, 15, // n
+        3, 0, // txt4_len
+        'b','a','r', // txt4
+        'f', 'o', 'o', '!', // string
+        'x', 'x', // garbage at the end
+    };
+
+    size_t msg_len;
+    message_destructor_t free_message;
+
+    // demarshal array with @zero_terminated data
+    SpiceMsgMainZeroLen1 *msg = (SpiceMsgMainZeroLen1 *)
+        spice_parse_msg(data, data + sizeof(data), SPICE_CHANNEL_MAIN, SPICE_MSG_MAIN_ZEROLEN1,
+                        0, &msg_len, &free_message);
+
+    g_assert_nonnull(msg);
+    g_assert_cmpmem(msg->txt1, 5, "data", 5);
+    g_assert_cmpint(msg->sep1, ==, 123);
+    g_assert_cmpint(msg->txt2_len, ==, 4);
+    g_assert_cmpint(msg->n , ==, 0x0f0e0d0c);
+    g_assert_nonnull(msg->txt2);
+    g_assert_cmpmem(msg->txt2, 5, "foo!", 5);
+    g_assert_nonnull(msg->txt3);
+    g_assert_cmpmem(msg->txt3, 5, "neko", 5);
+    g_assert_cmpint(msg->txt4_len, ==, 3);
+    g_assert_cmpmem(msg->txt4, 4, "bar", 4);
+
+    free_message((uint8_t *) msg);
+}
+
 int main(void)
 {
     SpiceMarshaller *marshaller;
@@ -135,6 +173,8 @@ int main(void)
     }
     spice_marshaller_reset(marshaller);
 
+    test_zerolen1();
+
     SpiceMsgMainZeroes msg_zeroes = { 0x0102 };
 
     spice_marshall_msg_main_Zeroes(marshaller, &msg_zeroes);
diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
index 8ca736e..8470e38 100644
--- a/tests/test-marshallers.h
+++ b/tests/test-marshallers.h
@@ -3,6 +3,8 @@
 
 #include <stdint.h>
 
+#include "generated_test_messages.h"
+
 typedef struct {
     uint32_t data_size;
     uint8_t dummy_byte;
diff --git a/tests/test-marshallers.proto b/tests/test-marshallers.proto
index eabd487..6ca2d4c 100644
--- a/tests/test-marshallers.proto
+++ b/tests/test-marshallers.proto
@@ -24,6 +24,17 @@ channel TestChannel {
         uint32 dummy[2];
         uint8 data[] @end;
     } LenMessage;
+
+    message {
+        uint8 txt1[4] @zero_terminated;
+        uint8 sep1;
+        uint32 txt2_len;
+        uint8 *txt2[txt2_len] @zero_terminated;
+        uint8 txt3[txt2_len] @to_ptr @zero_terminated;
+        uint32 n;
+        uint16 txt4_len;
+        uint8 txt4[txt4_len] @end @zero_terminated;
+    } @declare ZeroLen1;
 };
 
 protocol Spice {