Revert "Revert (DynamoRIO#2940)"

This reverts commit 48db566 which
reverted the changes in PR DynamoRIO#2940. The changes pushed caused some apps to
overflow the modidx field (issue DynamoRIO#2956). PR DynamoRIO#2969 increased the width of
the modidx field. We can now safely revert the revert.

api/docs/release.dox

@@ -231,6 +231,9 @@ Further non-compatibility-affecting changes include:
    and drreg_is_instr_spill_or_restore().
  - Added dr_app_stop_and_cleanup_with_stats() to obtain stats values right before
    cleanup.
+ - drmodtrack now allocates an entry per segment for each loaded module.
+   Added a file offset field to module_segment_data_t for UNIX platforms.
+   drcachesim saves file offset information in modules.log on UNIX platforms.
 
 **************************************************
 <hr>

core/lib/instrument.c

@@ -1813,6 +1813,7 @@ create_and_initialize_module_data(app_pc start, app_pc end, app_pc entry_point,
             copy->segments[i].start = os_segments[i].start;
             copy->segments[i].end = os_segments[i].end;
             copy->segments[i].prot = os_segments[i].prot;
+            copy->segments[i].offset = os_segments[i].offset;
         }
     } else {
         ASSERT(segments != NULL);

core/lib/instrument_api.h

@@ -2918,6 +2918,7 @@ typedef struct _module_segment_data_t {
     app_pc start; /**< Start address of the segment, page-aligned backward. */
     app_pc end;   /**< End address of the segment, page-aligned forward. */
     uint prot;    /**< Protection attributes of the segment */
+    uint64 offset; /**< Offset of the segment from the beginning of the backing file */
 } module_segment_data_t;
 #endif
 

core/unix/module.c

@@ -470,7 +470,8 @@ module_add_segment_data(OUT os_module_data_t *out_data,
                         size_t segment_size,
                         uint segment_prot, /* MEMPROT_ */
                         size_t alignment,
-                        bool shared)
+                        bool shared,
+                        uint64 offset)
 {
     uint seg, i;
     if (out_data->alignment == 0) {
@@ -522,6 +523,7 @@ module_add_segment_data(OUT os_module_data_t *out_data,
         ALIGN_FORWARD(segment_start + segment_size, PAGE_SIZE);
     out_data->segments[seg].prot = segment_prot;
     out_data->segments[seg].shared = shared;
+    out_data->segments[seg].offset = offset;
     if (seg > 0) {
         ASSERT(out_data->segments[seg].start >= out_data->segments[seg - 1].end);
         if (out_data->segments[seg].start > out_data->segments[seg - 1].end)

core/unix/module.h

@@ -48,6 +48,7 @@ typedef struct _module_segment_t {
     app_pc end;
     uint prot;
     bool shared; /* not unique to this module */
+    uint64 offset;
 } module_segment_t;
 
 typedef struct _os_module_data_t {
@@ -151,7 +152,8 @@ module_add_segment_data(OUT os_module_data_t *out_data,
                         size_t segment_size,
                         uint segment_prot,
                         size_t alignment,
-                        bool shared);
+                        bool shared,
+                        uint64 offset);
 
 /* Redirected functions for loaded module,
  * they are also used by __wrap_* functions in instrument.c

core/unix/module_elf.c

@@ -539,7 +539,8 @@ module_walk_program_headers(app_pc base, size_t view_size, bool at_map, bool dyn
                                             (app_pc) prog_hdr->p_vaddr + load_delta,
                                             prog_hdr->p_memsz,
                                             module_segment_prot_to_osprot(prog_hdr),
-                                            prog_hdr->p_align, false/*!shared*/);
+                                            prog_hdr->p_align, false/*!shared*/,
+                                            prog_hdr->p_offset);
                 }
                 found_load = true;
             }

core/unix/module_macho.c

