Merge branch 'feat/heap-trace-stop-recording-allocs-but-not-free' into 'master'

feat(heap-trace): Add a pause state to the heap tracing

Closes IDFGH-12836

See merge request espressif/esp-idf!30962

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++;
 

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.
  *

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;

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: