From 48db566af975c96f984cc5060e78294ef99e0ce7 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Wed, 25 Apr 2018 15:01:10 -0400 Subject: [PATCH] Revert "i#2006 generalize drcachesim: Split segments and record segment offsets (#2940)" (#2963) This reverts commit 8c90e2dc81eb9b9dfdcf2313bdf91b02f12c4de4 as it pushes the module count for drcachesim offline traces beyond the limit in the module index bitfield (#2956) for some apps. Issue: #2006, #2939, #2956. --- 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, 43 insertions(+), 90 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index 0992fb7e3eb..ef15dafe23b 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -229,9 +229,6 @@ Further non-compatibility-affecting changes include: of built basic blocks. - Added drreg_reservation_info_ex(), drreg_statelessly_restore_app_value(), and drreg_is_instr_spill_or_restore(). - - 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 36ccb714782..57731241e3a 100644 --- a/core/lib/instrument.c +++ b/core/lib/instrument.c @@ -1813,7 +1813,6 @@ 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 a4b53e34bec..f3b33936cbf 100644 --- a/core/lib/instrument_api.h +++ b/core/lib/instrument_api.h @@ -2918,7 +2918,6 @@ 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 503c433ded9..e4c635e436c 100644 --- a/core/unix/module.c +++ b/core/unix/module.c @@ -470,8 +470,7 @@ module_add_segment_data(OUT os_module_data_t *out_data, size_t segment_size, uint segment_prot, /* MEMPROT_ */ size_t alignment, - bool shared, - uint64 offset) + bool shared) { uint seg, i; if (out_data->alignment == 0) { @@ -523,7 +522,6 @@ 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 f71eb2dc3b7..df00bdd1a2e 100644 --- a/core/unix/module.h +++ b/core/unix/module.h @@ -48,7 +48,6 @@ 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 { @@ -152,8 +151,7 @@ module_add_segment_data(OUT os_module_data_t *out_data, size_t segment_size, uint segment_prot, size_t alignment, - bool shared, - uint64 offset); + bool shared); /* 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 275c0433bde..2fbfca41788 100644 --- a/core/unix/module_elf.c +++ b/core/unix/module_elf.c @@ -539,8 +539,7 @@ 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_offset); + prog_hdr->p_align, false/*!shared*/); } found_load = true; } diff --git a/core/unix/module_macho.c b/core/unix/module_macho.c index 7d2e9b16f23..17f21a5f845 100644 --- a/core/unix/module_macho.c +++ b/core/unix/module_macho.c @@ -288,8 +288,7 @@ module_walk_program_headers(app_pc base, size_t view_size, bool at_map, bool dyn * ignoring for now */ PAGE_SIZE, - shared, - seg->fileoff); + shared); } } 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 5b583b7b1de..b320ce2215c 100644 --- a/ext/drcovlib/drcovlib.h +++ b/ext/drcovlib/drcovlib.h @@ -249,10 +249,6 @@ 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 fd73d97ffce..2034a1cfb28 100644 --- a/ext/drcovlib/modules.c +++ b/ext/drcovlib/modules.c @@ -38,7 +38,7 @@ #include #include -#define MODULE_FILE_VERSION 4 +#define MODULE_FILE_VERSION 3 #define NUM_GLOBAL_MODULE_CACHE 8 #define NUM_THREAD_MODULE_CACHE 4 @@ -55,10 +55,6 @@ 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 { @@ -199,27 +195,29 @@ 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 - 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); + 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); + } } #endif } @@ -314,15 +312,17 @@ event_module_unload(void *drcontext, const module_data_t *data) if (entry != NULL) { entry->unload = true; #ifndef WINDOWS - 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; + 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; + } } #endif } else @@ -399,7 +399,6 @@ 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 { @@ -414,10 +413,9 @@ 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 ", " - ZHEX64_FORMAT_STRING ", ", + len = dr_snprintf(buf, size, "%3u, %3u, " PFX ", " PFX ", " PFX ", ", idx, entry->containing_id, entry->base, - entry->base+entry->size, entry->entry, entry->offset); + entry->base+entry->size, entry->entry); if (len == -1) return -1; buf += len; @@ -468,10 +466,6 @@ 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); } @@ -490,7 +484,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, offset"); + len = dr_snprintf(buf, size, "Columns: id, containing_id, start, end, entry"); if (len == -1) return DRCOVLIB_ERROR_BUF_TOO_SMALL; buf += len; @@ -669,23 +663,13 @@ 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 - if (version == 3) { + } else { 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; @@ -743,8 +727,6 @@ 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 7729da0e061..b69ba308852 100644 --- a/suite/tests/client-interface/drmodtrack-test.dll.cpp +++ b/suite/tests/client-interface/drmodtrack-test.dll.cpp @@ -38,7 +38,6 @@ #include "drx.h" #include "client_tools.h" #include "string.h" -#include "stddef.h" #ifdef WINDOWS # pragma warning( disable : 4100) /* unreferenced formal parameter */ @@ -133,19 +132,6 @@ 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;