commit 9fa73003fca5e884c0c663c2035ce6e60994c339 Author: Nick Mathewson nickm@torproject.org Date: Wed Jun 20 10:19:56 2018 -0400

Remove dmalloc support; closes #26426

Dmalloc hasn't seen a release in over a decade, and there are much better tools to use these days. --- changes/ticket26426 | 4 +++ configure.ac | 20 ------------- doc/HACKING/HelpfulTools.md | 10 ------- src/common/crypto.c | 13 -------- src/common/crypto.h | 4 --- src/common/util.c | 73 ++++++++++----------------------------------- src/common/util.h | 61 +++++++++++-------------------------- src/or/main.c | 26 +--------------- src/test/testing_common.c | 18 +---------- 9 files changed, 40 insertions(+), 189 deletions(-)

diff --git a/changes/ticket26426 b/changes/ticket26426 new file mode 100644 index 000000000..05fa97494 --- /dev/null +++ b/changes/ticket26426 @@ -0,0 +1,4 @@ + o Removed features: + - Tor no longer supports building with the dmalloc library. For debugging + memory issues, we suggest using gperftools or msan instead. + Closes ticket 26426. diff --git a/configure.ac b/configure.ac index 8a0d135c5..d46bcf8a9 100644 --- a/configure.ac +++ b/configure.ac @@ -1726,26 +1726,6 @@ if test "$tor_cv_uint8_uchar" = "no"; then AC_MSG_ERROR([We assume that uint8_t is the same type as unsigned char, but your compiler disagrees.]) fi

-# Whether we should use the dmalloc memory allocation debugging library. -AC_MSG_CHECKING(whether to use dmalloc (debug memory allocation library)) -AC_ARG_WITH(dmalloc, -AS_HELP_STRING(--with-dmalloc, [use debug memory allocation library]), -[if [[ "$withval" = "yes" ]]; then - dmalloc=1 - AC_MSG_RESULT(yes) -else - dmalloc=1 - AC_MSG_RESULT(no) -fi], [ dmalloc=0; AC_MSG_RESULT(no) ] -) - -if [[ $dmalloc -eq 1 ]]; then - AC_CHECK_HEADERS(dmalloc.h, , AC_MSG_ERROR(dmalloc header file not found. Do you have the development files for dmalloc installed?)) - AC_SEARCH_LIBS(dmalloc_malloc, [dmallocth dmalloc], , AC_MSG_ERROR(Libdmalloc library not found. If you enable it you better have it installed.)) - AC_DEFINE(USE_DMALLOC, 1, [Debug memory allocation library]) - AC_CHECK_FUNCS(dmalloc_strdup dmalloc_strndup) -fi - AC_ARG_WITH(tcmalloc, AS_HELP_STRING(--with-tcmalloc, [use tcmalloc memory allocation library]), [ tcmalloc=yes ], [ tcmalloc=no ]) diff --git a/doc/HACKING/HelpfulTools.md b/doc/HACKING/HelpfulTools.md index a0795076e..eb068a91f 100644 --- a/doc/HACKING/HelpfulTools.md +++ b/doc/HACKING/HelpfulTools.md @@ -25,16 +25,6 @@ Jenkins

https://jenkins.torproject.org

-Dmalloc -------- - -The dmalloc library will keep track of memory allocation, so you can find out -if we're leaking memory, doing any double-frees, or so on. - - dmalloc -l -/dmalloc.log - (run the commands it tells you) - ./configure --with-dmalloc - Valgrind --------

diff --git a/src/common/crypto.c b/src/common/crypto.c index 14129086e..57eb9c64c 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -506,16 +506,3 @@ crypto_global_cleanup(void) }

/** @} */ - -#ifdef USE_DMALLOC -/** Tell the crypto library to use Tor's allocation functions rather than - * calling libc's allocation functions directly. Return 0 on success, -1 - * on failure. */ -int -crypto_use_tor_alloc_functions(void) -{ - int r = CRYPTO_set_mem_ex_functions(tor_malloc_, tor_realloc_, tor_free_); - return r ? 0 : -1; -} -#endif /* defined(USE_DMALLOC) */ - diff --git a/src/common/crypto.h b/src/common/crypto.h index 1a88a3d2f..e7d86eaf0 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -40,9 +40,6 @@ int crypto_early_init(void) ATTR_WUR; int crypto_global_init(int hardwareAccel, const char *accelName, const char *accelPath) ATTR_WUR; -#ifdef USE_DMALLOC -int crypto_use_tor_alloc_functions(void); -#endif

void crypto_thread_cleanup(void); int crypto_global_cleanup(void); @@ -77,4 +74,3 @@ int crypto_cipher_decrypt_with_iv(const char *key, void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in);

#endif /* !defined(TOR_CRYPTO_H) */ - diff --git a/src/common/util.c b/src/common/util.c index b3fdc8a43..5bf70c6b5 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -103,35 +103,13 @@ /* ===== * Memory management * ===== */ -#ifdef USE_DMALLOC - #undef strndup - #include <dmalloc.h> - /* Macro to pass the extra dmalloc args to another function. */ - #define DMALLOC_FN_ARGS , file, line - - #if defined(HAVE_DMALLOC_STRDUP) - /* the dmalloc_strdup should be fine as defined */ - #elif defined(HAVE_DMALLOC_STRNDUP) - #define dmalloc_strdup(file, line, string, xalloc_b) \ - dmalloc_strndup(file, line, (string), -1, xalloc_b) - #else - #error "No dmalloc_strdup or equivalent" -#endif /* defined(HAVE_DMALLOC_STRDUP) || ... */ - -#else /* !(defined(USE_DMALLOC)) */ - - #define DMALLOC_FN_ARGS -#endif /* defined(USE_DMALLOC) */

/** Allocate a chunk of <b>size</b> bytes of memory, and return a pointer to * result. On error, log and terminate the process. (Same as malloc(size), * but never returns NULL.) - * - * <b>file</b> and <b>line</b> are used if dmalloc is enabled, and - * ignored otherwise. */ void * -tor_malloc_(size_t size DMALLOC_PARAMS) +tor_malloc_(size_t size) { void *result;

@@ -144,11 +122,7 @@ tor_malloc_(size_t size DMALLOC_PARAMS) } #endif /* !defined(MALLOC_ZERO_WORKS) */

-#ifdef USE_DMALLOC - result = dmalloc_malloc(file, line, size, DMALLOC_FUNC_MALLOC, 0, 0); -#else result = raw_malloc(size); -#endif

if (PREDICT_UNLIKELY(result == NULL)) { /* LCOV_EXCL_START */ @@ -167,7 +141,7 @@ tor_malloc_(size_t size DMALLOC_PARAMS) * the process on error. (Same as calloc(size,1), but never returns NULL.) */ void * -tor_malloc_zero_(size_t size DMALLOC_PARAMS) +tor_malloc_zero_(size_t size) { /* You may ask yourself, "wouldn't it be smart to use calloc instead of * malloc+memset? Perhaps libc's calloc knows some nifty optimization trick @@ -175,7 +149,7 @@ tor_malloc_zero_(size_t size DMALLOC_PARAMS) * we're allocating something very big (it knows if it just got the memory * from the OS in a pre-zeroed state). We don't want to use tor_malloc_zero * for big stuff, so we don't bother with calloc. */ - void *result = tor_malloc_(size DMALLOC_FN_ARGS); + void *result = tor_malloc_(size); memset(result, 0, size); return result; } @@ -211,10 +185,10 @@ size_mul_check(const size_t x, const size_t y) * and a compile-time constant. */ void * -tor_calloc_(size_t nmemb, size_t size DMALLOC_PARAMS) +tor_calloc_(size_t nmemb, size_t size) { tor_assert(size_mul_check(nmemb, size)); - return tor_malloc_zero_((nmemb * size) DMALLOC_FN_ARGS); + return tor_malloc_zero_((nmemb * size)); }

/** Change the size of the memory block pointed to by <b>ptr</b> to <b>size</b> @@ -222,7 +196,7 @@ tor_calloc_(size_t nmemb, size_t size DMALLOC_PARAMS) * terminate. (Like realloc(ptr,size), but never returns NULL.) */ void * -tor_realloc_(void *ptr, size_t size DMALLOC_PARAMS) +tor_realloc_(void *ptr, size_t size) { void *result;

@@ -235,11 +209,7 @@ tor_realloc_(void *ptr, size_t size DMALLOC_PARAMS) } #endif /* !defined(MALLOC_ZERO_WORKS) */

-#ifdef USE_DMALLOC - result = dmalloc_realloc(file, line, ptr, size, DMALLOC_FUNC_REALLOC, 0); -#else result = raw_realloc(ptr, size); -#endif

if (PREDICT_UNLIKELY(result == NULL)) { /* LCOV_EXCL_START */ @@ -255,13 +225,13 @@ tor_realloc_(void *ptr, size_t size DMALLOC_PARAMS) * overflow. Unlike other allocation functions, return NULL on overflow. */ void * -tor_reallocarray_(void *ptr, size_t sz1, size_t sz2 DMALLOC_PARAMS) +tor_reallocarray_(void *ptr, size_t sz1, size_t sz2) { /* XXXX we can make this return 0, but we would need to check all the * reallocarray users. */ tor_assert(size_mul_check(sz1, sz2));

- return tor_realloc(ptr, (sz1 * sz2) DMALLOC_FN_ARGS); + return tor_realloc(ptr, (sz1 * sz2)); }

/** Return a newly allocated copy of the NUL-terminated string s. On @@ -269,16 +239,13 @@ tor_reallocarray_(void *ptr, size_t sz1, size_t sz2 DMALLOC_PARAMS) * NULL.) */ char * -tor_strdup_(const char *s DMALLOC_PARAMS) +tor_strdup_(const char *s) { char *duplicate; tor_assert(s);

-#ifdef USE_DMALLOC - duplicate = dmalloc_strdup(file, line, s, 0); -#else duplicate = raw_strdup(s); -#endif + if (PREDICT_UNLIKELY(duplicate == NULL)) { /* LCOV_EXCL_START */ log_err(LD_MM,"Out of memory on strdup(). Dying."); @@ -295,12 +262,12 @@ tor_strdup_(const char *s DMALLOC_PARAMS) * NULL.) */ char * -tor_strndup_(const char *s, size_t n DMALLOC_PARAMS) +tor_strndup_(const char *s, size_t n) { char *duplicate; tor_assert(s); tor_assert(n < SIZE_T_CEILING); - duplicate = tor_malloc_((n+1) DMALLOC_FN_ARGS); + duplicate = tor_malloc_((n+1)); /* Performance note: Ordinarily we prefer strlcpy to strncpy. But * this function gets called a whole lot, and platform strncpy is * much faster than strlcpy when strlen(s) is much longer than n. @@ -313,12 +280,12 @@ tor_strndup_(const char *s, size_t n DMALLOC_PARAMS) /** Allocate a chunk of <b>len</b> bytes, with the same contents as the * <b>len</b> bytes starting at <b>mem</b>. */ void * -tor_memdup_(const void *mem, size_t len DMALLOC_PARAMS) +tor_memdup_(const void *mem, size_t len) { char *duplicate; tor_assert(len < SIZE_T_CEILING); tor_assert(mem); - duplicate = tor_malloc_(len DMALLOC_FN_ARGS); + duplicate = tor_malloc_(len); memcpy(duplicate, mem, len); return duplicate; } @@ -326,12 +293,12 @@ tor_memdup_(const void *mem, size_t len DMALLOC_PARAMS) /** As tor_memdup(), but add an extra 0 byte at the end of the resulting * memory. */ void * -tor_memdup_nulterm_(const void *mem, size_t len DMALLOC_PARAMS) +tor_memdup_nulterm_(const void *mem, size_t len) { char *duplicate; tor_assert(len < SIZE_T_CEILING+1); tor_assert(mem); - duplicate = tor_malloc_(len+1 DMALLOC_FN_ARGS); + duplicate = tor_malloc_(len+1); memcpy(duplicate, mem, len); duplicate[len] = '\0'; return duplicate; @@ -365,13 +332,6 @@ tor_log_mallinfo(int severity) #else /* !(defined(HAVE_MALLINFO)) */ (void)severity; #endif /* defined(HAVE_MALLINFO) */ -#ifdef USE_DMALLOC - dmalloc_log_changed(0, /* Since the program started. */ - 1, /* Log info about non-freed pointers. */ - 0, /* Do not log info about freed pointers. */ - 0 /* Do not log individual pointers. */ - ); -#endif /* defined(USE_DMALLOC) */ } ENABLE_GCC_WARNING(aggregate-return)

@@ -5375,4 +5335,3 @@ tor_ntohll(uint64_t a) { return tor_htonll(a); } - diff --git a/src/common/util.h b/src/common/util.h index dcfa55f8f..7921357ac 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -34,45 +34,22 @@ #define O_NOFOLLOW 0 #endif

-/* If we're building with dmalloc, we want all of our memory allocation - * functions to take an extra file/line pair of arguments. If not, not. - * We define DMALLOC_PARAMS to the extra parameters to insert in the - * function prototypes, and DMALLOC_ARGS to the extra arguments to add - * to calls. */ -#ifdef USE_DMALLOC -#define DMALLOC_PARAMS , const char *file, const int line -#define DMALLOC_ARGS , SHORT_FILE__, __LINE__ -#else -#define DMALLOC_PARAMS -#define DMALLOC_ARGS -#endif /* defined(USE_DMALLOC) */ - /* Memory management */ -void *tor_malloc_(size_t size DMALLOC_PARAMS) ATTR_MALLOC; -void *tor_malloc_zero_(size_t size DMALLOC_PARAMS) ATTR_MALLOC; -void *tor_calloc_(size_t nmemb, size_t size DMALLOC_PARAMS) ATTR_MALLOC; -void *tor_realloc_(void *ptr, size_t size DMALLOC_PARAMS); -void *tor_reallocarray_(void *ptr, size_t size1, size_t size2 DMALLOC_PARAMS); -char *tor_strdup_(const char *s DMALLOC_PARAMS) ATTR_MALLOC ATTR_NONNULL((1)); -char *tor_strndup_(const char *s, size_t n DMALLOC_PARAMS) +void *tor_malloc_(size_t size) ATTR_MALLOC; +void *tor_malloc_zero_(size_t size) ATTR_MALLOC; +void *tor_calloc_(size_t nmemb, size_t size) ATTR_MALLOC; +void *tor_realloc_(void *ptr, size_t size); +void *tor_reallocarray_(void *ptr, size_t size1, size_t size2); +char *tor_strdup_(const char *s) ATTR_MALLOC ATTR_NONNULL((1)); +char *tor_strndup_(const char *s, size_t n) ATTR_MALLOC ATTR_NONNULL((1)); -void *tor_memdup_(const void *mem, size_t len DMALLOC_PARAMS) +void *tor_memdup_(const void *mem, size_t len) ATTR_MALLOC ATTR_NONNULL((1)); -void *tor_memdup_nulterm_(const void *mem, size_t len DMALLOC_PARAMS) +void *tor_memdup_nulterm_(const void *mem, size_t len) ATTR_MALLOC ATTR_NONNULL((1)); void tor_free_(void *mem); uint64_t tor_htonll(uint64_t a); uint64_t tor_ntohll(uint64_t a); -#ifdef USE_DMALLOC -extern int dmalloc_free(const char *file, const int line, void *pnt, - const int func_id); -#define tor_free(p) STMT_BEGIN \ - if (PREDICT_LIKELY((p)!=NULL)) { \ - dmalloc_free(SHORT_FILE__, __LINE__, (p), 0); \ - (p)=NULL; \ - } \ - STMT_END -#else /* !(defined(USE_DMALLOC)) */ /** Release memory allocated by tor_malloc, tor_realloc, tor_strdup, * etc. Unlike the free() function, the tor_free() macro sets the * pointer value to NULL after freeing it. @@ -97,18 +74,17 @@ extern int dmalloc_free(const char *file, const int line, void *pnt, (p)=NULL; \ STMT_END #endif -#endif /* defined(USE_DMALLOC) */

-#define tor_malloc(size) tor_malloc_(size DMALLOC_ARGS) -#define tor_malloc_zero(size) tor_malloc_zero_(size DMALLOC_ARGS) -#define tor_calloc(nmemb,size) tor_calloc_(nmemb, size DMALLOC_ARGS) -#define tor_realloc(ptr, size) tor_realloc_(ptr, size DMALLOC_ARGS) +#define tor_malloc(size) tor_malloc_(size) +#define tor_malloc_zero(size) tor_malloc_zero_(size) +#define tor_calloc(nmemb,size) tor_calloc_(nmemb, size) +#define tor_realloc(ptr, size) tor_realloc_(ptr, size) #define tor_reallocarray(ptr, sz1, sz2) \ - tor_reallocarray_((ptr), (sz1), (sz2) DMALLOC_ARGS) -#define tor_strdup(s) tor_strdup_(s DMALLOC_ARGS) -#define tor_strndup(s, n) tor_strndup_(s, n DMALLOC_ARGS) -#define tor_memdup(s, n) tor_memdup_(s, n DMALLOC_ARGS) -#define tor_memdup_nulterm(s, n) tor_memdup_nulterm_(s, n DMALLOC_ARGS) + tor_reallocarray_((ptr), (sz1), (sz2)) +#define tor_strdup(s) tor_strdup_(s) +#define tor_strndup(s, n) tor_strndup_(s, n) +#define tor_memdup(s, n) tor_memdup_(s, n) +#define tor_memdup_nulterm(s, n) tor_memdup_nulterm_(s, n)

/* Aliases for the underlying system malloc/realloc/free. Only use * them to indicate "I really want the underlying system function, I know @@ -569,4 +545,3 @@ int size_mul_check(const size_t x, const size_t y); #define ARRAY_LENGTH(x) ((sizeof(x)) / sizeof(x[0]))

#endif /* !defined(TOR_UTIL_H) */ - diff --git a/src/or/main.c b/src/or/main.c index a1d7a5e9a..55ff41fd9 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -110,9 +110,6 @@ #include "or/tor_api_internal.h" #include "common/util_process.h" #include "or/ext_orport.h" -#ifdef USE_DMALLOC -#include <dmalloc.h> -#endif #include "common/memarea.h" #include "common/sandbox.h"

@@ -2695,11 +2692,6 @@ do_hup(void) { const or_options_t *options = get_options();

-#ifdef USE_DMALLOC - dmalloc_log_stats(); - dmalloc_log_changed(0, 1, 0, 0); -#endif - log_notice(LD_GENERAL,"Received reload signal (hup). Reloading config and " "resetting internal state."); if (accounting_is_enabled(options)) @@ -3639,7 +3631,7 @@ release_lockfile(void) * only the parts of memory that we won't touch. If !<b>postfork</b>, * Tor is shutting down and we should free everything. * - * Helps us find the real leaks with dmalloc and the like. Also valgrind + * Helps us find the real leaks with sanitizers and the like. Also valgrind * should then report 0 reachable in its leak report (in an ideal world -- * in practice libevent, SSL, libc etc never quite free everything). */ void @@ -3795,18 +3787,11 @@ tor_cleanup(void)

timers_shutdown();

-#ifdef USE_DMALLOC - dmalloc_log_stats(); -#endif tor_free_all(0); /* We could move tor_free_all back into the ifdef below later, if it makes shutdown unacceptably slow. But for now, leave it here: it's helped us catch bugs in the past. */ crypto_global_cleanup(); -#ifdef USE_DMALLOC - dmalloc_log_unfreed(); - dmalloc_shutdown(); -#endif }

/** Read/create keys as needed, and echo our fingerprint to stdout. */ @@ -4237,14 +4222,6 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) tor_compress_init(); init_logging(0); monotime_init(); -#ifdef USE_DMALLOC - { - /* Instruct OpenSSL to use our internal wrappers for malloc, - realloc and free. */ - int r = crypto_use_tor_alloc_functions(); - tor_assert(r == 0); - } -#endif /* defined(USE_DMALLOC) */ #ifdef NT_SERVICE { int done = 0; @@ -4313,4 +4290,3 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) tor_cleanup(); return result; } - diff --git a/src/test/testing_common.c b/src/test/testing_common.c index 18b5b6106..8b83265d8 100644 --- a/src/test/testing_common.c +++ b/src/test/testing_common.c @@ -32,11 +32,6 @@ #include <dirent.h> #endif /* defined(_WIN32) */

-#ifdef USE_DMALLOC -#include <dmalloc.h> -#include "or/main.h" -#endif - /** Temporary directory (set up by setup_directory) under which we store all * our files during testing. */ static char temp_dir[256]; @@ -231,13 +226,6 @@ main(int c, const char **v) /* We must initialise logs before we call tor_assert() */ init_logging(1);

-#ifdef USE_DMALLOC - { - int r = crypto_use_tor_alloc_functions(); - tor_assert(r == 0); - } -#endif /* defined(USE_DMALLOC) */ - update_approx_time(time(NULL)); options = options_new(); tor_threads_init(); @@ -319,10 +307,7 @@ main(int c, const char **v) int have_failed = (tinytest_main(c, v, testgroups) != 0);

free_pregenerated_keys(); -#ifdef USE_DMALLOC - tor_free_all(0); - dmalloc_log_unfreed(); -#endif + crypto_global_cleanup();

if (have_failed) @@ -330,4 +315,3 @@ main(int c, const char **v) else return 0; } -