i#3544 core, part 1: Use semantic register names

When code uses register names or offsets directly it is hard to follow
whether a given register has a special purpose or any scratch register
can be used.
It also results in IF_<arch>_ELSE ladders which become less readable
with each new architecture added.

This commit proposes potential solution to this: choose semantic
register names and shows several use-cases:
1. core/translate.c: Unit tests for IBL used DR_REG_XCX/R2 directly
   where TBL_TARGET_SLOT and IBL_TARGET_REG are defined. In such
   scenarios - replace direct register name with the semantic one to
   avoid confusion.
2. core/ir/decodelib.c and core/arch/arch.h: Two places utilizing same
   register fields and requiring synchronization. Instead define
   semantic name and use it in both places.
3. core/arch/arch.h: According to documentation dr_insert_call()
   register r11 is clobbered. However this works only because X86 and
   ARM both have a register named r11. New architectures might not, i.e.
   RISC-V names registers xN. Instead add a CALL_SCRATCH_REG definition
   inside arch.h and use this instead. Similarly it seems that if
   documentation would use semantic names, it would be easier to produce
   a single table appendix with mapping to concrete registers.

Issue: DynamoRIO#3544

Signed-off-by: Stanislaw Kardach <[email protected]>

core/arch/arch.h

@@ -118,6 +118,7 @@ mixed_mode_enabled(void)
 #    define SCRATCH_REG3_OFFS XDX_OFFSET
 #    define SCRATCH_REG4_OFFS XSI_OFFSET
 #    define SCRATCH_REG5_OFFS XDI_OFFSET
+#    define CALL_SCRATCH_REG DR_REG_R11
 #elif defined(AARCHXX)
 #    define R0_OFFSET ((MC_OFFS) + (offsetof(priv_mcontext_t, r0)))
 #    define REG0_OFFSET R0_OFFSET
@@ -151,6 +152,7 @@ mixed_mode_enabled(void)
 #    define SCRATCH_REG4_OFFS R4_OFFSET
 #    define SCRATCH_REG5_OFFS R5_OFFSET
 #    define REG_OFFSET(reg) (R0_OFFSET + ((reg)-DR_REG_R0) * sizeof(reg_t))
+#    define CALL_SCRATCH_REG DR_REG_R11
 #endif /* X86/ARM */
 #define XSP_OFFSET ((MC_OFFS) + (offsetof(priv_mcontext_t, xsp)))
 #define XFLAGS_OFFSET ((MC_OFFS) + (offsetof(priv_mcontext_t, xflags)))

core/arch/arch_exports.h

@@ -1674,8 +1674,8 @@ _dynamorio_runtime_resolve(void);
 #    define APP_PARAM(mc, offs) APP_PARAM_##offs(mc)
 #endif /* X86/ARM */
 
-#define MCXT_SYSNUM_REG(mc) ((mc)->IF_X86_ELSE(xax, IF_ARM_ELSE(r7, r8)))
-#define MCXT_FIRST_REG_FIELD(mc) ((mc)->IF_X86_ELSE(xdi, r0))
+#define MCXT_SYSNUM_REG(mc) ((mc)->MCXT_FLD_SYSNUM_REG)
+#define MCXT_FIRST_REG_FIELD(mc) ((mc)->MCXT_FLD_FIRST_REG)
 
 static inline reg_t
 get_mcontext_frame_ptr(dcontext_t *dcontext, priv_mcontext_t *mc)

core/arch/emit_utils_shared.c

@@ -2354,7 +2354,7 @@ append_call_dispatch(dcontext_t *dcontext, instrlist_t *ilist, bool absolute)
     /* d_r_dispatch() shouldn't return! */
     insert_reachable_cti(dcontext, ilist, NULL, vmcode_get_start(),
                          (byte *)unexpected_return, true /*jmp*/, false /*!returns*/,
-                         false /*!precise*/, DR_REG_R11 /*scratch*/, NULL);
+                         false /*!precise*/, CALL_SCRATCH_REG /*scratch*/, NULL);
 }
 
 /*
@@ -5271,7 +5271,7 @@ emit_new_thread_dynamo_start(dcontext_t *dcontext, byte *pc)
     /* should not return */
     insert_reachable_cti(dcontext, &ilist, NULL, vmcode_get_start(),
                          (byte *)unexpected_return, true /*jmp*/, false /*!returns*/,
-                         false /*!precise*/, DR_REG_R11 /*scratch*/, NULL);
+                         false /*!precise*/, CALL_SCRATCH_REG /*scratch*/, NULL);
 
     /* now encode the instructions */
     pc = instrlist_encode_to_copy(dcontext, &ilist, vmcode_get_writable_addr(pc), pc,

core/arch/mangle_shared.c

@@ -499,7 +499,7 @@ insert_meta_call_vargs(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
      */
     direct = insert_reachable_cti(dcontext, ilist, instr, encode_pc, (byte *)callee,
                                   false /*call*/, TEST(META_CALL_RETURNS, flags),
-                                  false /*!precise*/, DR_REG_R11, NULL);
+                                  false /*!precise*/, CALL_SCRATCH_REG, NULL);
     if (stack_for_params > 0) {
         /* XXX PR 245936: let user decide whether to clean up?
          * i.e., support calling a stdcall routine?

core/arch/x86/mangle.c

@@ -104,8 +104,8 @@ insert_out_of_line_context_switch(dcontext_t *dcontext, instrlist_t *ilist,
     insert_reachable_cti(dcontext, ilist, instr, encode_pc,
                          save ? get_clean_call_save(dcontext _IF_X64(GENCODE_X64))
                               : get_clean_call_restore(dcontext _IF_X64(GENCODE_X64)),
-                         false /*call*/, true /*returns*/, false /*!precise*/, DR_REG_R11,
-                         NULL);
+                         false /*call*/, true /*returns*/, false /*!precise*/,
+                         CALL_SCRATCH_REG, NULL);
     return get_clean_call_switch_stack_size();
 }
 

