Merge pull request #312 from doctashay/fix-realloc-leak

libretro: fix`_realloc_r` memory leaks and zero-size reallocation

AutomatedTests/CMakeLists.txt

@@ -59,6 +59,8 @@ test(Empty.c)
 
 test(File.c PROPERTIES PASS_REGULAR_EXPRESSION "OK")
 
+test(Realloc.c PROPERTIES PASS_REGULAR_EXPRESSION "OK")
+
 #test(Timeout.c PROPERTIES PASS_REGULAR_EXPRESSION "One" DISABLED TRUE)
 
 test(Log.c PROPERTIES PASS_REGULAR_EXPRESSION "One\nTwo\nThree")

AutomatedTests/Realloc.c

@@ -0,0 +1,120 @@
+#include "Test.h"
+
+#include <MacMemory.h>
+#include <stdlib.h>
+#include <string.h>
+
+enum
+{
+    initialSize = 64 * 1024,
+    grownSize = 128 * 1024,
+    blockerSize = 8 * 1024,
+    blockerCount = 96,
+    releasedBlockers = (grownSize / blockerSize) + 4,
+    freeMemoryTolerance = 1024
+};
+
+int main(void)
+{
+    Size freeBefore = FreeMem();
+    unsigned char *allocation = malloc(initialSize);
+    void *blockers[blockerCount];
+    unsigned char *grown;
+    Size freeAfter;
+    int failure = 0;
+    int numBlockers = 0;
+    int ok = allocation != NULL;
+    int i;
+
+    memset(blockers, 0, sizeof(blockers));
+
+    if(!ok)
+        failure = 1;
+
+    if(ok)
+    {
+        for(i = 0; i < initialSize; ++i)
+            allocation[i] = (unsigned char)(i * 37 + 11);
+
+        while(numBlockers < blockerCount &&
+            (blockers[numBlockers] = malloc(blockerSize)) != NULL)
+        {
+            memset(blockers[numBlockers], 0x5a, blockerSize);
+            ++numBlockers;
+        }
+
+        if(numBlockers <= releasedBlockers)
+        {
+            ok = 0;
+            failure = 6;
+        }
+
+        for(i = numBlockers - releasedBlockers; i < numBlockers; ++i)
+        {
+            if(i >= 0)
+            {
+                free(blockers[i]);
+                blockers[i] = NULL;
+            }
+        }
+
+        grown = realloc(allocation, grownSize);
+        if(!grown)
+        {
+            ok = 0;
+            failure = 2;
+        }
+        else if(grown == allocation)
+        {
+            ok = 0;
+            failure = 3;
+        }
+
+        if(grown)
+        {
+            for(i = 0; ok && i < initialSize; ++i)
+            {
+                if(grown[i] != (unsigned char)(i * 37 + 11))
+                {
+                    ok = 0;
+                    failure = 4;
+                }
+            }
+            free(grown);
+        }
+        else
+        {
+            free(allocation);
+        }
+
+    }
+    else
+    {
+        free(allocation);
+    }
+
+    for(i = 0; i < numBlockers; ++i)
+        free(blockers[i]);
+
+    freeAfter = FreeMem();
+    if(ok && freeAfter + freeMemoryTolerance < freeBefore)
+    {
+        ok = 0;
+        failure = 5;
+    }
+
+    if(ok)
+    {
+        TEST_LOG_OK();
+    }
+    else
+    {
+        char result[3];
+        result[0] = 'N';
+        result[1] = (char)('0' + failure);
+        result[2] = '\0';
+        TEST_LOG_SIZED(result, 2);
+    }
+
+    return ok ? 0 : 1;
+}

libretro/malloc.c

@@ -69,27 +69,30 @@ void *_realloc_r(struct _reent *reent_ptr, void *ptr, size_t sz)
     }
     else
     {
+        if(sz == 0)
+        {
+            DisposePtr(ptr);
+            return NULL;
+        }
+
+        /* Save this now in case the block has to be moved. */
+        size_t oldSz = GetPtrSize(ptr);
+
         MemError();
         SetPtrSize(ptr, sz);
         if(MemError())
         {
-            size_t oldSz = GetPtrSize(ptr);
-            if(sz > oldSz)
-            {
-                void *newPtr = NewPtr(sz);
-                if(!newPtr)
-                {
-                    reent_ptr->_errno = ENOMEM;
-                    return NULL;
-                }
-                memcpy(newPtr, ptr, oldSz);
-                return newPtr;
-            }
-            else
+            void *newPtr = NewPtr(sz);
+            if(!newPtr)
             {
                 reent_ptr->_errno = ENOMEM;
                 return NULL;
             }
+
+            memcpy(newPtr, ptr, sz < oldSz ? sz : oldSz);
+            /* The new block owns the data, so the old one can be released. */
+            DisposePtr(ptr);
+            return newPtr;
         }
         else
             return ptr;