Merge branch 'master' into iX-disassemble-identify-evex-mask-opnd-cor…

…rectly

api/docs/release.dox

@@ -129,7 +129,8 @@ changes:
  - No compatibility changes yet.
 
 Further non-compatibility-affecting changes include:
- - No changes yet.
+ - Added instr_get_operation_size() and instr_set_operation_size() APIs for
+   #DR_ISA_REGDEPS #instr_t.
 
 **************************************************
 <hr>

clients/drcachesim/tests/invariant_checker_test.cpp

@@ -3581,6 +3581,49 @@ check_regdeps(void)
 {
     std::cerr << "Testing regdeps traces\n";
 
+    // Incorrect: TRACE_MARKER_TYPE_SIGNAL_NUMBER not allowed.
+    {
+        std::vector<memref_t> memrefs = {
+            gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ARCH_REGDEPS),
+            gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64),
+            gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096),
+            gen_marker(TID_A, TRACE_MARKER_TYPE_SIGNAL_NUMBER, 42),
+            gen_instr(TID_A),
+            gen_exit(TID_A),
+        };
+        if (!run_checker(
+                memrefs, true,
+                { "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have "
+                  "TRACE_MARKER_TYPE_SIGNAL_NUMBER markers",
+                  /*tid=*/TID_A,
+                  /*ref_ordinal=*/4, /*last_timestamp=*/0,
+                  /*instrs_since_last_timestamp=*/0 },
+                "Failed to catch non-allowed TRACE_MARKER_TYPE_SIGNAL_NUMBER marker"))
+            return false;
+    }
+
+    // Incorrect: TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION not allowed.
+    // XXX i#7155: Allow these markers once we update their values in record_filter.
+    {
+        std::vector<memref_t> memrefs = {
+            gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ARCH_REGDEPS),
+            gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64),
+            gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096),
+            gen_marker(TID_A, TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION, 42),
+            gen_instr(TID_A),
+            gen_exit(TID_A),
+        };
+        if (!run_checker(memrefs, true,
+                         { "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have "
+                           "TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION markers",
+                           /*tid=*/TID_A,
+                           /*ref_ordinal=*/4, /*last_timestamp=*/0,
+                           /*instrs_since_last_timestamp=*/0 },
+                         "Failed to catch non-allowed "
+                         "TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION marker"))
+            return false;
+    }
+
     // Incorrect: OFFLINE_FILE_TYPE_ARCH_AARCH64 not allowed.
     {
         std::vector<memref_t> memrefs = {

clients/drcachesim/tools/invariant_checker.cpp

@@ -1707,6 +1707,17 @@ invariant_checker_t::check_regdeps_invariants(per_shard_t *shard, const memref_t
                          "was not built on a Linux platform.\n";
 #endif
         } break;
+        case TRACE_MARKER_TYPE_SIGNAL_NUMBER:
+            report_if_false(shard, false,
+                            "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have "
+                            "TRACE_MARKER_TYPE_SIGNAL_NUMBER markers");
+            break;
+        // XXX i#7155: Allow these markers once we update their values in record_filter.
+        case TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION:
+            report_if_false(shard, false,
+                            "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have "
+                            "TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION markers");
+            break;
         default:
             // All other markers are allowed.
             break;

core/arch/aarchxx/mangle.c

@@ -596,18 +596,18 @@ insert_push_all_registers(dcontext_t *dcontext, clean_call_info_t *cci,
     PRE(ilist, instr,
         INSTR_CREATE_vstmdb(dcontext, OPND_CREATE_MEMLIST(DR_REG_SP), SIMD_REG_LIST_LEN,
                             SIMD_REG_LIST_0_15));
-
     dstack_offs += proc_num_simd_registers() * sizeof(dr_simd_t);
     ASSERT(proc_num_simd_registers() == MCXT_NUM_SIMD_SLOTS);
+    ASSERT(ALIGNED(dstack_offs, get_ABI_stack_alignment()));
 
     /* pc and aflags */
     if (cci->skip_save_flags) {
         /* even if we skip flag saves we want to keep mcontext shape */
         int offs_beyond_xmm = 2 * XSP_SZ;
-        dstack_offs += offs_beyond_xmm;
         PRE(ilist, instr,
             XINST_CREATE_sub(dcontext, opnd_create_reg(DR_REG_SP),
                              OPND_CREATE_INT(offs_beyond_xmm)));
+        dstack_offs += offs_beyond_xmm;
     } else {
         uint slot = TLS_REG0_SLOT;
         bool spill = scratch == REG_NULL;
@@ -633,39 +633,49 @@ insert_push_all_registers(dcontext_t *dcontext, clean_call_info_t *cci,
             PRE(ilist, instr,
                 XINST_CREATE_load_int(dcontext, opnd_create_reg(scratch), push_pc));
             PRE(ilist, instr, INSTR_CREATE_push(dcontext, opnd_create_reg(scratch)));
+            dstack_offs += XSP_SZ;
         } else {
             ASSERT(opnd_is_reg(push_pc));
             PRE(ilist, instr, INSTR_CREATE_push(dcontext, push_pc));
+            dstack_offs += XSP_SZ;
         }
         if (spill)
             PRE(ilist, instr, instr_create_restore_from_tls(dcontext, scratch, slot));
-        dstack_offs += XSP_SZ;
     }
+    ASSERT(ALIGNED(dstack_offs, get_ABI_stack_alignment()));
 
     /* We rely on dr_get_mcontext_priv() to fill in the app's stolen reg value
      * and sp value.
      */
     if (dr_get_isa_mode(dcontext) == DR_ISA_ARM_THUMB) {
         /* We can't use sp with stm */
         PRE(ilist, instr, INSTR_CREATE_push(dcontext, opnd_create_reg(DR_REG_LR)));
+        dstack_offs += XSP_SZ;
         /* We can't push sp w/ writeback, and in fact dr_get_mcontext() gets
          * sp from the stack swap so we can leave this empty.
          */
         PRE(ilist, instr,
             XINST_CREATE_sub(dcontext, opnd_create_reg(DR_REG_SP),
                              OPND_CREATE_INT(XSP_SZ)));
+        dstack_offs += XSP_SZ;
         PRE(ilist, instr,
             INSTR_CREATE_stmdb_wb(dcontext, OPND_CREATE_MEMLIST(DR_REG_SP),
                                   DR_REG_LIST_LENGTH_T32, DR_REG_LIST_T32));
+        dstack_offs += DR_REG_LIST_LENGTH_T32 * XSP_SZ;
     } else {
         PRE(ilist, instr,
             INSTR_CREATE_stmdb_wb(dcontext, OPND_CREATE_MEMLIST(DR_REG_SP),
                                   DR_REG_LIST_LENGTH_ARM, DR_REG_LIST_ARM));
+        dstack_offs += DR_REG_LIST_LENGTH_ARM * XSP_SZ;
     }
-    dstack_offs += 15 * XSP_SZ;
-
-    /* Make dstack_offs 8-byte algined, as we only accounted for 17 4-byte slots. */
+    /* Make dstack_offs 8-byte aligned as we have just pushed an odd
+     * number of 4-byte registers.
+     */
+    PRE(ilist, instr,
+        XINST_CREATE_sub(dcontext, opnd_create_reg(DR_REG_SP), OPND_CREATE_INT(XSP_SZ)));
     dstack_offs += XSP_SZ;
+    ASSERT(ALIGNED(dstack_offs, get_ABI_stack_alignment()));
+
 #endif
     ASSERT(cci->skip_save_flags || cci->num_simd_skip != 0 || cci->num_regs_skip != 0 ||
            dstack_offs == (uint)get_clean_call_switch_stack_size());
@@ -769,6 +779,9 @@ insert_pop_all_registers(dcontext_t *dcontext, clean_call_info_t *cci, instrlist
     }
 
 #else
+    /* This undoes the XINST_CREATE_sub done for alignment in XINST_CREATE_sub. */
+    PRE(ilist, instr,
+        XINST_CREATE_add(dcontext, opnd_create_reg(DR_REG_SP), OPND_CREATE_INT(XSP_SZ)));
     /* We rely on dr_set_mcontext_priv() to set the app's stolen reg value,
      * and the stack swap to set the sp value: we assume the stolen reg on
      * the stack still has our TLS base in it.

core/ir/instr_api.h

@@ -1995,6 +1995,22 @@ DR_API
 instr_t *
 instr_convert_short_meta_jmp_to_long(void *drcontext, instrlist_t *ilist, instr_t *instr);
 
+DR_API
+/**
+ * Returns the operation size of \p instr if it's a #DR_ISA_REGDEPS instruction, #OPSZ_NA
+ * otherwise.
+ */
+opnd_size_t
+instr_get_operation_size(instr_t *instr);
+
+DR_API
+/**
+ * Sets the operation size of \p instr to \p operation_size only if instr is a
+ * #DR_ISA_REGDEPS instruction.
+ */
+void
+instr_set_operation_size(instr_t *instr, opnd_size_t operation_size);
+
 DR_API
 /**
  * Converts a real ISA (e.g., #DR_ISA_AMD64) instruction \p instr_real_isa into a

core/ir/instr_shared.c

@@ -2999,6 +2999,27 @@ instr_uses_fp_reg(instr_t *instr)
     return false;
 }
 
+opnd_size_t
+instr_get_operation_size(instr_t *instr)
+{
+    /* Only DR_ISA_REGDEPS instructions can have an operation_size.
+     */
+    if (instr_get_isa_mode(instr) == DR_ISA_REGDEPS)
+        return instr->operation_size;
+
+    return OPSZ_NA;
+}
+
+void
+instr_set_operation_size(instr_t *instr, opnd_size_t operation_size)
+{
+    CLIENT_ASSERT(instr_get_isa_mode(instr) != DR_ISA_REGDEPS,
+                  "instr_set_operation_size: only DR_ISA_REGDEPS instructions can have "
+                  "an operation_size");
+    instr->operation_size = operation_size;
+    return;
+}
+
 void
 instr_convert_to_isa_regdeps(void *drcontext, instr_t *instr_real_isa,
                              instr_t *instr_regdeps_isa)
@@ -3097,10 +3118,14 @@ instr_convert_to_isa_regdeps(void *drcontext, instr_t *instr_real_isa,
     instr_set_category(instr_regdeps_isa, instr_get_category(instr_real_isa));
 
     /* Convert max_src_opnd_size_bytes from number of bytes to opnd_size_t (which holds
-     * OPSZ_ enum values).
+     * OPSZ_ enum values). If we have no operands, operation_size is OPSZ_0, same as a
+     * decoded DR_ISA_REGDEPS instruction.
      */
     opnd_size_t max_opnd_size = opnd_size_from_bytes(max_src_opnd_size_bytes);
-    instr_regdeps_isa->operation_size = max_opnd_size;
+    if (num_dsts + num_srcs > 0)
+        instr_regdeps_isa->operation_size = max_opnd_size;
+    else
+        instr_regdeps_isa->operation_size = OPSZ_0;
 
     /* Set the source and destination register operands for the converted instruction.
      */

suite/tests/CMakeLists.txt

@@ -4838,43 +4838,29 @@ if (BUILD_CLIENTS)
 
     if (X86 AND X64 AND UNIX AND NOT APPLE)
         # Run the invariant_checker on an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace of
-        # ci_shared_app.
+        # ci_shared_app and kernel_xfer_app. We expect no invariant errors.
+        # Generate an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace by running record_filter
+        # with: 1) -filter_encodings2regdeps to change instruction encodings; 2)
+        # -filter_keep_func_ids 4294967498 (which is SYS_futex, associated to the only
+        # TRACE_MARKER_TYPE_FUNC_* markers we want to keep),
+        # -filter_marker_types 19,25,27,28,30,40,41 (which correspond to
+        # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL,
+        # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END,
+        # TRACE_MARKER_TYPE_SYSCALL_FAILED, TRACE_MARKER_TYPE_SIGNAL_NUMBER,
+        # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION), note that the removal of
+        # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION is only temporary (xref i#7155);
+        # 3) -filter_modify_marker_value 3,-1 (which changes the value of
+        # TRACE_MARKER_TYPE_CPU_ID == 3 to INVALID_CPU_MARKER_VALUE == (uintptr_t)-1).
         set(testname "tool.invariant_checker_on_regdeps_trace_ci_shared_app")
         torun_record_filter("${testname}" ${ci_shared_app}
           "invariant_checker_on_regdeps_trace_ci_shared_app"
-          # Generate an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace by running record_filter
-          # with -filter_encodings2regdeps to change instruction encodings,
-          # -filter_keep_func_ids 4294967498 (which is SYS_futex, associated to the only
-          # TRACE_MARKER_TYPE_FUNC_ markers we want to keep),
-          # -filter_marker_types 19,25,27,28,30 (which correspond to
-          # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL,
-          # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END,
-          # TRACE_MARKER_TYPE_SYSCALL_FAILED), and -filter_modify_marker_value 3,-1
-          # (which changes the value of TRACE_MARKER_TYPE_CPU_ID == 3 to
-          # INVALID_CPU_MARKER_VALUE == (uintptr_t)-1).
-          "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${ci_shared_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1"
-          # We run the invariant_checker analyzer on the REGDEPS filtered trace.
-          # We expect no invariant errors.
+          "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${ci_shared_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30,40,41@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1"
           "invariant_checker")
 
-        # Run the invariant_checker on an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace of
-        # kernel_xfer_app.
         set(testname "tool.invariant_checker_on_regdeps_trace_kernel_xfer_app")
         torun_record_filter("${testname}" ${kernel_xfer_app}
           "invariant_checker_on_regdeps_trace_kernel_xfer_app"
-          # Generate an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace by running record_filter
-          # with -filter_encodings2regdeps to change instruction encodings,
-          # -filter_keep_func_ids 4294967498 (which is SYS_futex, associated to the only
-          # TRACE_MARKER_TYPE_FUNC_ markers we want to keep),
-          # -filter_marker_types 19,25,27,28,30 (which correspond to
-          # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL,
-          # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END,
-          # TRACE_MARKER_TYPE_SYSCALL_FAILED), and -filter_modify_marker_value 3,-1
-          # (which changes the value of TRACE_MARKER_TYPE_CPU_ID == 3 to
-          # INVALID_CPU_MARKER_VALUE == (uintptr_t)-1).
-          "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1"
-          # We run the invariant_checker analyzer on the REGDEPS filtered trace.
-          # We expect no invariant errors.
+          "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30,40,41@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1"
           "invariant_checker")
     endif ()
 

suite/tests/api/ir_regdeps.c

@@ -108,11 +108,23 @@ test_instr_encode_decode_disassemble_synthetic(void *dc, instr_t *instr,
     byte *next_pc_decode = decode(dc, bytes, instr_synthetic_decoded);
     ASSERT(next_pc_decode != NULL);
     ASSERT(next_pc_encode == next_pc_decode);
+
+    /* Check instruction length.
+     */
+    ASSERT((next_pc_encode - bytes) == instr_length(dc, instr_synthetic_decoded));
+    ASSERT(instr_length(dc, instr_synthetic_decoded) != 0);
+
     /* Check for overflow.
      */
     ASSERT((next_pc_encode - bytes) <= sizeof(bytes));
     ASSERT((next_pc_decode - bytes) <= sizeof(bytes));
 
+    /* Check operation size.
+     */
+    ASSERT(instr_get_operation_size(instr_synthetic_decoded) != OPSZ_NA);
+    ASSERT(instr_get_operation_size(instr_synthetic_decoded) ==
+           instr_get_operation_size(instr_synthetic_converted));
+
     /* Disassemble regdeps synthetic encodings to buffer.
      */
     char dbuf[512];