core/ir/decodelib.c

@@ -39,6 +39,7 @@
 
 #    include "../globals.h"
 #    include "instr.h"
+#    include "opnd.h"
 
 #    ifdef UNIX
 #        include <unistd.h>
@@ -153,7 +154,7 @@ proc_restore_fpstate(byte *buf)
 priv_mcontext_t *
 dr_mcontext_as_priv_mcontext(dr_mcontext_t *mc)
 {
-    return (priv_mcontext_t *)(&mc->IF_X86_ELSE(xdi, r0));
+    return (priv_mcontext_t *)(&mc->MCXT_FLD_FIRST_REG);
 }
 
 /* XXX: the code below is duplicated w/ only minor changes from utils.c.

core/ir/opnd.h

@@ -282,6 +282,18 @@ enum {
     REGPARM_END_ALIGN = 8,
 #endif
 };
+
+#ifdef X86
+#    define MCXT_FLD_FIRST_REG xdi
+#    define MCXT_FLD_SYSNUM_REG xax
+#elif defined(AARCHXX)
+#    define MCXT_FLD_FIRST_REG r0
+#    ifdef X64
+#    define MCXT_FLD_SYSNUM_REG r8
+#    else
+#    define MCXT_FLD_SYSNUM_REG r7
+#    endif /* 64/32 */
+#endif
 extern const reg_id_t d_r_regparms[];
 
 /* arch-specific */

core/translate.c

@@ -1976,9 +1976,7 @@ stress_test_recreate_state(dcontext_t *dcontext, fragment_t *f, instrlist_t *ili
                     (reg_t)d_r_get_tls(spill_ibreg_outstanding_offs) + 1;
             } else {
                 mc.IF_X86_ELSE(xcx, r2) =
-                    (reg_t)d_r_get_tls(os_tls_offset(
-                        (ushort)reg_spill_tls_offs(IF_X86_ELSE(DR_REG_XCX, DR_REG_R2)))) +
-                    1;
+                    (reg_t)d_r_get_tls(os_tls_offset((ushort)IBL_TARGET_SLOT)) + 1;
             }
             mc.xsp = STRESS_XSP_INIT;
             mc.pc = cpc;
@@ -1995,8 +1993,7 @@ stress_test_recreate_state(dcontext_t *dcontext, fragment_t *f, instrlist_t *ili
                 " vs " PFX "\n",
                 res, mc.pc, mc.xsp, STRESS_XSP_INIT - /*negate*/ xsp_adjust,
                 mc.IF_X86_ELSE(xcx, r2),
-                d_r_get_tls(os_tls_offset(
-                    (ushort)reg_spill_tls_offs(IF_X86_ELSE(DR_REG_XCX, DR_REG_R2)))));
+                d_r_get_tls(os_tls_offset((ushort)IBL_TARGET_SLOT)));
             /* We should only have failures at tail end of mangle regions.
              * No instrs after a failing instr should touch app memory.
              */
@@ -2020,7 +2017,7 @@ stress_test_recreate_state(dcontext_t *dcontext, fragment_t *f, instrlist_t *ili
             uint offs = UINT_MAX;
             if (instr_is_DR_reg_spill_or_restore(dcontext, in, NULL, &spill, &reg,
                                                  &offs) &&
-                reg == IF_X86_ELSE(DR_REG_XCX, DR_REG_R2)) {
+                reg == IBL_TARGET_REG) {
                 if (spill)
                     spill_ibreg_outstanding_offs = offs;
                 else