From 2e3f8ca226df71a38089d36efc357dc5aec5ddcb Mon Sep 17 00:00:00 2001 From: Snehasish Kumar Date: Sat, 28 Apr 2018 22:23:41 -0400 Subject: [PATCH] Revert "Revert (#2940)" This reverts commit 48db566af975c96f984cc5060e78294ef99e0ce7 which reverted the changes in PR #2940. The changes pushed caused some apps to overflow the modidx field (issue #2956). PR #2969 increased the width of the modidx field. We can now safely revert the revert. --- api/docs/release.dox | 3 + core/lib/instrument.c | 1 + core/lib/instrument_api.h | 1 + core/unix/module.c | 4 +- core/unix/module.h | 4 +- core/unix/module_elf.c | 3 +- core/unix/module_macho.c | 3 +- ext/drcovlib/drcovlib.h | 4 + ext/drcovlib/modules.c | 96 +++++++++++-------- .../client-interface/drmodtrack-test.dll.cpp | 14 +++ 10 files changed, 90 insertions(+), 43 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index c8c28fc3c7b..643b0a30594 100644 --- a/api/docs/release.dox +++ b/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. **************************************************
diff --git a/core/lib/instrument.c b/core/lib/instrument.c index 94dd77cbdcd..9ccb59d06a5 100644 --- a/core/lib/instrument.c +++ b/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); diff --git a/core/lib/instrument_api.h b/core/lib/instrument_api.h index 4ce5ca70ee9..c5617e4fa65 100644 --- a/core/lib/instrument_api.h +++ b/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 diff --git a/core/unix/module.c b/core/unix/module.c index e4c635e436c..503c433ded9 100644 --- a/core/unix/module.c +++ b/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) diff --git a/core/unix/module.h b/core/unix/module.h index df00bdd1a2e..f71eb2dc3b7 100644 --- a/core/unix/module.h +++ b/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 diff --git a/core/unix/module_elf.c b/core/unix/module_elf.c index 2fbfca41788..275c0433bde 100644 --- a/core/unix/module_elf.c +++ b/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; } diff --git a/core/unix/module_macho.c b/core/unix/module_macho.c index 17f21a5f845..7d2e9b16f23 100644 --- a/core/unix/module_macho.c +++ b/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 */ diff --git a/ext/drcovlib/drcovlib.h b/ext/drcovlib/drcovlib.h index b320ce2215c..5b583b7b1de 100644 --- a/ext/drcovlib/drcovlib.h +++ b/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 diff --git a/ext/drcovlib/modules.c b/ext/drcovlib/modules.c index 2034a1cfb28..fd73d97ffce 100644 --- a/ext/drcovlib/modules.c +++ b/ext/drcovlib/modules.c @@ -38,7 +38,7 @@ #include #include -#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; } diff --git a/suite/tests/client-interface/drmodtrack-test.dll.cpp b/suite/tests/client-interface/drmodtrack-test.dll.cpp index b69ba308852..7729da0e061 100644 --- a/suite/tests/client-interface/drmodtrack-test.dll.cpp +++ b/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;