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

Ray, Ian (GE Healthcare) ian.ray at ge.com
Wed Aug 29 06:45:09 UTC 2018



> On 29 Aug 2018, at 9.17, Daniel Stone <daniels at collabora.com> 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>
> ---
> 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();

Nit: the above line could/should have been deleted.

Reviewed-by: Ian Ray <ian.ray at ge.com>


> 	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