Merge pull request #6563 from libgit2/pks/test-allocator

tests: add allocator with limited number of bytes

include/git2/sys/alloc.h

@@ -25,28 +25,6 @@ typedef struct {
 	void * GIT_CALLBACK(gmalloc)(size_t n, const char *file, int line);
 
 	/**
-	 * Allocate memory for an array of `nelem` elements, where each element
-	 * has a size of `elsize`. Returned memory shall be initialized to
-	 * all-zeroes
-	 */
-	void * GIT_CALLBACK(gcalloc)(size_t nelem, size_t elsize, const char *file, int line);
-
-	/** Allocate memory for the string `str` and duplicate its contents. */
-	char * GIT_CALLBACK(gstrdup)(const char *str, const char *file, int line);
-
-	/**
-	 * Equivalent to the `gstrdup` function, but only duplicating at most
-	 * `n + 1` bytes
-	 */
-	char * GIT_CALLBACK(gstrndup)(const char *str, size_t n, const char *file, int line);
-
-	/**
-	 * Equivalent to `gstrndup`, but will always duplicate exactly `n` bytes
-	 * of `str`. Thus, out of bounds reads at `str` may happen.
-	 */
-	char * GIT_CALLBACK(gsubstrdup)(const char *str, size_t n, const char *file, int line);
-
-	/**
 	 * This function shall deallocate the old object `ptr` and return a
 	 * pointer to a new object that has the size specified by `size`. In
 	 * case `ptr` is `NULL`, a new array shall be allocated.
@@ -54,18 +32,6 @@ typedef struct {
 	void * GIT_CALLBACK(grealloc)(void *ptr, size_t size, const char *file, int line);
 
 	/**
-	 * This function shall be equivalent to `grealloc`, but allocating
-	 * `neleme * elsize` bytes.
-	 */
-	void * GIT_CALLBACK(greallocarray)(void *ptr, size_t nelem, size_t elsize, const char *file, int line);
-
-	/**
-	 * This function shall allocate a new array of `nelem` elements, where
-	 * each element has a size of `elsize` bytes.
-	 */
-	void * GIT_CALLBACK(gmallocarray)(size_t nelem, size_t elsize, const char *file, int line);
-
-	/**
 	 * This function shall free the memory pointed to by `ptr`. In case
 	 * `ptr` is `NULL`, this shall be a no-op.
 	 */

src/libgit2/threadstate.c

@@ -75,10 +75,23 @@ git_threadstate *git_threadstate_get(void)
 	if ((threadstate = git_tlsdata_get(tls_key)) != NULL)
 		return threadstate;
 
-	if ((threadstate = git__calloc(1, sizeof(git_threadstate))) == NULL ||
-	    git_str_init(&threadstate->error_buf, 0) < 0)
+	/*
+	 * Avoid git__malloc here, since if it fails, it sets an error
+	 * message, which requires thread state, which would allocate
+	 * here, which would fail, which would set an error message...
+	 */
+
+	if ((threadstate = git__allocator.gmalloc(sizeof(git_threadstate),
+			__FILE__, __LINE__)) == NULL)
 		return NULL;
 
+	memset(threadstate, 0, sizeof(git_threadstate));
+
+	if (git_str_init(&threadstate->error_buf, 0) < 0) {
+		git__allocator.gfree(threadstate);
+		return NULL;
+	}
+
 	git_tlsdata_set(tls_key, threadstate);
 	return threadstate;
 }

src/util/alloc.c

@@ -15,16 +15,75 @@
 /* Fail any allocation until git_libgit2_init is called. */
 git_allocator git__allocator = {
 	git_failalloc_malloc,
-	git_failalloc_calloc,
-	git_failalloc_strdup,
-	git_failalloc_strndup,
-	git_failalloc_substrdup,
 	git_failalloc_realloc,
-	git_failalloc_reallocarray,
-	git_failalloc_mallocarray,
 	git_failalloc_free
 };
 
