Merge branch 'bugfix/heap_tests_esp32s2beta' into 'master'

bugfix/heap test failure fixing for esp32s2 beta

Closes IDF-1021

See merge request espressif/esp-idf!6659

components/expat/expat

@@ -1 +1 @@
-Subproject commit a7bc26b69768f7fb24f0c7976fae24b157b85b13
+Subproject commit 968b8cc46dbee47b83318d5f31a8e7907199614b

components/heap/heap_caps.c

@@ -107,6 +107,7 @@ IRAM_ATTR void *heap_caps_malloc( size_t size, uint32_t caps )
                         //we need to 'invert' it (lowest address in DRAM == highest address in IRAM and vice-versa) and
                         //add a pointer to the DRAM equivalent before the address we're going to return.
                         ret = multi_heap_malloc(heap->heap, size + 4);  // int overflow checked above
+
                         if (ret != NULL) {
                             return dram_alloc_to_iram_addr(ret, size + 4);  // int overflow checked above
                         }
@@ -274,6 +275,10 @@ IRAM_ATTR void heap_caps_free( void *ptr)
 
 IRAM_ATTR void *heap_caps_realloc( void *ptr, size_t size, int caps)
 {
+    bool ptr_in_diram_case = false;
+    heap_t *heap = NULL;
+    void *dram_ptr = NULL;
+    
     if (ptr == NULL) {
         return heap_caps_malloc(size, caps);
     }
@@ -287,15 +292,30 @@ IRAM_ATTR void *heap_caps_realloc( void *ptr, size_t size, int caps)
         return NULL;
     }
 
-    heap_t *heap = find_containing_heap(ptr);
+    //The pointer to memory may be aliased, we need to 
-
+    //recover the corresponding address before to manage a new allocation:
-    assert(heap != NULL && "realloc() pointer is outside heap areas");
+    if(esp_ptr_in_diram_iram((void *)ptr)) {
+        uint32_t *dram_addr = (uint32_t *)ptr;
+        dram_ptr  = (void *)dram_addr[-1];
+        
+        heap = find_containing_heap(dram_ptr);
+        assert(heap != NULL && "realloc() pointer is outside heap areas");
+        
+        //with pointers that reside on diram space, we avoid using 
+        //the realloc implementation due to address translation issues,
+        //instead force a malloc/copy/free
+        ptr_in_diram_case = true;
+    
+    } else {
+        heap = find_containing_heap(ptr);
+        assert(heap != NULL && "realloc() pointer is outside heap areas");
+    }
 
     // are the existing heap's capabilities compatible with the
     // requested ones?
     bool compatible_caps = (caps & get_all_caps(heap)) == caps;
 
