From 570ae6bb525abafbd4b49089b4eba71c63085d44 Mon Sep 17 00:00:00 2001 From: Paul Bartell Date: Thu, 18 Feb 2021 10:15:01 -0800 Subject: [PATCH] Add unity memory extension, fake_assert, and enable -fsanitize=address (#506) * Enable libunitymemory extension to track dynamic memory usage during unit tests * Use UnityMemory in timers_utest.c * Add fake_assert.h to allow mocking of configASSERT calls * Add .editorconfig to make github show indentation correctly * Add unity memory and fake_assert to queue_utest.c * Add -fsanitize=address CFLAG when running unit tests * Define mtCOVERAGE_TEST_MARKER macro to include mtCOVERAGE_TEST_MARKER lines in coverage figures * Add additional memory check / protection CFLAGS for CMock tests * Fix out of bounds array access in list_utest.c * Move the fake_assert.h include to the top of FreeRTOSConfig.h --- .editorconfig | 8 ++++ .github/scripts/core_checker.py | 5 ++- FreeRTOS/Test/CMock/Makefile | 48 +++++++++++++-------- FreeRTOS/Test/CMock/config/FreeRTOSConfig.h | 12 ++++-- FreeRTOS/Test/CMock/config/fake_assert.h | 37 ++++++++++++++++ FreeRTOS/Test/CMock/list/list_utest.c | 10 ++--- FreeRTOS/Test/CMock/makefile.in | 32 ++++++++++---- FreeRTOS/Test/CMock/queue/Makefile | 1 + FreeRTOS/Test/CMock/queue/queue_utest.c | 17 +++++--- FreeRTOS/Test/CMock/timers/Makefile | 1 + FreeRTOS/Test/CMock/timers/timers_utest.c | 39 +++++++++++------ 11 files changed, 151 insertions(+), 59 deletions(-) create mode 100644 .editorconfig create mode 100644 FreeRTOS/Test/CMock/config/fake_assert.h diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000000..612203583d --- /dev/null +++ b/.editorconfig @@ -0,0 +1,8 @@ +root = true + +[*] +indent_style = space +indent_size = 4 + +[{Makefile,**.mk,Makefile.in}] +indent_style = tab diff --git a/.github/scripts/core_checker.py b/.github/scripts/core_checker.py index 7ac053eebf..ec835048c8 100755 --- a/.github/scripts/core_checker.py +++ b/.github/scripts/core_checker.py @@ -255,14 +255,15 @@ FREERTOS_IGNORED_PATTERNS = [ r'.*/Makefile', r'.*/trcConfig\.h.*', r'.*/trcConfig\.c.*', - r'.*/trcSnapshotConfig\.h.*', + r'.*/trcSnapshotConfig\.h.*' ] FREERTOS_IGNORED_FILES = [ 'fyi-another-way-to-ignore-file.txt', 'mbedtls_config.h', 'requirements.txt', - 'run-cbmc-proofs.py' + 'run-cbmc-proofs.py', + '.editorconfig' ] FREERTOS_HEADER = [ diff --git a/FreeRTOS/Test/CMock/Makefile b/FreeRTOS/Test/CMock/Makefile index aa4950a80a..d8863dc855 100644 --- a/FreeRTOS/Test/CMock/Makefile +++ b/FreeRTOS/Test/CMock/Makefile @@ -12,8 +12,10 @@ include makefile.in all: doc coverage execs: $(UNITS) | directories - -$(UNITS) : ${LIB_DIR}/libcmock.so ${LIB_DIR}/libunity.so | directories +$(UNITS) : ${LIB_DIR}/libcmock.so \ + ${LIB_DIR}/libunity.so \ + ${LIB_DIR}/libunitymemory.so \ + | directories $(MAKE) -C $@ doc: | directories @@ -41,23 +43,31 @@ help: @echo -e 'Usage: $$ make \n ' @echo -e ' where is one of: $(UNITS) doc all run run_formatted run_col run_col_formatted coverage' -$(LIB_DIR)/libcmock.so : ${CMOCK_SRC_DIR}/cmock.c \ - ${CMOCK_SRC_DIR}/cmock.h \ - ${LIB_DIR}/libunity.so \ +$(LIB_DIR)/libcmock.so : ${CMOCK_SRC_DIR}/cmock.c \ + ${CMOCK_SRC_DIR}/cmock.h \ + ${LIB_DIR}/libunity.so \ Makefile | directories ${CC} -o $@ -shared -fPIC $< ${INCLUDE_DIR} -$(LIB_DIR)/libunity.so : ${UNITY_SRC_DIR}/unity.c \ - ${UNITY_SRC_DIR}/unity.h \ - Makefile | directories +$(LIB_DIR)/libunity.so : ${UNITY_SRC_DIR}/unity.c \ + ${UNITY_SRC_DIR}/unity.h \ + Makefile | directories ${CC} -o $@ -shared -fPIC $< +$(LIB_DIR)/libunitymemory.so: ${UNITY_EXTRAS_DIR}/memory/src/unity_memory.c \ + ${UNITY_EXTRAS_DIR}/memory/src/unity_memory.h \ + ${LIB_DIR}/libunity.so \ + Makefile | directories + ${CC} -o $@ -shared -fPIC $< ${INCLUDE_DIR} + run : $(UNITS) directories -rm $(BUILD_DIR)/unit_test_report.txt - for f in $(BIN_DIR)/*; do \ - $${f} | tee -a $(BUILD_DIR)/unit_test_report.txt ; done - cd $(BUILD_DIR) && \ - ruby $(UNITY_BIN_DIR)/parse_output.rb -xml $(BUILD_DIR)/unit_test_report.txt + for f in $(BIN_DIR)/*; do \ + $${f} | tee -a $(BUILD_DIR)/unit_test_report.txt; \ + done + cd $(BUILD_DIR) && \ + ruby $(UNITY_BIN_DIR)/parse_output.rb -xml \ + $(BUILD_DIR)/unit_test_report.txt run_col : $(UNITS) zero_coverage | directories for f in $(BIN_DIR)/*; do \ @@ -65,17 +75,17 @@ run_col : $(UNITS) zero_coverage | directories run_formatted : $(UNITS) zero_coverage | directories for f in $(BIN_DIR)/*; do \ - $${f} > $(BUILD_DIR)/output; \ - ruby $(UNITY_BIN_DIR)/parse_output.rb $(BUILD_DIR)/output ; \ - done + $${f} > $(BUILD_DIR)/output; \ + ruby $(UNITY_BIN_DIR)/parse_output.rb $(BUILD_DIR)/output ; \ + done run_col_formatted : $(UNITS) zero_coverage | directories for f in $(BIN_DIR)/*; do \ - $${f} > $(BUILD_DIR)/output; \ - ruby -r $(UNITY_BIN_DIR)/colour_reporter.rb \ + $${f} > $(BUILD_DIR)/output; \ + ruby -r $(UNITY_BIN_DIR)/colour_reporter.rb \ -e "report('$$(ruby $(UNITY_BIN_DIR)/parse_output.rb \ - $(BUILD_DIR)/output)')"; \ - done + $(BUILD_DIR)/output)')"; \ + done zero_coverage : lcov --zerocounters --directory $(BUILD_DIR) diff --git a/FreeRTOS/Test/CMock/config/FreeRTOSConfig.h b/FreeRTOS/Test/CMock/config/FreeRTOSConfig.h index a45f97c327..527aa21ee6 100644 --- a/FreeRTOS/Test/CMock/config/FreeRTOSConfig.h +++ b/FreeRTOS/Test/CMock/config/FreeRTOSConfig.h @@ -19,16 +19,17 @@ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. * - * http://www.FreeRTOS.org - * http://aws.amazon.com/freertos + * https://www.FreeRTOS.org + * https://github.com/FreeRTOS * - * 1 tab == 4 spaces! */ #ifndef FREERTOS_CONFIG_H #define FREERTOS_CONFIG_H +#include "fake_assert.h" + /*----------------------------------------------------------- * Application specific definitions. * @@ -114,7 +115,10 @@ functions anyway. */ /* It is a good idea to define configASSERT() while developing. configASSERT() uses the same semantics as the standard C assert() macro. */ -#define configASSERT( x ) +#define configASSERT( x ) \ + vFakeAssert( (bool) ( x ), __FILE__, __LINE__) + +#define mtCOVERAGE_TEST_MARKER() __asm volatile( "NOP" ) #define configINCLUDE_MESSAGE_BUFFER_AMP_DEMO 0 #if ( configINCLUDE_MESSAGE_BUFFER_AMP_DEMO == 1 ) diff --git a/FreeRTOS/Test/CMock/config/fake_assert.h b/FreeRTOS/Test/CMock/config/fake_assert.h new file mode 100644 index 0000000000..69b4035807 --- /dev/null +++ b/FreeRTOS/Test/CMock/config/fake_assert.h @@ -0,0 +1,37 @@ +/* + * FreeRTOS V202012.00 + * Copyright (C) 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + * + * https://www.FreeRTOS.org + * https://github.com/FreeRTOS + * + */ + +#ifndef FAKE_ASSERT_H +#define FAKE_ASSERT_H + +/* Fake assert.h */ +#include + +void vFakeAssert( bool x, + char * file, + int line ); + +#endif /* FAKE_ASSERT_H */ diff --git a/FreeRTOS/Test/CMock/list/list_utest.c b/FreeRTOS/Test/CMock/list/list_utest.c index 31501b8eb7..28e170b90d 100644 --- a/FreeRTOS/Test/CMock/list/list_utest.c +++ b/FreeRTOS/Test/CMock/list/list_utest.c @@ -740,7 +740,7 @@ void test_macro_listGET_ITEM_VALUE_OF_HEAD_ENTRY( void ) initialise_list_items( pxNewListItem, 2 ); pxNewListItem[ 0 ].xItemValue = 0; - pxNewListItem[ 2 ].xItemValue = 2; + pxNewListItem[ 1 ].xItemValue = 1; vListInsert( &pxList, &pxNewListItem[ 0 ] ); vListInsert( &pxList, &pxNewListItem[ 1 ] ); @@ -762,7 +762,7 @@ void test_macro_listGET_HEAD_ENTRY( void ) initialise_list_items( pxNewListItem, 2 ); pxNewListItem[ 0 ].xItemValue = 0; - pxNewListItem[ 2 ].xItemValue = 2; + pxNewListItem[ 1 ].xItemValue = 1; vListInsert( &pxList, &pxNewListItem[ 0 ] ); vListInsert( &pxList, &pxNewListItem[ 1 ] ); @@ -784,7 +784,7 @@ void test_macro_listGET_NEXT( void ) initialise_list_items( pxNewListItem, 2 ); pxNewListItem[ 0 ].xItemValue = 0; - pxNewListItem[ 2 ].xItemValue = 2; + pxNewListItem[ 1 ].xItemValue = 1; vListInsert( &pxList, &pxNewListItem[ 0 ] ); vListInsert( &pxList, &pxNewListItem[ 1 ] ); @@ -808,7 +808,7 @@ void test_macro_listGET_END_MARKER( void ) initialise_list_items( pxNewListItem, 2 ); pxNewListItem[ 0 ].xItemValue = 0; - pxNewListItem[ 2 ].xItemValue = 2; + pxNewListItem[ 1 ].xItemValue = 1; vListInsert( &pxList, &pxNewListItem[ 0 ] ); vListInsert( &pxList, &pxNewListItem[ 1 ] ); @@ -858,7 +858,7 @@ void test_macro_listGET_OWNER_OF_HEAD_ENTRY( void ) initialise_list_items( pxNewListItem, 2 ); pxNewListItem[ 0 ].xItemValue = 0; - pxNewListItem[ 2 ].xItemValue = 2; + pxNewListItem[ 1 ].xItemValue = 1; listSET_LIST_ITEM_OWNER( &pxNewListItem[ 0 ], owner1 ); listSET_LIST_ITEM_OWNER( &pxNewListItem[ 1 ], owner2 ); diff --git a/FreeRTOS/Test/CMock/makefile.in b/FreeRTOS/Test/CMock/makefile.in index ce96d7c2fe..41e5ab147b 100644 --- a/FreeRTOS/Test/CMock/makefile.in +++ b/FreeRTOS/Test/CMock/makefile.in @@ -17,12 +17,14 @@ FREERTOS_DIR := $(abspath $(FREERTOS_DIR_REL)) KERNEL_DIR_REL := ../../../FreeRTOS/Source KERNEL_DIR := $(abspath $(KERNEL_DIR_REL)) -CMOCK_DIR := $(UT_ROOT_DIR)/CMock -CMOCK_SRC_DIR := $(CMOCK_DIR)/src -UNITY_DIR := $(CMOCK_DIR)/vendor/unity -UNITY_SRC_DIR := $(UNITY_DIR)/src -UNITY_INC_DIR := $(UNITY_DIR)/src -UNITY_BIN_DIR := $(UNITY_DIR)/auto +CMOCK_DIR := $(UT_ROOT_DIR)/CMock +CMOCK_SRC_DIR := $(CMOCK_DIR)/src +UNITY_DIR := $(CMOCK_DIR)/vendor/unity +UNITY_SRC_DIR := $(UNITY_DIR)/src +UNITY_INC_DIR := $(UNITY_DIR)/src +UNITY_BIN_DIR := $(UNITY_DIR)/auto +UNITY_EXTRAS_DIR := $(UNITY_DIR)/extras +UNITY_MEM_SRC_DIR := $(UNITY_EXTRAS_DIR)/memory/src CMOCK_EXEC_DIR := $(CMOCK_DIR)/lib @@ -30,13 +32,25 @@ CMOCK_EXEC_DIR := $(CMOCK_DIR)/lib INCLUDE_DIR := -I$(KERNEL_DIR)/include -I. -I$(UT_ROOT_DIR)/config INCLUDE_DIR += -I$(UNITY_INC_DIR) INCLUDE_DIR += -I$(CMOCK_SRC_DIR) +INCLUDE_DIR += -I$(UNITY_MEM_SRC_DIR) CPPFLAGS := CFLAGS := $(INCLUDE_DIR) -O0 -ggdb -pthread --std=c99 -Werror -Wall -LDFLAGS := -L$(LIB_DIR) -lunity -lcmock -Wl,-rpath,$(LIB_DIR) -pthread -lgcov +CFLAGS += -fsanitize=address,undefined -fsanitize-recover=address +CFLAGS += -fstack-protector-all +CFLAGS += -Wformat -Werror=format-security -Werror=array-bounds +CFLAGS += -D_FORTIFY_SOURCE=2 + +LDFLAGS := -L$(LIB_DIR) -Wl,-rpath,$(LIB_DIR) +LDFLAGS += -fsanitize=address,undefined +LDFLAGS += -pthread +LDFLAGS += -lunity +LDFLAGS += -lunitymemory +LDFLAGS += -lcmock +LDFLAGS += -lgcov export BUILD_DIR -export DOC_DIR +export DOC_DIR export GENERATED_DIR export COVERAGE_DIR export BIN_DIR @@ -48,4 +62,4 @@ export CMOCK_EXEC_DIR export KERNEL_DIR export UNITY_BIN_DIR export LIB_DIR - +export UT_ROOT_DIR diff --git a/FreeRTOS/Test/CMock/queue/Makefile b/FreeRTOS/Test/CMock/queue/Makefile index 20717008d5..d5044cc03d 100644 --- a/FreeRTOS/Test/CMock/queue/Makefile +++ b/FreeRTOS/Test/CMock/queue/Makefile @@ -8,6 +8,7 @@ PROJECT := queue # List the dependency files you wish to mock MOCK_FILES_FP := $(KERNEL_DIR)/include/task.h MOCK_FILES_FP += $(KERNEL_DIR)/include/list.h +MOCK_FILES_FP += $(UT_ROOT_DIR)/config/fake_assert.h # List the options the compilation would need CPPFLAGS += -DportUSING_MPU_WRAPPERS=0 diff --git a/FreeRTOS/Test/CMock/queue/queue_utest.c b/FreeRTOS/Test/CMock/queue/queue_utest.c index 111b7fe924..e6f9439a09 100644 --- a/FreeRTOS/Test/CMock/queue/queue_utest.c +++ b/FreeRTOS/Test/CMock/queue/queue_utest.c @@ -36,23 +36,24 @@ /* Test includes. */ #include "unity.h" +#include "unity_memory.h" /* Mock includes. */ #include "mock_task.h" #include "mock_list.h" +#include "mock_fake_assert.h" /* ============================ GLOBAL VARIABLES =========================== */ -static uint16_t usMallocFreeCalls = 0; /* ========================== CALLBACK FUNCTIONS =========================== */ void * pvPortMalloc( size_t xSize ) { - return malloc( xSize ); + return unity_malloc( xSize ); } void vPortFree( void * pv ) { - return free( pv ); + return unity_free( pv ); } /******************************************************************************* @@ -60,15 +61,16 @@ void vPortFree( void * pv ) ******************************************************************************/ void setUp( void ) { + vFakeAssert_Ignore(); + + /* Track calls to malloc / free */ + UnityMalloc_StartTest(); } /*! called before each testcase */ void tearDown( void ) { - TEST_ASSERT_EQUAL_INT_MESSAGE( 0, usMallocFreeCalls, - "free is not called the same number of times as malloc," - "you might have a memory leak!!" ); - usMallocFreeCalls = 0; + UnityMalloc_EndTest(); } /*! called at the beginning of the whole suite */ @@ -92,4 +94,5 @@ void test_xQueueCreate_Success( void ) QueueHandle_t xQueue = xQueueCreate( 1, 1 ); TEST_ASSERT_NOT_EQUAL( NULL, xQueue ); + vQueueDelete(xQueue); } diff --git a/FreeRTOS/Test/CMock/timers/Makefile b/FreeRTOS/Test/CMock/timers/Makefile index 2b686877ec..44b823d426 100644 --- a/FreeRTOS/Test/CMock/timers/Makefile +++ b/FreeRTOS/Test/CMock/timers/Makefile @@ -9,6 +9,7 @@ PROJECT := timers MOCK_FILES_FP := $(KERNEL_DIR)/include/task.h MOCK_FILES_FP += $(KERNEL_DIR)/include/queue.h MOCK_FILES_FP += $(KERNEL_DIR)/include/list.h +MOCK_FILES_FP += $(UT_ROOT_DIR)/config/fake_assert.h # List the options the compilation would need CPPFLAGS += -DconfigSUPPORT_DYNAMIC_ALLOCATION=1 diff --git a/FreeRTOS/Test/CMock/timers/timers_utest.c b/FreeRTOS/Test/CMock/timers/timers_utest.c index 2e0def39fb..5b7747223c 100644 --- a/FreeRTOS/Test/CMock/timers/timers_utest.c +++ b/FreeRTOS/Test/CMock/timers/timers_utest.c @@ -34,10 +34,12 @@ #include "FreeRTOSConfig.h" #include "timers.h" #include "unity.h" +#include "unity_memory.h" /* Mock includes. */ #include "mock_queue.h" #include "mock_list.h" +#include "mock_fake_assert.h" /* ============================ GLOBAL VARIABLES =========================== */ @@ -47,11 +49,11 @@ static uint16_t usMallocFreeCalls = 0; void * pvPortMalloc( size_t xSize ) { - return malloc( xSize ); + return unity_malloc( xSize ); } void vPortFree( void * pv ) { - return free( pv ); + return unity_free( pv ); } /******************************************************************************* @@ -59,6 +61,10 @@ void vPortFree( void * pv ) ******************************************************************************/ void setUp( void ) { + vFakeAssert_Ignore(); + + /* Track calls to malloc / free */ + UnityMalloc_StartTest(); } /*! called before each testcase */ @@ -68,6 +74,8 @@ void tearDown( void ) "free is not called the same number of times as malloc," "you might have a memory leak!!" ); usMallocFreeCalls = 0; + + UnityMalloc_EndTest(); } /*! called at the beginning of the whole suite */ @@ -81,31 +89,34 @@ int suiteTearDown( int numFailures ) return numFailures; } - -static void _xCallback_Test( TimerHandle_t xTimer ) -{} +static void xCallback_Test( TimerHandle_t xTimer ) +{ +} /** * @brief xTimerCreate happy path - * + * */ void test_xTimerCreate_Success( void ) { - uint32_t ulID = 0; + uint32_t ulID = 0; TimerHandle_t xTimer = NULL; vListInitialise_Ignore(); - xQueueGenericCreateStatic_IgnoreAndReturn( (QueueHandle_t)1 ); + xQueueGenericCreateStatic_IgnoreAndReturn( ( QueueHandle_t ) 1 ); vQueueAddToRegistry_Ignore(); vListInitialiseItem_Ignore(); xTimer = xTimerCreate( "ut-timer", - pdMS_TO_TICKS(1000), - pdTRUE, - &ulID, - _xCallback_Test ); + pdMS_TO_TICKS( 1000 ), + pdTRUE, + &ulID, + xCallback_Test ); TEST_ASSERT_NOT_EQUAL( NULL, xTimer ); + + /* HACK: Free the timer directly */ + vPortFree( ( void * ) xTimer ); } void vApplicationGetTimerTaskMemory( StaticTask_t ** ppxTimerTaskTCBBuffer, @@ -114,10 +125,12 @@ void vApplicationGetTimerTaskMemory( StaticTask_t ** ppxTimerTaskTCBBuffer, { static StaticTask_t xTimerTaskTCB; static StackType_t uxTimerTaskStack[ configTIMER_TASK_STACK_DEPTH ]; + *ppxTimerTaskTCBBuffer = &xTimerTaskTCB; *ppxTimerTaskStackBuffer = uxTimerTaskStack; *pulTimerTaskStackSize = configTIMER_TASK_STACK_DEPTH; } void vApplicationDaemonTaskStartupHook( void ) -{} +{ +}