+void *git__calloc(size_t nelem, size_t elsize)
+{
+	size_t newsize;
+	void *ptr;
+
+	if (GIT_MULTIPLY_SIZET_OVERFLOW(&newsize, nelem, elsize))
+		return NULL;
+
+	if ((ptr = git__malloc(newsize)))
+		memset(ptr, 0, newsize);
+
+	return ptr;
+}
+
+void *git__reallocarray(void *ptr, size_t nelem, size_t elsize)
+{
+	size_t newsize;
+
+	if (GIT_MULTIPLY_SIZET_OVERFLOW(&newsize, nelem, elsize))
+		return NULL;
+
+	return git__realloc(ptr, newsize);
+}
+
+void *git__mallocarray(size_t nelem, size_t elsize)
+{
+	return git__reallocarray(NULL, nelem, elsize);
+}
+
+char *git__strdup(const char *str)
+{
+	size_t len = strlen(str) + 1;
+	void *ptr = git__malloc(len);
+
+	if (ptr)
+		memcpy(ptr, str, len);
+
+	return ptr;
+}
+
+char *git__strndup(const char *str, size_t n)
+{
+	size_t len = p_strnlen(str, n);
+	char *ptr = git__malloc(len + 1);
+
+	if (ptr) {
+		memcpy(ptr, str, len);
+		ptr[len] = '\0';
+	}
+
+	return ptr;
+}
+
+char *git__substrdup(const char *str, size_t n)
+{
+	char *ptr = git__malloc(n + 1);
+
+	if (ptr) {
+		memcpy(ptr, str, n);
+		ptr[n] = '\0';
+	}
+
+	return ptr;
+}
+
 static int setup_default_allocator(void)
 {
 #if defined(GIT_WIN32_LEAKCHECK)

src/util/alloc.h

@@ -10,17 +10,42 @@
 
 #include "git2/sys/alloc.h"
 
+#include "git2_util.h"
+
 extern git_allocator git__allocator;
 
-#define git__malloc(len)                      git__allocator.gmalloc(len, __FILE__, __LINE__)
-#define git__calloc(nelem, elsize)            git__allocator.gcalloc(nelem, elsize, __FILE__, __LINE__)
-#define git__strdup(str)                      git__allocator.gstrdup(str, __FILE__, __LINE__)
-#define git__strndup(str, n)                  git__allocator.gstrndup(str, n, __FILE__, __LINE__)
-#define git__substrdup(str, n)                git__allocator.gsubstrdup(str, n, __FILE__, __LINE__)
-#define git__realloc(ptr, size)               git__allocator.grealloc(ptr, size, __FILE__, __LINE__)
-#define git__reallocarray(ptr, nelem, elsize) git__allocator.greallocarray(ptr, nelem, elsize, __FILE__, __LINE__)
-#define git__mallocarray(nelem, elsize)       git__allocator.gmallocarray(nelem, elsize, __FILE__, __LINE__)
-#define git__free                             git__allocator.gfree
+GIT_INLINE(void *) git__malloc(size_t len)
+{
+	void *p = git__allocator.gmalloc(len, __FILE__, __LINE__);
+
+	if (!p)
+		git_error_set_oom();
+
+	return p;
+}
+
+GIT_INLINE(void *) git__realloc(void *ptr, size_t size)
+{
+	void *p = git__allocator.grealloc(ptr, size, __FILE__, __LINE__);
+
+	if (!p)
+		git_error_set_oom();
+
+	return p;
+}
+
+GIT_INLINE(void) git__free(void *ptr)
+{
+	git__allocator.gfree(ptr);
+}
+
+extern void *git__calloc(size_t nelem, size_t elsize);
+extern void *git__mallocarray(size_t nelem, size_t elsize);
+extern void *git__reallocarray(void *ptr, size_t nelem, size_t elsize);
+
+extern char *git__strdup(const char *str);
+extern char *git__strndup(const char *str, size_t n);
+extern char *git__substrdup(const char *str, size_t n);
 
 /**
  * This function is being called by our global setup routines to

src/util/allocators/failalloc.c

@@ -16,45 +16,6 @@ void *git_failalloc_malloc(size_t len, const char *file, int line)
 	return NULL;
 }
 
-void *git_failalloc_calloc(size_t nelem, size_t elsize, const char *file, int line)
-{
-	GIT_UNUSED(nelem);
-	GIT_UNUSED(elsize);
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-	return NULL;
-}
-
-char *git_failalloc_strdup(const char *str, const char *file, int line)
-{
-	GIT_UNUSED(str);
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-	return NULL;
-}
-
-char *git_failalloc_strndup(const char *str, size_t n, const char *file, int line)
-{
-	GIT_UNUSED(str);
-	GIT_UNUSED(n);
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-	return NULL;
-}
-
-char *git_failalloc_substrdup(const char *start, size_t n, const char *file, int line)
-{
-	GIT_UNUSED(start);
-	GIT_UNUSED(n);
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-	return NULL;
-}
-
 void *git_failalloc_realloc(void *ptr, size_t size, const char *file, int line)
 {
 	GIT_UNUSED(ptr);
@@ -65,27 +26,6 @@ void *git_failalloc_realloc(void *ptr, size_t size, const char *file, int line)
 	return NULL;
 }
 
-void *git_failalloc_reallocarray(void *ptr, size_t nelem, size_t elsize, const char *file, int line)
-{
-	GIT_UNUSED(ptr);
-	GIT_UNUSED(nelem);
-	GIT_UNUSED(elsize);
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-	return NULL;
-}
-
-void *git_failalloc_mallocarray(size_t nelem, size_t elsize, const char *file, int line)
-{
-	GIT_UNUSED(nelem);
-	GIT_UNUSED(elsize);
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-	return NULL;
-}
-
 void git_failalloc_free(void *ptr)
 {
 	GIT_UNUSED(ptr);

src/util/allocators/failalloc.h

@@ -11,13 +11,7 @@
 #include "git2_util.h"
 
 extern void *git_failalloc_malloc(size_t len, const char *file, int line);
-extern void *git_failalloc_calloc(size_t nelem, size_t elsize, const char *file, int line);
-extern char *git_failalloc_strdup(const char *str, const char *file, int line);
-extern char *git_failalloc_strndup(const char *str, size_t n, const char *file, int line);
-extern char *git_failalloc_substrdup(const char *start, size_t n, const char *file, int line);
 extern void *git_failalloc_realloc(void *ptr, size_t size, const char *file, int line);
-extern void *git_failalloc_reallocarray(void *ptr, size_t nelem, size_t elsize, const char *file, int line);
-extern void *git_failalloc_mallocarray(size_t nelem, size_t elsize, const char *file, int line);
 extern void git_failalloc_free(void *ptr);
 
 #endif

src/util/allocators/stdalloc.c

@@ -9,8 +9,6 @@
 
 static void *stdalloc__malloc(size_t len, const char *file, int line)
 {
-	void *ptr;
-
 	GIT_UNUSED(file);
 	GIT_UNUSED(line);
 
@@ -19,86 +17,11 @@ static void *stdalloc__malloc(size_t len, const char *file, int line)
 		return NULL;
 #endif
 
-	ptr = malloc(len);
-
-	if (!ptr)
-		git_error_set_oom();
-
-	return ptr;
-}
-
-static void *stdalloc__calloc(size_t nelem, size_t elsize, const char *file, int line)
-{
-	void *ptr;
-
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-#ifdef GIT_DEBUG_STRICT_ALLOC
-	if (!elsize || !nelem)
-		return NULL;
-#endif
-
-	ptr = calloc(nelem, elsize);
-
-	if (!ptr)
-		git_error_set_oom();
-
-	return ptr;
-}
-
-static char *stdalloc__strdup(const char *str, const char *file, int line)
-{
-	char *ptr;
-
-	GIT_UNUSED(file);
-	GIT_UNUSED(line);
-
-	ptr = strdup(str);
-
-	if (!ptr)
-		git_error_set_oom();
-
-	return ptr;
-}
-
-static char *stdalloc__strndup(const char *str, size_t n, const char *file, int line)
-{
-	size_t length = 0, alloclength;
-	char *ptr;
-
-	length = p_strnlen(str, n);
-
-	if (GIT_ADD_SIZET_OVERFLOW(&alloclength, length, 1) ||
-	    !(ptr = stdalloc__malloc(alloclength, file, line)))
-		return NULL;
-
-	if (length)
-		memcpy(ptr, str, length);
-
-	ptr[length] = '\0';
-
-	return ptr;
-}
-
-static char *stdalloc__substrdup(const char *start, size_t n, const char *file, int line)
-{
-	char *ptr;
-	size_t alloclen;
-
-	if (GIT_ADD_SIZET_OVERFLOW(&alloclen, n, 1) ||
-	    !(ptr = stdalloc__malloc(alloclen, file, line)))
-		return NULL;
-
-	memcpy(ptr, start, n);
-	ptr[n] = '\0';
-	return ptr;
+	return malloc(len);
 }
 
 static void *stdalloc__realloc(void *ptr, size_t size, const char *file, int line)
 {
-	void *new_ptr;
-
 	GIT_UNUSED(file);
 	GIT_UNUSED(line);
 
@@ -107,27 +30,7 @@ static void *stdalloc__realloc(void *ptr, size_t size, const char *file, int lin
 		return NULL;
 #endif
 
-	new_ptr = realloc(ptr, size);
-
-	if (!new_ptr)
-		git_error_set_oom();
-
-	return new_ptr;
-}
-
-static void *stdalloc__reallocarray(void *ptr, size_t nelem, size_t elsize, const char *file, int line)
-{
-	size_t newsize;
-
-	if (GIT_MULTIPLY_SIZET_OVERFLOW(&newsize, nelem, elsize))
-		return NULL;
-
-	return stdalloc__realloc(ptr, newsize, file, line);
-}
-
-static void *stdalloc__mallocarray(size_t nelem, size_t elsize, const char *file, int line)
-{
-	return stdalloc__reallocarray(NULL, nelem, elsize, file, line);
+	return realloc(ptr, size);
 }
 
 static void stdalloc__free(void *ptr)
@@ -138,13 +41,7 @@ static void stdalloc__free(void *ptr)
 int git_stdalloc_init_allocator(git_allocator *allocator)
 {
 	allocator->gmalloc = stdalloc__malloc;
-	allocator->gcalloc = stdalloc__calloc;
-	allocator->gstrdup = stdalloc__strdup;
-	allocator->gstrndup = stdalloc__strndup;
-	allocator->gsubstrdup = stdalloc__substrdup;
 	allocator->grealloc = stdalloc__realloc;
-	allocator->greallocarray = stdalloc__reallocarray;
-	allocator->gmallocarray = stdalloc__mallocarray;
 	allocator->gfree = stdalloc__free;
 	return 0;
 }

src/util/allocators/win32_leakcheck.c

@@ -18,53 +18,6 @@ static void *leakcheck_malloc(size_t len, const char *file, int line)
 	return ptr;
 }
 
-static void *leakcheck_calloc(size_t nelem, size_t elsize, const char *file, int line)
-{
-	void *ptr = _calloc_dbg(nelem, elsize, _NORMAL_BLOCK, git_win32_leakcheck_stacktrace(1,file), line);
-	if (!ptr) git_error_set_oom();
-	return ptr;
-}
-
-static char *leakcheck_strdup(const char *str, const char *file, int line)
-{
-	char *ptr = _strdup_dbg(str, _NORMAL_BLOCK, git_win32_leakcheck_stacktrace(1,file), line);
-	if (!ptr) git_error_set_oom();
-	return ptr;
-}
-
-static char *leakcheck_strndup(const char *str, size_t n, const char *file, int line)
-{
-	size_t length = 0, alloclength;
-	char *ptr;
-
-	length = p_strnlen(str, n);
-
-	if (GIT_ADD_SIZET_OVERFLOW(&alloclength, length, 1) ||
-		!(ptr = leakcheck_malloc(alloclength, file, line)))
-		return NULL;
-
-	if (length)
-		memcpy(ptr, str, length);
-
-	ptr[length] = '\0';
-
-	return ptr;
-}
-
-static char *leakcheck_substrdup(const char *start, size_t n, const char *file, int line)
-{
-	char *ptr;
-	size_t alloclen;
-
-	if (GIT_ADD_SIZET_OVERFLOW(&alloclen, n, 1) ||
-		!(ptr = leakcheck_malloc(alloclen, file, line)))
-		return NULL;
-
-	memcpy(ptr, start, n);
-	ptr[n] = '\0';
-	return ptr;
-}
-
 static void *leakcheck_realloc(void *ptr, size_t size, const char *file, int line)
 {
 	void *new_ptr = _realloc_dbg(ptr, size, _NORMAL_BLOCK, git_win32_leakcheck_stacktrace(1,file), line);
@@ -72,21 +25,6 @@ static void *leakcheck_realloc(void *ptr, size_t size, const char *file, int lin
 	return new_ptr;
 }
 
-static void *leakcheck_reallocarray(void *ptr, size_t nelem, size_t elsize, const char *file, int line)
-{
-	size_t newsize;
-
-	if (GIT_MULTIPLY_SIZET_OVERFLOW(&newsize, nelem, elsize))
-		return NULL;
-
-	return leakcheck_realloc(ptr, newsize, file, line);
-}
-
-static void *leakcheck_mallocarray(size_t nelem, size_t elsize, const char *file, int line)
-{
-	return leakcheck_reallocarray(NULL, nelem, elsize, file, line);
-}
-
 static void leakcheck_free(void *ptr)
 {
 	free(ptr);
@@ -95,13 +33,7 @@ static void leakcheck_free(void *ptr)
 int git_win32_leakcheck_init_allocator(git_allocator *allocator)
 {
 	allocator->gmalloc = leakcheck_malloc;
-	allocator->gcalloc = leakcheck_calloc;
-	allocator->gstrdup = leakcheck_strdup;
-	allocator->gstrndup = leakcheck_strndup;
-	allocator->gsubstrdup = leakcheck_substrdup;
 	allocator->grealloc = leakcheck_realloc;
-	allocator->greallocarray = leakcheck_reallocarray;
-	allocator->gmallocarray = leakcheck_mallocarray;
 	allocator->gfree = leakcheck_free;
 	return 0;
 }

tests/clar/clar_libgit2_alloc.c

+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+
+#include "clar_libgit2_alloc.h"
+
+static size_t bytes_available;
+
+/*
+ * The clar allocator uses a tagging mechanism for pointers that
+ * prepends the actual pointer's number bytes as `size_t`.
+ *
+ * First, this is required in order to be able to implement
+ * proper bookkeeping of allocated bytes in both `free` and
+ * `realloc`.
+ *
+ * Second, it may also be able to spot bugs that are
+ * otherwise hard to grasp, as the returned pointer cannot be
+ * free'd directly via free(3P). Instead, one is forced to use
+ * the tandem of `cl__malloc` and `cl__free`, as otherwise the
+ * code is going to crash hard. This is considered to be a
+ * feature, as it helps e.g. in finding cases where by accident
+ * malloc(3P) and free(3P) were used instead of git__malloc and
+ * git__free, respectively.
+ *
+ * The downside is obviously that each allocation grows by
+ * sizeof(size_t) bytes. As the allocator is for testing purposes
+ * only, this tradeoff is considered to be perfectly fine,
+ * though.
+ */
+
+static void *cl__malloc(size_t len, const char *file, int line)
+{
+	char *ptr = NULL;
+	size_t alloclen;
+
+	GIT_UNUSED(file);
+	GIT_UNUSED(line);
+
+	if (len > bytes_available)
+		goto out;
+
+	if (GIT_ADD_SIZET_OVERFLOW(&alloclen, len, sizeof(size_t)) ||
+	    (ptr = malloc(alloclen)) == NULL)
+		goto out;
+	memcpy(ptr, &len, sizeof(size_t));
+
+	bytes_available -= len;
+
+out:
+	return ptr ? ptr + sizeof(size_t) : NULL;
+}
+
+static void cl__free(void *ptr)
+{
+	if (ptr) {
+		char *p = ptr;
+		size_t len;
+		memcpy(&len, p - sizeof(size_t), sizeof(size_t));
+		free(p - sizeof(size_t));
+		bytes_available += len;
+	}
+}
+
+static void *cl__realloc(void *ptr, size_t size, const char *file, int line)
+{
+	size_t copybytes = 0;
+	char *p = ptr;
+	void *new;
+
+	if (p)
+		memcpy(&copybytes, p - sizeof(size_t), sizeof(size_t));
+
+	if (copybytes > size)
+		copybytes = size;
+
+	if ((new = cl__malloc(size, file, line)) == NULL)
+		goto out;
+
+	if (p) {
+		memcpy(new, p, copybytes);
+		cl__free(p);
+	}
+
+out:
+	return new;
+}
+
+void cl_alloc_limit(size_t bytes)
+{
+	git_allocator alloc;
+
+	alloc.gmalloc = cl__malloc;
+	alloc.grealloc = cl__realloc;
+	alloc.gfree = cl__free;
+
+	git_allocator_setup(&alloc);
+
+	bytes_available = bytes;
+}
+
+void cl_alloc_reset(void)
+{
+	git_allocator stdalloc;
+	git_stdalloc_init_allocator(&stdalloc);
+	git_allocator_setup(&stdalloc);
+}

tests/clar/clar_libgit2_alloc.h

+#ifndef __CLAR_LIBGIT2_ALLOC__
+#define __CLAR_LIBGIT2_ALLOC__
+
+#include "clar.h"
+#include "common.h"
+#include "git2/sys/alloc.h"
+
+void cl_alloc_limit(size_t bytes);
+void cl_alloc_reset(void);
+
+#endif

tests/util/alloc.c

+#include "clar_libgit2.h"
+#include "clar_libgit2_alloc.h"
+#include "alloc.h"
+
+void test_alloc__cleanup(void)
+{
+	cl_alloc_reset();
+}
+
+void test_alloc__oom(void)
+{
+	void *ptr = NULL;
+
+	cl_alloc_limit(0);
+
+	cl_assert(git__malloc(1) == NULL);
+	cl_assert(git__calloc(1, 1) == NULL);
+	cl_assert(git__realloc(ptr, 1) == NULL);
+	cl_assert(git__strdup("test") == NULL);
+	cl_assert(git__strndup("test", 4) == NULL);
+}
+
+void test_alloc__single_byte_is_exhausted(void)
+{
+	void *ptr;
+
+	cl_alloc_limit(1);
+
+	cl_assert(ptr = git__malloc(1));
+	cl_assert(git__malloc(1) == NULL);
+	git__free(ptr);
+}
+
+void test_alloc__free_replenishes_byte(void)
+{
+	void *ptr;
+
+	cl_alloc_limit(1);
+
+	cl_assert(ptr = git__malloc(1));
+	cl_assert(git__malloc(1) == NULL);
+	git__free(ptr);
+	cl_assert(ptr = git__malloc(1));
+	git__free(ptr);
+}
+
+void test_alloc__realloc(void)
+{
+	char *ptr = NULL;
+
+	cl_alloc_limit(3);
+
+	cl_assert(ptr = git__realloc(ptr, 1));
+	*ptr = 'x';
+
+	cl_assert(ptr = git__realloc(ptr, 1));
+	cl_assert_equal_i(*ptr, 'x');
+
+	cl_assert(ptr = git__realloc(ptr, 2));
+	cl_assert_equal_i(*ptr, 'x');
+
+	cl_assert(git__realloc(ptr, 2) == NULL);
+
+	cl_assert(ptr = git__realloc(ptr, 1));
+	cl_assert_equal_i(*ptr, 'x');
+
+	git__free(ptr);
+}

tests/util/str/oom.c

 #include "clar_libgit2.h"
+#include "clar_libgit2_alloc.h"
 
 /* Override default allocators with ones that will fail predictably. */
 
@@ -56,3 +57,15 @@ void test_str_oom__grow_by(void)
 	cl_assert(git_str_grow_by(&buf, 101) == -1);
 	cl_assert(git_str_oom(&buf));
 }
+
+void test_str_oom__allocation_failure(void)
+{
+	git_str buf = GIT_STR_INIT;
+
+	cl_alloc_limit(10);
+
+	cl_git_pass(git_str_puts(&buf, "foobar"));
+	cl_git_fail(git_str_puts(&buf, "foobar"));
+
+	cl_alloc_reset();
+}