i#7729: Fix incorrect drmemtrace addr for stolen reg (#7730)

Fixes a bug where drmemtrace records the wrong address for a load with
no index and a base of the stolen register.

Adds a test case to burst_aarch64_sys which fails without the fix.

Adds a drutil base reg test case to the stolen-reg-index test as well,
though drutil operates correctly here.

Fixes #7729

Author: derekbruening

clients/drcachesim/tests/burst_aarch64_sys.cpp

@@ -119,12 +119,32 @@ ic_unprivileged_flush()
     __asm__ __volatile__("ic ivau , %0" : : "r"(ic_unprivileged_flush));
 }
 
+static int64_t global_var[2];
+constexpr int SENTINEL_MOV = 12345;
+
+static void
+stolen_reg_load()
+{
+    // Test the stolen register as a base reg of a load (test i#7729).
+    assert(dr_get_stolen_reg() == DR_REG_X28);
+    // Use adrp for the page and the ":lo12:" pattern for the offset to
+    // get &global_var[1] into the x28 register.
+    __asm__ __volatile__("adrp x28, %0\n\t"
+                         "add x28, x28, #:lo12:%0\n\t"
+                         "mov x0, #%1\n\n"  // Marker.
+                         "ldr x0, [x28, 8]" // Test a non-zero offset.
+                         :
+                         : "S"(&global_var), "i"(SENTINEL_MOV)
+                         : "x0", "x28");
+}
+
 static void
 do_some_work()
 {
     dc_zva();
     dc_unprivileged_flush();
     ic_unprivileged_flush();
+    stolen_reg_load();
 
     // Testing privileged instructions requires handling SIGILL.
     // We use sigsetjmp/siglongjmp to resume execution after handling the
@@ -214,6 +234,39 @@ is_dc_zva_instr(void *dr_context, memref_t memref)
     return is_dc_zva;
 }
 
+bool
+is_mov_immed_instr(void *dr_context, memref_t memref, int64_t &immed)
+{
+    if (!type_is_instr(memref.instr.type))
+        return false;
+    app_pc pc = (app_pc)memref.instr.addr;
+    instr_t instr;
+    instr_init(dr_context, &instr);
+    auto *ret = decode(dr_context, pc, &instr);
+    assert(ret != NULL && instr_valid(&instr));
+    bool is_mov_immed = instr_is_mov_constant(&instr, &immed);
+    instr_free(dr_context, &instr);
+    return is_mov_immed;
+}
+
+bool
+is_stolen_reg_load_instr(void *dr_context, memref_t memref)
+{
+    if (!type_is_instr(memref.instr.type))
+        return false;
+    app_pc pc = (app_pc)memref.instr.addr;
+    instr_t instr;
+    instr_init(dr_context, &instr);
+    auto *ret = decode(dr_context, pc, &instr);
+    assert(ret != NULL && instr_valid(&instr));
+    bool res = instr_reads_memory(&instr) &&
+        opnd_is_base_disp(instr_get_src(&instr, 0)) &&
+        opnd_get_base(instr_get_src(&instr, 0)) == dr_get_stolen_reg() &&
+        opnd_get_index(instr_get_src(&instr, 0)) == DR_REG_NULL;
+    instr_free(dr_context, &instr);
+    return res;
+}
+
 int
 test_main(int argc, const char *argv[])
 {
@@ -237,6 +290,9 @@ test_main(int argc, const char *argv[])
     int dc_zva_instr_count = 0;
     int dc_zva_memref_count = 0;
     addr_t last_dc_zva_pc = 0;
+    bool prev_instr_was_mov_sentinel = false;
+    bool prev_instr_was_stolen_reg_load = false;
+    bool found_stolen_reg_global_var = false;
     auto *stream = scheduler.get_stream(0);
     memref_t memref;
     for (scheduler_t::stream_status_t status = stream->next_record(memref);
@@ -257,6 +313,19 @@ test_main(int argc, const char *argv[])
             last_dc_zva_pc = memref.instr.addr;
             continue;
         }
+        if (type_is_instr(memref.instr.type)) {
+            int64_t immed = 0;
+            if (is_mov_immed_instr(dr_context, memref, immed) && immed == SENTINEL_MOV) {
+                prev_instr_was_mov_sentinel = true;
+            } else {
+                if (prev_instr_was_mov_sentinel &&
+                    is_stolen_reg_load_instr(dr_context, memref)) {
+                    prev_instr_was_stolen_reg_load = true;
+                } else
+                    prev_instr_was_stolen_reg_load = false;
+                prev_instr_was_mov_sentinel = false;
+            }
+        }
         // Look for _memref_data_t entries.
         if ((memref.data.type == TRACE_TYPE_READ ||
              memref.data.type == TRACE_TYPE_WRITE || type_is_prefetch(memref.data.type))
@@ -267,6 +336,14 @@ test_main(int argc, const char *argv[])
             assert(ALIGNED(memref.data.addr, proc_get_cache_line_size()));
             assert(memref.data.size == proc_get_cache_line_size());
         }
+        if (memref.data.type == TRACE_TYPE_READ && prev_instr_was_stolen_reg_load) {
+            if (memref.data.addr == reinterpret_cast<addr_t>(&global_var[1])) {
+                found_stolen_reg_global_var = true;
+            } else {
+                std::cerr << "Found stolen reg load of 0x" << std::hex << memref.data.addr
+                          << " vs expected " << &global_var[1] << std::dec << "\n";
+            }
+        }
     }
     std::cerr << "DC ZVA count: " << dc_zva_instr_count << "\n";
     std::cerr << "DC ZVA memref count: " << dc_zva_memref_count << "\n";
@@ -275,6 +352,7 @@ test_main(int argc, const char *argv[])
     assert(dc_zva_instr_count == dc_zva_memref_count);
     assert(found_cache_line_size_marker);
     assert(found_page_size_marker);
+    assert(found_stolen_reg_global_var);
     dr_standalone_exit();
 
     return 0;

clients/drcachesim/tracer/instru_offline.cpp

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2016-2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2016-2025 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -645,9 +645,10 @@ offline_instru_t::insert_save_addr(void *drcontext, instrlist_t *ilist, instr_t
          * store the base reg directly and add the disp during post-processing.
          */
         reg_addr = opnd_get_base(ref);
-        if (opnd_get_base(ref) == reg_ptr) {
-            /* Here we do need a scratch reg, and raw2trace can't identify this case:
-             * so we set disp to 0 and use the regular path below.
+        if (opnd_get_base(ref) == reg_ptr || opnd_get_base(ref) == dr_get_stolen_reg()) {
+            /* Here we do need a scratch reg, and raw2trace can't identify these cases:
+             * so we set disp to 0 (since raw2trace will add it on) and use the
+             * regular path below.
              */
             opnd_set_disp(&ref, 0);
         } else

suite/tests/client-interface/stolen-reg-index.c

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2023-2025 Google, Inc.  All rights reserved.
  * Copyright (c) 2022 Arm Limited   All rights reserved.
  * **********************************************************/
 
@@ -44,6 +44,8 @@
 /* In asm code. */
 void
 indexed_mem_test(int *val);
+void
+base_mem_test(int *val);
 
 int
 main(int argc, char *argv[])
@@ -56,6 +58,14 @@ main(int argc, char *argv[])
         print("indexed_mem_test() passed.\n");
 
     print("Tested the use of stolen register as memory index register.\n");
+
+    base_mem_test(&value);
+    if (value != 43)
+        print("base_mem_test() failed with %d, expected 43.\n", value);
+    else
+        print("base_mem_test() passed.\n");
+    print("Tested the use of stolen register as memory base register.\n");
+
     return 0;
 }
 
@@ -84,6 +94,27 @@ GLOBAL_LABEL(FUNCNAME:)
         END_FUNC(FUNCNAME)
 #  undef FUNCNAME
 
+
+#define FUNCNAME base_mem_test
+        DECLARE_EXPORTED_FUNC(FUNCNAME)
+GLOBAL_LABEL(FUNCNAME:)
+
+        stp      x0, x1, [sp, #-16]!
+
+        /* Load passed in value using index register X28, then increment. */
+        mov      x28, x0
+        ldr      x1, [x28]
+        add      x1, x1, #1
+
+        /* Store incremented value using index register W28. */
+        str      x1, [x28]
+
+        ldp      x0, x1, [sp], #16
+        ret
+
+        END_FUNC(FUNCNAME)
+#  undef FUNCNAME
+
 END_FILE
 /* clang-format on */
 #endif /* ASM_CODE_ONLY */

suite/tests/client-interface/stolen-reg-index.dll.c

@@ -1,4 +1,5 @@
 /* **********************************************************
+ * Copyright (c) 2025 Google, Inc.  All rights reserved.
  * Copyright (c) 2022 Arm Limited   All rights reserved.
  * **********************************************************/
 
@@ -89,12 +90,14 @@ insert_get_addr(void *drcontext, instrlist_t *ilist, instr_t *instr, opnd_t mref
     }
 
     /* Look for the precise stolen register cases in the test app. */
-    if (opnd_get_base(mref) == DR_REG_X0 &&
-        (opnd_get_index(mref) == dr_get_stolen_reg() ||
-         opnd_get_index(mref) == DR_REG_W28 && opnd_get_base(mref) == DR_REG_X0)) {
+    if (opnd_get_base(mref) == dr_get_stolen_reg() ||
+        (opnd_get_base(mref) == DR_REG_X0 &&
+         reg_to_pointer_sized(opnd_get_index(mref)) == dr_get_stolen_reg())) {
         /* Call out to confirm we got the right address.
          * DR's clean call args only support pointer-sized so we
-         * deconstruct the opnd_t.
+         * deconstruct the opnd_t, which works if it fits in our 2 args
+         * as we pass it to ourselves so we know the layout is identical
+         * for the reading code.
          */
         DR_ASSERT(sizeof(mref) <= 2 * sizeof(ptr_uint_t));
         ptr_uint_t opnd_top = *(((ptr_uint_t *)&mref) + 1);
@@ -143,6 +146,13 @@ event_app_instruction(void *drcontext, void *tag, instrlist_t *bb, instr_t *inst
             if (opnd_get_index(opnd) == DR_REG_W28 && opnd_get_base(opnd) == DR_REG_X0 &&
                 prev_is_mov_0(inst, DR_REG_W28))
                 dr_printf("store memref with index reg W28\n");
+            if (insert_get_addr(drcontext, bb, inst, opnd))
+                return DR_EMIT_DEFAULT;
+            else
+                DR_ASSERT(false);
+        } else if (opnd_is_memory_reference(opnd) && opnd_is_base_disp(opnd) &&
+                   opnd_get_base(opnd) == dr_get_stolen_reg()) {
+            /* Go ahead and translate all all base-stolen addresses. */
             if (insert_get_addr(drcontext, bb, inst, instr_get_dst(inst, 0)))
                 return DR_EMIT_DEFAULT;
             else
@@ -162,7 +172,14 @@ event_app_instruction(void *drcontext, void *tag, instrlist_t *bb, instr_t *inst
             if (opnd_get_index(opnd) == DR_REG_W28 && opnd_get_base(opnd) == DR_REG_X0 &&
                 prev_is_mov_0(inst, DR_REG_W28))
                 dr_printf("load memref with index reg W28\n");
-            if (insert_get_addr(drcontext, bb, inst, instr_get_src(inst, 0)))
+            if (insert_get_addr(drcontext, bb, inst, opnd))
+                return DR_EMIT_DEFAULT;
+            else
+                DR_ASSERT(false);
+        } else if (opnd_is_memory_reference(opnd) && opnd_is_base_disp(opnd) &&
+                   opnd_get_base(opnd) == dr_get_stolen_reg()) {
+            /* Go ahead and translate all all base-stolen addresses. */
+            if (insert_get_addr(drcontext, bb, inst, opnd))
                 return DR_EMIT_DEFAULT;
             else
                 DR_ASSERT(false);

suite/tests/client-interface/stolen-reg-index.expect

@@ -2,3 +2,5 @@ load memref with index reg X28
 store memref with index reg W28
 indexed_mem_test() passed.
 Tested the use of stolen register as memory index register.
+base_mem_test() passed.
+Tested the use of stolen register as memory base register.