diff --git a/ext/drcovlib/modules.c b/ext/drcovlib/modules.c index df20199cc37..286277ae639 100644 --- a/ext/drcovlib/modules.c +++ b/ext/drcovlib/modules.c @@ -1,5 +1,5 @@ /* *************************************************************************** - * Copyright (c) 2012-2017 Google, Inc. All rights reserved. + * Copyright (c) 2012-2019 Google, Inc. All rights reserved. * ***************************************************************************/ /* @@ -595,7 +595,7 @@ drmodtrack_offline_read(file_t file, const char *map, OUT const char **next_line OUT void **handle, OUT uint *num_mods) { module_read_info_t *info = NULL; - uint i; + uint i, mods_parsed = 0; uint64 file_size; size_t map_size = 0; const char *buf, *map_start; @@ -644,6 +644,7 @@ drmodtrack_offline_read(file_t file, const char *map, OUT const char **next_line info->mod = (module_read_entry_t *)dr_global_alloc(*num_mods * sizeof(*info->mod)); /* module lists */ + mods_parsed = 0; for (i = 0; i < *num_mods; i++) { uint mod_id; if (version == 1) { @@ -692,6 +693,7 @@ drmodtrack_offline_read(file_t file, const char *map, OUT const char **next_line buf = module_parse_cb(buf, &info->mod[i].custom); if (buf == NULL) goto read_error; + ++mods_parsed; info->mod[i].path = info->mod[i].path_buf; if (dr_sscanf(buf, " %[^\n\r]", info->mod[i].path) != 1) goto read_error; @@ -706,6 +708,10 @@ drmodtrack_offline_read(file_t file, const char *map, OUT const char **next_line return DRCOVLIB_SUCCESS; read_error: + if (module_free_cb != NULL) { + for (i = 0; i < mods_parsed; i++) + module_free_cb(info->mod[i].custom); + } if (info != NULL) { dr_global_free(info->mod, *num_mods * sizeof(*info->mod)); dr_global_free(info, sizeof(*info)); diff --git a/suite/tests/client-interface/drmodtrack-test.dll.cpp b/suite/tests/client-interface/drmodtrack-test.dll.cpp index 199a05a760e..7f41686ce0a 100644 --- a/suite/tests/client-interface/drmodtrack-test.dll.cpp +++ b/suite/tests/client-interface/drmodtrack-test.dll.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2017 Google, Inc. All rights reserved. + * Copyright (c) 2017-2019 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -82,6 +82,63 @@ free_cb(void *data) /* Nothing. */ } +/* We do simple leak checking via a counter. We assume single-threaded code. */ +static int alloc_counter; + +static void * +my_alloc(void) +{ + ++alloc_counter; + return dr_global_alloc(sizeof(app_pc)); +} + +static void +my_free(void *ptr) +{ + CHECK(alloc_counter > 0, "double free"); + --alloc_counter; + dr_global_free(ptr, sizeof(app_pc)); +} + +static const char * +parse_alloc_cb(const char *src, OUT void **data) +{ + const char *res; + app_pc module_start; + if (dr_sscanf(src, PIFX ",", &module_start) != 1) + return NULL; + /* We store it on the heap to test leaks. */ + app_pc *ptr_to_start = (app_pc *)my_alloc(); + *ptr_to_start = module_start; + *data = ptr_to_start; + res = strchr(src, ','); + return (res == NULL) ? NULL : res + 1; +} + +static const char * +parse_alloc_error_cb(const char *src, OUT void **data) +{ + static int count_calls; + if (++count_calls == 4) + return NULL; /* fail to test parsing errors */ + const char *res; + app_pc module_start; + if (dr_sscanf(src, PIFX ",", &module_start) != 1) + return NULL; + /* We store it on the heap to test leaks. */ + app_pc *ptr_to_start = (app_pc *)my_alloc(); + *ptr_to_start = module_start; + *data = ptr_to_start; + res = strchr(src, ','); + return (res == NULL) ? NULL : res + 1; +} + +static void +free_alloc_cb(void *data) +{ + my_free(data); +} + static void event_exit(void) { @@ -165,16 +222,41 @@ event_exit(void) CHECK(wrote == strlen(buf_offline) + 1 /*null*/, "returned size off"); CHECK(strcmp(buf_online, buf_offline) == 0, "buffers do not match"); - dr_close_file(f); - ok = dr_delete_file(fname); - CHECK(ok, "failed to delete file"); - res = drmodtrack_offline_exit(modhandle); CHECK(res == DRCOVLIB_SUCCESS, "exit failed"); - + modhandle = NULL; dr_global_free(buf_online, size_online); dr_global_free(buf_offline, size_offline); + /* We do some more offline reads, to test leaks on parsing errors. */ + /* First ensure no leaks on successful parsing. */ + res = drmodtrack_add_custom_data(load_cb, print_cb, parse_alloc_cb, free_alloc_cb); + CHECK(res == DRCOVLIB_SUCCESS, "customization failed"); + void *modhandle2; + res = drmodtrack_offline_read(f, NULL, NULL, &modhandle2, &num_mods); + CHECK(res == DRCOVLIB_SUCCESS, "read failed"); + res = drmodtrack_offline_exit(modhandle2); + CHECK(res == DRCOVLIB_SUCCESS, "exit failed"); + modhandle2 = NULL; + CHECK(alloc_counter == 0, "memory leak"); + /* Now ensure no leaks on a parsing error. */ + res = drmodtrack_add_custom_data(load_cb, print_cb, parse_alloc_error_cb, + free_alloc_cb); + CHECK(res == DRCOVLIB_SUCCESS, "customization failed"); + void *modhandle3; + res = drmodtrack_offline_read(f, NULL, NULL, &modhandle3, &num_mods); + CHECK(res == DRCOVLIB_ERROR, "read should fail"); + modhandle3 = NULL; + CHECK(alloc_counter == 0, "memory leak"); + + /* Final cleanup. */ + dr_close_file(f); + ok = dr_delete_file(fname); + CHECK(ok, "failed to delete file"); + + /* We have to restore the old free_cb as it will be called on the live table. */ + res = drmodtrack_add_custom_data(load_cb, print_cb, parse_cb, free_cb); + CHECK(res == DRCOVLIB_SUCCESS, "customization failed"); res = drmodtrack_exit(); CHECK(res == DRCOVLIB_SUCCESS, "module exit failed"); }