i#2006 generalize drcachesim: improve custom field support (#2244)

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. 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. 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.
DynamoRIO · Feb 28, 2017 · 370a9b2 · 370a9b2
1 parent dc18f91
commit 370a9b2
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 20 deletions.
diff --git 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) {
@@ -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/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++) {

diff --git 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
@@ -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;
@@ -293,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
@@ -321,11 +333,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
@@ -580,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;
@@ -595,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;
@@ -675,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;
 
@@ -703,17 +707,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 +738,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
@@ -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) {
@@ -133,16 +133,18 @@ 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);
         size_offline *= 2;
     } 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);