From 57cf611a4e4345f214b72693fb6c7b58f1c6c9d8 Mon Sep 17 00:00:00 2001 From: "Memfault Inc." Date: Thu, 5 Sep 2024 09:20:01 -0400 Subject: [PATCH] Memfault Firmware SDK 1.11.3 (Build local) --- CHANGELOG.md | 20 ++++ VERSION | 6 +- components/core/src/memfault_heap_stats.c | 9 +- .../src/memfault_ram_reboot_info_tracking.c | 22 +++- .../include/memfault/core/reboot_tracking.h | 3 - .../include/memfault/panics/fault_handling.h | 14 ++- components/include/memfault/version.h | 4 +- .../panics/src/memfault_fault_handling_arm.c | 8 +- .../src/memfault_fault_handling_riscv.c | 3 +- .../src/memfault_fault_handling_xtensa.c | 3 +- .../panics/src/memfault_stdlib_assert.c | 4 + .../config/memfault_platform_config.h | 2 +- ports/zephyr/Kconfig | 22 +++- ports/zephyr/common/CMakeLists.txt | 2 +- ports/zephyr/common/memfault-build-id.ld | 2 +- ports/zephyr/common/memfault_platform_http.c | 2 +- tests/MakefileWorkerOverrides.mk | 2 + .../makefiles/Makefile_memfault_heap_stats.mk | 3 +- tests/src/test_memfault_heap_stats.cpp | 111 ++++++++++++++++++ 19 files changed, 211 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cf085bfd..12d612f74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.11.3] - 2024-09-05 + +### :chart_with_upwards_trend: Improvements + +- General: + + - Improve assert stack unwinding for Clang builds when `-flto` is enabled. + +- Zephyr: + + - Add a new Kconfig option, `CONFIG_MEMFAULT_SOC_FAMILY_ESP32`, to resolve an + error when referencing the deprecated `CONFIG_SOC_FAMILY_ESP32` Kconfig + (removed in + [Zephyr 3.7.0](https://github.com/zephyrproject-rtos/zephyr/commit/8dc3f856229ce083c956aa301c31a23e65bd8cd8) + as part of Hardware Model V2). This new option is used in the Memfault + Zephyr port to check if an ESP32 SOC is being used. + + - Remove references to the deprecated `BOOTLOADER_ESP_IDF` Kconfig symbol in + the Memfault Zephyr port (removed in Zephyr 3.7.0). + ## [1.11.2] - 2024-08-29 ### :chart_with_upwards_trend: Improvements diff --git a/VERSION b/VERSION index 0b7ce30ce..fb251157d 100644 --- a/VERSION +++ b/VERSION @@ -1,3 +1,3 @@ -BUILD ID: 9770 -GIT COMMIT: ea6fadf472 -VERSION: 1.11.2 +BUILD ID: local +GIT COMMIT: c29972dd3ef +VERSION: 1.11.3 diff --git a/components/core/src/memfault_heap_stats.c b/components/core/src/memfault_heap_stats.c index feae89f9d..1daa549ae 100644 --- a/components/core/src/memfault_heap_stats.c +++ b/components/core/src/memfault_heap_stats.c @@ -20,6 +20,7 @@ #include "memfault/core/math.h" #include "memfault/core/platform/debug_log.h" #include "memfault/core/platform/overrides.h" +#include "memfault/core/sdk_assert.h" #define MEMFAULT_HEAP_STATS_VERSION 2 @@ -119,6 +120,12 @@ void memfault_heap_stats_malloc(const void *lr, const void *ptr, size_t size) { g_memfault_heap_stats.max_in_use_block_count = g_memfault_heap_stats.in_use_block_count; } uint16_t new_entry_index = prv_get_new_entry_index(); + + // Ensure a valid entry index is returned. An invalid index can indicate a concurrency + // misconfiguration. MEMFAULT_HEAP_STATS_MALLOC/FREE must prevent concurrent access via + // memfault_lock/unlock or other means + MEMFAULT_SDK_ASSERT(new_entry_index != MEMFAULT_HEAP_STATS_LIST_END); + sMfltHeapStatEntry *new_entry = &g_memfault_heap_stats_pool[new_entry_index]; *new_entry = (sMfltHeapStatEntry){ .lr = lr, @@ -165,7 +172,7 @@ void memfault_heap_stats_free(const void *ptr) { g_memfault_heap_stats.stats_pool_head = entry->info.next_entry_index; } - // If there's a valid previous entry, set it's next index to removed entry's + // If there's a valid previous entry, set its next index to removed entry's if (previous_entry_index != MEMFAULT_HEAP_STATS_LIST_END) { g_memfault_heap_stats_pool[previous_entry_index].info.next_entry_index = entry->info.next_entry_index; diff --git a/components/core/src/memfault_ram_reboot_info_tracking.c b/components/core/src/memfault_ram_reboot_info_tracking.c index 741071d5c..d70715ec4 100644 --- a/components/core/src/memfault_ram_reboot_info_tracking.c +++ b/components/core/src/memfault_ram_reboot_info_tracking.c @@ -26,6 +26,16 @@ #define MEMFAULT_REBOOT_INFO_VERSION 2 +// clang-format off +// Internal value used to initialize sMfltRebootInfo.last_reboot_reason. Care must be taken when +// comparing this value to eMemfaultRebootReason as different platforms produce different behavior +// comparing uint32_t to enum types. In general, cast this value to the type it is compared against. +// Examples: +// +// uint32_t last_reboot_reason != MEMFAULT_REBOOT_REASON_NOT_SET (GOOD) +// eMemfaultRebootReason reboot_reason != (eMemfaultRebootReason)MEMFAULT_REBOOT_REASON_NOT_SET (GOOD) +// uint32_t last_reboot_reason != (eMemfaultRebootReason)MEMFAULT_REBOOT_REASON_NOT_SET (BAD) +// clang-format on #define MEMFAULT_REBOOT_REASON_NOT_SET 0xffffffff typedef MEMFAULT_PACKED_STRUCT MfltRebootInfo { @@ -56,6 +66,10 @@ sMfltRebootInfo; MEMFAULT_STATIC_ASSERT(sizeof(sMfltRebootInfo) == MEMFAULT_REBOOT_TRACKING_REGION_SIZE, "struct doesn't match expected size"); +MEMFAULT_STATIC_ASSERT(sizeof(eMemfaultRebootReason) <= + sizeof(((sMfltRebootInfo){ 0 }).last_reboot_reason), + "enum does not fit within sMfltRebootInfo.last_reboot_reason"); + static sMfltRebootInfo *s_mflt_reboot_info; //! Struct to retrieve reboot reason data from. Matches the fields of sMfltRebootReason @@ -90,6 +104,7 @@ static bool prv_check_or_init_struct(void) { } static bool prv_read_reset_info(sMfltResetReasonInfo *info) { + // prior_stored_reason is a uint32_t, no need to cast MEMFAULT_REBOOT_REASON_NOT_SET if ((s_mflt_reboot_info->last_reboot_reason == MEMFAULT_REBOOT_REASON_NOT_SET) && (s_mflt_reboot_info->reset_reason_reg0 == 0)) { return false; // no reset crashes! @@ -118,7 +133,8 @@ static void prv_record_reboot_reason(eMemfaultRebootReason reboot_reg_reason, uint32_t prior_stored_reason) { s_reboot_reason_data.reboot_reg_reason = reboot_reg_reason; - if (prior_stored_reason != (eMemfaultRebootReason)MEMFAULT_REBOOT_REASON_NOT_SET) { + // prior_stored_reason is a uint32_t, no need to cast MEMFAULT_REBOOT_REASON_NOT_SET + if (prior_stored_reason != MEMFAULT_REBOOT_REASON_NOT_SET) { s_reboot_reason_data.prior_stored_reason = (eMemfaultRebootReason)prior_stored_reason; } else { s_reboot_reason_data.prior_stored_reason = reboot_reg_reason; @@ -131,6 +147,9 @@ static bool prv_get_unexpected_reboot_occurred(void) { // Use prior_stored_reason as source of reboot reason. Fallback to reboot_reg_reason if // prior_stored_reason is not set eMemfaultRebootReason reboot_reason; + + // s_reboot_reason_data.prior_stored_reason is an eMemfaultRebootReason type, cast + // MEMFAULT_REBOOT_REASON_NOT_SET if (s_reboot_reason_data.prior_stored_reason != (eMemfaultRebootReason)MEMFAULT_REBOOT_REASON_NOT_SET) { reboot_reason = s_reboot_reason_data.prior_stored_reason; @@ -151,6 +170,7 @@ static void prv_record_reboot_event(eMemfaultRebootReason reboot_reason, // s_mflt_reboot_info can be cleared by any call to memfault_reboot_tracking_collect_reset_info prv_record_reboot_reason(reboot_reason, s_mflt_reboot_info->last_reboot_reason); + // last_reboot_reason is a uint32_t, no need to cast MEMFAULT_REBOOT_REASON_NOT_SET if (s_mflt_reboot_info->last_reboot_reason != MEMFAULT_REBOOT_REASON_NOT_SET) { // we are already tracking a reboot. We don't overwrite this because generally the first reboot // in a loop reveals what started the crash loop diff --git a/components/include/memfault/core/reboot_tracking.h b/components/include/memfault/core/reboot_tracking.h index 8dc3de510..639e7c857 100644 --- a/components/include/memfault/core/reboot_tracking.h +++ b/components/include/memfault/core/reboot_tracking.h @@ -63,9 +63,6 @@ typedef struct MfltRebootType { eMemfaultRebootReason prior_stored_reason; } sMfltRebootReason; -//! Value used to determine state of reboot tracking data -#define MEMFAULT_REBOOT_REASON_NOT_SET 0xffffffff - #define MEMFAULT_REBOOT_TRACKING_REGION_SIZE 64 //! Sets the memory region used for reboot tracking. diff --git a/components/include/memfault/panics/fault_handling.h b/components/include/memfault/panics/fault_handling.h index a79a15aae..79c8f383d 100644 --- a/components/include/memfault/panics/fault_handling.h +++ b/components/include/memfault/panics/fault_handling.h @@ -82,9 +82,10 @@ MEMFAULT_NAKED_FUNC void MEMFAULT_EXC_HANDLER_WATCHDOG(void); //! @param lr The return address //! @see MEMFAULT_ASSERT_RECORD //! @see MEMFAULT_ASSERT -#if defined(__CC_ARM) || defined(__ICCARM__) || (defined(__clang__) && defined(__ti__)) -//! ARMCC, IAR, and tiarmclang will optimize away link register stores from callsites which makes it -//! impossible for a reliable backtrace to be resolved so we don't use the NORETURN attribute +#if defined(__CC_ARM) || defined(__ICCARM__) || defined(__clang__) +//! ARMCC, IAR, and clang (include tiarmclang) will optimize away link register stores from +//! callsites which makes it impossible for a reliable backtrace to be resolved so we don't use the +//! NORETURN attribute #else MEMFAULT_NORETURN #endif @@ -97,9 +98,10 @@ void memfault_fault_handling_assert(void *pc, void *lr); //! @param extra_info Additional information used by the assert handler //! @see MEMFAULT_ASSERT_RECORD //! @see MEMFAULT_ASSERT -#if defined(__CC_ARM) || defined(__ICCARM__) || (defined(__clang__) && defined(__ti__)) -//! ARMCC, IAR, and tiarmclang optimize away link register stores from callsites which makes it -//! impossible for a reliable backtrace to be resolved so we don't use the NORETURN attribute +#if defined(__CC_ARM) || defined(__ICCARM__) || defined(__clang__) +//! ARMCC, IAR, and clang (include tiarmclang) will optimize away link register stores from +//! callsites which makes it impossible for a reliable backtrace to be resolved so we don't use the +//! NORETURN attribute #else MEMFAULT_NORETURN #endif diff --git a/components/include/memfault/version.h b/components/include/memfault/version.h index 7b22099db..1ae17a8e3 100644 --- a/components/include/memfault/version.h +++ b/components/include/memfault/version.h @@ -19,8 +19,8 @@ typedef struct { uint8_t patch; } sMfltSdkVersion; -#define MEMFAULT_SDK_VERSION { .major = 1, .minor = 11, .patch = 2 } -#define MEMFAULT_SDK_VERSION_STR "1.11.2" +#define MEMFAULT_SDK_VERSION { .major = 1, .minor = 11, .patch = 3 } +#define MEMFAULT_SDK_VERSION_STR "1.11.3" #ifdef __cplusplus } diff --git a/components/panics/src/memfault_fault_handling_arm.c b/components/panics/src/memfault_fault_handling_arm.c index c15be4867..b071f714b 100644 --- a/components/panics/src/memfault_fault_handling_arm.c +++ b/components/panics/src/memfault_fault_handling_arm.c @@ -553,7 +553,13 @@ void MEMFAULT_ASSERT_TRAP(void) { #define MEMFAULT_ASSERT_TRAP() __asm volatile("udf #77") #endif -static void prv_fault_handling_assert(void *pc, void *lr, eMemfaultRebootReason reason) { + #if defined(__clang__) +// When using clang with LTO, the compiler can optimize storing the LR + FP from this function, +// disable optimizations to fix +MEMFAULT_NO_OPT + #endif + static void + prv_fault_handling_assert(void *pc, void *lr, eMemfaultRebootReason reason) { // Only set the crash reason if it's unset, in case we are in a nested assert if (s_crash_reason == kMfltRebootReason_Unknown) { sMfltRebootTrackingRegInfo info = { diff --git a/components/panics/src/memfault_fault_handling_riscv.c b/components/panics/src/memfault_fault_handling_riscv.c index dbe23da70..95c8c8931 100644 --- a/components/panics/src/memfault_fault_handling_riscv.c +++ b/components/panics/src/memfault_fault_handling_riscv.c @@ -45,8 +45,7 @@ void memfault_arch_fault_handling_assert(void *pc, void *lr, eMemfaultRebootReas // For non-esp-idf riscv implementations, provide a full assert handler and // other utilities. - #if defined(__ZEPHYR__) && \ - (defined(CONFIG_SOC_FAMILY_ESP32) || defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32)) + #if defined(__ZEPHYR__) && (defined(CONFIG_MEMFAULT_SOC_FAMILY_ESP32)) #include #include diff --git a/components/panics/src/memfault_fault_handling_xtensa.c b/components/panics/src/memfault_fault_handling_xtensa.c index d1e49357f..9a950d4ae 100644 --- a/components/panics/src/memfault_fault_handling_xtensa.c +++ b/components/panics/src/memfault_fault_handling_xtensa.c @@ -42,8 +42,7 @@ void memfault_arch_fault_handling_assert(void *pc, void *lr, eMemfaultRebootReas // For Zephyr Xtensa, provide an assert handler and other utilities. // The Kconfig for the ESP32 family changed in v3.7.0. Support both Kconfigs - #if defined(__ZEPHYR__) && \ - (defined(CONFIG_SOC_FAMILY_ESP32) || defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32)) + #if defined(__ZEPHYR__) && (defined(CONFIG_MEMFAULT_SOC_FAMILY_ESP32)) #include #include diff --git a/components/panics/src/memfault_stdlib_assert.c b/components/panics/src/memfault_stdlib_assert.c index 742abd420..df368a2c4 100644 --- a/components/panics/src/memfault_stdlib_assert.c +++ b/components/panics/src/memfault_stdlib_assert.c @@ -24,6 +24,10 @@ void __assert_func(const char *file, int line, const char *func, const char *fai (void)file, (void)line, (void)func, (void)failedexpr; #endif MEMFAULT_ASSERT(0); + + #if defined(__clang__) + MEMFAULT_UNREACHABLE; + #endif } #endif // MEMFAULT_ASSERT_CSTDLIB_HOOK_ENABLED && !defined(__TI_ARM__) diff --git a/examples/zephyr/qemu/qemu-app/config/memfault_platform_config.h b/examples/zephyr/qemu/qemu-app/config/memfault_platform_config.h index fc9019acf..f091ce8c4 100644 --- a/examples/zephyr/qemu/qemu-app/config/memfault_platform_config.h +++ b/examples/zephyr/qemu/qemu-app/config/memfault_platform_config.h @@ -13,7 +13,7 @@ extern "C" { #endif -#if defined(CONFIG_SOC_FAMILY_ESP32) || defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32) +#if defined(CONFIG_MEMFAULT_SOC_FAMILY_ESP32) #define ZEPHYR_DATA_REGION_START _data_start #define ZEPHYR_DATA_REGION_END _data_end #endif diff --git a/ports/zephyr/Kconfig b/ports/zephyr/Kconfig index 3a28749e5..529e95808 100644 --- a/ports/zephyr/Kconfig +++ b/ports/zephyr/Kconfig @@ -14,6 +14,7 @@ if MEMFAULT config MEMFAULT_CACHE_FAULT_REGS bool "Memfault Cache ARM fault registers" default y + depends on CPU_CORTEX_M help Save a copy of the ARMv7's fault registers before Zephyr modifies them to provide a more accurate crash analysis @@ -40,6 +41,17 @@ config MEMFAULT_USER_CONFIG_SILENT_FAIL memfault_metrics_heartbeat_config.def memfault_trace_reason_user_config.def +config MEMFAULT_SOC_FAMILY_ESP32 + bool "Enable ESP32 support" + # Note: the generic ESP32 family config changed from SOC_FAMILY_ESP32 to + # SOC_FAMILY_ESPRESSIF_ESP32 in Zephyr 3.7.0. To provide backwards + # compatible support without referencing unavailable Kconfig values, + # generate a Memfault-specific option that works on Zephyr 3.7.0 and + # also earlier releases. + default y if SOC_FAMILY_ESPRESSIF_ESP32 || SOC_SERIES_ESP32 || SOC_SERIES_ESP32S2 || SOC_SERIES_ESP32S3 + help + Enable ESP32 support in the Memfault SDK + # sub menu for coredump settings menu "Memfault Coredump Settings" @@ -148,7 +160,7 @@ endmenu # Memfault Coredump Settings config MEMFAULT_HEAP_STATS bool "Collect system heap stats with coredumps" # our wrapper conflicts with the esp32 heap_caps implementation - default y if (!SOC_FAMILY_ESP32 && !SOC_FAMILY_ESPRESSIF_ESP32) + default y if (!MEMFAULT_SOC_FAMILY_ESP32) select SYS_HEAP_RUNTIME_STATS depends on HEAP_MEM_POOL_SIZE > 0 help @@ -417,7 +429,7 @@ config MEMFAULT_REBOOT_REASON_GET_CUSTOM config MEMFAULT_REBOOT_TRACKING_REGION string "Memfault Reboot Tracking Region" - default ".rtc_noinit" if (SOC_FAMILY_ESP32 || SOC_FAMILY_ESPRESSIF_ESP32) + default ".rtc_noinit" if (MEMFAULT_SOC_FAMILY_ESP32) default ".noinit.mflt_reboot_info" help The region in memory where the reboot tracking information will be stored. @@ -657,8 +669,8 @@ config MEMFAULT_CDR_ENABLE choice MEMFAULT_BUILD_ID_TYPE prompt "Build ID type" - default MEMFAULT_USE_GNU_BUILD_ID if !BOOTLOADER_ESP_IDF - default MEMFAULT_USE_MEMFAULT_BUILD_ID if BOOTLOADER_ESP_IDF + default MEMFAULT_USE_MEMFAULT_BUILD_ID if (MEMFAULT_SOC_FAMILY_ESP32 && !BOOTLOADER_MCUBOOT) + default MEMFAULT_USE_GNU_BUILD_ID help Choose the type of build ID to include in the image @@ -672,7 +684,7 @@ config MEMFAULT_USE_MEMFAULT_BUILD_ID menuconfig MEMFAULT_USE_GNU_BUILD_ID bool "Use a GNU build ID in an image" - depends on !BOOTLOADER_ESP_IDF + depends on !(MEMFAULT_SOC_FAMILY_ESP32 && !BOOTLOADER_MCUBOOT) if MEMFAULT_USE_GNU_BUILD_ID diff --git a/ports/zephyr/common/CMakeLists.txt b/ports/zephyr/common/CMakeLists.txt index f91461f74..d50a3e2a3 100644 --- a/ports/zephyr/common/CMakeLists.txt +++ b/ports/zephyr/common/CMakeLists.txt @@ -72,7 +72,7 @@ if(CONFIG_MEMFAULT_HEAP_STATS AND CONFIG_HEAP_MEM_POOL_SIZE GREATER 0) zephyr_ld_options(-Wl,--wrap=k_free) endif() -if(CONFIG_SOC_FAMILY_ESP32 OR CONFIG_SOC_FAMILY_ESPRESSIF_ESP32) +if(CONFIG_MEMFAULT_SOC_FAMILY_ESP32) zephyr_linker_sources(SECTIONS memfault-rtc-noinit-region.ld) endif() diff --git a/ports/zephyr/common/memfault-build-id.ld b/ports/zephyr/common/memfault-build-id.ld index 55f87fb9c..ba97ce5b1 100644 --- a/ports/zephyr/common/memfault-build-id.ld +++ b/ports/zephyr/common/memfault-build-id.ld @@ -8,7 +8,7 @@ SECTION_PROLOGUE(.note.gnu.build-id,,) * GROUP_DATA_LINK_IN() is newer to Zephyr so it may not catch all cases; * #ifdef on the ESP32 family for now. */ -#if defined(CONFIG_SOC_FAMILY_ESP32) || defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32) +#if defined(CONFIG_MEMFAULT_SOC_FAMILY_ESP32) } GROUP_DATA_LINK_IN(RODATA_REGION, ROMABLE_REGION) #else } GROUP_LINK_IN(ROMABLE_REGION) diff --git a/ports/zephyr/common/memfault_platform_http.c b/ports/zephyr/common/memfault_platform_http.c index 12a056a81..2d652210a 100644 --- a/ports/zephyr/common/memfault_platform_http.c +++ b/ports/zephyr/common/memfault_platform_http.c @@ -27,7 +27,7 @@ #include "memfault/ports/zephyr/root_cert_storage.h" #include "memfault/ports/zephyr/deprecated_root_cert.h" -#if MEMFAULT_ZEPHYR_VERSION_GT(3, 6) +#if MEMFAULT_ZEPHYR_VERSION_GT_STRICT(3, 6) //! Zephyr 3.7.0 deprecated NET_SOCKETS_POSIX_NAMES, and requires POSIX_API to be enabled instead. //! Confirm it's set. diff --git a/tests/MakefileWorkerOverrides.mk b/tests/MakefileWorkerOverrides.mk index 5cf23646b..482476533 100644 --- a/tests/MakefileWorkerOverrides.mk +++ b/tests/MakefileWorkerOverrides.mk @@ -96,6 +96,7 @@ COMPILER_SPECIFIC_WARNINGS += \ -Wno-zero-length-array \ -Wno-unsafe-buffer-usage \ -Wno-cast-function-type-strict \ + -Wc23-extensions \ else # GCC-only warnings @@ -103,6 +104,7 @@ COMPILER_SPECIFIC_WARNINGS += \ -Wformat-signedness \ -fno-extended-identifiers \ -Wbidi-chars \ + -Wc11-c2x-compat \ # Only enable -fanalyzer if GCC version is >= 12 GCC_VERSION_GTEQ_12 := $(shell expr $$($(CC) -dumpversion | cut -f1 -d.) \>= 12) diff --git a/tests/makefiles/Makefile_memfault_heap_stats.mk b/tests/makefiles/Makefile_memfault_heap_stats.mk index 6a0d51d8a..ab48fe83b 100644 --- a/tests/makefiles/Makefile_memfault_heap_stats.mk +++ b/tests/makefiles/Makefile_memfault_heap_stats.mk @@ -2,7 +2,8 @@ SRC_FILES = \ $(MFLT_COMPONENTS_DIR)/core/src/memfault_heap_stats.c MOCK_AND_FAKE_SRC_FILES += \ - $(MFLT_TEST_FAKE_DIR)/fake_memfault_platform_locking.c + $(MFLT_TEST_FAKE_DIR)/fake_memfault_platform_locking.c \ + $(MFLT_TEST_FAKE_DIR)/fake_memfault_sdk_assert.c TEST_SRC_FILES = \ $(MFLT_TEST_SRC_DIR)/test_memfault_heap_stats.cpp \ diff --git a/tests/src/test_memfault_heap_stats.cpp b/tests/src/test_memfault_heap_stats.cpp index 7257cfb53..bc31d64c6 100644 --- a/tests/src/test_memfault_heap_stats.cpp +++ b/tests/src/test_memfault_heap_stats.cpp @@ -8,6 +8,8 @@ #include #include +#include + #include "CppUTest/MemoryLeakDetectorMallocMacros.h" #include "CppUTest/MemoryLeakDetectorNewMacros.h" #include "CppUTest/TestHarness.h" @@ -451,3 +453,112 @@ TEST(MemfaultHeapStats, Test_NeverUsedVsUnused) { }; CHECK(prv_heap_stat_equality(&expected, &g_memfault_heap_stats_pool[0])); } + +static void prv_check_list_end(void) { + bool list_end_found = false; + if (g_memfault_heap_stats.stats_pool_head == MEMFAULT_HEAP_STATS_LIST_END) { + printf("Found end at head\n"); + list_end_found = true; + } else { + uint16_t current_index = g_memfault_heap_stats.stats_pool_head; + for (size_t i = 0; i < MEMFAULT_HEAP_STATS_MAX_COUNT; i++) { + uint16_t next_index = g_memfault_heap_stats_pool[current_index].info.next_entry_index; + if (next_index == MEMFAULT_HEAP_STATS_LIST_END) { + printf("Found end at %u\n", current_index); + list_end_found = true; + break; + } + current_index = next_index; + } + } + CHECK(list_end_found); +} + +static void prv_print_list(void) { + uint16_t current_index = g_memfault_heap_stats.stats_pool_head; + printf("List: "); + for (size_t i = 0; i < MEMFAULT_HEAP_STATS_MAX_COUNT; i++) { + printf("%hu", current_index); + if (current_index == MEMFAULT_HEAP_STATS_LIST_END) { + break; + } + printf("[%s] ", g_memfault_heap_stats_pool[current_index].info.in_use ? "used" : "free"); + current_index = g_memfault_heap_stats_pool[current_index].info.next_entry_index; + } + printf("\n"); +} + +static void prv_print_heap_stats(void) { + printf("Heap Stats: "); + for (size_t i = 0; i < MEMFAULT_HEAP_STATS_MAX_COUNT; i++) { + sMfltHeapStatEntry *entry = &g_memfault_heap_stats_pool[i]; + printf("Entry[%lu]{lr[%p], ptr[%p], size[%d], used:[%d], next:[%hu]} ", i, entry->lr, + entry->ptr, entry->info.size, entry->info.in_use, entry->info.next_entry_index); + } + printf("\n"); +} + +static void prv_check_for_end(void) { + bool result = false; + uint16_t current_index = g_memfault_heap_stats.stats_pool_head; + for (size_t i = 0; i < MEMFAULT_HEAP_STATS_MAX_COUNT; i++) { + if (current_index == MEMFAULT_HEAP_STATS_LIST_END) { + result = true; + break; + } + current_index = g_memfault_heap_stats_pool[current_index].info.next_entry_index; + } + if (current_index == MEMFAULT_HEAP_STATS_LIST_END) { + result = true; + } + CHECK(result); +} + +static void prv_run_list_checks(void) { + prv_print_list(); + prv_print_heap_stats(); + prv_check_list_end(); + prv_check_for_end(); +} + +TEST(MemfaultHeapStats, Test_NoCycle) { + // Setup the random generator + std::random_device rd; + std::mt19937 gen(rd()); + + // Create a distribution of addresses between 1 (0 would be NULL which is invalid) and the max + // number of entries + 1. This gives a range of MEMFAULT_HEAP_STATS_MAX_COUNT + std::uniform_int_distribution heap_stats_entry_address_generator( + 1, MEMFAULT_HEAP_STATS_MAX_COUNT + 1); + std::uniform_int_distribution<> malloc_operation_generator(0, 1); + + // Test pathological case where entries are freed in reverse order (newest first) + for (uintptr_t i = 0; i < MEMFAULT_HEAP_STATS_MAX_COUNT; i++) { + MEMFAULT_HEAP_STATS_MALLOC((void *)(i + 1), i + 2); + } + prv_run_list_checks(); + + // Free every entry at the head of the list until empty + for (uintptr_t i = 0; i < MEMFAULT_HEAP_STATS_MAX_COUNT; i++) { + MEMFAULT_HEAP_STATS_FREE((void *)(MEMFAULT_HEAP_STATS_MAX_COUNT - i)); + } + prv_run_list_checks(); + CHECK(g_memfault_heap_stats.stats_pool_head == MEMFAULT_HEAP_STATS_LIST_END); + + // Fill entries again, and then randomly malloc and free many times + for (uintptr_t i = 0; i < MEMFAULT_HEAP_STATS_MAX_COUNT * 10; i++) { + MEMFAULT_HEAP_STATS_MALLOC((void *)(i + 1 + MEMFAULT_HEAP_STATS_MAX_COUNT), i + 2); + } + + prv_run_list_checks(); + + for (size_t i = 0; i < 10 * MEMFAULT_HEAP_STATS_MAX_COUNT; i++) { + unsigned long entry_addr = heap_stats_entry_address_generator(gen); + if (malloc_operation_generator(gen)) { + MEMFAULT_HEAP_STATS_MALLOC((void *)entry_addr, entry_addr + 2); + } else { + MEMFAULT_HEAP_STATS_FREE((void *)entry_addr); + } + prv_run_list_checks(); + } +}