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