From 667964d7befefff8309bc52b215f6bed3cc81e7d Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Sat, 25 Feb 2017 13:10:09 -0500 Subject: [PATCH 1/3] i#2006 generalize drcachesim: improve custom field support Adds a size-written OUT parameter to drmodtrack_offline_write() to avoid callers needing an expensive strlen() to figure out how much to write from the buffer to a file. Includes the checksum and timestamp in the returned offline fields now, to avoid having to check the struct size for compatibility when we inevitably add them later. --- clients/drcachesim/tests/burst_replace.cpp | 6 ++++-- ext/drcovlib/drcovlib.h | 14 +++++++++++--- ext/drcovlib/modules.c | 13 +++++++++++-- .../tests/client-interface/drmodtrack-test.dll.cpp | 4 +++- 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/clients/drcachesim/tests/burst_replace.cpp b/clients/drcachesim/tests/burst_replace.cpp index fba237c644f..d7e46585d1a 100644 --- a/clients/drcachesim/tests/burst_replace.cpp +++ b/clients/drcachesim/tests/burst_replace.cpp @@ -195,22 +195,24 @@ post_process() char *buf_offline; size_t size_offline = 8192; + size_t wrote; do { buf_offline = (char *)dr_global_alloc(size_offline); - res = drmodtrack_offline_write(modhandle, buf_offline, size_offline); + res = drmodtrack_offline_write(modhandle, buf_offline, size_offline, &wrote); if (res == DRCOVLIB_SUCCESS) break; dr_global_free(buf_offline, size_offline); size_offline *= 2; } while (res == DRCOVLIB_ERROR_BUF_TOO_SMALL); assert(res == DRCOVLIB_SUCCESS); + assert(wrote == strlen(buf_offline) + 1/*null*/); dr_close_file(f); drmodtrack_offline_exit(modhandle); /* Now replace the file */ f = dr_open_file(modlist_path, DR_FILE_WRITE_OVERWRITE); assert(f != INVALID_FILE); - dr_write_file(f, buf_offline, strlen(buf_offline)); + dr_write_file(f, buf_offline, wrote - 1/*don't need null in file*/); dr_close_file(f); } diff --git a/ext/drcovlib/drcovlib.h b/ext/drcovlib/drcovlib.h index 0ce44c71409..bde0302eef7 100644 --- a/ext/drcovlib/drcovlib.h +++ b/ext/drcovlib/drcovlib.h @@ -236,6 +236,12 @@ typedef struct _drmodtrack_info_t { * size #MAXIMUM_PATH. It can be modified. */ char *path; +#ifdef WINDOWS + /** The checksum field as stored in the module headers. */ + uint checksum; + /** The timestamp field as stored in the module headers. */ + uint timestamp; +#endif /** The custom field set by the \p load_cb passed to drmodtrack_add_custom_data(). */ void *custom; } drmodtrack_info_t; @@ -321,11 +327,13 @@ DR_EXPORT * Writes the module information that was read by drmodtrack_offline_read(), * and potentially modified by drmodtrack_offline_lookup(), to \p buf, whose * maximum size is specified in \p size. - * Returns DRCOVLIB_SUCCESS on success. - * If the buffer is too small, returns DRCOVLIB_ERROR_BUF_TOO_SMALL. + * Returns DRCOVLIB_SUCCESS on success and stores the number of bytes written to + * \p buf (including the terminating null) in \p wrote if \p wrote is not NULL. + * If the buffer is too small, returns DRCOVLIB_ERROR_BUF_TOO_SMALL (and does not + * set \p wrote). */ drcovlib_status_t -drmodtrack_offline_write(void *handle, char *buf, size_t size); +drmodtrack_offline_write(void *handle, char *buf, size_t buf_size, OUT size_t *wrote); DR_EXPORT /** diff --git a/ext/drcovlib/modules.c b/ext/drcovlib/modules.c index 3f6239b5b20..bb742ac27ff 100644 --- a/ext/drcovlib/modules.c +++ b/ext/drcovlib/modules.c @@ -557,6 +557,7 @@ move_to_next_line(const char *ptr) if (end == NULL) { ptr += strlen(ptr); } else { + //NOCHECKIN don't walk off end of mmap -- or require \0 at end, and put that in place ourselves for (ptr = end; *ptr == '\n' || *ptr == '\r'; ptr++) ; /* do nothing */ } @@ -703,17 +704,23 @@ drmodtrack_offline_lookup(void *handle, uint index, OUT drmodtrack_info_t *out) out->start = info->mod[index].base; out->size = (size_t)info->mod[index].size; out->path = info->mod[index].path; +#ifdef WINDOWS + out->checksum = info->mod[index].checksum; + out->timestamp = info->mod[index].timestamp; +#endif out->custom = info->mod[index].custom; return DRCOVLIB_SUCCESS; } drcovlib_status_t -drmodtrack_offline_write(void *handle, OUT char *buf, size_t size) +drmodtrack_offline_write(void *handle, OUT char *buf_start, size_t size, + OUT size_t *wrote) { int len; uint i; drcovlib_status_t res; module_read_info_t *info = (module_read_info_t *)handle; + char *buf = buf_start; if (info == NULL || buf == NULL) return DRCOVLIB_ERROR_INVALID_PARAMETER; res = drmodtrack_dump_buf_headers(buf, size, info->num_mods, &len); @@ -728,7 +735,9 @@ drmodtrack_offline_write(void *handle, OUT char *buf, size_t size) buf += len; size -= len; } - buf[size-1] = '\0'; + *buf = '\0'; + if (wrote != NULL) + *wrote = buf + 1/*null*/ - buf_start; return DRCOVLIB_SUCCESS; } diff --git a/suite/tests/client-interface/drmodtrack-test.dll.cpp b/suite/tests/client-interface/drmodtrack-test.dll.cpp index 17f896aa0b8..0548af8220f 100644 --- a/suite/tests/client-interface/drmodtrack-test.dll.cpp +++ b/suite/tests/client-interface/drmodtrack-test.dll.cpp @@ -133,9 +133,10 @@ event_exit(void) char *buf_offline; size_t size_offline = 8192; + size_t wrote; do { buf_offline = (char *)dr_global_alloc(size_offline); - res = drmodtrack_offline_write(modhandle, buf_offline, size_offline); + res = drmodtrack_offline_write(modhandle, buf_offline, size_offline, &wrote); if (res == DRCOVLIB_SUCCESS) break; dr_global_free(buf_offline, size_offline); @@ -143,6 +144,7 @@ event_exit(void) } while (res == DRCOVLIB_ERROR_BUF_TOO_SMALL); CHECK(res == DRCOVLIB_SUCCESS, "offline write failed"); CHECK(size_online == size_offline, "sizes do not match"); + 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); From 0bbff299f2c7c1cb33a1f12834f4b9e211146d87 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 27 Feb 2017 18:09:19 -0500 Subject: [PATCH 2/3] Separates the returned buffer end into a separate parameter for drmodtrack_offline_read() to avoid reading off the end for cases where the buffer is standalone. --- clients/drcachesim/tests/burst_replace.cpp | 2 +- clients/drcov/postprocess/drcov2lcov.cpp | 2 +- ext/drcovlib/drcovlib.h | 12 +++++++++--- ext/drcovlib/modules.c | 15 +++++++++------ .../client-interface/drmodtrack-test.dll.cpp | 2 +- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/clients/drcachesim/tests/burst_replace.cpp b/clients/drcachesim/tests/burst_replace.cpp index d7e46585d1a..452485d5b89 100644 --- a/clients/drcachesim/tests/burst_replace.cpp +++ b/clients/drcachesim/tests/burst_replace.cpp @@ -182,7 +182,7 @@ post_process() assert(f != INVALID_FILE); void *modhandle; uint num_mods; - res = drmodtrack_offline_read(f, NULL, &modhandle, &num_mods); + res = drmodtrack_offline_read(f, NULL, NULL, &modhandle, &num_mods); assert(res == DRCOVLIB_SUCCESS); for (uint i = 0; i < num_mods; ++i) { diff --git a/clients/drcov/postprocess/drcov2lcov.cpp b/clients/drcov/postprocess/drcov2lcov.cpp index 72536e678dc..0df86e8f1e1 100644 --- a/clients/drcov/postprocess/drcov2lcov.cpp +++ b/clients/drcov/postprocess/drcov2lcov.cpp @@ -759,7 +759,7 @@ read_module_list(const char *buf, module_table_t ***tables, uint *num_mods) PRINT(3, "Reading module table...\n"); /* module table header */ - if (drmodtrack_offline_read(INVALID_FILE, &buf, &handle, num_mods) != + if (drmodtrack_offline_read(INVALID_FILE, buf, &buf, &handle, num_mods) != DRCOVLIB_SUCCESS) { WARN(2, "Failed to read module table"); return NULL; diff --git a/ext/drcovlib/drcovlib.h b/ext/drcovlib/drcovlib.h index bde0302eef7..8fbe83e14f0 100644 --- a/ext/drcovlib/drcovlib.h +++ b/ext/drcovlib/drcovlib.h @@ -299,15 +299,21 @@ DR_EXPORT /** * Usable from standalone mode (hence the "offline" name). Reads a file written * by drmodtrack_dump(). If \p file is not INVALID_FILE, reads from \p file; - * otherwise, assumes the target file has been mapped into memory at \p *map and - * reads from there, returning in \p *map the end of the module list region. + * otherwise, assumes the target file has been mapped into memory at \p map and + * reads from there. If \p next_line is not NULL, this routine reads one + * character past the final newline of the final module in the list, and returns + * in \p *next_line a pointer to that character (this is intended for users who + * are embedding a module list inside a file with further data following the + * list in the file). If \p next_line is NULL, this routine stops reading at + * the final newline: thus, \p map need not be NULL-terminated. + * * Returns an identifier in \p handle to use with other offline routines, along * with the number of modules read in \p num_mods. Information on each module * can be obtained by calling drmodtrack_offline_lookup() and passing integers * from 0 to the number of modules minus one as the \p index. */ drcovlib_status_t -drmodtrack_offline_read(file_t file, const char **map, +drmodtrack_offline_read(file_t file, const char *map, OUT const char **next_line, OUT void **handle, OUT uint *num_mods); DR_EXPORT diff --git a/ext/drcovlib/modules.c b/ext/drcovlib/modules.c index bb742ac27ff..bb3936eab7b 100644 --- a/ext/drcovlib/modules.c +++ b/ext/drcovlib/modules.c @@ -557,7 +557,6 @@ move_to_next_line(const char *ptr) if (end == NULL) { ptr += strlen(ptr); } else { - //NOCHECKIN don't walk off end of mmap -- or require \0 at end, and put that in place ourselves for (ptr = end; *ptr == '\n' || *ptr == '\r'; ptr++) ; /* do nothing */ } @@ -581,7 +580,7 @@ skip_commas_and_spaces(const char *ptr, uint num_skip) } drcovlib_status_t -drmodtrack_offline_read(file_t file, const char **map, +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; @@ -596,8 +595,10 @@ drmodtrack_offline_read(file_t file, const char **map, if (file == INVALID_FILE) { if (map == NULL) return DRCOVLIB_ERROR_INVALID_PARAMETER; - map_start = *map; + map_start = map; } else { + if (next_line != NULL || map != NULL) + return DRCOVLIB_ERROR_INVALID_PARAMETER; if (!dr_file_size(file, &file_size)) return DRCOVLIB_ERROR_INVALID_PARAMETER; map_size = (size_t)file_size; @@ -676,10 +677,12 @@ drmodtrack_offline_read(file_t file, const char **map, if (dr_sscanf(buf, " %[^\n\r]", info->mod[i].path) != 1) goto read_error; } - buf = move_to_next_line(buf); + /* Avoid reading off the end, unless caller wants to advance to the next line. */ + if (i < *num_mods - 1 || next_line != NULL) + buf = move_to_next_line(buf); } - if (file == INVALID_FILE) - *map = buf; + if (file == INVALID_FILE && next_line != NULL) + *next_line = buf; *handle = (void *)info; return DRCOVLIB_SUCCESS; diff --git a/suite/tests/client-interface/drmodtrack-test.dll.cpp b/suite/tests/client-interface/drmodtrack-test.dll.cpp index 0548af8220f..119aba2c7c0 100644 --- a/suite/tests/client-interface/drmodtrack-test.dll.cpp +++ b/suite/tests/client-interface/drmodtrack-test.dll.cpp @@ -120,7 +120,7 @@ event_exit(void) void *modhandle; uint num_mods; f = dr_open_file(fname, DR_FILE_READ); - res = drmodtrack_offline_read(f, NULL, &modhandle, &num_mods); + res = drmodtrack_offline_read(f, NULL, NULL, &modhandle, &num_mods); CHECK(res == DRCOVLIB_SUCCESS, "read failed"); for (uint i = 0; i < num_mods; ++i) { From 697fa1e7884e487baf9c15222ce5a9e74850fbae Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 27 Feb 2017 18:49:28 -0500 Subject: [PATCH 3/3] Fix raw2trace --- clients/drcachesim/tracer/raw2trace.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index 255dc5f7c79..4025d1bcc58 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -109,7 +109,7 @@ raw2trace_t::read_and_map_modules(void) file_t modfile = dr_open_file(modfilename.c_str(), DR_FILE_READ); if (modfile == INVALID_FILE) FATAL_ERROR("Failed to open module file %s", modfilename.c_str()); - if (drmodtrack_offline_read(modfile, NULL, &modhandle, &num_mods) != + if (drmodtrack_offline_read(modfile, NULL, NULL, &modhandle, &num_mods) != DRCOVLIB_SUCCESS) FATAL_ERROR("Failed to parse module file %s", modfilename.c_str()); for (uint i = 0; i < num_mods; i++) {