Merge pull request #3039 from jeffhostetler/jeffhostetler/msvc_crtdbg

Add memory leak detection/reporting using MSVC CRTDBG facility.

CMakeLists.txt

@@ -74,6 +74,11 @@ IF(WIN32)
 	OPTION(	WINHTTP			"Use Win32 WinHTTP routines"	ON	)
 ENDIF()
 
+IF(MSVC)
+	# Enable MSVC CRTDBG memory leak reporting when in debug mode.
+	OPTION(MSVC_CRTDBG "Enable CRTDBG memory leak reporting" OFF)
+ENDIF()
+
 # This variable will contain the libraries we need to put into
 # libgit2.pc's Requires.private. That is, what we're linking to or
 # what someone who's statically linking us needs to link to.
@@ -305,6 +310,10 @@ IF (MSVC)
 		SET(CRT_FLAG_RELEASE "/MD")
 	ENDIF()
 
+	IF (MSVC_CRTDBG)
+		SET(CRT_FLAG_DEBUG "${CRT_FLAG_DEBUG} /DGIT_MSVC_CRTDBG")
+	ENDIF()
+
 	# /Zi - Create debugging information
 	# /Od - Disable optimization
 	# /D_DEBUG - #define _DEBUG

appveyor.yml

@@ -29,8 +29,8 @@ build_script:
     mkdir build
     cd build
     if ($env:GENERATOR -ne "MSYS Makefiles") {
-      cmake -D ENABLE_TRACE=ON -D BUILD_CLAR=ON .. -G"$env:GENERATOR"
-      cmake --build . --config RelWithDebInfo
+      cmake -D ENABLE_TRACE=ON -D BUILD_CLAR=ON -D MSVC_CRTDBG=ON .. -G"$env:GENERATOR"
+      cmake --build . --config Debug
     }
 - cmd: |
     if "%GENERATOR%"=="MSYS Makefiles" (C:\MinGW\msys\1.0\bin\sh --login /c/projects/libgit2/script/appveyor-mingw.sh)

src/util.h

@@ -7,6 +7,36 @@
 #ifndef INCLUDE_util_h__
 #define INCLUDE_util_h__
 
+#if defined(GIT_MSVC_CRTDBG)
+/* Enable MSVC CRTDBG memory leak reporting.
+ *
+ * We DO NOT use the "_CRTDBG_MAP_ALLOC" macro described in the MSVC
+ * documentation because all allocs/frees in libgit2 already go through
+ * the "git__" routines defined in this file.  Simply using the normal
+ * reporting mechanism causes all leaks to be attributed to a routine
+ * here in util.h (ie, the actual call to calloc()) rather than the
+ * caller of git__calloc().
+ *
+ * Therefore, we declare a set of "git__crtdbg__" routines to replace
+ * the corresponding "git__" routines and re-define the "git__" symbols
+ * as macros.  This allows us to get and report the file:line info of
+ * the real caller.
+ *
+ * We DO NOT replace the "git__free" routine because it needs to remain
+ * a function pointer because it is used as a function argument when
+ * setting up various structure "destructors".
+ *
+ * We also DO NOT use the "_CRTDBG_MAP_ALLOC" macro because it causes
+ * "free" to be remapped to "_free_dbg" and this causes problems for
+ * structures which define a field named "free".
+ *
+ * Finally, CRTDBG must be explicitly enabled and configured at program
+ * startup.  See tests/main.c for an example.
+ */
+#include <stdlib.h>
+#include <crtdbg.h>
+#endif
+
 #include "common.h"
 #include "strnlen.h"
 
@@ -31,6 +61,91 @@
  */
 #define CONST_STRLEN(x) ((sizeof(x)/sizeof(x[0])) - 1)
 
