diff --git a/clients/drcachesim/tracer/instru.h b/clients/drcachesim/tracer/instru.h index 3efbb08ea..09bc60233 100644 --- a/clients/drcachesim/tracer/instru.h +++ b/clients/drcachesim/tracer/instru.h @@ -214,6 +214,11 @@ public: virtual void set_entry_addr(byte *buf_ptr, addr_t addr) = 0; + // Fills the first pointer-sized-integer of each record with "sentinel" and the + // rest (if any) with 0. + virtual void + fill_with_sentinel(byte *start, size_t size, ptr_int_t sentinel) = 0; + // All of these return how many bytes to advance the buffer pointer. virtual int @@ -349,6 +354,9 @@ public: void set_entry_addr(byte *buf_ptr, addr_t addr) override; + void + fill_with_sentinel(byte *start, size_t size, ptr_int_t sentinel) override; + int append_pid(byte *buf_ptr, process_id_t pid) override; int @@ -440,6 +448,9 @@ public: void set_entry_addr(byte *buf_ptr, addr_t addr) override; + void + fill_with_sentinel(byte *start, size_t size, ptr_int_t sentinel) override; + int append_pid(byte *buf_ptr, process_id_t pid) override; int diff --git a/clients/drcachesim/tracer/instru_offline.cpp b/clients/drcachesim/tracer/instru_offline.cpp index e7ee2795f..a2cd9d450 100644 --- a/clients/drcachesim/tracer/instru_offline.cpp +++ b/clients/drcachesim/tracer/instru_offline.cpp @@ -298,6 +298,20 @@ offline_instru_t::set_entry_addr(byte *buf_ptr, addr_t addr) entry->addr.addr = addr; } +void +offline_instru_t::fill_with_sentinel(byte *start, size_t size, ptr_int_t sentinel) +{ + for (size_t i = 0; i < size; i += sizeof_entry()) { + // Our records are of type 8-byte offline_entry_t. + *(ptr_int_t *)(start + i) = sentinel; +#ifndef X64 + // insert_save_addr() relies on the top word already being 0 to avoid + // having to write out a top zero word for every 32-bit address. + *(ptr_int_t *)(start + i + 4) = 0; +#endif + } +} + int offline_instru_t::append_pid(byte *buf_ptr, process_id_t pid) { @@ -699,6 +713,9 @@ offline_instru_t::insert_save_addr(void *drcontext, instrlist_t *ilist, instr_t } reserved = true; } + // 32-bit relies on the buffer's top words starting out zero (main buffer area memset + // to 0, and redzone's top word set to 0 by fill_with_sentinel()) so we don't have to + // write a zero into the top word here. MINSERT(ilist, where, XINST_CREATE_store(drcontext, OPND_CREATE_MEMPTR(reg_ptr, disp), opnd_create_reg(reg_addr))); diff --git a/clients/drcachesim/tracer/instru_online.cpp b/clients/drcachesim/tracer/instru_online.cpp index 278aa90ef..b12a84d2c 100644 --- a/clients/drcachesim/tracer/instru_online.cpp +++ b/clients/drcachesim/tracer/instru_online.cpp @@ -110,6 +110,20 @@ online_instru_t::set_entry_addr(byte *buf_ptr, addr_t addr) entry->addr = addr; } +void +online_instru_t::fill_with_sentinel(byte *start, size_t size, ptr_int_t sentinel) +{ + for (size_t i = 0; i + sizeof_entry() <= size; i += sizeof_entry()) { + // Our records are of type 12-byte (8-byte for 32-bit) trace_entry_t. + *(ptr_int_t *)(start + i) = sentinel; +#ifdef X64 + *(int *)(start + i + 8) = 0; +#else + *(int *)(start + i + 4) = 0; +#endif + } +} + int online_instru_t::append_pid(byte *buf_ptr, process_id_t pid) { diff --git a/clients/drcachesim/tracer/output.cpp b/clients/drcachesim/tracer/output.cpp index 1ce2aaa9e..d8a1eff6e 100644 --- a/clients/drcachesim/tracer/output.cpp +++ b/clients/drcachesim/tracer/output.cpp @@ -837,7 +837,11 @@ create_buffer(per_thread_t *data) /* dr_raw_mem_alloc guarantees to give us zeroed memory, so no need for a memset. */ /* Set sentinel value in redzone. */ size_t redzone_size = data->max_buf_size - data->trace_buf_size; - memset(data->buf_base + data->trace_buf_size, -1, redzone_size); + instru->fill_with_sentinel(data->buf_base + data->trace_buf_size, redzone_size, + REDZONE_SENTINEL); + NOTIFY(4, "Created buffer %p-%p; set redzone %p-%p to %d\n", data->buf_base, + data->buf_base + data->max_buf_size, data->buf_base + data->trace_buf_size, + data->buf_base + data->trace_buf_size + redzone_size, REDZONE_SENTINEL); data->num_buffers++; if (data->num_buffers == 2) { /* Create a "reserve" buffer so we can continue after hitting OOM later. @@ -848,8 +852,10 @@ create_buffer(per_thread_t *data) */ data->reserve_buf = (byte *)dr_raw_mem_alloc( data->max_buf_size, DR_MEMPROT_READ | DR_MEMPROT_WRITE, NULL); - if (data->reserve_buf != NULL) - memset(data->reserve_buf + data->trace_buf_size, -1, redzone_size); + if (data->reserve_buf != NULL) { + instru->fill_with_sentinel(data->reserve_buf + data->trace_buf_size, + redzone_size, REDZONE_SENTINEL); + } } } @@ -1230,6 +1236,9 @@ process_and_output_buffer(void *drcontext, bool skip_size_cap, bool at_thread_ex } buf_ptr = BUF_PTR(data->seg_base); + NOTIFY(4, "In %s with %zd bytes written to buffer %p\n", __FUNCTION__, + buf_ptr - data->buf_base, data->buf_base); + // We may get called with nothing to write: e.g., on a syscall for // -L0I_filter and -L0D_filter. if (buf_ptr == data->buf_base + header_size) { @@ -1412,14 +1421,31 @@ process_and_output_buffer(void *drcontext, bool skip_size_cap, bool at_thread_ex } if (file_ops_func.handoff_buf == NULL) { - // Our instrumentation reads from buffer and skips the clean call if the - // content is 0, so we need set zero in the trace buffer and set non-zero - // in redzone. +#ifndef X64 + // 32-bit relies on a memset here to avoid an extra store to zero the + // top word of each load/store record. + // 64-bit avoids this, which improves performance and allows for larger + // buffers. memset(data->buf_base, 0, data->trace_buf_size); +#endif redzone = data->buf_base + data->trace_buf_size; if (buf_ptr > redzone) { - // Set sentinel (non-zero) value in redzone. - memset(redzone, -1, buf_ptr - redzone); + // Re-establish sentinel value in the part of the redzone we overwrote + // with trace data. + instru->fill_with_sentinel(redzone, buf_ptr - redzone, REDZONE_SENTINEL); + } else if (buf_ptr < redzone) { + // Did we hit a false positive sentinel from some prior trace content? + // Our sentinel is not impossible to match with a legitimate record. + // We don't look for this on function entry and just return without + // any output, as some callers expect prompt output: thread exit, filters + // where we output on each syscall, or a case where the caller plans to + // add multiple records and if we're near the end we could overflow by + // returning early. + if (*(ptr_int_t *)buf_ptr == REDZONE_SENTINEL) { + // Clear it to avoid an early output again on the same sentinel. + *(ptr_int_t *)buf_ptr = 0; + ++num_false_sentinels; + } } } BUF_PTR(data->seg_base) = data->buf_base; diff --git a/clients/drcachesim/tracer/tracer.cpp b/clients/drcachesim/tracer/tracer.cpp index aa7c4844c..8f7d4bf60 100644 --- a/clients/drcachesim/tracer/tracer.cpp +++ b/clients/drcachesim/tracer/tracer.cpp @@ -119,7 +119,7 @@ static file_t funclist_file = INVALID_FILE; static file_t encoding_file = INVALID_FILE; /* The redzone is allocated right after the trace buffer. - * We fill the redzone with sentinel value to detect when the redzone + * We fill the redzone with a sentinel value to detect when the redzone * is reached, i.e., when the trace buffer is full. * This holds the base size; any padding from page-aligning the buffer * is added to this, so typically the redzone ends up a full page size. @@ -160,6 +160,7 @@ static client_id_t client_id; void *mutex; /* for multithread support */ uint64 num_refs_racy; /* racy global memory reference count */ uint64 num_filter_refs_racy; /* racy global memory reference count in warmup mode */ +uint64 num_false_sentinels; /* Racy global count of false REDZONE_SENTINEL hit. */ static uint64 num_refs; /* keep a global memory reference count */ static uint64 num_writeouts; static uint64 num_v2p_writeouts; @@ -706,7 +707,7 @@ instrument_delay_instrs(void *drcontext, void *tag, instrlist_t *ilist, user_dat } /* Inserts a conditional branch that jumps to skip_label if reg_skip_if_zero's - * value is zero. + * value is zero (or, if "skip_if_not_value", if the value != "value"). * "*reg_tmp" must start out as DR_REG_NULL. It will hold a temp reg that must be passed * to any subsequent call here as well as to insert_conditional_skip_target() at * the point where skip_label should be inserted. Additionally, the @@ -718,7 +719,8 @@ static void insert_conditional_skip(void *drcontext, instrlist_t *ilist, instr_t *where, reg_id_t reg_skip_if_zero, reg_id_t *reg_tmp DR_PARAM_INOUT, instr_t *skip_label, bool short_reaches, - reg_id_set_t &app_regs_at_skip) + reg_id_set_t &app_regs_at_skip, bool skip_if_not_value = false, + int value = 0) { // Record the registers that will need barriers at the skip target. for (reg_id_t reg = DR_REG_START_GPR; reg <= DR_REG_STOP_GPR; ++reg) { @@ -732,30 +734,63 @@ insert_conditional_skip(void *drcontext, instrlist_t *ilist, instr_t *where, #ifdef X86 DR_ASSERT(reg_skip_if_zero == DR_REG_XCX); - if (short_reaches) { - MINSERT(ilist, where, - INSTR_CREATE_jecxz(drcontext, opnd_create_instr(skip_label))); - } else { - instr_t *should_skip = INSTR_CREATE_label(drcontext); + if (skip_if_not_value) { + if (value != 0) { + MINSERT(ilist, where, + INSTR_CREATE_lea( + drcontext, opnd_create_reg(reg_skip_if_zero), + OPND_CREATE_MEM_lea(reg_skip_if_zero, DR_REG_NULL, 0, -value))); + } instr_t *no_skip = INSTR_CREATE_label(drcontext); - MINSERT(ilist, where, - INSTR_CREATE_jecxz(drcontext, opnd_create_instr(should_skip))); - MINSERT(ilist, where, - INSTR_CREATE_jmp_short(drcontext, opnd_create_instr(no_skip))); - /* XXX i#2825: we need this to not match instr_is_cti_short_rewrite() */ - MINSERT(ilist, where, INSTR_CREATE_nop(drcontext)); - MINSERT(ilist, where, should_skip); - MINSERT(ilist, where, INSTR_CREATE_jmp(drcontext, opnd_create_instr(skip_label))); + MINSERT(ilist, where, INSTR_CREATE_jecxz(drcontext, opnd_create_instr(no_skip))); + if (short_reaches) { + MINSERT(ilist, where, + INSTR_CREATE_jmp_short(drcontext, opnd_create_instr(skip_label))); + /* XXX i#2825: we need this to not match instr_is_cti_short_rewrite() */ + MINSERT(ilist, where, INSTR_CREATE_nop(drcontext)); + } else { + MINSERT(ilist, where, + INSTR_CREATE_jmp(drcontext, opnd_create_instr(skip_label))); + } MINSERT(ilist, where, no_skip); + } else { + if (short_reaches) { + MINSERT(ilist, where, + INSTR_CREATE_jecxz(drcontext, opnd_create_instr(skip_label))); + } else { + instr_t *should_skip = INSTR_CREATE_label(drcontext); + instr_t *no_skip = INSTR_CREATE_label(drcontext); + MINSERT(ilist, where, + INSTR_CREATE_jecxz(drcontext, opnd_create_instr(should_skip))); + MINSERT(ilist, where, + INSTR_CREATE_jmp_short(drcontext, opnd_create_instr(no_skip))); + /* XXX i#2825: we need this to not match instr_is_cti_short_rewrite() */ + MINSERT(ilist, where, INSTR_CREATE_nop(drcontext)); + MINSERT(ilist, where, should_skip); + MINSERT(ilist, where, + INSTR_CREATE_jmp(drcontext, opnd_create_instr(skip_label))); + MINSERT(ilist, where, no_skip); + } } #elif defined(ARM) if (dr_get_isa_mode(drcontext) == DR_ISA_ARM_THUMB) { instr_t *noskip = INSTR_CREATE_label(drcontext); /* XXX: clean call is too long to use cbz to skip. */ DR_ASSERT(reg_skip_if_zero <= DR_REG_R7); /* cbnz can't take r8+ */ - MINSERT(ilist, where, - INSTR_CREATE_cbnz(drcontext, opnd_create_instr(noskip), - opnd_create_reg(reg_skip_if_zero))); + if (skip_if_not_value) { + if (value != 0) { + MINSERT(ilist, where, + XINST_CREATE_sub(drcontext, opnd_create_reg(reg_skip_if_zero), + OPND_CREATE_INT(value))); + } + MINSERT(ilist, where, + INSTR_CREATE_cbz(drcontext, opnd_create_instr(noskip), + opnd_create_reg(reg_skip_if_zero))); + } else { + MINSERT(ilist, where, + INSTR_CREATE_cbnz(drcontext, opnd_create_instr(noskip), + opnd_create_reg(reg_skip_if_zero))); + } MINSERT(ilist, where, XINST_CREATE_jump(drcontext, opnd_create_instr(skip_label))); MINSERT(ilist, where, noskip); @@ -774,16 +809,27 @@ insert_conditional_skip(void *drcontext, instrlist_t *ilist, instr_t *where, } MINSERT(ilist, where, INSTR_CREATE_cmp(drcontext, opnd_create_reg(reg_skip_if_zero), - OPND_CREATE_INT(0))); - MINSERT( - ilist, where, - instr_set_predicate( - XINST_CREATE_jump(drcontext, opnd_create_instr(skip_label)), DR_PRED_EQ)); + OPND_CREATE_INT(skip_if_not_value ? value : 0))); + MINSERT(ilist, where, + instr_set_predicate( + XINST_CREATE_jump(drcontext, opnd_create_instr(skip_label)), + skip_if_not_value ? DR_PRED_NE : DR_PRED_EQ)); } #elif defined(AARCH64) - MINSERT(ilist, where, - INSTR_CREATE_cbz(drcontext, opnd_create_instr(skip_label), - opnd_create_reg(reg_skip_if_zero))); + if (skip_if_not_value) { + if (value != 0) { + MINSERT(ilist, where, + XINST_CREATE_sub(drcontext, opnd_create_reg(reg_skip_if_zero), + OPND_CREATE_INT(value))); + } + MINSERT(ilist, where, + INSTR_CREATE_cbnz(drcontext, opnd_create_instr(skip_label), + opnd_create_reg(reg_skip_if_zero))); + } else { + MINSERT(ilist, where, + INSTR_CREATE_cbz(drcontext, opnd_create_instr(skip_label), + opnd_create_reg(reg_skip_if_zero))); + } #elif defined(RISCV64) MINSERT(ilist, where, INSTR_CREATE_beq(drcontext, opnd_create_instr(skip_label), @@ -829,15 +875,16 @@ insert_conditional_skip_target(void *drcontext, instrlist_t *ilist, instr_t *whe #endif } +// Compares the global mode at the absolute address "addr" with the local +// mode in TLS "slot" and subtracts them, such that "reg_result" holds 0 +// iff they are equal. static void insert_mode_comparison(void *drcontext, instrlist_t *ilist, instr_t *where, - reg_id_t reg_ptr, void *addr, uint slot) + reg_id_t reg_result, void *addr, uint slot) { - reg_id_t reg_mine = DR_REG_NULL, reg_global = DR_REG_NULL; - if (drreg_reserve_register(drcontext, ilist, where, NULL, ®_mine) != - DRREG_SUCCESS || - drreg_reserve_register(drcontext, ilist, where, NULL, ®_global) != - DRREG_SUCCESS) + reg_id_t reg_global = DR_REG_NULL; + if (drreg_reserve_register(drcontext, ilist, where, NULL, ®_global) != + DRREG_SUCCESS) FATAL("Fatal error: failed to reserve reg."); #ifdef AARCHXX instrlist_insert_mov_immed_ptrsz(drcontext, (ptr_int_t)addr, @@ -859,10 +906,10 @@ insert_mode_comparison(void *drcontext, instrlist_t *ilist, instr_t *where, OPND_CREATE_ABSMEM(addr, OPSZ_PTR))); #endif dr_insert_read_raw_tls(drcontext, ilist, where, tls_seg, - tls_offs + sizeof(void *) * slot, reg_mine); + tls_offs + sizeof(void *) * slot, reg_result); #ifdef AARCHXX MINSERT(ilist, where, - XINST_CREATE_sub(drcontext, opnd_create_reg(reg_mine), + XINST_CREATE_sub(drcontext, opnd_create_reg(reg_result), opnd_create_reg(reg_global))); #elif defined(RISCV64) /* XXX i#3544: Not implemented */ @@ -875,24 +922,11 @@ insert_mode_comparison(void *drcontext, instrlist_t *ilist, instr_t *where, INSTR_CREATE_lea(drcontext, opnd_create_reg(reg_global), OPND_CREATE_MEM_lea(reg_global, DR_REG_NULL, 0, 1))); MINSERT(ilist, where, - INSTR_CREATE_lea(drcontext, opnd_create_reg(reg_mine), - OPND_CREATE_MEM_lea(reg_mine, reg_global, 1, 0))); + INSTR_CREATE_lea(drcontext, opnd_create_reg(reg_result), + OPND_CREATE_MEM_lea(reg_result, reg_global, 1, 0))); #endif - // To avoid writing a 0 on top of the redzone, we read the buffer value and add - // that to the local ("mine") window minus the global window. The redzone is - // -1, so if we do mine minus global which will always be non-positive, we'll - // never write 0 for a redzone slot (and thus possibly overflowing). - MINSERT(ilist, where, - XINST_CREATE_load(drcontext, opnd_create_reg(reg_global), - OPND_CREATE_MEMPTR(reg_ptr, 0))); - MINSERT(ilist, where, - XINST_CREATE_add(drcontext, opnd_create_reg(reg_mine), - opnd_create_reg(reg_global))); - MINSERT(ilist, where, - XINST_CREATE_store(drcontext, OPND_CREATE_MEMPTR(reg_ptr, 0), - opnd_create_reg(reg_mine))); - if (drreg_unreserve_register(drcontext, ilist, where, reg_global) != DRREG_SUCCESS || - drreg_unreserve_register(drcontext, ilist, where, reg_mine) != DRREG_SUCCESS) + // reg_result now holds 0 iff the modes were the same. + if (drreg_unreserve_register(drcontext, ilist, where, reg_global) != DRREG_SUCCESS) FATAL("Fatal error: failed to unreserve scratch reg.\n"); } @@ -900,6 +934,9 @@ insert_mode_comparison(void *drcontext, instrlist_t *ilist, instr_t *where, * is reached. If redzone is reached, the clean call will be called. * Additionally, for tracing windows, we also check for a mode switch and * invoke the clean call if our tracing window is over. + * XXX i#8125: Maybe we should add a disassembly listing, or a tool that + * generates one, so it's easier to understand what the generated code looks + * like with the different modes and options? */ static void instrument_clean_call(void *drcontext, instrlist_t *ilist, instr_t *where, @@ -924,48 +961,86 @@ instrument_clean_call(void *drcontext, instrlist_t *ilist, instr_t *where, short_reaches = false; #endif - if (has_tracing_windows()) { + // We can share one reg_tmp across all insert_conditional_skip. + reg_id_t reg_tmp = DR_REG_NULL; + + reg_id_set_t app_regs_at_skip4windows, app_regs_at_skip4filter; + instr_t *skip2call_windows = INSTR_CREATE_label(drcontext); + instr_t *skip2call_filter = INSTR_CREATE_label(drcontext); + if (has_tracing_windows() || op_L0_filter_until_instrs.get_value()) { + reg_id_t reg_scratch; + if (drreg_reserve_register(drcontext, ilist, where, NULL, ®_scratch) != + DRREG_SUCCESS) + FATAL("Fatal error: failed to reserve scratch reg.\n"); + reg_id_t reg_result = reg_scratch; +#ifdef X86 + // The skip-if-zero reg must be xcx. + MINSERT(ilist, where, + INSTR_CREATE_xchg(drcontext, opnd_create_reg(reg_ptr), + opnd_create_reg(reg_scratch))); + reg_result = reg_ptr; +#endif // We need to do the clean call if the mode has changed back to counting. To // detect a double-change we compare the TLS-stored last window to the - // current tracing_window. To avoid flags and avoid another branch (jumping - // over the filter and redzone checks, e.g.), our strategy is to arrange for - // the redzone load to trigger the call for us if the two window values are - // not equal by writing their difference onto the next buffer slot. This - // requires a store and two scratch regs so perhaps this should be measured - // against a branch-based scheme, but we assume we're i/o bound and so this will - // not affect overhead. - insert_mode_comparison(drcontext, ilist, where, reg_ptr, &tracing_window, - MEMTRACE_TLS_OFFS_WINDOW); + // current tracing_window. We skip over the other skips of the call, + // to ensure the call is made. + if (has_tracing_windows()) { + insert_mode_comparison(drcontext, ilist, where, reg_result, &tracing_window, + MEMTRACE_TLS_OFFS_WINDOW); + insert_conditional_skip(drcontext, ilist, where, reg_result, ®_tmp, + skip2call_windows, short_reaches, + app_regs_at_skip4windows, + /*skip_if_not_value=*/true, 0); + } + if (op_L0_filter_until_instrs.get_value()) { + // Force a clean call when another thread changes tracing mode, so that + // we can update our trace appropriately. + insert_mode_comparison(drcontext, ilist, where, reg_result, &tracing_mode, + MEMTRACE_TLS_OFFS_MODE); + insert_conditional_skip(drcontext, ilist, where, reg_result, ®_tmp, + skip2call_filter, short_reaches, + app_regs_at_skip4filter, + /*skip_if_not_value=*/true, 0); + } +#ifdef X86 + MINSERT(ilist, where, + INSTR_CREATE_xchg(drcontext, opnd_create_reg(reg_ptr), + opnd_create_reg(reg_scratch))); +#endif + if (drreg_unreserve_register(drcontext, ilist, where, reg_scratch) != + DRREG_SUCCESS) + FATAL("Fatal error: failed to unreserve scratch reg.\n"); } - if (op_L0_filter_until_instrs.get_value()) { - // Force a clean call when another thread changes tracing mode, so that - // we can update our trace appropriately. - insert_mode_comparison(drcontext, ilist, where, reg_ptr, &tracing_mode, - MEMTRACE_TLS_OFFS_MODE); - } - - reg_id_t reg_tmp = DR_REG_NULL, reg_tmp2 = DR_REG_NULL; instr_t *skip_thread = INSTR_CREATE_label(drcontext); reg_id_set_t app_regs_at_skip_thread; bool is_L0I_enabled, is_L0D_enabled; get_L0_filters_enabled(mode, &is_L0I_enabled, &is_L0D_enabled); if ((is_L0I_enabled || is_L0D_enabled) && thread_filtering_enabled) { - insert_conditional_skip(drcontext, ilist, where, reg_ptr, ®_tmp2, skip_thread, + insert_conditional_skip(drcontext, ilist, where, reg_ptr, ®_tmp, skip_thread, short_reaches, app_regs_at_skip_thread); } + // We fill the redzone so that the first pointer-sized bytes of each record holds + // REDZONE_SENTINEL, which we load and look for here. MINSERT(ilist, where, XINST_CREATE_load(drcontext, opnd_create_reg(reg_ptr), OPND_CREATE_MEMPTR(reg_ptr, 0))); reg_id_set_t app_regs_at_skip_call; insert_conditional_skip(drcontext, ilist, where, reg_ptr, ®_tmp, skip_call, - short_reaches, app_regs_at_skip_call); + short_reaches, app_regs_at_skip_call, + /*skip_if_not_value=*/true, REDZONE_SENTINEL); + + insert_conditional_skip_target(drcontext, ilist, where, skip2call_filter, reg_tmp, + app_regs_at_skip4filter); + insert_conditional_skip_target(drcontext, ilist, where, skip2call_windows, reg_tmp, + app_regs_at_skip4windows); dr_insert_clean_call_ex(drcontext, ilist, where, (void *)clean_call, DR_CLEANCALL_ALWAYS_OUT_OF_LINE, 0); + insert_conditional_skip_target(drcontext, ilist, where, skip_call, reg_tmp, app_regs_at_skip_call); - insert_conditional_skip_target(drcontext, ilist, where, skip_thread, reg_tmp2, + insert_conditional_skip_target(drcontext, ilist, where, skip_thread, reg_tmp, app_regs_at_skip_thread); } @@ -1405,6 +1480,11 @@ event_app_instruction(void *drcontext, void *tag, instrlist_t *bb, instr_t *inst */ drreg_init_and_fill_vector(&rvec, false); #ifdef X86 + // Performance tests show it's better to limit ourselves to xcx and use the + // supposedly-slow OP_jecxz, versus saving aflags for cmp;jcc and using any + // scratch reg. + // XXX i#8125: Maybe we should try cmp;jcc for blocks that themselves end in + // cmp;jcc, moving the clean call back before the cmp? drreg_set_vector_entry(&rvec, DR_REG_XCX, true); #elif defined(RISCV64) /* XXX i#3544: Check if scratch reg can be used here. */ @@ -2214,8 +2294,9 @@ event_exit(void) num_refs); NOTIFY(1, "drmemtrace exiting process " PIDFMT "; traced " UINT64_FORMAT_STRING - " references in " UINT64_FORMAT_STRING " writeouts.\n", - dr_get_process_id(), num_refs, num_writeouts); + " references in " UINT64_FORMAT_STRING " writeouts with " UINT64_FORMAT_STRING + " false sentinels.\n", + dr_get_process_id(), num_refs, num_writeouts, num_false_sentinels); if (op_use_physical.get_value()) { dr_log(NULL, DR_LOG_ALL, 1, "drcachesim num physical address markers emitted: " UINT64_FORMAT_STRING @@ -2714,6 +2795,12 @@ drmemtrace_client_main(client_id_t id, int argc, const char *argv[]) if (!ipc_pipe.maximize_buffer()) NOTIFY(1, "Failed to maximize pipe buffer: performance may suffer.\n"); } + // Ensure the buffer record will read back as a 64-bit redzone sentinel, even + // with 12-byte trace_entry_t records. + byte redzone_test[32]; + DR_ASSERT(sizeof(redzone_test) > instru->sizeof_entry()); + instru->fill_with_sentinel(redzone_test, instru->sizeof_entry(), REDZONE_SENTINEL); + DR_ASSERT(*(ptr_int_t *)redzone_test == REDZONE_SENTINEL); if (op_offline.get_value() && !func_trace_init(append_marker_seg_base, file_ops_func.write_file, diff --git a/clients/drcachesim/tracer/tracer.h b/clients/drcachesim/tracer/tracer.h index f30266748..ff8e9f9a4 100644 --- a/clients/drcachesim/tracer/tracer.h +++ b/clients/drcachesim/tracer/tracer.h @@ -78,6 +78,11 @@ extern named_pipe_t ipc_pipe; /* Thread private data. This is all set to 0 at thread init. */ struct per_thread_t { byte *seg_base; + // Buffer for recording trace content. For 32-bit offline with its + // 8-byte offline_type_t records, we require that the top 4-byte word + // is pre-set to 0 so we don't have to zero it; this is done by + // memset-ing the main area to 0 and having instru_offline_t::fill_with_sentinel() + // set the top word for the redzone to 0. byte *buf_base; size_t trace_buf_size; // Size without the redzone. size_t max_buf_size; // Allocated size, which includes the redzone. @@ -260,6 +265,21 @@ extern char logsubdir[MAXIMUM_PATH]; extern char subdir_prefix[MAXIMUM_PATH]; /* Holds op_subdir_prefix. */ extern size_t buf_hdr_slots_size; +// We avoid having to memset the actual buffer by picking a sentinel that is very unlikely +// to be a real raw record. (If it is, we have only an early buffer output, not a +// correctness issue.) We also need a small enough value that we can use it as +// an immediate to subtract to reach 0 in insert_conditional_skip(), so this should +// be < 4095 (largest AArch64 SUB immediate). +// For x64 offline, this would be a load/store to address 1, which is +// not likely to happen (we do see prefetches of 0 so we avoid 0). +// For 32-bit this would be a load/store to an address with the 33rd bit set to 1 +// which is impossible. +// For online, we fill in the first pointer-sized bits of each trace_entry_t with +// this value, with the rest zeroed, so that's TRACE_TYPE_WRITE with size 0. +#define REDZONE_SENTINEL 1 +// Racy count of false sentinels we hit. +extern uint64 num_false_sentinels; + #define MAX_NUM_DELAY_INSTRS 32 // Really sizeof(trace_entry_t.length) #define MAX_NUM_DELAY_ENTRIES (MAX_NUM_DELAY_INSTRS / sizeof(addr_t)) diff --git a/core/ir/aarch64/instr_create_api.h b/core/ir/aarch64/instr_create_api.h index 4f3b2dc81..3b29c6b49 100644 --- a/core/ir/aarch64/instr_create_api.h +++ b/core/ir/aarch64/instr_create_api.h @@ -575,6 +575,7 @@ instr_create_1dst_4src((dc), OP_ands, (rd), (rn), \ opnd_create_reg_ex(opnd_get_reg(rm), 0, DR_OPND_SHIFTED), \ opnd_add_flags((sht), DR_OPND_IS_SHIFT), (sha)) + #define INSTR_CREATE_b(dc, pc) instr_create_0dst_1src((dc), OP_b, (pc)) /** \endcond disabled_until_i4106_is_fixed */ /**