-    if (compatible_caps) {
+    if (compatible_caps && !ptr_in_diram_case) {
         // try to reallocate this memory within the same heap
         // (which will resize the block if it can)
         void *r = multi_heap_realloc(heap->heap, ptr, size);
@@ -308,7 +328,16 @@ IRAM_ATTR void *heap_caps_realloc( void *ptr, size_t size, int caps)
     // in a different heap with requested capabilities.
     void *new_p = heap_caps_malloc(size, caps);
     if (new_p != NULL) {
-        size_t old_size = multi_heap_get_allocated_size(heap->heap, ptr);
+        size_t old_size = 0;
+
+        //If we're dealing with aliased ptr, information regarding its containing
+        //heap can only be obtained with translated address.
+        if(ptr_in_diram_case) {
+            old_size = multi_heap_get_allocated_size(heap->heap, dram_ptr);
+        } else {
+            old_size = multi_heap_get_allocated_size(heap->heap, ptr);
+        }
+
         assert(old_size > 0);
         memcpy(new_p, ptr, MIN(size, old_size));
         heap_caps_free(ptr);

components/heap/multi_heap_poisoning.c

@@ -268,8 +268,8 @@ void *multi_heap_realloc(multi_heap_handle_t heap, void *p, size_t size)
     new_head = multi_heap_malloc_impl(heap, size + POISON_OVERHEAD);
     if (new_head != NULL) {
         result = poison_allocated_region(new_head, size);
-        memcpy(result, p, MIN(size, orig_alloc_size));
+        memcpy(result, p, MIN(size, orig_alloc_size));      
-        multi_heap_free(heap, p);
+        multi_heap_free(heap, p);        
     }
 #endif
 

components/heap/test/test_leak.c

@@ -18,18 +18,18 @@ static char* check_calloc(int size)
 
 TEST_CASE("Check for leaks (no leak)", "[heap]")
 {
-    char *arr = check_calloc(7000);
+    char *arr = check_calloc(1000);
     free(arr);
 }
 
 TEST_CASE("Check for leaks (leak)", "[heap][ignore]")
 {
-    check_calloc(7000);
+    check_calloc(1000);
 }
 
 TEST_CASE("Not check for leaks", "[heap][leaks]")
 {
-    check_calloc(7000);
+    check_calloc(1000);
 }
 
 TEST_CASE("Set a leak level = 7016", "[heap][leaks=7016]")
@@ -39,7 +39,7 @@ TEST_CASE("Set a leak level = 7016", "[heap][leaks=7016]")
 
 static void test_fn(void)
 {
-    check_calloc(7000);
+    check_calloc(1000);
 }
 
 TEST_CASE_MULTIPLE_STAGES("Not check for leaks in MULTIPLE_STAGES mode", "[heap][leaks]", test_fn, test_fn, test_fn);
@@ -48,13 +48,13 @@ TEST_CASE_MULTIPLE_STAGES("Check for leaks in MULTIPLE_STAGES mode (leak)", "[he
 
 static void test_fn2(void)
 {
-    check_calloc(7000);
+    check_calloc(1000);
     esp_restart();
 }
 
 static void test_fn3(void)
 {
-    check_calloc(7000);
+    check_calloc(1000);
 }
 
-TEST_CASE_MULTIPLE_STAGES_ESP32("Check for leaks in MULTIPLE_STAGES mode (manual reset)", "[heap][leaks][reset=SW_CPU_RESET, SW_CPU_RESET]", test_fn2, test_fn2, test_fn3);
+TEST_CASE_MULTIPLE_STAGES("Check for leaks in MULTIPLE_STAGES mode (manual reset)", "[heap][leaks][reset=SW_CPU_RESET, SW_CPU_RESET]", test_fn2, test_fn2, test_fn3);

components/heap/test/test_malloc_caps.c

@@ -11,7 +11,7 @@
 #include <stdlib.h>
 #include <sys/param.h>
 
-TEST_CASE_ESP32("Capabilities allocator test", "[heap]")
+TEST_CASE("Capabilities allocator test", "[heap]")
 {
     char *m1, *m2[10];
     int x;
@@ -24,7 +24,7 @@ TEST_CASE_ESP32("Capabilities allocator test", "[heap]")
     free8start = heap_caps_get_free_size(MALLOC_CAP_8BIT);
     free32start = heap_caps_get_free_size(MALLOC_CAP_32BIT);
     printf("Free 8bit-capable memory (start): %dK, 32-bit capable memory %dK\n", free8start, free32start);
-    TEST_ASSERT(free32start>free8start);
+    TEST_ASSERT(free32start >= free8start);
 
     printf("Allocating 10K of 8-bit capable RAM\n");
     m1= heap_caps_malloc(10*1024, MALLOC_CAP_8BIT);
@@ -44,39 +44,60 @@ TEST_CASE_ESP32("Capabilities allocator test", "[heap]")
     size_t free_iram = heap_caps_get_free_size(MALLOC_CAP_INTERNAL) -
                            heap_caps_get_free_size(MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL);
     size_t alloc32 = MIN(free_iram / 2, 10*1024) & (~3);
-    printf("Freeing; allocating %u bytes of 32K-capable RAM\n", alloc32);
+    if(free_iram) {
-    m1 = heap_caps_malloc(alloc32, MALLOC_CAP_32BIT);
+        printf("Freeing; allocating %u bytes of 32K-capable RAM\n", alloc32);
-    printf("--> %p\n", m1);
+        m1 = heap_caps_malloc(alloc32, MALLOC_CAP_32BIT);
-    //Check that we got IRAM back
+        printf("--> %p\n", m1);
-    TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000);
+        //Check that we got IRAM back
-    free8 = heap_caps_get_free_size(MALLOC_CAP_8BIT);
+        TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000);
-    free32 = heap_caps_get_free_size(MALLOC_CAP_32BIT);
+        free8 = heap_caps_get_free_size(MALLOC_CAP_8BIT);
-    printf("Free 8bit-capable memory (after 32-bit): %dK, 32-bit capable memory %dK\n", free8, free32);
+        free32 = heap_caps_get_free_size(MALLOC_CAP_32BIT);
-    //Only 32-bit should have gone down by alloc32: 32-bit isn't necessarily 8bit capable
+        printf("Free 8bit-capable memory (after 32-bit): %dK, 32-bit capable memory %dK\n", free8, free32);
-    TEST_ASSERT(free32<(free32start-alloc32));
+        //Only 32-bit should have gone down by alloc32: 32-bit isn't necessarily 8bit capable
-    TEST_ASSERT(free8==free8start);
+        TEST_ASSERT(free32<(free32start-alloc32));
-    free(m1);
+        TEST_ASSERT(free8==free8start);
+        free(m1);
+    } else {
+        printf("This platform has no 32-bit only capable RAM, jumping to next test \n");
+    }
 
     printf("Allocating impossible caps\n");
     m1= heap_caps_malloc(10*1024, MALLOC_CAP_8BIT|MALLOC_CAP_EXEC);
     printf("--> %p\n", m1);
     TEST_ASSERT(m1==NULL);
-    printf("Testing changeover iram -> dram");
+
-    // priorities will exhaust IRAM first, then start allocating from DRAM
+    if(free_iram) {
-    for (x=0; x<10; x++) {
+        printf("Testing changeover iram -> dram");
-        m2[x]= heap_caps_malloc(alloc32, MALLOC_CAP_32BIT);
+        // priorities will exhaust IRAM first, then start allocating from DRAM
-        printf("--> %p\n", m2[x]);
+        for (x=0; x<10; x++) {
+            m2[x]= heap_caps_malloc(alloc32, MALLOC_CAP_32BIT);
+            printf("--> %p\n", m2[x]);
+        }
+        TEST_ASSERT((((int)m2[0])&0xFF000000)==0x40000000);
+        TEST_ASSERT((((int)m2[9])&0xFF000000)==0x3F000000);
+
+    } else {
+        printf("This platform has no IRAM-only so changeover will never occur, jumping to next test\n");
     }
-    TEST_ASSERT((((int)m2[0])&0xFF000000)==0x40000000);
+
-    TEST_ASSERT((((int)m2[9])&0xFF000000)==0x3F000000);
     printf("Test if allocating executable code still gives IRAM, even with dedicated IRAM region depleted\n");
-    // (the allocation should come from D/IRAM)
+    if(free_iram) {        
-    free_iram = heap_caps_get_free_size(MALLOC_CAP_EXEC);
+        // (the allocation should come from D/IRAM)
-    m1= heap_caps_malloc(MIN(free_iram / 2, 10*1024), MALLOC_CAP_EXEC);
+        free_iram = heap_caps_get_free_size(MALLOC_CAP_EXEC);
-    printf("--> %p\n", m1);
+        m1= heap_caps_malloc(MIN(free_iram / 2, 10*1024), MALLOC_CAP_EXEC);
-    TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000);
+        printf("--> %p\n", m1);
+        TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000);
+        for (x=0; x<10; x++) free(m2[x]);
+        
+    } else {
+        // (the allocation should come from D/IRAM)
+        free_iram = heap_caps_get_free_size(MALLOC_CAP_EXEC);
+        m1= heap_caps_malloc(MIN(free_iram / 2, 10*1024), MALLOC_CAP_EXEC);
+        printf("--> %p\n", m1);
+        TEST_ASSERT((((int)m1)&0xFF000000)==0x40000000);
+    }
+
     free(m1);
-    for (x=0; x<10; x++) free(m2[x]);
     printf("Done.\n");
 }
 

components/heap/test/test_realloc.c

@@ -7,6 +7,7 @@
 #include "unity.h"
 #include "sdkconfig.h"
 #include "esp_heap_caps.h"
+#include "soc/soc_memory_layout.h"
 
 
 #ifndef CONFIG_HEAP_POISONING_COMPREHENSIVE
@@ -22,7 +23,22 @@ TEST_CASE("realloc shrink buffer in place", "[heap]")
 
 #endif
 
-TEST_CASE_ESP32("realloc move data to a new heap type", "[heap]")
+TEST_CASE("realloc shrink buffer with EXEC CAPS", "[heap]")
+{
+    const size_t buffer_size = 64;
+
+    void *x = heap_caps_malloc(buffer_size, MALLOC_CAP_EXEC);
+    TEST_ASSERT(x);
+    void *y = heap_caps_realloc(x, buffer_size - 16, MALLOC_CAP_EXEC);
+    TEST_ASSERT(y);
+
+    //y needs to fall in a compatible memory area of IRAM:
+    TEST_ASSERT(esp_ptr_executable(y));
+    
+    free(y);
+}
+
+TEST_CASE("realloc move data to a new heap type", "[heap]")
 {
     const char *test = "I am some test content to put in the heap";
     char buf[64];
@@ -31,7 +47,6 @@ TEST_CASE_ESP32("realloc move data to a new heap type", "[heap]")
 
     char *a = malloc(64);
     memcpy(a, buf, 64);
-
     // move data from 'a' to IRAM
     char *b = heap_caps_realloc(a, 64, MALLOC_CAP_EXEC);
     TEST_ASSERT_NOT_NULL(b);
@@ -40,7 +55,7 @@ TEST_CASE_ESP32("realloc move data to a new heap type", "[heap]")
     TEST_ASSERT_EQUAL_HEX32_ARRAY(buf, b, 64/sizeof(uint32_t));
 
     // Move data back to DRAM
-    char *c = heap_caps_realloc(b, 48, MALLOC_CAP_8BIT);
+    char *c = heap_caps_realloc(b, 48, MALLOC_CAP_8BIT);    
     TEST_ASSERT_NOT_NULL(c);
     TEST_ASSERT_NOT_EQUAL(b, c);
     TEST_ASSERT(heap_caps_check_integrity(MALLOC_CAP_INVALID, true));

components/lwip/lwip

@@ -1 +1 @@
-Subproject commit c9e3b53c6f04052943f97210b858cec805cc98d9
+Subproject commit 31e24ae95a59e51d74f435f48fa21091b26c1430

tools/unit-test-app/components/test_utils/test_runner.c

@@ -87,6 +87,10 @@ void setUp(void)
 
 static void check_leak(size_t before_free, size_t after_free, const char *type)
 {
+    printf("MALLOC_CAP_%s leak: Leak threshold is: %u \n",
+            type, 
+            critical_leak_threshold);
+
     if (before_free <= after_free) {
         return;
     }