+#if defined(GIT_MSVC_CRTDBG)
+GIT_INLINE(void *) git__crtdbg__malloc(size_t len, const char *file, int line)
+{
+	void *ptr = _malloc_dbg(len, _NORMAL_BLOCK, file, line);
+	if (!ptr) giterr_set_oom();
+	return ptr;
+}
+
+GIT_INLINE(void *) git__crtdbg__calloc(size_t nelem, size_t elsize, const char *file, int line)
+{
+	void *ptr = _calloc_dbg(nelem, elsize, _NORMAL_BLOCK, file, line);
+	if (!ptr) giterr_set_oom();
+	return ptr;
+}
+
+GIT_INLINE(char *) git__crtdbg__strdup(const char *str, const char *file, int line)
+{
+	char *ptr = _strdup_dbg(str, _NORMAL_BLOCK, file, line);
+	if (!ptr) giterr_set_oom();
+	return ptr;
+}
+
+GIT_INLINE(char *) git__crtdbg__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 = git__crtdbg__malloc(alloclength, file, line)))
+		return NULL;
+
+	if (length)
+		memcpy(ptr, str, length);
+
+	ptr[length] = '\0';
+
+	return ptr;
+}
+
+GIT_INLINE(char *) git__crtdbg__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 = git__crtdbg__malloc(alloclen, file, line)))
+		return NULL;
+
+	memcpy(ptr, start, n);
+	ptr[n] = '\0';
+	return ptr;
+}
+
+GIT_INLINE(void *) git__crtdbg__realloc(void *ptr, size_t size, const char *file, int line)
+{
+	void *new_ptr = _realloc_dbg(ptr, size, _NORMAL_BLOCK, file, line);
+	if (!new_ptr) giterr_set_oom();
+	return new_ptr;
+}
+
+GIT_INLINE(void *) git__crtdbg__reallocarray(void *ptr, size_t nelem, size_t elsize, const char *file, int line)
+{
+	size_t newsize;
+	return GIT_MULTIPLY_SIZET_OVERFLOW(&newsize, nelem, elsize) ?
+		NULL : _realloc_dbg(ptr, newsize, _NORMAL_BLOCK, file, line);
+}
+
+GIT_INLINE(void *) git__crtdbg__mallocarray(size_t nelem, size_t elsize, const char *file, int line)
+{
+	return git__crtdbg__reallocarray(NULL, nelem, elsize, file, line);
+}
+
+#define git__malloc(len)                      git__crtdbg__malloc(len, __FILE__, __LINE__)
+#define git__calloc(nelem, elsize)            git__crtdbg__calloc(nelem, elsize, __FILE__, __LINE__)
+#define git__strdup(str)                      git__crtdbg__strdup(str, __FILE__, __LINE__)
+#define git__strndup(str, n)                  git__crtdbg__strndup(str, n, __FILE__, __LINE__)
+#define git__substrdup(str, n)                git__crtdbg__substrdup(str, n, __FILE__, __LINE__)
+#define git__realloc(ptr, size)               git__crtdbg__realloc(ptr, size, __FILE__, __LINE__)
+#define git__reallocarray(ptr, nelem, elsize) git__crtdbg__reallocarray(ptr, nelem, elsize, __FILE__, __LINE__)
+#define git__mallocarray(nelem, elsize)       git__crtdbg__mallocarray(nelem, elsize, __FILE__, __LINE__)
+
+#else
+
 /*
  * Custom memory allocation wrappers
  * that set error code and error message
@@ -118,6 +233,8 @@ GIT_INLINE(void *) git__mallocarray(size_t nelem, size_t elsize)
 	return git__reallocarray(NULL, nelem, elsize);
 }
 
+#endif /* !MSVC_CTRDBG */
+
 GIT_INLINE(void) git__free(void *ptr)
 {
 	free(ptr);

tests/main.c

+
+#if defined(GIT_MSVC_CRTDBG)
+/* Enable MSVC CRTDBG memory leak reporting.  See src/util.h for details. */
+#include <stdlib.h>
+#include <crtdbg.h>
+#endif
+
 #include "clar_libgit2.h"
 #include "clar_libgit2_trace.h"
 
@@ -9,6 +16,18 @@ int main(int argc, char *argv[])
 {
 	int res;
 
+#if defined(GIT_MSVC_CRTDBG)
+	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
+
+	_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE);
+	_CrtSetReportMode(_CRT_ERROR,  _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE);
+	_CrtSetReportMode(_CRT_WARN,   _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE);
+
+	_CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
+	_CrtSetReportFile(_CRT_ERROR,  _CRTDBG_FILE_STDERR);
+	_CrtSetReportFile(_CRT_WARN,   _CRTDBG_FILE_STDERR);
+#endif
+
 	clar_test_init(argc, argv);
 
 	git_libgit2_init();