Skip to content

Commit

Permalink
Revert "i#2006 generalize drcachesim: Split segments and record segme…
Browse files Browse the repository at this point in the history
…nt offsets (#2940)" (#2963)

This reverts commit 8c90e2d
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.
  • Loading branch information
derekbruening authored Apr 25, 2018
1 parent 816a849 commit 48db566
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 90 deletions.
3 changes: 0 additions & 3 deletions api/docs/release.dox
Original file line number Diff line number Diff line change
Expand Up @@ -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.

**************************************************
<hr>
Expand Down
1 change: 0 additions & 1 deletion core/lib/instrument.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion core/lib/instrument_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 1 addition & 3 deletions core/unix/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions core/unix/module.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions core/unix/module_elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 1 addition & 2 deletions core/unix/module_macho.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
4 changes: 0 additions & 4 deletions ext/drcovlib/drcovlib.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
96 changes: 39 additions & 57 deletions ext/drcovlib/modules.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#include <stdio.h>
#include <stddef.h>

#define MODULE_FILE_VERSION 4
#define MODULE_FILE_VERSION 3

#define NUM_GLOBAL_MODULE_CACHE 8
#define NUM_THREAD_MODULE_CACHE 4
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
14 changes: 0 additions & 14 deletions suite/tests/client-interface/drmodtrack-test.dll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 48db566

Please sign in to comment.