From 06bae56d30220246e7d3a30a3a54ff10ec8b4122 Mon Sep 17 00:00:00 2001 From: jsalling <jsalling@users.noreply.github.com> Date: Tue, 9 Aug 2016 00:51:38 -0500 Subject: [PATCH 1/4] Revert "Merge pull request #205 from bryongloden/patch-2" This reverts commit 783fcaea97cf11975d75254742e800d7759fdbd0 The guard memory bytes should never be freed inside unity_malloc() --- extras/fixture/src/unity_fixture.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/extras/fixture/src/unity_fixture.c b/extras/fixture/src/unity_fixture.c index 79d925e..7e4d1e8 100644 --- a/extras/fixture/src/unity_fixture.c +++ b/extras/fixture/src/unity_fixture.c @@ -207,9 +207,6 @@ void* unity_malloc(size_t size) mem = (char*)&(guard[1]); memcpy(&mem[size], end, sizeof(end)); -#ifndef UNITY_FIXTURE_MALLOC_OVERRIDES_H_ - free(guard); -#endif return (void*)mem; } From 03ac71b8c99312aa76f9f38cc9ce1bd1287c9d57 Mon Sep 17 00:00:00 2001 From: jsalling <jsalling@users.noreply.github.com> Date: Sun, 21 Aug 2016 11:27:47 -0500 Subject: [PATCH 2/4] Reorder free calls to free all memory The internal malloc must free in LIFO order --- extras/fixture/test/unity_fixture_Test.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/extras/fixture/test/unity_fixture_Test.c b/extras/fixture/test/unity_fixture_Test.c index 0729e1f..42c3aab 100644 --- a/extras/fixture/test/unity_fixture_Test.c +++ b/extras/fixture/test/unity_fixture_Test.c @@ -523,14 +523,11 @@ TEST(InternalMalloc, ReallocFailDoesNotFreeMem) void* n1 = malloc(10); void* out_of_mem = realloc(n1, UNITY_INTERNAL_HEAP_SIZE_BYTES/2 + 1); void* n2 = malloc(10); - TEST_ASSERT_NOT_NULL(m); - if (out_of_mem == NULL) - { - free(n1); - TEST_ASSERT_NULL(out_of_mem); - } - TEST_ASSERT_NOT_EQUAL(n2, n1); free(n2); + if (out_of_mem == NULL) free(n1); free(m); + TEST_ASSERT_NOT_NULL(m); + TEST_ASSERT_NULL(out_of_mem); + TEST_ASSERT_NOT_EQUAL(n2, n1); #endif } From d837342b15d461441e6a8ed4395d56d18e266023 Mon Sep 17 00:00:00 2001 From: jsalling <jsalling@users.noreply.github.com> Date: Sun, 21 Aug 2016 11:45:54 -0500 Subject: [PATCH 3/4] Move free() calls before test asserts, add comments to ReallocFail test --- extras/fixture/test/unity_fixture_Test.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/extras/fixture/test/unity_fixture_Test.c b/extras/fixture/test/unity_fixture_Test.c index 42c3aab..472ffb1 100644 --- a/extras/fixture/test/unity_fixture_Test.c +++ b/extras/fixture/test/unity_fixture_Test.c @@ -488,9 +488,9 @@ TEST(InternalMalloc, MallocPastBufferFails) #ifdef UNITY_EXCLUDE_STDLIB_MALLOC void* m = malloc(UNITY_INTERNAL_HEAP_SIZE_BYTES/2 + 1); void* n = malloc(UNITY_INTERNAL_HEAP_SIZE_BYTES/2); + free(m); TEST_ASSERT_NOT_NULL(m); TEST_ASSERT_NULL(n); - free(m); #endif } @@ -499,9 +499,9 @@ TEST(InternalMalloc, CallocPastBufferFails) #ifdef UNITY_EXCLUDE_STDLIB_MALLOC void* m = calloc(1, UNITY_INTERNAL_HEAP_SIZE_BYTES/2 + 1); void* n = calloc(1, UNITY_INTERNAL_HEAP_SIZE_BYTES/2); + free(m); TEST_ASSERT_NOT_NULL(m); TEST_ASSERT_NULL(n); - free(m); #endif } @@ -510,9 +510,9 @@ TEST(InternalMalloc, MallocThenReallocGrowsMemoryInPlace) #ifdef UNITY_EXCLUDE_STDLIB_MALLOC void* m = malloc(UNITY_INTERNAL_HEAP_SIZE_BYTES/2 + 1); void* n = realloc(m, UNITY_INTERNAL_HEAP_SIZE_BYTES/2 + 9); + free(n); TEST_ASSERT_NOT_NULL(m); TEST_ASSERT_EQUAL(m, n); - free(n); #endif } @@ -526,8 +526,9 @@ TEST(InternalMalloc, ReallocFailDoesNotFreeMem) free(n2); if (out_of_mem == NULL) free(n1); free(m); - TEST_ASSERT_NOT_NULL(m); - TEST_ASSERT_NULL(out_of_mem); - TEST_ASSERT_NOT_EQUAL(n2, n1); + + TEST_ASSERT_NOT_NULL(m); // Got a real memory location + TEST_ASSERT_NULL(out_of_mem); // The realloc should have failed + TEST_ASSERT_NOT_EQUAL(n2, n1); // If n1 != n2 then realloc did not free n1 #endif } From 92f6d5dd08c26ae8c9a681e8058561a5923e762b Mon Sep 17 00:00:00 2001 From: jsalling <jsalling@users.noreply.github.com> Date: Sun, 21 Aug 2016 11:53:15 -0500 Subject: [PATCH 4/4] Verify the tests for Internal Malloc implementation free all the heap Make it more clear that each test of the internal heap implementation should free in LIFO order. Without this check, memory can be stranded but still pass. --- extras/fixture/test/unity_fixture_Test.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/extras/fixture/test/unity_fixture_Test.c b/extras/fixture/test/unity_fixture_Test.c index 472ffb1..3f2d32f 100644 --- a/extras/fixture/test/unity_fixture_Test.c +++ b/extras/fixture/test/unity_fixture_Test.c @@ -479,6 +479,10 @@ TEST(LeakDetection, PointerSettingMax) //------------------------------------------------------------ TEST_GROUP(InternalMalloc); +#define TEST_ASSERT_MEMORY_ALL_FREE_LIFO_ORDER(first_mem_ptr, ptr) \ + ptr = malloc(10); free(ptr); \ + TEST_ASSERT_EQUAL_PTR_MESSAGE(first_mem_ptr, ptr, "Memory was stranded, free in LIFO order"); + TEST_SETUP(InternalMalloc) { } TEST_TEAR_DOWN(InternalMalloc) { } @@ -491,6 +495,7 @@ TEST(InternalMalloc, MallocPastBufferFails) free(m); TEST_ASSERT_NOT_NULL(m); TEST_ASSERT_NULL(n); + TEST_ASSERT_MEMORY_ALL_FREE_LIFO_ORDER(m, n); #endif } @@ -502,6 +507,7 @@ TEST(InternalMalloc, CallocPastBufferFails) free(m); TEST_ASSERT_NOT_NULL(m); TEST_ASSERT_NULL(n); + TEST_ASSERT_MEMORY_ALL_FREE_LIFO_ORDER(m, n); #endif } @@ -513,6 +519,7 @@ TEST(InternalMalloc, MallocThenReallocGrowsMemoryInPlace) free(n); TEST_ASSERT_NOT_NULL(m); TEST_ASSERT_EQUAL(m, n); + TEST_ASSERT_MEMORY_ALL_FREE_LIFO_ORDER(m, n); #endif } @@ -523,6 +530,7 @@ TEST(InternalMalloc, ReallocFailDoesNotFreeMem) void* n1 = malloc(10); void* out_of_mem = realloc(n1, UNITY_INTERNAL_HEAP_SIZE_BYTES/2 + 1); void* n2 = malloc(10); + free(n2); if (out_of_mem == NULL) free(n1); free(m); @@ -530,5 +538,6 @@ TEST(InternalMalloc, ReallocFailDoesNotFreeMem) TEST_ASSERT_NOT_NULL(m); // Got a real memory location TEST_ASSERT_NULL(out_of_mem); // The realloc should have failed TEST_ASSERT_NOT_EQUAL(n2, n1); // If n1 != n2 then realloc did not free n1 + TEST_ASSERT_MEMORY_ALL_FREE_LIFO_ORDER(m, n2); #endif }