From 6b662331f7bef5d9058029885184ea1f15e9ef0d Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 18 Sep 2020 08:49:02 +0100 Subject: 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 Acked-by: Victor Toso --- python_modules/demarshal.py | 27 ++++++++++++++++++++++++++- python_modules/ptypes.py | 2 ++ tests/Makefile.am | 3 ++- tests/meson.build | 8 +++++++- tests/test-marshallers.c | 42 +++++++++++++++++++++++++++++++++++++++++- tests/test-marshallers.h | 2 ++ tests/test-marshallers.proto | 11 +++++++++++ 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 +#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 { -- cgit v1.2.3