@@ -288,7 +288,8 @@ module_walk_program_headers(app_pc base, size_t view_size, bool at_map, bool dyn
                               * ignoring for now
                               */
                              PAGE_SIZE,
-                             shared);
+                             shared,
+                             seg->fileoff);
                     }
                 } else if (cmd->cmd == LC_SYMTAB) {
                     /* even if stripped, dynamic symbols are in this table */

ext/drcovlib/drcovlib.h

@@ -249,6 +249,10 @@ typedef struct _drmodtrack_info_t {
      * passed to drmodtrack_offline_lookup().
      */
     uint index;
+    /**
+     * The offset of this segment from the beginning of this backing file.
+     */
+    uint64 offset;
 } drmodtrack_info_t;
 
 DR_EXPORT

ext/drcovlib/modules.c

@@ -38,7 +38,7 @@
 #include <stdio.h>
 #include <stddef.h>
 
-#define MODULE_FILE_VERSION 3
+#define MODULE_FILE_VERSION 4
 
 #define NUM_GLOBAL_MODULE_CACHE 8
 #define NUM_THREAD_MODULE_CACHE 4
@@ -55,6 +55,10 @@ typedef struct _module_entry_t {
      */
     module_data_t *data;
     void *custom;
+#ifndef WINDOWS
+    /* The file offset of the segment */
+    uint64 offset;
+#endif
 } module_entry_t;
 
 typedef struct _module_table_t {
@@ -195,29 +199,27 @@ event_module_load(void *drcontext, const module_data_t *data, bool loaded)
             entry->custom = module_load_cb(entry->data);
         drvector_append(&module_table.vector, entry);
 #ifndef WINDOWS
-        if (!data->contiguous) {
-            uint j;
-            module_entry_t *sub_entry;
-            ASSERT(entry->start == data->segments[0].start, "illegal segments");
-            entry->end = data->segments[0].end;
-            for (j = 1/*we did 1st*/; j < data->num_segments; j++) {
-                if (data->segments[j].start == data->segments[j-1].end)
-                    continue; /* contiguous */
-                /* Add an entry for each separate piece.  On unload we assume
-                 * that these entries are consecutive following the main entry.
-                 */
-                sub_entry = dr_global_alloc(sizeof(*sub_entry));
-                sub_entry->id = module_table.vector.entries;
-                sub_entry->containing_id = entry->id;
-                sub_entry->start = data->segments[j].start;
-                sub_entry->end = data->segments[j].end;
-                sub_entry->unload = false;
-                /* These fields are shared. */
-                sub_entry->data = entry->data;
-                sub_entry->custom = entry->custom;
-                drvector_append(&module_table.vector, sub_entry);
-                global_module_cache_add(module_table.cache, sub_entry);
-            }
+        entry->offset = data->segments[0].offset;
+        uint j;
+        module_entry_t *sub_entry;
+        ASSERT(entry->start == data->segments[0].start, "illegal segments");
+        entry->end = data->segments[0].end;
+        for (j = 1/*we did 1st*/; j < data->num_segments; j++) {
+            /* Add an entry for each separate piece.  On unload we assume
+             * that these entries are consecutive following the main entry.
+             */
+            sub_entry = dr_global_alloc(sizeof(*sub_entry));
+            sub_entry->id = module_table.vector.entries;
+            sub_entry->containing_id = entry->id;
+            sub_entry->start = data->segments[j].start;
+            sub_entry->end = data->segments[j].end;
+            sub_entry->unload = false;
+            /* These fields are shared. */
+            sub_entry->data = entry->data;
+            sub_entry->custom = entry->custom;
+            sub_entry->offset = data->segments[j].offset;
+            drvector_append(&module_table.vector, sub_entry);
+            global_module_cache_add(module_table.cache, sub_entry);
         }
 #endif
     }
@@ -312,17 +314,15 @@ event_module_unload(void *drcontext, const module_data_t *data)
     if (entry != NULL) {
         entry->unload = true;
 #ifndef WINDOWS
-        if (!data->contiguous) {
-            int j;
-            /* Find non-contiguous entries, which are consecutive and after the main. */
-            for (j = i + 1; j < module_table.vector.entries; j++) {
-                module_entry_t *sub_entry = drvector_get_entry(&module_table.vector, j);
-                ASSERT(sub_entry != NULL, "fail to get module entry");
-                if (sub_entry->containing_id == entry->id)
-                    sub_entry->unload = true;
-                else
-                    break;
-            }
+        int j;
+        /* Find non-contiguous entries, which are consecutive and after the main. */
+        for (j = i + 1; j < module_table.vector.entries; j++) {
+            module_entry_t *sub_entry = drvector_get_entry(&module_table.vector, j);
+            ASSERT(sub_entry != NULL, "fail to get module entry");
+            if (sub_entry->containing_id == entry->id)
+                sub_entry->unload = true;
+            else
+                break;
         }
 #endif
     } else
@@ -399,6 +399,7 @@ typedef struct _module_read_entry_t {
     char *path; /* may or may not point to path_buf */
     char path_buf[MAXIMUM_PATH];
     void *custom;
+    uint64 offset;
 } module_read_entry_t;
 
 typedef struct _module_read_info_t {
@@ -413,9 +414,10 @@ static int
 module_read_entry_print(module_read_entry_t *entry, uint idx, char *buf, size_t size)
 {
     int len, total_len = 0;
-    len = dr_snprintf(buf, size, "%3u, %3u, " PFX ", " PFX ", " PFX ", ",
+    len = dr_snprintf(buf, size, "%3u, %3u, " PFX ", " PFX ", " PFX ", "
+                      ZHEX64_FORMAT_STRING ", ",
                       idx, entry->containing_id, entry->base,
-                      entry->base+entry->size, entry->entry);
+                      entry->base+entry->size, entry->entry, entry->offset);
     if (len == -1)
         return -1;
     buf += len;
@@ -466,6 +468,10 @@ module_table_entry_print(module_entry_t *entry, char *buf, size_t size)
 #endif
     read_entry.path = full_path;
     read_entry.custom = entry->custom;
+#ifndef WINDOWS
+    // For unices we record the physical offset from the backing file.
+    read_entry.offset = entry->offset;
+#endif
     return module_read_entry_print(&read_entry, entry->id, buf, size);
 }
 
@@ -484,7 +490,7 @@ drmodtrack_dump_buf_headers(char *buf_in, size_t size, uint count, OUT int *len_
     buf += len;
     size -= len;
 
-    len = dr_snprintf(buf, size, "Columns: id, containing_id, start, end, entry");
+    len = dr_snprintf(buf, size, "Columns: id, containing_id, start, end, entry, offset");
     if (len == -1)
         return DRCOVLIB_ERROR_BUF_TOO_SMALL;
     buf += len;
@@ -663,13 +669,23 @@ drmodtrack_offline_read(file_t file, const char *map, OUT const char **next_line
                     goto read_error;
                 info->mod[i].containing_id = mod_id;
                 buf = skip_commas_and_spaces(buf, 4);
-            } else {
+            } else
+            if (version == 3) {
                 if (dr_sscanf(buf, "%u, %u, "PIFX", "PIFX", "PIFX", ",
                               &mod_id, &info->mod[i].containing_id,
                               &info->mod[i].base, &end, &info->mod[i].entry) != 5 ||
                     mod_id != i)
                     goto read_error;
                 buf = skip_commas_and_spaces(buf, 5);
+            } else { // version == 4
+                if (dr_sscanf(buf, "%u, %u, "PIFX", "PIFX", "
+                              PIFX", "HEX64_FORMAT_STRING", ",
+                              &mod_id, &info->mod[i].containing_id,
+                              &info->mod[i].base, &end, &info->mod[i].entry,
+                              &info->mod[i].offset) != 6 ||
+                    mod_id != i)
+                    goto read_error;
+                buf = skip_commas_and_spaces(buf, 6);
             }
             if (buf == NULL)
                 goto read_error;
@@ -727,6 +743,8 @@ drmodtrack_offline_lookup(void *handle, uint index, OUT drmodtrack_info_t *out)
     out->custom = info->mod[index].custom;
     if (out->struct_size > offsetof(drmodtrack_info_t, index))
         out->index = index;
+    if (out->struct_size > offsetof(drmodtrack_info_t, offset))
+        out->offset = info->mod[index].offset;
     return DRCOVLIB_SUCCESS;
 }
 

suite/tests/client-interface/drmodtrack-test.dll.cpp

@@ -38,6 +38,7 @@
 #include "drx.h"
 #include "client_tools.h"
 #include "string.h"
+#include "stddef.h"
 
 #ifdef WINDOWS
 # pragma warning( disable : 4100) /* unreferenced formal parameter */
@@ -132,6 +133,19 @@ event_exit(void)
         CHECK(((app_pc)info.custom) == info.start ||
               info.containing_index != i, "custom field doesn't match");
         CHECK(info.index == i, "index field doesn't match");
+#ifndef WINDOWS
+        if (info.struct_size > offsetof(drmodtrack_info_t, offset)) {
+            module_data_t * data = dr_lookup_module(info.start);
+            for (uint j = 0; j < data->num_segments; j++) {
+                module_segment_data_t *seg = data->segments + j;
+                if (seg->start == info.start) {
+                    CHECK(seg->offset == info.offset,
+                        "Module data offset and drmodtrack offset don't match");
+                }
+            }
+            dr_free_module_data(data);
+        }
+#endif
     }
 
     char *buf_offline;