Widen modidx field in offline_entry_t.

clients/drcachesim/common/trace_entry.h

@@ -298,7 +298,12 @@ typedef enum {
 #define EXT_VALUE_A_BITS 48
 #define EXT_VALUE_B_BITS 8
 
-#define OFFLINE_FILE_VERSION 1
+#define PC_MODOFFS_BITS 33
+#define PC_MODIDX_BITS 16
+#define PC_INSTR_COUNT_BITS 12
+#define PC_TYPE_BITS 3
+
+#define OFFLINE_FILE_VERSION 2
 
 START_PACKED_STRUCTURE
 struct _offline_entry_t {
@@ -311,10 +316,10 @@ struct _offline_entry_t {
         } addr;
         struct {
             // This describes the entire basic block.
-            uint64_t modoffs:33;
-            uint64_t modidx:12;
-            uint64_t instr_count:16;
-            uint64_t type:3;
+            uint64_t modoffs:PC_MODOFFS_BITS;
+            uint64_t modidx:PC_MODIDX_BITS;
+            uint64_t instr_count:PC_INSTR_COUNT_BITS;
+            uint64_t type:PC_TYPE_BITS;
         } pc;
         struct {
             uint64_t tid:61;

clients/drcachesim/tracer/instru_offline.cpp

@@ -339,8 +339,13 @@ offline_instru_t::insert_save_pc(void *drcontext, instrlist_t *ilist, instr_t *w
     offline_entry_t entry;
     entry.pc.type = OFFLINE_TYPE_PC;
     // We put the ARM vs Thumb mode into the modoffs to ensure proper decoding.
-    entry.pc.modoffs =
-        dr_app_pc_as_jump_target(instr_get_isa_mode(where), pc) - modbase;
+    uint64_t modoffs = dr_app_pc_as_jump_target(instr_get_isa_mode(where), pc) - modbase;
+    // 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.
+    DR_ASSERT(modoffs < uint64_t(1) << PC_MODOFFS_BITS);
+    DR_ASSERT(modidx < uint64_t(1) << PC_MODIDX_BITS);
+    DR_ASSERT(instr_count < uint64_t(1) << PC_INSTR_COUNT_BITS);
+    entry.pc.modoffs = modoffs;
     entry.pc.modidx = modidx;
     entry.pc.instr_count = instr_count;
     return insert_save_entry(drcontext, ilist, where, reg_ptr, scratch, adjust, &entry);

clients/drcachesim/tracer/tracer.cpp

@@ -1785,10 +1785,13 @@ drmemtrace_client_main(client_id_t id, int argc, const char *argv[])
      * instructions accessing memory once, which is fairly
      * pathological as by default that's 256 memrefs for one bb.  We double
      * it to ensure we cover skipping clean calls for sthg like strex.
+     * We also check here that the max_bb_instrs can fit in the instr_count
+     * bitfield in offline_entry_t.
      */
     uint64 max_bb_instrs;
     if (!dr_get_integer_option("max_bb_instrs", &max_bb_instrs))
         max_bb_instrs = 256; /* current default */
+    DR_ASSERT(max_bb_instrs < uint64(1) << PC_INSTR_COUNT_BITS);
     redzone_size = instru->sizeof_entry() * (size_t)max_bb_instrs * 2;
 
     max_buf_size = ALIGN_FORWARD(trace_buf_size + redzone_size, dr_page_size());