i#2062: Support filtering of non-module blocks (#6469)

Adds support for instrumenting non-module code instr-by-instr instead of
whole-block-at-a-time. This is required for the L0 filter mode which
instruments individual instructions rather than one pc entry for the
whole block.

Under the new scheme, the modoffs field in each trace PC entry point to
each non-mod instrs separately, rather than just the top of the non-mod
block. Specifically, we store the cumulative encoding length of instrs
stored to the encoding file prior to the recorded instr. This may
potentially allow too few gencode blocks for JIT apps (we'd support 8G
of JIT code); we could use multiple modidx to point to gencode (growing
downward from PC_MODIDX_INVALID) to help with that in the future. Added
a TODO.

Adds file type to the encoding file header. Adds a new encoding file
version to indicate the presence of file type. Adds a new file type bit
that denotes that the trace was filtered and that the module_mapper_t
should interpret the modoffs as pointing to a single instr as described
above. When this bit is not set, we use the existing scheme of
interpreting the pc modoffs as just the non-mod block-idx.

Note that even under the new scheme, we still write the encoding file
one mon-module block at a time because it is too inefficient to write
one encoding_entry_t for just one non-module instr.

Modifies record_instr_encodings to skip writing anything to the encoding
file for blocks without any app instr.

Adds a new variant of the tool.drcacheoff.gencode test that runs with
the L0_filter enabled. Modifies the test to add a sequence of instrs
that, without this fix, produces an error in raw2trace due to an
apparently out-of-block memref.

Issue: #2062

api/docs/release.dox

@@ -142,6 +142,10 @@ changes:
    refers to timestamps and direct switches, which is what most users should want.
 
 Further non-compatibility-affecting changes include:
+ - Added a new scheme for the modoffs field in the PC trace entry which allows L0
+   filtering of non-module code; see
+   #dynamorio::drmemtrace::ENCODING_FILE_TYPE_SEPARATE_NON_MOD_INSTRS. Also added
+   file type entry to the header of encoding files.
  - Fixed a bug in the AArch64 codec with the way that SVE scalar+immediate predicated
    contiguous load and store instructions represented the immediate offset in the IR.
    In 10.0.0 the memory operand in these instruction used the immediate value from the

clients/drcachesim/common/trace_entry.h

@@ -821,6 +821,12 @@ typedef enum {
 #define PC_MODOFFS_BITS 33
 #define PC_MODIDX_BITS 16
 // We reserve the top value to indicate non-module generated code.
+// TODO i#2062: Filtered traces use a different scheme for modoffs (see
+// ENCODING_FILE_TYPE_SEPARATE_NON_MOD_INSTRS) where the total non-module
+// code is limited to 8GB (33 bytes worth of addressing). We can potentially
+// allow more gencode by using multiple modidx (and not just
+// PC_MODIDX_INVALID) for pointing to non-module code, growing downward from
+// PC_MODIDX_INVALID.
 #define PC_MODIDX_INVALID ((1 << PC_MODIDX_BITS) - 1)
 #define PC_INSTR_COUNT_BITS 12
 #define PC_TYPE_BITS 3
@@ -984,7 +990,32 @@ typedef union {
 // The encoding file begins with a 64-bit integer holding a version number,
 // followed by a series of records of type encoding_entry_t.
 #define ENCODING_FILE_INITIAL_VERSION 0
-#define ENCODING_FILE_VERSION ENCODING_FILE_INITIAL_VERSION
+// Encoding files have a file type as the second uint64_t in their header.
+#define ENCODING_FILE_VERSION_HAS_FILE_TYPE 1
+#define ENCODING_FILE_VERSION ENCODING_FILE_VERSION_HAS_FILE_TYPE
+
+/**
+ * Bitfields used to describe the type of the encoding file. This is stored as the
+ * second uint64_t after the encoding file version.
+ */
+typedef enum {
+    /**
+     * Default encoding file type.
+     */
+    ENCODING_FILE_TYPE_DEFAULT = 0x0,
+    /**
+     * This encoding file type tells the module_mapper_t that the non-module PC
+     * entries in the trace correspond to an individual instr. The modoffs field is
+     * interpreted as the cumulative encoding length of all instrs written to the
+     * encoding file before the recorded instr. Note that the encoding file itself
+     * is still written one mon-module block at a time because it is too inefficient
+     * to write one encoding_entry_t for just one non-module instr.
+     *
+     * If this file type is not set, then the PC entries' modoffs fields are
+     * interpreted as the non-mod block's idx.
+     */
+    ENCODING_FILE_TYPE_SEPARATE_NON_MOD_INSTRS = 0x1,
+} encoding_file_type_t;
 
 // All fields are little-endian.
 START_PACKED_STRUCTURE

clients/drcachesim/tests/burst_gencode.cpp

@@ -151,6 +151,23 @@ class code_generator_t {
                          XINST_CREATE_store(dc, OPND_CREATE_MEMPTR(base, -ptrsz),
                                             opnd_create_reg(base)));
 
+        // Test raw2trace for filtered traces.
+        instr_t *nop1 = XINST_CREATE_nop(dc);
+        // Start new basic block.
+        instrlist_append(ilist, XINST_CREATE_jump(dc, opnd_create_instr(nop1)));
+        // First instr is one without a memref.
+        instrlist_append(ilist, nop1);
+        // Second instr has a memref. If raw2trace always picks the first bb instr for
+        // gencode bb instrs, this will result in a "memref entry found outside of bb"
+        // error.
+        instrlist_append(ilist,
+                         XINST_CREATE_load_int(dc, opnd_create_reg(base4imm),
+                                               OPND_CREATE_INT32(kGencodeMagic2)));
+        instr_t *nop2 = XINST_CREATE_nop(dc);
+        instrlist_append(ilist, XINST_CREATE_jump(dc, opnd_create_instr(nop2)));
+        // End basic block.
+        instrlist_append(ilist, nop2);
+
 #ifdef LINUX
         // Test a signal in non-module code.
 #    ifdef X86
@@ -287,38 +304,43 @@ post_process()
 }
 
 static std::string
-gather_trace()
+gather_trace(const std::string &add_env)
 {
 #ifdef LINUX
     intercept_signal(SIGILL, handle_signal, false);
 #endif
 
-    if (!my_setenv("DYNAMORIO_OPTIONS",
+    std::string env =
 #if defined(LINUX) && defined(X64)
-                   // We pass -satisfy_w_xor_x to further stress that option
-                   // interacting with standalone mode (xref i#5621).
-                   "-satisfy_w_xor_x "
+        // We pass -satisfy_w_xor_x to further stress that option
+        // interacting with standalone mode (xref i#5621).
+        "-satisfy_w_xor_x "
 #endif
-                   "-stderr_mask 0xc -client_lib ';;-offline"))
+        "-stderr_mask 0xc -client_lib ';;-offline";
+    env += add_env;
+    if (!my_setenv("DYNAMORIO_OPTIONS", env.c_str()))
         std::cerr << "failed to set env var!\n";
     code_generator_t gen(false);
-    std::cerr << "pre-DR init\n";
+    std::cerr << "pre-DR init\n" << std::flush;
     dr_app_setup();
     assert(!dr_app_running_under_dynamorio());
     drmemtrace_status_t res = drmemtrace_buffer_handoff(nullptr, exit_cb, nullptr);
     assert(res == DRMEMTRACE_SUCCESS);
-    std::cerr << "pre-DR start\n";
+    std::cerr << "pre-DR start\n" << std::flush;
     dr_app_start();
     if (do_some_work(gen) < 0)
         std::cerr << "error in computation\n";
-    std::cerr << "pre-DR detach\n";
+    // TODO i#6490: This app produces incorrect output when run under DR if we do
+    // not flush. std::endl makes the issue worse even though it should do an
+    // internal flush.
+    std::cerr << "pre-DR detach\n" << std::flush;
     dr_app_stop_and_cleanup();
-    std::cerr << "all done\n";
+    std::cerr << "all done\n" << std::flush;
     return post_process();
 }
 
 static int
-look_for_gencode(std::string trace_dir)
+look_for_gencode(std::string trace_dir, bool look_for_magic)
 {
     void *dr_context = dr_standalone_init();
     scheduler_t scheduler;
@@ -372,15 +394,23 @@ look_for_gencode(std::string trace_dir)
         instr_free(dr_context, &instr);
     }
     dr_standalone_exit();
-    assert(found_magic2);
+    assert(!look_for_magic || found_magic2);
     return 0;
 }
 
 int
 test_main(int argc, const char *argv[])
 {
-    std::string trace_dir = gather_trace();
-    return look_for_gencode(trace_dir);
+    std::string extra_client_opts = "";
+    for (int i = 1; i < argc; ++i) {
+        extra_client_opts += " ";
+        extra_client_opts += argv[i];
+    }
+    std::string trace_dir = gather_trace(extra_client_opts);
+    bool look_for_magic = true;
+    if (extra_client_opts.find("-L0_filter") != std::string::npos)
+        look_for_magic = false;
+    return look_for_gencode(trace_dir, look_for_magic);
 }
 
 } // namespace drmemtrace

