[PATCH wayland 6/6] tests: Remove memory leak checking infrastructure

Peter Hutterer peter.hutterer at who-t.net
Wed Aug 29 06:49:47 UTC 2018


On Wed, Aug 29, 2018 at 07:17:15AM +0100, Daniel Stone wrote:
> There are far better ways to detect memory leaks, such as either
> valgrind or ASan. Having Meson makes it really easy to use these tools
> in our tests, and we can do that in CI as well.
> 
> Having these local wrappers actually completely broke ASan usage, so
> remove them in favour of using the more powerful options.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>

series Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Cheers,
   Peter

> ---
>  tests/sanity-test.c     | 66 ++--------------------------------
>  tests/test-compositor.c |  5 ++-
>  tests/test-runner.c     | 79 ++++++-----------------------------------
>  tests/test-runner.h     | 13 +++----
>  4 files changed, 20 insertions(+), 143 deletions(-)
> 
> diff --git a/tests/sanity-test.c b/tests/sanity-test.c
> index 2495a115..98beca8d 100644
> --- a/tests/sanity-test.c
> +++ b/tests/sanity-test.c
> @@ -35,7 +35,7 @@
>  
>  #include "test-compositor.h"
>  
> -extern int leak_check_enabled;
> +extern int fd_leak_check_enabled;
>  
>  TEST(empty)
>  {
> @@ -83,71 +83,11 @@ FAIL_TEST(sanity_assert)
>  	assert(0);
>  }
>  
> -FAIL_TEST(sanity_malloc_direct)
> -{
> -	void *p;
> -
> -	assert(leak_check_enabled);
> -
> -	p = malloc(10);	/* memory leak */
> -	assert(p);	/* assert that we got memory, also prevents
> -			 * the malloc from getting optimized away. */
> -	free(NULL);	/* NULL must not be counted */
> -	test_disable_coredumps();
> -}
> -
> -TEST(disable_leak_checks)
> -{
> -	volatile void *mem;
> -	assert(leak_check_enabled);
> -	/* normally this should be on the beginning of the test.
> -	 * Here we need to be sure, that the leak checks are
> -	 * turned on */
> -	DISABLE_LEAK_CHECKS;
> -
> -	mem = malloc(16);
> -	assert(mem);
> -}
> -
> -FAIL_TEST(sanity_malloc_indirect)
> -{
> -	struct wl_array array;
> -
> -	assert(leak_check_enabled);
> -
> -	wl_array_init(&array);
> -
> -	/* call into library that calls malloc */
> -	wl_array_add(&array, 14);
> -
> -	/* not freeing array, must leak */
> -
> -	test_disable_coredumps();
> -}
> -
> -FAIL_TEST(tc_client_memory_leaks)
> -{
> -	struct display *d = display_create();
> -	client_create_noarg(d, sanity_malloc_direct);
> -	display_run(d);
> -	test_disable_coredumps();
> -	display_destroy(d);
> -}
> -
> -FAIL_TEST(tc_client_memory_leaks2)
> -{
> -	struct display *d = display_create();
> -	client_create_noarg(d, sanity_malloc_indirect);
> -	display_run(d);
> -	test_disable_coredumps();
> -	display_destroy(d);
> -}
> -
>  FAIL_TEST(sanity_fd_leak)
>  {
>  	int fd[2];
>  
> -	assert(leak_check_enabled);
> +	assert(fd_leak_check_enabled);
>  
>  	/* leak 2 file descriptors */
>  	if (pipe(fd) < 0)
> @@ -185,7 +125,7 @@ sanity_fd_no_leak(void)
>  {
>  	int fd[2];
>  
> -	assert(leak_check_enabled);
> +	assert(fd_leak_check_enabled);
>  
>  	/* leak 2 file descriptors */
>  	if (pipe(fd) < 0)
> diff --git a/tests/test-compositor.c b/tests/test-compositor.c
> index 0631f614..72f63515 100644
> --- a/tests/test-compositor.c
> +++ b/tests/test-compositor.c
> @@ -156,7 +156,7 @@ run_client(void (*client_main)(void *data), void *data,
>  	   int wayland_sock, int client_pipe)
>  {
>  	char s[8];
> -	int cur_alloc, cur_fds;
> +	int cur_fds;
>  	int can_continue = 0;
>  
>  	/* Wait until display signals that client can continue */
> @@ -169,7 +169,6 @@ run_client(void (*client_main)(void *data), void *data,
>  	snprintf(s, sizeof s, "%d", wayland_sock);
>  	setenv("WAYLAND_SOCKET", s, 0);
>  
> -	cur_alloc = get_current_alloc_num();
>  	cur_fds = count_open_fds();
>  
>  	client_main(data);
> @@ -182,7 +181,7 @@ run_client(void (*client_main)(void *data), void *data,
>  	if (!getenv("WAYLAND_SOCKET"))
>  		cur_fds--;
>  
> -	check_leaks(cur_alloc, cur_fds);
> +	check_fd_leaks(cur_fds);
>  }
>  
>  static struct client_info *
> diff --git a/tests/test-runner.c b/tests/test-runner.c
> index 82a0a7b8..1487dc48 100644
> --- a/tests/test-runner.c
> +++ b/tests/test-runner.c
> @@ -44,16 +44,10 @@
>  
>  #include "test-runner.h"
>  
> -static int num_alloc;
> -static void* (*sys_malloc)(size_t);
> -static void (*sys_free)(void*);
> -static void* (*sys_realloc)(void*, size_t);
> -static void* (*sys_calloc)(size_t, size_t);
> -
> -/* when set to 1, check if tests are not leaking memory and opened files.
> +/* when set to 1, check if tests are not leaking opened files.
>   * It is turned on by default. It can be turned off by
>   * WAYLAND_TEST_NO_LEAK_CHECK environment variable. */
> -int leak_check_enabled;
> +int fd_leak_check_enabled;
>  
>  /* when this var is set to 0, every call to test_set_timeout() is
>   * suppressed - handy when debugging the test. Can be set by
> @@ -65,40 +59,6 @@ static int is_atty = 0;
>  
>  extern const struct test __start_test_section, __stop_test_section;
>  
> -__attribute__ ((visibility("default"))) void *
> -malloc(size_t size)
> -{
> -	num_alloc++;
> -	return sys_malloc(size);
> -}
> -
> -__attribute__ ((visibility("default"))) void
> -free(void* mem)
> -{
> -	if (mem != NULL)
> -		num_alloc--;
> -	sys_free(mem);
> -}
> -
> -__attribute__ ((visibility("default"))) void *
> -realloc(void* mem, size_t size)
> -{
> -	if (mem == NULL)
> -		num_alloc++;
> -	return sys_realloc(mem, size);
> -}
> -
> -__attribute__ ((visibility("default"))) void *
> -calloc(size_t nmemb, size_t size)
> -{
> -	if (sys_calloc == NULL)
> -		return NULL;
> -
> -	num_alloc++;
> -
> -	return sys_calloc(nmemb, size);
> -}
> -
>  static const struct test *
>  find_test(const char *name)
>  {
> @@ -156,25 +116,12 @@ sigalrm_handler(int signum)
>  	abort();
>  }
>  
> -int
> -get_current_alloc_num(void)
> -{
> -	return num_alloc;
> -}
> -
>  void
> -check_leaks(int supposed_alloc, int supposed_fds)
> +check_fd_leaks(int supposed_fds)
>  {
>  	int num_fds;
>  
> -	if (leak_check_enabled) {
> -		if (supposed_alloc != num_alloc) {
> -			fprintf(stderr, "Memory leak detected in test. "
> -				"Allocated %d blocks, unfreed %d\n", num_alloc,
> -				num_alloc - supposed_alloc);
> -			abort();
> -		}
> -
> +	if (fd_leak_check_enabled) {
>  		num_fds = count_open_fds();
>  		if (supposed_fds != num_fds) {
>  			fprintf(stderr, "fd leak detected in test. "
> @@ -183,14 +130,14 @@ check_leaks(int supposed_alloc, int supposed_fds)
>  			abort();
>  		}
>  	} else {
> -		fprintf(stderr, "Leak checks disabled\n");
> +		fprintf(stderr, "FD leak checks disabled\n");
>  	}
>  }
>  
>  static void
>  run_test(const struct test *t)
>  {
> -	int cur_alloc, cur_fds;
> +	int cur_fds;
>  	struct sigaction sa;
>  
>  	if (timeouts_enabled) {
> @@ -200,7 +147,7 @@ run_test(const struct test *t)
>  		assert(sigaction(SIGALRM, &sa, NULL) == 0);
>  	}
>  
> -	cur_alloc = get_current_alloc_num();
> +	//cur_alloc = get_current_alloc_num();
>  	cur_fds = count_open_fds();
>  
>  	t->run();
> @@ -209,7 +156,7 @@ run_test(const struct test *t)
>  	if (timeouts_enabled)
>  		alarm(0);
>  
> -	check_leaks(cur_alloc, cur_fds);
> +	check_fd_leaks(cur_fds);
>  
>  	exit(EXIT_SUCCESS);
>  }
> @@ -348,20 +295,14 @@ int main(int argc, char *argv[])
>  	int total, pass;
>  	siginfo_t info;
>  
> -	/* Load system malloc, free, and realloc */
> -	sys_calloc = dlsym(RTLD_NEXT, "calloc");
> -	sys_realloc = dlsym(RTLD_NEXT, "realloc");
> -	sys_malloc = dlsym(RTLD_NEXT, "malloc");
> -	sys_free = dlsym(RTLD_NEXT, "free");
> -
>  	if (isatty(fileno(stderr)))
>  		is_atty = 1;
>  
>  	if (is_debugger_attached()) {
> -		leak_check_enabled = 0;
> +		fd_leak_check_enabled = 0;
>  		timeouts_enabled = 0;
>  	} else {
> -		leak_check_enabled = !getenv("WAYLAND_TEST_NO_LEAK_CHECK");
> +		fd_leak_check_enabled = !getenv("WAYLAND_TEST_NO_LEAK_CHECK");
>  		timeouts_enabled = !getenv("WAYLAND_TEST_NO_TIMEOUTS");
>  	}
>  
> diff --git a/tests/test-runner.h b/tests/test-runner.h
> index 9c47a2b0..d0734009 100644
> --- a/tests/test-runner.h
> +++ b/tests/test-runner.h
> @@ -63,11 +63,8 @@ count_open_fds(void);
>  void
>  exec_fd_leak_check(int nr_expected_fds); /* never returns */
>  
> -int
> -get_current_alloc_num(void);
> -
>  void
> -check_leaks(int supposed_allocs, int supposed_fds);
> +check_fd_leaks(int supposed_fds);
>  
>  /*
>   * set/reset the timeout in seconds. The timeout starts
> @@ -89,10 +86,10 @@ test_sleep(unsigned int);
>  void
>  test_disable_coredumps(void);
>  
> -#define DISABLE_LEAK_CHECKS			\
> -	do {					\
> -		extern int leak_check_enabled;	\
> -		leak_check_enabled = 0;		\
> +#define DISABLE_LEAK_CHECKS				\
> +	do {						\
> +		extern int fd_leak_check_enabled;	\
> +		fd_leak_check_enabled = 0;		\
>  	} while (0);
>  
>  #endif
> -- 
> 2.17.1
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 


More information about the wayland-devel mailing list