i#1569 AArch64: Distinguish SP (stack pointer) from ZR (zero register).

Both are encoded as "register 31", so they are never alternatives in the same encoding, but they can be alternatives in an alias. For example: mov x0, xzr is an alias for orr x0, xzr, xzr, lsl #0 mov x0, sp is an alias for add x0, sp, #0, lsl #0 Therefore it is best to clearly distinguish them, and to prevent accidents, make neither DR_REG_XZR nor DR_REG_XSP equal to (DR_REG_X30 + 1). At the same time, remove the AArch64 register enum constants from AArch32. Review-URL: https://codereview.appspot.com/298270043
DynamoRIO · May 27, 2016 · 6f02d27 · 6f02d27
1 parent a7909b2
commit 6f02d27
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 57 deletions.
diff --git a/core/arch/aarch64/encode.c b/core/arch/aarch64/encode.c
@@ -60,9 +60,10 @@ const reg_id_t dr_reg_fixer[] = {
     DR_REG_X16, DR_REG_X17,  DR_REG_X18,  DR_REG_X19, \
     DR_REG_X20, DR_REG_X21,  DR_REG_X22,  DR_REG_X23, \
     DR_REG_X24, DR_REG_X25,  DR_REG_X26,  DR_REG_X27, \
-    DR_REG_X28, DR_REG_X29,  DR_REG_X30,  DR_REG_X31,
-XREGS /* X0-X31 */
-XREGS /* W0-W31 */
+    DR_REG_X28, DR_REG_X29,  DR_REG_X30,  DR_REG_X31_INVALID, \
+    DR_REG_XZR, DR_REG_XSP,
+XREGS /* X0-XSP */
+XREGS /* W0-WSP */
 #undef XREGS
 
 #define QREGS \

diff --git a/core/arch/aarch64/instr.c b/core/arch/aarch64/instr.c
@@ -259,8 +259,7 @@ instr_predicate_is_cond(dr_pred_type_t pred)
 bool
 reg_is_gpr(reg_id_t reg)
 {
-    return ((DR_REG_X0 <= reg && reg <= DR_REG_X30) ||
-            (DR_REG_W0 <= reg && reg <= DR_REG_W30));
+    return (DR_REG_X0 <= reg && reg <= DR_REG_WSP);
 }
 
 bool

diff --git a/core/arch/arm/decode.c b/core/arch/arm/decode.c
@@ -822,7 +822,7 @@ decode_operand(decode_info_t *di, byte optype, opnd_size_t opsize, opnd_t *array
         if (*counter <= 0 || !opnd_is_reg(array[(*counter)-1]))
             return false;
         reg = opnd_get_reg(array[(*counter)-1]);
-        if (reg == DR_REG_STOP_32 || reg == DR_REG_STOP_64)
+        if (reg == DR_REG_STOP_32)
             return false;
         array[(*counter)++] = opnd_create_reg_ex(reg + 1, downsz, 0);
         return true;

diff --git a/core/arch/arm/encode.c b/core/arch/arm/encode.c
@@ -47,18 +47,8 @@
 const char * const reg_names[] = {
     "<NULL>",
     "<invalid>",
-    "x0",  "x1",   "x2",   "x3",    "x4",  "x5",   "x6",   "x7",
-    "x8",  "x9",   "x10",  "x11",  "x12", "x13",  "x14",  "x15",
-    "x16", "x17",  "x18",  "x19",  "x20", "x21",  "x22",  "x23",
-    "x24", "x25",  "x26",  "x27",  "x28", "x29",  "lr",   "sp", /* sometimes "xzr" */
-    "w0",  "w1",   "w2",   "w3",    "w4",  "w5",   "w6",   "w7",
-    "w8",  "w9",   "w10",  "w11",  "w12", "w13",  "w14",  "w15",
-    "w16", "w17",  "w18",  "w19",  "w20", "w21",  "w22",  "w23",
-    "w24", "w25",  "w26",  "w27",  "w28", "w29",  "w30",  "w31", /* sometimes "wzr" */
-#ifndef X64
     "r0",  "r1",   "r2",   "r3",    "r4",  "r5",   "r6",   "r7",
     "r8",  "r9",   "r10",  "r11",  "r12",  "sp",   "lr",   "pc",
-#endif
     "q0",  "q1",   "q2",   "q3",    "q4",  "q5",   "q6",   "q7",
     "q8",  "q9",   "q10",  "q11",  "q12", "q13",  "q14",  "q15",
     "q16", "q17",  "q18",  "q19",  "q20", "q21",  "q22",  "q23",
@@ -91,22 +81,6 @@ const char * const reg_names[] = {
 const reg_id_t dr_reg_fixer[] = {
     REG_NULL,
     REG_NULL,
-    DR_REG_X0,  DR_REG_X1,   DR_REG_X2,   DR_REG_X3,
-    DR_REG_X4,  DR_REG_X5,   DR_REG_X6,   DR_REG_X7,
-    DR_REG_X8,  DR_REG_X9,   DR_REG_X10,  DR_REG_X11,
-    DR_REG_X12, DR_REG_X13,  DR_REG_X14,  DR_REG_X15,
-    DR_REG_X16, DR_REG_X17,  DR_REG_X18,  DR_REG_X19,
-    DR_REG_X20, DR_REG_X21,  DR_REG_X22,  DR_REG_X23,
-    DR_REG_X24, DR_REG_X25,  DR_REG_X26,  DR_REG_X27,
-    DR_REG_X28, DR_REG_X29,  DR_REG_X30,  DR_REG_X31,
-    DR_REG_X0,  DR_REG_X1,   DR_REG_X2,   DR_REG_X3,
-    DR_REG_X4,  DR_REG_X5,   DR_REG_X6,   DR_REG_X7,
-    DR_REG_X8,  DR_REG_X9,   DR_REG_X10,  DR_REG_X11,
-    DR_REG_X12, DR_REG_X13,  DR_REG_X14,  DR_REG_X15,
-    DR_REG_X16, DR_REG_X17,  DR_REG_X18,  DR_REG_X19,
-    DR_REG_X20, DR_REG_X21,  DR_REG_X22,  DR_REG_X23,
-    DR_REG_X24, DR_REG_X25,  DR_REG_X26,  DR_REG_X27,
-    DR_REG_X28, DR_REG_X29,  DR_REG_X30,  DR_REG_X31,
     DR_REG_R0,  DR_REG_R1,   DR_REG_R2,   DR_REG_R3,
     DR_REG_R4,  DR_REG_R5,   DR_REG_R6,   DR_REG_R7,
     DR_REG_R8,  DR_REG_R9,   DR_REG_R10,  DR_REG_R11,

diff --git a/core/arch/arm/instr.c b/core/arch/arm/instr.c
@@ -701,7 +701,7 @@ instr_predicate_is_cond(dr_pred_type_t pred)
 bool
 reg_is_gpr(reg_id_t reg)
 {
-    return (reg >= DR_REG_X0 && reg < DR_REG_Q0);
+    return (DR_REG_R0 <= reg && reg <= DR_REG_R15);
 }
 
 bool

diff --git a/core/arch/opnd.h b/core/arch/opnd.h
@@ -182,6 +182,8 @@ enum {
     /****************************************************************************/
 #elif defined(ARM) || defined(AARCH64)
     DR_REG_INVALID, /**< Sentinel value indicating an invalid register. */
+
+# ifdef AARCH64
     /* 64-bit general purpose */
     DR_REG_X0,  DR_REG_X1,   DR_REG_X2,   DR_REG_X3,
     DR_REG_X4,  DR_REG_X5,   DR_REG_X6,   DR_REG_X7,
@@ -190,7 +192,9 @@ enum {
     DR_REG_X16, DR_REG_X17,  DR_REG_X18,  DR_REG_X19,
     DR_REG_X20, DR_REG_X21,  DR_REG_X22,  DR_REG_X23,
     DR_REG_X24, DR_REG_X25,  DR_REG_X26,  DR_REG_X27,
-    DR_REG_X28, DR_REG_X29,  DR_REG_X30,  DR_REG_X31,
+    DR_REG_X28, DR_REG_X29,  DR_REG_X30,  DR_REG_X31_INVALID,
+    DR_REG_XZR, /* zero register */
+    DR_REG_XSP, /* stack pointer */
 
     /* 32-bit general purpose */
     DR_REG_W0,  DR_REG_W1,   DR_REG_W2,   DR_REG_W3,
@@ -200,9 +204,10 @@ enum {
     DR_REG_W16, DR_REG_W17,  DR_REG_W18,  DR_REG_W19,
     DR_REG_W20, DR_REG_W21,  DR_REG_W22,  DR_REG_W23,
     DR_REG_W24, DR_REG_W25,  DR_REG_W26,  DR_REG_W27,
-    DR_REG_W28, DR_REG_W29,  DR_REG_W30,  DR_REG_W31,
-
-# ifndef X64
+    DR_REG_W28, DR_REG_W29,  DR_REG_W30,  DR_REG_W31_INVALID,
+    DR_REG_WZR, /* zero register */
+    DR_REG_WSP, /* bottom half of stack pointer */
+# else
     /* 32-bit general purpose */
     DR_REG_R0,  DR_REG_R1,   DR_REG_R2,   DR_REG_R3,
     DR_REG_R4,  DR_REG_R5,   DR_REG_R6,   DR_REG_R7,
@@ -257,7 +262,7 @@ enum {
     DR_REG_B24, DR_REG_B25,  DR_REG_B26,  DR_REG_B27,
     DR_REG_B28, DR_REG_B29,  DR_REG_B30,  DR_REG_B31,
 
-# ifndef X64
+# ifndef AARCH64
     /* Coprocessor registers */
     DR_REG_CR0,  DR_REG_CR1,  DR_REG_CR2,  DR_REG_CR3,
     DR_REG_CR4,  DR_REG_CR5,  DR_REG_CR6,  DR_REG_CR7,
@@ -284,7 +289,7 @@ enum {
      * OP_mrs and OP_msr to distinguish them and make things clearer.
      */
 # endif
-# ifdef X64
+# ifdef AARCH64
     DR_REG_NZCV, DR_REG_FPCR, DR_REG_FPSR,
 # else
     DR_REG_CPSR, DR_REG_SPSR, DR_REG_FPSCR,
@@ -296,7 +301,7 @@ enum {
 
     /* Aliases below here: */
 
-# ifdef X64
+# ifdef AARCH64
     DR_REG_R0  = DR_REG_X0,  /**< Alias for the x0 register. */
     DR_REG_R1  = DR_REG_X1,  /**< Alias for the x1 register. */
     DR_REG_R2  = DR_REG_X2,  /**< Alias for the x2 register. */
@@ -328,12 +333,8 @@ enum {
     DR_REG_R28 = DR_REG_X28, /**< Alias for the x28 register. */
     DR_REG_R29 = DR_REG_X29, /**< Alias for the x29 register. */
     DR_REG_R30 = DR_REG_X30, /**< Alias for the x30 register. */
-    DR_REG_R31 = DR_REG_X31, /**< Alias for the x31 register. */
-    DR_REG_SP  = DR_REG_X31, /**< The stack pointer register. */
+    DR_REG_SP  = DR_REG_XSP, /**< The stack pointer register. */
     DR_REG_LR  = DR_REG_X30, /**< The link register. */
-    DR_REG_XZR = DR_REG_X31, /**< The 64-bit zero register. */
-    DR_REG_WSP = DR_REG_W31, /**< The bottom half of the stack pointer register. */
-    DR_REG_WZR = DR_REG_W31, /**< The 32-bit zero register. */
 # else
     DR_REG_SP = DR_REG_R13, /**< The stack pointer register. */
     DR_REG_LR = DR_REG_R14, /**< The link register. */
@@ -342,7 +343,7 @@ enum {
     DR_REG_SL = DR_REG_R10, /**< Alias for the r10 register. */
     DR_REG_FP = DR_REG_R11, /**< Alias for the r11 register. */
     DR_REG_IP = DR_REG_R12, /**< Alias for the r12 register. */
-# ifndef X64
+# ifndef AARCH64
     /** Alias for cpsr register (thus this is the full cpsr, not just the apsr bits). */
     DR_REG_APSR = DR_REG_CPSR,
 # endif
@@ -356,27 +357,29 @@ enum {
     DR_REG_CP15_C13_2  = DR_REG_TPIDRURW, /**< User Read/Write Thread ID Register */
     DR_REG_CP15_C13_3  = DR_REG_TPIDRURO, /**< User Read-Olny Thread ID Register */
 
-    DR_NUM_GPR_REGS = IF_X64_ELSE(32, 16),
-
     DR_REG_LAST_VALID_ENUM = DR_REG_TPIDRURO, /**< Last valid register enum */
     DR_REG_LAST_ENUM = DR_REG_TPIDRURO, /**< Last value of register enums */
 
+# ifdef AARCH64
     DR_REG_START_64  = DR_REG_X0,  /**< Start of 64-bit general register enum values */
-    DR_REG_STOP_64   = DR_REG_X31, /**< End of 64-bit general register enum values */
-# ifdef X64
-    DR_REG_START_GPR = DR_REG_X0,  /**< Start of general register registers */
-    DR_REG_STOP_GPR  = DR_REG_X31, /**< End of general register registers */
+    DR_REG_STOP_64   = DR_REG_XSP, /**< End of 64-bit general register enum values */
     DR_REG_START_32  = DR_REG_W0,  /**< Start of 32-bit general register enum values */
-    DR_REG_STOP_32   = DR_REG_W31, /**< End of 32-bit general register enum values */
+    DR_REG_STOP_32   = DR_REG_WSP, /**< End of 32-bit general register enum values */
+    DR_REG_START_GPR = DR_REG_X0,  /**< Start of full-size general-purpose registers */
+    DR_REG_STOP_GPR  = DR_REG_XSP, /**< End of full-size general-purpose registers */
 # else
-    DR_REG_START_GPR = DR_REG_R0,  /**< Start of general register registers */
-    DR_REG_STOP_GPR  = DR_REG_R15, /**< End of general register registers */
     DR_REG_START_32  = DR_REG_R0,  /**< Start of 32-bit general register enum values */
     DR_REG_STOP_32   = DR_REG_R15, /**< End of 32-bit general register enum values */
+    DR_REG_START_GPR = DR_REG_R0,  /**< Start of general register registers */
+    DR_REG_STOP_GPR  = DR_REG_R15, /**< End of general register registers */
 # endif
 
+    DR_NUM_GPR_REGS = DR_REG_STOP_GPR - DR_REG_START_GPR + 1,
+
+# ifndef AARCH64
     /** Platform-independent way to refer to stack pointer. */
     DR_REG_XSP       = DR_REG_SP,
+# endif
 
 #endif /* X86/ARM */
 };
@@ -477,8 +480,10 @@ extern const reg_id_t dr_reg_fixer[];
 /* DR_API EXPORT VERBATIM */
 #define REG_NULL            DR_REG_NULL
 #define REG_INVALID         DR_REG_INVALID
-#define REG_START_64        DR_REG_START_64
-#define REG_STOP_64         DR_REG_STOP_64
+#ifndef ARM
+# define REG_START_64       DR_REG_START_64
+# define REG_STOP_64        DR_REG_STOP_64
+#endif
 #define REG_START_32        DR_REG_START_32
 #define REG_STOP_32         DR_REG_STOP_32
 #define REG_LAST_VALID_ENUM DR_REG_LAST_VALID_ENUM

diff --git a/core/arch/opnd_shared.c b/core/arch/opnd_shared.c
@@ -135,6 +135,7 @@ reg_is_32bit(reg_id_t reg)
     return (reg >= REG_START_32 && reg <= REG_STOP_32);
 }
 
+#if defined(X86) || defined(AARCH64)
 bool
 opnd_is_reg_64bit(opnd_t opnd)
 {
@@ -148,6 +149,7 @@ reg_is_64bit(reg_id_t reg)
 {
     return (reg >= REG_START_64 && reg <= REG_STOP_64);
 }
+#endif /* !ARM */
 
 bool
 opnd_is_reg_pointer_sized(opnd_t opnd)
-Original file line number
+Diff line change
@@ Expand Up / @@ -701,7 +701,7 @@ instr_predicate_is_cond(dr_pred_type_t pred) @@
     bool
     reg_is_gpr(reg_id_t reg)
     {
-        return (reg >= DR_REG_X0 && reg < DR_REG_Q0);
+        return (DR_REG_R0 <= reg && reg <= DR_REG_R15);
     }
     bool
@@ Expand Down @@