Merge branch 'master' of github.com:DynamoRIO/dynamorio into i7224-sk…

…ip-docs

clients/drcachesim/scheduler/scheduler.h

@@ -267,6 +267,10 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
          * with no timeout but do not include a corresponding
          * #TRACE_MARKER_TYPE_SYSCALL_SCHEDULE for wakeup, an input could remain
          * unscheduled.
+         *
+         * Also beware that this can skip over trace header entries (like
+         * #TRACE_MARKER_TYPE_FILETYPE), which should ideally be obtained from the
+         * #dynamorio::drmemtrace::memtrace_stream_t API instead.
          */
         std::vector<range_t> regions_of_interest;
     };
@@ -1041,7 +1045,8 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
          * #TRACE_MARKER_TYPE_VERSION record in the trace header.
          * This can be queried prior to explicitly retrieving any records from
          * output streams, unless #dynamorio::drmemtrace::scheduler_tmpl_t::
-         * scheduler_options_t.read_inputs_in_init is false.
+         * scheduler_options_t.read_inputs_in_init is false (which is the
+         * case for online drmemtrace analysis).
          */
         uint64_t
         get_version() const override
@@ -1055,7 +1060,8 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
          * #TRACE_MARKER_TYPE_FILETYPE record in the trace header.
          * This can be queried prior to explicitly retrieving any records from
          * output streams, unless #dynamorio::drmemtrace::scheduler_tmpl_t::
-         * scheduler_options_t.read_inputs_in_init is false.
+         * scheduler_options_t.read_inputs_in_init is false (which is the
+         * case for online drmemtrace analysis).
          */
         uint64_t
         get_filetype() const override

clients/drcachesim/tests/offline-skip_instrs_opcode_mix.templatex

@@ -0,0 +1,8 @@
+Hello, world!
+Opcode mix tool results:
+    *[0-9]* : total executed instructions
+    *[0-9]* : [a-z ]*
+    *[0-9]* : [a-z ]*
+    *[0-9]* : [a-z ]*
+    *[0-9]* : [a-z ]*
+.*

clients/drcachesim/tests/opcode_mix_test.cpp

@@ -82,6 +82,22 @@ class test_opcode_mix_t : public opcode_mix_t {
     instrlist_t *instrs_;
 };
 
+class test_stream_t : public default_memtrace_stream_t {
+public:
+    test_stream_t(uint64_t filetype)
+        : filetype_(filetype)
+    {
+    }
+    uint64_t
+    get_filetype() const override
+    {
+        return filetype_;
+    }
+
+private:
+    uint64_t filetype_;
+};
+
 std::string
 check_opcode_mix(void *drcontext, bool use_module_mapper)
 {
@@ -92,10 +108,10 @@ check_opcode_mix(void *drcontext, bool use_module_mapper)
     instrlist_t *ilist = instrlist_create(drcontext);
     instrlist_append(ilist, nop);
     instrlist_append(ilist, ret);
+    uint64_t filetype = OFFLINE_FILE_TYPE_SYSCALL_NUMBERS |
+        (use_module_mapper ? 0 : OFFLINE_FILE_TYPE_ENCODINGS);
     std::vector<memref_with_IR_t> memref_setup = {
-        { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE,
-                     OFFLINE_FILE_TYPE_SYSCALL_NUMBERS |
-                         (use_module_mapper ? 0 : OFFLINE_FILE_TYPE_ENCODINGS)),
+        { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, static_cast<uintptr_t>(filetype)),
           nullptr },
         { gen_instr(TID_A), nop },
         { gen_instr(TID_A), ret },
@@ -116,9 +132,10 @@ check_opcode_mix(void *drcontext, bool use_module_mapper)
         // Set up the second nop memref to reuse the same encoding as the first nop.
         memrefs[3].instr.encoding_is_new = false;
     }
+    test_stream_t stream(filetype);
     test_opcode_mix_t opcode_mix(ilist_for_test);
-    opcode_mix.initialize();
-    void *shard_data = opcode_mix.parallel_shard_init_stream(0, nullptr, nullptr);
+    opcode_mix.initialize_stream(/*serial_stream=*/nullptr);
+    void *shard_data = opcode_mix.parallel_shard_init_stream(0, nullptr, &stream);
     for (const memref_t &memref : memrefs) {
         if (!opcode_mix.parallel_shard_memref(shard_data, memref)) {
             return opcode_mix.parallel_shard_error(shard_data);

clients/drcachesim/tools/common/decode_cache.h

@@ -423,15 +423,22 @@ template <class DecodeInfo> class decode_cache_t : public decode_cache_base_t {
      * indeed has embedded encodings or not, and initializing the
      * #dynamorio::drmemtrace::module_mapper_t if the module path is provided.
      *
-     * It is important to note that the trace filetype may be obtained using the
-     * get_filetype() API on a #dynamorio::drmemtrace::memtrace_stream_t. However,
-     * instances of #dynamorio::drmemtrace::memtrace_stream_t have the filetype
-     * available at init time (before the #TRACE_MARKER_TYPE_FILETYPE is actually
-     * returned by the stream) only for offline analysis. In the online analysis
-     * case, the user would need to call this API after init time when the
-     * #TRACE_MARKER_TYPE_FILETYPE marker is seen (which is fine as it occurs
-     * before any instr record). For tools intended for both offline and online
-     * analysis, following just the latter strategy would work fine.
+     * It is important to note some nuances in how the filetype can be obtained:
+     * - the trace filetype may be obtained using the get_filetype() API on the
+     *   #dynamorio::drmemtrace::memtrace_stream_t. However, instances of
+     *   #dynamorio::drmemtrace::memtrace_stream_t have the filetype available at
+     *   init time (in the #dynamorio::drmemtrace::analysis_tool_t::initialize()
+     *   or #dynamorio::drmemtrace::analysis_tool_t::initialize_stream()
+     *   functions) only for offline analysis, not for online analysis.
+     * - when using the -skip_instrs or -skip_timestamp analyzer options, all
+     *   initial header entries are skipped over. Therefore, the analysis tool
+     *   may not see a #TRACE_MARKER_TYPE_FILETYPE at all.
+     *
+     * The most reliable way to obtain the filetype (and call this init() API),
+     * would be to use #dynamorio::drmemtrace::memtrace_stream_t::get_filetype()
+     * just before processing the first instruction memref in the
+     * #dynamorio::drmemtrace::analysis_tool_t::process_memref() or
+     * #dynamorio::drmemtrace::analysis_tool_t::parallel_shard_memref() APIs.
      *
      * If the \p module_file_path parameter is not empty, it instructs the
      * #dynamorio::drmemtrace::decode_cache_t object that it should look for the

clients/drcachesim/tools/opcode_mix.cpp

@@ -85,6 +85,17 @@ bool
 opcode_mix_t::init_decode_cache(shard_data_t *shard, void *dcontext,
                                 offline_file_type_t filetype)
 {
+    // XXX: Perhaps this should be moved into decode_cache_t::init().
+    // We remove OFFLINE_FILE_TYPE_ARCH_REGDEPS from this check since DR_ISA_REGDEPS
+    // is not a real ISA and can coexist with any real architecture.
+    if (TESTANY(OFFLINE_FILE_TYPE_ARCH_ALL & ~OFFLINE_FILE_TYPE_ARCH_REGDEPS, filetype) &&
+        !TESTANY(build_target_arch_type(), filetype)) {
+        shard->error = std::string("Architecture mismatch: trace recorded on ") +
+            trace_arch_string(static_cast<offline_file_type_t>(filetype)) +
+            " but tool built for " + trace_arch_string(build_target_arch_type());
+        return false;
+    }
+
     shard->decode_cache =
         std::unique_ptr<decode_cache_t<opcode_data_t>>(new decode_cache_t<opcode_data_t>(
             dcontext,
@@ -100,9 +111,12 @@ opcode_mix_t::init_decode_cache(shard_data_t *shard, void *dcontext,
 }
 
 std::string
-opcode_mix_t::initialize()
+opcode_mix_t::initialize_stream(dynamorio::drmemtrace::memtrace_stream_t *serial_stream)
 {
     dcontext_.dcontext = dr_standalone_init();
+    if (serial_stream != nullptr) {
+        serial_shard_.stream = serial_stream;
+    }
     return "";
 }
 
@@ -125,6 +139,7 @@ opcode_mix_t::parallel_shard_init_stream(
     dynamorio::drmemtrace::memtrace_stream_t *shard_stream)
 {
     auto shard = new shard_data_t();
+    shard->stream = shard_stream;
     std::lock_guard<std::mutex> guard(shard_map_mutex_);
     shard_map_[shard_index] = shard;
     return reinterpret_cast<void *>(shard);
@@ -145,22 +160,7 @@ opcode_mix_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
 {
     shard_data_t *shard = reinterpret_cast<shard_data_t *>(shard_data);
     if (memref.marker.type == TRACE_TYPE_MARKER &&
-        memref.marker.marker_type == TRACE_MARKER_TYPE_FILETYPE) {
-        shard->filetype = static_cast<offline_file_type_t>(memref.marker.marker_value);
-        /* We remove OFFLINE_FILE_TYPE_ARCH_REGDEPS from this check since DR_ISA_REGDEPS
-         * is not a real ISA and can coexist with any real architecture.
-         */
-        if (TESTANY(OFFLINE_FILE_TYPE_ARCH_ALL & ~OFFLINE_FILE_TYPE_ARCH_REGDEPS,
-                    memref.marker.marker_value) &&
-            !TESTANY(build_target_arch_type(), memref.marker.marker_value)) {
-            shard->error = std::string("Architecture mismatch: trace recorded on ") +
-                trace_arch_string(static_cast<offline_file_type_t>(
-                    memref.marker.marker_value)) +
-                " but tool built for " + trace_arch_string(build_target_arch_type());
-            return false;
-        }
-    } else if (memref.marker.type == TRACE_TYPE_MARKER &&
-               memref.marker.marker_type == TRACE_MARKER_TYPE_VECTOR_LENGTH) {
+        memref.marker.marker_type == TRACE_MARKER_TYPE_VECTOR_LENGTH) {
 #ifdef AARCH64
         const int new_vl_bits = memref.marker.marker_value * 8;
         if (dr_get_vector_length() != new_vl_bits) {
@@ -177,18 +177,19 @@ opcode_mix_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
     }
 
     /* At this point we start processing memref instructions, so we're past the header of
-     * the trace, which contains the trace filetype. If we didn't encounter a
-     * TRACE_MARKER_TYPE_FILETYPE we return an error and stop processing the trace.
-     * We expected the value of TRACE_MARKER_TYPE_FILETYPE to contain at least one of the
+     * the trace, which contains the trace filetype. If -skip_instrs was used, we may not
+     * have seen TRACE_MARKER_TYPE_FILETYPE itself but the memtrace_stream_t should be
+     * able to provide it to us.
+     */
+    shard->filetype = static_cast<offline_file_type_t>(shard->stream->get_filetype());
+
+    /* We expected the value of TRACE_MARKER_TYPE_FILETYPE to contain at least one of the
      * enum values of offline_file_type_t (e.g., OFFLINE_FILE_TYPE_ARCH_), so
      * OFFLINE_FILE_TYPE_DEFAULT (== 0) should never be present alone and can be used as
      * uninitialized value of filetype for an error check.
      * XXX i#6812: we could allow traces that have some shards with no filetype, as long
      * as there is at least one shard with it, by caching the filetype from shards that
      * have it and using that one. We can do this using memtrace_stream_t::get_filetype(),
-     * which requires updating opcode_mix to use parallel_shard_init_stream() rather than
-     * the current (and deprecated) parallel_shard_init(). However, we should first decide
-     * whether we want to allow traces that have some shards without a filetype.
      */
     if (shard->filetype == OFFLINE_FILE_TYPE_DEFAULT) {
         shard->error = "No file type found in this shard";

clients/drcachesim/tools/opcode_mix.h

@@ -62,7 +62,7 @@ class opcode_mix_t : public analysis_tool_t {
                  const std::string &alt_module_dir = "");
     virtual ~opcode_mix_t();
     std::string
-    initialize() override;
+    initialize_stream(dynamorio::drmemtrace::memtrace_stream_t *serial_stream) override;
     bool
     process_memref(const memref_t &memref) override;
     bool
@@ -142,20 +142,14 @@ class opcode_mix_t : public analysis_tool_t {
 
     struct shard_data_t {
         shard_data_t()
-            : instr_count(0)
-            , last_trace_module_start(nullptr)
-            , last_trace_module_size(0)
-            , last_mapped_module_start(nullptr)
         {
         }
 
-        int64_t instr_count;
+        int64_t instr_count = 0;
         std::unordered_map<int, int64_t> opcode_counts;
         std::unordered_map<uint, int64_t> category_counts;
         std::string error;
-        app_pc last_trace_module_start;
-        size_t last_trace_module_size;
-        app_pc last_mapped_module_start;
+        dynamorio::drmemtrace::memtrace_stream_t *stream = nullptr;
         std::unique_ptr<decode_cache_t<opcode_data_t>> decode_cache;
         offline_file_type_t filetype = OFFLINE_FILE_TYPE_DEFAULT;
     };

clients/drcachesim/tracer/raw2trace_shared.cpp

@@ -287,8 +287,8 @@ module_mapper_t::read_and_map_modules()
             modvec_.push_back(module_t(info.path, info.start, NULL, 0, 0, 0));
         } else if (info.containing_index != info.index) {
             // For split segments, we assume our mapped layout matches the original.
-            byte *seg_map_base = modvec_[info.containing_index].map_seg_base +
-                (info.start - modvec_[info.containing_index].orig_seg_base);
+            byte *seg_map_base = modvec_[info.containing_index].map_seg_base -
+                modvec_[info.containing_index].orig_seg_base + info.start;
             VPRINT(1, "Secondary segment: module %d seg %p-%p = %s\n",
                    (int)modvec_.size(), seg_map_base, seg_map_base + info.size,
                    info.path);

clients/drcachesim/tracer/raw2trace_shared.h

@@ -286,7 +286,7 @@ class module_mapper_t {
                 reinterpret_cast<app_pc>(entry->start_pc);
         } else {
             size_t idx = static_cast<size_t>(modidx); // Avoid win32 warnings.
-            app_pc res = map_pc - modvec_[idx].map_seg_base + modvec_[idx].orig_seg_base;
+            app_pc res = modvec_[idx].orig_seg_base - modvec_[idx].map_seg_base + map_pc;
 #ifdef ARM
             // Match Thumb vs Arm mode by setting LSB.
             if (TESTANY(1, modoffs))
@@ -312,8 +312,8 @@ class module_mapper_t {
             size_t idx = static_cast<size_t>(modidx); // Avoid win32 warnings.
             // Cast to unsigned pointer-sized int first to avoid sign-extending.
             return reinterpret_cast<app_pc>(
-                       reinterpret_cast<ptr_uint_t>(modvec_[idx].orig_seg_base)) +
-                (modoffs - modvec_[idx].seg_offs);
+                reinterpret_cast<ptr_uint_t>(modvec_[idx].orig_seg_base) +
+                (modoffs - modvec_[idx].seg_offs));
         }
     }
 

core/drlibc/drlibc_module_elf.c

@@ -198,7 +198,7 @@ module_vaddr_from_prog_header(app_pc prog_header, uint num_segments,
             min_vaddr =
                 MIN(min_vaddr, (app_pc)ALIGN_BACKWARD(prog_hdr->p_vaddr, PAGE_SIZE));
             if (min_vaddr == (app_pc)prog_hdr->p_vaddr)
-                first_end = (app_pc)prog_hdr->p_vaddr + prog_hdr->p_memsz;
+                first_end = (app_pc)(prog_hdr->p_vaddr + prog_hdr->p_memsz);
             max_end = MAX(
                 max_end,
                 (app_pc)ALIGN_FORWARD(prog_hdr->p_vaddr + prog_hdr->p_memsz, PAGE_SIZE));
@@ -456,7 +456,7 @@ elf_loader_map_phdrs(elf_loader_t *elf, bool fixed, map_fn_t map_func,
              * (notably some kernels) seem to ignore it.  These corner cases are left
              * as unsolved for now.
              */
-            seg_base = (app_pc)ALIGN_BACKWARD(prog_hdr->p_vaddr, PAGE_SIZE) + delta;
+            seg_base = (app_pc)(ALIGN_BACKWARD(prog_hdr->p_vaddr, PAGE_SIZE) + delta);
             seg_end =
                 (app_pc)ALIGN_FORWARD(prog_hdr->p_vaddr + prog_hdr->p_filesz, PAGE_SIZE) +
                 delta;
@@ -522,7 +522,7 @@ elf_loader_map_phdrs(elf_loader_t *elf, bool fixed, map_fn_t map_func,
                     }
                     ASSERT(map != NULL);
                     /* fill zeros at extend size */
-                    file_end = (app_pc)prog_hdr->p_vaddr + prog_hdr->p_filesz;
+                    file_end = (app_pc)(prog_hdr->p_vaddr + prog_hdr->p_filesz);
                     if (seg_end > file_end + delta) {
                         /* There is typically one RW PT_LOAD segment for .data and
                          * .bss.  If .data ends and .bss starts before filesz bytes,

core/ir/aarch64/codec.c

@@ -1081,8 +1081,8 @@ static bool
 decode_opnd_adr_page(int scale, uint enc, byte *pc, OUT opnd_t *opnd)
 {
     uint bits = (enc >> 3 & 0x1ffffc) | (enc >> 29 & 3);
-    byte *addr = ((byte *)((ptr_uint_t)pc >> scale << scale) +
-                  extract_int(bits, 0, 21) * ((ptr_int_t)1 << scale));
+    byte *addr = (byte *)(((ptr_uint_t)pc >> scale << scale) +
+                          extract_int(bits, 0, 21) * ((ptr_int_t)1 << scale));
     *opnd = opnd_create_rel_addr(addr, OPSZ_0);
     return true;
 }

core/ir/instrlist.c

@@ -572,7 +572,7 @@ instrlist_encode_to_copy(void *drcontext, instrlist_t *ilist, byte *copy_pc,
         byte *pc = instr_encode_to_copy(dcontext, inst, copy_pc, final_pc);
         if (pc == NULL)
             return NULL;
-        final_pc += pc - copy_pc;
+        final_pc = (byte *)((ptr_uint_t)final_pc + (pc - copy_pc));
         copy_pc = pc;
     }
     return copy_pc;

core/ir/x86/encode.c

@@ -3012,7 +3012,7 @@ encode_cti(instr_t *instr, byte *copy_pc, byte *final_pc,
         CLIENT_ASSERT(!instr_is_cti_short_rewrite(instr, NULL),
                       "encode_cti error: jecxz/loop already mangled");
         /* offset is from start of next instr */
-        offset = target - ((ptr_int_t)(pc + 1 - copy_pc + final_pc));
+        offset = target - (pc + 1 - copy_pc + (ptr_uint_t)final_pc);
         if (check_reachable && !(offset >= INT8_MIN && offset <= INT8_MAX)) {
             CLIENT_ASSERT(!assert_reachable,
                           "encode_cti error: target beyond 8-bit reach");
@@ -3023,7 +3023,7 @@ encode_cti(instr_t *instr, byte *copy_pc, byte *final_pc,
     } else {
         /* 32-bit offset */
         /* offset is from start of next instr */
-        ptr_int_t offset = target - ((ptr_int_t)(pc + 4 - copy_pc + final_pc));
+        ptr_int_t offset = target - (pc + 4 - copy_pc + (ptr_uint_t)final_pc);
 #ifdef X64
         if (check_reachable && !REL32_REACHABLE_OFFS(offset)) {
             CLIENT_ASSERT(!assert_reachable,

ext/drcovlib/modules.c

@@ -467,7 +467,8 @@ module_read_entry_print(module_read_entry_t *entry, uint idx, char *buf, size_t
     len = dr_snprintf(buf, size,
                       "%3u, %3u, " PFX ", " PFX ", " PFX ", " ZHEX64_FORMAT_STRING
                       ", " PFX ", ",
-                      idx, entry->containing_id, entry->base, entry->base + entry->size,
+                      idx, entry->containing_id, entry->base,
+                      (app_pc)((ptr_uint_t)entry->base + (ptr_uint_t)entry->size),
                       entry->entry, entry->offset, entry->preferred_base);
     if (len == -1)
         return -1;

suite/tests/CMakeLists.txt

@@ -4452,6 +4452,7 @@ if (BUILD_CLIENTS)
       # TODO i#3544: Port tests to RISC-V 64
       torunonly_drcacheoff(opcode_mix ${ci_shared_app} ""
         "@-tool@opcode_mix" "")
+
       # Ensure the tool works without the raw/ subdir.
       set(tool.drcacheoff.opcode_mix_postcmd
         "firstglob@${drraw2trace_path}@-indir@${dir_prefix}.*.dir")
@@ -4460,6 +4461,9 @@ if (BUILD_CLIENTS)
       set(tool.drcacheoff.opcode_mix_postcmd3
         "firstglob@${drcachesim_path}@-indir@${dir_prefix}.*.dir@-tool@opcode_mix")
 
+      torunonly_drcacheoff(skip_instrs_opcode_mix ${ci_shared_app} ""
+        "@-tool@opcode_mix@-skip_instrs@1" "")
+
       torunonly_drcacheoff(view ${ci_shared_app} ""
         "@-tool@view@-sim_refs@16384" "")
       unset(tool.drcacheoff.view_rawtemp) # Use preprocessor