From 3f366d6e8b2b53cf7f350f493451da051a843daa Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Mon, 20 May 2024 14:18:26 +0200 Subject: [PATCH 1/2] feat(heap-trace): Add a pause state to the heap tracing This commit adds a feature to pause the heap tracing in the sense that in this state, the heap tracing will no longer record the new allocations but will continue to monitor the free() in order to keep track of the status of the allocations present in the list of records. See https://github.com/espressif/esp-idf/issues/13803 --- components/heap/heap_trace_standalone.c | 44 +++++++----- components/heap/include/esp_heap_trace.h | 19 +++++- .../heap_tests/main/test_heap_trace.c | 67 ++++++++++++++++++- 3 files changed, 111 insertions(+), 19 deletions(-) diff --git a/components/heap/heap_trace_standalone.c b/components/heap/heap_trace_standalone.c index 3a292a13ed..2c123d50ac 100644 --- a/components/heap/heap_trace_standalone.c +++ b/components/heap/heap_trace_standalone.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -24,8 +24,15 @@ static __attribute__((unused)) const char* TAG = "heaptrace"; #if CONFIG_HEAP_TRACING_STANDALONE +typedef enum { + TRACING_STARTED, // start recording allocs and free + TRACING_STOPPED, // stop recording allocs and free + TRACING_ALLOC_PAUSED, // stop recording allocs but keep recording free + TRACING_UNKNOWN // default value +} tracing_state_t; + static portMUX_TYPE trace_mux = portMUX_INITIALIZER_UNLOCKED; -static bool tracing; +static tracing_state_t tracing = TRACING_UNKNOWN; static heap_trace_mode_t mode; /* Define struct: linked list of records */ @@ -154,7 +161,7 @@ static HEAP_IRAM_ATTR heap_trace_record_t* map_find_and_remove(void *p) esp_err_t heap_trace_init_standalone(heap_trace_record_t *record_buffer, size_t num_records) { - if (tracing) { + if ((tracing == TRACING_STARTED) || (tracing == TRACING_ALLOC_PAUSED)) { return ESP_ERR_INVALID_STATE; } @@ -181,12 +188,12 @@ esp_err_t heap_trace_init_standalone(heap_trace_record_t *record_buffer, size_t return ESP_OK; } -static esp_err_t set_tracing(bool enable) +static esp_err_t set_tracing(tracing_state_t state) { - if (tracing == enable) { + if (tracing == state) { return ESP_ERR_INVALID_STATE; } - tracing = enable; + tracing = state; return ESP_OK; } @@ -198,7 +205,7 @@ esp_err_t heap_trace_start(heap_trace_mode_t mode_param) portENTER_CRITICAL(&trace_mux); - set_tracing(false); + set_tracing(TRACING_STOPPED); mode = mode_param; // clear buffers @@ -220,7 +227,7 @@ esp_err_t heap_trace_start(heap_trace_mode_t mode_param) total_allocations = 0; total_frees = 0; - const esp_err_t ret_val = set_tracing(true); + const esp_err_t ret_val = set_tracing(TRACING_STARTED); portEXIT_CRITICAL(&trace_mux); return ret_val; @@ -229,7 +236,15 @@ esp_err_t heap_trace_start(heap_trace_mode_t mode_param) esp_err_t heap_trace_stop(void) { portENTER_CRITICAL(&trace_mux); - const esp_err_t ret_val = set_tracing(false); + const esp_err_t ret_val = set_tracing(TRACING_STOPPED); + portEXIT_CRITICAL(&trace_mux); + return ret_val; +} + +esp_err_t heap_trace_alloc_pause(void) +{ + portENTER_CRITICAL(&trace_mux); + const esp_err_t ret_val = set_tracing(TRACING_ALLOC_PAUSED); portEXIT_CRITICAL(&trace_mux); return ret_val; } @@ -237,7 +252,7 @@ esp_err_t heap_trace_stop(void) esp_err_t heap_trace_resume(void) { portENTER_CRITICAL(&trace_mux); - const esp_err_t ret_val = set_tracing(true); + const esp_err_t ret_val = set_tracing(TRACING_STARTED); portEXIT_CRITICAL(&trace_mux); return ret_val; } @@ -425,13 +440,12 @@ static void heap_trace_dump_base(bool internal_ram, bool psram) /* Add a new allocation to the heap trace records */ static HEAP_IRAM_ATTR void record_allocation(const heap_trace_record_t *r_allocation) { - if (!tracing || r_allocation->address == NULL) { + if ((tracing != TRACING_STARTED) || (r_allocation->address == NULL)) { return; } - portENTER_CRITICAL(&trace_mux); - if (tracing) { + if (tracing == TRACING_STARTED) { // If buffer is full, pop off the oldest // record to make more space if (records.count == records.capacity) { @@ -465,7 +479,7 @@ static HEAP_IRAM_ATTR void record_allocation(const heap_trace_record_t *r_alloca */ static HEAP_IRAM_ATTR void record_free(void *p, void **callers) { - if (!tracing || p == NULL) { + if ((tracing == TRACING_STOPPED) || (p == NULL)) { return; } @@ -478,7 +492,7 @@ static HEAP_IRAM_ATTR void record_free(void *p, void **callers) return; } - if (tracing) { + if (tracing != TRACING_STOPPED) { total_frees++; diff --git a/components/heap/include/esp_heap_trace.h b/components/heap/include/esp_heap_trace.h index 977d729fea..65116efe64 100644 --- a/components/heap/include/esp_heap_trace.h +++ b/components/heap/include/esp_heap_trace.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -112,10 +112,25 @@ esp_err_t heap_trace_start(heap_trace_mode_t mode); * @return * - ESP_ERR_NOT_SUPPORTED Project was compiled without heap tracing enabled in menuconfig. * - ESP_ERR_INVALID_STATE Heap tracing was not in progress. - * - ESP_OK Heap tracing stopped.. + * - ESP_OK Heap tracing stopped. */ esp_err_t heap_trace_stop(void); +/** + * @brief Pause heap tracing of allocations. + * + * @note This function puts the heap tracing in the state where the new allocations + * will no longer be traced but the free will still be. This can be used to e.g., + * strategically monitor a set of allocations to make sure each of them will get freed + * without polluting the list of records with unwanted allocations. + * + * @return + * - ESP_ERR_NOT_SUPPORTED Project was compiled without heap tracing enabled in menuconfig. + * - ESP_ERR_INVALID_STATE Heap tracing was not in progress. + * - ESP_OK Heap tracing paused. + */ +esp_err_t heap_trace_alloc_pause(void); + /** * @brief Resume heap tracing which was previously stopped. * diff --git a/components/heap/test_apps/heap_tests/main/test_heap_trace.c b/components/heap/test_apps/heap_tests/main/test_heap_trace.c index 5755f499ea..1d0ae4af6f 100644 --- a/components/heap/test_apps/heap_tests/main/test_heap_trace.c +++ b/components/heap/test_apps/heap_tests/main/test_heap_trace.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -88,7 +88,7 @@ TEST_CASE("heap trace wrapped buffer check", "[heap-trace]") ptrs[i] = malloc(i*3); } - // becuase other mallocs happen as part of this control flow, + // because other mallocs happen as part of this control flow, // we can't guarantee N entries of ptrs[] are in the heap check buffer. // but we should guarantee at least the last one is bool saw_last_ptr = false; @@ -169,6 +169,69 @@ TEST_CASE("can trace allocations made by newlib", "[heap-trace]") TEST_ASSERT(heap_trace_get_count() > 3); } +TEST_CASE("can stop recording allocs but continue recording frees", "[heap-trace]") +{ + const size_t N = 2; + heap_trace_record_t recs[N]; + void *ptrs[N]; + const size_t alloc_size = 100; + heap_trace_init_standalone(recs, N); + heap_trace_start(HEAP_TRACE_LEAKS); + + size_t current_count = N; + + /* allocate N blocks of memory to fill N records */ + for (size_t i = 0; i < N; i ++) { + ptrs[i] = heap_caps_malloc(alloc_size, MALLOC_CAP_INTERNAL); + TEST_ASSERT_NOT_NULL(ptrs[i]); + } + + /* get the current allocation count to make sure it is + * accurately tracking N allocations. */ + TEST_ASSERT(heap_trace_get_count() == current_count); + + /* free an allocation and make sure the new count has dropped by one */ + heap_caps_free(ptrs[current_count - 1]); + TEST_ASSERT(heap_trace_get_count() == current_count - 1); + current_count--; + + /* pause the tracing */ + heap_trace_alloc_pause(); + + /* try to allocate a new block of memory and make + * sure the trace count has not increased. */ + ptrs[current_count] = heap_caps_malloc(alloc_size, MALLOC_CAP_INTERNAL); + TEST_ASSERT_NOT_NULL(ptrs[current_count]); + TEST_ASSERT(heap_trace_get_count() == current_count); + + /* free the newly allocated block and make sure the trace count + * has not dropped since the freed block of memory is not the one + * present in the trace records */ + heap_caps_free(ptrs[current_count]); + TEST_ASSERT(heap_trace_get_count() == current_count); + + /* free a block of memory that is in the record and make sure + * that the trace count has dropped by one */ + heap_caps_free(ptrs[current_count - 1]); + TEST_ASSERT(heap_trace_get_count() == current_count - 1); + current_count--; + + /* start the tracing again, allocate a new block of memory and + * make sure the trace count is increased by 1 */ + heap_trace_resume(); + ptrs[current_count + 1] = heap_caps_malloc(alloc_size, MALLOC_CAP_INTERNAL); + TEST_ASSERT_NOT_NULL(ptrs[current_count + 1]); + TEST_ASSERT(heap_trace_get_count() == current_count + 1); + current_count++; + + /* free the allocated blocks of memory */ + for (size_t i = 0; i < current_count; i++) { + heap_caps_free(ptrs[i]); + } + + heap_trace_stop(); +} + TEST_CASE("check for summary value validity", "[heap-trace]") { const size_t alloc_size = 100; const size_t counter_size = 2; From 63b8143e32fb02ed8db07d517f8ea2a75de211e0 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Tue, 21 May 2024 11:37:37 +0200 Subject: [PATCH 2/2] docs(heap_debug): Docuement the heap_trace_alloc_pause function Document the newly introduced heap_trace_alloc_pause function in the heap tracing section. --- docs/en/api-reference/system/heap_debug.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/en/api-reference/system/heap_debug.rst b/docs/en/api-reference/system/heap_debug.rst index f36f1de52b..3d23939584 100644 --- a/docs/en/api-reference/system/heap_debug.rst +++ b/docs/en/api-reference/system/heap_debug.rst @@ -216,7 +216,8 @@ Once you have identified the code which you think is leaking: - Enable the :ref:`CONFIG_HEAP_TRACING_DEST` option. - Call the function :cpp:func:`heap_trace_init_standalone` early in the program, to register a buffer that can be used to record the memory trace. - Call the function :cpp:func:`heap_trace_start` to begin recording all mallocs or frees in the system. Call this immediately before the piece of code which you suspect is leaking memory. -- Call the function :cpp:func:`heap_trace_stop` to stop the trace once the suspect piece of code has finished executing. +- Call the function :cpp:func:`heap_trace_stop` to stop the trace once the suspect piece of code has finished executing. This state will stop the tracing of both allocations and frees. +- Call the function :cpp:func:`heap_trace_alloc_pause` to pause the tracing of new allocations while continuing to trace the frees. Call this immediately after the piece of code which you suspect is leaking memory to prevent any new allocations to be recorded. - Call the function :cpp:func:`heap_trace_dump` to dump the results of the heap trace. The following code snippet demonstrates how application code would typically initialize, start, and stop heap tracing: