From 83157e3362b039bbaba6e75629ed49b055492956 Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Fri, 26 Dec 2014 17:08:17 +0200 Subject: [PATCH] Backport mem_release testcase from 0.3.1. Also backports cdbf51 generator bugfix. --- generator/nanopb_generator.py | 2 + tests/SConstruct | 8 +- tests/alltypes_pointer/SConscript | 28 ++--- tests/common/SConscript | 29 +++++ tests/{fuzztest => common}/malloc_wrappers.c | 0 tests/{fuzztest => common}/malloc_wrappers.h | 0 .../malloc_wrappers_syshdr.h} | 0 tests/extensions/extensions.proto | 1 - tests/fuzztest/SConscript | 41 ++----- tests/fuzztest/fuzzstub.c | 2 +- tests/fuzztest/fuzztest.c | 2 +- tests/mem_release/SConscript | 12 ++ tests/mem_release/mem_release.c | 106 ++++++++++++++++++ tests/mem_release/mem_release.proto | 23 ++++ 14 files changed, 200 insertions(+), 54 deletions(-) rename tests/{fuzztest => common}/malloc_wrappers.c (100%) rename tests/{fuzztest => common}/malloc_wrappers.h (100%) rename tests/{fuzztest/fuzz_syshdr.h => common/malloc_wrappers_syshdr.h} (100%) create mode 100644 tests/mem_release/SConscript create mode 100644 tests/mem_release/mem_release.c create mode 100644 tests/mem_release/mem_release.proto diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index a0532bf..a0a5aeb 100755 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -599,6 +599,8 @@ class Message: else: parts.append(field.get_initializer(null_init)) elif field.allocation == 'POINTER': + if field.rules == 'REPEATED': + parts.append('0') parts.append('NULL') elif field.allocation == 'CALLBACK': if field.pbtype == 'EXTENSION': diff --git a/tests/SConstruct b/tests/SConstruct index 1890670..9092d4f 100644 --- a/tests/SConstruct +++ b/tests/SConstruct @@ -136,12 +136,18 @@ elif 'g++' in env['CXX'] or 'gcc' in env['CXX']: env.Append(CXXFLAGS = '-g -Wall -Werror -Wextra -Wno-missing-field-initializers') elif 'cl' in env['CXX']: env.Append(CXXFLAGS = '/Zi /W2 /WX') - + # Now include the SConscript files from all subdirectories import os.path env['VARIANT_DIR'] = 'build' env['BUILD'] = '#' + env['VARIANT_DIR'] env['COMMON'] = '#' + env['VARIANT_DIR'] + '/common' + +# Include common/SConscript first to make sure its exports are available +# to other SConscripts. +SConscript("common/SConscript", exports = 'env', variant_dir = env['VARIANT_DIR'] + '/common') + for subdir in Glob('*/SConscript') + Glob('regression/*/SConscript'): + if str(subdir).startswith("common"): continue SConscript(subdir, exports = 'env', variant_dir = env['VARIANT_DIR'] + '/' + os.path.dirname(str(subdir))) diff --git a/tests/alltypes_pointer/SConscript b/tests/alltypes_pointer/SConscript index e48d6aa..4b840c2 100644 --- a/tests/alltypes_pointer/SConscript +++ b/tests/alltypes_pointer/SConscript @@ -1,30 +1,20 @@ # Encode the AllTypes message using pointers for all fields, and verify the # output against the normal AllTypes test case. -Import("env") - -# We need our own pb_decode.o for the malloc support -env = env.Clone() -env.Append(CPPDEFINES = {'PB_ENABLE_MALLOC': 1}); - -# Disable libmudflap, because it will confuse valgrind -# and other memory leak detection tools. -if '-fmudflap' in env["CCFLAGS"]: - env["CCFLAGS"].remove("-fmudflap") - env["LINKFLAGS"].remove("-fmudflap") - env["LIBS"].remove("mudflap") - -strict = env.Clone() -strict.Append(CFLAGS = strict['CORECFLAGS']) -strict.Object("pb_decode_with_malloc.o", "$NANOPB/pb_decode.c") -strict.Object("pb_encode_with_malloc.o", "$NANOPB/pb_encode.c") +Import("env", "malloc_env") c = Copy("$TARGET", "$SOURCE") env.Command("alltypes.proto", "#alltypes/alltypes.proto", c) env.NanopbProto(["alltypes", "alltypes.options"]) -enc = env.Program(["encode_alltypes_pointer.c", "alltypes.pb.c", "pb_encode_with_malloc.o"]) -dec = env.Program(["decode_alltypes_pointer.c", "alltypes.pb.c", "pb_decode_with_malloc.o"]) +enc = malloc_env.Program(["encode_alltypes_pointer.c", + "alltypes.pb.c", + "$COMMON/pb_encode_with_malloc.o", + "$COMMON/malloc_wrappers.o"]) +dec = malloc_env.Program(["decode_alltypes_pointer.c", + "alltypes.pb.c", + "$COMMON/pb_decode_with_malloc.o", + "$COMMON/malloc_wrappers.o"]) # Encode and compare results to non-pointer alltypes test case env.RunTest(enc) diff --git a/tests/common/SConscript b/tests/common/SConscript index 144f149..f444418 100644 --- a/tests/common/SConscript +++ b/tests/common/SConscript @@ -8,6 +8,7 @@ env.NanopbProto("unittestproto") # Protocol definitions for basic_buffer/stream tests env.NanopbProto("person") +#-------------------------------------------- # Binaries of the pb_decode.c and pb_encode.c # These are built using more strict warning flags. strict = env.Clone() @@ -15,3 +16,31 @@ strict.Append(CFLAGS = strict['CORECFLAGS']) strict.Object("pb_decode.o", "$NANOPB/pb_decode.c") strict.Object("pb_encode.o", "$NANOPB/pb_encode.c") +#----------------------------------------------- +# Binaries of pb_decode etc. with malloc support +# Uses malloc_wrappers.c to count allocations. +malloc_env = env.Clone() +malloc_env.Append(CPPDEFINES = {'PB_ENABLE_MALLOC': 1, + 'PB_SYSTEM_HEADER': '\\"malloc_wrappers_syshdr.h\\"'}) +malloc_env.Append(CPPPATH = ["$COMMON"]) + +if 'SYSHDR' in malloc_env: + malloc_env.Append(CPPDEFINES = {'PB_OLD_SYSHDR': malloc_env['SYSHDR']}) + +# Disable libmudflap, because it will confuse valgrind +# and other memory leak detection tools. +if '-fmudflap' in env["CCFLAGS"]: + malloc_env["CCFLAGS"].remove("-fmudflap") + malloc_env["LINKFLAGS"].remove("-fmudflap") + malloc_env["LIBS"].remove("mudflap") + +malloc_strict = malloc_env.Clone() +malloc_strict.Append(CFLAGS = malloc_strict['CORECFLAGS']) +malloc_strict.Object("pb_decode_with_malloc.o", "$NANOPB/pb_decode.c") +malloc_strict.Object("pb_encode_with_malloc.o", "$NANOPB/pb_encode.c") + +mw = env.Object("malloc_wrappers.o", "malloc_wrappers.c") +Depends(mw, ["malloc_wrappers_syshdr.h"]) + +Export("malloc_env") + diff --git a/tests/fuzztest/malloc_wrappers.c b/tests/common/malloc_wrappers.c similarity index 100% rename from tests/fuzztest/malloc_wrappers.c rename to tests/common/malloc_wrappers.c diff --git a/tests/fuzztest/malloc_wrappers.h b/tests/common/malloc_wrappers.h similarity index 100% rename from tests/fuzztest/malloc_wrappers.h rename to tests/common/malloc_wrappers.h diff --git a/tests/fuzztest/fuzz_syshdr.h b/tests/common/malloc_wrappers_syshdr.h similarity index 100% rename from tests/fuzztest/fuzz_syshdr.h rename to tests/common/malloc_wrappers_syshdr.h diff --git a/tests/extensions/extensions.proto b/tests/extensions/extensions.proto index da8432e..38f7ad3 100644 --- a/tests/extensions/extensions.proto +++ b/tests/extensions/extensions.proto @@ -7,7 +7,6 @@ extend AllTypes { message ExtensionMessage { extend AllTypes { optional ExtensionMessage AllTypes_extensionfield2 = 254; - required ExtensionMessage AllTypes_extensionfield3 = 253; repeated ExtensionMessage AllTypes_extensionfield4 = 252; } diff --git a/tests/fuzztest/SConscript b/tests/fuzztest/SConscript index 1fd4de9..d7e64a4 100644 --- a/tests/fuzztest/SConscript +++ b/tests/fuzztest/SConscript @@ -1,29 +1,9 @@ # Run a fuzz test to verify robustness against corrupted/malicious data. -Import("env") - -# We need our own pb_decode.o for the malloc support -env = env.Clone() -env.Append(CPPDEFINES = {'PB_ENABLE_MALLOC': 1, - 'PB_SYSTEM_HEADER': '\\"fuzz_syshdr.h\\"'}) -env.Append(CPPPATH = ".") - -if 'SYSHDR' in env: - env.Append(CPPDEFINES = {'PB_OLD_SYSHDR': env['SYSHDR']}) - -# Disable libmudflap, because it will confuse valgrind -# and other memory leak detection tools. -if '-fmudflap' in env["CCFLAGS"]: - env["CCFLAGS"].remove("-fmudflap") - env["LINKFLAGS"].remove("-fmudflap") - env["LIBS"].remove("mudflap") - -strict = env.Clone() -strict.Append(CFLAGS = strict['CORECFLAGS']) -strict.Object("pb_decode_with_malloc.o", "$NANOPB/pb_decode.c") -strict.Object("pb_encode_with_malloc.o", "$NANOPB/pb_encode.c") +Import("env", "malloc_env") # We want both pointer and static versions of the AllTypes message +# Prefix them with package name. env.Command("alltypes_static.proto", "#alltypes/alltypes.proto", lambda target, source, env: open(str(target[0]), 'w').write("package alltypes_static;\n" @@ -35,21 +15,20 @@ env.Command("alltypes_pointer.proto", "#alltypes/alltypes.proto", p1 = env.NanopbProto(["alltypes_pointer", "alltypes_pointer.options"]) p2 = env.NanopbProto(["alltypes_static", "alltypes_static.options"]) -fuzz = env.Program(["fuzztest.c", +fuzz = malloc_env.Program(["fuzztest.c", "alltypes_pointer.pb.c", "alltypes_static.pb.c", - "pb_encode_with_malloc.o", - "pb_decode_with_malloc.o", - "malloc_wrappers.c"]) -Depends([p1, p2, fuzz], ["fuzz_syshdr.h", "malloc_wrappers.h"]) + "$COMMON/pb_encode_with_malloc.o", + "$COMMON/pb_decode_with_malloc.o", + "$COMMON/malloc_wrappers.o"]) env.RunTest(fuzz) -fuzzstub = env.Program(["fuzzstub.c", +fuzzstub = malloc_env.Program(["fuzzstub.c", "alltypes_pointer.pb.c", "alltypes_static.pb.c", - "pb_encode_with_malloc.o", - "pb_decode_with_malloc.o", - "malloc_wrappers.c"]) + "$COMMON/pb_encode_with_malloc.o", + "$COMMON/pb_decode_with_malloc.o", + "$COMMON/malloc_wrappers.o"]) diff --git a/tests/fuzztest/fuzzstub.c b/tests/fuzztest/fuzzstub.c index 5099841..ce14b9b 100644 --- a/tests/fuzztest/fuzzstub.c +++ b/tests/fuzztest/fuzzstub.c @@ -10,7 +10,7 @@ #include #include #include -#include "malloc_wrappers.h" +#include #include "alltypes_static.pb.h" #include "alltypes_pointer.pb.h" diff --git a/tests/fuzztest/fuzztest.c b/tests/fuzztest/fuzztest.c index 996ed45..d370172 100644 --- a/tests/fuzztest/fuzztest.c +++ b/tests/fuzztest/fuzztest.c @@ -9,7 +9,7 @@ #include #include #include -#include "malloc_wrappers.h" +#include #include "alltypes_static.pb.h" #include "alltypes_pointer.pb.h" diff --git a/tests/mem_release/SConscript b/tests/mem_release/SConscript new file mode 100644 index 0000000..6515f6a --- /dev/null +++ b/tests/mem_release/SConscript @@ -0,0 +1,12 @@ +Import("env", "malloc_env") + +env.NanopbProto("mem_release.proto") + +test = malloc_env.Program(["mem_release.c", + "mem_release.pb.c", + "$COMMON/pb_encode_with_malloc.o", + "$COMMON/pb_decode_with_malloc.o", + "$COMMON/malloc_wrappers.o"]) + +env.RunTest(test) + diff --git a/tests/mem_release/mem_release.c b/tests/mem_release/mem_release.c new file mode 100644 index 0000000..9cf72b5 --- /dev/null +++ b/tests/mem_release/mem_release.c @@ -0,0 +1,106 @@ +/* Make sure that all fields are freed in various scenarios. */ + +#include +#include +#include +#include +#include +#include "mem_release.pb.h" + +#define TEST(x) if (!(x)) { \ + fprintf(stderr, "Test " #x " on line %d failed.\n", __LINE__); \ + return false; \ + } + +static char *test_str_arr[] = {"1", "2", ""}; +static SubMessage test_msg_arr[] = {SubMessage_init_zero, SubMessage_init_zero}; + +static bool do_test() +{ + uint8_t buffer[256]; + size_t msgsize; + + /* Construct a message with various fields filled in */ + { + TestMessage msg = TestMessage_init_zero; + pb_extension_t ext2; + pb_ostream_t stream; + msg.static_req_submsg.dynamic_str = "12345"; + msg.static_req_submsg.dynamic_str_arr_count = 3; + msg.static_req_submsg.dynamic_str_arr = test_str_arr; + msg.static_req_submsg.dynamic_submsg_count = 2; + msg.static_req_submsg.dynamic_submsg = test_msg_arr; + msg.static_req_submsg.dynamic_submsg[1].dynamic_str = "abc"; + msg.static_opt_submsg.dynamic_str = "abc"; + msg.has_static_opt_submsg = true; + msg.dynamic_submsg = &msg.static_req_submsg; + + msg.extensions = &ext2; + ext2.type = &static_ext; + ext2.dest = &msg.static_req_submsg; + ext2.next = NULL; + + stream = pb_ostream_from_buffer(buffer, sizeof(buffer)); + if (!pb_encode(&stream, TestMessage_fields, &msg)) + { + fprintf(stderr, "Encode failed: %s\n", PB_GET_ERROR(&stream)); + return false; + } + msgsize = stream.bytes_written; + } + + /* Output encoded message for debug */ + SET_BINARY_MODE(stdout); + fwrite(buffer, 1, msgsize, stdout); + + /* Decode memory using dynamic allocation */ + { + TestMessage msg = TestMessage_init_zero; + pb_istream_t stream; + SubMessage ext2_dest = SubMessage_init_zero; + pb_extension_t ext2; + + msg.extensions = &ext2; + ext2.type = &static_ext; + ext2.dest = &ext2_dest; + ext2.next = NULL; + + stream = pb_istream_from_buffer(buffer, msgsize); + if (!pb_decode(&stream, TestMessage_fields, &msg)) + { + fprintf(stderr, "Decode failed: %s\n", PB_GET_ERROR(&stream)); + return false; + } + + /* Make sure it encodes back to same data */ + { + uint8_t buffer2[256]; + pb_ostream_t ostream = pb_ostream_from_buffer(buffer2, sizeof(buffer2)); + TEST(pb_encode(&ostream, TestMessage_fields, &msg)); + TEST(ostream.bytes_written == msgsize); + TEST(memcmp(buffer, buffer2, msgsize) == 0); + } + + /* Make sure that malloc counters work */ + TEST(get_alloc_count() > 0); + + /* Make sure that pb_release releases everything */ + pb_release(TestMessage_fields, &msg); + TEST(get_alloc_count() == 0); + + /* Check that double-free is a no-op */ + pb_release(TestMessage_fields, &msg); + TEST(get_alloc_count() == 0); + } + + return true; +} + +int main() +{ + if (do_test()) + return 0; + else + return 1; +} + diff --git a/tests/mem_release/mem_release.proto b/tests/mem_release/mem_release.proto new file mode 100644 index 0000000..e9bf8b7 --- /dev/null +++ b/tests/mem_release/mem_release.proto @@ -0,0 +1,23 @@ +syntax = "proto2"; +import "nanopb.proto"; + +message SubMessage +{ + optional string dynamic_str = 1 [(nanopb).type = FT_POINTER]; + repeated string dynamic_str_arr = 2 [(nanopb).type = FT_POINTER]; + repeated SubMessage dynamic_submsg = 3 [(nanopb).type = FT_POINTER]; +} + +message TestMessage +{ + required SubMessage static_req_submsg = 1 [(nanopb).type = FT_STATIC]; + optional SubMessage dynamic_submsg = 2 [(nanopb).type = FT_POINTER]; + optional SubMessage static_opt_submsg = 3 [(nanopb).type = FT_STATIC]; + extensions 100 to 200; +} + +extend TestMessage +{ + optional SubMessage static_ext = 101 [(nanopb).type = FT_STATIC]; +} +