clients/drcachesim/tests/offline-gencode_filtered.templatex

@@ -0,0 +1,6 @@
+pre-DR init
+pre-DR start
+pre-DR detach
+all done
+Opcode mix tool results:
+.*

clients/drcachesim/tracer/instru.h

@@ -417,7 +417,7 @@ class offline_instru_t : public instru_t {
                      drvector_t *reg_vector,
                      ssize_t (*write_file)(file_t file, const void *data, size_t count),
                      file_t module_file, file_t encoding_file,
-                     bool disable_optimizations = false,
+                     bool disable_optimizations = false, bool instrs_are_separate = false,
                      void (*log)(uint level, const char *fmt, ...) = nullptr);
     virtual ~offline_instru_t();
 
@@ -521,6 +521,8 @@ class offline_instru_t : public instru_t {
     struct per_block_t {
         uint64_t id = 0;
         uint instr_count = 0;
+        app_pc start_pc = 0;
+        uint64_t encoding_length_start = 0;
     };
 
     bool
@@ -587,7 +589,9 @@ class offline_instru_t : public instru_t {
     size_t encoding_buf_sz_ = 0;
     byte *encoding_buf_ptr_ = nullptr;
     uint64_t encoding_id_ = 0;
+    uint64_t encoding_length_ = 0;
     uint64_t encoding_bytes_written_ = 0;
+    bool instrs_are_separate_ = false;
 };
 
 } // namespace drmemtrace

clients/drcachesim/tracer/instru_offline.cpp

@@ -78,13 +78,14 @@ offline_instru_t::offline_instru_t(
     drvector_t *reg_vector,
     ssize_t (*write_file)(file_t file, const void *data, size_t count),
     file_t module_file, file_t encoding_file, bool disable_optimizations,
-    void (*log)(uint level, const char *fmt, ...))
+    bool instrs_are_separate, void (*log)(uint level, const char *fmt, ...))
     : instru_t(insert_load_buf, reg_vector, sizeof(offline_entry_t),
                disable_optimizations)
     , write_file_func_(write_file)
     , modfile_(module_file)
     , log_(log)
     , encoding_file_(encoding_file)
+    , instrs_are_separate_(instrs_are_separate)
 {
     drcovlib_status_t res = drmodtrack_init();
     DR_ASSERT(res == DRCOVLIB_SUCCESS);
@@ -110,9 +111,19 @@ offline_instru_t::offline_instru_t(
     encoding_buf_start_ = reinterpret_cast<byte *>(
         dr_raw_mem_alloc(encoding_buf_sz_, DR_MEMPROT_READ | DR_MEMPROT_WRITE, nullptr));
     encoding_buf_ptr_ = encoding_buf_start_;
-    // Write out the header which is just a 64-bit version.
+    // Write out the encoding file header.
+    // 64-bit version.
     *reinterpret_cast<uint64_t *>(encoding_buf_ptr_) = ENCODING_FILE_VERSION;
     encoding_buf_ptr_ += sizeof(uint64_t);
+    // 64-bit file type.
+    uint64_t encoding_file_type =
+        static_cast<uint64_t>(encoding_file_type_t::ENCODING_FILE_TYPE_DEFAULT);
+    if (instrs_are_separate_) {
+        encoding_file_type |= static_cast<uint64_t>(
+            encoding_file_type_t::ENCODING_FILE_TYPE_SEPARATE_NON_MOD_INSTRS);
+    }
+    *reinterpret_cast<uint64_t *>(encoding_buf_ptr_) = encoding_file_type;
+    encoding_buf_ptr_ += sizeof(uint64_t);
 }
 
 offline_instru_t::~offline_instru_t()
@@ -480,6 +491,7 @@ offline_instru_t::record_instr_encodings(void *drcontext, app_pc tag_pc,
     log_(3, "%s: new block id " UINT64_FORMAT_STRING " for %p\n", __FUNCTION__,
          encoding_id_, tag_pc);
     per_block->id = encoding_id_++;
+    per_block->encoding_length_start = encoding_length_;
 
     if (encoding_buf_ptr_ + max_block_encoding_size_ >=
         encoding_buf_start_ + encoding_buf_sz_) {
@@ -518,6 +530,13 @@ offline_instru_t::record_instr_encodings(void *drcontext, app_pc tag_pc,
         DR_ASSERT(buf < encoding_buf_start_ + encoding_buf_sz_);
     }
 
+    DR_ASSERT(buf >= buf_start + sizeof(encoding_entry_t));
+    if (buf == buf_start + sizeof(encoding_entry_t)) {
+        // If the given ilist has no app instr, we skip writing anything to the
+        // encoding file.
+        dr_mutex_unlock(encoding_lock_);
+        return;
+    }
     encoding_entry_t *enc = reinterpret_cast<encoding_entry_t *>(buf_start);
     enc->length = buf - buf_start;
     enc->id = per_block->id;
@@ -526,7 +545,7 @@ offline_instru_t::record_instr_encodings(void *drcontext, app_pc tag_pc,
         dr_app_pc_as_jump_target(instr_get_isa_mode(instrlist_first(ilist)), tag_pc));
     log_(2, "%s: Recorded %zu bytes for id " UINT64_FORMAT_STRING " @ %p\n", __FUNCTION__,
          enc->length, enc->id, tag_pc);
-
+    encoding_length_ += (enc->length - sizeof(encoding_entry_t));
     encoding_buf_ptr_ += enc->length;
     dr_mutex_unlock(encoding_lock_);
 }
@@ -570,7 +589,12 @@ offline_instru_t::insert_save_pc(void *drcontext, instrlist_t *ilist, instr_t *w
         modidx = PC_MODIDX_INVALID;
         // For generated code we store the id for matching with the encodings recorded
         // into the encoding file.
-        modoffs = per_block->id;
+        if (instrs_are_separate_) {
+            DR_ASSERT(pc >= per_block->start_pc);
+            modoffs = pc - per_block->start_pc + per_block->encoding_length_start;
+        } else {
+            modoffs = per_block->id;
+        }
     }
     // Check that the values we want to assign to the bitfields in offline_entry_t do not
     // overflow. In i#2956 we observed an overflow for the modidx field.
@@ -840,10 +864,12 @@ offline_instru_t::bb_analysis(void *drcontext, void *tag, void **bb_field,
 
     per_block->instr_count = instru_t::count_app_instrs(ilist);
 
+    app_pc tag_pc = dr_fragment_app_pc(tag);
+    per_block->start_pc = tag_pc;
+
     identify_elidable_addresses(drcontext, ilist, OFFLINE_FILE_VERSION,
                                 memref_needs_full_info);
 
-    app_pc tag_pc = dr_fragment_app_pc(tag);
     if (does_pc_require_encoding(drcontext, tag_pc, nullptr, nullptr)) {
         // For (unmodified) library code we do not need to record encodings as we
         // rely on access to the binary during post-processing.

clients/drcachesim/tracer/raw2trace.cpp

@@ -349,17 +349,31 @@ module_mapper_t::do_encoding_parsing()
         dr_map_file(encoding_file_, &map_size, 0, NULL, DR_MEMPROT_READ, 0));
     if (map_start == nullptr || map_size < file_size)
         return "Failed to map encoding file";
-    if (*reinterpret_cast<uint64_t *>(map_start) != ENCODING_FILE_VERSION)
+    byte *map_at = map_start;
+    byte *map_end = map_start + file_size;
+    uint64_t encoding_file_version = *reinterpret_cast<uint64_t *>(map_at);
+    map_at += sizeof(uint64_t);
+    if (encoding_file_version > ENCODING_FILE_VERSION)
         return "Encoding file has invalid version";
-    size_t offs = sizeof(uint64_t);
-    while (offs < file_size) {
-        encoding_entry_t *entry = reinterpret_cast<encoding_entry_t *>(map_start + offs);
-        if (entry->length < sizeof(encoding_entry_t))
+    if (encoding_file_version >= ENCODING_FILE_VERSION_HAS_FILE_TYPE) {
+        if (map_at + sizeof(uint64_t) > map_end)
+            return "Encoding file header is truncated";
+        uint64_t encoding_file_type = *reinterpret_cast<uint64_t *>(map_at);
+        map_at += sizeof(uint64_t);
+        separate_non_mod_instrs_ =
+            TESTANY(ENCODING_FILE_TYPE_SEPARATE_NON_MOD_INSTRS, encoding_file_type);
+    }
+    uint64_t cumulative_encoding_length = 0;
+    while (map_at < map_end) {
+        encoding_entry_t *entry = reinterpret_cast<encoding_entry_t *>(map_at);
+        if (entry->length <= sizeof(encoding_entry_t))
             return "Encoding file is corrupted";
-        if (offs + entry->length > file_size)
+        if (map_at + entry->length > map_end)
             return "Encoding file is truncated";
+        cum_block_enc_len_to_encoding_id_[cumulative_encoding_length] = entry->id;
+        cumulative_encoding_length += (entry->length - sizeof(encoding_entry_t));
         encodings_[entry->id] = entry;
-        offs += entry->length;
+        map_at += entry->length;
     }
     return "";
 }