i#8125 a64 opt: Invert redzone check (#8135)

Inverts the drmemtrace trace buffer redzone check. Instead of re-setting
the buffer to 0 every time and the redzone to -1 and identifying when to
output by reading a non-0 value, we set the redzone to hold a sentinel
record value: the value 1, which would be a load/store of the address 1,
which seems pretty unlikely (we do see 0 in prefetches). If it does
happen, we'll just output early; the output code is updated here to
clobber the 1 so the same record won't trigger again.

For online, with 12-byte records, we set the 1st pointer-sized bytes to
the sentinel 1 and the final 4 to 0. This is a write to 0, which should
also be rare.

For 32-bit, we have to keep the memset, as loads and stores assume the
top word is already zeroed, but 32-bit is not a performance priority.

Changing the clean call skip is pretty simple on arm; on x86 it adds a
redirection jump. The filter mode change checks that leverage the
redzone check are more complicated and we change those to branches that
skip over the other skips.

On SPECCPU2017 INT this provides a 2% performance improvement, but it
enables larger buffer sizes without paying a memset cost. A future
update will make the main thread's buffer 8x larger, which is a 14%
no-i/o improvement and half that with LZ4 output.

Issue: #8125

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

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)));

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)
 {

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;

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, &reg_mine) !=
-            DRREG_SUCCESS ||
-        drreg_reserve_register(drcontext, ilist, where, NULL, &reg_global) !=
-            DRREG_SUCCESS)
+    reg_id_t reg_global = DR_REG_NULL;
+    if (drreg_reserve_register(drcontext, ilist, where, NULL, &reg_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, &reg_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, &reg_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, &reg_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, &reg_tmp2, skip_thread,
+        insert_conditional_skip(drcontext, ilist, where, reg_ptr, &reg_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, &reg_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,

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))

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 */
 /**